FIX: Live update upcoming change body classes (#36748)

Followup 2fdc9af28e

We need to take into account the admin changing client
site settings, which sends a MessageBus evvent to the
client to update the siteSettings service. We need to
both:

* Update siteSettings.currentUserUpcomingChanges to keep it
  in sync
* Use siteSettings[name] in upcomingChangeBodyClasses to
  ensure we always have the latest calculated value for
  the classes

This commit fixes both issues and adds a related system spec.

frontend/discourse/app/controllers/application.js

@@ -34,13 +34,12 @@ export default class ApplicationController extends Controller {
 
     const classes = [];
 
-    for (const [key, value] of Object.entries(
-      this.siteSettings.currentUserUpcomingChanges
-    )) {
-      if (value) {
+    Object.keys(this.siteSettings.currentUserUpcomingChanges).forEach((key) => {
+      if (this.siteSettings[key]) {
         classes.push(`uc-${dasherize(key)}`);
       }
-    }
+    });
+
     return classes.join(" ");
   }
 

frontend/discourse/app/instance-initializers/subscribe-user-notifications.js

@@ -270,6 +270,12 @@ class SubscribeUserNotificationsInit {
     }
 
     this.siteSettings[data.name] = data.value;
+
+    if (
+      this.siteSettings.currentUserUpcomingChanges.hasOwnProperty(data.name)
+    ) {
+      this.siteSettings.currentUserUpcomingChanges[data.name] = data.value;
+    }
   }
 
   @bind

frontend/discourse/app/services/site-settings.js

@@ -36,9 +36,11 @@ export function createSiteSettingsFromPreloaded(
 
     // Includes upcoming changes which apply to the anon user (Everyone changes)
     settings.currentUserUpcomingChanges = upcomingChanges;
+  } else {
+    settings.currentUserUpcomingChanges = {};
   }
 
-  // localize locale names here as they are not localized in the backend
+  // Localize locale names here as they are not localized in the backend
   // due to initialization order and caching
   if (settings.available_locales) {
     const locales = JSON.parse(settings.available_locales);

spec/system/admin_upcoming_changes_spec.rb

@@ -55,12 +55,7 @@ describe "Admin upcoming changes", type: :system do
 
     expect(upcoming_changes_page.change_item(:enable_upload_debug_mode)).to be_disabled
     upcoming_changes_page.change_item(:enable_upload_debug_mode).select_enabled_for("everyone")
-    expect(page).to have_content(
-      I18n.t(
-        "admin_js.admin.upcoming_changes.change_enabled_for_success",
-        enabledFor: I18n.t("admin_js.admin.upcoming_changes.enabled_for_options.everyone").downcase,
-      ),
-    )
+    expect(upcoming_changes_page).to have_enabled_for_success_toast("everyone")
     expect(upcoming_changes_page.change_item(:enable_upload_debug_mode)).to be_enabled
     expect(SiteSetting.enable_upload_debug_mode).to be_truthy
 
@@ -68,7 +63,7 @@ describe "Admin upcoming changes", type: :system do
     upcoming_changes_page.visit
     expect(upcoming_changes_page.change_item(:enable_upload_debug_mode)).to be_enabled
     upcoming_changes_page.change_item(:enable_upload_debug_mode).select_enabled_for("no_one")
-    expect(page).to have_content(I18n.t("admin_js.admin.upcoming_changes.change_disabled"))
+    expect(upcoming_changes_page).to have_disabled_success_toast
     expect(upcoming_changes_page.change_item(:enable_upload_debug_mode)).to be_disabled
 
     expect(SiteSetting.enable_upload_debug_mode).to be_falsey
@@ -94,7 +89,7 @@ describe "Admin upcoming changes", type: :system do
 
     # Test 'no_one' option - should disable the change and clear groups
     upcoming_changes_page.change_item(:enable_upload_debug_mode).select_enabled_for("no_one")
-    expect(page).to have_content(I18n.t("admin_js.admin.upcoming_changes.change_disabled"))
+    expect(upcoming_changes_page).to have_disabled_success_toast
     expect(upcoming_changes_page.change_item(:enable_upload_debug_mode)).to be_disabled
 
     upcoming_changes_page.visit
@@ -104,12 +99,7 @@ describe "Admin upcoming changes", type: :system do
 
     # Test 'everyone' option - should enable the change and clear groups
     upcoming_changes_page.change_item(:enable_upload_debug_mode).select_enabled_for("everyone")
-    expect(page).to have_content(
-      I18n.t(
-        "admin_js.admin.upcoming_changes.change_enabled_for_success",
-        enabledFor: I18n.t("admin_js.admin.upcoming_changes.enabled_for_options.everyone").downcase,
-      ),
-    )
+    expect(upcoming_changes_page).to have_enabled_for_success_toast("everyone")
     expect(upcoming_changes_page.change_item(:enable_upload_debug_mode)).to be_enabled
 
     upcoming_changes_page.visit
@@ -118,12 +108,7 @@ describe "Admin upcoming changes", type: :system do
 
     # Test 'staff' option - should enable the change and set staff group
     upcoming_changes_page.change_item(:enable_upload_debug_mode).select_enabled_for("staff")
-    expect(page).to have_content(
-      I18n.t(
-        "admin_js.admin.upcoming_changes.change_enabled_for_success",
-        enabledFor: I18n.t("admin_js.admin.upcoming_changes.enabled_for_options.staff").downcase,
-      ),
-    )
+    expect(upcoming_changes_page).to have_enabled_for_success_toast("staff")
     expect(upcoming_changes_page.change_item(:enable_upload_debug_mode)).to be_enabled
 
     upcoming_changes_page.visit
@@ -134,16 +119,12 @@ describe "Admin upcoming changes", type: :system do
     upcoming_changes_page.change_item(:enable_upload_debug_mode).select_enabled_for("groups")
     upcoming_changes_page.change_item(:enable_upload_debug_mode).add_group("trust_level_4")
     upcoming_changes_page.change_item(:enable_upload_debug_mode).save_groups
-    expect(page).to have_content(
-      I18n.t(
-        "admin_js.admin.upcoming_changes.change_enabled_for_success",
-        enabledFor:
-          I18n.t(
-            "admin_js.admin.upcoming_changes.enabled_for_options.specific_groups_with_group_names",
-            groupNames: "staff, trust_level_4",
-            count: 2,
-          ).downcase,
-      ),
+    expect(upcoming_changes_page).to have_enabled_for_success_toast(
+      "specific_groups_with_group_names",
+      translation_args: {
+        groupNames: "staff, trust_level_4",
+        count: 2,
+      },
     )
 
     upcoming_changes_page.visit

spec/system/member_upcoming_changes_spec.rb

@@ -2,6 +2,8 @@
 
 RSpec.describe "Member upcoming changes", type: :system do
   fab!(:current_user, :user)
+  fab!(:admin)
+  let(:upcoming_changes_page) { PageObjects::Pages::AdminUpcomingChanges.new }
 
   before do
     SiteSetting.enable_upcoming_changes = true
@@ -45,6 +47,21 @@ RSpec.describe "Member upcoming changes", type: :system do
       visit "/"
       expect(page).to have_css("body.uc-enable-upload-debug-mode")
     end
+
+    it "adds and removes the body class based on MessageBus subscription for client site settings" do
+      visit "/"
+      expect(page).to have_css("body.uc-enable-upload-debug-mode")
+
+      using_session(:admin) do
+        sign_in(admin)
+
+        upcoming_changes_page.visit
+        upcoming_changes_page.change_item(:enable_upload_debug_mode).select_enabled_for("no_one")
+        expect(upcoming_changes_page).to have_disabled_success_toast
+      end
+
+      expect(page).to have_no_css("body.uc-enable-upload-debug-mode")
+    end
   end
 
   context "when user is anonymous" do

spec/system/page_objects/pages/admin_upcoming_changes.rb

@@ -30,6 +30,29 @@ module PageObjects
           has_multiple_dropdowns: true,
         )
       end
+
+      def has_enabled_for_success_toast?(enabled_for, translation_args: {})
+        enabled_for_text =
+          if enabled_for == "specific_groups_with_group_names"
+            I18n.t(
+              "admin_js.admin.upcoming_changes.enabled_for_options.#{enabled_for}",
+              translation_args,
+            ).downcase
+          else
+            I18n.t("admin_js.admin.upcoming_changes.enabled_for_options.#{enabled_for}").downcase
+          end
+
+        page.has_content?(
+          I18n.t(
+            "admin_js.admin.upcoming_changes.change_enabled_for_success",
+            enabledFor: enabled_for_text,
+          ),
+        )
+      end
+
+      def has_disabled_success_toast?
+        page.has_content?(I18n.t("admin_js.admin.upcoming_changes.change_disabled"))
+      end
     end
   end
 end