harden request_changes API endpoint and expand test coverage

- Return 422 if submitter has not yet completed the form
- Wrap state update in a transaction and create a SubmissionEvent (with optional reason and requested_by) to match web controller behavior
- Add tests for: SubmissionEvent creation, reason param, non-completed submitter guard, and idempotent protection (duplicate creation on multiple calls)
- Clean up routes: replace %i[] with [] and 'slug' string with :slug symbol

Author: ryanarakawa

app/controllers/api/submitters_request_changes_controller.rb

@@ -5,7 +5,23 @@ class SubmittersRequestChangesController < ApiBaseController
     before_action :load_submitter
 
     def request_changes
-      @submitter.update!(changes_requested_at: Time.current, completed_at: nil) unless @submitter.changes_requested_at?
+      unless @submitter.completed_at?
+        return render json: { error: 'Submitter has not completed the form' }, status: :unprocessable_entity
+      end
+
+      unless @submitter.changes_requested_at?
+        ApplicationRecord.transaction do
+          @submitter.update!(changes_requested_at: Time.current, completed_at: nil)
+
+          SubmissionEvents.create_with_tracking_data(
+            @submitter,
+            'request_changes',
+            request,
+            { reason: params[:reason], requested_by: current_user.id },
+            current_user
+          )
+        end
+      end
 
       render json: Submitters::SerializeForApi.call(@submitter), status: :ok
     end

config/routes.rb

@@ -34,7 +34,7 @@
     resources :submitter_email_clicks, only: %i[create]
     resources :submitter_form_views, only: %i[create]
     resources :submitters, only: %i[index show update]
-    resources :submitters, only: %i[], param: 'slug' do
+    resources :submitters, only: [], param: :slug do
       member do
         post :request_changes, controller: 'submitters_request_changes'
       end

spec/requests/api_submitters_request_changes_spec.rb

@@ -27,14 +27,55 @@
         expect(response).to have_http_status(:ok)
       end
 
-      it 'is idempotent when changes already requested' do
-        submitter.update!(changes_requested_at: 1.hour.ago)
+      it 'creates a submission event' do
+        expect do
+          post "/api/submitters/#{submitter.slug}/request_changes",
+               headers: { 'x-auth-token': user.access_token.token }
+        end.to change(SubmissionEvent, :count).by(1)
 
+        event = SubmissionEvent.last
+        expect(event.event_type).to eq('request_changes')
+        expect(event.submitter).to eq(submitter)
+      end
+
+      it 'records the reason in the submission event when provided' do
         expect do
+          post "/api/submitters/#{submitter.slug}/request_changes",
+               params: { reason: 'Missing signature on page 2' }.to_json,
+               headers: { 'x-auth-token': user.access_token.token }
+        end.to change(SubmissionEvent, :count).by(1)
+
+        expect(SubmissionEvent.last.data).to include('reason' => 'Missing signature on page 2')
+      end
+
+      context 'when submitter has not completed the form' do
+        let(:submitter) do
+          create(
+            :submitter,
+            submission:,
+            account:,
+            completed_at: nil,
+            uuid: template.submitters.first['uuid']
+          )
+        end
+
+        it 'returns unprocessable entity' do
           post "/api/submitters/#{submitter.slug}/request_changes",
                headers: { 'x-auth-token': user.access_token.token }
-        end.not_to(change { submitter.reload.changes_requested_at })
 
+          expect(response).to have_http_status(:unprocessable_entity)
+        end
+      end
+
+      it 'is idempotent when changes already requested' do
+        submitter.update!(changes_requested_at: 1.hour.ago)
+        original_changes_requested_at = submitter.changes_requested_at
+
+        post "/api/submitters/#{submitter.slug}/request_changes",
+             headers: { 'x-auth-token': user.access_token.token }
+
+        expect(submitter.reload.changes_requested_at).to eq(original_changes_requested_at)
+        expect(SubmissionEvent.count).to eq(0)
         expect(response).to have_http_status(:ok)
       end
     end