From 1733a3bd1f30e4292bd881bb40bce56a50ce610c Mon Sep 17 00:00:00 2001 From: Daniel Baark <5101747+baarkerlounger@users.noreply.github.com> Date: Fri, 15 Oct 2021 10:43:37 +0100 Subject: [PATCH 01/23] Api update (#48) * Add PUT/PATCH/DELETE case log to API --- Gemfile | 4 + Gemfile.lock | 20 +- app/controllers/case_logs_controller.rb | 44 ++++- app/helpers/tasklist_helper.rb | 6 +- app/models/case_log.rb | 42 ++++- app/views/case_logs/index.html.erb | 6 +- app/views/form/_check_answers_table.html.erb | 4 +- app/views/form/check_answers.html.erb | 6 +- ...014154616_add_discarded_at_to_case_logs.rb | 6 + db/schema.rb | 4 +- spec/factories/case_log.rb | 6 +- spec/helpers/tasklist_helper_spec.rb | 6 +- spec/requests/case_log_controller_spec.rb | 171 +++++++++++++++--- spec/views/case_log_index_view_spec.rb | 16 +- 14 files changed, 269 insertions(+), 72 deletions(-) create mode 100644 db/migrate/20211014154616_add_discarded_at_to_case_logs.rb diff --git a/Gemfile b/Gemfile index f28f6dee0..3039202f0 100644 --- a/Gemfile +++ b/Gemfile @@ -19,8 +19,12 @@ gem "jbuilder", "~> 2.7" gem "bootsnap", ">= 1.4.4", require: false # Gov.UK frontend components gem "govuk-components" +# Gov.UK component form builder DSL gem "govuk_design_system_formbuilder" +# Turbo & Stimulus gem "hotwire-rails" +# Soft delete ActiveRecords objects +gem "discard" group :development, :test do # Call 'byebug' anywhere in the code to stop execution and get a debugger console diff --git a/Gemfile.lock b/Gemfile.lock index 328d2a9ef..176bb4202 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -123,12 +123,14 @@ GEM rack-test (>= 0.6.3) regexp_parser (>= 1.5, < 3.0) xpath (~> 3.2) - childprocess (3.0.0) + childprocess (4.1.0) coderay (1.1.3) concurrent-ruby (1.1.9) crass (1.0.6) deep_merge (1.2.1) diff-lcs (1.4.4) + discard (1.2.0) + activerecord (>= 4.2, < 7) docile (1.4.0) dotenv (2.7.6) dotenv-rails (2.7.6) @@ -143,7 +145,7 @@ GEM ffi (1.15.4) globalid (0.5.2) activesupport (>= 5.0) - govuk-components (2.1.2) + govuk-components (2.1.3) activemodel (>= 6.0) railties (>= 6.0) view_component (~> 2.39.0) @@ -171,7 +173,7 @@ GEM mini_mime (>= 0.1.1) marcel (1.0.2) method_source (1.0.0) - mini_mime (1.1.1) + mini_mime (1.1.2) minitest (5.14.4) msgpack (1.4.2) nio4r (2.5.8) @@ -194,7 +196,7 @@ GEM byebug (~> 11.0) pry (~> 0.13.0) public_suffix (4.0.6) - puma (5.5.0) + puma (5.5.2) nio4r (~> 2.0) racc (1.5.2) rack (2.2.3) @@ -277,8 +279,9 @@ GEM sass (~> 3.5, >= 3.5.5) scss_lint-govuk (0.2.0) scss_lint - selenium-webdriver (3.142.7) - childprocess (>= 0.5, < 4.0) + selenium-webdriver (4.0.0) + childprocess (>= 0.5, < 5.0) + rexml (~> 3.2, >= 3.2.5) rubyzip (>= 1.2.2) semantic_range (3.0.0) simplecov (0.21.2) @@ -294,10 +297,10 @@ GEM actionpack (>= 4.0) activesupport (>= 4.0) sprockets (>= 3.0.0) - stimulus-rails (0.7.0) + stimulus-rails (0.7.1) rails (>= 6.0.0) thor (1.1.0) - turbo-rails (0.8.1) + turbo-rails (7.1.0) rails (>= 6.0.0) tzinfo (2.0.4) concurrent-ruby (~> 1.0) @@ -331,6 +334,7 @@ DEPENDENCIES bootsnap (>= 1.4.4) byebug capybara + discard dotenv-rails factory_bot_rails govuk-components diff --git a/app/controllers/case_logs_controller.rb b/app/controllers/case_logs_controller.rb index ef73bdd7c..20cc5c64f 100644 --- a/app/controllers/case_logs_controller.rb +++ b/app/controllers/case_logs_controller.rb @@ -1,17 +1,17 @@ class CaseLogsController < ApplicationController - skip_before_action :verify_authenticity_token, if: :json_create_request? - before_action :authenticate, if: :json_create_request? + skip_before_action :verify_authenticity_token, if: :json_api_request? + before_action :authenticate, if: :json_api_request? # rubocop:disable Style/ClassVars @@form_handler = FormHandler.instance # rubocop:enable Style/ClassVars def index - @submitted_case_logs = CaseLog.where(status: 1) - @in_progress_case_logs = CaseLog.where(status: 0) + @completed_case_logs = CaseLog.where(status: 2) + @in_progress_case_logs = CaseLog.where(status: 1) end def create - case_log = CaseLog.create(create_params) + case_log = CaseLog.create(api_case_log_params) respond_to do |format| format.html { redirect_to case_log } format.json do @@ -24,6 +24,18 @@ class CaseLogsController < ApplicationController end end + def update + if case_log = CaseLog.find_by(id: params[:id]) + if case_log.update(api_case_log_params) + render json: case_log, status: :ok + else + render json: { errors: case_log.errors.full_messages }, status: :unprocessable_entity + end + else + render json: { error: "Case Log #{params[:id]} not found" }, status: :not_found + end + end + # We don't have a dedicated non-editable show view def show edit @@ -51,12 +63,24 @@ class CaseLogsController < ApplicationController end end + def destroy + if case_log = CaseLog.find_by(id: params[:id]) + if case_log.discard + head :no_content + else + render json: { errors: case_log.errors.full_messages }, status: :unprocessable_entity + end + else + render json: { error: "Case Log #{params[:id]} not found" }, status: :not_found + end + end + def check_answers form = @@form_handler.get_form("2021_2022") @case_log = CaseLog.find(params[:case_log_id]) current_url = request.env["PATH_INFO"] subsection = current_url.split("/")[-2] - render "form/check_answers", locals: { case_log: @case_log, subsection: subsection, form: form } + render "form/check_answers", locals: { subsection: subsection, form: form } end form = @@form_handler.get_form("2021_2022") @@ -69,6 +93,8 @@ class CaseLogsController < ApplicationController private + API_ACTIONS = %w[create update destroy].freeze + def question_responses(questions_for_page) questions_for_page.each_with_object({}) do |(question_key, question_info), result| question_params = params["case_log"][question_key] @@ -83,15 +109,15 @@ private end end - def json_create_request? - (request["action"] == "create") && request.format.json? + def json_api_request? + API_ACTIONS.include?(request["action"]) && request.format.json? end def authenticate http_basic_authenticate_or_request_with name: ENV["API_USER"], password: ENV["API_KEY"] end - def create_params + def api_case_log_params return {} unless params[:case_log] params.require(:case_log).permit(CaseLog.editable_fields) diff --git a/app/helpers/tasklist_helper.rb b/app/helpers/tasklist_helper.rb index 549ef2d22..3da3ddc52 100644 --- a/app/helpers/tasklist_helper.rb +++ b/app/helpers/tasklist_helper.rb @@ -15,7 +15,7 @@ module TasklistHelper def get_subsection_status(subsection_name, case_log, questions) if subsection_name == "declaration" - return all_questions_completed(case_log) ? :not_started : :cannot_start_yet + return case_log.completed? ? :not_started : :cannot_start_yet end return :not_started if questions.all? { |question| case_log[question].blank? } @@ -47,10 +47,6 @@ module TasklistHelper private - def all_questions_completed(case_log) - case_log.attributes.all? { |_question, answer| answer.present? } - end - def is_incomplete?(subsection, case_log, questions) status = get_subsection_status(subsection, case_log, questions) %i[not_started in_progress].include?(status) diff --git a/app/models/case_log.rb b/app/models/case_log.rb index bcbbc432a..efcf09f92 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -26,29 +26,59 @@ class CaseLogValidator < ActiveModel::Validator end class CaseLog < ApplicationRecord + include Discard::Model + default_scope -> { kept } + validate :instance_validations before_save :update_status! attr_writer :previous_page - enum status: { "in progress" => 0, "submitted" => 1 } + enum status: { "not_started" => 0, "in_progress" => 1, "completed" => 2 } - AUTOGENERATED_FIELDS = %w[status created_at updated_at id].freeze + AUTOGENERATED_FIELDS = %w[id status created_at updated_at discarded_at].freeze def instance_validations validates_with CaseLogValidator, ({ previous_page: @previous_page } || {}) end + def self.editable_fields + attribute_names - AUTOGENERATED_FIELDS + end + + def completed? + status == "completed" + end + + def not_started? + status == "not started" + end + + def in_progress? + status == "in progress" + end + + private + def update_status! - self.status = (all_fields_completed? && errors.empty? ? "submitted" : "in progress") + self.status = if all_fields_completed? && errors.empty? + "completed" + elsif all_fields_nil? + "not_started" + else + "in_progress" + end 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 + def all_fields_nil? + mandatory_fields.all? { |_key, val| val.nil? } + end + + def mandatory_fields + attributes.except(*AUTOGENERATED_FIELDS) end end diff --git a/app/views/case_logs/index.html.erb b/app/views/case_logs/index.html.erb index 35ae34399..7352c304c 100644 --- a/app/views/case_logs/index.html.erb +++ b/app/views/case_logs/index.html.erb @@ -10,10 +10,10 @@ <%= render partial: "log_list", locals: { case_logs: @in_progress_case_logs, title: "Logs you need to complete", date_title: "Last Changed" } %> <% end %> - <% if @submitted_case_logs.present? %> - <%= render partial: "log_list", locals: { case_logs: @submitted_case_logs, title: "Logs you've submitted", date_title: "Date Submitted" } %> + <% if @completed_case_logs.present? %> + <%= render partial: "log_list", locals: { case_logs: @completed_case_logs, title: "Logs you've submitted", date_title: "Date Submitted" } %> <% end %> -

See all completed logs (<%= @submitted_case_logs.count %>)

+

See all completed logs (<%= @completed_case_logs.count %>)

diff --git a/app/views/form/_check_answers_table.html.erb b/app/views/form/_check_answers_table.html.erb index 88567bc9c..3c5fb2d2c 100644 --- a/app/views/form/_check_answers_table.html.erb +++ b/app/views/form/_check_answers_table.html.erb @@ -4,10 +4,10 @@ <%= question_info["check_answer_label"].to_s %>
- <%= case_log[question_title] %> + <%= @case_log[question_title] %>
- <%= create_update_answer_link(case_log[question_title], case_log["id"], page)%> + <%= create_update_answer_link(@case_log[question_title], @case_log.id, page)%>
diff --git a/app/views/form/check_answers.html.erb b/app/views/form/check_answers.html.erb index 1ab567e44..f5b0fe93e 100644 --- a/app/views/form/check_answers.html.erb +++ b/app/views/form/check_answers.html.erb @@ -2,11 +2,11 @@

Check the answers you gave for <%= subsection.humanize(capitalize: false) %>

- <%= display_answered_questions_summary(subsection, case_log, form) %> + <%= display_answered_questions_summary(subsection, @case_log, form) %> <% form.pages_for_subsection(subsection).each do |page, page_info| %> <% page_info["questions"].each do |question_title, question_info| %> - <% if total_questions(subsection, case_log, form).include?(question_title) %> - <%= render partial: 'form/check_answers_table', locals: { question_title: question_title, question_info: question_info, case_log: case_log, page: page } %> + <% if total_questions(subsection, @case_log, form).include?(question_title) %> + <%= render partial: 'form/check_answers_table', locals: { question_title: question_title, question_info: question_info, case_log: @case_log, page: page } %> <%end %> <%end %> <% end %> diff --git a/db/migrate/20211014154616_add_discarded_at_to_case_logs.rb b/db/migrate/20211014154616_add_discarded_at_to_case_logs.rb new file mode 100644 index 000000000..429640759 --- /dev/null +++ b/db/migrate/20211014154616_add_discarded_at_to_case_logs.rb @@ -0,0 +1,6 @@ +class AddDiscardedAtToCaseLogs < ActiveRecord::Migration[6.1] + def change + add_column :case_logs, :discarded_at, :datetime + add_index :case_logs, :discarded_at + end +end diff --git a/db/schema.rb b/db/schema.rb index 3e73a03e0..fbfb2467e 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_13_113607) do +ActiveRecord::Schema.define(version: 2021_10_14_154616) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -131,6 +131,8 @@ ActiveRecord::Schema.define(version: 2021_10_13_113607) do t.boolean "reasonable_preference_reason_medical_grounds" t.boolean "reasonable_preference_reason_avoid_hardship" t.boolean "reasonable_preference_reason_do_not_know" + t.datetime "discarded_at" + t.index ["discarded_at"], name: "index_case_logs_on_discarded_at" end end diff --git a/spec/factories/case_log.rb b/spec/factories/case_log.rb index a9aa4ab56..044a4cbac 100644 --- a/spec/factories/case_log.rb +++ b/spec/factories/case_log.rb @@ -2,14 +2,14 @@ FactoryBot.define do factory :case_log do sequence(:id) { |i| i } trait :in_progress do - status { 0 } + status { 1 } tenant_code { "TH356" } property_postcode { "SW2 6HI" } previous_postcode { "P0 5ST" } tenant_age { "12" } end - trait :submitted do - status { 1 } + trait :completed do + status { 2 } tenant_code { "BZ737" } property_postcode { "NW1 7TY" } end diff --git a/spec/helpers/tasklist_helper_spec.rb b/spec/helpers/tasklist_helper_spec.rb index 28860962f..e2ced980f 100644 --- a/spec/helpers/tasklist_helper_spec.rb +++ b/spec/helpers/tasklist_helper_spec.rb @@ -1,8 +1,9 @@ require "rails_helper" RSpec.describe TasklistHelper do - let!(:empty_case_log) { FactoryBot.create(:case_log) } - let!(:case_log) { FactoryBot.create(:case_log, :in_progress) } + let(:empty_case_log) { FactoryBot.build(:case_log) } + let(:case_log) { FactoryBot.build(:case_log, :in_progress) } + let(:completed_case_log) { FactoryBot.build(:case_log, :completed) } form_handler = FormHandler.instance let(:form) { form_handler.get_form("test_form") } @@ -35,7 +36,6 @@ RSpec.describe TasklistHelper do end it "returns not started if the subsection is declaration and all the questions are completed" do - completed_case_log = CaseLog.new(case_log.attributes.map { |key, value| Hash[key, value || "value"] }.reduce(:merge)) status = get_subsection_status("declaration", completed_case_log, declaration_questions) expect(status).to eq(:not_started) end diff --git a/spec/requests/case_log_controller_spec.rb b/spec/requests/case_log_controller_spec.rb index cf874bb86..0cdf5d791 100644 --- a/spec/requests/case_log_controller_spec.rb +++ b/spec/requests/case_log_controller_spec.rb @@ -1,26 +1,33 @@ require "rails_helper" RSpec.describe CaseLogsController, type: :request do + let(:api_username) { "test_user" } + let(:api_password) { "test_password" } + let(:basic_credentials) do + ActionController::HttpAuthentication::Basic + .encode_credentials(api_username, api_password) + end + + let(:headers) do + { + "Content-Type" => "application/json", + "Accept" => "application/json", + "Authorization" => basic_credentials, + } + end + + before do + allow(ENV).to receive(:[]) + allow(ENV).to receive(:[]).with("API_USER").and_return(api_username) + allow(ENV).to receive(:[]).with("API_KEY").and_return(api_password) + end + describe "POST #create" do let(:tenant_code) { "T365" } let(:tenant_age) { 35 } let(:property_postcode) { "SE11 6TY" } - let(:api_username) { "test_user" } - let(:api_password) { "test_password" } - let(:basic_credentials) do - ActionController::HttpAuthentication::Basic - .encode_credentials(api_username, api_password) - end - let(:in_progress) { "in progress" } - let(:submitted) { "submitted" } - - let(:headers) do - { - "Content-Type" => "application/json", - "Accept" => "application/json", - "Authorization" => basic_credentials, - } - end + let(:in_progress) { "in_progress" } + let(:completed) { "completed" } let(:params) do { @@ -31,9 +38,6 @@ RSpec.describe CaseLogsController, type: :request do end before do - allow(ENV).to receive(:[]) - allow(ENV).to receive(:[]).with("API_USER").and_return(api_username) - allow(ENV).to receive(:[]).with("API_KEY").and_return(api_password) post "/case_logs", headers: headers, params: params.to_json end @@ -75,9 +79,134 @@ RSpec.describe CaseLogsController, type: :request do JSON.parse(File.open("spec/fixtures/complete_case_log.json").read) end - it "marks the record as submitted" do + it "marks the record as completed" do json_response = JSON.parse(response.body) - expect(json_response["status"]).to eq(submitted) + expect(json_response["status"]).to eq(completed) + end + end + + context "request with invalid credentials" do + let(:basic_credentials) do + ActionController::HttpAuthentication::Basic.encode_credentials(api_username, "Oops") + end + + it "returns 401" do + expect(response).to have_http_status(:unauthorized) + end + end + end + + describe "PATCH" do + let(:case_log) do + FactoryBot.create(:case_log, :in_progress, tenant_code: "Old Value", property_postcode: "Old Value") + end + let(:params) do + { tenant_code: "New Value" } + end + let(:id) { case_log.id } + + before do + patch "/case_logs/#{id}", headers: headers, params: params.to_json + end + + it "returns http success" do + expect(response).to have_http_status(:success) + end + + it "updates the case log with the given fields and keeps original values where none are passed" do + case_log.reload + expect(case_log.tenant_code).to eq("New Value") + expect(case_log.property_postcode).to eq("Old Value") + end + + context "invalid case log id" do + let(:id) { (CaseLog.order(:id).last&.id || 0) + 1 } + + it "returns 404" do + expect(response).to have_http_status(:not_found) + end + end + + context "request with invalid credentials" do + let(:basic_credentials) do + ActionController::HttpAuthentication::Basic.encode_credentials(api_username, "Oops") + end + + it "returns 401" do + expect(response).to have_http_status(:unauthorized) + end + end + end + + # We don't really have any meaningful distinction between PUT and PATCH here since you can update some or all + # fields in both cases, and both route to #Update. Rails generally recommends PATCH as it more closely matches + # what actually happens to an ActiveRecord object and what we're doing here, but either is allowed. + describe "PUT" do + let(:case_log) do + FactoryBot.create(:case_log, :in_progress, tenant_code: "Old Value", property_postcode: "Old Value") + end + let(:params) do + { tenant_code: "New Value" } + end + let(:id) { case_log.id } + + before do + put "/case_logs/#{id}", headers: headers, params: params.to_json + end + + it "returns http success" do + expect(response).to have_http_status(:success) + end + + it "updates the case log with the given fields and keeps original values where none are passed" do + case_log.reload + expect(case_log.tenant_code).to eq("New Value") + expect(case_log.property_postcode).to eq("Old Value") + end + + context "invalid case log id" do + let(:id) { (CaseLog.order(:id).last&.id || 0) + 1 } + + it "returns 404" do + expect(response).to have_http_status(:not_found) + end + end + + context "request with invalid credentials" do + let(:basic_credentials) do + ActionController::HttpAuthentication::Basic.encode_credentials(api_username, "Oops") + end + + it "returns 401" do + expect(response).to have_http_status(:unauthorized) + end + end + end + + describe "DELETE" do + let!(:case_log) do + FactoryBot.create(:case_log, :in_progress) + end + let(:id) { case_log.id } + + before do + delete "/case_logs/#{id}", headers: headers + end + + it "returns http success" do + expect(response).to have_http_status(:success) + end + + it "soft deletes the case log" do + expect { CaseLog.find(id) }.to raise_error(ActiveRecord::RecordNotFound) + expect(CaseLog.with_discarded.find(id)).to be_a(CaseLog) + end + + context "invalid case log id" do + let(:id) { (CaseLog.order(:id).last&.id || 0) + 1 } + + it "returns 404" do + expect(response).to have_http_status(:not_found) end end diff --git a/spec/views/case_log_index_view_spec.rb b/spec/views/case_log_index_view_spec.rb index b97347cb6..cc826e3c8 100644 --- a/spec/views/case_log_index_view_spec.rb +++ b/spec/views/case_log_index_view_spec.rb @@ -1,12 +1,12 @@ require "rails_helper" RSpec.describe "case_logs/index" do let(:in_progress_log) { FactoryBot.build(:case_log, :in_progress) } - let(:submitted_log) { FactoryBot.build(:case_log, :submitted) } + let(:completed_log) { FactoryBot.build(:case_log, :completed) } context "given an in progress log list" do it "renders a table for in progress logs only" do assign(:in_progress_case_logs, [in_progress_log]) - assign(:submitted_case_logs, []) + assign(:completed_case_logs, []) render expect(rendered).to match(//) expect(rendered).to match(/Logs you need to complete/) @@ -16,23 +16,23 @@ RSpec.describe "case_logs/index" do end end - context "given a submitted log list" do + context "given a completed log list" do it "renders a table for in progress logs only" do assign(:in_progress_case_logs, []) - assign(:submitted_case_logs, [submitted_log]) + assign(:completed_case_logs, [completed_log]) render expect(rendered).to match(/
/) expect(rendered).to match(/Logs you've submitted/) expect(rendered).not_to match(/Logs you need to complete/) - expect(rendered).to match(submitted_log.tenant_code) - expect(rendered).to match(submitted_log.property_postcode) + expect(rendered).to match(completed_log.tenant_code) + expect(rendered).to match(completed_log.property_postcode) end end - context "given a submitted log list and an in_progress log list" do + context "given a completed log list and an in_progress log list" do it "renders two tables, one for each status" do assign(:in_progress_case_logs, [in_progress_log]) - assign(:submitted_case_logs, [submitted_log]) + assign(:completed_case_logs, [completed_log]) render expect(rendered).to match(/
/) expect(rendered).to match(/Logs you've submitted/) From 882a1de227a04607043d42d6dbb6b121355c09e6 Mon Sep 17 00:00:00 2001 From: baarkerlounger Date: Fri, 15 Oct 2021 11:52:50 +0100 Subject: [PATCH 02/23] Test case log state machine --- app/models/case_log.rb | 4 ++-- spec/models/case_log_spec.rb | 17 +++++++++++++++++ spec/requests/case_log_controller_spec.rb | 13 +++++++++++++ 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/app/models/case_log.rb b/app/models/case_log.rb index efcf09f92..30a2b3980 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -51,11 +51,11 @@ class CaseLog < ApplicationRecord end def not_started? - status == "not started" + status == "not_started" end def in_progress? - status == "in progress" + status == "in_progress" end private diff --git a/spec/models/case_log_spec.rb b/spec/models/case_log_spec.rb index 86f4e9d9b..b456e6988 100644 --- a/spec/models/case_log_spec.rb +++ b/spec/models/case_log_spec.rb @@ -14,4 +14,21 @@ RSpec.describe Form, type: :model do expect { CaseLog.create!(tenant_age: 0) }.to raise_error(ActiveRecord::RecordInvalid) end end + + describe "status" do + let!(:empty_case_log) { FactoryBot.create(:case_log) } + let!(:in_progress_case_log) { FactoryBot.create(:case_log, :in_progress) } + + it "is set to not started for an empty case log" do + expect(empty_case_log.not_started?).to be(true) + expect(empty_case_log.in_progress?).to be(false) + expect(empty_case_log.completed?).to be(false) + end + + it "is set to not started for an empty case log" do + expect(in_progress_case_log.in_progress?).to be(true) + expect(in_progress_case_log.not_started?).to be(false) + expect(in_progress_case_log.completed?).to be(false) + end + end end diff --git a/spec/requests/case_log_controller_spec.rb b/spec/requests/case_log_controller_spec.rb index 0cdf5d791..a1c552a52 100644 --- a/spec/requests/case_log_controller_spec.rb +++ b/spec/requests/case_log_controller_spec.rb @@ -127,6 +127,19 @@ RSpec.describe CaseLogsController, type: :request do end end + context "invalid case log params" do + let(:params) { {tenant_age: 200} } + + it "returns 422" do + expect(response).to have_http_status(:unprocessable_entity) + end + + it "returns an error message" do + json_response = JSON.parse(response.body) + expect(json_response["errors"]).to eq(["Tenant age must be between 0 and 120"]) + end + end + context "request with invalid credentials" do let(:basic_credentials) do ActionController::HttpAuthentication::Basic.encode_credentials(api_username, "Oops") From 5a7183f450df5046bf1663488bc662e02cfdc36e Mon Sep 17 00:00:00 2001 From: Matthew Phelan Date: Fri, 15 Oct 2021 13:18:57 +0100 Subject: [PATCH 03/23] CLCD-495 ready for review --- app/models/case_log.rb | 16 +++++++++++----- ...update_property_number_of_times_relet_type.rb | 5 +++++ db/schema.rb | 4 ++-- spec/requests/case_log_controller_spec.rb | 5 ++++- 4 files changed, 22 insertions(+), 8 deletions(-) create mode 100644 db/migrate/20211015090040_update_property_number_of_times_relet_type.rb diff --git a/app/models/case_log.rb b/app/models/case_log.rb index bcbbc432a..167925e8e 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -1,7 +1,7 @@ class CaseLogValidator < ActiveModel::Validator - # Methods need to be named 'validate_' followed by field name - # this is how the metaprogramming of the method name being - # call in the validate method works. + # Methods to be used on save and continue need to be named 'validate_' + # followed by field name this is how the metaprogramming of the method + # name being call in the validate method works. def validate_tenant_age(record) if record.tenant_age && !/^[1-9][0-9]?$|^120$/.match?(record.tenant_age.to_s) @@ -9,13 +9,19 @@ class CaseLogValidator < ActiveModel::Validator end end + def validate_property_number_of_times_relet(record) + if record.property_number_of_times_relet && !/^[1-9]$|^0[1-9]$|^1[0-9]$|^20$/.match?(record.property_number_of_times_relet.to_s) + record.errors.add :property_number_of_times_relet, "must be between 0 and 20" + 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 question_to_validate && respond_to?("validate_#{question_to_validate}") - public_send("validate_#{question_to_validate}", record) + 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 @@ -30,7 +36,7 @@ class CaseLog < ApplicationRecord 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 diff --git a/db/migrate/20211015090040_update_property_number_of_times_relet_type.rb b/db/migrate/20211015090040_update_property_number_of_times_relet_type.rb new file mode 100644 index 000000000..4e436e2d2 --- /dev/null +++ b/db/migrate/20211015090040_update_property_number_of_times_relet_type.rb @@ -0,0 +1,5 @@ +class UpdatePropertyNumberOfTimesReletType < ActiveRecord::Migration[6.1] + def change + change_column :case_logs, :property_number_of_times_relet, :integer + end +end diff --git a/db/schema.rb b/db/schema.rb index 3e73a03e0..7202cd339 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_13_113607) do +ActiveRecord::Schema.define(version: 2021_10_15_090040) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -84,7 +84,6 @@ ActiveRecord::Schema.define(version: 2021_10_13_113607) do t.string "property_void_date" t.string "property_major_repairs" t.string "property_major_repairs_date" - t.string "property_number_of_times_relet" t.string "property_wheelchair_accessible" t.string "net_income" t.string "net_income_frequency" @@ -131,6 +130,7 @@ ActiveRecord::Schema.define(version: 2021_10_13_113607) do t.boolean "reasonable_preference_reason_medical_grounds" t.boolean "reasonable_preference_reason_avoid_hardship" t.boolean "reasonable_preference_reason_do_not_know" + t.integer "property_number_of_times_relet" end end diff --git a/spec/requests/case_log_controller_spec.rb b/spec/requests/case_log_controller_spec.rb index cf874bb86..b4429c4dc 100644 --- a/spec/requests/case_log_controller_spec.rb +++ b/spec/requests/case_log_controller_spec.rb @@ -4,6 +4,7 @@ RSpec.describe CaseLogsController, type: :request do describe "POST #create" do let(:tenant_code) { "T365" } let(:tenant_age) { 35 } + let(:property_number_of_times_relet) { 12 } let(:property_postcode) { "SE11 6TY" } let(:api_username) { "test_user" } let(:api_password) { "test_password" } @@ -27,6 +28,7 @@ RSpec.describe CaseLogsController, type: :request do "tenant_code": tenant_code, "tenant_age": tenant_age, "property_postcode": property_postcode, + "property_number_of_times_relet": property_number_of_times_relet } end @@ -55,11 +57,12 @@ RSpec.describe CaseLogsController, type: :request do context "invalid json params" do let(:tenant_age) { 2000 } + let(:property_number_of_times_relet) { 21 } 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"]) + expect(json_response["errors"]).to match_array(["Tenant age must be between 0 and 120", "Property number of times relet must be between 0 and 20"]) end end From 9de3d9dcecf1ba8b7c4aa19f921fbe78e6bcb872 Mon Sep 17 00:00:00 2001 From: Matthew Phelan Date: Fri, 15 Oct 2021 13:25:14 +0100 Subject: [PATCH 04/23] finishing touches --- app/models/case_log.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/case_log.rb b/app/models/case_log.rb index 167925e8e..876e42766 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -21,7 +21,7 @@ class CaseLogValidator < ActiveModel::Validator # we want to validate all data fields. question_to_validate = options[:previous_page] if question_to_validate && respond_to?("validate_#{question_to_validate}") - public_send("validate_#{question_to_validate}", record) + 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 From 5248a96db3bf6fc6fe9d43b658a48f54f0ee3a34 Mon Sep 17 00:00:00 2001 From: baarkerlounger Date: Fri, 15 Oct 2021 14:44:24 +0100 Subject: [PATCH 05/23] Linting --- app/controllers/case_logs_controller.rb | 4 ++-- app/models/case_log.rb | 14 +++++++------- app/models/form_handler.rb | 2 +- spec/requests/case_log_controller_spec.rb | 2 +- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/app/controllers/case_logs_controller.rb b/app/controllers/case_logs_controller.rb index 20cc5c64f..cd99837e6 100644 --- a/app/controllers/case_logs_controller.rb +++ b/app/controllers/case_logs_controller.rb @@ -25,7 +25,7 @@ class CaseLogsController < ApplicationController end def update - if case_log = CaseLog.find_by(id: params[:id]) + if (case_log = CaseLog.find_by(id: params[:id])) if case_log.update(api_case_log_params) render json: case_log, status: :ok else @@ -64,7 +64,7 @@ class CaseLogsController < ApplicationController end def destroy - if case_log = CaseLog.find_by(id: params[:id]) + if (case_log = CaseLog.find_by(id: params[:id])) if case_log.discard head :no_content else diff --git a/app/models/case_log.rb b/app/models/case_log.rb index 30a2b3980..64c1cdb95 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -58,16 +58,16 @@ class CaseLog < ApplicationRecord status == "in_progress" end - private +private def update_status! self.status = if all_fields_completed? && errors.empty? - "completed" - elsif all_fields_nil? - "not_started" - else - "in_progress" - end + "completed" + elsif all_fields_nil? + "not_started" + else + "in_progress" + end end def all_fields_completed? diff --git a/app/models/form_handler.rb b/app/models/form_handler.rb index c6cd755d4..4e8bee80b 100644 --- a/app/models/form_handler.rb +++ b/app/models/form_handler.rb @@ -12,8 +12,8 @@ class FormHandler @forms[form] ||= Form.new(form) end +private - private def get_all_forms forms = {} directories = ["config/forms", "spec/fixtures/forms"] diff --git a/spec/requests/case_log_controller_spec.rb b/spec/requests/case_log_controller_spec.rb index a1c552a52..e02329f04 100644 --- a/spec/requests/case_log_controller_spec.rb +++ b/spec/requests/case_log_controller_spec.rb @@ -128,7 +128,7 @@ RSpec.describe CaseLogsController, type: :request do end context "invalid case log params" do - let(:params) { {tenant_age: 200} } + let(:params) { { tenant_age: 200 } } it "returns 422" do expect(response).to have_http_status(:unprocessable_entity) From ad7050e2059d4c66a0d63c641a45e4211e852c28 Mon Sep 17 00:00:00 2001 From: baarkerlounger Date: Fri, 15 Oct 2021 14:52:17 +0100 Subject: [PATCH 06/23] Remove the need for disabling cops --- app/controllers/case_logs_controller.rb | 11 ++++------- app/helpers/check_answers_helper.rb | 4 +--- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/app/controllers/case_logs_controller.rb b/app/controllers/case_logs_controller.rb index cd99837e6..2a0c6b84a 100644 --- a/app/controllers/case_logs_controller.rb +++ b/app/controllers/case_logs_controller.rb @@ -1,9 +1,6 @@ class CaseLogsController < ApplicationController skip_before_action :verify_authenticity_token, if: :json_api_request? before_action :authenticate, if: :json_api_request? - # rubocop:disable Style/ClassVars - @@form_handler = FormHandler.instance - # rubocop:enable Style/ClassVars def index @completed_case_logs = CaseLog.where(status: 2) @@ -42,13 +39,13 @@ class CaseLogsController < ApplicationController end def edit - @form = @@form_handler.get_form("2021_2022") + @form = FormHandler.instance.get_form("2021_2022") @case_log = CaseLog.find(params[:id]) render :edit end def submit_form - form = @@form_handler.get_form("2021_2022") + form = FormHandler.instance.get_form("2021_2022") @case_log = CaseLog.find(params[:id]) previous_page = params[:case_log][:previous_page] questions_for_page = form.questions_for_page(previous_page) @@ -76,14 +73,14 @@ class CaseLogsController < ApplicationController end def check_answers - form = @@form_handler.get_form("2021_2022") + form = FormHandler.instance.get_form("2021_2022") @case_log = CaseLog.find(params[:case_log_id]) current_url = request.env["PATH_INFO"] subsection = current_url.split("/")[-2] render "form/check_answers", locals: { subsection: subsection, form: form } end - form = @@form_handler.get_form("2021_2022") + form = FormHandler.instance.get_form("2021_2022") form.all_pages.map do |page_key, page_info| define_method(page_key) do |_errors = {}| @case_log = CaseLog.find(params[:case_log_id]) diff --git a/app/helpers/check_answers_helper.rb b/app/helpers/check_answers_helper.rb index 26cd34b0d..c86d10adb 100644 --- a/app/helpers/check_answers_helper.rb +++ b/app/helpers/check_answers_helper.rb @@ -25,9 +25,7 @@ module CheckAnswersHelper def condition_not_met(case_log, question_key, question, condition) case question["type"] when "numeric" - # rubocop:disable Security/Eval - case_log[question_key].blank? || !eval(case_log[question_key].to_s + condition) - # rubocop:enable Security/Eval + case_log[question_key].blank? || !case_log[question_key].send(condition[0].to_sym, condition[1].to_i) when "radio" case_log[question_key].blank? || !condition.include?(case_log[question_key]) else From f211373ffafcce1923421fe5bc10e033bef8a356 Mon Sep 17 00:00:00 2001 From: baarkerlounger Date: Fri, 15 Oct 2021 14:57:40 +0100 Subject: [PATCH 07/23] Use scopes for bulk status lookups --- app/controllers/case_logs_controller.rb | 4 ++-- app/models/case_log.rb | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/app/controllers/case_logs_controller.rb b/app/controllers/case_logs_controller.rb index 2a0c6b84a..f3753058e 100644 --- a/app/controllers/case_logs_controller.rb +++ b/app/controllers/case_logs_controller.rb @@ -3,8 +3,8 @@ class CaseLogsController < ApplicationController before_action :authenticate, if: :json_api_request? def index - @completed_case_logs = CaseLog.where(status: 2) - @in_progress_case_logs = CaseLog.where(status: 1) + @completed_case_logs = CaseLog.completed + @in_progress_case_logs = CaseLog.in_progress end def create diff --git a/app/models/case_log.rb b/app/models/case_log.rb index 64c1cdb95..42a020fd4 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -28,6 +28,8 @@ end class CaseLog < ApplicationRecord include Discard::Model default_scope -> { kept } + scope :in_progress, -> { where(status: "in_progress") } + scope :completed, -> { where(status: "completed") } validate :instance_validations before_save :update_status! From 6cfdb9a0b7f5dadf470144b84ce2bac8954a85e2 Mon Sep 17 00:00:00 2001 From: Matthew Phelan Date: Fri, 15 Oct 2021 15:03:40 +0100 Subject: [PATCH 08/23] number of times relet validation unit tests --- spec/models/case_log_spec.rb | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/spec/models/case_log_spec.rb b/spec/models/case_log_spec.rb index b456e6988..6b0805250 100644 --- a/spec/models/case_log_spec.rb +++ b/spec/models/case_log_spec.rb @@ -13,6 +13,18 @@ RSpec.describe Form, type: :model do it "validates age is over 0" do expect { CaseLog.create!(tenant_age: 0) }.to raise_error(ActiveRecord::RecordInvalid) end + + it "validates number of relets is a number" do + expect { CaseLog.create!(property_number_of_times_relet: "random") }.to raise_error(ActiveRecord::RecordInvalid) + end + + it "validates number of relets is under 20" do + expect { CaseLog.create!(property_number_of_times_relet: 21) }.to raise_error(ActiveRecord::RecordInvalid) + end + + it "validates number of relets is over 0" do + expect { CaseLog.create!(property_number_of_times_relet: 0) }.to raise_error(ActiveRecord::RecordInvalid) + end end describe "status" do From 9eb067e60861726327ab56c4a9fef9a904f0c889 Mon Sep 17 00:00:00 2001 From: baarkerlounger Date: Fri, 15 Oct 2021 15:29:17 +0100 Subject: [PATCH 09/23] Fix some doc typos --- doc/adr/adr-007-data-validations.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/adr/adr-007-data-validations.md b/doc/adr/adr-007-data-validations.md index 24b8f539c..1241314f9 100644 --- a/doc/adr/adr-007-data-validations.md +++ b/doc/adr/adr-007-data-validations.md @@ -9,7 +9,7 @@ These are handled slightly differently: ##### Validity checks -These run for all submitted data. Every time a form page (in the UI is submitted), the fields related to that form page will be checked to ensure that any responses given are valid. If they are not, an error message will be shown on screen, and it will not be possible to "Save and continue" until the response is fixed or removed. +These run for all submitted data. Every time a form page (in the UI) is submitted, the fields related to that form page will be checked to ensure that any responses given are valid. If they are not, an error message will be shown on screen, and it will not be possible to "Save and continue" until the response is fixed or removed. Similarly if an API request is made to create a case log with data that contains _invalid_ fields, that data will be rejected, and an error message will be returned. @@ -18,10 +18,10 @@ Similarly if an API request is made to create a case log with data that contains These are not strictly error checks since it's possible to submit partial data. In the form UI it is possible to click "Save and continue" and move past questions that you might not know right now, and leave them to come back to later. We shouldn't prevent this workflow. -Similar the API client (3rd party software system) may not have all the required data and may only be submitting a partial log. This is still a valid use case so we should not be enforcing presence checks and returning errors based on them for either submission type. +Similarly the API client (3rd party software system) may not have all the required data and may only be submitting a partial log. This is still a valid use case so we should not be enforcing presence checks and returning errors based on them for either submission type. Instead we determine the _status_ of the case log based the presence checks. Every time data is submitted (via a form page, bulk upload or API), before saving the data, the system will check whether all fields have been completed *and* pass validity checks. If so, the case log will be marked as *completed*, if not it will be marked as *in progress*. By default all fields that a Case Log has will be assumed to be required unless explicitly marked as not required (for example as a result of other answers rendering a question inapplicable). -On the form UI this will work by by not allowing you to "submit" the form, until all presence checks have been satisfied, but all other navigation is allowed. On the API this will work by returning a Case Log that is "in progress" if you've submitted a partial log, or "completed" if you've submitted a full log, or "Errors" if you've submitted an invalid log. +On the form UI this will work by not allowing you to "submit" the form, until all presence checks have been satisfied, but all other navigation is allowed. On the API this will work by returning a Case Log that is "in progress" if you've submitted a partial log, or "completed" if you've submitted a full log, or "Errors" if you've submitted an invalid log. From ae05c05deef6a51e81d9990188be99ab72c7aa29 Mon Sep 17 00:00:00 2001 From: baarkerlounger Date: Mon, 18 Oct 2021 10:08:13 +0100 Subject: [PATCH 10/23] Linting --- app/models/case_log.rb | 2 +- spec/requests/case_log_controller_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/case_log.rb b/app/models/case_log.rb index a6e07adcc..46d766ccc 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -1,5 +1,5 @@ class CaseLogValidator < ActiveModel::Validator - # Methods to be used on save and continue need to be named 'validate_' + # Methods to be used on save and continue need to be named 'validate_' # followed by field name this is how the metaprogramming of the method # name being call in the validate method works. diff --git a/spec/requests/case_log_controller_spec.rb b/spec/requests/case_log_controller_spec.rb index 89e275694..9dab538b0 100644 --- a/spec/requests/case_log_controller_spec.rb +++ b/spec/requests/case_log_controller_spec.rb @@ -35,7 +35,7 @@ RSpec.describe CaseLogsController, type: :request do "tenant_code": tenant_code, "tenant_age": tenant_age, "property_postcode": property_postcode, - "property_number_of_times_relet": property_number_of_times_relet + "property_number_of_times_relet": property_number_of_times_relet, } end From df95c4f1c6b12504aac6ccfa61be863f02075c0c Mon Sep 17 00:00:00 2001 From: baarkerlounger Date: Mon, 18 Oct 2021 10:31:57 +0100 Subject: [PATCH 11/23] Fix column change migration --- ...0211015090040_update_property_number_of_times_relet_type.rb | 3 ++- db/schema.rb | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/db/migrate/20211015090040_update_property_number_of_times_relet_type.rb b/db/migrate/20211015090040_update_property_number_of_times_relet_type.rb index 4e436e2d2..cf4958044 100644 --- a/db/migrate/20211015090040_update_property_number_of_times_relet_type.rb +++ b/db/migrate/20211015090040_update_property_number_of_times_relet_type.rb @@ -1,5 +1,6 @@ class UpdatePropertyNumberOfTimesReletType < ActiveRecord::Migration[6.1] def change - change_column :case_logs, :property_number_of_times_relet, :integer + remove_column :case_logs, :property_number_of_times_relet, :string + add_column :case_logs, :property_number_of_times_relet, :integer end end diff --git a/db/schema.rb b/db/schema.rb index 42b14adc9..03a612ecd 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -130,8 +130,8 @@ ActiveRecord::Schema.define(version: 2021_10_15_090040) do t.boolean "reasonable_preference_reason_medical_grounds" t.boolean "reasonable_preference_reason_avoid_hardship" t.boolean "reasonable_preference_reason_do_not_know" - t.integer "property_number_of_times_relet" t.datetime "discarded_at" + t.integer "property_number_of_times_relet" t.index ["discarded_at"], name: "index_case_logs_on_discarded_at" end From 64bec55985c99bfbbb096156fe18bfc7b9c4ade9 Mon Sep 17 00:00:00 2001 From: baarkerlounger Date: Mon, 18 Oct 2021 10:56:57 +0100 Subject: [PATCH 12/23] Add deployment troubleshooting to readme --- README.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/README.md b/README.md index 20046146f..844ffa88d 100644 --- a/README.md +++ b/README.md @@ -70,6 +70,16 @@ Once the app is deployed: 2. Check logs:\ `cf logs dluhc-core --recent` +#### Troubleshooting deployments + +A failed Github deployment action will occasionally leave a Cloud Foundry deployment in a broken state. As a result all subsequent Github deployment actions will also fail with the message `Cannot update this process while a deployment is in flight`. + +` +cf cancel-deployment dluhc-core +` + +You'd then need to check the logs and fix the issue that caused the initial deployment to fail. + ## CI/CD When a commit is made to `main` the following GitHub action jobs are triggered: From 8ad852454f45bb8a3f56c55cf78ebe85ff8dbad6 Mon Sep 17 00:00:00 2001 From: Matthew Phelan Date: Tue, 19 Oct 2021 10:58:09 +0100 Subject: [PATCH 13/23] Why reasonable preference? validation added --- .../conditional_question_controller.js | 5 ++- app/models/case_log.rb | 24 ++++++++++++-- spec/models/case_log_spec.rb | 33 +++++++++++++++++++ 3 files changed, 59 insertions(+), 3 deletions(-) diff --git a/app/javascript/controllers/conditional_question_controller.js b/app/javascript/controllers/conditional_question_controller.js index c49ba490a..9aa6fddee 100644 --- a/app/javascript/controllers/conditional_question_controller.js +++ b/app/javascript/controllers/conditional_question_controller.js @@ -28,7 +28,10 @@ export default class extends Controller { div.style.display = "block" } else { div.style.display = "none" - let buttons = document.getElementsByName(`case_log[${targetQuestion}]`) + let buttons = document.getElementsByName(`case_log[${targetQuestion}]`); + if (buttons.length == 0){ + buttons = document.getElementsByName(`case_log[${targetQuestion}][]`); + } Object.entries(buttons).forEach(([idx, button]) => { button.checked = false; }) diff --git a/app/models/case_log.rb b/app/models/case_log.rb index 46d766ccc..9e8716569 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -15,13 +15,33 @@ class CaseLogValidator < ActiveModel::Validator end end + def validate_api_reasonable_preference(record) + if record.reasonable_preference == "No" + if record.reasonable_preference_reason_homeless || record.reasonable_preference_reason_unsatisfactory_housing || record.reasonable_preference_reason_medical_grounds || record.reasonable_preference_reason_avoid_hardship || record.reasonable_preference_reason_do_not_know + record.errors.add :reasonable_preference_reason, "- no resasons can be set to true, if reasonable preference is No" + end + end + end + + def validate_reasonable_preference(record) + if record.homelessness == "No" && record.reasonable_preference == "Yes" + record.errors.add :reasonable_preference, "can not be Yes if Not Homesless imediately prior to this letting has been selected" + elsif record.reasonable_preference == "Yes" + if !record.reasonable_preference_reason_homeless && !record.reasonable_preference_reason_unsatisfactory_housing && !record.reasonable_preference_reason_medical_grounds && !record.reasonable_preference_reason_avoid_hardship && !record.reasonable_preference_reason_do_not_know + record.errors.add :reasonable_preference_reason, "- if reasonable preference is Yes, a reason must be given" + end + 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 question_to_validate && respond_to?("validate_#{question_to_validate}") - public_send("validate_#{question_to_validate}", record) + if question_to_validate + if respond_to?("validate_#{question_to_validate}") + public_send("validate_#{question_to_validate}", record) + end else # This assumes that all methods in this class other than this one are # validations to be run diff --git a/spec/models/case_log_spec.rb b/spec/models/case_log_spec.rb index 6b0805250..236ddf7dc 100644 --- a/spec/models/case_log_spec.rb +++ b/spec/models/case_log_spec.rb @@ -25,6 +25,39 @@ RSpec.describe Form, type: :model do it "validates number of relets is over 0" do expect { CaseLog.create!(property_number_of_times_relet: 0) }.to raise_error(ActiveRecord::RecordInvalid) end + + describe "reasonable preference validation" do + it "if given reasonable preference is yes a reason must be selected" do + expect { + CaseLog.create!(reasonable_preference: "Yes", + reasonable_preference_reason_homeless: nil, + reasonable_preference_reason_unsatisfactory_housing: nil, + reasonable_preference_reason_medical_grounds: nil, + reasonable_preference_reason_avoid_hardship: nil, + reasonable_preference_reason_do_not_know: nil + ) + }.to raise_error(ActiveRecord::RecordInvalid) + end + + it "if not previously homesless reasonable preference should not be selected" do + expect { + CaseLog.create!( + homelessness: "No", + reasonable_preference: "Yes" + ) + }.to raise_error(ActiveRecord::RecordInvalid) + end + + it "if not given reasonable preference a reason should not be selected" do + expect { + CaseLog.create!( + homelessness: "Yes", + reasonable_preference: "No", + reasonable_preference_reason_homeless: true + ) + }.to raise_error(ActiveRecord::RecordInvalid) + end + end end describe "status" do From 9d95707e35eaa227e308fb0dd02ba95d6b2a6c3c Mon Sep 17 00:00:00 2001 From: MadeTech Dushan Date: Tue, 19 Oct 2021 15:10:06 +0100 Subject: [PATCH 14/23] add other reason left settled home validation --- app/models/case_log.rb | 10 ++++++++++ spec/models/case_log_spec.rb | 16 ++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/app/models/case_log.rb b/app/models/case_log.rb index 46d766ccc..1f4219d57 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -15,6 +15,16 @@ class CaseLogValidator < ActiveModel::Validator end end + def validate_other_reason_for_leaving_last_settled_home(record) + if record.reason_for_leaving_last_settled_home == "Other" && record.other_reason_for_leaving_last_settled_home.blank? + record.errors.add :other_reason_for_leaving_last_settled_home, "If reason for leaving settled home is other then the other reason must be provided" + end + + if record.reason_for_leaving_last_settled_home != "Other" && record.other_reason_for_leaving_last_settled_home.present? + record.errors.add :other_reason_for_leaving_last_settled_home, "The other reason must not be provided if the reason for leaving settled home was not other" + 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 diff --git a/spec/models/case_log_spec.rb b/spec/models/case_log_spec.rb index 6b0805250..dff400389 100644 --- a/spec/models/case_log_spec.rb +++ b/spec/models/case_log_spec.rb @@ -25,6 +25,22 @@ RSpec.describe Form, type: :model do it "validates number of relets is over 0" do expect { CaseLog.create!(property_number_of_times_relet: 0) }.to raise_error(ActiveRecord::RecordInvalid) end + + context "other reason for leaving last settled home validation" do + it "must be provided if main reason for leaving last settled home was given as other" do + expect { + CaseLog.create!(reason_for_leaving_last_settled_home: "Other", + other_reason_for_leaving_last_settled_home: nil) + }.to raise_error(ActiveRecord::RecordInvalid) + end + + it "must not be provided if the main reason for leaving settled home is not other" do + expect { + CaseLog.create!(reason_for_leaving_last_settled_home: "Repossession", + other_reason_for_leaving_last_settled_home: "the other reason provided") + }.to raise_error(ActiveRecord::RecordInvalid) + end + end end describe "status" do From 6529157ad5070d55c6d72c0fcb69539ffd598514 Mon Sep 17 00:00:00 2001 From: baarkerlounger Date: Tue, 19 Oct 2021 20:12:12 +0100 Subject: [PATCH 15/23] More generic numeric conditions --- app/helpers/check_answers_helper.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/helpers/check_answers_helper.rb b/app/helpers/check_answers_helper.rb index c86d10adb..ae2d72fc2 100644 --- a/app/helpers/check_answers_helper.rb +++ b/app/helpers/check_answers_helper.rb @@ -25,7 +25,9 @@ module CheckAnswersHelper def condition_not_met(case_log, question_key, question, condition) case question["type"] when "numeric" - case_log[question_key].blank? || !case_log[question_key].send(condition[0].to_sym, condition[1].to_i) + operator = condition[/[<>=]+/].to_sym + operand = condition[/\d+/].to_i + case_log[question_key].blank? || !case_log[question_key].send(operator, operand) when "radio" case_log[question_key].blank? || !condition.include?(case_log[question_key]) else From b87f1cbdeed39c955e461e0a2428a69393ebeb6a Mon Sep 17 00:00:00 2001 From: Matthew Phelan Date: Wed, 20 Oct 2021 07:41:11 +0100 Subject: [PATCH 16/23] CLDC-487 - Code review comments update --- app/models/case_log.rb | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/app/models/case_log.rb b/app/models/case_log.rb index 9e8716569..519485580 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -15,14 +15,6 @@ class CaseLogValidator < ActiveModel::Validator end end - def validate_api_reasonable_preference(record) - if record.reasonable_preference == "No" - if record.reasonable_preference_reason_homeless || record.reasonable_preference_reason_unsatisfactory_housing || record.reasonable_preference_reason_medical_grounds || record.reasonable_preference_reason_avoid_hardship || record.reasonable_preference_reason_do_not_know - record.errors.add :reasonable_preference_reason, "- no resasons can be set to true, if reasonable preference is No" - end - end - end - def validate_reasonable_preference(record) if record.homelessness == "No" && record.reasonable_preference == "Yes" record.errors.add :reasonable_preference, "can not be Yes if Not Homesless imediately prior to this letting has been selected" @@ -30,6 +22,10 @@ class CaseLogValidator < ActiveModel::Validator if !record.reasonable_preference_reason_homeless && !record.reasonable_preference_reason_unsatisfactory_housing && !record.reasonable_preference_reason_medical_grounds && !record.reasonable_preference_reason_avoid_hardship && !record.reasonable_preference_reason_do_not_know record.errors.add :reasonable_preference_reason, "- if reasonable preference is Yes, a reason must be given" end + elsif record.reasonable_preference == "No" + if record.reasonable_preference_reason_homeless || record.reasonable_preference_reason_unsatisfactory_housing || record.reasonable_preference_reason_medical_grounds || record.reasonable_preference_reason_avoid_hardship || record.reasonable_preference_reason_do_not_know + record.errors.add :reasonable_preference_reason, "- if reasonable preference is no, no reasons should be given" + end end end From f25603bd7dd4812e087f12f55804a0ffa71e5513 Mon Sep 17 00:00:00 2001 From: Matthew Phelan Date: Wed, 20 Oct 2021 08:05:34 +0100 Subject: [PATCH 17/23] test fix for reasonable preference --- spec/fixtures/complete_case_log.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/fixtures/complete_case_log.json b/spec/fixtures/complete_case_log.json index 11f073714..4cf667b6a 100644 --- a/spec/fixtures/complete_case_log.json +++ b/spec/fixtures/complete_case_log.json @@ -38,7 +38,7 @@ "person_8_age": 2, "person_8_gender": "Prefer not to say", "person_8_economic_status": "Child under 16", - "homelessness": "No", + "homelessness": "Yes - other homelessness", "reason_for_leaving_last_settled_home": "Other problems with neighbours", "benefit_cap_spare_room_subsidy": "No", "armed_forces_active": "No", @@ -107,7 +107,7 @@ "condition_effects_mental_health": false, "condition_effects_social_or_behavioral": false, "condition_effects_other": false, - "condition_effects_prefer_not_to_say": false, + "condition_effects_prefer_not_to_say": true, "reasonable_preference_reason_homeless": false, "reasonable_preference_reason_unsatisfactory_housing": false, "reasonable_preference_reason_medical_grounds": false, From 30b985dfe57a41cad1743fff8869c36e1409350f Mon Sep 17 00:00:00 2001 From: Matthew Phelan Date: Wed, 20 Oct 2021 08:28:06 +0100 Subject: [PATCH 18/23] CLDC-487 - typo fixes --- app/models/case_log.rb | 4 ++-- spec/models/case_log_spec.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/case_log.rb b/app/models/case_log.rb index 519485580..5a390976b 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -17,14 +17,14 @@ class CaseLogValidator < ActiveModel::Validator def validate_reasonable_preference(record) if record.homelessness == "No" && record.reasonable_preference == "Yes" - record.errors.add :reasonable_preference, "can not be Yes if Not Homesless imediately prior to this letting has been selected" + record.errors.add :reasonable_preference, "can not be Yes if Not Homeless immediately prior to this letting has been selected" elsif record.reasonable_preference == "Yes" if !record.reasonable_preference_reason_homeless && !record.reasonable_preference_reason_unsatisfactory_housing && !record.reasonable_preference_reason_medical_grounds && !record.reasonable_preference_reason_avoid_hardship && !record.reasonable_preference_reason_do_not_know record.errors.add :reasonable_preference_reason, "- if reasonable preference is Yes, a reason must be given" end elsif record.reasonable_preference == "No" if record.reasonable_preference_reason_homeless || record.reasonable_preference_reason_unsatisfactory_housing || record.reasonable_preference_reason_medical_grounds || record.reasonable_preference_reason_avoid_hardship || record.reasonable_preference_reason_do_not_know - record.errors.add :reasonable_preference_reason, "- if reasonable preference is no, no reasons should be given" + record.errors.add :reasonable_preference_reason, "- if reasonable preference is No, no reasons should be given" end end end diff --git a/spec/models/case_log_spec.rb b/spec/models/case_log_spec.rb index 236ddf7dc..b27b0594e 100644 --- a/spec/models/case_log_spec.rb +++ b/spec/models/case_log_spec.rb @@ -39,7 +39,7 @@ RSpec.describe Form, type: :model do }.to raise_error(ActiveRecord::RecordInvalid) end - it "if not previously homesless reasonable preference should not be selected" do + it "if not previously homeless reasonable preference should not be selected" do expect { CaseLog.create!( homelessness: "No", From ac5223a09ee123a50785a6358ab70ed551ec03fc Mon Sep 17 00:00:00 2001 From: Daniel Baark <5101747+baarkerlounger@users.noreply.github.com> Date: Wed, 20 Oct 2021 10:08:06 +0100 Subject: [PATCH 19/23] CLDC-542: OpenAPI documentation (#53) * API doc init * Rename docs dir * Try YAML * Index page * Chars * Case Log * Summaries * Add to readme * Add sandbox server * Dry * Add some 422 examples * GET doesn't have a request body * Accept header * Add show method to JSON API and document not found response * Set header default * Add some field guidance * Don't need a default value if we only have one enum --- README.md | 6 + app/controllers/case_logs_controller.rb | 15 +- config/forms/2021_2022.json | 2 +- .../adr-001-initial-architecture-decisions.md | 0 {doc => docs}/adr/adr-002-repositories.md | 0 .../adr/adr-003-form-submission-flow.md | 0 {doc => docs}/adr/adr-004-gov-paas.md | 0 {doc => docs}/adr/adr-005-form-definition.md | 0 {doc => docs}/adr/adr-006-saving-values.md | 0 {doc => docs}/adr/adr-007-data-validations.md | 0 docs/api/DLUHC-CORE-Data.v1.json | 1154 +++++++++++++++++ docs/index.html | 17 + spec/requests/case_log_controller_spec.rb | 18 + 13 files changed, 1208 insertions(+), 4 deletions(-) rename {doc => docs}/adr/adr-001-initial-architecture-decisions.md (100%) rename {doc => docs}/adr/adr-002-repositories.md (100%) rename {doc => docs}/adr/adr-003-form-submission-flow.md (100%) rename {doc => docs}/adr/adr-004-gov-paas.md (100%) rename {doc => docs}/adr/adr-005-form-definition.md (100%) rename {doc => docs}/adr/adr-006-saving-values.md (100%) rename {doc => docs}/adr/adr-007-data-validations.md (100%) create mode 100644 docs/api/DLUHC-CORE-Data.v1.json create mode 100644 docs/index.html diff --git a/README.md b/README.md index 844ffa88d..5af61d33e 100644 --- a/README.md +++ b/README.md @@ -4,6 +4,12 @@ This is the codebase for the Ruby on Rails app that will handle the submission of Lettings and Sales of Social Housing in England data. + +## API documentation + +API documentation can be found here: https://communitiesuk.github.io/mhclg-data-collection-beta/. This is driven by [OpenAPI docs](docs/api/DLUHC-CORE-Data.v1.json) + + ## Required Setup Pre-requisites: diff --git a/app/controllers/case_logs_controller.rb b/app/controllers/case_logs_controller.rb index f3753058e..8201254d5 100644 --- a/app/controllers/case_logs_controller.rb +++ b/app/controllers/case_logs_controller.rb @@ -33,9 +33,18 @@ class CaseLogsController < ApplicationController end end - # We don't have a dedicated non-editable show view def show - edit + respond_to do |format| + # We don't have a dedicated non-editable show view + format.html { edit } + format.json do + if (case_log = CaseLog.find_by(id: params[:id])) + render json: case_log, status: :ok + else + render json: { error: "Case Log #{params[:id]} not found" }, status: :not_found + end + end + end end def edit @@ -90,7 +99,7 @@ class CaseLogsController < ApplicationController private - API_ACTIONS = %w[create update destroy].freeze + API_ACTIONS = %w[create show update destroy].freeze def question_responses(questions_for_page) questions_for_page.each_with_object({}) do |(question_key, question_info), result| diff --git a/config/forms/2021_2022.json b/config/forms/2021_2022.json index 8bed823cf..47378e9f0 100644 --- a/config/forms/2021_2022.json +++ b/config/forms/2021_2022.json @@ -31,7 +31,7 @@ "hint_text": "", "type": "numeric", "min": 0, - "max": 150, + "max": 120, "step": 1 } } diff --git a/doc/adr/adr-001-initial-architecture-decisions.md b/docs/adr/adr-001-initial-architecture-decisions.md similarity index 100% rename from doc/adr/adr-001-initial-architecture-decisions.md rename to docs/adr/adr-001-initial-architecture-decisions.md diff --git a/doc/adr/adr-002-repositories.md b/docs/adr/adr-002-repositories.md similarity index 100% rename from doc/adr/adr-002-repositories.md rename to docs/adr/adr-002-repositories.md diff --git a/doc/adr/adr-003-form-submission-flow.md b/docs/adr/adr-003-form-submission-flow.md similarity index 100% rename from doc/adr/adr-003-form-submission-flow.md rename to docs/adr/adr-003-form-submission-flow.md diff --git a/doc/adr/adr-004-gov-paas.md b/docs/adr/adr-004-gov-paas.md similarity index 100% rename from doc/adr/adr-004-gov-paas.md rename to docs/adr/adr-004-gov-paas.md diff --git a/doc/adr/adr-005-form-definition.md b/docs/adr/adr-005-form-definition.md similarity index 100% rename from doc/adr/adr-005-form-definition.md rename to docs/adr/adr-005-form-definition.md diff --git a/doc/adr/adr-006-saving-values.md b/docs/adr/adr-006-saving-values.md similarity index 100% rename from doc/adr/adr-006-saving-values.md rename to docs/adr/adr-006-saving-values.md diff --git a/doc/adr/adr-007-data-validations.md b/docs/adr/adr-007-data-validations.md similarity index 100% rename from doc/adr/adr-007-data-validations.md rename to docs/adr/adr-007-data-validations.md diff --git a/docs/api/DLUHC-CORE-Data.v1.json b/docs/api/DLUHC-CORE-Data.v1.json new file mode 100644 index 000000000..8a2b02fc7 --- /dev/null +++ b/docs/api/DLUHC-CORE-Data.v1.json @@ -0,0 +1,1154 @@ +{ + "openapi": "3.0.0", + "info": { + "title": "DLUHC CORE Data", + "version": "1.0", + "description": "Submit or Update CORE Case Log Data on Lettings and Sales of Social Housing in England" + }, + "servers": [ + { + "url": "https://dluhc-core.london.cloudapps.digital", + "description": "Sandbox" + } + ], + "paths": { + "/case_logs/:id": { + "parameters": [], + "get": { + "summary": "Get Case Log Info by Case Log ID", + "tags": [], + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Case-Log" + }, + "examples": {} + } + } + }, + "404": { + "description": "Not Found", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": {} + }, + "examples": { + "Not found": { + "value": { + "error": "Case Log 67 not found" + } + } + } + } + } + } + }, + "operationId": "get-case_logs-case_logs-:id", + "description": "Retrieve data for a specific case log", + "parameters": [ + { + "schema": { + "type": "string", + "enum": [ + "application/json" + ] + }, + "in": "header", + "name": "Accept", + "required": true + } + ] + }, + "patch": { + "summary": "Update Case Log Information", + "operationId": "patch-case_logs-case_logs-:id", + "responses": { + "200": { + "description": "Case Log Updated", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Case-Log" + }, + "examples": {} + } + } + }, + "404": { + "description": "Not Found", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": {} + }, + "examples": { + "Not found": { + "value": { + "error": "Case Log 67 not found" + } + } + } + } + } + }, + "422": { + "description": "Unprocessable Entity ", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": {} + }, + "examples": { + "Invalid Age": { + "value": { + "errors": [ + "Tenant age must be between 0 and 120" + ] + } + } + } + } + } + } + }, + "description": "Update the information of an existing case log", + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Case-Log" + }, + "examples": {} + } + }, + "description": "Patch case log properties to update." + }, + "parameters": [ + { + "schema": { + "type": "string", + "enum": [ + "application/json" + ] + }, + "in": "header", + "name": "Accept", + "required": true + } + ] + }, + "delete": { + "summary": "Delete a Case Log by Case Log ID", + "operationId": "delete-case_logs-:id", + "responses": { + "204": { + "description": "No Content" + }, + "404": { + "description": "Not Found", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": {} + }, + "examples": { + "Not found": { + "value": { + "error": "Case Log 67 not found" + } + } + } + } + } + } + }, + "description": "Delete a case log", + "parameters": [ + { + "schema": { + "type": "string", + "enum": [ + "application/json" + ] + }, + "in": "header", + "name": "Accept", + "required": true + } + ] + } + }, + "/case_logs": { + "post": { + "summary": "Create New Case Log", + "operationId": "post-caselog", + "responses": { + "200": { + "description": "Case Log Created", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Case-Log" + }, + "examples": {} + } + } + }, + "422": { + "description": "Unprocessable Entity ", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": {} + }, + "examples": { + "Invalid Age": { + "value": { + "errors": [ + "Tenant age must be between 0 and 120" + ] + } + } + } + } + } + } + }, + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Case-Log" + }, + "examples": {} + } + }, + "description": "Post the necessary fields for the API to create a new case log." + }, + "description": "Create a new case log.", + "parameters": [ + { + "schema": { + "type": "string", + "pattern": "application/json", + "enum": [ + "application/json" + ] + }, + "in": "header", + "name": "Accept", + "required": true + } + ] + }, + "parameters": [] + } + }, + "components": { + "schemas": { + "Case-Log": { + "description": "", + "type": "object", + "x-examples": { + "example-1": { + "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 + } + }, + "title": "Case Log", + "x-internal": false, + "properties": { + "tenant_code": { + "type": "string", + "minLength": 1 + }, + "tenant_age": { + "type": "number", + "description": "The age of the lead tenant", + "maximum": 120, + "minimum": 0 + }, + "tenant_gender": { + "type": "string", + "minLength": 1, + "enum": [ + "Female", + "Male", + "Non-binary", + "Prefer not to say" + ] + }, + "tenant_ethnic_group": { + "type": "string", + "minLength": 1, + "enum": [ + "White: English/Scottish/Welsh/Northern Irish/British", + "White: Irish", + "White: Gypsy/Irish Traveller", + "White: Other", + "Mixed: White & Black Caribbean", + "Mixed: White & Black African", + "Mixed: White & Asian", + "Mixed: Other", + "Asian or Asian British: Indian", + "Asian or Asian British: Pakistani", + "Asian or Asian British: Bangladeshi", + "Asian or Asian British: Chinese", + "Asian or Asian British: Other", + "Black: Caribbean", + "Black: African", + "Black: Other", + "Other Ethnic Group: Arab", + "Other Ethnic Group: Other", + "Prefer not to say" + ] + }, + "tenant_nationality": { + "type": "string", + "minLength": 1, + "enum": [ + "UK national resident in UK", + "A current or former reserve in the UK Armed Forces (exc. National Service)", + "UK national returning from residence overseas", + "Czech Republic", + "Estonia", + "Hungary", + "Latvia", + "Lithuania", + "Poland", + "Slovakia", + "Bulgaria", + "Romania", + "Ireland", + "Other EU Economic Area (EEA country)", + "Any other country", + "Prefer not to say" + ] + }, + "previous_housing_situation": { + "type": "string", + "minLength": 1 + }, + "armed_forces": { + "type": "string", + "minLength": 1 + }, + "tenant_economic_status": { + "type": "string", + "minLength": 1, + "enum": [ + "Part-time - Less than 30 hours", + "Full-time - 30 hours or more", + "In government training into work, such as New Deal", + "Jobseeker", + "Retired", + "Not seeking work", + "Full-time student", + "Unable to work because of long term sick or disability", + "Child under 16", + "Other", + "Prefer not to say" + ] + }, + "household_number_of_other_members": { + "type": "number", + "minimum": 0, + "maximum": 7 + }, + "person_2_relationship": { + "type": "string", + "minLength": 1, + "enum": [ + "Partner", + "Child - includes young adult and grown-up", + "Other", + "Prefer not to say" + ] + }, + "person_2_age": { + "type": "number", + "maximum": 120, + "minimum": 0 + }, + "person_2_gender": { + "type": "string", + "minLength": 1, + "enum": [ + "Female", + "Male", + "Non-binary", + "Prefer not to say" + ] + }, + "person_2_economic_status": { + "type": "string", + "minLength": 1, + "enum": [ + "Part-time - Less than 30 hours", + "Full-time - 30 hours or more", + "In government training into work, such as New Deal", + "Jobseeker", + "Retired", + "Not seeking work", + "Full-time student", + "Unable to work because of long term sick or disability", + "Child under 16", + "Other", + "Prefer not to say" + ] + }, + "person_3_relationship": { + "type": "string", + "minLength": 1, + "enum": [ + "Partner", + "Child - includes young adult and grown-up", + "Other", + "Prefer not to say" + ] + }, + "person_3_age": { + "type": "number", + "maximum": 120, + "minimum": 0 + }, + "person_3_gender": { + "type": "string", + "minLength": 1, + "enum": [ + "Female", + "Male", + "Non-binary", + "Prefer not to say" + ] + }, + "person_3_economic_status": { + "type": "string", + "minLength": 1, + "enum": [ + "Part-time - Less than 30 hours", + "Full-time - 30 hours or more", + "In government training into work, such as New Deal", + "Jobseeker", + "Retired", + "Not seeking work", + "Full-time student", + "Unable to work because of long term sick or disability", + "Child under 16", + "Other", + "Prefer not to say" + ] + }, + "person_4_relationship": { + "type": "string", + "minLength": 1, + "enum": [ + "Partner", + "Child - includes young adult and grown-up", + "Other", + "Prefer not to say" + ] + }, + "person_4_age": { + "type": "number", + "maximum": 120, + "minimum": 0 + }, + "person_4_gender": { + "type": "string", + "minLength": 1, + "enum": [ + "Female", + "Male", + "Non-binary", + "Prefer not to say" + ] + }, + "person_4_economic_status": { + "type": "string", + "minLength": 1, + "enum": [ + "Part-time - Less than 30 hours", + "Full-time - 30 hours or more", + "In government training into work, such as New Deal", + "Jobseeker", + "Retired", + "Not seeking work", + "Full-time student", + "Unable to work because of long term sick or disability", + "Child under 16", + "Other", + "Prefer not to say" + ] + }, + "person_5_relationship": { + "type": "string", + "minLength": 1, + "enum": [ + "Partner", + "Child - includes young adult and grown-up", + "Other", + "Prefer not to say" + ] + }, + "person_5_age": { + "type": "number", + "maximum": 120, + "minimum": 0 + }, + "person_5_gender": { + "type": "string", + "minLength": 1, + "enum": [ + "Female", + "Male", + "Non-binary", + "Prefer not to say" + ] + }, + "person_5_economic_status": { + "type": "string", + "minLength": 1, + "enum": [ + "Part-time - Less than 30 hours", + "Full-time - 30 hours or more", + "In government training into work, such as New Deal", + "Jobseeker", + "Retired", + "Not seeking work", + "Full-time student", + "Unable to work because of long term sick or disability", + "Child under 16", + "Other", + "Prefer not to say" + ] + }, + "person_6_relationship": { + "type": "string", + "minLength": 1, + "enum": [ + "Partner", + "Child - includes young adult and grown-up", + "Other", + "Prefer not to say" + ] + }, + "person_6_age": { + "type": "number", + "maximum": 120, + "minimum": 0 + }, + "person_6_gender": { + "type": "string", + "minLength": 1, + "enum": [ + "Female", + "Male", + "Non-binary", + "Prefer not to say" + ] + }, + "person_6_economic_status": { + "type": "string", + "minLength": 1, + "enum": [ + "Part-time - Less than 30 hours", + "Full-time - 30 hours or more", + "In government training into work, such as New Deal", + "Jobseeker", + "Retired", + "Not seeking work", + "Full-time student", + "Unable to work because of long term sick or disability", + "Child under 16", + "Other", + "Prefer not to say" + ] + }, + "person_7_relationship": { + "type": "string", + "minLength": 1, + "enum": [ + "Partner", + "Child - includes young adult and grown-up", + "Other", + "Prefer not to say" + ] + }, + "person_7_age": { + "type": "number", + "maximum": 120, + "minimum": 0 + }, + "person_7_gender": { + "type": "string", + "minLength": 1, + "enum": [ + "Female", + "Male", + "Non-binary", + "Prefer not to say" + ] + }, + "person_7_economic_status": { + "type": "string", + "minLength": 1, + "enum": [ + "Part-time - Less than 30 hours", + "Full-time - 30 hours or more", + "In government training into work, such as New Deal", + "Jobseeker", + "Retired", + "Not seeking work", + "Full-time student", + "Unable to work because of long term sick or disability", + "Child under 16", + "Other", + "Prefer not to say" + ] + }, + "person_8_relationship": { + "type": "string", + "minLength": 1, + "enum": [ + "Partner", + "Child - includes young adult and grown-up", + "Other", + "Prefer not to say" + ] + }, + "person_8_age": { + "type": "number", + "maximum": 120, + "minimum": 0 + }, + "person_8_gender": { + "type": "string", + "minLength": 1, + "enum": [ + "Female", + "Male", + "Non-binary", + "Prefer not to say" + ] + }, + "person_8_economic_status": { + "type": "string", + "minLength": 1, + "enum": [ + "Part-time - Less than 30 hours", + "Full-time - 30 hours or more", + "In government training into work, such as New Deal", + "Jobseeker", + "Retired", + "Not seeking work", + "Full-time student", + "Unable to work because of long term sick or disability", + "Child under 16", + "Other", + "Prefer not to say" + ] + }, + "homelessness": { + "type": "string", + "minLength": 1 + }, + "reason_for_leaving_last_settled_home": { + "type": "string", + "minLength": 1 + }, + "benefit_cap_spare_room_subsidy": { + "type": "string", + "minLength": 1 + }, + "armed_forces_active": { + "type": "string", + "minLength": 1 + }, + "armed_forces_injured": { + "type": "string", + "minLength": 1 + }, + "armed_forces_partner": { + "type": "string", + "minLength": 1 + }, + "medical_conditions": { + "type": "string", + "minLength": 1 + }, + "pregnancy": { + "type": "string", + "minLength": 1 + }, + "accessibility_requirements": { + "type": "string", + "minLength": 1 + }, + "condition_effects": { + "type": "string", + "minLength": 1 + }, + "tenancy_code": { + "type": "string", + "minLength": 1 + }, + "tenancy_start_date": { + "type": "string", + "minLength": 1 + }, + "starter_tenancy": { + "type": "string", + "minLength": 1 + }, + "fixed_term_tenancy": { + "type": "string", + "minLength": 1 + }, + "tenancy_type": { + "type": "string", + "minLength": 1 + }, + "letting_type": { + "type": "string", + "minLength": 1 + }, + "letting_provider": { + "type": "string", + "minLength": 1 + }, + "property_location": { + "type": "string", + "minLength": 1 + }, + "previous_postcode": { + "type": "string", + "minLength": 1 + }, + "property_relet": { + "type": "string", + "minLength": 1 + }, + "property_vacancy_reason": { + "type": "string", + "minLength": 1 + }, + "property_reference": { + "type": "string", + "minLength": 1 + }, + "property_unit_type": { + "type": "string", + "minLength": 1 + }, + "property_building_type": { + "type": "string", + "minLength": 1 + }, + "property_number_of_bedrooms": { + "type": "number" + }, + "property_void_date": { + "type": "string", + "minLength": 1 + }, + "property_major_repairs": { + "type": "string", + "minLength": 1 + }, + "property_major_repairs_date": { + "type": "string", + "minLength": 1 + }, + "property_number_of_times_relet": { + "type": "number" + }, + "property_wheelchair_accessible": { + "type": "boolean" + }, + "net_income": { + "type": "number" + }, + "net_income_frequency": { + "type": "string", + "minLength": 1 + }, + "net_income_uc_proportion": { + "type": "string", + "minLength": 1 + }, + "housing_benefit": { + "type": "string", + "minLength": 1 + }, + "rent_frequency": { + "type": "string", + "minLength": 1 + }, + "basic_rent": { + "type": "number" + }, + "service_charge": { + "type": "number" + }, + "personal_service_charge": { + "type": "number" + }, + "support_charge": { + "type": "number" + }, + "total_charge": { + "type": "number" + }, + "outstanding_amount": { + "type": "string", + "minLength": 1 + }, + "time_lived_in_la": { + "type": "string", + "minLength": 1 + }, + "time_on_la_waiting_list": { + "type": "string", + "minLength": 1 + }, + "previous_la": { + "type": "string", + "minLength": 1 + }, + "property_postcode": { + "type": "string", + "minLength": 1 + }, + "reasonable_preference": { + "type": "string", + "minLength": 1 + }, + "reasonable_preference_reason": { + "type": "string", + "minLength": 1 + }, + "cbl_letting": { + "type": "boolean" + }, + "chr_letting": { + "type": "boolean" + }, + "cap_letting": { + "type": "boolean" + }, + "outstanding_rent_or_charges": { + "type": "number" + }, + "other_reason_for_leaving_last_settled_home": { + "type": "string", + "minLength": 1 + }, + "accessibility_requirements_fully_wheelchair_accessible_housing": { + "type": "boolean" + }, + "accessibility_requirements_wheelchair_access_to_essential_rooms": { + "type": "boolean" + }, + "accessibility_requirements_level_access_housing": { + "type": "boolean" + }, + "accessibility_requirements_other_disability_requirements": { + "type": "boolean" + }, + "accessibility_requirements_no_disability_requirements": { + "type": "boolean" + }, + "accessibility_requirements_do_not_know": { + "type": "boolean" + }, + "accessibility_requirements_prefer_not_to_say": { + "type": "boolean" + }, + "condition_effects_vision": { + "type": "boolean" + }, + "condition_effects_hearing": { + "type": "boolean" + }, + "condition_effects_mobility": { + "type": "boolean" + }, + "condition_effects_dexterity": { + "type": "boolean" + }, + "condition_effects_stamina": { + "type": "boolean" + }, + "condition_effects_learning": { + "type": "boolean" + }, + "condition_effects_memory": { + "type": "boolean" + }, + "condition_effects_mental_health": { + "type": "boolean" + }, + "condition_effects_social_or_behavioral": { + "type": "boolean" + }, + "condition_effects_other": { + "type": "boolean" + }, + "condition_effects_prefer_not_to_say": { + "type": "boolean" + }, + "reasonable_preference_reason_homeless": { + "type": "boolean" + }, + "reasonable_preference_reason_unsatisfactory_housing": { + "type": "boolean" + }, + "reasonable_preference_reason_medical_grounds": { + "type": "boolean" + }, + "reasonable_preference_reason_avoid_hardship": { + "type": "boolean" + }, + "reasonable_preference_reason_do_not_know": { + "type": "boolean" + } + }, + "required": [ + "tenant_code", + "tenant_age", + "tenant_gender", + "tenant_ethnic_group", + "tenant_nationality", + "previous_housing_situation", + "armed_forces", + "tenant_economic_status", + "household_number_of_other_members", + "person_2_relationship", + "person_2_age", + "person_2_gender", + "person_2_economic_status", + "person_3_relationship", + "person_3_age", + "person_3_gender", + "person_3_economic_status", + "person_4_relationship", + "person_4_age", + "person_4_gender", + "person_4_economic_status", + "person_5_relationship", + "person_5_age", + "person_5_gender", + "person_5_economic_status", + "person_6_relationship", + "person_6_age", + "person_6_gender", + "person_6_economic_status", + "person_7_relationship", + "person_7_age", + "person_7_gender", + "person_7_economic_status", + "person_8_relationship", + "person_8_age", + "person_8_gender", + "person_8_economic_status", + "homelessness", + "reason_for_leaving_last_settled_home", + "benefit_cap_spare_room_subsidy", + "armed_forces_active", + "armed_forces_injured", + "armed_forces_partner", + "medical_conditions", + "pregnancy", + "accessibility_requirements", + "condition_effects", + "tenancy_code", + "tenancy_start_date", + "starter_tenancy", + "fixed_term_tenancy", + "tenancy_type", + "letting_type", + "letting_provider", + "property_location", + "previous_postcode", + "property_relet", + "property_vacancy_reason", + "property_reference", + "property_unit_type", + "property_building_type", + "property_number_of_bedrooms", + "property_void_date", + "property_major_repairs", + "property_major_repairs_date", + "property_number_of_times_relet", + "property_wheelchair_accessible", + "net_income", + "net_income_frequency", + "net_income_uc_proportion", + "housing_benefit", + "rent_frequency", + "basic_rent", + "service_charge", + "personal_service_charge", + "support_charge", + "total_charge", + "outstanding_amount", + "time_lived_in_la", + "time_on_la_waiting_list", + "previous_la", + "property_postcode", + "reasonable_preference", + "reasonable_preference_reason", + "cbl_letting", + "chr_letting", + "cap_letting", + "outstanding_rent_or_charges", + "other_reason_for_leaving_last_settled_home", + "accessibility_requirements_fully_wheelchair_accessible_housing", + "accessibility_requirements_wheelchair_access_to_essential_rooms", + "accessibility_requirements_level_access_housing", + "accessibility_requirements_other_disability_requirements", + "accessibility_requirements_no_disability_requirements", + "accessibility_requirements_do_not_know", + "accessibility_requirements_prefer_not_to_say", + "condition_effects_vision", + "condition_effects_hearing", + "condition_effects_mobility", + "condition_effects_dexterity", + "condition_effects_stamina", + "condition_effects_learning", + "condition_effects_memory", + "condition_effects_mental_health", + "condition_effects_social_or_behavioral", + "condition_effects_other", + "condition_effects_prefer_not_to_say", + "reasonable_preference_reason_homeless", + "reasonable_preference_reason_unsatisfactory_housing", + "reasonable_preference_reason_medical_grounds", + "reasonable_preference_reason_avoid_hardship", + "reasonable_preference_reason_do_not_know" + ] + } + }, + "securitySchemes": {} + } +} diff --git a/docs/index.html b/docs/index.html new file mode 100644 index 000000000..1f0ae16fa --- /dev/null +++ b/docs/index.html @@ -0,0 +1,17 @@ + + + + + + +OpenAPI DLUHC CORE Data Collection +
+ + diff --git a/spec/requests/case_log_controller_spec.rb b/spec/requests/case_log_controller_spec.rb index 9dab538b0..dc67b76e1 100644 --- a/spec/requests/case_log_controller_spec.rb +++ b/spec/requests/case_log_controller_spec.rb @@ -99,6 +99,24 @@ RSpec.describe CaseLogsController, type: :request do end end + describe "GET" do + let(:case_log) { FactoryBot.create(:case_log, :completed) } + let(:id) { case_log.id } + + before do + get "/case_logs/#{id}", headers: headers + end + + it "returns http success" do + expect(response).to have_http_status(:success) + end + + it "returns a serialized Case Log" do + json_response = JSON.parse(response.body) + expect(json_response["status"]).to eq(case_log.status) + end + end + describe "PATCH" do let(:case_log) do FactoryBot.create(:case_log, :in_progress, tenant_code: "Old Value", property_postcode: "Old Value") From 9a8945ea9a8667822ae4853287e3094617acdd15 Mon Sep 17 00:00:00 2001 From: MadeTech Dushan Date: Wed, 20 Oct 2021 10:10:22 +0100 Subject: [PATCH 20/23] dynamically not required attributes for case logs --- app/models/case_log.rb | 10 +++++++++- spec/fixtures/complete_case_log.json | 2 +- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/app/models/case_log.rb b/app/models/case_log.rb index 1f4219d57..9a84c633e 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -97,6 +97,14 @@ private end def mandatory_fields - attributes.except(*AUTOGENERATED_FIELDS) + required = attributes.except(*AUTOGENERATED_FIELDS) + + dynamically_not_required = [] + + if reason_for_leaving_last_settled_home != "Other" + dynamically_not_required << "other_reason_for_leaving_last_settled_home" + end + + required.delete_if { |key, _value| dynamically_not_required.include?(key) } end end diff --git a/spec/fixtures/complete_case_log.json b/spec/fixtures/complete_case_log.json index 11f073714..da2e0da20 100644 --- a/spec/fixtures/complete_case_log.json +++ b/spec/fixtures/complete_case_log.json @@ -89,7 +89,7 @@ "chr_letting": false, "cap_letting": false, "outstanding_rent_or_charges": 25, - "other_reason_for_leaving_last_settled_home": "Other reason", + "other_reason_for_leaving_last_settled_home": null, "accessibility_requirements_fully_wheelchair_accessible_housing": true, "accessibility_requirements_wheelchair_access_to_essential_rooms": false, "accessibility_requirements_level_access_housing": false, From 1fce70bd3cfd9e08ce2a9e34742b6ce55c67a2a9 Mon Sep 17 00:00:00 2001 From: baarkerlounger Date: Wed, 20 Oct 2021 10:28:59 +0100 Subject: [PATCH 21/23] Rubocop fixes --- app/controllers/case_logs_controller.rb | 2 +- ...5090040_update_property_number_of_times_relet_type.rb | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/app/controllers/case_logs_controller.rb b/app/controllers/case_logs_controller.rb index 8201254d5..85f1ae1b1 100644 --- a/app/controllers/case_logs_controller.rb +++ b/app/controllers/case_logs_controller.rb @@ -35,7 +35,7 @@ class CaseLogsController < ApplicationController def show respond_to do |format| - # We don't have a dedicated non-editable show view + # We don't have a dedicated non-editable show view format.html { edit } format.json do if (case_log = CaseLog.find_by(id: params[:id])) diff --git a/db/migrate/20211015090040_update_property_number_of_times_relet_type.rb b/db/migrate/20211015090040_update_property_number_of_times_relet_type.rb index cf4958044..5fa030571 100644 --- a/db/migrate/20211015090040_update_property_number_of_times_relet_type.rb +++ b/db/migrate/20211015090040_update_property_number_of_times_relet_type.rb @@ -1,6 +1,9 @@ class UpdatePropertyNumberOfTimesReletType < ActiveRecord::Migration[6.1] - def change - remove_column :case_logs, :property_number_of_times_relet, :string - add_column :case_logs, :property_number_of_times_relet, :integer + def up + change_column :case_logs, :property_number_of_times_relet, "integer USING CAST(property_number_of_times_relet AS integer)" + end + + def down + change_column :case_logs, :property_number_of_times_relet, :string end end From e32779104c3928ebef3ef5240d3cf3a4cb11f44a Mon Sep 17 00:00:00 2001 From: MadeTech Dushan Date: Wed, 20 Oct 2021 16:01:29 +0100 Subject: [PATCH 22/23] Make net income frequency not required dynamically No errors need to be added if the net income is given as 0 but no frequency is provided. It just needs to be made not required if the net income is refused (given as 0) so that the record can still be added. --- app/models/case_log.rb | 4 ++++ spec/fixtures/complete_case_log.json | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/app/models/case_log.rb b/app/models/case_log.rb index 9a84c633e..ae16a4945 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -105,6 +105,10 @@ private dynamically_not_required << "other_reason_for_leaving_last_settled_home" end + if net_income.to_i == 0 + dynamically_not_required << "net_income_frequency" + end + required.delete_if { |key, _value| dynamically_not_required.include?(key) } end end diff --git a/spec/fixtures/complete_case_log.json b/spec/fixtures/complete_case_log.json index da2e0da20..050246722 100644 --- a/spec/fixtures/complete_case_log.json +++ b/spec/fixtures/complete_case_log.json @@ -68,8 +68,8 @@ "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": 0, + "net_income_frequency": null, "net_income_uc_proportion": "Some", "housing_benefit": "Universal Credit with housing element, but not Housing Benefit", "rent_frequency": "Weekly", From 37fe1b4845e6c19467f82d2b9538472cbaf82272 Mon Sep 17 00:00:00 2001 From: MadeTech Dushan Date: Wed, 20 Oct 2021 16:08:46 +0100 Subject: [PATCH 23/23] lint fixes --- app/models/case_log.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/case_log.rb b/app/models/case_log.rb index ae16a4945..a524a9e42 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -105,8 +105,8 @@ private dynamically_not_required << "other_reason_for_leaving_last_settled_home" end - if net_income.to_i == 0 - dynamically_not_required << "net_income_frequency" + if net_income.to_i.zero? + dynamically_not_required << "net_income_frequency" end required.delete_if { |key, _value| dynamically_not_required.include?(key) }