Skip to content

Conversation

GreenGogh47
Copy link

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

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"
2 participants