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

Adds pagination for custom props & fixes an issue with errant (none) return values #1382

Merged
merged 5 commits into from
Oct 14, 2021

Conversation

Vigasaurus
Copy link
Contributor

@Vigasaurus Vigasaurus commented Oct 9, 2021

Changes

Fixes #1353 by adding a "Load More" button to the bottom of the custom props list
Potentially fixes #550 (but that may have already been fixed with the old way of limiting to 100)

Also fixes an issue where the (none) value for custom props was being returned at all times, meaning that if you requested 100 items, it would return those 100 items + the none entry if there were the full 100 items to be returned, meaning that in a paginated setup, the none entry would be in the first page & not in its correct sorted order. Fixes this by 1. not applying the limit nor the offset to the none entry query (since it's extra filtered and would always be returning one row) and 2. removing the none entry from the zipped results if it was the 101st (or rather, the entry with an index == limit) entry in the array (this implies that it was added where it was not yet necessary, and it should instead be returned on the next (or further) page.

Tests

  • This PR does not require tests

Changelog

  • Entry has been added to changelog

Documentation

  • This change does not need a documentation update

@metmarkosaric
Copy link
Contributor

This is great, thanks @Vigasaurus !

Copy link
Contributor

@ukutaht ukutaht left a comment

Choose a reason for hiding this comment

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

Looks god to me.

Historically I haven't been testing things thoroughly but I think we're at a point where the project is getting pretty big and complex and I want to rein things in a bit.

This can be tested in the breakdown_test.exs file because there you can specify a low value for limit so you don't have to create 100 DB entries just for testing.

@Vigasaurus Vigasaurus requested a review from ukutaht October 13, 2021 14:01
Copy link
Contributor

@ukutaht ukutaht left a comment

Choose a reason for hiding this comment

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

Thanks!

@ukutaht ukutaht merged commit 7e9d83d into plausible:master Oct 14, 2021
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.

"Outbound link: Click" only returns top 100 URLs Page freeze with large datasets
3 participants