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

Add doctrine filters applicator #14246

Merged
merged 1 commit into from Jul 12, 2019

Conversation

@sarjon
Copy link
Member

commented Jun 17, 2019

Questions Answers
Branch? develop
Description? When implementing query builders for grid, developers often copy-paste that applies filters on query. This PR attempts to abstract this behavior.
Type? improvement
Category? CO
BC breaks? no
Deprecations? no
Fixed ticket? n/a
How to test? Filters for webservice key list should be working as before.

This change is Reviewable

@sarjon sarjon requested a review from PrestaShop/prestashop-core-developers as a code owner Jun 17, 2019

@sarjon

This comment has been minimized.

Copy link
Member Author

commented Jun 17, 2019

@matks ready for review.

@@ -33,15 +34,6 @@ services:
- "@=service('prestashop.adapter.legacy.context').getContext().shop.id"
public: true

prestashop.core.grid.query_builder.webservice:

This comment has been minimized.

Copy link
@sarjon

sarjon Jun 17, 2019

Author Member

Removes duplicate service definition.

@matks matks added the migration label Jun 24, 2019

@matks

matks approved these changes Jul 12, 2019

@matks

This comment has been minimized.

Copy link
Contributor

commented Jul 12, 2019

It's great 👍

Now I'm thinking : how can we test it ? And how can we test, in a more global manner, Query Builders ?

Some Behat tests that would perform pre-configured queries ? 🤔

@matks

This comment has been minimized.

Copy link
Contributor

commented Jul 12, 2019

TODO after merge: apply this everywhere

@matks matks added this to the 1.7.7.0 milestone Jul 12, 2019

@sarahdib sarahdib added QA ✔️ and removed waiting for QA labels Jul 12, 2019

@PierreRambaud PierreRambaud merged commit 2212366 into PrestaShop:develop Jul 12, 2019

2 checks passed

PrettyCI Code formatting
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@PierreRambaud

This comment has been minimized.

Copy link
Contributor

commented Jul 12, 2019

Thanks @sarjon

@sarjon sarjon deleted the sarjon:filters-applicator branch Jul 15, 2019

mbadrani added a commit to mbadrani/PrestaShop that referenced this pull request Jul 18, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.