FIX: improve admin api for artifact key values (#1425)

Previously we had a logic error and were showing admins keys
that are not theirs when querying for all keys

This makes the API cleaner, to get all results you need to be explicit always

app/controllers/discourse_ai/ai_bot/artifact_key_values_controller.rb

@@ -102,8 +102,12 @@ module DiscourseAi
 
         query = query.where("key = ?", index_params[:key]) if index_params[:key].present?
 
-        if !index_params[:all_users].to_s == "true" && current_user
-          query = query.where(user_id: current_user.id)
+        if index_params[:all_users].to_s != "true"
+          if current_user
+            query = query.where(user_id: current_user.id)
+          else
+            query = query.where("1 = 0")
+          end
         end
 
         query

spec/requests/ai_bot/artifact_key_values_controller_spec.rb

@@ -50,9 +50,23 @@ RSpec.describe DiscourseAi::AiBot::ArtifactKeyValuesController do
       )
     end
 
+    fab!(:admin_key_value) do
+      Fabricate(
+        :ai_artifact_key_value,
+        ai_artifact: artifact,
+        user: admin,
+        key: "admin_key",
+        value: "admin_value",
+        public: false,
+      )
+    end
+
     context "when not logged in" do
       it "returns only public key values" do
-        get "/discourse-ai/ai-bot/artifact-key-values/#{artifact.id}.json"
+        get "/discourse-ai/ai-bot/artifact-key-values/#{artifact.id}.json",
+            params: {
+              all_users: true,
+            }
 
         expect(response.status).to eq(200)
         json = response.parsed_body
@@ -86,7 +100,10 @@ RSpec.describe DiscourseAi::AiBot::ArtifactKeyValuesController do
       before { sign_in(user) }
 
       it "returns public key values and own private key values" do
-        get "/discourse-ai/ai-bot/artifact-key-values/#{artifact.id}.json"
+        get "/discourse-ai/ai-bot/artifact-key-values/#{artifact.id}.json",
+            params: {
+              all_users: true,
+            }
 
         expect(response.status).to eq(200)
         json = response.parsed_body
@@ -138,17 +155,13 @@ RSpec.describe DiscourseAi::AiBot::ArtifactKeyValuesController do
     context "when logged in as admin" do
       before { sign_in(admin) }
 
-      it "returns all key values including private ones from other users" do
+      it "returns only my own keys by default" do
         get "/discourse-ai/ai-bot/artifact-key-values/#{artifact.id}.json"
 
         expect(response.status).to eq(200)
         json = response.parsed_body
-        expect(json["key_values"].length).to eq(3)
-        expect(json["key_values"].map { |kv| kv["key"] }).to contain_exactly(
-          "test_key",
-          "private_key",
-          "other_key",
-        )
+        expect(json["key_values"].length).to eq(1)
+        expect(json["key_values"].map { |kv| kv["key"] }).to contain_exactly("admin_key")
       end
 
       it "can access private artifacts" do
@@ -420,24 +433,4 @@ RSpec.describe DiscourseAi::AiBot::ArtifactKeyValuesController do
       end
     end
   end
-
-  describe "private methods" do
-    let(:controller) { described_class.new }
-
-    before do
-      controller.instance_variable_set(:@artifact, artifact)
-      allow(controller).to receive(:params).and_return(
-        ActionController::Parameters.new(test_params),
-      )
-    end
-
-    describe "#key_value_params" do
-      let(:test_params) { { key: "test", value: "value", public: true, extra: "ignored" } }
-
-      it "permits only allowed parameters" do
-        # This would need to be tested by calling the actual method or through integration tests
-        # since private methods are typically tested through their public interfaces
-      end
-    end
-  end
 end