From a5a39dd2ee5a71365cd349cabc1f34e261235d24 Mon Sep 17 00:00:00 2001 From: Jarek Radosz Date: Fri, 28 Jun 2024 00:59:51 +0200 Subject: [PATCH] DEV: Clean up after #677 (#694) Follow up to b863ddc94bf03e1868845e10ba744bef1f68841d Ruby: * Validate `summary` (the column is `not null`) * Fix `name` validation (the column has `max_length` 100) * Fix table annotations * Accept missing `parameter` attributes (`required, `enum`, `enum_values`) JS: * Use native classes * Don't use ember's array extensions * Add explicit service injections * Correct class names * Use `||=` operator * Use `store` service to create records * Remove unused service injections * Extract consts * Group actions together * Use `async`/`await` * Use `withEventValue` * Sort html attributes * Use DButtons `@label` arg * Use `input` elements instead of Ember's `Input` component (same w/ textarea) * Remove `btn-default` class (automatically applied by DButton) * Don't mix `I18n.t` and `i18n` in the same template * Don't track props that aren't used in a template * Correct invalid `target.value` code * Remove unused/invalid `this.parameter`/`onChange` code * Whitespace * Use the new service import `inject as service` -> `service` * Use `Object.entries()` * Add missing i18n strings * Fix an error in `addEnumValue` (calling `pushObject` on `undefined`) * Use `TrackedArray`/`TrackedObject` * Transform tool `parameters` keys (`enumValues` -> `enum_values`) --- ...min-plugins-show-discourse-ai-tools-new.js | 15 +- ...in-plugins-show-discourse-ai-tools-show.js | 15 +- .../admin-plugins-show-discourse-ai-tools.js | 3 + .../discourse_ai/admin/ai_tools_controller.rb | 2 +- app/models/ai_tool.rb | 7 +- .../discourse/admin/adapters/ai-tool.js | 2 +- .../discourse/admin/models/ai-tool.js | 20 ++- .../discourse/components/ai-tool-editor.gjs | 131 ++++++++++-------- .../components/ai-tool-list-editor.gjs | 5 +- .../components/ai-tool-parameter-editor.gjs | 122 ++++++++-------- .../components/modal/ai-tool-test-modal.gjs | 22 +-- assets/javascripts/discourse/lib/utilities.js | 29 ++-- config/locales/client.en.yml | 2 + 13 files changed, 197 insertions(+), 178 deletions(-) diff --git a/admin/assets/javascripts/discourse/routes/admin-plugins-show-discourse-ai-tools-new.js b/admin/assets/javascripts/discourse/routes/admin-plugins-show-discourse-ai-tools-new.js index 5f669195..5c05907f 100644 --- a/admin/assets/javascripts/discourse/routes/admin-plugins-show-discourse-ai-tools-new.js +++ b/admin/assets/javascripts/discourse/routes/admin-plugins-show-discourse-ai-tools-new.js @@ -1,16 +1,15 @@ import DiscourseRoute from "discourse/routes/discourse"; -export default DiscourseRoute.extend({ +export default class DiscourseAiToolsNewRoute extends DiscourseRoute { async model() { - const record = this.store.createRecord("ai-tool"); - return record; - }, + return this.store.createRecord("ai-tool"); + } - setupController(controller, model) { - this._super(controller, model); + setupController(controller) { + super.setupController(...arguments); const toolsModel = this.modelFor("adminPlugins.show.discourse-ai-tools"); controller.set("allTools", toolsModel); controller.set("presets", toolsModel.resultSetMeta.presets); - }, -}); + } +} diff --git a/admin/assets/javascripts/discourse/routes/admin-plugins-show-discourse-ai-tools-show.js b/admin/assets/javascripts/discourse/routes/admin-plugins-show-discourse-ai-tools-show.js index d111e02e..c76083b2 100644 --- a/admin/assets/javascripts/discourse/routes/admin-plugins-show-discourse-ai-tools-show.js +++ b/admin/assets/javascripts/discourse/routes/admin-plugins-show-discourse-ai-tools-show.js @@ -1,17 +1,18 @@ import DiscourseRoute from "discourse/routes/discourse"; -export default DiscourseRoute.extend({ +export default class DiscourseAiToolsShowRoute extends DiscourseRoute { async model(params) { const allTools = this.modelFor("adminPlugins.show.discourse-ai-tools"); const id = parseInt(params.id, 10); - return allTools.findBy("id", id); - }, - setupController(controller, model) { - this._super(controller, model); + return allTools.find((tool) => tool.id === id); + } + + setupController(controller) { + super.setupController(...arguments); const toolsModel = this.modelFor("adminPlugins.show.discourse-ai-tools"); controller.set("allTools", toolsModel); controller.set("presets", toolsModel.resultSetMeta.presets); - }, -}); + } +} diff --git a/admin/assets/javascripts/discourse/routes/admin-plugins-show-discourse-ai-tools.js b/admin/assets/javascripts/discourse/routes/admin-plugins-show-discourse-ai-tools.js index b24e96ab..b8d1117f 100644 --- a/admin/assets/javascripts/discourse/routes/admin-plugins-show-discourse-ai-tools.js +++ b/admin/assets/javascripts/discourse/routes/admin-plugins-show-discourse-ai-tools.js @@ -1,6 +1,9 @@ +import { service } from "@ember/service"; import DiscourseRoute from "discourse/routes/discourse"; export default class DiscourseAiToolsRoute extends DiscourseRoute { + @service store; + model() { return this.store.findAll("ai-tool"); } diff --git a/app/controllers/discourse_ai/admin/ai_tools_controller.rb b/app/controllers/discourse_ai/admin/ai_tools_controller.rb index 0e61d923..a0341089 100644 --- a/app/controllers/discourse_ai/admin/ai_tools_controller.rb +++ b/app/controllers/discourse_ai/admin/ai_tools_controller.rb @@ -81,7 +81,7 @@ module DiscourseAi :description, :script, :summary, - parameters: %i[name type description], + parameters: [:name, :type, :description, :required, :enum, enum_values: []], ) end end diff --git a/app/models/ai_tool.rb b/app/models/ai_tool.rb index 79287573..8b286c38 100644 --- a/app/models/ai_tool.rb +++ b/app/models/ai_tool.rb @@ -1,8 +1,9 @@ # frozen_string_literal: true class AiTool < ActiveRecord::Base - validates :name, presence: true, length: { maximum: 255 } + validates :name, presence: true, length: { maximum: 100 } validates :description, presence: true, length: { maximum: 1000 } + validates :summary, presence: true, length: { maximum: 255 } validates :script, presence: true, length: { maximum: 100_000 } validates :created_by_id, presence: true belongs_to :created_by, class_name: "User" @@ -174,10 +175,12 @@ end # # id :bigint not null, primary key # name :string not null -# description :text not null +# description :string not null +# summary :string not null # parameters :jsonb not null # script :text not null # created_by_id :integer not null +# enabled :boolean default(TRUE), not null # created_at :datetime not null # updated_at :datetime not null # diff --git a/assets/javascripts/discourse/admin/adapters/ai-tool.js b/assets/javascripts/discourse/admin/adapters/ai-tool.js index 15cfeb84..bcd38472 100644 --- a/assets/javascripts/discourse/admin/adapters/ai-tool.js +++ b/assets/javascripts/discourse/admin/adapters/ai-tool.js @@ -1,6 +1,6 @@ import RestAdapter from "discourse/adapters/rest"; -export default class Adapter extends RestAdapter { +export default class AiToolAdapter extends RestAdapter { jsonMode = true; basePath() { diff --git a/assets/javascripts/discourse/admin/models/ai-tool.js b/assets/javascripts/discourse/admin/models/ai-tool.js index d1f32ce5..7f1a0877 100644 --- a/assets/javascripts/discourse/admin/models/ai-tool.js +++ b/assets/javascripts/discourse/admin/models/ai-tool.js @@ -1,3 +1,4 @@ +import { TrackedArray, TrackedObject } from "@ember-compat/tracked-built-ins"; import RestModel from "discourse/models/rest"; const CREATE_ATTRIBUTES = [ @@ -20,8 +21,21 @@ export default class AiTool extends RestModel { } workingCopy() { - let attrs = this.getProperties(CREATE_ATTRIBUTES); - attrs.parameters = attrs.parameters || []; - return AiTool.create(attrs); + const attrs = this.getProperties(CREATE_ATTRIBUTES); + + attrs.parameters = new TrackedArray( + attrs.parameters?.map((p) => { + const parameter = new TrackedObject(p); + + if (parameter.enum_values) { + parameter.enumValues = new TrackedArray(parameter.enum_values); + delete parameter.enum_values; + } + + return parameter; + }) + ); + + return this.store.createRecord("ai-tool", attrs); } } diff --git a/assets/javascripts/discourse/components/ai-tool-editor.gjs b/assets/javascripts/discourse/components/ai-tool-editor.gjs index 1882f440..73893689 100644 --- a/assets/javascripts/discourse/components/ai-tool-editor.gjs +++ b/assets/javascripts/discourse/components/ai-tool-editor.gjs @@ -1,14 +1,15 @@ import Component from "@glimmer/component"; import { tracked } from "@glimmer/tracking"; -import { Input } from "@ember/component"; +import { fn } from "@ember/helper"; +import { on } from "@ember/modifier"; import { action } from "@ember/object"; import didInsert from "@ember/render-modifiers/modifiers/did-insert"; import didUpdate from "@ember/render-modifiers/modifiers/did-update"; -import { inject as service } from "@ember/service"; +import { service } from "@ember/service"; import BackButton from "discourse/components/back-button"; import DButton from "discourse/components/d-button"; -import Textarea from "discourse/components/d-textarea"; import DTooltip from "discourse/components/d-tooltip"; +import withEventValue from "discourse/helpers/with-event-value"; import { popupAjaxError } from "discourse/lib/ajax-error"; import I18n from "discourse-i18n"; import AceEditor from "admin/components/ace-editor"; @@ -16,9 +17,11 @@ import ComboBox from "select-kit/components/combo-box"; import AiToolParameterEditor from "./ai-tool-parameter-editor"; import AiToolTestModal from "./modal/ai-tool-test-modal"; +const ACE_EDITOR_MODE = "javascript"; +const ACE_EDITOR_THEME = "chrome"; + export default class AiToolEditor extends Component { @service router; - @service store; @service dialog; @service modal; @service toasts; @@ -26,18 +29,8 @@ export default class AiToolEditor extends Component { @tracked isSaving = false; @tracked editingModel = null; @tracked showDelete = false; - @tracked selectedPreset = null; - aceEditorMode = "javascript"; - aceEditorTheme = "chrome"; - - @action - updateModel() { - this.editingModel = this.args.model.workingCopy(); - this.showDelete = !this.args.model.isNew; - } - get presets() { return this.args.presets.map((preset) => { return { @@ -51,6 +44,12 @@ export default class AiToolEditor extends Component { return !this.selectedPreset && this.args.model.isNew; } + @action + updateModel() { + this.editingModel = this.args.model.workingCopy(); + this.showDelete = !this.args.model.isNew; + } + @action configurePreset() { this.selectedPreset = this.args.presets.findBy("preset_id", this.presetId); @@ -64,16 +63,23 @@ export default class AiToolEditor extends Component { this.isSaving = true; try { - await this.args.model.save( - this.editingModel.getProperties( - "name", - "description", - "parameters", - "script", - "summary" - ) + const data = this.editingModel.getProperties( + "name", + "description", + "parameters", + "script", + "summary" ); + for (const p of data.parameters) { + if (p.enumValues) { + p.enum_values = p.enumValues; + delete p.enumValues; + } + } + + await this.args.model.save(data); + this.toasts.success({ data: { message: I18n.t("discourse_ai.tools.saved") }, duration: 2000, @@ -97,22 +103,15 @@ export default class AiToolEditor extends Component { delete() { return this.dialog.confirm({ message: I18n.t("discourse_ai.tools.confirm_delete"), - didConfirm: () => { - return this.args.model.destroyRecord().then(() => { - this.args.tools.removeObject(this.args.model); - this.router.transitionTo( - "adminPlugins.show.discourse-ai-tools.index" - ); - }); + didConfirm: async () => { + await this.args.model.destroyRecord(); + + this.args.tools.removeObject(this.args.model); + this.router.transitionTo("adminPlugins.show.discourse-ai-tools.index"); }, }); } - @action - updateScript(script) { - this.editingModel.script = script; - } - @action openTestModal() { this.modal.show(AiToolTestModal, { @@ -129,9 +128,9 @@ export default class AiToolEditor extends Component { />
{{#if this.showPresets}}
@@ -145,18 +144,18 @@ export default class AiToolEditor extends Component {
- {{I18n.t "discourse_ai.tools.next.title"}} - + @label="discourse_ai.tools.next.title" + class="ai-tool-editor__next" + />
{{else}}
-
+
-
+
-
+
+
+
{{I18n.t "discourse_ai.tools.test"}} + @label="discourse_ai.tools.test" + class="ai-tool-editor__test-button" + /> + {{I18n.t "discourse_ai.tools.save"}} + class="btn-primary ai-tool-editor__save" + /> + {{#if this.showDelete}} - {{I18n.t "discourse_ai.tools.delete"}} - + /> {{/if}}
{{/if}} diff --git a/assets/javascripts/discourse/components/ai-tool-list-editor.gjs b/assets/javascripts/discourse/components/ai-tool-list-editor.gjs index a8a470d7..9896007e 100644 --- a/assets/javascripts/discourse/components/ai-tool-list-editor.gjs +++ b/assets/javascripts/discourse/components/ai-tool-list-editor.gjs @@ -1,12 +1,11 @@ import { LinkTo } from "@ember/routing"; import icon from "discourse-common/helpers/d-icon"; -import i18n from "discourse-common/helpers/i18n"; import I18n from "discourse-i18n";