From 16e3bc3ff4f5c0e9191d45985c4af956aee32c41 Mon Sep 17 00:00:00 2001
From: Bianca Nenciu <nbianca@users.noreply.github.com>
Date: Wed, 7 Dec 2022 18:27:10 +0200
Subject: [PATCH] FIX: Do not reset seen popups when skip_new_user_tips is
 false  (#19345)

* FIX: Save only visible fields from the sidebar page

* FIX: Do not reset seen popups when set to false

If the option was unchecked, but it was not changed at all by the user
it was still sent to the server as a 'false' value which reset all seen
popups. This removes that behavior and resetting the list of seen popups
must be done using the "skip new user tips" button.
---
 .../discourse/app/controllers/preferences/sidebar.js  |  8 +++++++-
 app/services/user_updater.rb                          |  4 ++--
 spec/services/user_updater_spec.rb                    | 11 ++++++-----
 3 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/app/assets/javascripts/discourse/app/controllers/preferences/sidebar.js b/app/assets/javascripts/discourse/app/controllers/preferences/sidebar.js
index d74cbdde0cf..8a2ac607227 100644
--- a/app/assets/javascripts/discourse/app/controllers/preferences/sidebar.js
+++ b/app/assets/javascripts/discourse/app/controllers/preferences/sidebar.js
@@ -13,6 +13,12 @@ export default class extends Controller {
   @tracked selectedSidebarCategories = [];
   @tracked selectedSidebarTagNames = [];
 
+  saveAttrNames = [
+    "sidebar_category_ids",
+    "sidebar_tag_names",
+    "sidebar_list_destination",
+  ];
+
   sidebarListDestinations = [
     {
       name: I18n.t("user.experimental_sidebar.list_destination_default"),
@@ -42,7 +48,7 @@ export default class extends Controller {
     );
 
     this.model
-      .save()
+      .save(this.saveAttrNames)
       .then((result) => {
         if (result.user.sidebar_tags) {
           this.model.set("sidebar_tags", result.user.sidebar_tags);
diff --git a/app/services/user_updater.rb b/app/services/user_updater.rb
index a614e3a326e..81f7dafbabf 100644
--- a/app/services/user_updater.rb
+++ b/app/services/user_updater.rb
@@ -180,8 +180,8 @@ class UserUpdater
       end
     end
 
-    if attributes.key?(:skip_new_user_tips)
-      user.user_option.seen_popups = user.user_option.skip_new_user_tips ? [-1] : nil
+    if attributes.key?(:skip_new_user_tips) && user.user_option.skip_new_user_tips
+      user.user_option.seen_popups = [-1]
     end
 
     # automatically disable digests when mailing_list_mode is enabled
diff --git a/spec/services/user_updater_spec.rb b/spec/services/user_updater_spec.rb
index 82da2c0721b..714e0d2289a 100644
--- a/spec/services/user_updater_spec.rb
+++ b/spec/services/user_updater_spec.rb
@@ -534,14 +534,15 @@ RSpec.describe UserUpdater do
         expect(user.user_option.skip_new_user_tips).to eq(true)
         expect(user.user_option.seen_popups).to eq([-1])
         expect(messages.map(&:data)).to contain_exactly([-1])
+      end
 
-        messages = MessageBus.track_publish('/user-tips') do
-          UserUpdater.new(Discourse.system_user, user).update(skip_new_user_tips: false)
-        end
+      it 'does not reset seen_popups' do
+        user.user_option.update!(seen_popups: [1, 2, 3])
+
+        UserUpdater.new(Discourse.system_user, user).update(skip_new_user_tips: false)
 
         expect(user.user_option.skip_new_user_tips).to eq(false)
-        expect(user.user_option.seen_popups).to eq(nil)
-        expect(messages.map(&:data)).to contain_exactly(nil)
+        expect(user.user_option.seen_popups).to eq([1, 2, 3])
       end
     end