DEV: Fix flaky user preferences interface system test (#21800)

Why is this change required?

The flaky system test was due to the fact that we had to poll for the
user preferences interface page to reload after saving. However, this
turns out to be a bug on the user perferences interface page because the
page should only reload if the user has selected a new theme that is
different from the site's default but we were reloading the page for
users that did not have any user theme selected. Therefore there was an
unnecessary reload happening when saving other fields on the user
preferences interface page.

app/assets/javascripts/discourse/app/controllers/preferences/interface.js

@@ -341,7 +341,7 @@ export default Controller.extend({
 
           this.homeChanged();
 
-          if (this.themeId !== this.currentThemeId) {
+          if (this.themeId && this.themeId !== this.currentThemeId) {
             reload();
           }
         })

app/assets/javascripts/discourse/app/templates/preferences/interface.hbs

@@ -216,7 +216,7 @@
       @valueProperty="value"
       @content={{this.bookmarkAfterNotificationModes}}
       @value={{this.model.user_option.bookmark_auto_delete_preference}}
-      @id="boookmark-after-notification-mode"
+      @id="bookmark-after-notification-mode"
       @onChange={{action
         (mut this.model.user_option.bookmark_auto_delete_preference)
       }}

spec/system/page_objects/pages/user_preferences.rb

@@ -8,6 +8,11 @@ module PageObjects
         self
       end
 
+      def click_interface_tab
+        click_link "Interface"
+        self
+      end
+
       def click_secondary_navigation_menu_scroll_right
         find(".horizontal-overflow-nav__scroll-right").click
       end

spec/system/page_objects/pages/user_preferences_interface.rb

@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+module PageObjects
+  module Pages
+    class UserPreferencesInterface < PageObjects::Pages::Base
+      def visit(user)
+        page.visit("/u/#{user.username}/preferences/interface")
+        self
+      end
+
+      def has_bookmark_after_notification_mode?(value)
+        page.has_css?(
+          "#bookmark-after-notification-mode .select-kit-header[data-value=\"#{value}\"]",
+        )
+      end
+
+      def select_bookmark_after_notification_mode(value)
+        page.find("#bookmark-after-notification-mode").click
+        page.find(".select-kit-row[data-value=\"#{value}\"]").click
+        self
+      end
+
+      def save_changes
+        click_button "Save Changes"
+        expect(page).to have_content(I18n.t("js.saved"))
+        self
+      end
+    end
+  end
+end

spec/system/user_preferences_interface_spec.rb

@@ -3,34 +3,29 @@
 describe "User preferences for Interface", type: :system, js: true do
   fab!(:user) { Fabricate(:user) }
   let(:user_preferences_page) { PageObjects::Pages::UserPreferences.new }
+  let(:user_preferences_interface_page) { PageObjects::Pages::UserPreferencesInterface.new }
 
   before { sign_in(user) }
 
   describe "Bookmarks" do
     it "changes the bookmark after notification preference" do
-      user_preferences_page.visit(user)
-      click_link "Interface"
+      user_preferences_page.visit(user).click_interface_tab
 
       # preselects the default user_option.bookmark_auto_delete_preference value of 3 (clear_reminder)
-      expect(page).to have_css(
-        "#boookmark-after-notification-mode .select-kit-header[data-value='#{Bookmark.auto_delete_preferences[:clear_reminder]}']",
+      expect(user_preferences_interface_page).to have_bookmark_after_notification_mode(
+        Bookmark.auto_delete_preferences[:clear_reminder],
       )
-      page.find("#boookmark-after-notification-mode").click
-      page.find(
-        ".select-kit-row[data-value=\"#{Bookmark.auto_delete_preferences[:when_reminder_sent]}\"]",
-      ).click
 
-      click_button "Save Changes"
+      user_preferences_interface_page.select_bookmark_after_notification_mode(
+        Bookmark.auto_delete_preferences[:when_reminder_sent],
+      ).save_changes
 
-      # the preference page reloads after saving, so we need to poll the db
-      try_until_success do
-        expect(
-          UserOption.exists?(
-            user_id: user.id,
-            bookmark_auto_delete_preference: Bookmark.auto_delete_preferences[:when_reminder_sent],
-          ),
-        ).to be_truthy
-      end
+      expect(
+        UserOption.exists?(
+          user_id: user.id,
+          bookmark_auto_delete_preference: Bookmark.auto_delete_preferences[:when_reminder_sent],
+        ),
+      ).to eq(true)
     end
   end
 end