From 66c54a35cd980bd0223034263ab76be8e54eaa49 Mon Sep 17 00:00:00 2001
From: Natalie Tay <natalie.tay@gmail.com>
Date: Wed, 3 Jan 2024 14:05:03 +0800
Subject: [PATCH] FIX: Continue to send the PM to others when there is a group
 or user that does not exist (#274)

Don't error out  when there is a group or user that does not exist
and also allow admins to be sent
---
 lib/report_generator.rb       | 40 ++++++++++++++++++++---------------
 spec/report_generator_spec.rb | 26 ++++++++++++++++++++++-
 2 files changed, 48 insertions(+), 18 deletions(-)

diff --git a/lib/report_generator.rb b/lib/report_generator.rb
index 67b730b..99e85ff 100644
--- a/lib/report_generator.rb
+++ b/lib/report_generator.rb
@@ -9,9 +9,12 @@ module ::DiscourseDataExplorer
     def generate(query_id, query_params, recipients)
       query = DiscourseDataExplorer::Query.find(query_id)
       return [] unless query
+      return [] if recipients.empty?
+
+      creator = User.find_by(id: @creator_user_id)
+      return [] unless Guardian.new(creator).can_send_private_messages?
 
       usernames = filter_recipients_by_query_access(recipients, query)
-      return [] if usernames.empty?
       params = params_to_hash(query_params)
 
       result = DataExplorer.run_query(query, params)
@@ -22,22 +25,6 @@ module ::DiscourseDataExplorer
       build_report_pms(query, table, usernames)
     end
 
-    def filter_recipients_by_query_access(recipients, query)
-      return [] if recipients.empty?
-      creator = User.find(@creator_user_id)
-      return [] unless Guardian.new(creator).can_send_private_messages?
-
-      recipients.reduce([]) do |names, recipient|
-        if (group = Group.find_by(name: recipient))
-          next names unless query.query_groups.exists?(group_id: group.id)
-          next names.concat group.users.pluck(:username)
-        elsif (user = User.find_by(username: recipient))
-          next names unless Guardian.new(user).user_can_access_query?(query)
-          next names << recipient
-        end
-      end
-    end
-
     def params_to_hash(query_params)
       params = JSON.parse(query_params)
       params_hash = {}
@@ -72,5 +59,24 @@ module ::DiscourseDataExplorer
       end
       pms
     end
+
+    private
+
+    def filter_recipients_by_query_access(recipients, query)
+      recipients.reduce([]) do |names, recipient|
+        if (group = Group.find_by(name: recipient)) &&
+             (
+               group.id == Group::AUTO_GROUPS[:admins] ||
+                 query.query_groups.exists?(group_id: group.id)
+             )
+          names.concat group.users.pluck(:username)
+        elsif (user = User.find_by(username: recipient)) &&
+              Guardian.new(user).user_can_access_query?(query)
+          names << recipient
+        end
+
+        names
+      end
+    end
   end
 end
diff --git a/spec/report_generator_spec.rb b/spec/report_generator_spec.rb
index 79a8a03..35a40fa 100644
--- a/spec/report_generator_spec.rb
+++ b/spec/report_generator_spec.rb
@@ -9,7 +9,6 @@ describe DiscourseDataExplorer::ReportGenerator do
   fab!(:group) { Fabricate(:group, users: [user]) }
 
   fab!(:query) { DiscourseDataExplorer::Query.find(-1) }
-  fab!(:query_group) { Fabricate(:query_group, query: query, group: group) }
 
   let(:query_params) { [%w[from_days_ago 0], %w[duration_days 15]] }
 
@@ -23,6 +22,7 @@ describe DiscourseDataExplorer::ReportGenerator do
     end
 
     it "returns [] if the recipient is not in query group" do
+      Fabricate(:query_group, query: query, group: group)
       result =
         described_class.new(user.id).generate(
           query.id,
@@ -54,5 +54,29 @@ describe DiscourseDataExplorer::ReportGenerator do
         ],
       )
     end
+
+    it "still returns a list of pms if a group or user does not exist" do
+      admin = Fabricate(:admin)
+      SiteSetting.personal_message_enabled_groups = group.id
+      DiscourseDataExplorer::ResultToMarkdown.expects(:convert).returns("le table")
+      freeze_time
+
+      result =
+        described_class.new(user.id).generate(query.id, query_params, %w[admins non-existent-group])
+
+      expect(result).to eq(
+        [
+          {
+            "title" => "Scheduled Report for #{query.name}",
+            "target_usernames" => [admin.username],
+            "raw" =>
+              "Hi #{admin.username}, your data explorer report is ready.\n\n" +
+                "Query Name:\n#{query.name}\n\nHere are the results:\nle table\n\n" +
+                "<a href='#{Discourse.base_url}/admin/plugins/explorer?id=#{query.id}'>View query in Data Explorer</a>\n\n" +
+                "Report created at #{Time.zone.now.strftime("%Y-%m-%d at %H:%M:%S")} (#{Time.zone.name})",
+          },
+        ],
+      )
+    end
   end
 end