-
-
Notifications
You must be signed in to change notification settings - Fork 572
Fix request fulfilled issue #5334
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix request fulfilled issue #5334
Conversation
| flash[:notice] = "Request started" | ||
| redirect_to new_distribution_path(request_id: request.id) | ||
| rescue ActiveRecord::RecordInvalid | ||
| flash[:alert] = "Request has already been fulfilled" |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
|
@GreenGogh47 this looks good now, but can you please fix the conflicts? |
|
Passes technical review, over to @awwaiid for functional. |
|
Passed Functional Testing:
@ruestitch This looks good to me! |
|
@GreenGogh47: Your PR |


Resolves #5326
Description
Originally it was possible to change the status of an already fulfilled request to "started".
This PR adds:
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
How Has This Been Tested?
Rspec tests have been added, and I've tested it manually.
