Set status directly instead of in form data

apps/src/code-studio/pd/form_components_func/FormController.jsx

@@ -294,12 +294,13 @@ const FormController = props => {
    *
    * @returns {Object}
    */
-  const serializeFormData = formData => {
+  const serializeFormData = (formData, status) => {
     if (!formData) {
       throw new Error(`formData cannot be undefined`);
     }
     return {
       form_data: formData,
+      status: status,
       ...serializeAdditionalData()
     };
   };
@@ -318,16 +319,13 @@ const FormController = props => {
   };
 
   const makeRequest = applicationStatus => {
-    const dataWithStatus = {...data, status: applicationStatus};
-    setData(dataWithStatus);
-
     const ajaxRequest = (method, endpoint) =>
       $.ajax({
         method: method,
         url: endpoint,
         contentType: 'application/json',
         dataType: 'json',
-        data: JSON.stringify(serializeFormData(dataWithStatus))
+        data: JSON.stringify(serializeFormData(data, applicationStatus))
       });
 
     return updatedApplicationId

apps/test/unit/code-studio/pd/form_components_func/FormControllerTest.js

@@ -180,6 +180,7 @@ describe('FormController', () => {
     });
 
     describe('Saving', () => {
+      // [MEG] TODO: Refactor this to use savedStatus instead of form_data
       it('Overrides application status as incomplete to data', () => {
         const testData = {
           field1: 'value 1',
@@ -192,7 +193,7 @@ describe('FormController', () => {
         );
         form.findAll('Button')[1].props.onClick();
 
-        expect(getData(DummyPage1)).to.eql({...testData, status: 'incomplete'});
+        expect(getData(DummyPage1)).to.eql({...testData});
       });
 
       it('Disables the save button during save', () => {
@@ -375,6 +376,7 @@ describe('FormController', () => {
           expect(form.findOne('#submit').props.disabled).to.be.false;
         });
 
+        // [MEG] TODO: Refactor this to use savedStatus instead of form_data
         it('Overrides application status as unreviewed on submit', () => {
           const testData = {
             field1: 'value 1',
@@ -389,8 +391,7 @@ describe('FormController', () => {
           triggerSubmit();
 
           expect(getData(DummyPage3)).to.eql({
-            ...testData,
-            status: 'unreviewed'
+            ...testData
           });
         });
 

dashboard/app/controllers/api/v1/pd/application/teacher_applications_controller.rb

@@ -13,10 +13,16 @@ def new_form
 
     # PATCH /api/v1/pd/application/teacher/<applicationId>
     def update
-      form_data_hash = params.try(:[], :form_data) || {}
-      form_data_json = form_data_hash.to_unsafe_h.to_json.strip_utf8mb4
+      form_data_hash = params.try(:[], :form_data)
+      if form_data_hash
+        form_data_json = form_data_hash.to_unsafe_h.to_json.strip_utf8mb4
+        @application.form_data_hash = JSON.parse(form_data_json)
+      end
 
-      @application.form_data_hash = JSON.parse(form_data_json)
+      status = params.try(:[], :status)
+      if status
+        @application.status = status
+      end
 
       if @application.save
         render json: @application, status: :ok

dashboard/app/controllers/api/v1/pd/forms_controller.rb

@@ -4,12 +4,17 @@ def new_form
   end
 
   def create
-    form_data_hash = params.try(:[], :form_data) || {}
-    form_data_json = form_data_hash.to_unsafe_h.to_json.strip_utf8mb4
-
     form = new_form
+
+    form_data_hash = params.try(:[], :form_data)
+    form_data_json = form_data_hash ? form_data_hash.to_unsafe_h.to_json.strip_utf8mb4 : {}.to_json
     form.form_data_hash = JSON.parse(form_data_json)
 
+    status = params.try(:[], :status)
+    if status
+      @application.status = status
+    end
+
     # Check for idempotence
     existing_form = form.check_idempotency
     return render json: {id: existing_form.id}, status: :ok if existing_form

dashboard/app/controllers/pd/application/teacher_application_controller.rb

@@ -24,6 +24,7 @@ def new
       if @application
         return render :submitted if @application.status != 'incomplete'
         @application_id = @application.try(:id)
+        @saved_status = @application.try(:status)
         @form_data = @application.try(:form_data)
       end
 
@@ -36,6 +37,7 @@ def new
           userId: current_user.id,
           schoolId: current_user.school_info&.school&.id,
           applicationId: @application_id,
+          savedStatus: @saved_status,
           savedFormData: @form_data
         }.to_json
       }

dashboard/app/models/concerns/pd/form.rb

@@ -70,7 +70,7 @@ def validate_required_fields
     hash = sanitize_and_trim_form_data_hash
 
     # No validation is required if the application is still in progress
-    return if hash && hash[:status] == 'incomplete'
+    return if status == 'incomplete'
 
     self.class.required_fields.each do |key|
       add_key_error(key) unless hash.key?(key)

dashboard/app/models/pd/application/application_base.rb

@@ -56,10 +56,7 @@ class ApplicationBase < ApplicationRecord
 
     after_initialize :set_type_and_year
 
-    # Set the status only if (a) the application is new, or (b) the applicant updates form_data.
-    # Avoid setting the status when an RP or admin changes the application status, which only happens
-    # from the application dashboard, not from changing form_data.
-    before_validation :set_status
+    before_validation -> {self.status = 'unreviewed' unless status}
     validate :status_is_valid_for_application_type
     validates_presence_of :type
     validates_presence_of :user_id, unless: proc {|application| application.application_type == PRINCIPAL_APPROVAL_APPLICATION}
@@ -70,7 +67,6 @@ class ApplicationBase < ApplicationRecord
     # An application either has an "incomplete" or "unreviewed" state when created.
     # After creation, an RP or admin can change the status to "accepted," which triggers update_accepted_data.
     before_save :update_accepted_date, if: :status_changed?
-    before_save :remove_status_from_form_data
 
     before_create :generate_application_guid, if: -> {application_guid.blank?}
     after_destroy :delete_unsent_email
@@ -89,17 +85,6 @@ class ApplicationBase < ApplicationRecord
       csa
     ).index_by(&:to_sym).freeze
 
-    def set_status
-      return unless new_record? || (sanitize_form_data_hash && sanitize_form_data_hash[:status])
-      self.status = 'unreviewed' if new_record?
-      self.status = sanitize_form_data_hash[:status] if sanitize_form_data_hash && sanitize_form_data_hash[:status]
-    end
-
-    def remove_status_from_form_data
-      return unless form_data_hash && form_data_hash["status"]
-      self.form_data = form_data_hash.except("status").to_json
-    end
-
     def set_type_and_year
       # Override in derived classes and set to valid values.
       # Setting them to nil here fails those validations and prevents this base class from being saved.

dashboard/test/controllers/api/v1/pd/application/teacher_applications_controller_test.rb

@@ -123,7 +123,7 @@ class TeacherApplicationsControllerTest < ::ActionController::TestCase
 
     test 'can submit an empty form if application is incomplete' do
       sign_in @applicant
-      put :create, params: {form_data: {status: 'incomplete'}}
+      put :create, params: {status: 'incomplete'}
 
       assert_equal 'incomplete', TEACHER_APPLICATION_CLASS.last.status
       assert_response :created
@@ -134,7 +134,7 @@ class TeacherApplicationsControllerTest < ::ActionController::TestCase
       Pd::Application::TeacherApplication.expects(:queue_email).never
 
       sign_in @applicant
-      put :create, params: {form_data: {status: 'incomplete'}}
+      put :create, params: {status: 'incomplete'}
       assert_response :created
     end
 
@@ -144,7 +144,8 @@ class TeacherApplicationsControllerTest < ::ActionController::TestCase
       original_data = application.form_data_hash
       original_school_info = @applicant.school_info
 
-      put :update, params: {id: application.id, form_data: {status: 'incomplete'}}
+      # Keep cs_total_course_hours because it is calculated on create or update
+      put :update, params: {id: application.id, status: 'incomplete', form_data: {"cs_total_course_hours": 80}}
       application.reload
       refute_equal original_data, application.form_data_hash
       assert_nil application.course

dashboard/test/controllers/pd/application/teacher_application_controller_test.rb

@@ -34,8 +34,8 @@ class TeacherApplicationControllerTest < ::ActionController::TestCase
 
     test 'teachers with an incomplete application have an application id and saved form data' do
       application = create :pd_teacher_application, form_data_hash: (
-        build :pd_teacher_application_hash, :incomplete
-      )
+        build :pd_teacher_application_hash
+      ), status: 'incomplete'
       sign_in application.user
       get :new
       assert_response :success
@@ -44,6 +44,7 @@ class TeacherApplicationControllerTest < ::ActionController::TestCase
       @script_data = assigns(:script_data)
       assert_equal application.id, JSON.parse(@script_data.dig(:props)).dig('applicationId')
       assert_equal application.form_data, JSON.parse(@script_data.dig(:props)).dig('savedFormData')
+      assert_equal application.status, JSON.parse(@script_data.dig(:props)).dig('savedStatus')
     end
 
     test 'teachers without an application have no id nor form data' do

dashboard/test/factories/pd_factories.rb

@@ -809,10 +809,6 @@
       school_type 'Public school'
     end
 
-    trait :incomplete do
-      status 'incomplete'
-    end
-
     trait :with_no_school do
       school(-1)
     end

dashboard/test/models/pd/application/application_base_test.rb

@@ -36,22 +36,15 @@ class ApplicationBaseTest < ActiveSupport::TestCase
       assert application.unreviewed?
     end
 
-    test 'form data overwrites default status' do
-      application = create TEACHER_APPLICATION_FACTORY, form_data: {status: 'incomplete'}.to_json
+    test 'can set a different status from the default' do
+      application = create TEACHER_APPLICATION_FACTORY, status: 'incomplete'
       application.update_status_timestamp_change_log(nil)
 
       assert application.incomplete?
       assert_equal application.sanitize_status_timestamp_change_log.length, 1
       assert application.sanitize_status_timestamp_change_log[0].value?('incomplete')
     end
 
-    test 'status is removed from form_data after status is set' do
-      application = create TEACHER_APPLICATION_FACTORY, form_data: {status: 'incomplete'}.to_json
-
-      assert application.incomplete?
-      assert_nil application.form_data_hash['status']
-    end
-
     test 'can update status' do
       application = create FACILITATOR_APPLICATION_FACTORY
       assert application.unreviewed?

dashboard/test/models/pd/application/teacher_application_test.rb

@@ -55,8 +55,8 @@ class TeacherApplicationTest < ActiveSupport::TestCase
       }
 
       application = create :pd_teacher_application, form_data_hash: (
-        build :pd_teacher_application_hash, :incomplete, custom_minutes_hours_weeks
-      )
+        build :pd_teacher_application_hash, custom_minutes_hours_weeks
+      ), status: 'incomplete'
 
       assert_equal 112, application.sanitize_form_data_hash[:cs_total_course_hours]
     end
@@ -70,8 +70,8 @@ class TeacherApplicationTest < ActiveSupport::TestCase
 
       %i(cs_how_many_minutes cs_how_many_days_per_week cs_how_many_weeks_per_year).each do |attribute|
         application = create :pd_teacher_application, form_data_hash: (
-          build :pd_teacher_application_hash, :incomplete, empty_minutes_hours_weeks.slice(attribute)
-        )
+          build :pd_teacher_application_hash, empty_minutes_hours_weeks.slice(attribute)
+        ), status: 'incomplete'
 
         assert_nil application.sanitize_form_data_hash[:cs_total_course_hours]
       end