FIX: Staff group causing errors in upcoming changes from localization (#37797)

We have a "Staff" option for upcoming change items that was sending
the `staff` group name to the server. I had assumed that our automatic
groups could not be renamed, but this is not the case. If your
`default_locale` for your site is changed, the automatic group names
change via `Group.refresh_automatic_groups!` in a background job.

This means that if you change your `default_locale` to `de` (German),
the `staff` group becomes `Team`, and the upcoming changes that are set
to "Staff" will now be looking for the `staff` group, which no longer
exists, causing errors.

This commit fixes the issue by always using the localized staff
automatic group name, both for the value of the dropdown, and in
successs messsages etc.

c.f.

https://meta.discourse.org/t/issue-with-enabling-upcoming-changes/395881/16

---------

Co-authored-by: Régis Hanol <regis@hanol.fr>

config/locales/client.de.yml

@@ -5518,7 +5518,7 @@ de:
         enabled_for_options:
           no_one: "Keinen"
           everyone: "Jeden"
-          staff: "Nur Team-Mitglieder"
+          staff: "Nur %{staffGroupName}"
           specific_groups: "Bestimmte Gruppe(n)"
           specific_groups_with_group_names:
             one: "die Gruppe %{groupNames}"

config/locales/client.en.yml

@@ -5961,7 +5961,7 @@ en:
         enabled_for_options:
           no_one: "No one"
           everyone: "Everyone"
-          staff: "Staff only"
+          staff: "%{staffGroupName} only"
           specific_groups: "Specific group(s)"
           specific_groups_with_group_names:
             one: "the group %{groupNames}"
@@ -5984,7 +5984,7 @@ en:
           all: "All"
           enabled_all: "All enabled states"
           enabled: "Enabled"
-          enabled_for_staff: "Enabled for staff"
+          enabled_for_staff: "Enabled for %{staffGroupName}"
           enabled_for_specific_groups: "Enabled for specific groups"
           disabled: "Disabled"
           impact_type_all: "All impact types"

frontend/discourse/admin/components/admin-config-areas/upcoming-change-item.gjs

@@ -7,6 +7,7 @@ import { action } from "@ember/object";
 import { LinkTo } from "@ember/routing";
 import { cancel } from "@ember/runloop";
 import { service } from "@ember/service";
+import { capitalize } from "@ember/string";
 import { htmlSafe } from "@ember/template";
 import { modifier } from "ember-modifier";
 import DButton from "discourse/components/d-button";
@@ -73,8 +74,8 @@ export default class UpcomingChangeItem extends Component {
     if (!this.args.change.upcoming_change.disallow_enabled_for_groups) {
       options.push(
         {
-          label: i18n("admin.upcoming_changes.enabled_for_options.staff"),
-          value: "staff",
+          label: capitalize(this.staffGroupName),
+          value: this.staffGroupName,
         },
         {
           label: i18n(
@@ -88,6 +89,10 @@ export default class UpcomingChangeItem extends Component {
     return options;
   }
 
+  get staffGroupName() {
+    return this.site.groupsById[AUTO_GROUPS.staff.id].name;
+  }
+
   get enabledForDisabled() {
     return (
       this.args.change.upcoming_change.status === "permanent" ||
@@ -190,9 +195,10 @@ export default class UpcomingChangeItem extends Component {
       enabledForLabel = i18n(
         "admin.upcoming_changes.enabled_for_options.everyone"
       );
-    } else if (enabledFor === "staff") {
+    } else if (enabledFor === this.staffGroupName) {
       enabledForLabel = i18n(
-        "admin.upcoming_changes.enabled_for_options.staff"
+        "admin.upcoming_changes.enabled_for_options.staff",
+        { staffGroupName: capitalize(this.staffGroupName) }
       );
     } else if (enabledFor === "groups") {
       const groupNames = this.bufferedGroups.split(",");
@@ -246,8 +252,8 @@ export default class UpcomingChangeItem extends Component {
     try {
       await this.toggleChange(isEnabled, newValue);
 
-      if (newValue === "staff") {
-        this.groupsChanged(this.site.groupsById[AUTO_GROUPS.staff.id].name);
+      if (newValue === this.staffGroupName) {
+        this.groupsChanged(this.staffGroupName);
       } else if (newValue === "everyone" || newValue === "no_one") {
         this.groupsChanged("");
       }

frontend/discourse/app/lib/constants.js

@@ -26,6 +26,8 @@ export const CATEGORY_STYLE_TYPES = { square: 0, icon: 1, emoji: 2 };
 
 export const CATEGORY_TEXT_COLORS = ["FFFFFF", "000000"];
 
+// NOTE: Group names are changed based on the site's locale, see
+// Group.refresh_automatic_group! for more details
 export const AUTO_GROUPS = {
   everyone: { id: 0, automatic: true, name: "everyone" },
   admins: { id: 1, automatic: true, name: "admins" },

lib/tasks/javascript.rake

@@ -66,6 +66,8 @@ task "javascript:update_constants" => :environment do
 
     export const CATEGORY_TEXT_COLORS = #{Category::DEFAULT_TEXT_COLORS};
 
+    // NOTE: Group names are changed based on the site's locale, see
+    // Group.refresh_automatic_group! for more details
     export const AUTO_GROUPS = #{auto_groups.to_json};
 
     export const GROUP_SMTP_SSL_MODES = #{Group.smtp_ssl_modes.to_json};

spec/system/admin_upcoming_changes_spec.rb

@@ -132,7 +132,12 @@ 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(upcoming_changes_page).to have_enabled_for_success_toast("staff")
+    expect(upcoming_changes_page).to have_enabled_for_success_toast(
+      "staff",
+      translation_args: {
+        staffGroupName: I18n.t("groups.default_names.staff").titleize,
+      },
+    )
     expect(upcoming_changes_page.change_item(:enable_upload_debug_mode)).to be_enabled
 
     upcoming_changes_page.visit
@@ -255,4 +260,30 @@ describe "Admin upcoming changes", type: :system do
       ".sidebar-section-link-suffix.admin-sidebar-nav-link__dot",
     )
   end
+
+  context "when the staff group name has been localized" do
+    before do
+      SiteSetting.default_locale = "de"
+      Group.refresh_automatic_group!(:staff)
+    end
+
+    it "displays the localized name in the enabled for options and enabling staff works correctly" do
+      upcoming_changes_page.visit
+
+      upcoming_changes_page.change_item(:enable_upload_debug_mode).select_enabled_for(
+        Group.find(Group::AUTO_GROUPS[:staff]).name,
+      )
+      expect(upcoming_changes_page).to have_enabled_for_success_toast(
+        "staff",
+        translation_args: {
+          staffGroupName: I18n.t("groups.default_names.staff", locale: SiteSetting.default_locale),
+        },
+      )
+      expect(upcoming_changes_page.change_item(:enable_upload_debug_mode)).to be_enabled
+      expect(SiteSetting.enable_upload_debug_mode).to be_truthy
+      expect(SiteSettingGroup.find_by(name: "enable_upload_debug_mode").group_ids).to include(
+        Group::AUTO_GROUPS[:staff].to_s,
+      )
+    end
+  end
 end

spec/system/page_objects/pages/admin_upcoming_changes.rb

@@ -31,23 +31,32 @@ module PageObjects
         )
       end
 
-      def has_enabled_for_success_toast?(enabled_for, translation_args: {})
+      def has_enabled_for_success_toast?(
+        enabled_for,
+        translation_args: {},
+        locale: SiteSetting.default_locale
+      )
         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,
+              translation_args.merge(locale: locale),
             ).downcase
           else
-            I18n.t("admin_js.admin.upcoming_changes.enabled_for_options.#{enabled_for}").downcase
+            I18n.t(
+              "admin_js.admin.upcoming_changes.enabled_for_options.#{enabled_for}",
+              translation_args.merge(locale: locale),
+            ).downcase
           end
 
-        page.has_content?(
+        enabled_for_success_text =
           I18n.t(
             "admin_js.admin.upcoming_changes.change_enabled_for_success",
             enabledFor: enabled_for_text,
-          ),
-        )
+            locale: locale,
+          )
+
+        page.has_content?(enabled_for_success_text)
       end
 
       def has_disabled_success_toast?