5223 error message on nonvisible item request #5405
Conversation
| end | ||
|
|
||
| def validate_visible_items!(child_ids) | ||
| invisible_item_requests = current_partner |
There was a problem hiding this comment.
You can reuse most if not all of the query that was created earlier. You can pass the query object into this method and just add the changes you need onto it.
There was a problem hiding this comment.
Having said that, all of this is starting to seem very heavy for controller logic. I think we should move most of this to the service class.
| family_requests_attributes = build_family_requests_attributes(params) | ||
| begin | ||
| family_requests_attributes = build_family_requests_attributes(params) | ||
| rescue ArgumentError => e |
There was a problem hiding this comment.
ArgumentError is a built-in Ruby exception that shouldn't be used for business logic validation. Probably best to create your own error class.
There was a problem hiding this comment.
Makes sense. Where would be the best place for the error class?
The file that looks most like is app/events/inventory_error.rb, so something like app/events/item_visibility_error.rb?
There was a problem hiding this comment.
If we're moving this to the service class, you can define the error inside the class itself.
Resolves #5223
Description
The error message related to partners requesting
Itemswherevisible_to_partnersisfalsenow specifies which item and which children are causing the error.NOTE: There is a small change unrelated to the specific issue. In the
app/views/layouts/partners/application.html.erbfile, there were two overlapping "X" icons to dismiss the error message. I've removed one of them so now there's just one icon.Type of change
How Has This Been Tested?
I've added a test to the
spec/requests/partners/family_requests_requests_spec.rbfile that expects the format of the error message to include details about a non-visible item, and the child requesting it.Screenshots
Before:
After: