Race condition check: invite volunteers system spec

[stefannibrasil]

Related #6698

• removes unused setup
• removes test that does not have any user interaction and is already covered by a unit test
• checks for what the user sees in the page when inviting volunteers, especially before checking the DB

[FireLemons]

After reviewing the test, it turns out that most of the scenarios here are already being tested in the volunteers and invite request specs, so the system tests were redundant.

The request tests are unit tests

see: https://www.geeksforgeeks.org/software-testing/difference-between-unit-testing-and-system-testing/

[stefannibrasil]

Yes, that's correct. The only difference between what's being tested in the system tests vs the request tests was filling out the form vs submitting the params. So reviewing the system test showed that unit tests were being performed more than UI interaction. And the UI interaction that was left could be extracted to a view test.

I think using a view spec to verify that the form fields are rendered and relying on the request tests for validity is a good alternative, though. Otherwise, we're having duplicated logic being tested in several places.

[Copilot]

Pull request overview

This PR reduces CI load and avoids race-condition-prone assertions by removing redundant volunteer invitation system specs and relocating coverage into cheaper request and view specs, aligning with issue #6698’s guidance to avoid backend-only expectations in system tests.

Changes:

• Removed spec/system/volunteers/invite_spec.rb (system-level invite/resend/acceptance scenarios).
• Added/adjusted view specs to verify volunteer form fields render and that the resend-invitation UI is conditionally shown/hidden.
• Refactored spec/requests/volunteers_spec.rb to cover invite/SMS/resend behaviors with less factory setup and more explicit redirect/flash assertions.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

File	Description
spec/views/volunteers/new.html.erb_spec.rb	Adds a view spec asserting key fields render on the new volunteer form.
spec/views/volunteers/edit.html.erb_spec.rb	Adds a view spec ensuring the resend-invitation UI is hidden once an invitation is accepted.
spec/system/volunteers/invite_spec.rb	Removes expensive system coverage for invite/resend/accept flows.
spec/requests/volunteers_spec.rb	Expands request coverage for volunteer creation/invitation, SMS behavior, unauthorized access behavior, and resend invitation emails.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[stefannibrasil]

Let me know if you have other ideas. This looks more relevant in this context 👀

[stefannibrasil]

@FireLemons @compwron I addressed copilot's comments in 9deee4c and added a new view test in fa8da1c

Let me know if you prefer to have the system test still. After reviewing it carefully, it shows that they can be replaced between the request and view specs to ensure the fields are displayed and the emails are sent, which is primarily what the system test was doing. Thanks for your reviews!

[stefannibrasil]

This test is not an integration one because there is no UI interaction. It's also covered by this unit test already: https://github.com/rubyforgood/casa/blob/main/spec/models/volunteer_spec.rb#L388

[compwron]

we could argue about tests forever but I am ok with merging this

[stefannibrasil]

True! I did keep the PR more focused though. Thanks for reviewing it!