Skip to content
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.

Sign up for GitHub

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

OBPIH-5739 Fix filtering for handling requirements in products list page #4931

Merged
merged 1 commit into from
Nov 8, 2024
Merged

OBPIH-5739 Fix filtering for handling requirements in products list page #4931

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 13 additions & 11 deletions grails-app/services/org/pih/warehouse/product/ProductService.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,17 +362,19 @@ class ProductService {
}

if (!handlingRequirements.empty) {
if (handlingRequirements.contains(ProductField.COLD_CHAIN)) {
eq("coldChain", true)
}
if (handlingRequirements.contains(ProductField.CONTROLLED_SUBSTANCE)) {
eq("controlledSubstance", true)
}
if (handlingRequirements.contains(ProductField.HAZARDOUS_MATERIAL)) {
eq("hazardousMaterial", true)
}
if (handlingRequirements.contains(ProductField.RECONDITIONED)) {
eq("reconditioned", true)
or {
if (handlingRequirements.contains(ProductField.COLD_CHAIN)) {
eq("coldChain", true)
}
if (handlingRequirements.contains(ProductField.CONTROLLED_SUBSTANCE)) {
eq("controlledSubstance", true)
}
if (handlingRequirements.contains(ProductField.HAZARDOUS_MATERIAL)) {
eq("hazardousMaterial", true)
}
if (handlingRequirements.contains(ProductField.RECONDITIONED)) {
eq("reconditioned", true)
}
Comment on lines +365 to +377
Copy link
Collaborator

Choose a reason for hiding this comment

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

What do you think about doing it more generic? I mean that now when we are adding a new handling requirement we have to add it to the product domain, to the ProductField enum, and to this place to make the filtering work. So maybe we can add a function for listing all of the handling requirements - something similar to what we have in the RequisitionStatus enum (listOubtoundOptions, listOutboundOptionsWhenApprovalRequired, listRequestOptionsWhenApprovalRequired, etc.). Then we can do a loop over this list and if the current requirement from the loop is in handling requirement we can do eq(elemenetFromLoop, true), so doing it this way we don't have to handle adding new requirements in this place. (I believe we can do a loop here)

cc @awalkowiak

Copy link
Collaborator

Choose a reason for hiding this comment

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

(my thought - you have valid point, but imho this is fine for now, because I think there are no plans to add new handling requirements 🤔)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I had the same feeling, initially I considered doing this with a generic method but then I realized that we wouldn’t achieve much by that, since those are only 4 lines of the code with a low risk to expand

}
}

Expand Down
Loading