From 45bdfa1c8488092e37ae11b68da4f12d96f112b9 Mon Sep 17 00:00:00 2001 From: Krzysztof Kotlarek Date: Wed, 19 Oct 2022 11:48:36 +1100 Subject: [PATCH] FIX: sidebar_list_destination on CurrentUserSerializer (#18660) Before, `sidebar_list_destination` was an attribute on UserOptionSerializer. The problem was that this attribute was added to user model only when the user entered the preferences panel. We want that attribute to be available all the time, therefore it was moved to CurrentUserSerializer. --- .../app/controllers/preferences/sidebar.js | 4 ++++ .../javascripts/discourse/app/models/user.js | 2 +- .../sidebar-user-categories-section-test.js | 12 +++------- .../sidebar-user-community-section-test.js | 24 +++++-------------- .../sidebar-user-tags-section-test.js | 12 +++------- .../user-preferences-sidebar-test.js | 4 +--- app/serializers/current_user_serializer.rb | 7 +++++- app/serializers/user_option_serializer.rb | 7 +----- .../current_user_serializer_spec.rb | 9 +++++++ spec/serializers/user_serializer_spec.rb | 9 ------- 10 files changed, 34 insertions(+), 56 deletions(-) diff --git a/app/assets/javascripts/discourse/app/controllers/preferences/sidebar.js b/app/assets/javascripts/discourse/app/controllers/preferences/sidebar.js index b4c1067d8f3..9e7f5b80cb8 100644 --- a/app/assets/javascripts/discourse/app/controllers/preferences/sidebar.js +++ b/app/assets/javascripts/discourse/app/controllers/preferences/sidebar.js @@ -46,6 +46,10 @@ export default class extends Controller { if (result.user.sidebar_tags) { this.model.set("sidebar_tags", result.user.sidebar_tags); } + this.model.set( + "sidebar_list_destination", + this.newSidebarListDestination + ); this.saved = true; }) diff --git a/app/assets/javascripts/discourse/app/models/user.js b/app/assets/javascripts/discourse/app/models/user.js index d80c75fc1f2..352aa1e09f0 100644 --- a/app/assets/javascripts/discourse/app/models/user.js +++ b/app/assets/javascripts/discourse/app/models/user.js @@ -350,7 +350,7 @@ const User = RestModel.extend({ ); }, - sidebarListDestination: readOnly("user_option.sidebar_list_destination"), + sidebarListDestination: readOnly("sidebar_list_destination"), changeUsername(new_username) { return ajax(userPath(`${this.username_lower}/preferences/username`), { diff --git a/app/assets/javascripts/discourse/tests/acceptance/sidebar-user-categories-section-test.js b/app/assets/javascripts/discourse/tests/acceptance/sidebar-user-categories-section-test.js index d47e21dca7a..c5e11bf95d7 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/sidebar-user-categories-section-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/sidebar-user-categories-section-test.js @@ -295,9 +295,7 @@ acceptance("Sidebar - Logged on user - Categories Section", function (needs) { test("clicking section links - sidebar_list_destination set to unread/new and no unread or new topics", async function (assert) { updateCurrentUser({ - user_option: { - sidebar_list_destination: "unread_new", - }, + sidebar_list_destination: "unread_new", }); const { category1 } = setupUserSidebarCategories(); @@ -336,9 +334,7 @@ acceptance("Sidebar - Logged on user - Categories Section", function (needs) { created_in_new_period: true, }); updateCurrentUser({ - user_option: { - sidebar_list_destination: "unread_new", - }, + sidebar_list_destination: "unread_new", }); await visit("/"); @@ -384,9 +380,7 @@ acceptance("Sidebar - Logged on user - Categories Section", function (needs) { created_in_new_period: true, }); updateCurrentUser({ - user_option: { - sidebar_list_destination: "unread_new", - }, + sidebar_list_destination: "unread_new", }); await visit("/"); diff --git a/app/assets/javascripts/discourse/tests/acceptance/sidebar-user-community-section-test.js b/app/assets/javascripts/discourse/tests/acceptance/sidebar-user-community-section-test.js index 2c36491d70c..dbfdc5ebfd0 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/sidebar-user-community-section-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/sidebar-user-community-section-test.js @@ -162,9 +162,7 @@ acceptance("Sidebar - Logged on user - Community Section", function (needs) { test("clicking on everything link - sidebar_list_destination set to unread/new and no unread or new topics", async function (assert) { updateCurrentUser({ - user_option: { - sidebar_list_destination: "unread_new", - }, + sidebar_list_destination: "unread_new", }); await visit("/t/280"); @@ -201,9 +199,7 @@ acceptance("Sidebar - Logged on user - Community Section", function (needs) { created_in_new_period: true, }); updateCurrentUser({ - user_option: { - sidebar_list_destination: "unread_new", - }, + sidebar_list_destination: "unread_new", }); await visit("/t/280"); await click(".sidebar-section-community .sidebar-section-link-everything"); @@ -248,9 +244,7 @@ acceptance("Sidebar - Logged on user - Community Section", function (needs) { created_in_new_period: true, }); updateCurrentUser({ - user_option: { - sidebar_list_destination: "unread_new", - }, + sidebar_list_destination: "unread_new", }); await visit("/t/280"); await click(".sidebar-section-community .sidebar-section-link-everything"); @@ -299,9 +293,7 @@ acceptance("Sidebar - Logged on user - Community Section", function (needs) { test("clicking on tracked link - sidebar_list_destination set to unread/new and no unread or new topics", async function (assert) { updateCurrentUser({ - user_option: { - sidebar_list_destination: "unread_new", - }, + sidebar_list_destination: "unread_new", }); await visit("/t/280"); @@ -340,9 +332,7 @@ acceptance("Sidebar - Logged on user - Community Section", function (needs) { created_in_new_period: true, }); updateCurrentUser({ - user_option: { - sidebar_list_destination: "unread_new", - }, + sidebar_list_destination: "unread_new", }); await visit("/t/280"); await click(".sidebar-section-community .sidebar-section-link-tracked"); @@ -389,9 +379,7 @@ acceptance("Sidebar - Logged on user - Community Section", function (needs) { created_in_new_period: true, }); updateCurrentUser({ - user_option: { - sidebar_list_destination: "unread_new", - }, + sidebar_list_destination: "unread_new", }); await visit("/t/280"); await click(".sidebar-section-community .sidebar-section-link-tracked"); diff --git a/app/assets/javascripts/discourse/tests/acceptance/sidebar-user-tags-section-test.js b/app/assets/javascripts/discourse/tests/acceptance/sidebar-user-tags-section-test.js index ba97bd9ac7e..fd962631485 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/sidebar-user-tags-section-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/sidebar-user-tags-section-test.js @@ -238,9 +238,7 @@ acceptance("Sidebar - Logged on user - Tags section", function (needs) { test("clicking tag section links - sidebar_list_destination set to unread/new and no unread or new topics", async function (assert) { updateCurrentUser({ - user_option: { - sidebar_list_destination: "unread_new", - }, + sidebar_list_destination: "unread_new", }); await visit("/"); @@ -266,9 +264,7 @@ acceptance("Sidebar - Logged on user - Tags section", function (needs) { test("clicking tag section links - sidebar_list_destination set to unread/new with new topics", async function (assert) { updateCurrentUser({ - user_option: { - sidebar_list_destination: "unread_new", - }, + sidebar_list_destination: "unread_new", }); this.container.lookup("service:topic-tracking-state").loadStates([ @@ -308,9 +304,7 @@ acceptance("Sidebar - Logged on user - Tags section", function (needs) { test("clicking tag section links - sidebar_list_destination set to unread/new with unread topics", async function (assert) { updateCurrentUser({ - user_option: { - sidebar_list_destination: "unread_new", - }, + sidebar_list_destination: "unread_new", }); this.container.lookup("service:topic-tracking-state").loadStates([ diff --git a/app/assets/javascripts/discourse/tests/acceptance/user-preferences-sidebar-test.js b/app/assets/javascripts/discourse/tests/acceptance/user-preferences-sidebar-test.js index a78df47e32e..85db3fbb62b 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/user-preferences-sidebar-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/user-preferences-sidebar-test.js @@ -46,9 +46,7 @@ acceptance("User Preferences - Sidebar", function (needs) { { name: "monkey", pm_only: false }, { name: "gazelle", pm_only: false }, ], - user_option: { - sidebar_list_destination: "unread_new", - }, + sidebar_list_destination: "unread_new", }, }); } diff --git a/app/serializers/current_user_serializer.rb b/app/serializers/current_user_serializer.rb index a144b560e3b..62b358e1652 100644 --- a/app/serializers/current_user_serializer.rb +++ b/app/serializers/current_user_serializer.rb @@ -81,7 +81,8 @@ class CurrentUserSerializer < BasicUserSerializer :likes_notifications_disabled, :grouped_unread_notifications, :redesigned_user_menu_enabled, - :redesigned_user_page_nav_enabled + :redesigned_user_page_nav_enabled, + :sidebar_list_destination delegate :user_stat, to: :object, private: true delegate :any_posts, :draft_count, :pending_posts_count, :read_faq?, to: :user_stat @@ -160,6 +161,10 @@ class CurrentUserSerializer < BasicUserSerializer object.user_option.bookmark_auto_delete_preference end + def sidebar_list_destination + object.user_option.sidebar_list_none_selected? ? SiteSetting.default_sidebar_list_destination : object.user_option.sidebar_list_destination + end + def can_send_private_email_messages scope.can_send_private_messages_to_email? end diff --git a/app/serializers/user_option_serializer.rb b/app/serializers/user_option_serializer.rb index 582e00b7f23..19e7c794b0b 100644 --- a/app/serializers/user_option_serializer.rb +++ b/app/serializers/user_option_serializer.rb @@ -34,8 +34,7 @@ class UserOptionSerializer < ApplicationSerializer :timezone, :skip_new_user_tips, :default_calendar, - :oldest_search_log_date, - :sidebar_list_destination + :oldest_search_log_date def auto_track_topics_after_msecs object.auto_track_topics_after_msecs || SiteSetting.default_other_auto_track_topics_after_msecs @@ -52,8 +51,4 @@ class UserOptionSerializer < ApplicationSerializer def theme_ids object.theme_ids.presence || [SiteSetting.default_theme_id] end - - def sidebar_list_destination - object.sidebar_list_none_selected? ? SiteSetting.default_sidebar_list_destination : object.sidebar_list_destination - end end diff --git a/spec/serializers/current_user_serializer_spec.rb b/spec/serializers/current_user_serializer_spec.rb index 267eedac163..a8095e7557d 100644 --- a/spec/serializers/current_user_serializer_spec.rb +++ b/spec/serializers/current_user_serializer_spec.rb @@ -388,5 +388,14 @@ RSpec.describe CurrentUserSerializer do end end + describe "#sidebar_list_destination" do + it "returns choosen value or default" do + expect(serializer.as_json[:sidebar_list_destination]).to eq(SiteSetting.default_sidebar_list_destination) + + user.user_option.update!(sidebar_list_destination: "unread_new") + expect(serializer.as_json[:sidebar_list_destination]).to eq("unread_new") + end + end + include_examples "#display_sidebar_tags", described_class end diff --git a/spec/serializers/user_serializer_spec.rb b/spec/serializers/user_serializer_spec.rb index dc82b84251c..b5abe360d24 100644 --- a/spec/serializers/user_serializer_spec.rb +++ b/spec/serializers/user_serializer_spec.rb @@ -426,13 +426,4 @@ RSpec.describe UserSerializer do end include_examples "#display_sidebar_tags", UserSerializer - - describe "#sidebar_list_destination" do - it "returns choosen value or default" do - expect(serializer.as_json[:user_option][:sidebar_list_destination]).to eq(SiteSetting.default_sidebar_list_destination) - - user.user_option.update!(sidebar_list_destination: "unread_new") - expect(serializer.as_json[:user_option][:sidebar_list_destination]).to eq("unread_new") - end - end end