From d726c4889e5c5d7726ed1035c37d0cd60a3bb3d6 Mon Sep 17 00:00:00 2001 From: Gary Pendergast Date: Mon, 10 Feb 2025 14:54:01 +1100 Subject: [PATCH] DEV: Split the Query Listing and Query Editing code (#356) The code for listing all of the defined queries is mixed together with the code for editing a single query. Notably, this results in large amounts of unnecessary data being loaded for the list view, which causes substantial rendering slowdowns. To address this issue, we now only load the necessary data for the list view, and load the full data when it's actually needed (any endpoint that returns a single query). The primary changes that achieve this are: - Create a new `QueryDetailsSerializer` serialiser, which includes all of the query info, and change the existing `QuerySerializer` serialiser to only include the necessary attributes of each query for generating a list of them all. - Split the monolith `/plugins/explorer` route into `/plugins/explorer` for showing just the list of queries, and `/plugins/explorer/queries/:query_id`, for showing/editing/running a specific query. --- .../query_controller.rb | 8 +- .../query_details_serializer.rb | 11 + .../query_serializer.rb | 16 +- .../admin-plugins-explorer-index.js | 192 +++++++++ ...admin-plugins-explorer-queries-details.js} | 172 +------- .../discourse/explorer-route-map.js | 6 +- assets/javascripts/discourse/models/query.js | 8 +- .../routes/admin-plugins-explorer-index.js | 47 ++ ...admin-plugins-explorer-queries-details.js} | 16 +- .../admin/plugins-explorer-index.hbs | 166 +++++++ .../plugins-explorer-queries-details.hbs | 223 ++++++++++ .../templates/admin/plugins-explorer.hbs | 405 ------------------ config/locales/server.en.yml | 4 +- config/routes.rb | 2 +- lib/discourse_data_explorer/data_explorer.rb | 2 +- spec/system/explorer_spec.rb | 28 +- spec/system/param_input_spec.rb | 2 +- .../acceptance/list-queries-test.js | 22 - test/javascripts/acceptance/new-query-test.js | 28 +- .../acceptance/param-input-test.js | 83 ++-- test/javascripts/acceptance/run-query-test.js | 63 ++- 21 files changed, 837 insertions(+), 667 deletions(-) create mode 100644 app/serializers/discourse_data_explorer/query_details_serializer.rb create mode 100644 assets/javascripts/discourse/controllers/admin-plugins-explorer-index.js rename assets/javascripts/discourse/controllers/{admin-plugins-explorer.js => admin-plugins-explorer-queries-details.js} (56%) create mode 100644 assets/javascripts/discourse/routes/admin-plugins-explorer-index.js rename assets/javascripts/discourse/routes/{admin-plugins-explorer.js => admin-plugins-explorer-queries-details.js} (72%) create mode 100644 assets/javascripts/discourse/templates/admin/plugins-explorer-index.hbs create mode 100644 assets/javascripts/discourse/templates/admin/plugins-explorer-queries-details.hbs delete mode 100644 assets/javascripts/discourse/templates/admin/plugins-explorer.hbs diff --git a/app/controllers/discourse_data_explorer/query_controller.rb b/app/controllers/discourse_data_explorer/query_controller.rb index 2438a7a..84c6263 100644 --- a/app/controllers/discourse_data_explorer/query_controller.rb +++ b/app/controllers/discourse_data_explorer/query_controller.rb @@ -40,7 +40,7 @@ module ::DiscourseDataExplorer end return raise Discourse::NotFound if !guardian.user_can_access_query?(@query) || @query.hidden - render_serialized @query, QuerySerializer, root: "query" + render_serialized @query, QueryDetailsSerializer, root: "query" end def groups @@ -68,7 +68,7 @@ module ::DiscourseDataExplorer query_group = QueryGroup.find_by(query_id: @query.id, group_id: @group.id) render json: { - query: serialize_data(@query, QuerySerializer, root: nil), + query: serialize_data(@query, QueryDetailsSerializer, root: nil), query_group: serialize_data(query_group, QueryGroupSerializer, root: nil), } end @@ -93,7 +93,7 @@ module ::DiscourseDataExplorer ) group_ids = params.require(:query)[:group_ids] group_ids&.each { |group_id| query.query_groups.find_or_create_by!(group_id: group_id) } - render_serialized query, QuerySerializer, root: "query" + render_serialized query, QueryDetailsSerializer, root: "query" end def update @@ -107,7 +107,7 @@ module ::DiscourseDataExplorer group_ids&.each { |group_id| @query.query_groups.find_or_create_by!(group_id: group_id) } end - render_serialized @query, QuerySerializer, root: "query" + render_serialized @query, QueryDetailsSerializer, root: "query" rescue ValidationError => e render_json_error e.message end diff --git a/app/serializers/discourse_data_explorer/query_details_serializer.rb b/app/serializers/discourse_data_explorer/query_details_serializer.rb new file mode 100644 index 0000000..d211777 --- /dev/null +++ b/app/serializers/discourse_data_explorer/query_details_serializer.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module ::DiscourseDataExplorer + class QueryDetailsSerializer < QuerySerializer + attributes :sql, :param_info, :created_at, :hidden + + def param_info + object&.params&.uniq { |p| p.identifier }&.map(&:to_hash) + end + end +end diff --git a/app/serializers/discourse_data_explorer/query_serializer.rb b/app/serializers/discourse_data_explorer/query_serializer.rb index 2a87ad4..724e255 100644 --- a/app/serializers/discourse_data_explorer/query_serializer.rb +++ b/app/serializers/discourse_data_explorer/query_serializer.rb @@ -2,21 +2,7 @@ module ::DiscourseDataExplorer class 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&.uniq { |p| p.identifier }&.map(&:to_hash) - end + attributes :id, :name, :description, :username, :group_ids, :last_run_at, :user_id def username object&.user&.username diff --git a/assets/javascripts/discourse/controllers/admin-plugins-explorer-index.js b/assets/javascripts/discourse/controllers/admin-plugins-explorer-index.js new file mode 100644 index 0000000..9b28f88 --- /dev/null +++ b/assets/javascripts/discourse/controllers/admin-plugins-explorer-index.js @@ -0,0 +1,192 @@ +import { tracked } from "@glimmer/tracking"; +import Controller from "@ember/controller"; +import { action } from "@ember/object"; +import { service } from "@ember/service"; +import { Promise } from "rsvp"; +import { popupAjaxError } from "discourse/lib/ajax-error"; +import { bind } from "discourse/lib/decorators"; +import { i18n } from "discourse-i18n"; + +export default class PluginsExplorerController extends Controller { + @service dialog; + @service appEvents; + @service router; + + @tracked sortByProperty = "last_run_at"; + @tracked sortDescending = true; + @tracked params; + @tracked search; + @tracked newQueryName; + @tracked showCreate; + @tracked loading = false; + + queryParams = ["id"]; + explain = false; + acceptedImportFileTypes = ["application/json"]; + order = null; + form = null; + + get sortedQueries() { + const sortedQueries = this.model.sortBy(this.sortByProperty); + return this.sortDescending ? sortedQueries.reverse() : sortedQueries; + } + + get parsedParams() { + return this.params ? JSON.parse(this.params) : null; + } + + get filteredContent() { + const regexp = new RegExp(this.search, "i"); + return this.sortedQueries.filter( + (result) => regexp.test(result.name) || regexp.test(result.description) + ); + } + + get createDisabled() { + return (this.newQueryName || "").trim().length === 0; + } + + addCreatedRecord(record) { + this.model.pushObject(record); + this.router.transitionTo( + "adminPlugins.explorer.queries.details", + record.id + ); + } + + async _importQuery(file) { + const json = await this._readFileAsTextAsync(file); + const query = this._parseQuery(json); + const record = this.store.createRecord("query", query); + const response = await record.save(); + return response.target; + } + + _parseQuery(json) { + const parsed = JSON.parse(json); + const query = parsed.query; + if (!query || !query.sql) { + throw new TypeError(); + } + query.id = 0; // 0 means no Id yet + return query; + } + + _readFileAsTextAsync(file) { + return new Promise((resolve, reject) => { + const reader = new FileReader(); + reader.onload = () => { + resolve(reader.result); + }; + reader.onerror = reject; + + reader.readAsText(file); + }); + } + + @bind + dragMove(e) { + if (!e.movementY && !e.movementX) { + return; + } + + const editPane = document.querySelector(".query-editor"); + const target = editPane.querySelector(".panels-flex"); + const grippie = editPane.querySelector(".grippie"); + + // we need to get the initial height / width of edit pane + // before we manipulate the size + if (!this.initialPaneWidth && !this.originalPaneHeight) { + this.originalPaneWidth = target.clientWidth; + this.originalPaneHeight = target.clientHeight; + } + + const newHeight = Math.max( + this.originalPaneHeight, + target.clientHeight + e.movementY + ); + const newWidth = Math.max( + this.originalPaneWidth, + target.clientWidth + e.movementX + ); + + target.style.height = newHeight + "px"; + target.style.width = newWidth + "px"; + grippie.style.width = newWidth + "px"; + this.appEvents.trigger("ace:resize"); + } + + @bind + scrollTop() { + window.scrollTo(0, 0); + } + + @action + async import(files) { + try { + this.loading = true; + const file = files[0]; + const record = await this._importQuery(file); + this.addCreatedRecord(record); + } catch (e) { + if (e.jqXHR) { + popupAjaxError(e); + } else if (e instanceof SyntaxError) { + this.dialog.alert(i18n("explorer.import.unparseable_json")); + } else if (e instanceof TypeError) { + this.dialog.alert(i18n("explorer.import.wrong_json")); + } else { + this.dialog.alert(i18n("errors.desc.unknown")); + // eslint-disable-next-line no-console + console.error(e); + } + } finally { + this.loading = false; + } + } + + @action + displayCreate() { + this.showCreate = true; + } + + @action + resetParams() { + this.selectedItem.resetParams(); + } + + @action + updateSortProperty(property) { + if (this.sortByProperty === property) { + this.sortDescending = !this.sortDescending; + } else { + this.sortByProperty = property; + this.sortDescending = true; + } + } + + @action + async create() { + try { + const name = this.newQueryName.trim(); + this.loading = true; + this.showCreate = false; + const result = await this.store.createRecord("query", { name }).save(); + this.addCreatedRecord(result.target); + } catch (error) { + popupAjaxError(error); + } finally { + this.loading = false; + } + } + + @action + updateSearch(value) { + this.search = value; + } + + @action + updateNewQueryName(value) { + this.newQueryName = value; + } +} diff --git a/assets/javascripts/discourse/controllers/admin-plugins-explorer.js b/assets/javascripts/discourse/controllers/admin-plugins-explorer-queries-details.js similarity index 56% rename from assets/javascripts/discourse/controllers/admin-plugins-explorer.js rename to assets/javascripts/discourse/controllers/admin-plugins-explorer-queries-details.js index 578953e..694cca3 100644 --- a/assets/javascripts/discourse/controllers/admin-plugins-explorer.js +++ b/assets/javascripts/discourse/controllers/admin-plugins-explorer-queries-details.js @@ -6,44 +6,29 @@ import { Promise } from "rsvp"; import { ajax } from "discourse/lib/ajax"; import { popupAjaxError } from "discourse/lib/ajax-error"; import { bind } from "discourse/lib/decorators"; -import { i18n } from "discourse-i18n"; import QueryHelp from "discourse/plugins/discourse-data-explorer/discourse/components/modal/query-help"; import { ParamValidationError } from "discourse/plugins/discourse-data-explorer/discourse/components/param-input-form"; import Query from "discourse/plugins/discourse-data-explorer/discourse/models/query"; -const NoQuery = Query.create({ name: "No queries", fake: true, group_ids: [] }); - export default class PluginsExplorerController extends Controller { @service modal; - @service dialog; @service appEvents; @service router; - @tracked sortByProperty = "last_run_at"; - @tracked sortDescending = true; @tracked params; - @tracked search; - @tracked newQueryName; - @tracked showCreate; @tracked editingName = false; @tracked editingQuery = false; - @tracked selectedQueryId; @tracked loading = false; @tracked showResults = false; @tracked hideSchema = false; - @tracked results = this.selectedItem.results; + @tracked results = this.model.results; @tracked dirty = false; - queryParams = ["params", { selectedQueryId: "id" }]; + queryParams = ["params"]; explain = false; - acceptedImportFileTypes = ["application/json"]; order = null; form = null; - get validQueryPresent() { - return !!this.selectedItem.id; - } - get saveDisabled() { return !this.dirty; } @@ -52,33 +37,12 @@ export default class PluginsExplorerController extends Controller { return this.dirty; } - get sortedQueries() { - const sortedQueries = this.model.sortBy(this.sortByProperty); - return this.sortDescending ? sortedQueries.reverse() : sortedQueries; - } - get parsedParams() { return this.params ? JSON.parse(this.params) : null; } - get filteredContent() { - const regexp = new RegExp(this.search, "i"); - return this.sortedQueries.filter( - (result) => regexp.test(result.name) || regexp.test(result.description) - ); - } - - get createDisabled() { - return (this.newQueryName || "").trim().length === 0; - } - - get selectedItem() { - const query = this.model.findBy("id", parseInt(this.selectedQueryId, 10)); - return query || NoQuery; - } - get editDisabled() { - return parseInt(this.selectedQueryId, 10) < 0 ? true : false; + return parseInt(this.model.id, 10) < 0 ? true : false; } get groupOptions() { @@ -89,27 +53,11 @@ export default class PluginsExplorerController extends Controller { }); } - get othersDirty() { - return !!this.model.find((q) => q !== this.selectedItem && this.dirty); - } - - addCreatedRecord(record) { - this.model.pushObject(record); - this.selectedQueryId = record.id; - this.dirty = false; - this.setProperties({ - showResults: false, - results: null, - editingName: true, - editingQuery: true, - }); - } - @action async save() { try { this.loading = true; - await this.selectedItem.save(); + await this.model.save(); this.dirty = false; this.editingName = false; @@ -194,17 +142,10 @@ export default class PluginsExplorerController extends Controller { @bind didEndDrag() {} - @bind - scrollTop() { - window.scrollTo(0, 0); - this.editingName = false; - this.editingQuery = false; - } - @action updateGroupIds(value) { this.dirty = true; - this.selectedItem.set("group_ids", value); + this.model.set("group_ids", value); } @action @@ -212,36 +153,6 @@ export default class PluginsExplorerController extends Controller { this.hideSchema = value; } - @action - async import(files) { - try { - this.loading = true; - const file = files[0]; - const record = await this._importQuery(file); - this.addCreatedRecord(record); - } catch (e) { - if (e.jqXHR) { - popupAjaxError(e); - } else if (e instanceof SyntaxError) { - this.dialog.alert(i18n("explorer.import.unparseable_json")); - } else if (e instanceof TypeError) { - this.dialog.alert(i18n("explorer.import.wrong_json")); - } else { - this.dialog.alert(i18n("errors.desc.unknown")); - // eslint-disable-next-line no-console - console.error(e); - } - } finally { - this.loading = false; - this.dirty = true; - } - } - - @action - displayCreate() { - this.showCreate = true; - } - @action editName() { this.editingName = true; @@ -254,18 +165,12 @@ export default class PluginsExplorerController extends Controller { @action download() { - window.open(this.selectedItem.downloadUrl, "_blank"); + window.open(this.model.downloadUrl, "_blank"); } @action goHome() { - this.order = null; - this.showResults = false; - this.selectedQueryId = null; - this.params = null; - this.sortByProperty = "last_run_at"; - this.sortDescending = true; - this.router.transitionTo({ queryParams: { id: null, params: null } }); + this.router.transitionTo("adminPlugins.explorer"); } @action @@ -275,48 +180,17 @@ export default class PluginsExplorerController extends Controller { @action resetParams() { - this.selectedItem.resetParams(); - } - - @action - updateSortProperty(property) { - if (this.sortByProperty === property) { - this.sortDescending = !this.sortDescending; - } else { - this.sortByProperty = property; - this.sortDescending = true; - } - } - - @action - async create() { - try { - const name = this.newQueryName.trim(); - this.loading = true; - this.showCreate = false; - const result = await this.store.createRecord("query", { name }).save(); - this.addCreatedRecord(result.target); - } catch (error) { - popupAjaxError(error); - } finally { - this.loading = false; - this.dirty = true; - } + this.model.resetParams(); } @action async discard() { try { this.loading = true; - const result = await this.store.find("query", this.selectedItem.id); - this.selectedItem.setProperties( - result.getProperties(Query.updatePropertyNames) - ); - if ( - !this.selectedItem.group_ids || - !Array.isArray(this.selectedItem.group_ids) - ) { - this.selectedItem.set("group_ids", []); + const result = await this.store.find("query", this.model.id); + this.model.setProperties(result.getProperties(Query.updatePropertyNames)); + if (!this.model.group_ids || !Array.isArray(this.model.group_ids)) { + this.model.set("group_ids", []); } this.dirty = false; } catch (error) { @@ -331,8 +205,8 @@ export default class PluginsExplorerController extends Controller { try { this.loading = true; this.showResults = false; - await this.store.destroyRecord("query", this.selectedItem); - this.selectedItem.set("destroyed", true); + await this.store.destroyRecord("query", this.model); + this.model.set("destroyed", true); } catch (error) { popupAjaxError(error); } finally { @@ -345,8 +219,8 @@ export default class PluginsExplorerController extends Controller { try { this.loading = true; this.showResults = true; - await this.selectedItem.save(); - this.selectedItem.set("destroyed", false); + await this.model.save(); + this.model.set("destroyed", false); } catch (error) { popupAjaxError(error); } finally { @@ -359,16 +233,6 @@ export default class PluginsExplorerController extends Controller { this.form = form; } - @action - updateSearch(value) { - this.search = value; - } - - @action - updateNewQueryName(value) { - this.newQueryName = value; - } - @action setDirty() { this.dirty = true; @@ -382,7 +246,7 @@ export default class PluginsExplorerController extends Controller { @action async run() { let params = null; - if (this.selectedItem.hasParams) { + if (this.model.hasParams) { try { params = await this.form?.submit(); } catch (err) { @@ -400,7 +264,7 @@ export default class PluginsExplorerController extends Controller { params: JSON.stringify(params), }); - ajax("/admin/plugins/explorer/queries/" + this.selectedItem.id + "/run", { + ajax("/admin/plugins/explorer/queries/" + this.model.id + "/run", { type: "POST", data: { params: JSON.stringify(params), diff --git a/assets/javascripts/discourse/explorer-route-map.js b/assets/javascripts/discourse/explorer-route-map.js index 985d520..8b793f7 100644 --- a/assets/javascripts/discourse/explorer-route-map.js +++ b/assets/javascripts/discourse/explorer-route-map.js @@ -2,6 +2,10 @@ export default { resource: "admin.adminPlugins", path: "/plugins", map() { - this.route("explorer"); + this.route("explorer", function () { + this.route("queries", function () { + this.route("details", { path: "/:query_id" }); + }); + }); }, }; diff --git a/assets/javascripts/discourse/models/query.js b/assets/javascripts/discourse/models/query.js index 1561806..b57fd41 100644 --- a/assets/javascripts/discourse/models/query.js +++ b/assets/javascripts/discourse/models/query.js @@ -24,18 +24,18 @@ export default class Query extends RestModel { return getURL(`/admin/plugins/explorer/queries/${this.id}.json?export=1`); } - @computed("param_info", "updateing") + @computed("param_info", "updating") get hasParams() { // When saving, we need to refresh the param-input component to clean up the old key - return this.param_info.length && !this.updateing; + return this.param_info.length && !this.updating; } beforeUpdate() { - this.set("updateing", true); + this.set("updating", true); } afterUpdate() { - this.set("updateing", false); + this.set("updating", false); } resetParams() { diff --git a/assets/javascripts/discourse/routes/admin-plugins-explorer-index.js b/assets/javascripts/discourse/routes/admin-plugins-explorer-index.js new file mode 100644 index 0000000..d3936f5 --- /dev/null +++ b/assets/javascripts/discourse/routes/admin-plugins-explorer-index.js @@ -0,0 +1,47 @@ +import { service } from "@ember/service"; +import { ajax } from "discourse/lib/ajax"; +import DiscourseRoute from "discourse/routes/discourse"; + +export default class AdminPluginsExplorerIndex extends DiscourseRoute { + @service router; + + beforeModel(transition) { + // Redirect old /explorer?id=123 route to /explorer/queries/123 + if (transition.to.queryParams.id) { + this.router.transitionTo( + "adminPlugins.explorer.queries.details", + transition.to.queryParams.id + ); + } + } + + model() { + if (!this.currentUser.admin) { + // display "Only available to admins" message + return { model: null, schema: null, disallow: true, groups: null }; + } + + const groupPromise = ajax("/admin/plugins/explorer/groups.json"); + const queryPromise = this.store.findAll("query"); + + return groupPromise.then((groups) => { + let groupNames = {}; + groups.forEach((g) => { + groupNames[g.id] = g.name; + }); + return queryPromise.then((model) => { + model.forEach((query) => { + query.set( + "group_names", + (query.group_ids || []).map((id) => groupNames[id]) + ); + }); + return { model, groups }; + }); + }); + } + + setupController(controller, model) { + controller.setProperties(model); + } +} diff --git a/assets/javascripts/discourse/routes/admin-plugins-explorer.js b/assets/javascripts/discourse/routes/admin-plugins-explorer-queries-details.js similarity index 72% rename from assets/javascripts/discourse/routes/admin-plugins-explorer.js rename to assets/javascripts/discourse/routes/admin-plugins-explorer-queries-details.js index 85a9be1..8953618 100644 --- a/assets/javascripts/discourse/routes/admin-plugins-explorer.js +++ b/assets/javascripts/discourse/routes/admin-plugins-explorer-queries-details.js @@ -1,8 +1,8 @@ import { ajax } from "discourse/lib/ajax"; import DiscourseRoute from "discourse/routes/discourse"; -export default class AdminPluginsExplorer extends DiscourseRoute { - model() { +export default class AdminPluginsExplorerQueriesDetails extends DiscourseRoute { + model(params) { if (!this.currentUser.admin) { // display "Only available to admins" message return { model: null, schema: null, disallow: true, groups: null }; @@ -12,7 +12,7 @@ export default class AdminPluginsExplorer extends DiscourseRoute { const schemaPromise = ajax("/admin/plugins/explorer/schema.json", { cache: true, }); - const queryPromise = this.store.findAll("query"); + const queryPromise = this.store.find("query", params.query_id); return groupPromise.then((groups) => { let groupNames = {}; @@ -21,12 +21,10 @@ export default class AdminPluginsExplorer extends DiscourseRoute { }); return schemaPromise.then((schema) => { return queryPromise.then((model) => { - model.forEach((query) => { - query.set( - "group_names", - (query.group_ids || []).map((id) => groupNames[id]) - ); - }); + model.set( + "group_names", + (model.group_ids || []).map((id) => groupNames[id]) + ); return { model, schema, groups }; }); }); diff --git a/assets/javascripts/discourse/templates/admin/plugins-explorer-index.hbs b/assets/javascripts/discourse/templates/admin/plugins-explorer-index.hbs new file mode 100644 index 0000000..80166b0 --- /dev/null +++ b/assets/javascripts/discourse/templates/admin/plugins-explorer-index.hbs @@ -0,0 +1,166 @@ +{{#if this.disallow}} +

{{i18n "explorer.admins_only"}}

+{{else}} +
+ + + +
+ + {{#if this.showCreate}} +
+ + +
+ {{/if}} + + {{#if this.othersDirty}} +
+ {{d-icon "triangle-exclamation"}} + {{i18n "explorer.others_dirty"}} +
+ {{/if}} + + {{#if this.model.length}} + + +
+ + + + + + + + + {{#each this.filteredContent as |query|}} + + + + + + + {{else}} +
+ + {{i18n "explorer.no_search_results"}} + + {{/each}} + +
+
+ +
+ 		+
+ +
+ 		+
+ {{i18n "explorer.query_groups"}} +
+ 		+
+ +
+
+ + {{query.name}} + {{query.description}} + + +
+ {{i18n "explorer.query_user"}} +
+ {{#if query.username}} + + {{/if}} + 		+
+ {{i18n "explorer.query_groups"}} +
+
+ {{#each query.group_names as |group|}} + + {{/each}} +
+ 		+
+ {{i18n "explorer.query_time"}} +
+ {{#if query.last_run_at}} + + {{bound-date query.last_run_at}} + + {{else if query.created_at}} + + {{bound-date query.created_at}} + + {{/if}} +
+
+ +
+ {{/if}} +{{/if}} \ No newline at end of file diff --git a/assets/javascripts/discourse/templates/admin/plugins-explorer-queries-details.hbs b/assets/javascripts/discourse/templates/admin/plugins-explorer-queries-details.hbs new file mode 100644 index 0000000..c64d1e6 --- /dev/null +++ b/assets/javascripts/discourse/templates/admin/plugins-explorer-queries-details.hbs @@ -0,0 +1,223 @@ +{{#if this.disallow}} +

{{i18n "explorer.admins_only"}}

+{{else}} + +
+ {{#if this.editingName}} +
+ + +
+ +
+
+ +
+ +
+ {{else}} +
+ + +

+ {{this.model.name}} + {{#unless this.editDisabled}} + + {{d-icon "pencil"}} + + {{/unless}} +

+
+ +
+ {{this.model.description}} +
+ {{/if}} + + {{#unless this.model.destroyed}} +
+ {{i18n "explorer.allow_groups"}} + + + +
+ {{/unless}} + +
+ + {{#if this.editingQuery}} +
+
+
+ +
+ +
+ +
+
+ +
+ {{d-icon "discourse-expand"}} +
+ +
+
+ {{else}} +
+ +
+ {{/if}} + +
+ +
+ {{#if this.editingQuery}} + + {{else}} + {{#unless this.editDisabled}} + + {{/unless}} + {{/if}} + + + + {{#if this.editingQuery}} + + {{/if}} +
+ +
+ {{#if this.model.destroyed}} + + {{else}} + {{#if this.editingQuery}} + + {{/if}} + + + {{/if}} +
+
+
+ +
+ {{#if this.model.hasParams}} + + {{/if}} + + {{#if this.runDisabled}} + {{#if this.saveDisabled}} + + {{else}} + + {{/if}} + {{else}} + + {{/if}} + + + +
+ + + + +{{/if}} \ No newline at end of file diff --git a/assets/javascripts/discourse/templates/admin/plugins-explorer.hbs b/assets/javascripts/discourse/templates/admin/plugins-explorer.hbs deleted file mode 100644 index 901fa84..0000000 --- a/assets/javascripts/discourse/templates/admin/plugins-explorer.hbs +++ /dev/null @@ -1,405 +0,0 @@ -{{#if this.disallow}} -

{{i18n "explorer.admins_only"}}

-{{else}} - {{#unless this.validQueryPresent}} -
- - - -
- - {{#if this.showCreate}} -
- - -
- {{/if}} - - {{#if this.othersDirty}} -
- {{d-icon "triangle-exclamation"}} - {{i18n "explorer.others_dirty"}} -
- {{/if}} - {{/unless}} - - {{#if this.model.length}} - {{#unless this.selectedItem.fake}} -
- {{#if this.selectedItem}} - {{#if this.editingName}} -
- - -
- -
-
- -
- -
- {{else}} -
- - -

- {{this.selectedItem.name}} - {{#unless this.editDisabled}} - - {{d-icon "pencil"}} - - {{/unless}} -

-
- -
- {{this.selectedItem.description}} -
- {{/if}} - - {{#unless this.selectedItem.destroyed}} -
- {{i18n "explorer.allow_groups"}} - - - -
- {{/unless}} - -
- - {{#if this.editingQuery}} -
-
-
- -
- -
- -
-
- -
- {{d-icon "discourse-expand"}} -
- -
-
- {{else}} -
- -
- {{/if}} - -
- -
- {{#if this.editingQuery}} - - {{else}} - {{#unless this.editDisabled}} - - {{/unless}} - {{/if}} - - - - {{#if this.editingQuery}} - - {{/if}} -
- -
- {{#if this.selectedItem.destroyed}} - - {{else}} - {{#if this.editingQuery}} - - {{/if}} - - - {{/if}} -
- -
- {{/if}} -
- -
- {{#if this.selectedItem.hasParams}} - - {{/if}} - - {{#if this.runDisabled}} - {{#if this.saveDisabled}} - - {{else}} - - {{/if}} - {{else}} - - {{/if}} - - - -
- {{/unless}} - - - - {{#unless this.selectedItem.fake}} - - {{/unless}} - - {{#unless this.validQueryPresent}} -
- - - - - - - - - {{#each this.filteredContent as |query|}} - - - - - - - {{else}} -
- - {{i18n "explorer.no_search_results"}} - - {{/each}} - -
-
- -
- 		-
- -
- 		-
- {{i18n "explorer.query_groups"}} -
- 		-
- -
-
- - {{query.name}} - {{query.description}} - - -
- {{i18n "explorer.query_user"}} -
- {{#if query.username}} - - {{/if}} - 		-
- {{i18n "explorer.query_groups"}} -
-
- {{#each query.group_names as |group|}} - - {{/each}} -
- 		-
- {{i18n "explorer.query_time"}} -
- {{#if query.last_run_at}} - - {{bound-date query.last_run_at}} - - {{else if query.created_at}} - - {{bound-date query.created_at}} - - {{/if}} -
-
- {{/unless}} - -
- {{/if}} -{{/if}} \ No newline at end of file diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 083ef16..55cf55f 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -23,7 +23,7 @@ en: Here are the results: %{table} - View query in Data Explorer + View query in Data Explorer Report created at %{created_at} (%{timezone}) post: @@ -33,7 +33,7 @@ en: Here are the results: %{table} - View query in Data Explorer + View query in Data Explorer Report created at %{created_at} (%{timezone}) upload_appendix: "Appendix: [%{filename}|attachment](%{short_url})" diff --git a/config/routes.rb b/config/routes.rb index c34b206..d3434be 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -3,12 +3,12 @@ DiscourseDataExplorer::Engine.routes.draw do root to: "query#index" get "queries" => "query#index" + get "queries/:id" => "query#show" scope "/", defaults: { format: :json } do get "schema" => "query#schema" get "groups" => "query#groups" post "queries" => "query#create" - get "queries/:id" => "query#show" put "queries/:id" => "query#update" delete "queries/:id" => "query#destroy" post "queries/:id/run" => "query#run", :constraints => { format: /(json|csv)/ } diff --git a/lib/discourse_data_explorer/data_explorer.rb b/lib/discourse_data_explorer/data_explorer.rb index f9843a8..2dd1777 100644 --- a/lib/discourse_data_explorer/data_explorer.rb +++ b/lib/discourse_data_explorer/data_explorer.rb @@ -46,7 +46,7 @@ module ::DiscourseDataExplorer sql = <<-SQL /* * DiscourseDataExplorer Query - * Query: /admin/plugins/explorer?id=#{query.id} + * Query: /admin/plugins/explorer/queries/#{query.id} * Started by: #{opts[:current_user]} */ WITH query AS ( diff --git a/spec/system/explorer_spec.rb b/spec/system/explorer_spec.rb index d1954a8..bb99da2 100644 --- a/spec/system/explorer_spec.rb +++ b/spec/system/explorer_spec.rb @@ -29,6 +29,32 @@ RSpec.describe "Explorer", type: :system, js: true do end end + context "with the old url format" do + fab!(:query_1) do + Fabricate( + :query, + name: "My query", + description: "Test query", + sql: "SELECT * FROM users", + user: admin, + ) + end + + it "redirects to the new url format" do + visit("/admin/plugins/explorer/?id=#{query_1.id}") + + expect(page).to have_current_path("/admin/plugins/explorer/queries/#{query_1.id}") + end + + it "redirects to the new url format with params" do + visit("/admin/plugins/explorer/?id=#{query_1.id}¶ms=%7B%22limit%22%3A%2210%22%7D") + + expect(page).to have_current_path( + "/admin/plugins/explorer/queries/#{query_1.id}?params=%7B%22limit%22%3A%2210%22%7D", + ) + end + end + context "with a group_list param" do fab!(:q2) do Fabricate( @@ -43,7 +69,7 @@ RSpec.describe "Explorer", type: :system, js: true do it "supports setting a group_list param" do visit( - "/admin/plugins/explorer?id=#{q2.id}¶ms=%7B\"groups\"%3A\"admins%2Ctrust_level_1\"%7D", + "/admin/plugins/explorer/queries/#{q2.id}?params=%7B\"groups\"%3A\"admins%2Ctrust_level_1\"%7D", ) find(".query-run .btn-primary").click diff --git a/spec/system/param_input_spec.rb b/spec/system/param_input_spec.rb index 27c4b2f..fe6efdd 100644 --- a/spec/system/param_input_spec.rb +++ b/spec/system/param_input_spec.rb @@ -62,7 +62,7 @@ RSpec.describe "Param input", type: :system, js: true do end it "correctly displays parameter input boxes" do - visit("/admin/plugins/explorer?id=#{all_params_query.id}") + visit("/admin/plugins/explorer/queries/#{all_params_query.id}") ::DiscourseDataExplorer::Parameter .create_from_sql(ALL_PARAMS_SQL) diff --git a/test/javascripts/acceptance/list-queries-test.js b/test/javascripts/acceptance/list-queries-test.js index 0663a63..74d7505 100644 --- a/test/javascripts/acceptance/list-queries-test.js +++ b/test/javascripts/acceptance/list-queries-test.js @@ -88,44 +88,22 @@ acceptance("Data Explorer Plugin | List Queries", function (needs) { queries: [ { id: -5, - sql: "-- [params]\n-- int :months_ago = 1\n\nWITH query_period AS\n(SELECT date_trunc('month', CURRENT_DATE) - INTERVAL ':months_ago months' AS period_start,\n date_trunc('month', CURRENT_DATE) - INTERVAL ':months_ago months' + INTERVAL '1 month' - INTERVAL '1 second' AS period_end)\nSELECT t.id AS topic_id,\n t.category_id,\n COUNT(p.id) AS reply_count\nFROM topics t\nJOIN posts p ON t.id = p.topic_id\nJOIN query_period qp ON p.created_at >= qp.period_start\nAND p.created_at <= qp.period_end\nWHERE t.archetype = 'regular'\nAND t.user_id > 0\nGROUP BY t.id\nORDER BY COUNT(p.id) DESC, t.score DESC\nLIMIT 100\n", name: "Top 100 Active Topics", description: "based on the number of replies, it accepts a ‘months_ago’ parameter, defaults to 1 to give results for the last calendar month.", - param_info: [ - { - identifier: "months_ago", - type: "int", - default: "1", - nullable: false, - }, - ], - created_at: "2021-02-05T16:42:45.572Z", username: "system", group_ids: [], last_run_at: "2021-02-08T15:37:49.188Z", - hidden: false, user_id: -1, }, { id: -6, - sql: "-- [params]\n-- int :months_ago = 1\n\nWITH query_period AS (\n SELECT\n date_trunc('month', CURRENT_DATE) - INTERVAL ':months_ago months' as period_start,\n date_trunc('month', CURRENT_DATE) - INTERVAL ':months_ago months' + INTERVAL '1 month' - INTERVAL '1 second' as period_end\n )\n\n SELECT\n ua.user_id,\n count(1) AS like_count\n FROM user_actions ua\n INNER JOIN query_period qp\n ON ua.created_at >= qp.period_start\n AND ua.created_at <= qp.period_end\n WHERE ua.action_type = 1\n GROUP BY ua.user_id\n ORDER BY like_count DESC\n LIMIT 100\n", name: "Top 100 Likers", description: "returns the top 100 likers for a given monthly period ordered by like_count. It accepts a ‘months_ago’ parameter, defaults to 1 to give results for the last calendar month.", - param_info: [ - { - identifier: "months_ago", - type: "int", - default: "1", - nullable: false, - }, - ], - created_at: "2021-02-02T12:21:11.449Z", username: "system", group_ids: [], last_run_at: "2021-02-11T08:29:59.337Z", - hidden: false, user_id: -1, }, ], diff --git a/test/javascripts/acceptance/new-query-test.js b/test/javascripts/acceptance/new-query-test.js index b25ab23..344f8a5 100644 --- a/test/javascripts/acceptance/new-query-test.js +++ b/test/javascripts/acceptance/new-query-test.js @@ -46,6 +46,32 @@ acceptance("Data Explorer Plugin | New Query", function (needs) { }, }); }); + + server.get("/admin/plugins/explorer/queries/-15", () => { + return helper.response({ + query: { + id: -15, + sql: "-- [params]\n-- int :months_ago = 1\n\nWITH query_period AS\n(SELECT date_trunc('month', CURRENT_DATE) - INTERVAL ':months_ago months' AS period_start,\n date_trunc('month', CURRENT_DATE) - INTERVAL ':months_ago months' + INTERVAL '1 month' - INTERVAL '1 second' AS period_end)\nSELECT t.id AS topic_id,\n t.category_id,\n COUNT(p.id) AS reply_count\nFROM topics t\nJOIN posts p ON t.id = p.topic_id\nJOIN query_period qp ON p.created_at >= qp.period_start\nAND p.created_at <= qp.period_end\nWHERE t.archetype = 'regular'\nAND t.user_id > 0\nGROUP BY t.id\nORDER BY COUNT(p.id) DESC, t.score DESC\nLIMIT 100\n", + name: "foo", + description: + "based on the number of replies, it accepts a ‘months_ago’ parameter, defaults to 1 to give results for the last calendar month.", + param_info: [ + { + identifier: "months_ago", + type: "int", + default: "1", + nullable: false, + }, + ], + created_at: "2021-02-05T16:42:45.572Z", + username: "system", + group_ids: [], + last_run_at: "2021-02-08T15:37:49.188Z", + hidden: false, + user_id: -1, + }, + }); + }); }); test("creates a new query", async function (assert) { @@ -57,6 +83,6 @@ acceptance("Data Explorer Plugin | New Query", function (needs) { // select create new query button await click(".query-create button"); - assert.strictEqual(currentURL(), "/admin/plugins/explorer?id=-15"); + assert.strictEqual(currentURL(), "/admin/plugins/explorer/queries/-15"); }); }); diff --git a/test/javascripts/acceptance/param-input-test.js b/test/javascripts/acceptance/param-input-test.js index 6397e72..84174eb 100644 --- a/test/javascripts/acceptance/param-input-test.js +++ b/test/javascripts/acceptance/param-input-test.js @@ -91,62 +91,62 @@ acceptance("Data Explorer Plugin | Param Input", function (needs) { queries: [ { id: -6, - sql: "-- [params]\n-- int :months_ago = 1\n\nWITH query_period AS (\n SELECT\n date_trunc('month', CURRENT_DATE) - INTERVAL ':months_ago months' as period_start,\n date_trunc('month', CURRENT_DATE) - INTERVAL ':months_ago months' + INTERVAL '1 month' - INTERVAL '1 second' as period_end\n )\n\n SELECT\n ua.user_id,\n count(1) AS like_count\n FROM user_actions ua\n INNER JOIN query_period qp\n ON ua.created_at >= qp.period_start\n AND ua.created_at <= qp.period_end\n WHERE ua.action_type = 1\n GROUP BY ua.user_id\n ORDER BY like_count DESC\n LIMIT 100\n", name: "Top 100 Likers", description: "returns the top 100 likers for a given monthly period ordered by like_count. It accepts a ‘months_ago’ parameter, defaults to 1 to give results for the last calendar month.", - param_info: [ - { - identifier: "months_ago", - type: "int", - default: "1", - nullable: false, - }, - ], - created_at: "2021-02-02T12:21:11.449Z", username: "system", group_ids: [], last_run_at: "2021-02-11T08:29:59.337Z", - hidden: false, user_id: -1, }, { id: -7, - sql: "-- [params]\n-- user_id :user\n\nSELECT :user_id\n\n", name: "Invalid Query", description: "", - param_info: [ - { - identifier: "user", - type: "user_id", - default: null, - nullable: false, - }, - ], - created_at: "2022-01-14T16:40:05.458Z", username: "bianca", group_ids: [], last_run_at: "2022-01-14T16:47:34.244Z", - hidden: false, user_id: 1, }, { id: 3, - sql: "SELECT 1", name: "Params test", description: "test for params.", - param_info: [], - created_at: "2021-02-02T12:21:11.449Z", username: "system", group_ids: [41], last_run_at: "2021-02-11T08:29:59.337Z", - hidden: false, user_id: -1, }, ], }); }); + server.get("/admin/plugins/explorer/queries/-6", () => { + return helper.response({ + query: { + id: -6, + sql: "-- [params]\n-- int :months_ago = 1\n\nWITH query_period AS (\n SELECT\n date_trunc('month', CURRENT_DATE) - INTERVAL ':months_ago months' as period_start,\n date_trunc('month', CURRENT_DATE) - INTERVAL ':months_ago months' + INTERVAL '1 month' - INTERVAL '1 second' as period_end\n )\n\n SELECT\n ua.user_id,\n count(1) AS like_count\n FROM user_actions ua\n INNER JOIN query_period qp\n ON ua.created_at >= qp.period_start\n AND ua.created_at <= qp.period_end\n WHERE ua.action_type = 1\n GROUP BY ua.user_id\n ORDER BY like_count DESC\n LIMIT 100\n", + name: "Top 100 Likers", + description: + "returns the top 100 likers for a given monthly period ordered by like_count. It accepts a ‘months_ago’ parameter, defaults to 1 to give results for the last calendar month.", + param_info: [ + { + identifier: "months_ago", + type: "int", + default: "1", + nullable: false, + }, + ], + created_at: "2021-02-02T12:21:11.449Z", + username: "system", + group_ids: [], + last_run_at: "2021-02-11T08:29:59.337Z", + hidden: false, + user_id: -1, + }, + }); + }); + server.put("/admin/plugins/explorer/queries/-6", () => { return helper.response({ success: true, @@ -244,6 +244,31 @@ acceptance("Data Explorer Plugin | Param Input", function (needs) { }); }); + server.get("/admin/plugins/explorer/queries/-7", () => { + return helper.response({ + query: { + id: -7, + sql: "-- [params]\n-- user_id :user\n\nSELECT :user_id\n\n", + name: "Invalid Query", + description: "", + param_info: [ + { + identifier: "user", + type: "user_id", + default: null, + nullable: false, + }, + ], + created_at: "2022-01-14T16:40:05.458Z", + username: "bianca", + group_ids: [], + last_run_at: "2022-01-14T16:47:34.244Z", + hidden: false, + user_id: 1, + }, + }); + }); + server.post("/admin/plugins/explorer/queries/-7/run", () => { return helper.response({ success: true, @@ -351,7 +376,7 @@ acceptance("Data Explorer Plugin | Param Input", function (needs) { }); test("puts params for the query into the url", async function (assert) { - await visit("/admin/plugins/explorer?id=-6"); + await visit("/admin/plugins/explorer/queries/-6"); const monthsAgoValue = "2"; await fillIn(".query-params input", monthsAgoValue); await click("form.query-run button"); @@ -373,7 +398,7 @@ acceptance("Data Explorer Plugin | Param Input", function (needs) { }); test("loads the page if one of the parameter is null", async function (assert) { - await visit('/admin/plugins/explorer?id=-7¶ms={"user":null}'); + await visit('/admin/plugins/explorer/queries/-7?params={"user":null}'); assert.dom(".query-params .user-chooser").exists(); assert.dom(".query-run .btn.btn-primary").exists(); }); @@ -393,7 +418,7 @@ acceptance("Data Explorer Plugin | Param Input", function (needs) { }); test("creates input boxes if has parameters when save", async function (assert) { - await visit("/admin/plugins/explorer?id=3"); + await visit("/admin/plugins/explorer/queries/3"); assert.dom(".query-params input").doesNotExist(); await click(".query-edit .btn-edit-query"); await click(".query-editor .ace_text-input"); diff --git a/test/javascripts/acceptance/run-query-test.js b/test/javascripts/acceptance/run-query-test.js index a6ffc91..3f8d178 100644 --- a/test/javascripts/acceptance/run-query-test.js +++ b/test/javascripts/acceptance/run-query-test.js @@ -88,42 +88,71 @@ acceptance("Data Explorer Plugin | Run Query", function (needs) { queries: [ { id: -6, - sql: "-- [params]\n-- int :months_ago = 1\n\nWITH query_period AS (\n SELECT\n date_trunc('month', CURRENT_DATE) - INTERVAL ':months_ago months' as period_start,\n date_trunc('month', CURRENT_DATE) - INTERVAL ':months_ago months' + INTERVAL '1 month' - INTERVAL '1 second' as period_end\n )\n\n SELECT\n ua.user_id,\n count(1) AS like_count\n FROM user_actions ua\n INNER JOIN query_period qp\n ON ua.created_at >= qp.period_start\n AND ua.created_at <= qp.period_end\n WHERE ua.action_type = 1\n GROUP BY ua.user_id\n ORDER BY like_count DESC\n LIMIT 100\n", name: "Top 100 Likers", description: "returns the top 100 likers for a given monthly period ordered by like_count. It accepts a ‘months_ago’ parameter, defaults to 1 to give results for the last calendar month.", - param_info: [ - { - identifier: "months_ago", - type: "int", - default: "1", - nullable: false, - }, - ], - created_at: "2021-02-02T12:21:11.449Z", username: "system", group_ids: [], last_run_at: "2021-02-11T08:29:59.337Z", - hidden: false, user_id: -1, }, { id: 2, - sql: 'SELECT 0 zero, null "null", false "false"', name: "What about 0?", description: "", - param_info: [], - created_at: "2023-05-04T22:16:06.007Z", username: "system", group_ids: [], last_run_at: "2023-05-04T22:16:23.858Z", - hidden: false, user_id: 1, }, ], }); }); + server.get("/admin/plugins/explorer/queries/-6", () => { + return helper.response({ + query: { + id: -6, + sql: "-- [params]\n-- int :months_ago = 1\n\nWITH query_period AS (\n SELECT\n date_trunc('month', CURRENT_DATE) - INTERVAL ':months_ago months' as period_start,\n date_trunc('month', CURRENT_DATE) - INTERVAL ':months_ago months' + INTERVAL '1 month' - INTERVAL '1 second' as period_end\n )\n\n SELECT\n ua.user_id,\n count(1) AS like_count\n FROM user_actions ua\n INNER JOIN query_period qp\n ON ua.created_at >= qp.period_start\n AND ua.created_at <= qp.period_end\n WHERE ua.action_type = 1\n GROUP BY ua.user_id\n ORDER BY like_count DESC\n LIMIT 100\n", + name: "Top 100 Likers", + description: + "returns the top 100 likers for a given monthly period ordered by like_count. It accepts a ‘months_ago’ parameter, defaults to 1 to give results for the last calendar month.", + param_info: [ + { + identifier: "months_ago", + type: "int", + default: "1", + nullable: false, + }, + ], + created_at: "2021-02-02T12:21:11.449Z", + username: "system", + group_ids: [], + last_run_at: "2021-02-11T08:29:59.337Z", + hidden: false, + user_id: -1, + }, + }); + }); + + server.get("/admin/plugins/explorer/queries/2", () => { + return helper.response({ + query: { + id: 2, + sql: 'SELECT 0 zero, null "null", false "false"', + name: "What about 0?", + description: "", + param_info: [], + created_at: "2023-05-04T22:16:06.007Z", + username: "system", + group_ids: [], + last_run_at: "2023-05-04T22:16:23.858Z", + hidden: false, + user_id: 1, + }, + }); + }); + server.post("/admin/plugins/explorer/queries/-6/run", () => { return helper.response({ success: true, @@ -177,7 +206,7 @@ acceptance("Data Explorer Plugin | Run Query", function (needs) { }); test("runs query and renders data and a chart", async function (assert) { - await visit("/admin/plugins/explorer?id=-6"); + await visit("/admin/plugins/explorer/queries/-6"); assert .dom("div.name h1") @@ -205,7 +234,7 @@ acceptance("Data Explorer Plugin | Run Query", function (needs) { }); test("runs query and renders 0, false, and NULL values correctly", async function (assert) { - await visit("/admin/plugins/explorer?id=2"); + await visit("/admin/plugins/explorer/queries/2"); assert .dom("div.name h1")