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

Author: SamSaffron

app/controllers/discourse_ai/ai_bot/artifact_key_values_controller.rb

@@ -102,8 +102,12 @@ def build_index_query
 
         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 @@
       )
     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 @@
       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 @@
     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 @@
       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