UX: No admin header for edit personas tools or llms (#1021)

lis2 · web-flow · commit 04c4ff8cf059 · 2024-12-12T10:48:58.000+11:00
In this PR, we added functionality to hide the admin header for edit/new actions - discourse/discourse#30175 To make it work properly, we have to rename `show` to `edit` which is also a more accurate name.
diff --git a/admin/assets/javascripts/discourse/routes/admin-plugins-show-discourse-ai-llms-edit.js b/admin/assets/javascripts/discourse/routes/admin-plugins-show-discourse-ai-llms-edit.js
@@ -1,6 +1,6 @@
 import DiscourseRoute from "discourse/routes/discourse";
 
-export default class AdminPluginsShowDiscourseAiLlmsShow extends DiscourseRoute {
+export default class AdminPluginsShowDiscourseAiLlmsEdit extends DiscourseRoute {
   async model(params) {
     const allLlms = this.modelFor("adminPlugins.show.discourse-ai-llms");
     const id = parseInt(params.id, 10);
diff --git a/admin/assets/javascripts/discourse/routes/admin-plugins-show-discourse-ai-personas-edit.js b/admin/assets/javascripts/discourse/routes/admin-plugins-show-discourse-ai-personas-edit.js
@@ -1,6 +1,6 @@
 import DiscourseRoute from "discourse/routes/discourse";
 
-export default class AdminPluginsShowDiscourseAiPersonasShow extends DiscourseRoute {
+export default class AdminPluginsShowDiscourseAiPersonasEdit extends DiscourseRoute {
   async model(params) {
     const allPersonas = this.modelFor(
       "adminPlugins.show.discourse-ai-personas"
diff --git a/admin/assets/javascripts/discourse/routes/admin-plugins-show-discourse-ai-tools-edit.js b/admin/assets/javascripts/discourse/routes/admin-plugins-show-discourse-ai-tools-edit.js
@@ -1,6 +1,6 @@
 import DiscourseRoute from "discourse/routes/discourse";
 
-export default class DiscourseAiToolsShowRoute extends DiscourseRoute {
+export default class DiscourseAiToolsEditRoute extends DiscourseRoute {
   async model(params) {
     const allTools = this.modelFor("adminPlugins.show.discourse-ai-tools");
     const id = parseInt(params.id, 10);
diff --git a/admin/assets/javascripts/discourse/templates/admin-plugins/show/discourse-ai-llms/edit.hbs b/admin/assets/javascripts/discourse/templates/admin-plugins/show/discourse-ai-llms/edit.hbs
diff --git a/admin/assets/javascripts/discourse/templates/admin-plugins/show/discourse-ai-personas/edit.hbs b/admin/assets/javascripts/discourse/templates/admin-plugins/show/discourse-ai-personas/edit.hbs
diff --git a/admin/assets/javascripts/discourse/templates/admin-plugins/show/discourse-ai-tools/edit.hbs b/admin/assets/javascripts/discourse/templates/admin-plugins/show/discourse-ai-tools/edit.hbs
diff --git a/app/controllers/discourse_ai/admin/ai_llms_controller.rb b/app/controllers/discourse_ai/admin/ai_llms_controller.rb
@@ -30,7 +30,10 @@ def index
                }
       end
 
-      def show
+      def new
+      end
+
+      def edit
         llm_model = LlmModel.find(params[:id])
         render json: LlmModelSerializer.new(llm_model)
       end
diff --git a/app/controllers/discourse_ai/admin/ai_personas_controller.rb b/app/controllers/discourse_ai/admin/ai_personas_controller.rb
@@ -5,7 +5,7 @@ module Admin
     class AiPersonasController < ::Admin::AdminController
       requires_plugin ::DiscourseAi::PLUGIN_NAME
 
-      before_action :find_ai_persona, only: %i[show update destroy create_user]
+      before_action :find_ai_persona, only: %i[edit update destroy create_user]
 
       def index
         ai_personas =
@@ -33,7 +33,10 @@ def index
         render json: { ai_personas: ai_personas, meta: { tools: tools, llms: llms } }
       end
 
-      def show
+      def new
+      end
+
+      def edit
         render json: LocalizedAiPersonaSerializer.new(@ai_persona)
       end
 
diff --git a/app/controllers/discourse_ai/admin/ai_tools_controller.rb b/app/controllers/discourse_ai/admin/ai_tools_controller.rb
@@ -5,14 +5,17 @@ module Admin
     class AiToolsController < ::Admin::AdminController
       requires_plugin ::DiscourseAi::PLUGIN_NAME
 
-      before_action :find_ai_tool, only: %i[test show update destroy]
+      before_action :find_ai_tool, only: %i[test edit update destroy]
 
       def index
         ai_tools = AiTool.all
         render_serialized({ ai_tools: ai_tools }, AiCustomToolListSerializer, root: false)
       end
 
-      def show
+      def new
+      end
+
+      def edit
         render_serialized(@ai_tool, AiCustomToolSerializer)
       end
 
diff --git a/assets/javascripts/discourse/admin-discourse-ai-plugin-route-map.js b/assets/javascripts/discourse/admin-discourse-ai-plugin-route-map.js
@@ -6,17 +6,17 @@ export default {
   map() {
     this.route("discourse-ai-personas", { path: "ai-personas" }, function () {
       this.route("new");
-      this.route("show", { path: "/:id" });
+      this.route("edit", { path: "/:id/edit" });
     });
 
     this.route("discourse-ai-llms", { path: "ai-llms" }, function () {
       this.route("new");
-      this.route("show", { path: "/:id" });
+      this.route("edit", { path: "/:id/edit" });
     });
 
     this.route("discourse-ai-tools", { path: "ai-tools" }, function () {
       this.route("new");
-      this.route("show", { path: "/:id" });
+      this.route("edit", { path: "/:id/edit" });
     });
     this.route("discourse-ai-spam", { path: "ai-spam" });
     this.route("discourse-ai-usage", { path: "ai-usage" });
diff --git a/assets/javascripts/discourse/components/ai-llms-list-editor.gjs b/assets/javascripts/discourse/components/ai-llms-list-editor.gjs
@@ -167,7 +167,7 @@ export default class AiLlmsListEditor extends Component {
                     </td>
                     <td class="d-admin-row__controls">
                       <LinkTo
-                        @route="adminPlugins.show.discourse-ai-llms.show"
+                        @route="adminPlugins.show.discourse-ai-llms.edit"
                         class="btn btn-default btn-small ai-llm-list__edit-button"
                         @model={{llm.id}}
                       >
diff --git a/assets/javascripts/discourse/components/ai-persona-editor.gjs b/assets/javascripts/discourse/components/ai-persona-editor.gjs
@@ -135,7 +135,7 @@ export default class PersonaEditor extends Component {
       if (isNew && this.args.model.rag_uploads.length === 0) {
         this.args.personas.addObject(this.args.model);
         this.router.transitionTo(
-          "adminPlugins.show.discourse-ai-personas.show",
+          "adminPlugins.show.discourse-ai-personas.edit",
           this.args.model
         );
       } else {
diff --git a/assets/javascripts/discourse/components/ai-persona-list-editor.gjs b/assets/javascripts/discourse/components/ai-persona-list-editor.gjs
@@ -92,7 +92,7 @@ export default class AiPersonaListEditor extends Component {
                   </td>
                   <td class="d-admin-row__controls">
                     <LinkTo
-                      @route="adminPlugins.show.discourse-ai-personas.show"
+                      @route="adminPlugins.show.discourse-ai-personas.edit"
                       @model={{persona}}
                       class="btn btn-text btn-small"
                     >{{i18n "discourse_ai.ai_persona.edit"}} </LinkTo>
diff --git a/assets/javascripts/discourse/components/ai-tool-editor.gjs b/assets/javascripts/discourse/components/ai-tool-editor.gjs
@@ -103,7 +103,7 @@ export default class AiToolEditor extends Component {
       }
 
       this.router.transitionTo(
-        "adminPlugins.show.discourse-ai-tools.show",
+        "adminPlugins.show.discourse-ai-tools.edit",
         this.args.model
       );
     } catch (e) {
diff --git a/assets/javascripts/discourse/components/ai-tool-list-editor.gjs b/assets/javascripts/discourse/components/ai-tool-list-editor.gjs
@@ -57,7 +57,7 @@ export default class AiToolListEditor extends Component {
                 </td>
                 <td class="d-admin-row__controls">
                   <LinkTo
-                    @route="adminPlugins.show.discourse-ai-tools.show"
+                    @route="adminPlugins.show.discourse-ai-tools.edit"
                     @model={{tool}}
                     class="btn btn-text btn-small"
                   >{{I18n.t "discourse_ai.tools.edit"}}</LinkTo>
diff --git a/config/routes.rb b/config/routes.rb
@@ -53,15 +53,15 @@
 
   scope "/admin/plugins/discourse-ai", constraints: AdminConstraint.new do
     resources :ai_personas,
-              only: %i[index create show update destroy],
+              only: %i[index new create edit update destroy],
               path: "ai-personas",
               controller: "discourse_ai/admin/ai_personas"
 
     post "/ai-personas/stream-reply" => "discourse_ai/admin/ai_personas#stream_reply"
 
     resources(
       :ai_tools,
-      only: %i[index create show update destroy],
+      only: %i[index new create edit update destroy],
       path: "ai-tools",
       controller: "discourse_ai/admin/ai_tools",
     )
@@ -85,7 +85,7 @@
     post "/ai-spam/test", to: "discourse_ai/admin/ai_spam#test"
 
     resources :ai_llms,
-              only: %i[index create show update destroy],
+              only: %i[index new create edit update destroy],
               path: "ai-llms",
               controller: "discourse_ai/admin/ai_llms" do
       collection { get :test }
diff --git a/spec/requests/admin/ai_personas_controller_spec.rb b/spec/requests/admin/ai_personas_controller_spec.rb
@@ -141,9 +141,9 @@
     end
   end
 
-  describe "GET #show" do
+  describe "GET #edit" do
     it "returns a success response" do
-      get "/admin/plugins/discourse-ai/ai-personas/#{ai_persona.id}.json"
+      get "/admin/plugins/discourse-ai/ai-personas/#{ai_persona.id}/edit.json"
       expect(response).to be_successful
       expect(response.parsed_body["ai_persona"]["name"]).to eq(ai_persona.name)
     end
@@ -152,7 +152,7 @@
       upload = Fabricate(:upload)
       RagDocumentFragment.link_target_and_uploads(ai_persona, [upload.id])
 
-      get "/admin/plugins/discourse-ai/ai-personas/#{ai_persona.id}.json"
+      get "/admin/plugins/discourse-ai/ai-personas/#{ai_persona.id}/edit.json"
       expect(response).to be_successful
 
       serialized_persona = response.parsed_body["ai_persona"]
diff --git a/spec/requests/admin/ai_tools_controller_spec.rb b/spec/requests/admin/ai_tools_controller_spec.rb
@@ -35,9 +35,9 @@
     end
   end
 
-  describe "GET #show" do
+  describe "GET #edit" do
     it "returns a success response" do
-      get "/admin/plugins/discourse-ai/ai-tools/#{ai_tool.id}.json"
+      get "/admin/plugins/discourse-ai/ai-tools/#{ai_tool.id}/edit.json"
       expect(response).to be_successful
       expect(response.parsed_body["ai_tool"]["name"]).to eq(ai_tool.name)
     end
diff --git a/spec/system/admin_ai_persona_spec.rb b/spec/system/admin_ai_persona_spec.rb
@@ -2,6 +2,7 @@
 
 RSpec.describe "Admin AI persona configuration", type: :system, js: true do
   fab!(:admin)
+  let(:admin_header) { PageObjects::Components::AdminHeader.new }
 
   before do
     SiteSetting.ai_bot_enabled = true
@@ -11,7 +12,13 @@
 
   it "allows creation of a persona" do
     visit "/admin/plugins/discourse-ai/ai-personas"
+
+    expect(admin_header).to be_visible
+
     find(".ai-persona-list-editor__new-button").click()
+
+    expect(admin_header).to be_hidden
+
     find(".ai-persona-editor__name").set("Test Persona")
     find(".ai-persona-editor__description").fill_in(with: "I am a test persona")
     find(".ai-persona-editor__system_prompt").fill_in(with: "You are a helpful bot")
@@ -35,7 +42,7 @@
 
     expect(page).not_to have_current_path("/admin/plugins/discourse-ai/ai-personas/new")
 
-    persona_id = page.current_path.split("/").last.to_i
+    persona_id = page.current_path.split("/")[-2].to_i
 
     persona = AiPersona.find(persona_id)
     expect(persona.name).to eq("Test Persona")
@@ -46,15 +53,15 @@
   end
 
   it "will not allow deletion or editing of system personas" do
-    visit "/admin/plugins/discourse-ai/ai-personas/#{DiscourseAi::AiBot::Personas::Persona.system_personas.values.first}"
+    visit "/admin/plugins/discourse-ai/ai-personas/#{DiscourseAi::AiBot::Personas::Persona.system_personas.values.first}/edit"
     expect(page).not_to have_selector(".ai-persona-editor__delete")
     expect(find(".ai-persona-editor__system_prompt")).to be_disabled
   end
 
   it "will enable persona right away when you click on enable but does not save side effects" do
     persona = Fabricate(:ai_persona, enabled: false)
 
-    visit "/admin/plugins/discourse-ai/ai-personas/#{persona.id}"
+    visit "/admin/plugins/discourse-ai/ai-personas/#{persona.id}/edit"
 
     find(".ai-persona-editor__name").set("Test Persona 1")
     PageObjects::Components::DToggleSwitch.new(".ai-persona-editor__enabled").toggle
diff --git a/spec/system/ai_bot/tool_spec.rb b/spec/system/ai_bot/tool_spec.rb
@@ -4,6 +4,7 @@
 
 describe "AI Tool Management", type: :system do
   fab!(:admin)
+  let(:admin_header) { PageObjects::Components::AdminHeader.new }
 
   before do
     SiteSetting.ai_embeddings_enabled = true
@@ -36,9 +37,11 @@ def ensure_can_run_test
   it "allows admin to create a new AI tool from preset" do
     visit "/admin/plugins/discourse-ai/ai-tools"
 
+    expect(admin_header).to be_visible
     expect(page).to have_content("Tools")
 
     find(".ai-tool-list-editor__new-button").click
+    expect(admin_header).to be_hidden
 
     select_kit = PageObjects::Components::SelectKit.new(".ai-tool-editor__presets")
     select_kit.expand
@@ -58,7 +61,7 @@ def ensure_can_run_test
     expect(page).to have_content("Tool saved")
 
     last_tool = AiTool.order("id desc").limit(1).first
-    visit "/admin/plugins/discourse-ai/ai-tools/#{last_tool.id}"
+    visit "/admin/plugins/discourse-ai/ai-tools/#{last_tool.id}/edit"
 
     ensure_can_run_test
 
diff --git a/spec/system/llms/ai_llm_spec.rb b/spec/system/llms/ai_llm_spec.rb
@@ -2,6 +2,7 @@
 
 RSpec.describe "Managing LLM configurations", type: :system, js: true do
   fab!(:admin)
+  let(:admin_header) { PageObjects::Components::AdminHeader.new }
 
   before do
     SiteSetting.ai_bot_enabled = true
@@ -36,8 +37,10 @@
 
   it "manually configures an LLM" do
     visit "/admin/plugins/discourse-ai/ai-llms"
+    expect(admin_header).to be_visible
 
     find("[data-llm-id='none'] button").click()
+    expect(admin_header).to be_hidden
 
     find("input.ai-llm-editor__display-name").fill_in(with: "Self-hosted LLM")
     find("input.ai-llm-editor__name").fill_in(with: "llava-hf/llava-v1.6-mistral-7b-hf")
