From 3e5976f8436255185de556ae165d9d849ac8fb3a Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Fri, 16 Aug 2024 09:12:24 +1000 Subject: [PATCH] FEATURE: Always show full page "New Features" to admins (#28383) We used to show New Features in a tab on the dashboard, but this could get pushed down the page especially on our hosting. In https://github.com/discourse/discourse/commit/043117ca1389b8d7b564a7cfb9d98aeb86403ea7 we made a separate What's New page, so this commit removes the dashboard tab and changes the admin notification to send the admin to /admin/whats-new instead of the dashboard tab. --- .../components/dashboard-new-features.gjs | 2 +- .../admin-dashboard-new-features.js | 3 --- .../addon/controllers/admin-dashboard.js | 6 ----- .../admin/addon/models/admin-dashboard.js | 1 - .../routes/admin-dashboard-new-features.js | 3 --- .../admin/addon/routes/admin-route-map.js | 4 ---- .../admin/addon/templates/dashboard.hbs | 11 ---------- .../lib/notification-types/new-features.js | 2 +- .../tests/acceptance/dashboard-test.js | 14 ------------ .../tests/fixtures/dashboard-whats-new.js | 2 +- app/controllers/admin/dashboard_controller.rb | 1 - .../admin/dashboard_controller_spec.rb | 22 +++++++++---------- .../admin_dashboard_new_features_spec.rb | 4 ++-- ..._new_features.rb => admin_new_features.rb} | 4 ++-- 14 files changed, 18 insertions(+), 61 deletions(-) delete mode 100644 app/assets/javascripts/admin/addon/controllers/admin-dashboard-new-features.js delete mode 100644 app/assets/javascripts/admin/addon/routes/admin-dashboard-new-features.js rename spec/system/page_objects/pages/{admin_dashboard_new_features.rb => admin_new_features.rb} (89%) 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 508469e671f..38b28f09faf 100644 --- a/app/assets/javascripts/admin/addon/components/dashboard-new-features.gjs +++ b/app/assets/javascripts/admin/addon/components/dashboard-new-features.gjs @@ -18,7 +18,7 @@ export default class DashboardNewFeatures extends Component { @bind loadNewFeatures() { - ajax("/admin/dashboard/whats-new.json") + ajax("/admin/whats-new.json") .then((json) => { const items = json.new_features.reduce((acc, feature) => { const key = moment(feature.released_at || feature.created_at).format( diff --git a/app/assets/javascripts/admin/addon/controllers/admin-dashboard-new-features.js b/app/assets/javascripts/admin/addon/controllers/admin-dashboard-new-features.js deleted file mode 100644 index 946e1a03641..00000000000 --- a/app/assets/javascripts/admin/addon/controllers/admin-dashboard-new-features.js +++ /dev/null @@ -1,3 +0,0 @@ -import Controller from "@ember/controller"; - -export default class AdminDashboardNewFeaturesController extends Controller {} diff --git a/app/assets/javascripts/admin/addon/controllers/admin-dashboard.js b/app/assets/javascripts/admin/addon/controllers/admin-dashboard.js index e6130b47c3a..89a6a52ad19 100644 --- a/app/assets/javascripts/admin/addon/controllers/admin-dashboard.js +++ b/app/assets/javascripts/admin/addon/controllers/admin-dashboard.js @@ -50,11 +50,6 @@ export default class AdminDashboardController extends Controller { return this.visibleTabs.includes("reports"); } - @computed("visibleTabs") - get isNewFeaturesTabVisible() { - return this.visibleTabs.includes("features"); - } - fetchProblems() { if (this.isLoadingProblems) { return; @@ -91,7 +86,6 @@ 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 2850d879db0..3fcae043f67 100644 --- a/app/assets/javascripts/admin/addon/models/admin-dashboard.js +++ b/app/assets/javascripts/admin/addon/models/admin-dashboard.js @@ -13,7 +13,6 @@ export default class AdminDashboard extends EmberObject { const model = AdminDashboard.create(); model.setProperties({ version_check: json.version_check, - hasUnseenFeatures: json.has_unseen_features, }); return model; diff --git a/app/assets/javascripts/admin/addon/routes/admin-dashboard-new-features.js b/app/assets/javascripts/admin/addon/routes/admin-dashboard-new-features.js deleted file mode 100644 index 3141e23a6b2..00000000000 --- a/app/assets/javascripts/admin/addon/routes/admin-dashboard-new-features.js +++ /dev/null @@ -1,3 +0,0 @@ -import DiscourseRoute from "discourse/routes/discourse"; - -export default class AdminDashboardNewFeaturesRoute extends DiscourseRoute {} 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 3c3f963604d..5e8355fca95 100644 --- a/app/assets/javascripts/admin/addon/routes/admin-route-map.js +++ b/app/assets/javascripts/admin/addon/routes/admin-route-map.js @@ -14,10 +14,6 @@ export default function () { path: "/dashboard/reports", resetNamespace: true, }); - this.route("admin.dashboardNewFeatures", { - path: "/dashboard/whats-new", - resetNamespace: true, - }); }); this.route( diff --git a/app/assets/javascripts/admin/addon/templates/dashboard.hbs b/app/assets/javascripts/admin/addon/templates/dashboard.hbs index b6a96fa50a4..906d43ae065 100644 --- a/app/assets/javascripts/admin/addon/templates/dashboard.hbs +++ b/app/assets/javascripts/admin/addon/templates/dashboard.hbs @@ -50,17 +50,6 @@ {{/if}} - {{#if this.isNewFeaturesTabVisible}} -
  • - - {{#if this.hasUnseenFeatures}} - {{replace-emoji ":gift:"}} - {{/if}} - {{i18n "admin.dashboard.new_features.title"}} - -
    • - {{/if}} - 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 95dec640509..4955bec4818 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/whats-new"); + return getURL("/admin/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 ab25c9b3949..270acddc74c 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/dashboard-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/dashboard-test.js @@ -130,20 +130,6 @@ acceptance("Dashboard", function (needs) { "its set the value of the filter from the query params" ); }); - - test("new features", async function (assert) { - await visit("/admin"); - - 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")); - }); }); acceptance("Dashboard: dashboard_visible_tabs", function (needs) { diff --git a/app/assets/javascripts/discourse/tests/fixtures/dashboard-whats-new.js b/app/assets/javascripts/discourse/tests/fixtures/dashboard-whats-new.js index dac369154a2..ec14e25bc73 100644 --- a/app/assets/javascripts/discourse/tests/fixtures/dashboard-whats-new.js +++ b/app/assets/javascripts/discourse/tests/fixtures/dashboard-whats-new.js @@ -1,5 +1,5 @@ export default { - "/admin/dashboard/whats-new.json": { + "/admin/whats-new.json": { new_features: [ { id: 1, diff --git a/app/controllers/admin/dashboard_controller.rb b/app/controllers/admin/dashboard_controller.rb index 6d2014a1df6..0bbeabd950c 100644 --- a/app/controllers/admin/dashboard_controller.rb +++ b/app/controllers/admin/dashboard_controller.rb @@ -7,7 +7,6 @@ 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 diff --git a/spec/requests/admin/dashboard_controller_spec.rb b/spec/requests/admin/dashboard_controller_spec.rb index 48b33877665..1847e41a7b4 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/whats-new.json" + get "/admin/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/whats-new.json" + get "/admin/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/whats-new.json" + get "/admin/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/whats-new.json" + get "/admin/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/whats-new.json" + get "/admin/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,7 +218,7 @@ RSpec.describe Admin::DashboardController do date2 = 10.minutes.ago populate_new_features(date1, date2) - get "/admin/dashboard/whats-new.json" + get "/admin/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, @@ -233,7 +233,7 @@ RSpec.describe Admin::DashboardController do 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" + get "/admin/whats-new.json" expect(response.status).to eq(200) expect(DiscourseUpdates.new_features_last_seen(admin.id)).not_to eq(nil) @@ -244,7 +244,7 @@ RSpec.describe Admin::DashboardController do end it "doesn't error when there are no new features" do - get "/admin/dashboard/whats-new.json" + get "/admin/whats-new.json" expect(response.status).to eq(200) end end @@ -255,7 +255,7 @@ RSpec.describe Admin::DashboardController do it "includes new features when available" do populate_new_features - get "/admin/dashboard/whats-new.json" + get "/admin/whats-new.json" json = response.parsed_body @@ -270,7 +270,7 @@ RSpec.describe Admin::DashboardController do expect(DiscourseUpdates.get_last_viewed_feature_date(moderator.id)).to eq(nil) - get "/admin/dashboard/whats-new.json" + get "/admin/whats-new.json" expect(response.status).to eq(200) expect(DiscourseUpdates.get_last_viewed_feature_date(moderator.id)).to eq(nil) end @@ -280,7 +280,7 @@ RSpec.describe Admin::DashboardController do before { sign_in(user) } it "denies access with a 404 response" do - get "/admin/dashboard/whats-new.json" + get "/admin/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/admin_dashboard_new_features_spec.rb b/spec/system/admin_dashboard_new_features_spec.rb index 3fde15e6b35..1f86cf8e17f 100644 --- a/spec/system/admin_dashboard_new_features_spec.rb +++ b/spec/system/admin_dashboard_new_features_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true -describe "Admin Dashboard New Features Page", type: :system do - let(:new_features_page) { PageObjects::Pages::AdminDashboardNewFeatures.new } +describe "Admin New Features Page", type: :system do + let(:new_features_page) { PageObjects::Pages::AdminNewFeatures.new } fab!(:admin) before { sign_in(admin) } diff --git a/spec/system/page_objects/pages/admin_dashboard_new_features.rb b/spec/system/page_objects/pages/admin_new_features.rb similarity index 89% rename from spec/system/page_objects/pages/admin_dashboard_new_features.rb rename to spec/system/page_objects/pages/admin_new_features.rb index 3b7ed50ce38..dfa9600e6d3 100644 --- a/spec/system/page_objects/pages/admin_dashboard_new_features.rb +++ b/spec/system/page_objects/pages/admin_new_features.rb @@ -2,9 +2,9 @@ module PageObjects module Pages - class AdminDashboardNewFeatures < PageObjects::Pages::Base + class AdminNewFeatures < PageObjects::Pages::Base def visit - page.visit("/admin/dashboard/whats-new") + page.visit("/admin/whats-new") self end