From 105f3136bb46d12153f78f6c3c21510ba9600bf5 Mon Sep 17 00:00:00 2001
From: Dylan Griffith <dyl.griffith@gmail.com>
Date: Wed, 24 Jul 2024 17:06:58 +1000
Subject: [PATCH] Fix checkpointing API taking primary key id from client
The previous implementation had a bug where `declared_params` was
including the `id` param which was only meant to be in the URL and only
meant for looking up the workflow. But since this was being used to
create the `Checkpoint` the API was only able to create 1 checkpoint for
workflow because the 2nd one would have a duplicate primary key error.
---
ee/lib/api/ai/duo_workflows/workflows.rb | 3 ++-
.../requests/api/ai/duo_workflows/workflows_spec.rb | 13 +++++++++----
2 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/ee/lib/api/ai/duo_workflows/workflows.rb b/ee/lib/api/ai/duo_workflows/workflows.rb
index d9a10bcb76105..144be0b3d5b3f 100644
--- a/ee/lib/api/ai/duo_workflows/workflows.rb
+++ b/ee/lib/api/ai/duo_workflows/workflows.rb
@@ -54,8 +54,9 @@ def authorize_run_workflows!(project)
end
post '/:id/checkpoints' do
workflow = find_workflow!(params[:id])
+ checkpoint_params = declared_params(include_missing: false).except(:id)
service = ::Ai::DuoWorkflows::CreateCheckpointService.new(project: workflow.project,
- workflow: workflow, params: declared_params(include_missing: false))
+ workflow: workflow, params: checkpoint_params)
result = service.execute
bad_request!(result[:message]) if result[:status] == :error
diff --git a/ee/spec/requests/api/ai/duo_workflows/workflows_spec.rb b/ee/spec/requests/api/ai/duo_workflows/workflows_spec.rb
index a1b2fb1e2dbc5..b7e8a6efba0e2 100644
--- a/ee/spec/requests/api/ai/duo_workflows/workflows_spec.rb
+++ b/ee/spec/requests/api/ai/duo_workflows/workflows_spec.rb
@@ -66,18 +66,23 @@
end
describe 'POST /ai/duo_workflows/workflows/:id/checkpoints' do
- let(:thread_ts) { Time.current.to_s }
- let(:parent_ts) { (Time.current - 1.second).to_s }
+ let(:current_time) { Time.current }
+ let(:thread_ts) { current_time.to_s }
+ let(:later_thread_ts) { (current_time + 1.second).to_s }
+ let(:parent_ts) { (current_time - 1.second).to_s }
let(:checkpoint) { { key: 'value' } }
let(:metadata) { { key: 'value' } }
let(:params) { { thread_ts: thread_ts, checkpoint: checkpoint, parent_ts: parent_ts, metadata: metadata } }
let(:path) { "/ai/duo_workflows/workflows/#{workflow.id}/checkpoints" }
- it 'creates a new checkpoint' do
+ it 'allows creating multiple checkpoints for a workflow' do
expect do
post api(path, user), params: params
expect(response).to have_gitlab_http_status(:created)
- end.to change { workflow.reload.checkpoints.count }.by(1)
+
+ post api(path, user), params: params.merge(thread_ts: later_thread_ts, parent_ts: thread_ts)
+ expect(response).to have_gitlab_http_status(:created)
+ end.to change { workflow.reload.checkpoints.count }.by(2)
expect(json_response['id']).to eq(Ai::DuoWorkflows::Checkpoint.last.id)
end
--
GitLab