From 87d7958db86e9bb1aee36205788f057e348ade4e Mon Sep 17 00:00:00 2001
From: Martin Brennan <martin@discourse.org>
Date: Wed, 22 May 2024 09:18:09 +1000
Subject: [PATCH] FIX: Bookmarking group reports (#291)

Since https://github.com/discourse/discourse/commit/67a8080e338aa62afbd02efc98178b4a67751775
in core, the functionality to bookmark a report from the group
Reports tab has been broken. This commit fixes the issue and adds
system spec coverage to prevent regression.
---
 .../controllers/group-reports-show.js         | 11 ++--
 spec/system/bookmark_spec.rb                  | 58 +++++++++++++++++++
 2 files changed, 65 insertions(+), 4 deletions(-)
 create mode 100644 spec/system/bookmark_spec.rb

diff --git a/assets/javascripts/discourse/controllers/group-reports-show.js b/assets/javascripts/discourse/controllers/group-reports-show.js
index b475b41..6f20e1d 100644
--- a/assets/javascripts/discourse/controllers/group-reports-show.js
+++ b/assets/javascripts/discourse/controllers/group-reports-show.js
@@ -5,7 +5,7 @@ import { inject as service } from "@ember/service";
 import BookmarkModal from "discourse/components/modal/bookmark";
 import { ajax } from "discourse/lib/ajax";
 import { popupAjaxError } from "discourse/lib/ajax-error";
-import { BookmarkFormData } from "discourse/lib/bookmark";
+import { BookmarkFormData } from "discourse/lib/bookmark-form-data";
 import {
   NO_REMINDER_ICON,
   WITH_REMINDER_ICON,
@@ -91,12 +91,15 @@ export default class GroupReportsShowController extends Controller {
     return this.modal.show(BookmarkModal, {
       model: {
         bookmark: new BookmarkFormData(modalBookmark),
-        afterSave: (savedData) => {
-          const bookmark = this.store.createRecord("bookmark", savedData);
+        afterSave: (bookmarkFormData) => {
+          const bookmark = this.store.createRecord(
+            "bookmark",
+            bookmarkFormData.saveData
+          );
           this.queryGroupBookmark = bookmark;
           this.appEvents.trigger(
             "bookmarks:changed",
-            savedData,
+            bookmarkFormData.saveData,
             bookmark.attachedTo()
           );
         },
diff --git a/spec/system/bookmark_spec.rb b/spec/system/bookmark_spec.rb
new file mode 100644
index 0000000..34395c9
--- /dev/null
+++ b/spec/system/bookmark_spec.rb
@@ -0,0 +1,58 @@
+# frozen_string_literal: true
+
+describe "Bookmarking reports attached to a group", type: :system do
+  fab!(:current_user) { Fabricate(:admin) }
+  fab!(:query_1) do
+    Fabricate(
+      :query,
+      name: "My query",
+      description: "Test query",
+      sql: "SELECT * FROM users",
+      user: current_user,
+    )
+  end
+  fab!(:group) { Fabricate(:group, name: "group") }
+  fab!(:group_user) { Fabricate(:group_user, user: current_user, group: group) }
+  fab!(:query_group_1) { Fabricate(:query_group, query: query_1, group: group) }
+  let(:bookmark_modal) { PageObjects::Modals::Bookmark.new }
+
+  before do
+    SiteSetting.data_explorer_enabled = true
+    sign_in(current_user)
+  end
+
+  it "allows the user to bookmark a group report" do
+    visit("/g/group/reports/#{query_1.id}")
+    find(".query-group-bookmark").click
+    expect(bookmark_modal).to be_open
+    bookmark_modal.click_primary_button
+    expect(page).to have_css(".query-group-bookmark.bookmarked")
+    expect(Bookmark.exists?(user: current_user, bookmarkable: query_group_1)).to eq(true)
+  end
+
+  it "allows the user to edit and delete a group report bookmark" do
+    bookmark =
+      Fabricate(:bookmark, user: current_user, bookmarkable: query_group_1, reminder_at: nil)
+
+    visit("/g/group/reports/#{query_1.id}")
+    find(".query-group-bookmark").click
+    expect(bookmark_modal).to be_open
+    bookmark_modal.fill_name("Remember this query")
+    bookmark_modal.click_primary_button
+    expect(bookmark_modal).to be_closed
+    expect(bookmark.reload.name).to eq("Remember this query")
+
+    find(".query-group-bookmark").click
+    expect(bookmark_modal).to be_open
+    bookmark_modal.delete
+    expect(bookmark_modal).to be_closed
+    expect(page).not_to have_css(".query-group-bookmark.bookmarked")
+    expect(Bookmark.exists?(user: current_user, bookmarkable: query_group_1)).to eq(false)
+  end
+
+  it "shows bookmarked group reports in the user bookmark list" do
+    bookmark = Fabricate(:bookmark, user: current_user, bookmarkable: query_group_1)
+    visit("/u/#{current_user.username_lower}/activity/bookmarks")
+    expect(page.find(".bookmark-list")).to have_content("My query")
+  end
+end