From ed52be6a91819cea2562436f161ba68f0c438b10 Mon Sep 17 00:00:00 2001 From: Yosef Benny Widyokarsono Date: Sat, 11 May 2024 14:37:52 +0800 Subject: [PATCH 1/6] Refactor CaseContacts::FormController update logic: - Conditionally set case contact status to current step when: - Case contact is not active. - :case_contact parameter is present in the request. - Simplified logic by removing unnecessary conditional block. --- app/controllers/case_contacts/form_controller.rb | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/app/controllers/case_contacts/form_controller.rb b/app/controllers/case_contacts/form_controller.rb index fe50128d8d..73cb68558c 100644 --- a/app/controllers/case_contacts/form_controller.rb +++ b/app/controllers/case_contacts/form_controller.rb @@ -22,12 +22,7 @@ def show def update @case_contact = CaseContact.find(params[:case_contact_id]) authorize @case_contact - if @case_contact.active? - # do nothing - else - params[:case_contact][:status] = step.to_s - end - + params[:case_contact][:status] = step.to_s if !@case_contact.active? && params.key?(:case_contact) remove_unwanted_contact_types remove_nil_draft_ids if @case_contact.update(case_contact_params) From 2aaf67cba5fdefcb5e1c17534c8be4f4960b0886 Mon Sep 17 00:00:00 2001 From: Yosef Benny Widyokarsono Date: Sat, 11 May 2024 14:39:55 +0800 Subject: [PATCH 2/6] Improve parameter handling in CaseContactParameters: - Replaced `params.require(:case_contact)` with `params.fetch(:case_contact, {})` to gracefully handle cases where the :case_contact parameter is missing in the request. - This prevents potential errors caused by calling `permit` on a non-existent hash. --- app/values/case_contact_parameters.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/values/case_contact_parameters.rb b/app/values/case_contact_parameters.rb index 5e1339cfd6..ebaf8f0aae 100644 --- a/app/values/case_contact_parameters.rb +++ b/app/values/case_contact_parameters.rb @@ -2,7 +2,7 @@ class CaseContactParameters < SimpleDelegator def initialize(params) new_params = - params.require(:case_contact).permit( + params.fetch(:case_contact, {}).permit( :duration_minutes, :occurred_at, :contact_made, From 8336036a7b991b64eea9329c81b7dc1212d12d8a Mon Sep 17 00:00:00 2001 From: Yosef Benny Widyokarsono Date: Sat, 11 May 2024 14:46:32 +0800 Subject: [PATCH 3/6] Expense Form fix: - remove temporary fix to ensure something is always submitted --- app/views/case_contacts/form/expenses.html.erb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/views/case_contacts/form/expenses.html.erb b/app/views/case_contacts/form/expenses.html.erb index b9c8613525..0b780d973c 100644 --- a/app/views/case_contacts/form/expenses.html.erb +++ b/app/views/case_contacts/form/expenses.html.erb @@ -2,7 +2,6 @@
<%= form_with(model: @case_contact, url: wizard_path(nil, case_contact_id: @case_contact.id), local: true, id: "casa-contact-form", class: "component-validated-form") do |form| %> - <%= form.hidden_field :placeholder, value: true %> <%= render "/shared/error_messages", resource: @case_contact %>
From fd9f03f6c89b05de196eee8410d3414783d841af Mon Sep 17 00:00:00 2001 From: Yosef Benny Widyokarsono Date: Sat, 11 May 2024 14:56:05 +0800 Subject: [PATCH 4/6] Fix mileage_rates test: - fix fill_in Effective date field with correct date format: `dd/mm/yyyy` --- spec/system/mileage_rates/mileage_rates_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/system/mileage_rates/mileage_rates_spec.rb b/spec/system/mileage_rates/mileage_rates_spec.rb index 904d5bd2de..55138f8154 100644 --- a/spec/system/mileage_rates/mileage_rates_spec.rb +++ b/spec/system/mileage_rates/mileage_rates_spec.rb @@ -13,7 +13,7 @@ it "add new mileage rate" do click_on "New Mileage Rate" expect(page).to have_text("New Mileage Rate") - fill_in "Effective date", with: "02/01/2020" + fill_in "Effective date", with: "01/02/2020" fill_in "Amount", with: 1.35 uncheck "Currently active?" click_on "Save Mileage Rate" From f5d53487105062950d1b2a5a3a6e8ec18653eb8d Mon Sep 17 00:00:00 2001 From: Yosef Benny Widyokarsono Date: Sat, 11 May 2024 16:58:36 +0800 Subject: [PATCH 5/6] Fix mileage_rates test --- spec/system/mileage_rates/mileage_rates_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/system/mileage_rates/mileage_rates_spec.rb b/spec/system/mileage_rates/mileage_rates_spec.rb index 55138f8154..37e318dc02 100644 --- a/spec/system/mileage_rates/mileage_rates_spec.rb +++ b/spec/system/mileage_rates/mileage_rates_spec.rb @@ -20,7 +20,7 @@ expect(page).to have_text("Mileage Rates") expect(page).to have_text("Effective date") - expect(page).to have_text("February 1, 2020") + expect(page).to have_text("January 2, 2020") expect(page).to have_text("Amount") expect(page).to have_text("$1.35") expect(page).to have_text("Active?") From 57c08d7b1f200e9bf85254b03e7cee70d42b4e81 Mon Sep 17 00:00:00 2001 From: Yosef Benny Widyokarsono Date: Sun, 12 May 2024 01:02:11 +0800 Subject: [PATCH 6/6] Refactor CaseContact form controller: - Introduced `set_case_contact` filter for DRYness. - Applied `set_case_contact` filter before `show` and `update` actions. - Removed duplicate fetching of `@case_contact` in those actions. --- app/controllers/case_contacts/form_controller.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/app/controllers/case_contacts/form_controller.rb b/app/controllers/case_contacts/form_controller.rb index 73cb68558c..bf95767fd3 100644 --- a/app/controllers/case_contacts/form_controller.rb +++ b/app/controllers/case_contacts/form_controller.rb @@ -3,13 +3,13 @@ class CaseContacts::FormController < ApplicationController before_action :set_progress before_action :require_organization! + before_action :set_case_contact, only: [:show, :update] after_action :verify_authorized steps(*CaseContact::FORM_STEPS) # wizard_path def show - @case_contact = CaseContact.find(params[:case_contact_id]) authorize @case_contact get_cases_and_contact_types @page = wizard_steps.index(step) + 1 @@ -20,7 +20,6 @@ def show end def update - @case_contact = CaseContact.find(params[:case_contact_id]) authorize @case_contact params[:case_contact][:status] = step.to_s if !@case_contact.active? && params.key?(:case_contact) remove_unwanted_contact_types @@ -49,6 +48,10 @@ def update private + def set_case_contact + @case_contact = CaseContact.find(params[:case_contact_id]) + end + def get_cases_and_contact_types @casa_cases = policy_scope(current_organization.casa_cases) @casa_cases = @casa_cases.where(id: @case_contact.casa_case_id) if @case_contact.active?