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 pagination for existing relationships #50

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

crawfjs
Copy link

@crawfjs crawfjs commented Apr 20, 2020

Description of the Change

This change implements pagination for the existing relationships (PickerList) and resolves an issue where relationships are removed when updating a post and there are more than ten relationships defined. This limited the number of relationships that the plugin would allow to ten.

Alternate Designs

Another consideration was to handle pagination server side. This would have been a heavier implementation, including a move from a save-all approach, to individual change sets / transactions.

Benefits

This is a light weight approach for the pagination, encapsulating within the component itself.

Possible Drawbacks

Since pagination is handled on the client side, it implements a select all approach. Could put a really high limit here (2000), but we could run into the data loss issue again.

Verification Process

For the changes to the project / setup, run any of these
npm run build-js
or
npm run dev-build-js
or
npm run watch-js

Functionality was tested in the following scenarios:

  • No relationships
  • One page worth of relationships (<=10)
  • Multiple pages worth of relationships
  • Adding relationships (one page of relationships, multiple pages of relationships)
  • Deleting relationships (on first page, last page, etc)

Checklist:

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests passed.

Applicable Issues

n/a

Changelog Entry

@crawfjs crawfjs changed the title Feature/add pagination existing relationships Add pagination for existing relationships Apr 25, 2020
@jeffpaul jeffpaul added this to the 1.6.0 milestone May 6, 2020
@jeffpaul jeffpaul requested a review from moraleida May 6, 2020 22:47
@jeffpaul
Copy link
Member

jeffpaul commented May 7, 2020

@crawfjs welcome to WP Content Connect and thanks for the PR, it's greatly appreciated! I'm checking with a teammate to see if they can give this a review and either provide feedback or merge this in, will keep you posted as that happens and if there are any questions or updates needed. Thanks again!

@s3rgiosan
Copy link
Member

@jeffpaul are there any plans to release this enhancement any time soon? We have a client that needs the number of relationships "limit" bumped.

@jeffpaul
Copy link
Member

@s3rgiosan up to now @rickalee has been the core maintainer here, but if you feel comfortable leading things on a release then I'd be happy to help you get things tagged/released?

@jeffpaul
Copy link
Member

@s3rgiosan any interest in resolving the conflicts here for @rickalee to review/approve for release on this as well?

@s3rgiosan
Copy link
Member

Sure. I can take a look tomorrow.

@jeffpaul jeffpaul requested review from rickalee and removed request for moraleida February 15, 2024 21:12
@jeffpaul
Copy link
Member

jeffpaul commented Apr 2, 2024

@s3rgiosan any interest in resolving the conflicts here for @rickalee to review/approve for release on this as well?

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.

3 participants