Skip to content

Commit d726c48

Browse files
authored
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.
1 parent bd6263e commit d726c48

21 files changed

+837
-667
lines changed

app/controllers/discourse_data_explorer/query_controller.rb

+4-4
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ def show
4040
end
4141

4242
return raise Discourse::NotFound if !guardian.user_can_access_query?(@query) || @query.hidden
43-
render_serialized @query, QuerySerializer, root: "query"
43+
render_serialized @query, QueryDetailsSerializer, root: "query"
4444
end
4545

4646
def groups
@@ -68,7 +68,7 @@ def group_reports_show
6868
query_group = QueryGroup.find_by(query_id: @query.id, group_id: @group.id)
6969

7070
render json: {
71-
query: serialize_data(@query, QuerySerializer, root: nil),
71+
query: serialize_data(@query, QueryDetailsSerializer, root: nil),
7272
query_group: serialize_data(query_group, QueryGroupSerializer, root: nil),
7373
}
7474
end
@@ -93,7 +93,7 @@ def create
9393
)
9494
group_ids = params.require(:query)[:group_ids]
9595
group_ids&.each { |group_id| query.query_groups.find_or_create_by!(group_id: group_id) }
96-
render_serialized query, QuerySerializer, root: "query"
96+
render_serialized query, QueryDetailsSerializer, root: "query"
9797
end
9898

9999
def update
@@ -107,7 +107,7 @@ def update
107107
group_ids&.each { |group_id| @query.query_groups.find_or_create_by!(group_id: group_id) }
108108
end
109109

110-
render_serialized @query, QuerySerializer, root: "query"
110+
render_serialized @query, QueryDetailsSerializer, root: "query"
111111
rescue ValidationError => e
112112
render_json_error e.message
113113
end
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
# frozen_string_literal: true
2+
3+
module ::DiscourseDataExplorer
4+
class QueryDetailsSerializer < QuerySerializer
5+
attributes :sql, :param_info, :created_at, :hidden
6+
7+
def param_info
8+
object&.params&.uniq { |p| p.identifier }&.map(&:to_hash)
9+
end
10+
end
11+
end

app/serializers/discourse_data_explorer/query_serializer.rb

+1-15
Original file line numberDiff line numberDiff line change
@@ -2,21 +2,7 @@
22

33
module ::DiscourseDataExplorer
44
class QuerySerializer < ActiveModel::Serializer
5-
attributes :id,
6-
:sql,
7-
:name,
8-
:description,
9-
:param_info,
10-
:created_at,
11-
:username,
12-
:group_ids,
13-
:last_run_at,
14-
:hidden,
15-
:user_id
16-
17-
def param_info
18-
object&.params&.uniq { |p| p.identifier }&.map(&:to_hash)
19-
end
5+
attributes :id, :name, :description, :username, :group_ids, :last_run_at, :user_id
206

217
def username
228
object&.user&.username
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,192 @@
1+
import { tracked } from "@glimmer/tracking";
2+
import Controller from "@ember/controller";
3+
import { action } from "@ember/object";
4+
import { service } from "@ember/service";
5+
import { Promise } from "rsvp";
6+
import { popupAjaxError } from "discourse/lib/ajax-error";
7+
import { bind } from "discourse/lib/decorators";
8+
import { i18n } from "discourse-i18n";
9+
10+
export default class PluginsExplorerController extends Controller {
11+
@service dialog;
12+
@service appEvents;
13+
@service router;
14+
15+
@tracked sortByProperty = "last_run_at";
16+
@tracked sortDescending = true;
17+
@tracked params;
18+
@tracked search;
19+
@tracked newQueryName;
20+
@tracked showCreate;
21+
@tracked loading = false;
22+
23+
queryParams = ["id"];
24+
explain = false;
25+
acceptedImportFileTypes = ["application/json"];
26+
order = null;
27+
form = null;
28+
29+
get sortedQueries() {
30+
const sortedQueries = this.model.sortBy(this.sortByProperty);
31+
return this.sortDescending ? sortedQueries.reverse() : sortedQueries;
32+
}
33+
34+
get parsedParams() {
35+
return this.params ? JSON.parse(this.params) : null;
36+
}
37+
38+
get filteredContent() {
39+
const regexp = new RegExp(this.search, "i");
40+
return this.sortedQueries.filter(
41+
(result) => regexp.test(result.name) || regexp.test(result.description)
42+
);
43+
}
44+
45+
get createDisabled() {
46+
return (this.newQueryName || "").trim().length === 0;
47+
}
48+
49+
addCreatedRecord(record) {
50+
this.model.pushObject(record);
51+
this.router.transitionTo(
52+
"adminPlugins.explorer.queries.details",
53+
record.id
54+
);
55+
}
56+
57+
async _importQuery(file) {
58+
const json = await this._readFileAsTextAsync(file);
59+
const query = this._parseQuery(json);
60+
const record = this.store.createRecord("query", query);
61+
const response = await record.save();
62+
return response.target;
63+
}
64+
65+
_parseQuery(json) {
66+
const parsed = JSON.parse(json);
67+
const query = parsed.query;
68+
if (!query || !query.sql) {
69+
throw new TypeError();
70+
}
71+
query.id = 0; // 0 means no Id yet
72+
return query;
73+
}
74+
75+
_readFileAsTextAsync(file) {
76+
return new Promise((resolve, reject) => {
77+
const reader = new FileReader();
78+
reader.onload = () => {
79+
resolve(reader.result);
80+
};
81+
reader.onerror = reject;
82+
83+
reader.readAsText(file);
84+
});
85+
}
86+
87+
@bind
88+
dragMove(e) {
89+
if (!e.movementY && !e.movementX) {
90+
return;
91+
}
92+
93+
const editPane = document.querySelector(".query-editor");
94+
const target = editPane.querySelector(".panels-flex");
95+
const grippie = editPane.querySelector(".grippie");
96+
97+
// we need to get the initial height / width of edit pane
98+
// before we manipulate the size
99+
if (!this.initialPaneWidth && !this.originalPaneHeight) {
100+
this.originalPaneWidth = target.clientWidth;
101+
this.originalPaneHeight = target.clientHeight;
102+
}
103+
104+
const newHeight = Math.max(
105+
this.originalPaneHeight,
106+
target.clientHeight + e.movementY
107+
);
108+
const newWidth = Math.max(
109+
this.originalPaneWidth,
110+
target.clientWidth + e.movementX
111+
);
112+
113+
target.style.height = newHeight + "px";
114+
target.style.width = newWidth + "px";
115+
grippie.style.width = newWidth + "px";
116+
this.appEvents.trigger("ace:resize");
117+
}
118+
119+
@bind
120+
scrollTop() {
121+
window.scrollTo(0, 0);
122+
}
123+
124+
@action
125+
async import(files) {
126+
try {
127+
this.loading = true;
128+
const file = files[0];
129+
const record = await this._importQuery(file);
130+
this.addCreatedRecord(record);
131+
} catch (e) {
132+
if (e.jqXHR) {
133+
popupAjaxError(e);
134+
} else if (e instanceof SyntaxError) {
135+
this.dialog.alert(i18n("explorer.import.unparseable_json"));
136+
} else if (e instanceof TypeError) {
137+
this.dialog.alert(i18n("explorer.import.wrong_json"));
138+
} else {
139+
this.dialog.alert(i18n("errors.desc.unknown"));
140+
// eslint-disable-next-line no-console
141+
console.error(e);
142+
}
143+
} finally {
144+
this.loading = false;
145+
}
146+
}
147+
148+
@action
149+
displayCreate() {
150+
this.showCreate = true;
151+
}
152+
153+
@action
154+
resetParams() {
155+
this.selectedItem.resetParams();
156+
}
157+
158+
@action
159+
updateSortProperty(property) {
160+
if (this.sortByProperty === property) {
161+
this.sortDescending = !this.sortDescending;
162+
} else {
163+
this.sortByProperty = property;
164+
this.sortDescending = true;
165+
}
166+
}
167+
168+
@action
169+
async create() {
170+
try {
171+
const name = this.newQueryName.trim();
172+
this.loading = true;
173+
this.showCreate = false;
174+
const result = await this.store.createRecord("query", { name }).save();
175+
this.addCreatedRecord(result.target);
176+
} catch (error) {
177+
popupAjaxError(error);
178+
} finally {
179+
this.loading = false;
180+
}
181+
}
182+
183+
@action
184+
updateSearch(value) {
185+
this.search = value;
186+
}
187+
188+
@action
189+
updateNewQueryName(value) {
190+
this.newQueryName = value;
191+
}
192+
}

0 commit comments

Comments
 (0)