FEATURE: Add ability to dismiss admin notices (#28916)

his is a new feature that lets admins dismiss notices from the dashboard. This helps with self-service in cases where a notice is "stuck", while we work on provisions to prevent "sticking" in the first place.
This commit is contained in:
Ted Johansson 2024-09-17 14:43:34 +08:00 committed by GitHub
parent d7a46e1702
commit be33363f13
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
17 changed files with 231 additions and 67 deletions

View File

@ -0,0 +1,25 @@
import Component from "@glimmer/component";
import { action } from "@ember/object";
import { htmlSafe } from "@ember/template";
import DButton from "discourse/components/d-button";
import icon from "discourse-common/helpers/d-icon";
export default class AdminNotice extends Component {
@action
dismiss() {
this.args.dismissCallback(this.args.problem);
}
<template>
<div class="notice">
<div class="message">
{{if @icon (icon @icon)}}
{{htmlSafe @problem.message}}
</div>
<DButton
@action={{this.dismiss}}
@label="admin.dashboard.dismiss_notice"
/>
</div>
</template>
}

View File

@ -1,15 +1,33 @@
import Component from "@glimmer/component"; import Component from "@glimmer/component";
import { htmlSafe } from "@ember/template"; import { concat } from "@ember/helper";
import { action } from "@ember/object";
import { eq } from "truth-helpers";
import ConditionalLoadingSection from "discourse/components/conditional-loading-section"; import ConditionalLoadingSection from "discourse/components/conditional-loading-section";
import DButton from "discourse/components/d-button"; import DButton from "discourse/components/d-button";
import concatClass from "discourse/helpers/concat-class"; import concatClass from "discourse/helpers/concat-class";
import { ajax } from "discourse/lib/ajax";
import { popupAjaxError } from "discourse/lib/ajax-error";
import icon from "discourse-common/helpers/d-icon"; import icon from "discourse-common/helpers/d-icon";
import i18n from "discourse-common/helpers/i18n"; import i18n from "discourse-common/helpers/i18n";
import AdminNotice from "admin/components/admin-notice";
// eslint-disable-next-line ember/no-empty-glimmer-component-classes
export default class DashboardProblems extends Component { export default class DashboardProblems extends Component {
@action
async dismissProblem(problem) {
try {
await ajax(`/admin/admin_notices/${problem.id}`, { type: "DELETE" });
this.args.problems.removeObject(problem);
} catch (error) {
popupAjaxError(error);
}
}
get problems() {
return this.args.problems.sortBy("priority");
}
<template> <template>
{{#if @foundProblems}} {{#if @problems.length}}
<div class="section dashboard-problems"> <div class="section dashboard-problems">
<div class="section-title"> <div class="section-title">
<h2> <h2>
@ -20,35 +38,24 @@ export default class DashboardProblems extends Component {
<div class="section-body"> <div class="section-body">
<ConditionalLoadingSection @isLoading={{@loadingProblems}}> <ConditionalLoadingSection @isLoading={{@loadingProblems}}>
{{#if @highPriorityProblems.length}} <div class="problem-messages">
<div class="problem-messages priority-high">
<ul> <ul>
{{#each @highPriorityProblems as |problem|}} {{#each this.problems as |problem|}}
<li <li
class={{concatClass class={{concatClass
"dashboard-problem" "dashboard-problem"
"priority-" (concat "priority-" problem.priority)
problem.priority
}} }}
> >
{{icon "triangle-exclamation"}} <AdminNotice
{{htmlSafe problem.message}} @icon={{if
</li> (eq problem.priority "high")
{{/each}} "triangle-exclamation"
</ul>
</div>
{{/if}}
<div class="problem-messages priority-low">
<ul>
{{#each @lowPriorityProblems as |problem|}}
<li
class={{concatClass
"dashboard-problem "
"priority-"
problem.priority
}} }}
>{{htmlSafe problem.message}}</li> @problem={{problem}}
@dismissCallback={{this.dismissProblem}}
/>
</li>
{{/each}} {{/each}}
</ul> </ul>
</div> </div>

View File

@ -18,16 +18,6 @@ export default class AdminDashboardController extends Controller {
@setting("version_checks") showVersionChecks; @setting("version_checks") showVersionChecks;
@discourseComputed(
"lowPriorityProblems.length",
"highPriorityProblems.length"
)
foundProblems(lowPriorityProblemsLength, highPriorityProblemsLength) {
const problemsLength =
lowPriorityProblemsLength + highPriorityProblemsLength;
return this.currentUser.admin && problemsLength > 0;
}
@computed("siteSettings.dashboard_visible_tabs") @computed("siteSettings.dashboard_visible_tabs")
get visibleTabs() { get visibleTabs() {
return (this.siteSettings.dashboard_visible_tabs || "") return (this.siteSettings.dashboard_visible_tabs || "")
@ -106,16 +96,7 @@ export default class AdminDashboardController extends Controller {
}); });
AdminDashboard.fetchProblems() AdminDashboard.fetchProblems()
.then((model) => { .then((model) => this.set("problems", model.problems))
this.set(
"highPriorityProblems",
model.problems.filterBy("priority", "high")
);
this.set(
"lowPriorityProblems",
model.problems.filterBy("priority", "low")
);
})
.finally(() => this.set("loadingProblems", false)); .finally(() => this.set("loadingProblems", false));
} }

View File

@ -18,9 +18,7 @@
<DashboardProblems <DashboardProblems
@loadingProblems={{this.loadingProblems}} @loadingProblems={{this.loadingProblems}}
@foundProblems={{this.foundProblems}} @problems={{this.problems}}
@lowPriorityProblems={{this.lowPriorityProblems}}
@highPriorityProblems={{this.highPriorityProblems}}
@problemsTimestamp={{this.problemsTimestamp}} @problemsTimestamp={{this.problemsTimestamp}}
@refreshProblems={{action "refreshProblems"}} @refreshProblems={{action "refreshProblems"}}
/> />

View File

@ -4,7 +4,6 @@ import {
acceptance, acceptance,
count, count,
exists, exists,
query,
} from "discourse/tests/helpers/qunit-helpers"; } from "discourse/tests/helpers/qunit-helpers";
import selectKit from "discourse/tests/helpers/select-kit-helper"; import selectKit from "discourse/tests/helpers/select-kit-helper";
@ -60,13 +59,6 @@ acceptance("Dashboard", function (needs) {
exists(".admin-report.new-contributors"), exists(".admin-report.new-contributors"),
"new-contributors report" "new-contributors report"
); );
assert.strictEqual(
query(
".section.dashboard-problems .problem-messages ul li:first-child"
).innerHTML.trim(),
"Houston...",
"displays problems"
);
}); });
test("moderation tab", async function (assert) { test("moderation tab", async function (assert) {

View File

@ -246,16 +246,21 @@
.problem-messages { .problem-messages {
margin-bottom: 1em; margin-bottom: 1em;
&.priority-high {
background-color: var(--danger-low);
border: 1px solid var(--danger-medium);
}
ul { ul {
margin: 0 0 0 1.25em; margin: 0 0 0 1.25em;
li.dashboard-problem { li.dashboard-problem {
padding: 0.5em 0.5em; padding: 0.5em 0.5em;
.notice {
display: flex;
justify-content: space-between;
align-items: center;
}
.message {
margin-right: var(--space-4);
}
} }
} }
} }

View File

@ -0,0 +1,10 @@
# frozen_string_literal: true
class Admin::AdminNoticesController < Admin::StaffController
def destroy
AdminNotices::Dismiss.call do
on_success { render(json: success_json) }
on_failure { render(json: failed_json, status: 422) }
end
end
end

View File

@ -32,11 +32,14 @@ class ProblemCheckTracker < ActiveRecord::Base
end end
def no_problem!(next_run_at: nil) def no_problem!(next_run_at: nil)
reset
silence_the_alarm
end
def reset(next_run_at: nil)
now = Time.current now = Time.current
update!(blips: 0, last_run_at: now, last_success_at: now, next_run_at:) update!(blips: 0, last_run_at: now, last_success_at: now, next_run_at:)
silence_the_alarm
end end
def check def check

View File

@ -1,5 +1,5 @@
# frozen_string_literal: true # frozen_string_literal: true
class AdminNoticeSerializer < ApplicationSerializer class AdminNoticeSerializer < ApplicationSerializer
attributes :priority, :message, :identifier attributes :id, :priority, :message, :identifier
end end

View File

@ -0,0 +1,32 @@
# frozen_string_literal: true
class AdminNotices::Dismiss
include Service::Base
model :admin_notice
policy :invalid_access
transaction do
step :destroy
step :reset_problem_check
end
private
def fetch_admin_notice(id:)
AdminNotice.find_by(id: id)
end
def invalid_access(guardian:)
guardian.is_admin?
end
def destroy(admin_notice:)
admin_notice.destroy!
end
def reset_problem_check(admin_notice:)
ProblemCheckTracker.find_by(identifier: admin_notice.identifier)&.reset
end
end

View File

@ -5077,6 +5077,7 @@ en:
installed_version: "Installed" installed_version: "Installed"
latest_version: "Latest" latest_version: "Latest"
problems_found: "Some advice based on your current site settings" problems_found: "Some advice based on your current site settings"
dismiss_notice: "Dismiss"
new_features: new_features:
title: "What's new" title: "What's new"
subtitle: "We are releasing new features and improvements all the time. This page covers the highlights, but you can click 'Learn more' to see extensive release notes." subtitle: "We are releasing new features and improvements all the time. This page covers the highlights, but you can click 'Learn more' to see extensive release notes."

View File

@ -401,6 +401,8 @@ Discourse::Application.routes.draw do
collection { put "/" => "about#update" } collection { put "/" => "about#update" }
end end
end end
resources :admin_notices, only: %i[destroy], constraints: AdminConstraint.new
end # admin namespace end # admin namespace
get "email/unsubscribe/:key" => "email#unsubscribe", :as => "email_unsubscribe" get "email/unsubscribe/:key" => "email#unsubscribe", :as => "email_unsubscribe"

View File

@ -3,4 +3,5 @@
Fabricator(:admin_notice) do Fabricator(:admin_notice) do
priority { "low" } priority { "low" }
identifier { "test_notice" } identifier { "test_notice" }
subject { "problem" }
end end

View File

@ -212,4 +212,30 @@ RSpec.describe ProblemCheckTracker do
end end
end end
end end
describe "#reset" do
let(:problem_tracker) do
Fabricate(:problem_check_tracker, identifier: "twitter_login", **original_attributes)
end
let(:original_attributes) do
{
blips: 0,
last_problem_at: 1.week.ago,
last_success_at: Time.current,
last_run_at: 24.hours.ago,
next_run_at: nil,
}
end
let(:updated_attributes) { { blips: 0 } }
it do
freeze_time
expect { problem_tracker.reset(next_run_at: 24.hours.from_now) }.to change {
problem_tracker.attributes
}.to(hash_including(updated_attributes))
end
end
end end

View File

@ -0,0 +1,32 @@
# frozen_string_literal: true
RSpec.describe(AdminNotices::Dismiss) do
subject(:result) { described_class.call(id: admin_notice.id, guardian: current_user.guardian) }
let!(:admin_notice) { Fabricate(:admin_notice, identifier: "problem.test") }
let!(:problem_check) { Fabricate(:problem_check_tracker, identifier: "problem.test", blips: 3) }
context "when user is not allowed to perform the action" do
fab!(:current_user) { Fabricate(:user) }
it { is_expected.to fail_a_policy(:invalid_access) }
end
context "when user is allowed to perform the action" do
fab!(:current_user) { Fabricate(:admin) }
it { is_expected.to run_successfully }
it "sets the service result as successful" do
expect(result).to be_a_success
end
it "destroys the admin notice" do
expect { result }.to change { AdminNotice.count }.from(1).to(0)
end
it "resets any associated problem check" do
expect { result }.to change { problem_check.reload.blips }.from(3).to(0)
end
end
end

View File

@ -0,0 +1,25 @@
# frozen_string_literal: true
describe "Admin Notices", type: :system do
fab!(:admin)
let(:admin_dashboard) { PageObjects::Pages::AdminDashboard.new }
before do
Fabricate(:admin_notice)
I18n.backend.store_translations(:en, dashboard: { problem: { test_notice: "Houston" } })
sign_in(admin)
end
it "supports dismissing admin notices" do
admin_dashboard.visit
expect(admin_dashboard).to have_admin_notice(I18n.t("dashboard.problem.test_notice"))
admin_dashboard.dismiss_notice(I18n.t("dashboard.problem.test_notice"))
expect(admin_dashboard).to have_no_admin_notice(I18n.t("dashboard.problem.test_notice"))
end
end

View File

@ -0,0 +1,24 @@
# frozen_string_literal: true
module PageObjects
module Pages
class AdminDashboard < PageObjects::Pages::Base
def visit
page.visit("/admin")
self
end
def has_admin_notice?(message)
has_css?(".dashboard-problem", text: message)
end
def has_no_admin_notice?(message)
has_no_css?(".dashboard-problem", text: message)
end
def dismiss_notice(message)
find(".dashboard-problem", text: message).find(".btn").click
end
end
end
end