Race Condition Check: spec/system/case_contacts/ updates

Resolves rubyforgood#6701

When github's free CI servers are under heavy load, a race condition
between the page loading and checking the database causes tests to flake.
This is caused by a system test inputting data into a form then
immediately checking the database without waiting for the form to finish submitting.

For every database check in the system files, this ensures it's preceded
by a capybara matcher with automatic waiting, such as
checking for assertions after the page loads before
checking for DB values, or replaced the database
check with a check for something to appear on the page.

I also took the opportunity to refactor some small things to save
up test setup time:

- build factories instead of creating as much as possible
- require `:js` only when necessary
- remove unused variables and factories
- keep necessary factories close to where they are used to avoid
  creating extra factories when not needed
- update Date string to Date objects (these were flaky locally),
  similar to rubyforgood#6764

Author: stefannibrasil

spec/system/case_contacts/additional_expenses_spec.rb

@@ -1,6 +1,6 @@
 require "rails_helper"
 
-RSpec.describe "CaseContact AdditionalExpenses Form", :flipper, :js, type: :system do
+RSpec.describe "CaseContact AdditionalExpenses Form", :flipper, type: :system do
   subject do
     visit new_case_contact_path(casa_case)
     fill_in_contact_details(contact_types: [contact_type.name])
@@ -28,7 +28,7 @@
     expect(page).to have_no_button("Add Another Expense", visible: :all)
   end
 
-  it "is not shown until Reimbursement is checked and Add Another Expense clicked" do
+  it "is not shown until Reimbursement is checked and Add Another Expense clicked", :js do
     sign_in volunteer
     visit new_case_contact_path casa_case
     fill_in_contact_details
@@ -42,64 +42,64 @@
     expect(page).to have_field(class: "expense-amount-input")
   end
 
-  it "does not submit values if reimbursement is cancelled (unchecked)" do
+  it "does not submit values if reimbursement is cancelled (unchecked)", :js do
     subject
 
     click_on "Add Another Expense"
     fill_expense_fields 5.34, "Lunch"
     uncheck "Request travel or other reimbursement"
 
-    expect { click_on "Submit" }
-      .to change(CaseContact.active, :count).by(1)
+    click_on "Submit"
+    expect(page).to have_text("Case contact successfully created")
 
+    expect(CaseContact.active.count).to eq(1)
     case_contact = CaseContact.active.last
     expect(case_contact.additional_expenses).to be_empty
     expect(case_contact.miles_driven).to be_zero
     expect(case_contact.want_driving_reimbursement).to be false
   end
 
-  it "can remove an expense" do
+  it "can remove an expense", :js do
     subject
     fill_in_contact_details
     check "Request travel or other reimbursement"
     fill_in "case_contact_miles_driven", with: 50
     fill_in "case_contact_volunteer_address", with: "123 Params St"
 
-    expect {
-      click_on "Add Another Expense"
-      fill_expense_fields 1.50, "1st meal"
-      click_on "Add Another Expense"
-      fill_expense_fields 2.50, "2nd meal"
-      click_on "Add Another Expense"
-      fill_expense_fields 2.00, "3rd meal"
+    click_on "Add Another Expense"
+    fill_expense_fields 1.50, "1st meal"
+    click_on "Add Another Expense"
+    fill_expense_fields 2.50, "2nd meal"
+    click_on "Add Another Expense"
+    fill_expense_fields 2.00, "3rd meal"
 
-      within "#contact-form-expenses" do
-        click_on "Delete", match: :first
-      end
+    within "#contact-form-expenses" do
+      click_on "Delete", match: :first
+    end
 
-      expect(page).to have_field(class: "expense-amount-input", count: 2)
+    expect(page).to have_field(class: "expense-amount-input", count: 2)
 
-      click_on "Submit"
-      expect(page).to have_text("Case contact successfully created")
-    }
-      .to change(CaseContact.active, :count).by(1)
-      .and change(AdditionalExpense, :count).by(2)
+    click_on "Submit"
+    expect(page).to have_text("Case contact successfully created")
 
     case_contact = CaseContact.active.last
     expect(case_contact.additional_expenses.size).to eq(2)
+    expect(CaseContact.count).to eq(1)
+    expect(AdditionalExpense.count).to eq(2)
   end
 
-  it "requires a description for each additional expense" do
+  it "requires a description for each additional expense", :js do
     subject
 
     click_on "Add Another Expense"
     fill_expense_fields 5.34, nil
 
-    expect { click_on "Submit" }
-      .to not_change(CaseContact.active, :count)
-      .and not_change(AdditionalExpense, :count)
+    click_on "Submit"
 
     expect(page).to have_text("Other Expense Details can't be blank")
+
+    expect(CaseContact.active.count).to eq(0)
+    expect(AdditionalExpense.count).to eq(1)
   end
 
   context "when editing existing case contact expenses" do
@@ -125,38 +125,39 @@
       expect(page).to have_button "Add Another Expense"
     end
 
-    it "allows removing expenses" do
+    it "allows removing expenses", :js do
       subject
 
       expect(page).to have_css(".expense-amount-input", count: 2)
       expect(page).to have_css(".expense-describe-input", count: 2)
 
-      expect {
-        within "#contact-form-expenses" do
-          click_on "Delete", match: :first
-        end
+      within "#contact-form-expenses" do
+        click_on "Delete", match: :first
+      end
+
+      expect(page).to have_css(".expense-amount-input", count: 1)
+      expect(page).to have_css(".expense-describe-input", count: 1)
 
-        expect(page).to have_css(".expense-amount-input", count: 1)
-        expect(page).to have_css(".expense-describe-input", count: 1)
+      click_on "Submit"
 
-        click_on "Submit"
-      }
-        .to not_change(CaseContact.active, :count)
-        .and change(AdditionalExpense, :count).by(-1)
+      expect(page).to have_text(/Case contact .* was successfully updated./)
 
+      expect(CaseContact.active.count).to eq(1)
+      expect(AdditionalExpense.count).to eq(1)
       expect(case_contact.reload.additional_expenses.size).to eq(1)
     end
 
-    it "can add an expense" do
+    it "can add an expense", :js do
       subject
 
-      expect {
-        click_on "Add Another Expense"
-        fill_expense_fields 11.50, "Gas"
-        click_on "Submit"
-      }
-        .to change(AdditionalExpense, :count).by(1)
+      click_on "Add Another Expense"
+      fill_expense_fields 11.50, "Gas"
+      click_on "Submit"
+
+      expect(page).to have_text(/Case contact .* was successfully updated./)
+
       expect(case_contact.reload.additional_expenses.size).to eq(3)
+      expect(AdditionalExpense.count).to eq(3)
     end
   end
 end

spec/system/case_contacts/contact_topic_answers_spec.rb

@@ -1,6 +1,6 @@
 require "rails_helper"
 
-RSpec.describe "CaseContact form ContactTopicAnswers and notes", :js, type: :system do
+RSpec.describe "CaseContact form ContactTopicAnswers and notes", type: :system do
   subject do
     sign_in user
     visit new_case_contact_path(casa_case)
@@ -12,8 +12,7 @@
   let(:user) { volunteer }
 
   let!(:contact_type) { create :contact_type, casa_org: }
-  let(:topic_count) { 2 }
-  let!(:contact_topics) { create_list :contact_topic, topic_count, casa_org: }
+  let!(:contact_topics) { create_list :contact_topic, 2, casa_org: }
   let(:contact_topic_questions) { contact_topics.map(&:question) }
   let(:select_options) { contact_topic_questions + ["Select a discussion topic"] }
 
@@ -36,7 +35,7 @@ def notes_section
     expect(notes_section).to have_select(class: topic_select_class, with_options: contact_topic_questions)
   end
 
-  it "adds contact answers for the topics" do
+  it "adds contact answers for the topics", :js do
     subject
     fill_in_contact_details(contact_types: [contact_type.name])
 
@@ -65,22 +64,19 @@ def notes_section
     subject
     fill_in_contact_details(contact_types: [contact_type.name])
 
-    expect do
-      using_wait_time 6 do # autosave debounce may be longer than capybara's wait time
-        answer_topic contact_topics.first.question, "First discussion topic answer."
-        within notes_section do
-          expect(page).to have_text "Saved"  # autosave success alert
-          expect(page).to have_no_text "Saved" # wait for clearing of alert
-        end
-        answer_topic contact_topics.first.question, "Changing the first topic answer."
-        within notes_section { expect(page).to have_text "Saved" }
-      end
+    answer_topic contact_topics.first.question, "First discussion topic answer."
+    within notes_section do
+      expect(page).to have_text "Saved"  # autosave success alert
+      expect(page).to have_no_text "Saved" # wait for clearing of alert
+    end
+    answer_topic contact_topics.first.question, "Changing the first topic answer."
+    within notes_section { expect(page).to have_text "Saved" }
 
-      click_on "Submit"
-      expect(page).to have_content("Case contact successfully created.")
-    end.to change(CaseContact.active, :count).by(1)
-      .and change(ContactTopicAnswer, :count).by(0) # answer already exists on page load
+    click_on "Submit"
+    expect(page).to have_content("Case contact successfully created.")
 
+    expect(CaseContact.active.count).to eq(1)
+    expect(ContactTopicAnswer.count).to eq(1)
     case_contact = CaseContact.active.last
     created_answer = ContactTopicAnswer.last
     expect(created_answer.contact_topic).to eq(contact_topics.first)
@@ -89,7 +85,7 @@ def notes_section
     expect(case_contact.contact_topic_answers).to include created_answer
   end
 
-  it "prevents adding more answers than topics" do
+  it "prevents adding more answers than topics", :js do
     subject
 
     (contact_topics.size - 1).times do
@@ -99,7 +95,7 @@ def notes_section
     expect(notes_section).to have_button("Add Another Discussion Topic", disabled: true)
   end
 
-  it "disables contact topics that are already selected" do
+  it "disables contact topics that are already selected", :js do
     subject
 
     topic_one_question = contact_topics.first.question
@@ -115,42 +111,40 @@ def notes_section
   context "when casa org has no contact topics" do
     let(:contact_topics) { [] }
 
-    it "displays a field for contact.notes" do
+    it "displays a field for contact.notes", :js do
       subject
       expect(page).to have_no_button "Add Another Discussion Topic"
       expect(notes_section).to have_field "Additional Notes"
 
       fill_in_contact_details
       fill_in "Additional Notes", with: "This is a note."
 
-      expect do
-        click_on "Submit"
-        expect(page).to have_content("Case contact successfully created.")
-      end.to change(CaseContact.active, :count).by(1)
+      click_on "Submit"
+      expect(page).to have_content("Case contact successfully created.")
 
       contact = CaseContact.active.last
+      expect(CaseContact.active.count).to eq(1)
       expect(contact.contact_topic_answers).to be_empty
       expect(contact.notes).to eq "This is a note."
     end
 
-    it "saves 'Additional Notes' answer as contact.notes" do
+    it "saves 'Additional Notes' answer as contact.notes", :js do
       subject
       fill_in_contact_details(contact_types: [contact_type.name])
 
       fill_in "Additional Notes", with: "This is a fake a topic answer."
 
-      expect do
-        click_on "Submit"
-        expect(page).to have_text("Case contact successfully created")
-      end.to change(CaseContact.active, :count).by(1)
+      click_on "Submit"
+      expect(page).to have_text("Case contact successfully created")
 
       contact = CaseContact.active.last
+      expect(CaseContact.active.count).to eq(1)
       expect(contact.contact_topic_answers).to be_empty
       expect(contact.notes).to eq "This is a fake a topic answer."
     end
   end
 
-  context "when editing existing an case contact" do
+  context "when editing an existing case contact" do
     subject do
       sign_in user
       visit edit_case_contact_path(case_contact)
@@ -181,46 +175,46 @@ def notes_section
         )
       end
 
-      it "can remove an existing answer" do
+      it "can remove an existing answer", :js do
         subject
         fill_in_contact_details
 
         expect(notes_section).to have_select(class: topic_select_class, count: 2)
 
-        expect {
-          accept_confirm do
-            notes_section.find_button(text: "Delete", match: :first).click
-          end
+        accept_confirm do
+          notes_section.find_button(text: "Delete", match: :first).click
+        end
 
-          expect(notes_section).to have_select(class: topic_select_class, count: 1, visible: :all)
+        expect(notes_section).to have_select(class: topic_select_class, count: 1, visible: :all)
 
-          click_on "Submit"
-          expect(page).to have_content(/Case contact .* was successfully updated./)
-        }
-          .to change(ContactTopicAnswer, :count).by(-1)
+        click_on "Submit"
+        expect(page).to have_content(/Case contact .* was successfully updated./)
 
         case_contact.reload
+        expect(ContactTopicAnswer.count).to eq(1)
         expect(case_contact.contact_topic_answers.size).to eq(1)
       end
     end
 
-    it "autosaves form with answer inputs" do
-      expect { subject }.to change(CaseContact, :count).by(1)
-      case_contact = CaseContact.last
-      expect(case_contact.casa_case).to eq casa_case
+    it "autosaves form with answer inputs", :js do
+      subject
+
       fill_in_contact_details(
         contact_made: false, medium: "In Person", occurred_on: 1.day.ago.to_date, hours: 1, minutes: 5
       )
 
-      expect {
-        click_on "Add Another Discussion Topic"
-        answer_topic contact_topics.first.question, "Topic One answer."
-        within autosave_alert_div do
-          find(autosave_alert_css, text: autosave_alert_text, wait: 3)
-        end
-      }
-        .to change(ContactTopicAnswer, :count).by(1)
-      case_contact.reload
+      click_on "Add Another Discussion Topic"
+      answer_topic contact_topics.first.question, "Topic One answer."
+      within autosave_alert_div do
+        find(autosave_alert_css, text: autosave_alert_text, wait: 3)
+      end
+
+      expect(page).to have_content("Editing Existing Case Contact")
+
+      expect(CaseContact.count).to eq(1)
+      case_contact = CaseContact.last
+      expect(case_contact.casa_case).to eq casa_case
+      expect(ContactTopicAnswer.count).to eq(1)
       expect(case_contact.contact_topic_answers.size).to eq(1)
       expect(case_contact.contact_topic_answers.last.value).to eq "Topic One answer."
 

spec/system/case_contacts/drafts_spec.rb

@@ -1,15 +1,15 @@
 require "rails_helper"
 
-RSpec.describe "case_contacts/drafts", :js, type: :system do
-  let(:organization) { create(:casa_org) }
-  let(:admin) { create(:casa_admin, casa_org: organization) }
+RSpec.describe "case_contacts/drafts", type: :system do
+  let(:organization) { build(:casa_org) }
+  let(:admin) { build(:casa_admin, casa_org: organization) }
 
   context "with case contacts" do
-    let!(:casa_case) { create(:casa_case, casa_org: organization) }
-    let!(:other_org_case) { create(:case_contact, notes: "NOTE_A") }
-    let!(:past_contact) { create(:case_contact, casa_case: casa_case, occurred_at: 3.weeks.ago, notes: "NOTE_B") }
+    let!(:casa_case) { build(:casa_case, casa_org: organization) }
+    let!(:other_org_case) { build(:case_contact, notes: "NOTE_A") }
+    let!(:past_contact) { build(:case_contact, casa_case: casa_case, occurred_at: 3.weeks.ago, notes: "NOTE_B") }
     let!(:past_contact_draft) { create(:case_contact, :started_status, casa_case: casa_case, occurred_at: 3.weeks.ago, notes: "NOTE_C") }
-    let!(:recent_contact) { create(:case_contact, casa_case: casa_case, occurred_at: 3.days.ago, notes: "NOTE_D") }
+    let!(:recent_contact) { build(:case_contact, casa_case: casa_case, occurred_at: 3.days.ago, notes: "NOTE_D") }
     let!(:recent_contact_draft) { create(:case_contact, :started_status, casa_case: casa_case, occurred_at: 3.days.ago, notes: "NOTE_E") }
 
     it "shows only same orgs drafts" do