CaseContact status enum changes (rubyforgood#5975)

* explicit enum definition

* active or notes method

* use enum methods for step checks

* validate status in enum

* use status enum in factories

* use .statuses in form controller

* use .active scope

* use .not_active scope

* add enum default

* extra space

* better comments for factory

* use traits in specs

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
 

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

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) }

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") }

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,34 +41,38 @@
     end
 
     trait :started_status do
+      started # enum trait
+
       casa_case { nil }
       draft_case_ids { [] }
       medium_type { nil }
       occurred_at { nil }
       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|

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

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

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,15 +260,15 @@
 
       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]']").first.fill_in(with: "1 meal")
 
       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

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