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

Implement server side pagination and sorting for queries lists #2686

Merged
merged 9 commits into from
Aug 1, 2018

Conversation

jezdez
Copy link
Member

@jezdez jezdez commented Jul 18, 2018

This was initially reported in #78 but also applies to the queries search where the list of results can become quite large depending on the number of queries and search term used.

This was originally reported in mozilla#384 and mozilla#385.

Notes:

  • The sorting criteria are hardcoded and only allow specified columns (and related columns) to be used.

  • It adds a page size dropdown to the query-list page (and the appropriate backend handling) and allows clearing the search input to reset the query list to the non search result status.

  • It deprecates the standalone backend query search handler and redirects requests for it.

  • I believe the live paginator should also be used in more list views and the non-live paginator deprecated or outright removed.

  • I'm not sure how Redash is handling API backward compatibility, so I erred on the side of caution and followed a deprecation policy (that could mean removing old APIs after the next version for example).

Screenshot:

screen-shot-2018-07-18-22-31-32

TODOs:

@vercel
Copy link

vercel bot commented Jul 18, 2018

Since this Pull Request originated from a forked repository, Now cannot deploy it as there are potential security risks.

If you are a collaborator on this repository, consider making this Pull Request from a branch on the same repository instead of a fork.

jezdez added 2 commits July 18, 2018 22:55
- Redirect the old search API handler.
- Sort by specific database columns or relationships.
- Allow showing “my” queries per tag as well.
Also:

- Adds page size select tag list.
- Adds clear button to search input.
- Shows search also on “my” page.
@kocsmy
Copy link
Collaborator

kocsmy commented Jul 24, 2018

Thanks for the PR @jezdez

Can we:

  • Move it to the bottom of the left side of the results
  • Only show this if we have more than 20 results

And is there any way to make the dropdown look better in Firefox?

@arikfr arikfr changed the title Fix #78 - Implement server side pagination and sorting for queries lists Implement server side pagination and sorting for queries lists Jul 31, 2018
@arikfr
Copy link
Member

arikfr commented Jul 31, 2018

Btw, issue #78 is about when a single query's result is very large and needs pagination.

Copy link
Member

@arikfr arikfr left a comment

Choose a reason for hiding this comment

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

While reviewing I noticed two small issues that I fixed in place:

  • Null durations were shown as "0 seconds", which was confusing when trying to sort. Showing as '-' now. (494f43b)
  • schedule and created_at were not in the whitelisted order by list. (ccaf849)

Firefox dropdown & moving the page size selector: I'm thinking of switching to Ant's pagination control, which will address these issues. @kocsmy WDYT about this? Do you think it will fit in our design? (once you update the font)

@kocsmy
Copy link
Collaborator

kocsmy commented Jul 31, 2018

@arikfr yes, this is a really nice component. I'll make sure it fits to our current design.

@arikfr arikfr merged commit a014df3 into getredash:master Aug 1, 2018
@arikfr
Copy link
Member

arikfr commented Aug 1, 2018

🍾

@jezdez
Copy link
Member Author

jezdez commented Aug 9, 2018

Apologies for the late reply @kocsmy, I was out for a bit. Thank you for addressing the issues @arikfr! It's much appreciated 🎉 🎊 🍾

@jezdez jezdez deleted the serverside branch August 9, 2018 15:59
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.

4 participants