DEV: Improve logic for showing/hiding the reports tab in group screens (#107)

Previously this was adding an extra AJAX request to check if the group had any queries available. Now a boolean is included in the group serializer, so there is no need for the extra request.

Removing this ajax request will also stop other plugin JS integration tests from failing when the data-explorer plugin is installed.

This commit also fixes the HTML markup of the tab, so that it doesn't have a <ul> nested inside the existing <ul>. Also adds an icon for good measure.

app/controllers/data_explorer/query_controller.rb

@@ -47,11 +47,7 @@ class DataExplorer::QueryController < ::ApplicationController
 
     respond_to do |format|
       format.json do
-        queries = DataExplorer::Query
-          .where(hidden: false)
-          .joins("INNER JOIN data_explorer_query_groups
-                    ON data_explorer_query_groups.query_id = data_explorer_queries.id
-                    AND data_explorer_query_groups.group_id = #{@group.id}")
+        queries = DataExplorer::Query.for_group(@group)
         render_serialized(queries, DataExplorer::QuerySerializer, root: 'queries')
       end
     end

app/models/data_explorer/query.rb

@@ -7,6 +7,13 @@ module DataExplorer
     has_many :groups, through: :query_groups
     belongs_to :user
 
+    scope :for_group, ->(group) do
+      where(hidden: false)
+        .joins("INNER JOIN data_explorer_query_groups
+              ON data_explorer_query_groups.query_id = data_explorer_queries.id
+              AND data_explorer_query_groups.group_id = #{group.id}")
+    end
+
     def params
       @params ||= DataExplorer::Parameter.create_from_sql(sql)
     end

assets/javascripts/discourse/components/group-reports-nav-item.js.es6

@@ -1,27 +0,0 @@
-import { ajax } from "discourse/lib/ajax";
-
-export default Ember.Component.extend({
-  group: null,
-  showReportsTab: false,
-
-  checkForReports() {
-    return ajax(`/g/${this.group.name}/reports`).then((response) => {
-      return this.set("showReportsTab", response.queries.length > 0);
-    });
-  },
-
-  init(args) {
-    this.set("group", args.group);
-    if (
-      (this.get("currentUser.groups") || []).some(
-        (g) => g.id === this.group.id
-      ) ||
-      this.get("currentUser.admin")
-    ) {
-      // User is a part of the group (or an admin). Now check if the group has reports
-      this.checkForReports();
-    }
-
-    this._super(args);
-  },
-});

assets/javascripts/discourse/connectors/group-reports-nav-item/nav-item.js.es6

@@ -0,0 +1,5 @@
+export default {
+  shouldRender(args) {
+    return args.group.has_visible_data_explorer_queries;
+  },
+};

assets/javascripts/discourse/templates/components/group-reports-nav-item.hbs

@@ -1,9 +0,0 @@
-{{#if showReportsTab}}
-  <ul class ='nav-pills'>
-    <li>
-      {{#link-to 'group.reports'}}
-        {{i18n 'group.reports'}}
-      {{/link-to}}
-    </li>
-  </ul>
-{{/if}}

assets/javascripts/discourse/templates/connectors/group-reports-nav-item/nav-item.hbs

@@ -1 +1,3 @@
-{{group-reports-nav-item group = group}}
+{{#link-to 'group.reports'}}
+  {{d-icon 'chart-bar'}}{{i18n 'group.reports'}}
+{{/link-to}}

plugin.rb

@@ -60,6 +60,14 @@ after_initialize do
     return user_is_a_member_of_group?(group) && query.groups.exists?(id: group.id)
   end
 
+  add_to_serializer(:group_show, :has_visible_data_explorer_queries, false) do
+    DataExplorer::Query.for_group(object).exists?
+  end
+
+  add_to_serializer(:group_show, :include_has_visible_data_explorer_queries?, false) do
+    SiteSetting.data_explorer_enabled && scope.user_is_a_member_of_group?(object)
+  end
+
   module ::DataExplorer
     class Engine < ::Rails::Engine
       engine_name "data_explorer"

spec/requests/group_spec.rb

@@ -0,0 +1,44 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe "Data explorer group serializer additions" do
+  let(:group_user) { Fabricate(:user) }
+  let(:other_user) { Fabricate(:user) }
+  let(:group) { Fabricate(:group) }
+  let!(:query) { DataExplorer::Query.create!(name: "My query", sql: "") }
+
+  before do
+    SiteSetting.data_explorer_enabled = true
+    group.add(group_user)
+    DataExplorer::QueryGroup.create!(group: group, query: query)
+  end
+
+  it "query boolean is true for group user" do
+    sign_in group_user
+    get "/g/#{group.name}.json"
+    expect(response.status).to eq(200)
+    expect(response.parsed_body["group"]["has_visible_data_explorer_queries"]).to eq(true)
+  end
+
+  it "query boolean is false for group user when there are no queries" do
+    query.destroy!
+    sign_in group_user
+    get "/g/#{group.name}.json"
+    expect(response.status).to eq(200)
+    expect(response.parsed_body["group"]["has_visible_data_explorer_queries"]).to eq(false)
+  end
+
+  it "does not include query boolean for anon" do
+    get "/g/#{group.name}.json"
+    expect(response.status).to eq(200)
+    expect(response.parsed_body["group"]["has_visible_data_explorer_queries"]).to eq(nil)
+  end
+
+  it "does not include query boolean for non-group user" do
+    sign_in other_user
+    get "/g/#{group.name}.json"
+    expect(response.status).to eq(200)
+    expect(response.parsed_body["group"]["has_visible_data_explorer_queries"]).to eq(nil)
+  end
+end