Skip to content

Conversation

@GreenGogh47
Copy link
Contributor

Resolves #5326

Description

Originally it was possible to change the status of an already fulfilled request to "started".
This PR adds:

  • validation to the requests class to check it's already fulfilled before allowing a change
  • updates to the requests controller to fail gracefully if attempting to change a fulfilled request
  • a migration file to correct any existing requests that are "started", yet associated with a "complete" distribution
  • passing tests and rubocop

However, there are 5 failing tests that are unrelated to this pr. This is consistent with the current main branch. I just wanted to bring it to your attention.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Rspec tests have been added, and I've tested it manually.
image

flash[:notice] = "Request started"
redirect_to new_distribution_path(request_id: request.id)
rescue ActiveRecord::RecordInvalid
flash[:alert] = "Request has already been fulfilled"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're going to take this path, you should be reading the actual error and displaying it, rather than assuming what the error could be.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's been updated to show the actual error now
flash[:alert] = request.errors.full_messages.to_sentence

@dorner
Copy link
Collaborator

dorner commented Sep 26, 2025

@GreenGogh47 this looks good now, but can you please fix the conflicts?

@GreenGogh47 GreenGogh47 requested a review from dorner October 3, 2025 17:08
@dorner
Copy link
Collaborator

dorner commented Oct 3, 2025

Passes technical review, over to @awwaiid for functional.

@dorner dorner requested a review from awwaiid October 3, 2025 19:22
@janeewheatley
Copy link
Collaborator

Passed Functional Testing:

  1. Click on Requests
  2. View a request (Pawnee Homeless Shelter) in 2 different window tabs
  3. Click 'Fulfill request' button on only 1 of the requests
  4. Click 'Save' button on that same request
  5. Make the Distribution Complete
image
  1. Now, go to the other tab that also is still viewing the request (Pawnee Homeless Shelter0
  2. Click on the 'Fulfill request' button
  3. We now correctly see an error that says Status cannot be changed once fulfilled
image

@ruestitch This looks good to me!

@awwaiid awwaiid merged commit 819e933 into rubyforgood:main Nov 30, 2025
11 checks passed
@github-actions
Copy link
Contributor

@GreenGogh47: Your PR Fix request fulfilled issue is part of today's Human Essentials production release: 2025.11.30.
Thank you very much for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"started" requests that should be "fulfilled"

4 participants