From 7d9a823a559b336a2f36f588fb8cc33a34a77361 Mon Sep 17 00:00:00 2001
From: Alan Guo Xiang Tan <gxtan1990@gmail.com>
Date: Mon, 29 May 2023 12:56:21 +0900
Subject: [PATCH] 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/controllers/preferences/interface.js | 2 +-
.../app/templates/preferences/interface.hbs | 2 +-
.../page_objects/pages/user_preferences.rb | 5 +++
.../pages/user_preferences_interface.rb | 30 ++++++++++++++++++
.../system/user_preferences_interface_spec.rb | 31 ++++++++-----------
5 files changed, 50 insertions(+), 20 deletions(-)
create mode 100644 spec/system/page_objects/pages/user_preferences_interface.rb
diff --git a/app/assets/javascripts/discourse/app/controllers/preferences/interface.js b/app/assets/javascripts/discourse/app/controllers/preferences/interface.js
index d022aec535c..9f86caa8e1c 100644
--- a/app/assets/javascripts/discourse/app/controllers/preferences/interface.js
+++ b/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();
}
})
diff --git a/app/assets/javascripts/discourse/app/templates/preferences/interface.hbs b/app/assets/javascripts/discourse/app/templates/preferences/interface.hbs
index 4f6e5342257..9a5ac5a2c07 100644
--- a/app/assets/javascripts/discourse/app/templates/preferences/interface.hbs
+++ b/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)
}}
diff --git a/spec/system/page_objects/pages/user_preferences.rb b/spec/system/page_objects/pages/user_preferences.rb
index dda52924afe..f872c973905 100644
--- a/spec/system/page_objects/pages/user_preferences.rb
+++ b/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
diff --git a/spec/system/page_objects/pages/user_preferences_interface.rb b/spec/system/page_objects/pages/user_preferences_interface.rb
new file mode 100644
index 00000000000..d715e62163d
--- /dev/null
+++ b/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
diff --git a/spec/system/user_preferences_interface_spec.rb b/spec/system/user_preferences_interface_spec.rb
index d6a25c1a332..52137d06d62 100644
--- a/spec/system/user_preferences_interface_spec.rb
+++ b/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