diff --git a/app/serializers/discourse_data_explorer/query_serializer.rb b/app/serializers/discourse_data_explorer/query_serializer.rb
index 01fb471c..2a87ad4c 100644
--- a/app/serializers/discourse_data_explorer/query_serializer.rb
+++ b/app/serializers/discourse_data_explorer/query_serializer.rb
@@ -15,7 +15,7 @@ class QuerySerializer < ActiveModel::Serializer
:user_id
def param_info
- object&.params&.map(&:to_hash)
+ object&.params&.uniq { |p| p.identifier }&.map(&:to_hash)
end
def username
diff --git a/assets/javascripts/discourse/components/param-input.hbs b/assets/javascripts/discourse/components/param-input.hbs
index 5e9448fe..7df1f6c9 100644
--- a/assets/javascripts/discourse/components/param-input.hbs
+++ b/assets/javascripts/discourse/components/param-input.hbs
@@ -1,83 +1,63 @@
- {{#if (eq this.type "boolean")}}
- {{#if @info.nullable}}
-
+
+ {{#if (eq this.type "boolean")}}
+ {{#if @info.nullable}}
+
+ {{#each this.boolTypes as |t|}}
+ {{t.name}}
+ {{/each}}
+
+ {{else}}
+
+ {{/if}}
+ {{else if (eq this.type "int")}}
+
+ {{else if (eq this.type "string")}}
+
+ {{else if (eq this.type "user_id")}}
+
+
+
+ {{else if (eq this.type "group_list")}}
+
+
+
+ {{else if (eq this.type "user_list")}}
+
+
+
+ {{else if (eq this.type "category_id")}}
+
+
+
{{else}}
-
+
{{/if}}
- {{@info.identifier}}
-
- {{else if (eq this.type "int")}}
-
- {{@info.identifier}}
-
- {{else if (eq this.type "string")}}
-
- {{@info.identifier}}
-
- {{else if (eq this.type "user_id")}}
-
- {{@info.identifier}}
-
- {{else if (eq this.type "group_list")}}
-
- {{@info.identifier}}
-
- {{else if (eq this.type "user_list")}}
-
- {{@info.identifier}}
-
- {{else if (eq this.type "category_id")}}
-
- {{@info.identifier}}
-
- {{else}}
-
- {{@info.identifier}}
- {{/if}}
+
\ No newline at end of file
diff --git a/assets/javascripts/discourse/components/param-input.js b/assets/javascripts/discourse/components/param-input.js
index a00f0b79..6ee6dae1 100644
--- a/assets/javascripts/discourse/components/param-input.js
+++ b/assets/javascripts/discourse/components/param-input.js
@@ -9,9 +9,9 @@ import I18n from "I18n";
const layoutMap = {
int: "int",
- bigint: "int",
+ bigint: "string",
boolean: "boolean",
- string: "generic",
+ string: "string",
time: "generic",
date: "generic",
datetime: "generic",
@@ -28,12 +28,24 @@ const layoutMap = {
group_list: "group_list",
};
+const ERRORS = {
+ REQUIRED: I18n.t("form_kit.errors.required"),
+ NOT_AN_INTEGER: I18n.t("form_kit.errors.not_an_integer"),
+ NOT_A_NUMBER: I18n.t("form_kit.errors.not_a_number"),
+ OVERFLOW_HIGH: I18n.t("form_kit.errors.too_high", { count: 2147484647 }),
+ OVERFLOW_LOW: I18n.t("form_kit.errors.too_low", { count: -2147484648 }),
+ INVALID: I18n.t("explorer.form.errors.invalid"),
+ NO_SUCH_CATEGORY: I18n.t("explorer.form.errors.no_such_category"),
+ NO_SUCH_GROUP: I18n.t("explorer.form.errors.no_such_group"),
+};
+
export default class ParamInput extends Component {
@service site;
+ /**
+ * @type {string | string[] | boolean | null}
+ */
@tracked value;
- @tracked boolValue;
- @tracked nullableBoolValue;
boolTypes = [
{ name: I18n.t("explorer.types.bool.true"), id: "Y" },
@@ -47,42 +59,72 @@ export default class ParamInput extends Component {
const identifier = this.args.info.identifier;
const initialValues = this.args.initialValues;
- // access parsed params if present to update values to previously ran values
- if (initialValues && identifier in initialValues) {
- const initialValue = initialValues[identifier];
- if (this.type === "boolean") {
- if (this.args.info.nullable) {
- this.nullableBoolValue = initialValue;
- this.args.updateParams(
- this.args.info.identifier,
- this.nullableBoolValue
- );
- } else {
- this.boolValue = initialValue !== "false";
- this.args.updateParams(this.args.info.identifier, this.boolValue);
- }
- } else {
+ // defet it to next tick to prevent infinite looping.
+ setTimeout(() => {
+ // access parsed params if present to update values to previously ran values
+ if (initialValues && identifier in initialValues) {
+ const initialValue = initialValues[identifier];
this.value = this.normalizeValue(initialValue);
- this.args.updateParams(this.args.info.identifier, this.value);
+ this.args.updateParams(this.args.info.identifier, this.provideValue);
+ this.form.set(identifier, this.value);
+ } else {
+ // if no parsed params then get and set default values
+ const defaultValue = this.args.info.default;
+ this.value = this.normalizeValue(defaultValue);
+ if (this.value != null) {
+ this.args.updateParams(this.args.info.identifier, this.provideValue);
+ this.form.set(identifier, this.value);
+ }
}
- } else {
- // if no parsed params then get and set default values
- const defaultValue = this.args.info.default;
- this.value = this.normalizeValue(defaultValue);
- this.boolValue = defaultValue !== "false";
- this.nullableBoolValue = defaultValue;
- }
+ }, 0);
}
+ /** The value we will store in this.value */
normalizeValue(value) {
switch (this.args.info.type) {
case "category_id":
return this.digitalizeCategoryId(value);
+ case "boolean":
+ if (value == null) {
+ return this.args.info.nullable ? "#null" : false;
+ }
+ return value;
+ case "group_list":
+ case "user_list":
+ if (Array.isArray(value)) {
+ return value || null;
+ }
+ return value?.split(",") || null;
+ case "user_id":
+ if (Array.isArray(value)) {
+ return value[0];
+ }
+ return value;
default:
return value;
}
}
+ /** The value we submitted when updatingParams */
+ get provideValue() {
+ switch (this.type) {
+ case "string":
+ case "int":
+ return this.value != null ? String(this.value) : "";
+ case "boolean":
+ return String(this.value);
+ case "group_list":
+ case "user_list":
+ return this.value.join(",");
+ default:
+ return this.value;
+ }
+ }
+
+ get form() {
+ return this.args.form;
+ }
+
get type() {
const type = this.args.info.type;
if ((type === "time" || type === "date") && !allowsInputTypeTime()) {
@@ -91,71 +133,96 @@ export default class ParamInput extends Component {
return layoutMap[type] || "generic";
}
- get valid() {
+ /** @returns {null | string} */
+ getError() {
const nullable = this.args.info.nullable;
+ if (isEmpty(this.value)) {
+ return nullable ? null : ERRORS.REQUIRED;
+ }
+
// intentionally use 'this.args' here instead of 'this.type'
// to get the original key instead of the translated value from the layoutMap
const type = this.args.info.type;
- let value;
-
- if (type === "boolean") {
- value = nullable ? this.nullableBoolValue : this.boolValue;
- } else {
- value = this.value;
- }
-
- if (isEmpty(value)) {
- return nullable;
- }
+ const value = String(this.value);
const intVal = parseInt(value, 10);
- const intValid =
- !isNaN(intVal) && intVal < 2147483648 && intVal > -2147483649;
const isPositiveInt = /^\d+$/.test(value);
switch (type) {
case "int":
- return /^-?\d+$/.test(value) && intValid;
+ if (intVal >= 2147483648) {
+ return ERRORS.OVERFLOW_HIGH;
+ }
+ if (intVal <= -2147483649) {
+ return ERRORS.OVERFLOW_LOW;
+ }
+ return null;
case "bigint":
- return /^-?\d+$/.test(value) && !isNaN(intVal);
+ if (isNaN(intVal)) {
+ return ERRORS.NOT_A_NUMBER;
+ }
+ return /^-?\d+$/.test(value) ? null : ERRORS.NOT_AN_INTEGER;
case "boolean":
- return /^Y|N|#null|true|false/.test(value);
+ return /^Y|N|#null|true|false/.test(value) ? null : ERRORS.INVALID;
case "double":
- return (
- !isNaN(parseFloat(value)) ||
- /^(-?)Inf(inity)?$/i.test(value) ||
- /^(-?)NaN$/i.test(value)
- );
+ if (isNaN(parseFloat(value))) {
+ if (/^(-?)Inf(inity)?$/i.test(value) || /^(-?)NaN$/i.test(value)) {
+ return null;
+ }
+ return ERRORS.NOT_A_NUMBER;
+ }
+ return null;
case "int_list":
- return value.split(",").every((i) => /^(-?\d+|null)$/.test(i.trim()));
+ return value.split(",").every((i) => /^(-?\d+|null)$/.test(i.trim()))
+ ? null
+ : ERRORS.INVALID;
case "post_id":
- return (
- isPositiveInt ||
+ return isPositiveInt ||
/\d+\/\d+(\?u=.*)?$/.test(value) ||
/\/t\/[^/]+\/(\d+)(\?u=.*)?/.test(value)
- );
+ ? null
+ : ERRORS.INVALID;
case "topic_id":
- return isPositiveInt || /\/t\/[^/]+\/(\d+)/.test(value);
+ return isPositiveInt || /\/t\/[^/]+\/(\d+)/.test(value)
+ ? null
+ : ERRORS.INVALID;
case "category_id":
if (isPositiveInt) {
- return !!this.site.categories.find((c) => c.id === intVal);
+ return this.site.categories.find((c) => c.id === intVal)
+ ? null
+ : ERRORS.NO_SUCH_CATEGORY;
} else {
- return false;
+ return ERRORS.REQUIRED;
}
case "group_id":
const groups = this.site.get("groups");
if (isPositiveInt) {
- return !!groups.find((g) => g.id === intVal);
+ return groups.find((g) => g.id === intVal)
+ ? null
+ : ERRORS.NO_SUCH_GROUP;
} else {
- return !!groups.find((g) => g.name === value);
+ return groups.find((g) => g.name === value)
+ ? null
+ : ERRORS.NO_SUCH_GROUP;
}
}
- return true;
+ return null;
+ }
+
+ get valid() {
+ return this.getError() == null;
}
get allGroups() {
return this.site.get("groups");
}
+ get validation() {
+ if (this.type === "boolean") {
+ return "";
+ }
+ return this.args.info.nullable ? "" : "required";
+ }
+
digitalizeCategoryId(value) {
value = String(value || "");
const isPositiveInt = /^\d+$/.test(value);
@@ -178,37 +245,27 @@ export default class ParamInput extends Component {
}
@action
- updateValue(input) {
- // handle selectKit inputs as well as traditional inputs
- const value = input.target ? input.target.value : input;
- if (value.length) {
- this.value = this.normalizeValue(value.toString());
- } else {
- this.value = this.normalizeValue(value);
+ validate(name, value, { addError }) {
+ const message = this.getError();
+ if (message != null) {
+ // skip require validation for we have used them in @validation
+ if (message === ERRORS.REQUIRED) {
+ return;
+ }
+ addError(name, {
+ title: this.args.info.identifier,
+ message,
+ });
}
-
- this.args.updateParams(this.args.info.identifier, this.value);
}
@action
- updateBoolValue(input) {
- this.boolValue = input.target.checked;
- this.args.updateParams(
- this.args.info.identifier,
- this.boolValue.toString()
- );
- }
-
- @action
- updateNullableBoolValue(input) {
- this.nullableBoolValue = input;
- this.args.updateParams(this.args.info.identifier, this.nullableBoolValue);
- }
-
- @action
- updateGroupValue(input) {
- this.value = input;
- this.args.updateParams(this.args.info.identifier, this.value.join(","));
+ updateValue(input) {
+ // handle selectKit inputs as well as traditional inputs
+ const value = input?.target ? input.target.value : input;
+ this.value = this.normalizeValue(value);
+ this.args.updateParams(this.args.info.identifier, this.provideValue);
+ this.form.set(this.args.info.identifier, value);
}
}
diff --git a/assets/javascripts/discourse/components/param-inputs-wrapper.hbs b/assets/javascripts/discourse/components/param-inputs-wrapper.hbs
index 64707c64..f84b3aab 100644
--- a/assets/javascripts/discourse/components/param-inputs-wrapper.hbs
+++ b/assets/javascripts/discourse/components/param-inputs-wrapper.hbs
@@ -1,12 +1,15 @@
{{#if @hasParams}}
- {{#each @paramInfo as |pinfo|}}
-
- {{/each}}
+
{{/if}}
\ No newline at end of file
diff --git a/assets/javascripts/discourse/controllers/admin-plugins-explorer.js b/assets/javascripts/discourse/controllers/admin-plugins-explorer.js
index 5b6cf99c..1cbf48e9 100644
--- a/assets/javascripts/discourse/controllers/admin-plugins-explorer.js
+++ b/assets/javascripts/discourse/controllers/admin-plugins-explorer.js
@@ -37,6 +37,7 @@ export default class PluginsExplorerController extends Controller {
explain = false;
acceptedImportFileTypes = ["application/json"];
order = null;
+ form = null;
get validQueryPresent() {
return !!this.selectedItem.id;
@@ -352,6 +353,11 @@ export default class PluginsExplorerController extends Controller {
}
}
+ @action
+ onRegisterApi(form) {
+ this.form = form;
+ }
+
@action
updateParams(identifier, value) {
this.selectedItem.set(`params.${identifier}`, value);
@@ -379,6 +385,11 @@ export default class PluginsExplorerController extends Controller {
@action
run() {
+ // For now, this is just a form validation.
+ // defer the validation to load params
+ setTimeout(() => {
+ this.form?.submit();
+ }, 10);
this.setProperties({
loading: true,
showResults: false,
diff --git a/assets/javascripts/discourse/controllers/group-reports-show.js b/assets/javascripts/discourse/controllers/group-reports-show.js
index 8949ed91..92660c30 100644
--- a/assets/javascripts/discourse/controllers/group-reports-show.js
+++ b/assets/javascripts/discourse/controllers/group-reports-show.js
@@ -23,7 +23,7 @@ export default class GroupReportsShowController extends Controller {
@tracked queryGroupBookmark = this.queryGroup?.bookmark;
queryParams = ["params"];
-
+ form = null;
explain = false;
get parsedParams() {
@@ -55,6 +55,7 @@ export default class GroupReportsShowController extends Controller {
@bind
async run() {
+ this.form?.submit();
this.loading = true;
this.showResults = false;
@@ -129,4 +130,9 @@ export default class GroupReportsShowController extends Controller {
updateParams(identifier, value) {
this.set(`model.params.${identifier}`, value);
}
+
+ @action
+ onRegisterApi(form) {
+ this.form = form;
+ }
}
diff --git a/assets/javascripts/discourse/models/query.js b/assets/javascripts/discourse/models/query.js
index 7d7407d4..716cd7e1 100644
--- a/assets/javascripts/discourse/models/query.js
+++ b/assets/javascripts/discourse/models/query.js
@@ -24,9 +24,17 @@ export default class Query extends RestModel {
return getURL(`/admin/plugins/explorer/queries/${this.id}.json?export=1`);
}
- @computed("param_info")
+ @computed("param_info", "updateing")
get hasParams() {
- return this.param_info.length;
+ // When saving, we need to refresh the param-input component to clean up the old key
+ return this.param_info.length && !this.updateing;
+ }
+
+ beforeUpdate() {
+ this.set("updateing", true);
+ }
+ afterUpdate() {
+ this.set("updateing", false);
}
resetParams() {
diff --git a/assets/javascripts/discourse/templates/admin/plugins-explorer.hbs b/assets/javascripts/discourse/templates/admin/plugins-explorer.hbs
index 61a0f419..07c03da4 100644
--- a/assets/javascripts/discourse/templates/admin/plugins-explorer.hbs
+++ b/assets/javascripts/discourse/templates/admin/plugins-explorer.hbs
@@ -239,6 +239,7 @@
@initialValues={{this.parsedParams}}
@paramInfo={{this.selectedItem.param_info}}
@updateParams={{this.updateParams}}
+ @onRegisterApi={{this.onRegisterApi}}
/>
{{#if this.runDisabled}}
diff --git a/assets/javascripts/discourse/templates/group-reports-show.hbs b/assets/javascripts/discourse/templates/group-reports-show.hbs
index 7a4ebb92..f6a12bc7 100644
--- a/assets/javascripts/discourse/templates/group-reports-show.hbs
+++ b/assets/javascripts/discourse/templates/group-reports-show.hbs
@@ -9,6 +9,7 @@
@initialValues={{this.parsedParams}}
@paramInfo={{this.model.param_info}}
@updateParams={{this.updateParams}}
+ @onRegisterApi={{this.onRegisterApi}}
/>
input,
.param > .select-kit {
margin: 9px;
}
- .invalid > input {
+ .invalid input {
background-color: var(--danger-low);
}
.invalid .ac-wrap {
@@ -270,9 +275,6 @@ table.group-reports {
max-width: 250px;
}
}
- .param-name {
- margin-right: 1em;
- }
}
.query-list,
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index 3e63947c..d5f3a43e 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -89,6 +89,11 @@ en:
reset_params: "Reset"
search_placeholder: "Search..."
no_search_results: "Sorry, we couldn't find any results matching your text."
+ form:
+ errors:
+ invalid: "Invalid"
+ no_such_category: "No such category"
+ no_such_group: "No such group"
group:
reports: "Reports"
admin:
@@ -109,4 +114,3 @@ en:
label: Data Explorer Query parameters
skip_empty:
label: Skip sending PM if there are no results
-
diff --git a/test/javascripts/components/param-input-test.js b/test/javascripts/components/param-input-test.js
index 72033883..bb1ac5dc 100644
--- a/test/javascripts/components/param-input-test.js
+++ b/test/javascripts/components/param-input-test.js
@@ -25,12 +25,16 @@ module("Data Explorer Plugin | Component | param-input", function (hooks) {
updateParams,
});
- await render(hbs``);
+ await render(hbs`
+ `);
const categoryChooser = selectKit(".category-chooser");