diff --git a/app/controllers/case_contacts/form_controller.rb b/app/controllers/case_contacts/form_controller.rb index e73e7d15d0..bb30da7631 100644 --- a/app/controllers/case_contacts/form_controller.rb +++ b/app/controllers/case_contacts/form_controller.rb @@ -20,7 +20,7 @@ def show def update authorize @case_contact - params[:case_contact][:status] = step.to_s if !@case_contact.active? + params[:case_contact][:status] = CaseContact.statuses[step] if !@case_contact.active? remove_unwanted_contact_types remove_nil_draft_ids diff --git a/app/controllers/case_contacts_controller.rb b/app/controllers/case_contacts_controller.rb index 8e9d908ed2..96fdf6bc14 100644 --- a/app/controllers/case_contacts_controller.rb +++ b/app/controllers/case_contacts_controller.rb @@ -117,7 +117,7 @@ def additional_expense_params end def case_contact_drafts - CaseContact.where(creator: current_user).where.not(status: "active") + CaseContact.where(creator: current_user).not_active end def set_case_contact diff --git a/app/models/case_contact.rb b/app/models/case_contact.rb index 2ec14b11b2..462b3e96cc 100644 --- a/app/models/case_contact.rb +++ b/app/models/case_contact.rb @@ -45,16 +45,28 @@ class CaseContact < ApplicationRecord after_save_commit ::CaseContactMetadataCallback.new # Corresponds to the steps in the controller, so validations for certain columns can happen at those steps. - # These steps must be listed in order and have an html template in case_contacts/form. + # These steps must be listed in order, have an html template in case_contacts/form, & be listed in the status enum FORM_STEPS = %i[details notes expenses].freeze - enum status: (%w[started active] + FORM_STEPS.map(&:to_s)).zip((%w[started active] + FORM_STEPS.map(&:to_s))).to_h + # note: enum defines methods (active?) and scopes (.active, .not_active) for each member + # string values for wizard form steps, integer column would make db queries faster + enum :status, { + started: "started", + active: "active", + details: "details", + notes: "notes", + expenses: "expenses" + }, validate: true, default: :started def active_or_details? - status == "details" || active? + details? || active? end def active_or_expenses? - status == "expenses" || active? + expenses? || active? + end + + def active_or_notes? + notes? || active? end accepts_nested_attributes_for :additional_expenses, reject_if: :all_blank, allow_destroy: true @@ -150,7 +162,7 @@ def active_or_expenses? where(casa_case_id: case_ids) if case_ids.present? } - scope :no_drafts, ->(checked) { (checked == 1) ? where(status: "active") : all } + scope :no_drafts, ->(checked) { (checked == 1) ? active : all } scope :with_metadata_pair, ->(key, value) { where("metadata -> ? @> ?::jsonb", key.to_s, value.to_s) } scope :used_create_another, -> { with_metadata_pair(:create_another, true) } diff --git a/spec/callbacks/case_contact_metadata_callback_spec.rb b/spec/callbacks/case_contact_metadata_callback_spec.rb index a0678dcd63..0b116b2e93 100644 --- a/spec/callbacks/case_contact_metadata_callback_spec.rb +++ b/spec/callbacks/case_contact_metadata_callback_spec.rb @@ -23,7 +23,7 @@ end context "case contact is in started status" do - let(:case_contact) { create(:case_contact, status: "started", created_at: past) } + let(:case_contact) { create(:case_contact, :started, created_at: past) } context "updates to started status" do before { case_contact.update(status: "started") } @@ -59,7 +59,7 @@ end context "case contact is in details status" do - let(:case_contact) { create(:case_contact, status: "details", created_at: past) } + let(:case_contact) { create(:case_contact, :details, created_at: past) } context "updates to details status" do before { case_contact.update(status: "details") } @@ -89,7 +89,7 @@ end context "case contact is in notes status" do - let(:case_contact) { create(:case_contact, status: "notes", created_at: past) } + let(:case_contact) { create(:case_contact, :notes, created_at: past) } context "updates to notes status" do before { case_contact.update(status: "notes") } @@ -111,7 +111,7 @@ end context "case contact is in expenses status" do - let!(:case_contact) { create(:case_contact, status: "expenses", created_at: past) } + let!(:case_contact) { create(:case_contact, :expenses, created_at: past) } context "updates to expenses status" do before { case_contact.update(status: "expenses") } diff --git a/spec/factories/case_contacts.rb b/spec/factories/case_contacts.rb index 35a04e83bd..474df9e186 100644 --- a/spec/factories/case_contacts.rb +++ b/spec/factories/case_contacts.rb @@ -1,16 +1,25 @@ FactoryBot.define do + # NOTE: FactoryBot automatically creates traits for a model's enum attributes + # https://github.com/thoughtbot/factory_bot/blob/main/GETTING_STARTED.md#enum-traits + # For example, CaseContact status enum includes `active: "active"` state, so following already defined: + # trait :active do + # status { "active" } + # end + # ALSO, we can use any trait within other traits: + # https://github.com/thoughtbot/factory_bot/blob/main/GETTING_STARTED.md#traits-within-traits + # So, rather than `status { "active" }` - use enum trait like so: factory :case_contact do + active # use the `:active` enum trait association :creator, factory: :user casa_case contact_types { [create(:contact_type)] } duration_minutes { 60 } - occurred_at { Time.zone.now } + occurred_at { Time.zone.today } contact_made { false } medium_type { CaseContact::CONTACT_MEDIUMS.first } want_driving_reimbursement { false } deleted_at { nil } - status { "active" } draft_case_ids { [casa_case&.id] } trait :multi_line_note do @@ -32,6 +41,8 @@ end trait :started_status do + started # enum trait + casa_case { nil } draft_case_ids { [] } medium_type { nil } @@ -39,27 +50,29 @@ duration_minutes { nil } notes { nil } miles_driven { 0 } - status { "started" } end trait :details_status do + details # enum trait + casa_case { nil } draft_case_ids { [1] } notes { nil } miles_driven { 0 } - status { "details" } end trait :notes_status do + notes # enum trait + casa_case { nil } draft_case_ids { [1] } miles_driven { 0 } - status { "notes" } end trait :expenses_status do + expenses # enum trait + draft_case_ids { [1] } - status { "expenses" } end after(:create) do |case_contact, evaluator| diff --git a/spec/policies/case_contact_policy_spec.rb b/spec/policies/case_contact_policy_spec.rb index e4950f85c8..084af4fb66 100644 --- a/spec/policies/case_contact_policy_spec.rb +++ b/spec/policies/case_contact_policy_spec.rb @@ -103,7 +103,7 @@ end context "case_contact is not a draft" do - let(:case_contact) { build_stubbed(:case_contact, status: "active", creator: volunteer) } + let(:case_contact) { build_stubbed(:case_contact, :active, creator: volunteer) } it { is_expected.not_to permit(volunteer, case_contact) } end @@ -117,7 +117,7 @@ end context "case_contact is not a draft" do - let(:case_contact) { build_stubbed(:case_contact, status: "active", creator: build_stubbed(:volunteer)) } + let(:case_contact) { build_stubbed(:case_contact, :active, creator: build_stubbed(:volunteer)) } it { is_expected.not_to permit(volunteer, case_contact) } end diff --git a/spec/services/deployment/backfill_case_contact_started_metadata_service_spec.rb b/spec/services/deployment/backfill_case_contact_started_metadata_service_spec.rb index 0ff4fe8329..776e438eb1 100644 --- a/spec/services/deployment/backfill_case_contact_started_metadata_service_spec.rb +++ b/spec/services/deployment/backfill_case_contact_started_metadata_service_spec.rb @@ -14,7 +14,7 @@ let(:case_contact) { create(:case_contact) } context "when a case contact has status started metadata" do - let!(:case_contact) { create(:case_contact, created_at: past, status: "started") } + let!(:case_contact) { create(:case_contact, :started, created_at: past) } it "does not change metadata" do described_class.new.backfill_metadata diff --git a/spec/system/case_contacts/additional_expenses_spec.rb b/spec/system/case_contacts/additional_expenses_spec.rb index 32274640cb..65c31f74d8 100644 --- a/spec/system/case_contacts/additional_expenses_spec.rb +++ b/spec/system/case_contacts/additional_expenses_spec.rb @@ -67,7 +67,7 @@ expect { click_on "Submit" - }.to change(CaseContact.where(status: "active"), :count).by(1).and change(AdditionalExpense, :count).by(1) + }.to change(CaseContact.active, :count).by(1).and change(AdditionalExpense, :count).by(1) visit edit_case_contact_path(casa_case.reload.case_contacts.last) complete_details_page(contact_made: true) @@ -108,7 +108,7 @@ expect(page).to have_text("Add Another Expense") expect { click_on "Submit" - }.to change(CaseContact.where(status: "active"), :count).by(1).and change(AdditionalExpense, :count).by(2) + }.to change(CaseContact.active, :count).by(1).and change(AdditionalExpense, :count).by(2) visit edit_case_contact_path(casa_case.reload.case_contacts.last) complete_details_page(contact_made: true) @@ -132,7 +132,7 @@ expect { click_on "Submit" - }.to change(CaseContact.where(status: "active"), :count).by(0).and change(AdditionalExpense, :count).by(1) + }.to change(CaseContact.active, :count).by(0).and change(AdditionalExpense, :count).by(1) visit edit_case_contact_path(casa_case.reload.case_contacts.last) complete_details_page(contact_made: true) @@ -181,7 +181,7 @@ expect { click_on "Submit" - }.to change(CaseContact.where(status: "active"), :count).by(1).and change(AdditionalExpense, :count).by(12) + }.to change(CaseContact.active, :count).by(1).and change(AdditionalExpense, :count).by(12) visit edit_case_contact_path(casa_case.reload.case_contacts.last) complete_details_page(contact_made: true) @@ -227,7 +227,7 @@ expect { click_on "Submit" - }.to change(CaseContact.where(status: "active"), :count).by(1).and change(AdditionalExpense, :count).by(2) + }.to change(CaseContact.active, :count).by(1).and change(AdditionalExpense, :count).by(2) visit edit_case_contact_path(casa_case.reload.case_contacts.last) complete_details_page(contact_made: true) @@ -240,7 +240,7 @@ expect { click_on "Submit" - }.to change(CaseContact.where(status: "active"), :count).by(0).and change(AdditionalExpense, :count).by(-1) + }.to change(CaseContact.active, :count).by(0).and change(AdditionalExpense, :count).by(-1) end it "verifies that an additional expense without a description will cause an error", js: true do @@ -260,7 +260,7 @@ expect { click_on "Submit" - }.to change(CaseContact.where(status: "active"), :count).by(0).and change(AdditionalExpense, :count).by(0) + }.to change(CaseContact.active, :count).by(0).and change(AdditionalExpense, :count).by(0) expect(page).to have_text("error") @@ -268,7 +268,7 @@ expect { click_on "Submit" - }.to change(CaseContact.where(status: "active"), :count).by(1).and change(AdditionalExpense, :count).by(1) + }.to change(CaseContact.active, :count).by(1).and change(AdditionalExpense, :count).by(1) expect(page).not_to have_text("error") visit edit_case_contact_path(casa_case.reload.case_contacts.last) @@ -281,14 +281,14 @@ expect { click_on "Submit" - }.to change(CaseContact.where(status: "active"), :count).by(0).and change(AdditionalExpense, :count).by(0) + }.to change(CaseContact.active, :count).by(0).and change(AdditionalExpense, :count).by(0) expect(page).to have_text("error") all("input[name*='[additional_expenses_attributes]'][name$='[other_expenses_describe]']").last.fill_in(with: "2nd meal") expect { click_on "Submit" - }.to change(CaseContact.where(status: "active"), :count).by(0).and change(AdditionalExpense, :count).by(1) + }.to change(CaseContact.active, :count).by(0).and change(AdditionalExpense, :count).by(1) expect(page).not_to have_text("error") end end diff --git a/spec/system/case_contacts/new_spec.rb b/spec/system/case_contacts/new_spec.rb index 8da0e048b9..75fc4ea7d4 100644 --- a/spec/system/case_contacts/new_spec.rb +++ b/spec/system/case_contacts/new_spec.rb @@ -41,7 +41,7 @@ expect { click_on "Submit" - }.to change(CaseContact.where(status: "active"), :count).by(1) + }.to change(CaseContact.active, :count).by(1) contact = CaseContact.last expect(contact.casa_case_id).to eq casa_case.id expect(contact.contact_types.map(&:name)).to include("School", "Therapist") @@ -137,7 +137,7 @@ expect { click_on "Submit" - }.to change(CaseContact.where(status: "active"), :count).by(1) + }.to change(CaseContact.active, :count).by(1) expect(CaseContact.first.notes).to eq "" end @@ -154,7 +154,7 @@ expect { click_on "Submit" - }.to change(CaseContact.where(status: "active"), :count).by(1) + }.to change(CaseContact.active, :count).by(1) expect(CaseContact.first.notes).to eq "This is the note" end