diff --git a/app/controllers/case_logs_controller.rb b/app/controllers/case_logs_controller.rb index 311de1d7c..dbc9df1d6 100644 --- a/app/controllers/case_logs_controller.rb +++ b/app/controllers/case_logs_controller.rb @@ -1,6 +1,6 @@ class CaseLogsController < ApplicationController - skip_before_action :verify_authenticity_token, only: [:create], if: :json_request? - before_action :authenticate, only: [:create], if: :json_request? + skip_before_action :verify_authenticity_token, if: :json_create_request? + before_action :authenticate, if: :json_create_request? def index @submitted_case_logs = CaseLog.where(status: 1) @@ -8,10 +8,16 @@ class CaseLogsController < ApplicationController end def create - @case_log = CaseLog.create!(create_params) + case_log = CaseLog.create(create_params) respond_to do |format| - format.html { redirect_to @case_log } - format.json { render json: @case_log } + format.html { redirect_to case_log } + format.json do + if case_log.persisted? + render json: case_log, status: :created + else + render json: { errors: case_log.errors.full_messages }, status: :unprocessable_entity + end + end end end @@ -73,8 +79,8 @@ private end end - def json_request? - request.format.json? + def json_create_request? + (request["action"] == "create") && request.format.json? end def authenticate @@ -84,6 +90,6 @@ private def create_params return {} unless params[:case_log] - params.require(:case_log).permit(CaseLog.new.attributes.keys) + params.require(:case_log).permit(CaseLog.editable_fields) end end diff --git a/app/models/case_log.rb b/app/models/case_log.rb index aa17eddf6..bcbbc432a 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -3,35 +3,52 @@ class CaseLogValidator < ActiveModel::Validator # this is how the metaprogramming of the method name being # call in the validate method works. - def validate_tenant_code(record) - if record.tenant_code.blank? - record.errors.add :tenant_code, "Tenant code can't be blank" - end - end - def validate_tenant_age(record) - if record.tenant_age.blank? - record.errors.add :tenant_age, "Tenant age can't be blank" - elsif !/^[1-9][0-9]?$|^100$/.match?(record.tenant_age.to_s) - record.errors.add :tenant_age, "Tenant age must be between 0 and 100" + if record.tenant_age && !/^[1-9][0-9]?$|^120$/.match?(record.tenant_age.to_s) + record.errors.add :tenant_age, "must be between 0 and 120" end end def validate(record) + # If we've come from the form UI we only want to validate the specific fields + # that have just been submitted. If we're submitting a log via API or Bulk Upload + # we want to validate all data fields. question_to_validate = options[:previous_page] - if respond_to?("validate_#{question_to_validate}") + if question_to_validate && respond_to?("validate_#{question_to_validate}") public_send("validate_#{question_to_validate}", record) + else + # This assumes that all methods in this class other than this one are + # validations to be run + validation_methods = public_methods(false) - [__callee__] + validation_methods.each { |meth| public_send(meth, record) } end end end class CaseLog < ApplicationRecord validate :instance_validations + before_save :update_status! + attr_writer :previous_page enum status: { "in progress" => 0, "submitted" => 1 } + AUTOGENERATED_FIELDS = %w[status created_at updated_at id].freeze + def instance_validations validates_with CaseLogValidator, ({ previous_page: @previous_page } || {}) end + + def update_status! + self.status = (all_fields_completed? && errors.empty? ? "submitted" : "in progress") + end + + def all_fields_completed? + mandatory_fields = attributes.except(*AUTOGENERATED_FIELDS) + mandatory_fields.none? { |_key, val| val.nil? } + end + + def self.editable_fields + attribute_names - AUTOGENERATED_FIELDS + end end diff --git a/db/migrate/20211013113607_remove_redundant_field.rb b/db/migrate/20211013113607_remove_redundant_field.rb new file mode 100644 index 000000000..5e8be917f --- /dev/null +++ b/db/migrate/20211013113607_remove_redundant_field.rb @@ -0,0 +1,9 @@ +class RemoveRedundantField < ActiveRecord::Migration[6.1] + def up + remove_column :case_logs, :prior_homelessness + end + + def down + add_column :case_logs, :prior_homelessness, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index 08f44eb0a..3e73a03e0 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2021_10_11_115946) do +ActiveRecord::Schema.define(version: 2021_10_13_113607) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -25,7 +25,6 @@ ActiveRecord::Schema.define(version: 2021_10_11_115946) do t.string "tenant_ethnic_group" t.string "tenant_nationality" t.string "previous_housing_situation" - t.integer "prior_homelessness" t.string "armed_forces" t.string "tenant_economic_status" t.integer "household_number_of_other_members" diff --git a/spec/features/case_log_spec.rb b/spec/features/case_log_spec.rb index 4277ea46c..012607651 100644 --- a/spec/features/case_log_spec.rb +++ b/spec/features/case_log_spec.rb @@ -290,9 +290,9 @@ RSpec.describe "Test Features" do expect(page).to have_selector("#case-log-tenant-age-field-error") end - it " of greater than 100 it shows validation" do + it " of greater than 120 it shows validation" do visit("/case_logs/#{id}/tenant_age") - fill_in_number_question(empty_case_log.id, "tenant_age", 120) + fill_in_number_question(empty_case_log.id, "tenant_age", 121) expect(page).to have_selector("#error-summary-title") expect(page).to have_selector("#case-log-tenant-age-error") expect(page).to have_selector("#case-log-tenant-age-field-error") diff --git a/spec/fixtures/complete_case_log.json b/spec/fixtures/complete_case_log.json new file mode 100644 index 000000000..11f073714 --- /dev/null +++ b/spec/fixtures/complete_case_log.json @@ -0,0 +1,117 @@ +{ + "case_log": + { + "tenant_code": "T657", + "tenant_age": 35, + "tenant_gender": "Female", + "tenant_ethnic_group": "White: English/Scottish/Welsh/Northern Irish/British", + "tenant_nationality": "UK national resident in UK", + "previous_housing_situation": "Private sector tenancy", + "armed_forces": "Yes - a regular", + "tenant_economic_status": "Full-time - 30 hours or more", + "household_number_of_other_members": 7, + "person_2_relationship": "Partner", + "person_2_age": 32, + "person_2_gender": "Male", + "person_2_economic_status": "Not seeking work", + "person_3_relationship": "Child - includes young adult and grown-up", + "person_3_age": 12, + "person_3_gender": "Male", + "person_3_economic_status": "Child under 16", + "person_4_relationship": "Child - includes young adult and grown-up", + "person_4_age": 12, + "person_4_gender": "Female", + "person_4_economic_status": "Child under 16", + "person_5_relationship": "Child - includes young adult and grown-up", + "person_5_age": 10, + "person_5_gender": "Non-binary", + "person_5_economic_status": "Child under 16", + "person_6_relationship": "Child - includes young adult and grown-up", + "person_6_age": 5, + "person_6_gender": "Prefer not to say", + "person_6_economic_status": "Child under 16", + "person_7_relationship": "Child - includes young adult and grown-up", + "person_7_age": 5, + "person_7_gender": "Prefer not to say", + "person_7_economic_status": "Child under 16", + "person_8_relationship": "Child - includes young adult and grown-up", + "person_8_age": 2, + "person_8_gender": "Prefer not to say", + "person_8_economic_status": "Child under 16", + "homelessness": "No", + "reason_for_leaving_last_settled_home": "Other problems with neighbours", + "benefit_cap_spare_room_subsidy": "No", + "armed_forces_active": "No", + "armed_forces_injured": "No", + "armed_forces_partner": "No", + "medical_conditions": "Yes", + "pregnancy": "No", + "accessibility_requirements": "No", + "condition_effects": "dummy", + "tenancy_code": "BZ757", + "tenancy_start_date": "12/03/2019", + "starter_tenancy": "No", + "fixed_term_tenancy": "No", + "tenancy_type": "Fixed term – Secure", + "letting_type": "Affordable Rent - General Needs", + "letting_provider": "This landlord", + "property_location": "Barnet", + "previous_postcode": "NW1 5TY", + "property_relet": "No", + "property_vacancy_reason": "Relet - tenant abandoned property", + "property_reference": "P9876", + "property_unit_type": "House", + "property_building_type": "dummy", + "property_number_of_bedrooms": 3, + "property_void_date": "03/11/2019", + "property_major_repairs": "Yes", + "property_major_repairs_date": "05/05/2020", + "property_number_of_times_relet": 2, + "property_wheelchair_accessible": true, + "net_income": 1000, + "net_income_frequency": "Monthly", + "net_income_uc_proportion": "Some", + "housing_benefit": "Universal Credit with housing element, but not Housing Benefit", + "rent_frequency": "Weekly", + "basic_rent": 200, + "service_charge": 50, + "personal_service_charge": 40, + "support_charge": 35, + "total_charge": 325, + "outstanding_amount": "Yes", + "time_lived_in_la": "1 to 2 years", + "time_on_la_waiting_list": "Less than 1 year", + "previous_la": "Ashford", + "property_postcode": "SE2 6RT", + "reasonable_preference": "Yes", + "reasonable_preference_reason": "dummy", + "cbl_letting": true, + "chr_letting": false, + "cap_letting": false, + "outstanding_rent_or_charges": 25, + "other_reason_for_leaving_last_settled_home": "Other reason", + "accessibility_requirements_fully_wheelchair_accessible_housing": true, + "accessibility_requirements_wheelchair_access_to_essential_rooms": false, + "accessibility_requirements_level_access_housing": false, + "accessibility_requirements_other_disability_requirements": false, + "accessibility_requirements_no_disability_requirements": false, + "accessibility_requirements_do_not_know": false, + "accessibility_requirements_prefer_not_to_say": false, + "condition_effects_vision": false, + "condition_effects_hearing": true, + "condition_effects_mobility": false, + "condition_effects_dexterity": false, + "condition_effects_stamina": false, + "condition_effects_learning": false, + "condition_effects_memory": false, + "condition_effects_mental_health": false, + "condition_effects_social_or_behavioral": false, + "condition_effects_other": false, + "condition_effects_prefer_not_to_say": false, + "reasonable_preference_reason_homeless": false, + "reasonable_preference_reason_unsatisfactory_housing": false, + "reasonable_preference_reason_medical_grounds": false, + "reasonable_preference_reason_avoid_hardship": false, + "reasonable_preference_reason_do_not_know": true + } + } diff --git a/spec/models/case_log_spec.rb b/spec/models/case_log_spec.rb index 08b41d6c2..86f4e9d9b 100644 --- a/spec/models/case_log_spec.rb +++ b/spec/models/case_log_spec.rb @@ -1,4 +1,17 @@ require "rails_helper" RSpec.describe Form, type: :model do + describe "#new" do + it "validates age is a number" do + expect { CaseLog.create!(tenant_age: "random") }.to raise_error(ActiveRecord::RecordInvalid) + end + + it "validates age is under 120" do + expect { CaseLog.create!(tenant_age: 121) }.to raise_error(ActiveRecord::RecordInvalid) + end + + it "validates age is over 0" do + expect { CaseLog.create!(tenant_age: 0) }.to raise_error(ActiveRecord::RecordInvalid) + end + end end diff --git a/spec/requests/case_log_controller_spec.rb b/spec/requests/case_log_controller_spec.rb index 54ba319bf..cf874bb86 100644 --- a/spec/requests/case_log_controller_spec.rb +++ b/spec/requests/case_log_controller_spec.rb @@ -11,6 +11,8 @@ RSpec.describe CaseLogsController, type: :request do ActionController::HttpAuthentication::Basic .encode_credentials(api_username, api_password) end + let(:in_progress) { "in progress" } + let(:submitted) { "submitted" } let(:headers) do { @@ -51,6 +53,34 @@ RSpec.describe CaseLogsController, type: :request do expect(json_response["property_postcode"]).to eq(property_postcode) end + context "invalid json params" do + let(:tenant_age) { 2000 } + + it "validates case log parameters" do + json_response = JSON.parse(response.body) + expect(response).to have_http_status(:unprocessable_entity) + expect(json_response["errors"]).to eq(["Tenant age must be between 0 and 120"]) + end + end + + context "partial case log submission" do + it "marks the record as in_progress" do + json_response = JSON.parse(response.body) + expect(json_response["status"]).to eq(in_progress) + end + end + + context "complete case log submission" do + let(:params) do + JSON.parse(File.open("spec/fixtures/complete_case_log.json").read) + end + + it "marks the record as submitted" do + json_response = JSON.parse(response.body) + expect(json_response["status"]).to eq(submitted) + end + end + context "request with invalid credentials" do let(:basic_credentials) do ActionController::HttpAuthentication::Basic.encode_credentials(api_username, "Oops")