From 45b6e7eb4f0d5325c8306131ac2a85a671527635 Mon Sep 17 00:00:00 2001
From: Frank <frank3manuel@gmail.com>
Date: Tue, 14 Jun 2022 23:07:02 +0800
Subject: [PATCH] FEATURE: Bookmarkable QueryGroups. (#177)

Adds the ability for non-Admin users to bookmark Queries from inside Group > Reports > Query view.
---
 .../data_explorer/query_controller.rb         |   8 +-
 app/models/data_explorer/query_group.rb       |   2 +
 .../data_explorer/query_group_serializer.rb   |  25 +++
 ...xplorer_query_group_bookmark_serializer.rb |  38 +++++
 .../controllers/group-reports-show.js         |  51 ++++++
 .../discourse/routes/group-reports-show.js    |   9 +-
 .../templates/group-reports-show.hbs          |   8 +
 assets/stylesheets/explorer.scss              |   6 +
 lib/data_explorer_query_group_bookmarkable.rb |  57 +++++++
 plugin.rb                                     |   6 +
 spec/fabricators/query_fabricator.rb          |  13 ++
 ..._explorer_query_group_bookmarkable_spec.rb | 148 ++++++++++++++++++
 12 files changed, 368 insertions(+), 3 deletions(-)
 create mode 100644 app/serializers/data_explorer/query_group_serializer.rb
 create mode 100644 app/serializers/user_data_explorer_query_group_bookmark_serializer.rb
 create mode 100644 lib/data_explorer_query_group_bookmarkable.rb
 create mode 100644 spec/fabricators/query_fabricator.rb
 create mode 100644 spec/lib/data_explorer_query_group_bookmarkable_spec.rb

diff --git a/app/controllers/data_explorer/query_controller.rb b/app/controllers/data_explorer/query_controller.rb
index 81b6acd..e2fe7a5 100644
--- a/app/controllers/data_explorer/query_controller.rb
+++ b/app/controllers/data_explorer/query_controller.rb
@@ -65,7 +65,13 @@ class DataExplorer::QueryController < ::ApplicationController
 
     respond_to do |format|
       format.json do
-        render_serialized @query, DataExplorer::QuerySerializer, root: 'query'
+        query_group = DataExplorer::QueryGroup.find_by(query_id: @query.id, group_id: @group.id)
+
+        render json: {
+          query: serialize_data(@query, DataExplorer::QuerySerializer, root: nil),
+          query_group: serialize_data(query_group, DataExplorer::QueryGroupSerializer, root: nil),
+        }
+
       end
     end
   end
diff --git a/app/models/data_explorer/query_group.rb b/app/models/data_explorer/query_group.rb
index cd59885..d7b7e93 100644
--- a/app/models/data_explorer/query_group.rb
+++ b/app/models/data_explorer/query_group.rb
@@ -6,5 +6,7 @@ module DataExplorer
 
     belongs_to :query
     belongs_to :group
+
+    has_many :bookmarks, as: :bookmarkable
   end
 end
diff --git a/app/serializers/data_explorer/query_group_serializer.rb b/app/serializers/data_explorer/query_group_serializer.rb
new file mode 100644
index 0000000..21c9540
--- /dev/null
+++ b/app/serializers/data_explorer/query_group_serializer.rb
@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+class DataExplorer::QueryGroupSerializer < ActiveModel::Serializer
+  attributes :id, :group_id, :query_id, :bookmark,
+
+  def query_group_bookmark
+    @query_group_bookmark ||= Bookmark.find_by(user: scope.user, bookmarkable: object)
+  end
+
+  def include_bookmark?
+    query_group_bookmark.present?
+  end
+
+  def bookmark
+    {
+      id: query_group_bookmark.id,
+      reminder_at: query_group_bookmark.reminder_at,
+      name: query_group_bookmark.name,
+      auto_delete_preference: query_group_bookmark.auto_delete_preference,
+      bookmarkable_id: query_group_bookmark.bookmarkable_id,
+      bookmarkable_type: query_group_bookmark.bookmarkable_type
+    }
+  end
+
+end
diff --git a/app/serializers/user_data_explorer_query_group_bookmark_serializer.rb b/app/serializers/user_data_explorer_query_group_bookmark_serializer.rb
new file mode 100644
index 0000000..c3ae534
--- /dev/null
+++ b/app/serializers/user_data_explorer_query_group_bookmark_serializer.rb
@@ -0,0 +1,38 @@
+# frozen_string_literal: true
+
+class UserDataExplorerQueryGroupBookmarkSerializer < UserBookmarkBaseSerializer
+  def title
+    fancy_title
+  end
+
+  def fancy_title
+    data_explorer_query.name
+  end
+
+  def cooked
+    data_explorer_query.description
+  end
+
+  def bookmarkable_user
+    @bookmarkable_user ||= data_explorer_query.user
+  end
+
+  def bookmarkable_url
+    "/g/#{data_explorer_query_group.group.name}/reports/#{data_explorer_query_group.query_id}"
+  end
+
+  def excerpt
+    return nil unless cooked
+    @excerpt ||= PrettyText.excerpt(cooked, 300, keep_emoji_images: true)
+  end
+
+  private
+
+  def data_explorer_query
+    data_explorer_query_group.query
+  end
+
+  def data_explorer_query_group
+    object.bookmarkable
+  end
+end
diff --git a/assets/javascripts/discourse/controllers/group-reports-show.js b/assets/javascripts/discourse/controllers/group-reports-show.js
index 708f866..6bd75ba 100644
--- a/assets/javascripts/discourse/controllers/group-reports-show.js
+++ b/assets/javascripts/discourse/controllers/group-reports-show.js
@@ -1,5 +1,11 @@
 import { popupAjaxError } from "discourse/lib/ajax-error";
 import { ajax } from "discourse/lib/ajax";
+import Bookmark, {
+  NO_REMINDER_ICON,
+  WITH_REMINDER_ICON,
+} from "discourse/models/bookmark";
+import { openBookmarkModal } from "discourse/controllers/bookmark";
+import discourseComputed from "discourse-common/utils/decorators";
 
 export default Ember.Controller.extend({
   showResults: false,
@@ -35,5 +41,50 @@ export default Ember.Controller.extend({
         })
         .finally(() => this.set("loading", false));
     },
+
+    toggleBookmark() {
+      return openBookmarkModal(
+        this.queryGroup.bookmark ||
+          Bookmark.create({
+            bookmarkable_type: "DataExplorer::QueryGroup",
+            bookmarkable_id: this.queryGroup.id,
+            user_id: this.currentUser.id,
+          }),
+        {
+          onAfterSave: (savedData) => {
+            const bookmark = Bookmark.create(savedData);
+            this.set("queryGroup.bookmark", bookmark);
+            this.appEvents.trigger(
+              "bookmarks:changed",
+              savedData,
+              bookmark.attachedTo()
+            );
+          },
+          onAfterDelete: () => {
+            this.set("queryGroup.bookmark", null);
+          },
+        }
+      );
+    },
+  }, // actions
+
+  @discourseComputed("queryGroup.bookmark")
+  bookmarkLabel(bookmark) {
+    return bookmark ? "bookmarked.edit_bookmark" : "bookmarked.title";
+  },
+
+  @discourseComputed("queryGroup.bookmark")
+  bookmarkIcon(bookmark) {
+    if (bookmark && bookmark.reminder_at) {
+      return WITH_REMINDER_ICON;
+    }
+    return NO_REMINDER_ICON;
+  },
+
+  @discourseComputed("queryGroup.bookmark")
+  bookmarkClassName(bookmark) {
+    return bookmark
+      ? ["bookmark", "bookmarked", "query-group-bookmark"].join(" ")
+      : ["bookmark", "query-group-bookmark"].join(" ");
   },
 });
diff --git a/assets/javascripts/discourse/routes/group-reports-show.js b/assets/javascripts/discourse/routes/group-reports-show.js
index f49b9fa..123d930 100644
--- a/assets/javascripts/discourse/routes/group-reports-show.js
+++ b/assets/javascripts/discourse/routes/group-reports-show.js
@@ -9,15 +9,20 @@ export default DiscourseRoute.extend({
     const group = this.modelFor("group");
     return ajax(`/g/${group.name}/reports/${params.query_id}`)
       .then((response) => {
-        const queryParamInfo = response.query.param_info;
+        let query = response.query;
+        let queryGroup = response.query_group;
+
+        const queryParamInfo = query.param_info;
+
         const queryParams = queryParamInfo.reduce((acc, param) => {
           acc[param.identifier] = param.default;
           return acc;
         }, {});
 
         return {
-          model: Object.assign({ params: queryParams }, response.query),
+          model: Object.assign({ params: queryParams }, query),
           group,
+          queryGroup,
         };
       })
       .catch(() => {
diff --git a/assets/javascripts/discourse/templates/group-reports-show.hbs b/assets/javascripts/discourse/templates/group-reports-show.hbs
index 6bbf0c0..aeaa3d2 100644
--- a/assets/javascripts/discourse/templates/group-reports-show.hbs
+++ b/assets/javascripts/discourse/templates/group-reports-show.hbs
@@ -10,6 +10,14 @@
       </div>
     {{/if}}
     {{d-button action=(action "run") icon="play" label="explorer.run" class="btn-primary" type="submit"}}
+
+    {{d-button
+      action=(action "toggleBookmark")
+      label=bookmarkLabel
+      icon=bookmarkIcon
+      class=bookmarkClassName
+    }}
+
   </form>
   {{conditional-loading-spinner condition=loading}}
   {{#if results}}
diff --git a/assets/stylesheets/explorer.scss b/assets/stylesheets/explorer.scss
index 48da11e..0aec058 100644
--- a/assets/stylesheets/explorer.scss
+++ b/assets/stylesheets/explorer.scss
@@ -436,3 +436,9 @@ table.group-reports {
 .right-buttons .btn {
   margin-left: 0.5em;
 }
+
+.query-group-bookmark {
+  &.bookmark .d-icon {
+    color: var(--tertiary);
+  }
+}
diff --git a/lib/data_explorer_query_group_bookmarkable.rb b/lib/data_explorer_query_group_bookmarkable.rb
new file mode 100644
index 0000000..b38d72b
--- /dev/null
+++ b/lib/data_explorer_query_group_bookmarkable.rb
@@ -0,0 +1,57 @@
+# frozen_string_literal: true
+
+class DataExplorerQueryGroupBookmarkable < BaseBookmarkable
+  def self.model
+    DataExplorer::QueryGroup
+  end
+
+  def self.serializer
+    UserDataExplorerQueryGroupBookmarkSerializer
+  end
+
+  def self.preload_associations
+    [:data_explorer_queries, :groups]
+  end
+
+  def self.list_query(user, guardian)
+    group_ids = []
+    if !user.admin?
+      group_ids = user.visible_groups.pluck(:id)
+      return if group_ids.empty?
+    end
+
+    query = user.bookmarks_of_type("DataExplorer::QueryGroup")
+      .joins("INNER JOIN data_explorer_query_groups ON data_explorer_query_groups.id = bookmarks.bookmarkable_id")
+      .joins("LEFT JOIN data_explorer_queries ON data_explorer_queries.id = data_explorer_query_groups.query_id")
+    query = query.where("data_explorer_query_groups.group_id IN (?)", group_ids) if !user.admin?
+    query
+  end
+
+  # Searchable only by data_explorer_queries name
+  def self.search_query(bookmarks, query, ts_query, &bookmarkable_search)
+    bookmarkable_search.call(bookmarks,
+      "data_explorer_queries.name ILIKE :q")
+  end
+
+  def self.reminder_handler(bookmark)
+    bookmark.user.notifications.create!(
+      notification_type: Notification.types[:bookmark_reminder],
+      data: {
+        title: bookmark.bookmarkable.query.name,
+        display_username: bookmark.user.username,
+        bookmark_name: bookmark.name,
+        bookmarkable_url: "/g/#{bookmark.bookmarkable.group.name}/reports/#{bookmark.bookmarkable.query.id}"
+      }.to_json
+    )
+  end
+
+  def self.reminder_conditions(bookmark)
+    bookmark.bookmarkable.present?
+  end
+
+  def self.can_see?(guardian, bookmark)
+    return false if !bookmark.bookmarkable.group
+    guardian.user_is_a_member_of_group?(bookmark.bookmarkable.group)
+  end
+
+end
diff --git a/plugin.rb b/plugin.rb
index cbcb2b4..e9bea96 100644
--- a/plugin.rb
+++ b/plugin.rb
@@ -875,6 +875,12 @@ SQL
     end
   end
 
+  load File.expand_path('../lib/data_explorer_query_group_bookmarkable.rb', __FILE__)
+  load File.expand_path('../app/serializers/user_data_explorer_query_group_bookmark_serializer.rb', __FILE__)
+
+  # Making DataExplorer::QueryGroup Bookmarkable.
+  Bookmark.register_bookmarkable(DataExplorerQueryGroupBookmarkable)
+
   require_dependency 'application_controller'
   require_dependency File.expand_path('../lib/queries.rb', __FILE__)
 
diff --git a/spec/fabricators/query_fabricator.rb b/spec/fabricators/query_fabricator.rb
new file mode 100644
index 0000000..895b946
--- /dev/null
+++ b/spec/fabricators/query_fabricator.rb
@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+Fabricator(:query, from: "DataExplorer::Query") do
+  name
+  description
+  sql
+  user
+end
+
+Fabricator(:query_group, from: "DataExplorer::QueryGroup") do
+  query
+  group
+end
diff --git a/spec/lib/data_explorer_query_group_bookmarkable_spec.rb b/spec/lib/data_explorer_query_group_bookmarkable_spec.rb
new file mode 100644
index 0000000..11429ab
--- /dev/null
+++ b/spec/lib/data_explorer_query_group_bookmarkable_spec.rb
@@ -0,0 +1,148 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe DataExplorerQueryGroupBookmarkable do
+  fab!(:admin_user) { Fabricate(:admin) }
+  fab!(:user) { Fabricate(:user) }
+  fab!(:guardian) { Guardian.new(user) }
+  fab!(:group0) { Fabricate(:group) }
+  fab!(:group1) { Fabricate(:group) }
+  fab!(:group2) { Fabricate(:group) }
+  fab!(:group3) { Fabricate(:group) }
+  fab!(:query1) { Fabricate(:query, name: "My First Query",
+                                    description: "This is the description of my 1st query.",
+                                    sql: "Not really important",
+                                    user: admin_user) }
+  fab!(:query2) { Fabricate(:query, name: "My Second Query",
+                                    description: "This is my 2nd query's description.",
+                                    sql: "Not really important",
+                                    user: admin_user) }
+
+  before do
+    SiteSetting.data_explorer_enabled = true
+    Bookmark.register_bookmarkable(DataExplorerQueryGroupBookmarkable)
+  end
+
+  # Groups 0 and 1 have access to the Query 1.
+  let!(:query_group1) { Fabricate(:query_group, query: query1, group: group0) }
+  let!(:query_group2) { Fabricate(:query_group, query: query1, group: group1) }
+  # User is member of both groups.
+  let!(:group_user1) { Fabricate(:group_user, user: user, group: group0) }
+  let!(:group_user2) { Fabricate(:group_user, user: user, group: group1) }
+
+  # Group 1 also has access to query2.
+  let!(:query_group3) { Fabricate(:query_group, query: query2, group: group1) }
+
+  # Group 2 has access to query 1. User is NOT a member of this group.
+  let!(:query_group4) { Fabricate(:query_group, query: query1, group: group2) }
+
+  # User is a member of Group 3, which has no access to Query 1.
+  let!(:group_user3) { Fabricate(:group_user, user: user, group: group3) }
+
+  # User bookmarked the same Query 1 twice, from different Groups (0 and 1)
+  let!(:bookmark1) { Fabricate(:bookmark, user: user,
+                                          bookmarkable: query_group1,
+                                          name: "something i gotta do") }
+  let!(:bookmark2) { Fabricate(:bookmark, user: user,
+                                          bookmarkable: query_group2,
+                                          name: "something else i have to do") }
+
+  # User also bookmarked Query 2 from Group 1.
+  let!(:bookmark3) { Fabricate(:bookmark, user: user,
+                                          bookmarkable: query_group3,
+                                          name: "this is the other query I needed.") }
+
+  # User previously bookmarked Query 1 from Group 2, of which she is no longer a member.
+  let!(:bookmark4) { Fabricate(:bookmark, user: user,
+                                          bookmarkable: query_group4,
+                                          name: "something i gotta do also") }
+
+  subject { RegisteredBookmarkable.new(DataExplorerQueryGroupBookmarkable) }
+
+  describe "#perform_list_query" do
+    it "returns all the user's bookmarks" do
+      expect(subject.perform_list_query(user, guardian).map(&:id)).to match_array([bookmark1.id, bookmark2.id, bookmark3.id])
+    end
+
+    it "does not return bookmarks made from groups that the user is no longer a member of" do
+      expect(subject.perform_list_query(user, guardian).map(&:id)).not_to include(bookmark4.id)
+
+      # remove user from the other groups from which they bookmarked a query
+      group_user1.delete
+      group_user2.delete
+
+      # bookmarks is now empty, because user is not a member of any Groups with permission to see the query
+      expect(subject.perform_list_query(user, guardian)).to be_empty
+    end
+
+  end
+
+  describe "#perform_search_query" do
+    before do
+      SearchIndexer.enable
+    end
+
+    it "returns bookmarks that match by name" do
+      ts_query = Search.ts_query(term: "gotta", ts_config: "simple")
+      expect(subject.perform_search_query(subject.perform_list_query(user, guardian), "%gotta%", ts_query).map(&:id)).to match_array([bookmark1.id])
+    end
+
+    it "returns bookmarks that match by Query name" do
+      ts_query = Search.ts_query(term: "First", ts_config: "simple")
+      expect(subject.perform_search_query(subject.perform_list_query(user, guardian), "%First%", ts_query).map(&:id)).to match_array([bookmark1.id, bookmark2.id])
+    end
+
+  end
+
+  describe "#can_send_reminder?" do
+    it "cannot send the reminder if the group is revoked access to the query" do
+      expect(subject.can_send_reminder?(bookmark1)).to eq(true)
+      bookmark1.bookmarkable.delete
+      bookmark1.reload
+      expect(subject.can_send_reminder?(bookmark1)).to eq(false)
+    end
+  end
+
+  describe "#reminder_handler" do
+    it "creates a notification for the user with the correct details" do
+      expect { subject.send_reminder_notification(bookmark1) }.to change { Notification.count }.by(1)
+      notif = user.notifications.last
+      expect(notif.notification_type).to eq(Notification.types[:bookmark_reminder])
+      expect(notif.data).to eq(
+        {
+          title: bookmark1.bookmarkable.query.name,
+          display_username: bookmark1.user.username,
+          bookmark_name: bookmark1.name,
+          bookmarkable_url: "/g/#{bookmark1.bookmarkable.group.name}/reports/#{bookmark1.bookmarkable.query.id}"
+        }.to_json
+      )
+    end
+  end
+
+  describe "#can_see?" do
+    it "returns false if the user is not a member of the group from which they created the bookmark" do
+      expect(subject.can_see?(guardian, bookmark1)).to eq(true) #Query 1, Group 0
+      expect(subject.can_see?(guardian, bookmark2)).to eq(true) #Query 1, Group 1
+      expect(subject.can_see?(guardian, bookmark3)).to eq(true) #Query 2, Group 1
+      expect(subject.can_see?(guardian, bookmark4)).to eq(false) #Query 1, Group 2
+
+      # remove from Groups 0 and 1
+      group_user1.delete # Group 0
+      group_user2.delete # Group 1
+      guardian.user.reload
+
+      expect(subject.can_see?(guardian, bookmark1)).to eq(false)
+      expect(subject.can_see?(guardian, bookmark2)).to eq(false)
+      expect(subject.can_see?(guardian, bookmark3)).to eq(false)
+      expect(subject.can_see?(guardian, bookmark4)).to eq(false)
+
+      # And adding the user back to the group, just to be sure.
+      Fabricate(:group_user, user: user, group: group2)
+      guardian.user.reload
+
+      expect(subject.can_see?(guardian, bookmark4)).to eq(true)
+    end
+
+  end
+end