From 0d289a5690ab44c4ebd6943a05cda8dfb97deb59 Mon Sep 17 00:00:00 2001
From: Penar Musaraj <pmusaraj@gmail.com>
Date: Thu, 8 Aug 2024 12:12:01 -0400
Subject: [PATCH] UX: Do not delete narrative bot PM when skipping user tips
 (#28265)

Should fix https://meta.discourse.org/t/skip-tips-option-in-user-tip-should-not-delete-welcome-message/303420

Co-authored-by: Joffrey JAFFEUX <j.jaffeux@gmail.com>
---
 plugins/discourse-narrative-bot/plugin.rb     | 28 ----------------
 .../spec/system/user_spec.rb                  | 32 +++++++++++++++++++
 .../discourse-narrative-bot/spec/user_spec.rb | 10 ------
 3 files changed, 32 insertions(+), 38 deletions(-)
 create mode 100644 plugins/discourse-narrative-bot/spec/system/user_spec.rb

diff --git a/plugins/discourse-narrative-bot/plugin.rb b/plugins/discourse-narrative-bot/plugin.rb
index 76cb1e34275..447451bbb4c 100644
--- a/plugins/discourse-narrative-bot/plugin.rb
+++ b/plugins/discourse-narrative-bot/plugin.rb
@@ -63,10 +63,6 @@ after_initialize do
 
   self.on(:user_unstaged) { |user| user.enqueue_bot_welcome_post }
 
-  self.add_model_callback(UserOption, :after_save) do
-    user.delete_bot_welcome_post if saved_change_to_skip_new_user_tips? && self.skip_new_user_tips
-  end
-
   self.add_to_class(:user, :enqueue_bot_welcome_post) do
     return if SiteSetting.disable_discourse_narrative_bot_welcome_post
 
@@ -98,30 +94,6 @@ after_initialize do
         .include?(self.username)
   end
 
-  self.add_to_class(:user, :delete_bot_welcome_post) do
-    data = DiscourseNarrativeBot::Store.get(self.id) || {}
-    topic_id = data[:topic_id]
-    return if topic_id.blank? || data[:track] != DiscourseNarrativeBot::NewUserNarrative.to_s
-
-    topic_user = topic_users.find_by(topic_id: topic_id)
-    return if topic_user.present? && topic_user.last_read_post_number.present?
-
-    topic = Topic.find_by(id: topic_id)
-    return if topic.blank?
-
-    first_post = topic.ordered_posts.first
-
-    notification = Notification.where(topic_id: topic.id, post_number: first_post.post_number).first
-    if notification.present?
-      Notification.read(self, notification.id)
-      self.reload
-      self.publish_notifications_state
-    end
-
-    PostDestroyer.new(Discourse.system_user, first_post, skip_staff_log: true).destroy
-    DiscourseNarrativeBot::Store.remove(self.id)
-  end
-
   self.on(:post_created) do |post, options|
     user = post.user
 
diff --git a/plugins/discourse-narrative-bot/spec/system/user_spec.rb b/plugins/discourse-narrative-bot/spec/system/user_spec.rb
new file mode 100644
index 00000000000..82853ed28bc
--- /dev/null
+++ b/plugins/discourse-narrative-bot/spec/system/user_spec.rb
@@ -0,0 +1,32 @@
+# frozen_string_literal: true
+
+describe "Narrative Bot PM", type: :system do
+  fab!(:admin)
+  fab!(:user)
+  fab!(:topics) { Fabricate.times(2, :post).map(&:topic) }
+  fab!(:posts) { Fabricate.times(3, :post, topic: topics[0]) }
+
+  context "when user tips are enabled" do
+    before do
+      Jobs.run_immediately!
+      SiteSetting.enable_user_tips = true
+      SiteSetting.discourse_narrative_bot_enabled = true
+      # shortcut to generate welcome post since we're not going through user creation or first login
+      user.enqueue_bot_welcome_post
+    end
+
+    it "does not delete the narrative bot PM when skipping all tips" do
+      sign_in user
+      visit "/"
+
+      tooltip = PageObjects::Components::Tooltips.new("user-tip")
+      tooltip.find(".btn", text: "Skip tips").click
+
+      expect(tooltip).to be_not_present
+      expect(page).to have_css(".badge-notification.new-pms")
+
+      find("#toggle-current-user").click
+      expect(page).to have_css(".notification.unread.private-message", text: "Greetings!")
+    end
+  end
+end
diff --git a/plugins/discourse-narrative-bot/spec/user_spec.rb b/plugins/discourse-narrative-bot/spec/user_spec.rb
index 12463524a1d..dca8930b334 100644
--- a/plugins/discourse-narrative-bot/spec/user_spec.rb
+++ b/plugins/discourse-narrative-bot/spec/user_spec.rb
@@ -111,16 +111,6 @@ RSpec.describe User do
         SiteSetting.default_other_skip_new_user_tips = true
         expect { user }.to_not change { Post.count }
       end
-
-      it "should delete the existing PM" do
-        user.user_option.skip_new_user_tips = true
-
-        expect { user.user_option.save! }.to change { Topic.count }.by(-1).and not_change {
-                UserHistory.count
-              }.and change { user.unread_high_priority_notifications }.by(-1).and change {
-                            user.notifications.count
-                          }.by(-1)
-      end
     end
 
     context "when user is anonymous?" do