From dc2a0854b05083807499f8e80cff64aa3f901580 Mon Sep 17 00:00:00 2001 From: Krzysztof Kotlarek Date: Mon, 27 Nov 2023 09:32:28 +1100 Subject: [PATCH] UX: add gift emoji styling for new features (#24523) When admin has unseen new feature, gift emoji is added to a link. In addition, `/new-features` path was changed to `/whats-new` --- .../components/dashboard-new-features.gjs | 2 +- .../addon/controllers/admin-dashboard.js | 1 + .../admin/addon/models/admin-dashboard.js | 5 +- .../admin/addon/routes/admin-route-map.js | 2 +- .../admin/addon/templates/dashboard.hbs | 3 + .../lib/notification-types/new-features.js | 2 +- .../tests/acceptance/dashboard-test.js | 5 ++ ...new-features.js => dashboard-whats-new.js} | 2 +- .../discourse/tests/fixtures/dashboard.js | 1 + .../stylesheets/common/admin/dashboard.scss | 4 + app/assets/stylesheets/mobile/dashboard.scss | 7 +- app/controllers/admin/dashboard_controller.rb | 7 +- config/routes.rb | 3 +- .../admin/dashboard_controller_spec.rb | 80 +++++++------------ .../pages/admin_dashboard_new_features.rb | 2 +- 15 files changed, 64 insertions(+), 62 deletions(-) rename app/assets/javascripts/discourse/tests/fixtures/{dashboard-new-features.js => dashboard-whats-new.js} (96%) diff --git a/app/assets/javascripts/admin/addon/components/dashboard-new-features.gjs b/app/assets/javascripts/admin/addon/components/dashboard-new-features.gjs index d3e16110b62..4dd0c41c8be 100644 --- a/app/assets/javascripts/admin/addon/components/dashboard-new-features.gjs +++ b/app/assets/javascripts/admin/addon/components/dashboard-new-features.gjs @@ -13,7 +13,7 @@ export default class DashboardNewFeatures extends Component { @bind loadNewFeatures() { - ajax("/admin/dashboard/new-features.json") + ajax("/admin/dashboard/whats-new.json") .then((json) => { this.newFeatures = json.new_features; this.isLoaded = true; diff --git a/app/assets/javascripts/admin/addon/controllers/admin-dashboard.js b/app/assets/javascripts/admin/addon/controllers/admin-dashboard.js index 0086624e764..c1d2d1da261 100644 --- a/app/assets/javascripts/admin/addon/controllers/admin-dashboard.js +++ b/app/assets/javascripts/admin/addon/controllers/admin-dashboard.js @@ -91,6 +91,7 @@ export default class AdminDashboardController extends Controller { if (versionChecks) { properties.versionCheck = VersionCheck.create(model.version_check); } + properties.hasUnseenFeatures = model.hasUnseenFeatures; this.setProperties(properties); }) diff --git a/app/assets/javascripts/admin/addon/models/admin-dashboard.js b/app/assets/javascripts/admin/addon/models/admin-dashboard.js index c261a424411..2850d879db0 100644 --- a/app/assets/javascripts/admin/addon/models/admin-dashboard.js +++ b/app/assets/javascripts/admin/addon/models/admin-dashboard.js @@ -11,7 +11,10 @@ export default class AdminDashboard extends EmberObject { static fetch() { return ajax("/admin/dashboard.json").then((json) => { const model = AdminDashboard.create(); - model.set("version_check", json.version_check); + model.setProperties({ + version_check: json.version_check, + hasUnseenFeatures: json.has_unseen_features, + }); return model; }); diff --git a/app/assets/javascripts/admin/addon/routes/admin-route-map.js b/app/assets/javascripts/admin/addon/routes/admin-route-map.js index e615cddd209..1cbe6e46723 100644 --- a/app/assets/javascripts/admin/addon/routes/admin-route-map.js +++ b/app/assets/javascripts/admin/addon/routes/admin-route-map.js @@ -15,7 +15,7 @@ export default function () { resetNamespace: true, }); this.route("admin.dashboardNewFeatures", { - path: "/dashboard/new-features", + path: "/dashboard/whats-new", resetNamespace: true, }); }); diff --git a/app/assets/javascripts/admin/addon/templates/dashboard.hbs b/app/assets/javascripts/admin/addon/templates/dashboard.hbs index 60b72555e21..fabca7301b5 100644 --- a/app/assets/javascripts/admin/addon/templates/dashboard.hbs +++ b/app/assets/javascripts/admin/addon/templates/dashboard.hbs @@ -53,6 +53,9 @@ {{#if this.isNewFeaturesTabVisible}}
  • + {{#if this.hasUnseenFeatures}} + {{replace-emoji ":gift:"}} + {{/if}} {{i18n "admin.dashboard.new_features.title"}}
    • diff --git a/app/assets/javascripts/discourse/app/lib/notification-types/new-features.js b/app/assets/javascripts/discourse/app/lib/notification-types/new-features.js index 91a819a96f8..95dec640509 100644 --- a/app/assets/javascripts/discourse/app/lib/notification-types/new-features.js +++ b/app/assets/javascripts/discourse/app/lib/notification-types/new-features.js @@ -12,7 +12,7 @@ export default class extends NotificationTypeBase { } get linkHref() { - return getURL("/admin/dashboard/new-features"); + return getURL("/admin/dashboard/whats-new"); } get icon() { diff --git a/app/assets/javascripts/discourse/tests/acceptance/dashboard-test.js b/app/assets/javascripts/discourse/tests/acceptance/dashboard-test.js index 59377e1ebe9..e00827b0813 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/dashboard-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/dashboard-test.js @@ -142,6 +142,11 @@ acceptance("Dashboard", function (needs) { await click(".dashboard .navigation-item.new-features .navigation-link"); + assert.ok( + exists( + ".dashboard .navigation-item.new-features .navigation-link .emoji[title='gift']" + ) + ); assert.ok(exists(".dashboard-new-features")); assert.ok(exists("img.admin-new-feature-item__screenshot")); }); diff --git a/app/assets/javascripts/discourse/tests/fixtures/dashboard-new-features.js b/app/assets/javascripts/discourse/tests/fixtures/dashboard-whats-new.js similarity index 96% rename from app/assets/javascripts/discourse/tests/fixtures/dashboard-new-features.js rename to app/assets/javascripts/discourse/tests/fixtures/dashboard-whats-new.js index babd3d897d3..dac369154a2 100644 --- a/app/assets/javascripts/discourse/tests/fixtures/dashboard-new-features.js +++ b/app/assets/javascripts/discourse/tests/fixtures/dashboard-whats-new.js @@ -1,5 +1,5 @@ export default { - "/admin/dashboard/new-features.json": { + "/admin/dashboard/whats-new.json": { new_features: [ { id: 1, diff --git a/app/assets/javascripts/discourse/tests/fixtures/dashboard.js b/app/assets/javascripts/discourse/tests/fixtures/dashboard.js index cab9154ee32..036e557f50f 100644 --- a/app/assets/javascripts/discourse/tests/fixtures/dashboard.js +++ b/app/assets/javascripts/discourse/tests/fixtures/dashboard.js @@ -1,5 +1,6 @@ export default { "/admin/dashboard.json": { updated_at: "2018-04-25T08:06:11.292Z", + has_unseen_features: true }, }; diff --git a/app/assets/stylesheets/common/admin/dashboard.scss b/app/assets/stylesheets/common/admin/dashboard.scss index f367dac3d86..545eddc12b3 100644 --- a/app/assets/stylesheets/common/admin/dashboard.scss +++ b/app/assets/stylesheets/common/admin/dashboard.scss @@ -24,6 +24,10 @@ display: block; font-weight: bold; padding: 0.6em 1em 0.5em 1em; + + .emoji { + margin-right: 0.5em; + } } } diff --git a/app/assets/stylesheets/mobile/dashboard.scss b/app/assets/stylesheets/mobile/dashboard.scss index 9db867cfdd5..c65cc34f44f 100644 --- a/app/assets/stylesheets/mobile/dashboard.scss +++ b/app/assets/stylesheets/mobile/dashboard.scss @@ -4,8 +4,13 @@ font-size: var(--font-down-1); } .navigation a.navigation-link { - padding: 0.5em; + padding: 0.4em; font-size: var(--font-down-1); + .emoji { + width: 1.3em; + height: 1.3em; + margin-right: 0; + } } .dashboard-new-features .section-body { grid-template-columns: none; diff --git a/app/controllers/admin/dashboard_controller.rb b/app/controllers/admin/dashboard_controller.rb index d2ed26665dd..ae42623b313 100644 --- a/app/controllers/admin/dashboard_controller.rb +++ b/app/controllers/admin/dashboard_controller.rb @@ -7,6 +7,7 @@ class Admin::DashboardController < Admin::StaffController if SiteSetting.version_checks? data.merge!(version_check: DiscourseUpdates.check_version.as_json) end + data.merge!(has_unseen_features: DiscourseUpdates.has_unseen_features?(current_user.id)) render json: data end @@ -38,11 +39,15 @@ class Admin::DashboardController < Admin::StaffController has_unseen_features: DiscourseUpdates.has_unseen_features?(current_user.id), release_notes_link: AdminDashboardGeneralData.fetch_cached_stats["release_notes_link"], } + + mark_new_features_as_seen + render json: data end + private + def mark_new_features_as_seen DiscourseUpdates.mark_new_features_as_seen(current_user.id) - render json: success_json end end diff --git a/config/routes.rb b/config/routes.rb index 165c71a8356..663a8f8d604 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -320,8 +320,7 @@ Discourse::Application.routes.draw do get "dashboard/moderation" => "dashboard#moderation" get "dashboard/security" => "dashboard#security" get "dashboard/reports" => "dashboard#reports" - get "dashboard/new-features" => "dashboard#new_features" - put "dashboard/mark-new-features-as-seen" => "dashboard#mark_new_features_as_seen" + get "dashboard/whats-new" => "dashboard#new_features" resources :dashboard, only: [:index] do collection { get "problems" } diff --git a/spec/requests/admin/dashboard_controller_spec.rb b/spec/requests/admin/dashboard_controller_spec.rb index edc651d8d93..ef1772e0c9d 100644 --- a/spec/requests/admin/dashboard_controller_spec.rb +++ b/spec/requests/admin/dashboard_controller_spec.rb @@ -164,7 +164,7 @@ RSpec.describe Admin::DashboardController do before { sign_in(admin) } it "is empty by default" do - get "/admin/dashboard/new-features.json" + get "/admin/dashboard/whats-new.json" expect(response.status).to eq(200) json = response.parsed_body expect(json["new_features"]).to eq(nil) @@ -172,7 +172,7 @@ RSpec.describe Admin::DashboardController do it "fails gracefully for invalid JSON" do Discourse.redis.set("new_features", "INVALID JSON") - get "/admin/dashboard/new-features.json" + get "/admin/dashboard/whats-new.json" expect(response.status).to eq(200) json = response.parsed_body expect(json["new_features"]).to eq(nil) @@ -181,7 +181,7 @@ RSpec.describe Admin::DashboardController do it "includes new features when available" do populate_new_features - get "/admin/dashboard/new-features.json" + get "/admin/dashboard/whats-new.json" expect(response.status).to eq(200) json = response.parsed_body @@ -195,7 +195,7 @@ RSpec.describe Admin::DashboardController do populate_new_features DiscourseUpdates.mark_new_features_as_seen(admin.id) - get "/admin/dashboard/new-features.json" + get "/admin/dashboard/whats-new.json" expect(response.status).to eq(200) json = response.parsed_body @@ -209,7 +209,7 @@ RSpec.describe Admin::DashboardController do expect(DiscourseUpdates.get_last_viewed_feature_date(admin.id)).to eq(nil) - get "/admin/dashboard/new-features.json" + get "/admin/dashboard/whats-new.json" expect(response.status).to eq(200) expect(DiscourseUpdates.get_last_viewed_feature_date(admin.id)).to be_within_one_second_of( date2, @@ -218,15 +218,33 @@ RSpec.describe Admin::DashboardController do date2 = 10.minutes.ago populate_new_features(date1, date2) - get "/admin/dashboard/new-features.json" + get "/admin/dashboard/whats-new.json" expect(response.status).to eq(200) expect(DiscourseUpdates.get_last_viewed_feature_date(admin.id)).to be_within_one_second_of( date2, ) end + it "marks new features as seen" do + date1 = 30.minutes.ago + date2 = 20.minutes.ago + populate_new_features(date1, date2) + + expect(DiscourseUpdates.new_features_last_seen(admin.id)).to eq(nil) + expect(DiscourseUpdates.has_unseen_features?(admin.id)).to eq(true) + + get "/admin/dashboard/whats-new.json" + expect(response.status).to eq(200) + + expect(DiscourseUpdates.new_features_last_seen(admin.id)).not_to eq(nil) + expect(DiscourseUpdates.has_unseen_features?(admin.id)).to eq(false) + + expect(DiscourseUpdates.new_features_last_seen(moderator.id)).to eq(nil) + expect(DiscourseUpdates.has_unseen_features?(moderator.id)).to eq(true) + end + it "doesn't error when there are no new features" do - get "/admin/dashboard/new-features.json" + get "/admin/dashboard/whats-new.json" expect(response.status).to eq(200) end end @@ -237,7 +255,7 @@ RSpec.describe Admin::DashboardController do it "includes new features when available" do populate_new_features - get "/admin/dashboard/new-features.json" + get "/admin/dashboard/whats-new.json" json = response.parsed_body @@ -252,7 +270,7 @@ RSpec.describe Admin::DashboardController do expect(DiscourseUpdates.get_last_viewed_feature_date(moderator.id)).to eq(nil) - get "/admin/dashboard/new-features.json" + get "/admin/dashboard/whats-new.json" expect(response.status).to eq(200) expect(DiscourseUpdates.get_last_viewed_feature_date(moderator.id)).to eq(nil) end @@ -262,49 +280,7 @@ RSpec.describe Admin::DashboardController do before { sign_in(user) } it "denies access with a 404 response" do - get "/admin/dashboard/new-features.json" - - expect(response.status).to eq(404) - expect(response.parsed_body["errors"]).to include(I18n.t("not_found")) - end - end - end - - describe "#mark_new_features_as_seen" do - after { DiscourseUpdates.clean_state } - - context "when logged in as an admin" do - before { sign_in(admin) } - - it "resets last seen for a given user" do - populate_new_features - put "/admin/dashboard/mark-new-features-as-seen.json" - - expect(response.status).to eq(200) - expect(DiscourseUpdates.new_features_last_seen(admin.id)).not_to eq(nil) - expect(DiscourseUpdates.has_unseen_features?(admin.id)).to eq(false) - end - end - - context "when logged in as a moderator" do - before { sign_in(moderator) } - - it "resets last seen for moderator" do - populate_new_features - - put "/admin/dashboard/mark-new-features-as-seen.json" - - expect(response.status).to eq(200) - expect(DiscourseUpdates.new_features_last_seen(moderator.id)).not_to eq(nil) - expect(DiscourseUpdates.has_unseen_features?(moderator.id)).to eq(false) - end - end - - context "when logged in as a non-staff user" do - before { sign_in(user) } - - it "prevents marking new feature as seen with a 404 response" do - put "/admin/dashboard/mark-new-features-as-seen.json" + get "/admin/dashboard/whats-new.json" expect(response.status).to eq(404) expect(response.parsed_body["errors"]).to include(I18n.t("not_found")) diff --git a/spec/system/page_objects/pages/admin_dashboard_new_features.rb b/spec/system/page_objects/pages/admin_dashboard_new_features.rb index ffce8cdf93b..21ace47e677 100644 --- a/spec/system/page_objects/pages/admin_dashboard_new_features.rb +++ b/spec/system/page_objects/pages/admin_dashboard_new_features.rb @@ -4,7 +4,7 @@ module PageObjects module Pages class AdminDashboardNewFeatures < PageObjects::Pages::Base def visit - page.visit("/admin/dashboard/new-features") + page.visit("/admin/dashboard/whats-new") self end