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

Improve grid data providers #9402

Merged
merged 16 commits into from Sep 4, 2018

Conversation

Projects
None yet
7 participants
@sarjon
Member

sarjon commented Aug 2, 2018

Questions Answers
Branch? develop
Description? This PR aims to: 1. make pagination & sorting optional in SearchCriteria. This allows to use same data providers to export data when needed. 2. Introduces new hook to allow grid query modifications for modules.
Type? improvement
Category? CO
BC breaks? no
Deprecations? no
Fixed ticket? n/a
How to test? Pages with grid should work as before.

This change is Reviewable

@sarjon

This comment has been minimized.

Show comment
Hide comment
@sarjon
Member

sarjon commented Aug 2, 2018

@mickaelandrieu

maybe use an abstract class instead of a trait.

@mickaelandrieu

This comment has been minimized.

Show comment
Hide comment
@mickaelandrieu

mickaelandrieu Aug 14, 2018

Contributor

Ping @eternoendless, I need this to be merged in order to try to break my own pull request (because for now it's impossible as people don't have access to the query builder, so they can't add both positional and named parameters in one query)

Contributor

mickaelandrieu commented Aug 14, 2018

Ping @eternoendless, I need this to be merged in order to try to break my own pull request (because for now it's impossible as people don't have access to the query builder, so they can't add both positional and named parameters in one query)

@sarjon

This comment has been minimized.

Show comment
Hide comment
@sarjon

sarjon Aug 14, 2018

Member

@eternoendless @mickaelandrieu please review. 🙏

Member

sarjon commented Aug 14, 2018

@eternoendless @mickaelandrieu please review. 🙏

@sarjon

This comment has been minimized.

Show comment
Hide comment
@sarjon

sarjon Aug 17, 2018

Member

is there anything still missing for this PR? ^^

Member

sarjon commented Aug 17, 2018

is there anything still missing for this PR? ^^

@mickaelandrieu

This comment has been minimized.

Show comment
Hide comment
@mickaelandrieu

mickaelandrieu Aug 17, 2018

Contributor

is there anything still missing for this PR? ^^

It's up to @eternoendless to decide as we will apply his comments everywhere :)

Contributor

mickaelandrieu commented Aug 17, 2018

is there anything still missing for this PR? ^^

It's up to @eternoendless to decide as we will apply his comments everywhere :)

@sarjon

This comment has been minimized.

Show comment
Hide comment
@sarjon

sarjon Aug 21, 2018

Member

rebased.

Member

sarjon commented Aug 21, 2018

rebased.

@sarjon sarjon referenced this pull request Aug 25, 2018

Open

Checklist with Grid component improvements #68

0 of 12 tasks complete
@mickaelandrieu

This comment has been minimized.

Show comment
Hide comment
@mickaelandrieu

mickaelandrieu Aug 27, 2018

Contributor

Hello @sarjon, this one is the next and last one before freeze imo 👍

Contributor

mickaelandrieu commented Aug 27, 2018

Hello @sarjon, this one is the next and last one before freeze imo 👍

@sarjon

This comment has been minimized.

Show comment
Hide comment
@sarjon

sarjon Aug 28, 2018

Member

rebased 👍

Member

sarjon commented Aug 28, 2018

rebased 👍

@sarjon

This comment has been minimized.

Show comment
Hide comment
@sarjon

sarjon Sep 4, 2018

Member

rebased (again). 👍

Member

sarjon commented Sep 4, 2018

rebased (again). 👍

@mickaelandrieu mickaelandrieu added this to the 1.7.5.0 milestone Sep 4, 2018

@marionf marionf self-assigned this Sep 4, 2018

@marionf

This comment has been minimized.

Show comment
Hide comment
@marionf

marionf Sep 4, 2018

Contributor

Hello @sarjon

I have an exception in the emails page with this PR

capture d ecran_248

Contributor

marionf commented Sep 4, 2018

Hello @sarjon

I have an exception in the emails page with this PR

capture d ecran_248

@marionf marionf removed their assignment Sep 4, 2018

@sarjon

This comment has been minimized.

Show comment
Hide comment
@sarjon

sarjon Sep 4, 2018

Member

@marionf sorry, fixed now.

Member

sarjon commented Sep 4, 2018

@marionf sorry, fixed now.

@marionf marionf self-assigned this Sep 4, 2018

@marionf

This comment has been minimized.

Show comment
Hide comment
@marionf

marionf Sep 4, 2018

Contributor

Thank you @sarjon 👍

Contributor

marionf commented Sep 4, 2018

Thank you @sarjon 👍

@marionf marionf added QA ✔️ and removed waiting for QA labels Sep 4, 2018

@marionf marionf removed their assignment Sep 4, 2018

@PierreRambaud

This comment has been minimized.

Show comment
Hide comment
@PierreRambaud

PierreRambaud Sep 4, 2018

Contributor

@sarjon I rerun travis, wait for CI and merge :)

Contributor

PierreRambaud commented Sep 4, 2018

@sarjon I rerun travis, wait for CI and merge :)

@mickaelandrieu

This comment has been minimized.

Show comment
Hide comment
@mickaelandrieu

mickaelandrieu Sep 4, 2018

Contributor

And ... this one is the last feature we accept in the Grid component: time to complete docs and tests, next improvements will wait for 1.7.6.

Good job everyone, we did it: we made a complete and highly configurable Grid component in less than 3 months 🎉 !

Contributor

mickaelandrieu commented Sep 4, 2018

And ... this one is the last feature we accept in the Grid component: time to complete docs and tests, next improvements will wait for 1.7.6.

Good job everyone, we did it: we made a complete and highly configurable Grid component in less than 3 months 🎉 !

@PierreRambaud PierreRambaud merged commit ca67d30 into PrestaShop:develop Sep 4, 2018

1 check passed

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

This comment has been minimized.

Show comment
Hide comment
@PierreRambaud

PierreRambaud Sep 4, 2018

Contributor

Thanks @sarjon and everyone for reviews ;)

Contributor

PierreRambaud commented Sep 4, 2018

Thanks @sarjon and everyone for reviews ;)

@matks matks added the migration label Sep 19, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment