FEATURE: introduce data-explorer tables (#61)

Instead of using `PluginStoreRow` we should use plugin-specific models like `DataExplorer::Query` and `DataExplorer::QueryGroup`

app/controllers/data_explorer/query_controller.rb

@@ -0,0 +1,228 @@
+class DataExplorer::QueryController < ::ApplicationController
+  requires_plugin DataExplorer.plugin_name
+
+  before_action :set_group, only: %i(group_reports_index group_reports_show group_reports_run)
+  before_action :set_query, only: %i(group_reports_show group_reports_run show update)
+  skip_before_action :check_xhr, only: %i(show group_reports_run run)
+
+  def index
+    queries = DataExplorer::Query.where(hidden: false).order(:last_run_at, :name).includes(:groups).to_a
+
+    database_queries_ids = DataExplorer::Query.pluck(:id)
+    Queries.default.each do |params|
+      attributes = params.last
+      next if database_queries_ids.include?(attributes["id"])
+      query = DataExplorer::Query.new
+      query.id = attributes["id"]
+      query.sql = attributes["sql"]
+      query.name = attributes["name"]
+      query.description = attributes["description"]
+      query.user_id = Discourse::SYSTEM_USER_ID.to_s
+      queries << query 
+    end
+
+    render_serialized queries, DataExplorer::QuerySerializer, root: 'queries'
+  end
+
+  def show
+    check_xhr unless params[:export]
+
+    if params[:export]
+      response.headers['Content-Disposition'] = "attachment; filename=#{@query.slug}.dcquery.json"
+      response.sending_file = true
+    end
+
+    return raise Discourse::NotFound if !guardian.user_can_access_query?(@query) || @query.hidden
+    render_serialized @query, DataExplorer::QuerySerializer, root: 'query'
+  end
+
+  def groups
+    render json: Group.all.select(:id, :name), root: false
+  end
+
+  def group_reports_index
+    return raise Discourse::NotFound unless guardian.user_is_a_member_of_group?(@group)
+
+    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}")
+        render_serialized(queries, DataExplorer::QuerySerializer, root: 'queries')
+      end
+    end
+  end
+
+  def group_reports_show
+    return raise Discourse::NotFound if !guardian.group_and_user_can_access_query?(@group, @query) || @query.hidden
+
+    respond_to do |format|
+      format.json do
+        render_serialized @query, DataExplorer::QuerySerializer, root: 'query'
+      end
+    end
+  end
+
+  def group_reports_run
+    return raise Discourse::NotFound if !guardian.group_and_user_can_access_query?(@group, @query) || @query.hidden
+
+    run
+  end
+
+  def create
+    query = DataExplorer::Query.create!(params.require(:query).permit(:name, :description, :sql).merge(user_id: current_user.id, last_run_at: Time.now))
+    group_ids = params.require(:query)[:group_ids]
+    group_ids&.each do |group_id|
+      query.query_groups.find_or_create_by!(group_id: group_id)
+    end
+    render_serialized query, DataExplorer::QuerySerializer, root: 'query'
+  end
+
+  def update
+    ActiveRecord::Base.transaction do
+      @query.update!(params.require(:query).permit(:name, :sql, :description).merge(hidden: false))
+
+      group_ids = params.require(:query)[:group_ids]
+      DataExplorer::QueryGroup.where.not(group_id: group_ids).where(query_id: @query.id).delete_all
+      group_ids&.each do |group_id|
+        @query.query_groups.find_or_create_by!(group_id: group_id)
+      end
+    end
+
+    render_serialized @query, DataExplorer::QuerySerializer, root: 'query'
+  rescue DataExplorer::ValidationError => e
+    render_json_error e.message
+  end
+
+  def destroy
+    query = DataExplorer::Query.where(id: params[:id]).first_or_initialize
+    query.update!(hidden: true)
+
+    render json: { success: true, errors: [] }
+  end
+
+  def schema
+    schema_version = DB.query_single("SELECT max(version) AS tag FROM schema_migrations").first
+    if stale?(public: true, etag: schema_version, template: false)
+      render json: DataExplorer.schema
+    end
+  end
+
+  # Return value:
+  # success - true/false. if false, inspect the errors value.
+  # errors - array of strings.
+  # params - hash. Echo of the query parameters as executed.
+  # duration - float. Time to execute the query, in milliseconds, to 1 decimal place.
+  # columns - array of strings. Titles of the returned columns, in order.
+  # explain - string. (Optional - pass explain=true in the request) Postgres query plan, UNIX newlines.
+  # rows - array of array of strings. Results of the query. In the same order as 'columns'.
+  def run
+    check_xhr unless params[:download]
+
+    query = DataExplorer::Query.find(params[:id].to_i)
+    query.update!(last_run_at: Time.now)
+
+    if params[:download]
+      response.sending_file = true
+    end
+
+    params[:params] = params[:_params] if params[:_params] # testing workaround
+    query_params = {}
+    query_params = MultiJson.load(params[:params]) if params[:params]
+
+    opts = { current_user: current_user.username }
+    opts[:explain] = true if params[:explain] == "true"
+
+    opts[:limit] =
+      if params[:format] == "csv"
+        if params[:limit].present?
+          limit = params[:limit].to_i
+          limit = DataExplorer::QUERY_RESULT_MAX_LIMIT if limit > DataExplorer::QUERY_RESULT_MAX_LIMIT
+          limit
+        else
+          DataExplorer::QUERY_RESULT_MAX_LIMIT
+        end
+      elsif params[:limit].present?
+        params[:limit] == "ALL" ? "ALL" : params[:limit].to_i
+      end
+
+    result = DataExplorer.run_query(query, query_params, opts)
+
+    if result[:error]
+      err = result[:error]
+
+      # Pretty printing logic
+      err_class = err.class
+      err_msg = err.message
+      if err.is_a? ActiveRecord::StatementInvalid
+        err_class = err.original_exception.class
+        err_msg.gsub!("#{err_class}:", '')
+      else
+        err_msg = "#{err_class}: #{err_msg}"
+      end
+
+      render json: {
+        success: false,
+        errors: [err_msg]
+      }, status: 422
+    else
+      pg_result = result[:pg_result]
+      cols = pg_result.fields
+      respond_to do |format|
+        format.json do
+          if params[:download]
+            response.headers['Content-Disposition'] =
+              "attachment; filename=#{query.slug}@#{Slug.for(Discourse.current_hostname, 'discourse')}-#{Date.today}.dcqresult.json"
+          end
+          json = {
+            success: true,
+            errors: [],
+            duration: (result[:duration_secs].to_f * 1000).round(1),
+            result_count: pg_result.values.length || 0,
+            params: query_params,
+            columns: cols,
+            default_limit: DataExplorer::QUERY_RESULT_DEFAULT_LIMIT
+          }
+          json[:explain] = result[:explain] if opts[:explain]
+
+          if !params[:download]
+            relations, colrender = DataExplorer.add_extra_data(pg_result)
+            json[:relations] = relations
+            json[:colrender] = colrender
+          end
+
+          json[:rows] = pg_result.values
+
+          render json: json
+        end
+        format.csv do
+          response.headers['Content-Disposition'] =
+            "attachment; filename=#{query.slug}@#{Slug.for(Discourse.current_hostname, 'discourse')}-#{Date.today}.dcqresult.csv"
+
+          require 'csv'
+          text = CSV.generate do |csv|
+            csv << cols
+            pg_result.values.each do |row|
+              csv << row
+            end
+          end
+
+          render plain: text
+        end
+      end
+    end
+  end
+
+  private
+
+  def set_group
+    @group = Group.find_by(name: params["group_name"])
+  end
+
+  def set_query
+    @query = DataExplorer::Query.find_by(id: params[:id])
+    raise Discourse::NotFound unless @query
+  end
+end

app/models/data_explorer/query.rb

@@ -0,0 +1,40 @@
+# frozen_string_literal: true
+
+module DataExplorer
+  class Query < ActiveRecord::Base
+    self.table_name = 'data_explorer_queries'
+    has_many :query_groups
+    has_many :groups, through: :query_groups
+    belongs_to :user
+
+    def params
+      @params ||= DataExplorer::Parameter.create_from_sql(sql)
+    end
+
+    def cast_params(input_params)
+      result = {}.with_indifferent_access
+      self.params.each do |pobj|
+        result[pobj.identifier] = pobj.cast_to_ruby input_params[pobj.identifier]
+      end
+      result
+    end
+
+    def slug
+      Slug.for(name).presence || "query-#{id}"
+    end
+
+    def self.find(id)
+      if id.to_i < 0
+        default_query = Queries.default[id.to_s]
+        return raise ActiveRecord::RecordNotFound unless default_query
+        query = Query.find_by(id: id) || Query.new
+        query.attributes = default_query
+        query.user_id = Discourse::SYSTEM_USER_ID.to_s
+        query
+      else
+        super
+      end
+    end
+  end
+end
+

app/models/data_explorer/query_group.rb

@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+module DataExplorer
+  class QueryGroup < ActiveRecord::Base
+    self.table_name = 'data_explorer_query_groups'
+
+    belongs_to :query
+    belongs_to :group
+  end
+end
+

app/serializers/data_explorer/query_serializer.rb

@@ -0,0 +1,16 @@
+class DataExplorer::QuerySerializer < ActiveModel::Serializer
+  attributes :id, :sql, :name, :description, :param_info, :created_at, :username, :group_ids, :last_run_at, :hidden, :user_id
+
+  def param_info
+    object&.params&.map(&:to_hash)
+  end
+
+  def username
+    object&.user&.username
+  end
+
+  def group_ids
+    object.groups.map(&:id)
+  end
+end
+

app/serializers/data_explorer/small_badge_serializer.rb

@@ -0,0 +1,3 @@
+class DataExplorer::SmallBadgeSerializer < ApplicationSerializer
+  attributes :id, :name, :badge_type, :description, :icon
+end

app/serializers/data_explorer/small_post_with_excerpt_serializer.rb

@@ -0,0 +1,12 @@
+class DataExplorer::SmallPostWithExcerptSerializer < ApplicationSerializer
+  attributes :id, :topic_id, :post_number, :excerpt, :username, :avatar_template
+  def excerpt
+    Post.excerpt(object.cooked, 70)
+  end
+  def username
+    object.user && object.user.username
+  end
+  def avatar_template
+    object.user && object.user.avatar_template
+  end
+end

assets/javascripts/discourse/controllers/admin-plugins-explorer.js.es6

@@ -67,7 +67,7 @@ export default Ember.Controller.extend({
   @computed("groups")
   groupOptions(groups) {
     return groups.map(g => {
-      return { id: g.id.toString(), name: g.name };
+      return { id: g.id, name: g.name };
     });
   },
 

assets/javascripts/discourse/models/query.js.es6

@@ -82,7 +82,7 @@ Query.reopenClass({
     "name",
     "description",
     "sql",
-    "created_by",
+    "user_id",
     "created_at",
     "group_ids",
     "last_run_at"

db/migrate/20200810053843_create_data_explorer_queries.rb

@@ -0,0 +1,92 @@
+# frozen_string_literal: true
+
+class CreateDataExplorerQueries < ActiveRecord::Migration[6.0]
+  def up
+    create_table :data_explorer_queries do |t|
+      t.string :name
+      t.text :description
+      t.text :sql, default: "SELECT 1", null: false
+      t.integer :user_id
+      t.datetime :last_run_at
+      t.boolean :hidden, default: false, null: false
+      t.timestamps
+    end
+
+    create_table :data_explorer_query_groups do |t|
+      t.integer :query_id
+      t.integer :group_id
+      t.index :query_id
+      t.index :group_id
+    end
+    add_index(:data_explorer_query_groups, [:query_id, :group_id], unique: true)
+
+    DB.exec <<~SQL, now: Time.zone.now
+      INSERT INTO data_explorer_queries(id, name, description, sql, user_id, last_run_at, hidden, created_at, updated_at)
+      SELECT
+        (replace(key, 'q:',''))::integer,
+        value::json->>'name',
+        value::json->>'description',
+        value::json->>'sql',
+        (value::json->>'created_by')::integer,
+        CASE WHEN (value::json->'last_run_at')::text = 'null' THEN 
+          null
+        WHEN (value::json->'last_run_at')::text = '""' THEN
+          null
+        ELSE
+          (value::json->'last_run_at')::text::timestamptz
+        END,
+        CASE WHEN (value::json->'hidden')::text = 'null' THEN
+          false
+        ELSE
+          (value::json->'hidden')::text::boolean
+        END,
+        :now,
+        :now
+      FROM plugin_store_rows
+      WHERE plugin_name = 'discourse-data-explorer' AND type_name = 'JSON' AND (replace(key, 'q:',''))::integer < 0
+    SQL
+
+    DB.exec <<~SQL, now: Time.zone.now
+      INSERT INTO data_explorer_queries(name, description, sql, user_id, last_run_at, hidden, created_at, updated_at)
+      SELECT
+        value::json->>'name',
+        value::json->>'description',
+        value::json->>'sql',
+        (value::json->>'created_by')::integer,
+        CASE WHEN (value::json->'last_run_at')::text = 'null' THEN
+          null
+        WHEN (value::json->'last_run_at')::text = '""' THEN
+          null
+        ELSE
+          (value::json->'last_run_at')::text::timestamptz
+        END,
+        CASE WHEN (value::json->'hidden')::text = 'null' THEN 
+          false
+        ELSE
+          (value::json->'hidden')::text::boolean
+        END,
+        :now,
+        :now
+      FROM plugin_store_rows
+      WHERE plugin_name = 'discourse-data-explorer' AND type_name = 'JSON' AND (replace(key, 'q:',''))::integer > 0
+    SQL
+
+    DB.query("SELECT * FROM plugin_store_rows WHERE plugin_name = 'discourse-data-explorer' AND type_name = 'JSON'").each do |row|
+      json = JSON.parse(row.value)
+      next if json['group_ids'].blank?
+      query_id = DB.query("SELECT id FROM data_explorer_queries WHERE
+                          name = ? AND sql = ?", json['name'], json['sql']).first.id
+
+      json['group_ids'].each do |group_id|
+        DB.exec <<~SQL
+          INSERT INTO data_explorer_query_groups(query_id, group_id) 
+          VALUES(#{query_id}, #{group_id})
+        SQL
+      end
+    end
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end

lib/tasks/data_explorer.rake

@@ -4,12 +4,9 @@
 desc "Shows a list of hidden queries"
 task('data_explorer:list_hidden_queries').clear
 task 'data_explorer:list_hidden_queries' => :environment do |t|
-  hidden_queries = []
   puts "\nHidden Queries\n\n"
 
-  DataExplorer::Query.all.each do |query|
+  hidden_queries = DataExplorer::Query.where(hidden: false)
-    hidden_queries.push(query) if query.hidden
-  end
 
   hidden_queries.each do |query|
     puts "Name: #{query.name}"
@@ -25,18 +22,13 @@ task('data_explorer').clear
 task 'data_explorer' => :environment do |t, args|
   args.extras.each do |arg|
     id = arg.to_i
-
+    query = DataExplorer::Query.find_by(id: id)
-    if DataExplorer.pstore_get("q:#{id}").nil?
+    if query
-      puts "\nError finding query with id #{id}"
+      puts "\nFound query with id #{id}"
+      query.update!(hidden: true)
+      puts "Query no.#{id} is now hidden"
     else
-      q = DataExplorer::Query.find(id)
+      puts "\nError finding query with id #{id}"
-      if q
-        puts "\nFound query with id #{id}"
-      end
-
-      q.hidden = true
-      q.save
-      puts "Query no.#{id} is now hidden" if q.hidden
     end
   end
   puts ""
@@ -49,18 +41,13 @@ task('data_explorer:unhide_query').clear
 task 'data_explorer:unhide_query' => :environment do |t, args|
   args.extras.each do |arg|
     id = arg.to_i
-
+    query = DataExplorer::Query.find_by(id: id)
-    if DataExplorer.pstore_get("q:#{id}").nil?
+    if query
-      puts "\nError finding query with id #{id}"
+      puts "\nFound query with id #{id}"
+      query.update!(hidden: false)
+      puts "Query no.#{id} is now visible"
     else
-      q = DataExplorer::Query.find(id)
+      puts "\nError finding query with id #{id}"
-      if q
-        puts "\nFound query with id #{id}"
-      end
-
-      q.hidden = false
-      q.save
-      puts "Query no.#{id} is now visible" unless q.hidden
     end
   end
   puts ""
@@ -73,22 +60,19 @@ task('data_explorer:hard_delete').clear
 task 'data_explorer:hard_delete' => :environment do |t, args|
   args.extras.each do |arg|
     id = arg.to_i
+    query = DataExplorer::Query.find_by(id: id)
+    if query
+      puts "\nFound query with id #{id}"
 
-    if DataExplorer.pstore_get("q:#{id}").nil?
+      if query.hidden
-      puts "\nError finding query with id #{id}"
+        query.destroy!
-    else
-      q = DataExplorer::Query.find(id)
-      if q
-        puts "\nFound query with id #{id}"
-      end
-
-      if q.hidden
-        DataExplorer.pstore_delete "q:#{id}"
         puts "Query no.#{id} has been deleted"
       else
         puts "Query no.#{id} must be hidden in order to hard delete"
         puts "To hide the query, run: " + "rake data_explorer[#{id}]"
       end
+    else
+      puts "\nError finding query with id #{id}"
     end
   end
   puts ""

plugin.rb

@@ -31,21 +31,14 @@ module ::DataExplorer
   def self.plugin_name
     'discourse-data-explorer'.freeze
   end
-
-  def self.pstore_get(key)
-    PluginStore.get(DataExplorer.plugin_name, key)
-  end
-
-  def self.pstore_set(key, value)
-    PluginStore.set(DataExplorer.plugin_name, key, value)
-  end
-
-  def self.pstore_delete(key)
-    PluginStore.remove(DataExplorer.plugin_name, key)
-  end
 end
 
 after_initialize do
+  load File.expand_path('../app/models/data_explorer/query.rb', __FILE__)
+  load File.expand_path('../app/controllers/data_explorer/query_controller.rb', __FILE__)
+  load File.expand_path('../app/serializers/data_explorer/query_serializer.rb', __FILE__)
+  load File.expand_path('../app/serializers/data_explorer/small_badge_serializer.rb', __FILE__)
+  load File.expand_path('../app/serializers/data_explorer/small_post_with_excerpt_serializer.rb', __FILE__)
 
   add_to_class(:guardian, :user_is_a_member_of_group?) do |group|
     return false if !current_user
@@ -53,11 +46,18 @@ after_initialize do
     return current_user.group_ids.include?(group.id)
   end
 
-  add_to_class(:guardian, :user_can_access_query?) do |group, query|
+  add_to_class(:guardian, :user_can_access_query?) do |query|
     return false if !current_user
     return true if current_user.admin?
-    return user_is_a_member_of_group?(group) &&
+    query.groups.blank? || query.groups.any? do |group|
-           query.group_ids.include?(group.id.to_s)
+      user_is_a_member_of_group?(group)
+    end
+  end
+
+  add_to_class(:guardian, :group_and_user_can_access_query?) do |group, query|
+    return false if !current_user
+    return true if current_user.admin?
+    return user_is_a_member_of_group?(group) && query.groups.exists?(id: group.id)
   end
 
   module ::DataExplorer
@@ -69,24 +69,6 @@ after_initialize do
     class ValidationError < StandardError
     end
 
-    class SmallBadgeSerializer < ApplicationSerializer
-      attributes :id, :name, :badge_type, :description, :icon
-    end
-
-    class SmallPostWithExcerptSerializer < ApplicationSerializer
-      attributes :id, :topic_id, :post_number, :excerpt
-      attributes :username, :avatar_template
-      def excerpt
-        Post.excerpt(object.cooked, 70)
-      end
-      def username
-        object.user && object.user.username
-      end
-      def avatar_template
-        object.user && object.user.avatar_template
-      end
-    end
-
     # Run a data explorer query on the currently connected database.
     #
     # @param [DataExplorer::Query] query the Query object to run
@@ -633,143 +615,6 @@ SQL
     end
   end
 
-  # Reimplement a couple ActiveRecord methods, but use PluginStore for storage instead
-  require_dependency File.expand_path('../lib/queries.rb', __FILE__)
-  class DataExplorer::Query
-    attr_accessor :id, :name, :description, :sql, :created_by, :created_at, :group_ids, :last_run_at, :hidden
-
-    def initialize
-      @name = 'Unnamed Query'
-      @description = ''
-      @sql = 'SELECT 1'
-      @group_ids = []
-      @hidden = false
-    end
-
-    def slug
-      Slug.for(name).presence || "query-#{id}"
-    end
-
-    def params
-      @params ||= DataExplorer::Parameter.create_from_sql(sql)
-    end
-
-    def check_params!
-      DataExplorer::Parameter.create_from_sql(sql, strict: true)
-      nil
-    end
-
-    def cast_params(input_params)
-      result = {}.with_indifferent_access
-      self.params.each do |pobj|
-        result[pobj.identifier] = pobj.cast_to_ruby input_params[pobj.identifier]
-      end
-      result
-    end
-
-    def can_be_run_by(group)
-      @group_ids.include?(group.id.to_s)
-    end
-
-    # saving/loading functions
-    # May want to extract this into a library or something for plugins to use?
-    def self.alloc_id
-      DistributedMutex.synchronize('data-explorer_query-id') do
-        max_id = DataExplorer.pstore_get("q:_id")
-        max_id = 1 unless max_id
-        DataExplorer.pstore_set("q:_id", max_id + 1)
-        max_id
-      end
-    end
-
-    def self.from_hash(h)
-      query = DataExplorer::Query.new
-      [:name, :description, :sql, :created_by, :created_at, :last_run_at].each do |sym|
-        query.send("#{sym}=", h[sym].strip) if h[sym]
-      end
-      group_ids = (h[:group_ids] == "" || !h[:group_ids]) ? [] : h[:group_ids]
-      query.group_ids = group_ids
-      query.id = h[:id].to_i if h[:id]
-      query.hidden = h[:hidden]
-      query
-    end
-
-    def to_hash
-      {
-        id: @id,
-        name: @name,
-        description: @description,
-        sql: @sql,
-        created_by: @created_by,
-        created_at: @created_at,
-        group_ids: @group_ids,
-        last_run_at: @last_run_at,
-        hidden: @hidden
-      }
-    end
-
-    def self.find(id, opts = {})
-      if DataExplorer.pstore_get("q:#{id}").nil? && id < 0
-        hash = Queries.default[id.to_s]
-        hash[:id] = id
-        from_hash hash
-      else
-        unless hash = DataExplorer.pstore_get("q:#{id}")
-          return DataExplorer::Query.new if opts[:ignore_deleted]
-          raise Discourse::NotFound
-        end
-        from_hash hash
-      end
-    end
-
-    def save
-      check_params!
-      return save_default_query if @id && @id < 0
-
-      @id = @id || self.class.alloc_id
-      DataExplorer.pstore_set "q:#{id}", to_hash
-    end
-
-    def save_default_query
-      check_params!
-      # Read from queries.rb again to pick up any changes and save them
-      query = Queries.default[id.to_s]
-      @id = query["id"]
-      @sql = query["sql"]
-      @group_ids = @group_ids || []
-      @name = query["name"]
-      @description = query["description"]
-
-      DataExplorer.pstore_set "q:#{id}", to_hash
-    end
-
-    def destroy
-      # Instead of deleting the query from the store, we can set
-      # it to be hidden and not send it to the frontend
-      @hidden = true
-      DataExplorer.pstore_set "q:#{id}", to_hash
-    end
-
-    def read_attribute_for_serialization(attr)
-      self.send(attr)
-    end
-
-    def self.all
-      PluginStoreRow.where(plugin_name: DataExplorer.plugin_name)
-        .where("key LIKE 'q:%'")
-        .where("key != 'q:_id'")
-        .map do |psr|
-          DataExplorer::Query.from_hash PluginStore.cast_value(psr.type_name, psr.value)
-        end.sort_by { |query| query.name }
-    end
-
-    def self.destroy_all
-      PluginStoreRow.where(plugin_name: DataExplorer.plugin_name)
-        .where("key LIKE 'q:%'")
-        .destroy_all
-    end
-  end
-
   class DataExplorer::Parameter
     attr_accessor :identifier, :type, :default, :nullable
 
@@ -1012,279 +857,6 @@ SQL
 
   require_dependency 'application_controller'
   require_dependency File.expand_path('../lib/queries.rb', __FILE__)
-  class DataExplorer::QueryController < ::ApplicationController
-    requires_plugin DataExplorer.plugin_name
-
-    before_action :check_enabled
-    before_action :set_group, only: [:group_reports_index, :group_reports_show, :group_reports_run]
-    before_action :set_query, only: [:group_reports_show, :group_reports_run]
-
-    attr_reader :group, :query
-
-    def check_enabled
-      raise Discourse::NotFound unless SiteSetting.data_explorer_enabled?
-    end
-
-    def set_group
-      @group = Group.find_by(name: params["group_name"])
-    end
-
-    def set_query
-      @query = DataExplorer::Query.find(params[:id].to_i)
-    end
-
-    def index
-      # guardian.ensure_can_use_data_explorer!
-      queries = []
-      DataExplorer::Query.all.each do |query|
-        queries.push(query) unless query.hidden
-      end
-
-      Queries.default.each do |params|
-        query = DataExplorer::Query.new
-        query.id = params.second["id"]
-        query.sql = params.second["sql"]
-        query.name = params.second["name"]
-        query.description = params.second["description"]
-        query.created_by = Discourse::SYSTEM_USER_ID.to_s
-
-        # don't render this query if query with the same id already exists in pstore
-        queries.push(query) unless DataExplorer.pstore_get("q:#{query.id}").present?
-      end
-
-      render_serialized queries, DataExplorer::QuerySerializer, root: 'queries'
-    end
-
-    skip_before_action :check_xhr, only: [:show]
-    def show
-      check_xhr unless params[:export]
-
-      query = DataExplorer::Query.find(params[:id].to_i)
-
-      if params[:export]
-        response.headers['Content-Disposition'] = "attachment; filename=#{query.slug}.dcquery.json"
-        response.sending_file = true
-      end
-
-      # guardian.ensure_can_see! query
-      render_serialized query, DataExplorer::QuerySerializer, root: 'query'
-    end
-
-    def groups
-      render_serialized(Group.all, BasicGroupSerializer)
-    end
-
-    def group_reports_index
-      return raise Discourse::NotFound unless guardian.user_is_a_member_of_group?(group)
-
-      respond_to do |format|
-        format.html { render 'groups/show' }
-        format.json do
-          queries = DataExplorer::Query.all.select do |query|
-            !query.hidden && query.group_ids&.include?(group.id.to_s)
-          end
-          render_serialized(queries, DataExplorer::QuerySerializer, root: 'queries')
-        end
-      end
-    end
-
-    def group_reports_show
-      return raise Discourse::NotFound if !guardian.user_can_access_query?(group, query) || query.hidden
-
-      respond_to do |format|
-        format.html { render 'groups/show' }
-        format.json do
-          render_serialized query, DataExplorer::QuerySerializer, root: 'query'
-        end
-      end
-    end
-
-    skip_before_action :check_xhr, only: [:group_reports_run]
-    def group_reports_run
-      return raise Discourse::NotFound if !guardian.user_can_access_query?(group, query) || query.hidden
-
-      run
-    end
-
-    def create
-      # guardian.ensure_can_create_explorer_query!
-
-      query = DataExplorer::Query.from_hash params.require(:query)
-      query.created_at = Time.now
-      query.created_by = current_user.id.to_s
-      query.last_run_at = Time.now
-      query.id = nil # json import will assign an id, which is wrong
-      query.save
-
-      render_serialized query, DataExplorer::QuerySerializer, root: 'query'
-    end
-
-    def update
-      query = DataExplorer::Query.find(params[:id].to_i, ignore_deleted: true)
-      hash = params.require(:query)
-      hash[:group_ids] ||= []
-
-      # Undeleting
-      unless query.id
-        if hash[:id]
-          query.id = hash[:id].to_i
-        else
-          raise Discourse::NotFound
-        end
-      end
-
-      [:name, :sql, :description, :created_by, :created_at, :group_ids, :last_run_at, :hidden].each do |sym|
-        query.send("#{sym}=", hash[sym]) if hash[sym]
-      end
-
-      query.check_params!
-      query.hidden = false
-      query.save
-
-      render_serialized query, DataExplorer::QuerySerializer, root: 'query'
-    rescue DataExplorer::ValidationError => e
-      render_json_error e.message
-    end
-
-    def destroy
-      query = DataExplorer::Query.find(params[:id].to_i)
-      query.destroy
-
-      render json: { success: true, errors: [] }
-    end
-
-    def schema
-      schema_version = DB.query_single("SELECT max(version) AS tag FROM schema_migrations").first
-      if stale?(public: true, etag: schema_version, template: false)
-        render json: DataExplorer.schema
-      end
-    end
-
-    skip_before_action :check_xhr, only: [:run]
-    # Return value:
-    # success - true/false. if false, inspect the errors value.
-    # errors - array of strings.
-    # params - hash. Echo of the query parameters as executed.
-    # duration - float. Time to execute the query, in milliseconds, to 1 decimal place.
-    # columns - array of strings. Titles of the returned columns, in order.
-    # explain - string. (Optional - pass explain=true in the request) Postgres query plan, UNIX newlines.
-    # rows - array of array of strings. Results of the query. In the same order as 'columns'.
-    def run
-      check_xhr unless params[:download]
-
-      query = DataExplorer::Query.find(params[:id].to_i)
-      query.last_run_at = Time.now
-
-      if params[:id].to_i < 0
-        query.created_by = Discourse::SYSTEM_USER_ID.to_s
-        query.save_default_query
-      else
-        query.save
-      end
-
-      if params[:download]
-        response.sending_file = true
-      end
-
-      params[:params] = params[:_params] if params[:_params] # testing workaround
-      query_params = {}
-      query_params = MultiJson.load(params[:params]) if params[:params]
-
-      opts = { current_user: current_user.username }
-      opts[:explain] = true if params[:explain] == "true"
-
-      opts[:limit] =
-        if params[:format] == "csv"
-          if params[:limit].present?
-            limit = params[:limit].to_i
-            limit = DataExplorer::QUERY_RESULT_MAX_LIMIT if limit > DataExplorer::QUERY_RESULT_MAX_LIMIT
-            limit
-          else
-            DataExplorer::QUERY_RESULT_MAX_LIMIT
-          end
-        elsif params[:limit].present?
-          params[:limit] == "ALL" ? "ALL" : params[:limit].to_i
-        end
-
-      result = DataExplorer.run_query(query, query_params, opts)
-
-      if result[:error]
-        err = result[:error]
-
-        # Pretty printing logic
-        err_class = err.class
-        err_msg = err.message
-        if err.is_a? ActiveRecord::StatementInvalid
-          err_class = err.original_exception.class
-          err_msg.gsub!("#{err_class}:", '')
-        else
-          err_msg = "#{err_class}: #{err_msg}"
-        end
-
-        render json: {
-                 success: false,
-                 errors: [err_msg]
-               }, status: 422
-      else
-        pg_result = result[:pg_result]
-        cols = pg_result.fields
-        respond_to do |format|
-          format.json do
-            if params[:download]
-              response.headers['Content-Disposition'] =
-                "attachment; filename=#{query.slug}@#{Slug.for(Discourse.current_hostname, 'discourse')}-#{Date.today}.dcqresult.json"
-            end
-            json = {
-              success: true,
-              errors: [],
-              duration: (result[:duration_secs].to_f * 1000).round(1),
-              result_count: pg_result.values.length || 0,
-              params: query_params,
-              columns: cols,
-              default_limit: DataExplorer::QUERY_RESULT_DEFAULT_LIMIT
-            }
-            json[:explain] = result[:explain] if opts[:explain]
-
-            if !params[:download]
-              relations, colrender = DataExplorer.add_extra_data(pg_result)
-              json[:relations] = relations
-              json[:colrender] = colrender
-            end
-
-            json[:rows] = pg_result.values
-
-            render json: json
-          end
-          format.csv do
-            response.headers['Content-Disposition'] =
-              "attachment; filename=#{query.slug}@#{Slug.for(Discourse.current_hostname, 'discourse')}-#{Date.today}.dcqresult.csv"
-
-            require 'csv'
-            text = CSV.generate do |csv|
-              csv << cols
-              pg_result.values.each do |row|
-                csv << row
-              end
-            end
-
-            render plain: text
-          end
-        end
-      end
-    end
-  end
-
-  class DataExplorer::QuerySerializer < ActiveModel::Serializer
-    attributes :id, :sql, :name, :description, :param_info, :created_by, :created_at, :username, :group_ids, :last_run_at, :hidden
-
-    def param_info
-      object.params.map(&:to_hash) rescue nil
-    end
-
-    def username
-      User.find(created_by).username rescue nil
-    end
-  end
 
   DataExplorer::Engine.routes.draw do
     root to: "query#index"

spec/controllers/queries_controller_spec.rb

@@ -12,15 +12,11 @@ describe DataExplorer::QueryController do
   end
 
   def make_query(sql, opts = {}, group_ids = [])
-    q = DataExplorer::Query.new
+    query = DataExplorer::Query.create!(name: opts[:name] || "Query number", description: "A description for query number", sql: sql, hidden: opts[:hidden] || false)
-    q.id = Fabrication::Sequencer.sequence("query-id", 1)
+    group_ids.each do |group_id|
-    q.name = opts[:name] || "Query number #{q.id}"
+      query.query_groups.create!(group_id: group_id)
-    q.description = "A description for query number #{q.id}"
+    end
-    q.group_ids = group_ids
+    query
-    q.sql = sql
-    q.hidden = opts[:hidden] || false
-    q.save
-    q
   end
 
   describe "Admin" do

spec/guardian_spec.rb

@@ -7,13 +7,14 @@ describe Guardian do
   before { SiteSetting.data_explorer_enabled = true }
 
   def make_query(group_ids = [])
-    q = DataExplorer::Query.new
+    query = DataExplorer::Query.create!(name: "Query number #{Fabrication::Sequencer.sequence("query-id", 1)}", sql: "SELECT 1")
-    q.id = Fabrication::Sequencer.sequence("query-id", 1)
+    group_ids.each do |group_id|
-    q.name = "Query number #{q.id}"
+      query.query_groups.create!(group_id: group_id)
-    q.sql = "SELECT 1"
+    end
-    q.group_ids = group_ids
+    group_ids.each do |group_id|
-    q.save
+      query.query_groups.create!(group_id: group_id)
-    q
+    end
+    query
   end
 
   let(:user) { build(:user) }
@@ -37,30 +38,30 @@ describe Guardian do
     end
   end
 
-  describe "#user_can_access_query?" do
+  describe "#group_and_user_can_access_query?" do
     let(:group) { Fabricate(:group) }
 
     it "is true if the user is an admin" do
-      expect(Guardian.new(admin).user_can_access_query?(group, make_query)).to eq(true)
+      expect(Guardian.new(admin).group_and_user_can_access_query?(group, make_query)).to eq(true)
     end
 
     it "is true if the user is a member of the group, and query contains the group id" do
       query = make_query(["#{group.id}"])
       group.add(user)
 
-      expect(Guardian.new(user).user_can_access_query?(group, query)).to eq(true)
+      expect(Guardian.new(user).group_and_user_can_access_query?(group, query)).to eq(true)
     end
 
     it "is false if the query does not contain the group id" do
       group.add(user)
 
-      expect(Guardian.new(user).user_can_access_query?(group, make_query)).to eq(false)
+      expect(Guardian.new(user).group_and_user_can_access_query?(group, make_query)).to eq(false)
     end
 
     it "is false if the user is not member of the group" do
       query = make_query(["#{group.id}"])
 
-      expect(Guardian.new(user).user_can_access_query?(group, query)).to eq(false)
+      expect(Guardian.new(user).group_and_user_can_access_query?(group, query)).to eq(false)
     end
   end
 end

spec/lib/tasks/data_explorer_spec.rb

@@ -13,25 +13,15 @@ describe 'Data Explorer rake tasks' do
   end
 
   def make_query(sql, opts = {}, group_ids = [])
-    q = DataExplorer::Query.new
+    query = DataExplorer::Query.create!(id: opts[:id], name: opts[:name] || "Query number", description: "A description for query number", sql: sql, hidden: opts[:hidden] || false)
-    q.id = opts[:id] || Fabrication::Sequencer.sequence("query-id", 1)
+    group_ids.each do |group_id|
-    q.name = opts[:name] || "Query number #{q.id}"
+      query.query_groups.create!(group_id: group_id)
-    q.description = "A description for query number #{q.id}"
+    end
-    q.group_ids = group_ids
+    query
-    q.sql = sql
-    q.hidden = opts[:hidden] || false
-    q.save
-    q
   end
 
   def hidden_queries
-    hidden_queries = []
+    DataExplorer::Query.where(hidden: true)
-
-    DataExplorer::Query.all.each do |query|
-      hidden_queries.push(query) if query.hidden
-    end
-
-    hidden_queries
   end
 
   describe 'data_explorer' do