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
Technical debt : review PrestaShop\PrestaShop\Core\Grid\Search\SearchCriteriaInterface #29131
Comments
Hello @MeKeyCool, Thank you for your suggestion. The Product Team will take it into consideration for future developments. Thanks 😊 |
This issue is dedicated to developers. |
So I think it depends on one question - if we accept to remove the ability of having unlimited results in grids. Because that is exactly what we would achieve according to your suggestion. |
I don't think this requires a refacto 🤔 The interface represents the generic contract needed to perform a search on a grid so it has generic methods. And as @zuk3975 said nothing allows us to state that the limit should always be 10 and the offset 0. For the offset I agree it would make sense, but the default limit depends on use cases and we could have a grid with no limit by default. Besides, I think you're confusing the default values (which are handled and implemented by the Filters class) from the interface itself which has no notion of default values so far. The interface represents the required parameters at any moment in a grid life, whether it's the initial display that returns the default values or an intermediate state during pagination with specific values it doesn't matter for the interface, it will always need the same methods anyway. |
Sorry if I haven't been clear with my description. As you said, interface is a contract. Further more, defining some specific values for attributes that should be interpreted as a different state make things heavy PrestaShop/src/Core/Grid/Search/SearchCriteriaInterface.php Lines 49 to 52 in ad181f7
If the attribute is optional, I recommend to use a dedicated "Option" type as it is done in some other languages
Then allowing an "Optional" value for some attributes induces the question of what should I do if the Option value is None ?
|
Prerequisites
Hi ! This issue is opened to discuss about
PrestaShop\PrestaShop\Core\Grid\Search\SearchCriteriaInterface
.I think some refactoring is required but I may need some enlightenments
Is your feature request related to a problem?
Fixing #26218, in https://github.com/PrestaShop/PrestaShop/pull/28782/files#diff-7930ead1cb41932b3be50da6003f2f0159b4ec40ef0fb0fa70f28b4c930db401, I encountered some weird behaviors I had to manage and I think the
PrestaShop\PrestaShop\Core\Grid\Search\SearchCriteriaInterface
should be refactored.Describe the solution you'd like
I would like that offset is defined as positive integer with default value at 0 (what would mean "disabling offset" ?)
Same for limit : positive value with default at 10.
IMHO,
PrestaShop\PrestaShop\Core\Search\Filters::LIST_LIMIT
constantshould be defined in
PrestaShop\PrestaShop\Core\Grid\Search\SearchCriteriaInterface
as all default values.It is just few ideas. Final roadmap should be discussed with @PrestaShop/prestashop-maintainers .
Do you plan to work on this feature?
The text was updated successfully, but these errors were encountered: