Fix permission checks and tests [SCI-11341]

app/controllers/form_fields_controller.rb

@@ -17,6 +17,7 @@ class FormFieldsController < ApplicationController
           }
         )
       )
+
       if @form_field.save
         log_activity(:form_block_added, block_name: @form_field.name)
         render json: @form_field, serializer: FormFieldSerializer, user: current_user

app/controllers/step_elements/base_controller.rb

@@ -5,7 +5,6 @@ module StepElements
     before_action :load_step_and_protocol
     before_action :check_manage_permissions
 
-
     def move_targets
       render json: { targets: @protocol.steps.order(:position).where.not(id: @step.id).map{|i| [i.id, i.name] } }
     end

app/controllers/step_elements/form_responses_controller.rb

@@ -1,15 +1,18 @@
 # frozen_string_literal: true
+
 module StepElements
   class FormResponsesController < BaseController
     before_action :load_form, only: :create
-    before_action :load_parent, only: :create
+    before_action :load_step, only: :create
     before_action :load_form_response, except: :create
+    skip_before_action :check_manage_permissions, only: %i(submit reset)
 
     def create
-      render_403 and return unless can_create_protocol_form_responses?(@parent.protocol)
+      render_403 and return unless can_create_protocol_form_responses?(@step.protocol)
+
       ActiveRecord::Base.transaction do
         @form_response = FormResponse.create!(form: @form, created_by: current_user)
-        @parent.step_orderable_elements.create!(orderable: @form_response)
+        create_in_step!(@step, @form_response)
       end
 
       render_step_orderable_element(@form_response)
@@ -28,7 +31,7 @@ module StepElements
 
       new_form_response = @form_response.reset!(current_user)
 
-      render_step_orderable_element(@form_response)
+      render_step_orderable_element(new_form_response)
     end
 
     private
@@ -43,9 +46,10 @@ module StepElements
       render_404 unless @form && can_read_form?(@form)
     end
 
-    def load_parent
+    def load_step
-      @parent = Step.find_by(id: form_response_params[:step_id])
+      @step = Step.find_by(id: form_response_params[:step_id])
-      render_404 unless @parent
+
+      render_404 unless @step
     end
 
     def load_form_response

app/serializers/step_form_response_serializer.rb

@@ -31,8 +31,8 @@ class StepFormResponseSerializer < ActiveModel::Serializer
       add_value: form_response_form_field_values_path(object)
     }
 
-    list[:submit] = submit_step_form_response_path(object.step ,object) if can_submit_form_response?(user, object)
+    list[:submit] = submit_step_form_response_path(object.step, object) if can_submit_form_response?(user, object)
-    list[:reset] = reset_step_form_response_path(object.step ,object) if can_reset_form_response?(user, object)
+    list[:reset] = reset_step_form_response_path(object.step, object) if can_reset_form_response?(user, object)
 
     list
   end

spec/controllers/form_field_values_controller_spec.rb

@@ -18,7 +18,8 @@ describe FormFieldValuesController, type: :controller do
         form_response_id: form_response.id,
         form_field_value: {
           form_field_id: form_field.id,
-          value: 'Test value'
+          value: 'Test value',
+          not_applicable: false
         }
       }
     end
@@ -50,7 +51,8 @@ describe FormFieldValuesController, type: :controller do
           form_response_id: form_response.id,
           form_field_value: {
             form_field_id: -1,
-            value: 'Test value'
+            value: 'Test value',
+            not_applicable: false
           }
         }
       end
@@ -67,7 +69,8 @@ describe FormFieldValuesController, type: :controller do
           form_response_id: 0,
           form_field_value: {
             form_field_id: form_field.id,
-            value: 'Test value'
+            value: 'Test value',
+            not_applicable: false
           }
         }
       end

spec/controllers/form_fields_controller_spec.rb

@@ -8,8 +8,12 @@ describe FormFieldsController, type: :controller do
   include_context 'reference_project_structure'
 
   let!(:form) { create :form, team: team, created_by: user }
-  let!(:form_field) { create :form_field, form: form, created_by: user }
+  let!(:form_field) { create :form_field, position: 0, form: form, created_by: user }
-  let!(:form_fields) { create_list :form_field, 5, form: form, created_by: user }
+  let!(:form_fields) do
+    5.times.map.with_index do |_, i|
+      create :form_field, position: i + 1, form: form, created_by: user
+    end
+  end
 
   describe 'POST create' do
     let(:action) { post :create, params: params, format: :json }

spec/controllers/step_elements/form_responses_controller_spec.rb

@@ -2,7 +2,7 @@
 
 require 'rails_helper'
 
-describe FormResponsesController, type: :controller do
+describe StepElements::FormResponsesController, type: :controller do
   login_user
 
   include_context 'reference_project_structure'
@@ -11,31 +11,31 @@ describe FormResponsesController, type: :controller do
   let!(:step) { create(:step, protocol: protocol) }
   let!(:protocol) { create(:protocol, added_by: user) }
   let!(:form_response) { create(:form_response, form: form, created_by: user) }
+  let!(:step_orderable_element) { create(:step_orderable_element, orderable: form_response) }
 
   describe 'POST create' do
     let(:action) { post :create, params: params, format: :json }
     let(:params) do
       {
-        form_response: {
+        form_id: form.id,
-          form_id: form.id,
+        step_id: step.id
-          parent_id: step.id,
-          parent_type: 'Step'
-        }
       }
     end
 
     context 'when user has permissions' do
+      before { allow(controller).to receive(:can_manage_step?).and_return(true) }
       before { allow(controller).to receive(:can_create_protocol_form_responses?).and_return(true) }
 
       it 'creates a form response successfully' do
         expect { action }.to change(FormResponse, :count).by(1)
         expect(response).to have_http_status(:success)
         response_body = JSON.parse(response.body)
-        expect(response_body['data']['attributes']['form_id']).to eq form.id
+        expect(response_body.dig('data', 'attributes', 'orderable', 'form', 'id')).to eq form.id
       end
     end
 
     context 'when user lacks permissions' do
+      before { allow(controller).to receive(:can_manage_step?).and_return(true) }
       before { allow(controller).to receive(:can_create_protocol_form_responses?).and_return(false) }
 
       it 'returns a forbidden response' do
@@ -43,27 +43,18 @@ describe FormResponsesController, type: :controller do
         expect(response).to have_http_status(:forbidden)
       end
     end
-
-    context 'when invalid parent type' do
-      let(:params) do
-        {
-          form_response: {
-            form_id: form.id,
-            parent_id: step.id,
-            parent_type: 'InvalidType'
-          }
-        }
-      end
-
-      it 'returns an unprocessable entity response' do
-        action
-        expect(response).to have_http_status(:unprocessable_entity)
-      end
-    end
   end
 
   describe 'PUT submit' do
-    let(:action) { put :submit, params: { id: form_response.id }, format: :json }
+    let(:action) {
+      put :submit,
+      params: {
+        form_id: form.id,
+        step_id: step.id,
+        id: form_response.id
+      },
+      format: :json
+    }
 
     context 'when user has permissions' do
       before { allow(controller).to receive(:can_submit_form_response?).and_return(true) }
@@ -79,6 +70,7 @@ describe FormResponsesController, type: :controller do
     end
 
     context 'when user lacks permissions' do
+      before { allow(controller).to receive(:can_manage_step?).and_return(true) }
       before { allow(controller).to receive(:can_submit_form_response?).and_return(false) }
 
       it 'returns a forbidden response' do
@@ -89,7 +81,15 @@ describe FormResponsesController, type: :controller do
   end
 
   describe 'PUT reset' do
-    let(:action) { put :reset, params: { id: form_response.id }, format: :json }
+    let(:action) {
+      put :reset,
+      params: {
+        form_id: form.id,
+        step_id: step.id,
+        id: form_response.id
+      },
+      format: :json
+    }
 
     context 'when user has permissions and form is submitted' do
       before do