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.

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-5832 Rename pagination and filter command and implement validat… #4495

Merged
merged 2 commits into from
Feb 8, 2024

Conversation

kchelstowski
Copy link
Collaborator

…eable interface

The PR is addressing @jmiranda 's suggestions: #4438 (comment)
Additionally, I've added implements Validateable for those commands, so we do not get warnings in the console, even though we do not need any validation for the filter params now.

Comment on lines 51 to 53
if (params.hasErrors()) {
throw new ValidationException("Invalid params", params.errors)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thinking out loud...
My lazy butt is wondering if we should/could/want to create validationInterceptor that handles checking if a command object has errors and throw an exception instead of writing this logic everywhere ourselves.

Copy link
Member

Choose a reason for hiding this comment

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

I thought the same thing with respect to request.exception (maybe we could handle both cases in the same interceptor). However, something doesn't feel right about this approach so I'd prefer to explore it as part of a separate spike.

Copy link
Member

@jmiranda jmiranda left a comment

Choose a reason for hiding this comment

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

minor changes

// Store added aliases to avoid duplicate alias exceptions for product and supplier
// This could happen when params.searchTerm and e.g. sort by productCode/productName is applied
Set<String> usedAliases = new HashSet<>()

return ProductSupplier.createCriteria().list(max: params.max, offset: params.offset) {
return ProductSupplier.createCriteria().list(params.getPaginationParams()) {
Copy link
Member

Choose a reason for hiding this comment

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

command.paginationParams

@@ -46,12 +47,15 @@ class ProductSupplierService {
ProductSupplierDataService productSupplierGormService


List<ProductSupplier> getProductSuppliers(ProductSupplierListParams params) {
List<ProductSupplier> getProductSuppliers(ProductSupplierFilterCommand params) {
Copy link
Member

Choose a reason for hiding this comment

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

ProductSupplierFilterCommand command

Comment on lines 51 to 53
if (params.hasErrors()) {
throw new ValidationException("Invalid params", params.errors)
}
Copy link
Member

Choose a reason for hiding this comment

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

I thought the same thing with respect to request.exception (maybe we could handle both cases in the same interceptor). However, something doesn't feel right about this approach so I'd prefer to explore it as part of a separate spike.

@awalkowiak awalkowiak merged commit bed46b4 into feature/product-supplier-list-redesign Feb 8, 2024
@awalkowiak awalkowiak deleted the OBPIH-5832-fix branch February 8, 2024 11:41
awalkowiak pushed a commit that referenced this pull request Mar 12, 2024
#4495)

* OBPIH-5832 Rename pagination and filter command and implement validateable interface

* OBPIH-5832 Rename params to command
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.

None yet

5 participants