这是indexloc提供的服务，不要输入任何密码
Skip to content

console: decouple count query from fetch data query #4269

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 22 commits into from
Apr 7, 2020
Merged

console: decouple count query from fetch data query #4269

merged 22 commits into from
Apr 7, 2020

Conversation

@beerose
Copy link
Contributor

@beerose beerose commented Apr 1, 2020 •
edited by rikinsk
Loading

Description

This PR decouples count query from fetch data query.

Changelog

  • CHANGELOG.md is updated with user-facing content relevant to this PR.

Affected components

  • Console

Related Issues

#3793

@beerose beerose requested a review from a team as a code owner April 1, 2020 20:56
@beerose beerose added the c/console Related to console label Apr 1, 2020
@hasura-bot
Copy link
Contributor

hasura-bot commented Apr 1, 2020

Review app for commit b297ef6 deployed to Heroku: https://hge-ci-pull-4269.herokuapp.com
Docker image for server: hasura/graphql-engine:pull4269-b297ef6e

rikinsk
rikinsk requested changes Apr 2, 2020
View reviewed changes
Copy link
Member

@rikinsk rikinsk left a comment

Choose a reason for hiding this comment

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

There seems to be something going wrong when I am switching the tabs from Browse Rows to some other tab (like Insert row) and then coming back to the Browse rows tab. Its not consistent but sometimes the count doesnt seem to load. Feels like a race condition somehere.

Testing at https://hge-ci-pull-4269.herokuapp.com/console/data/schema/public/tables/test/browse

@rikinsk rikinsk assigned beerose and unassigned rikinsk Apr 2, 2020
@hasura-bot
Copy link
Contributor

hasura-bot commented Apr 2, 2020

Review app for commit cda0a3b deployed to Heroku: https://hge-ci-pull-4269.herokuapp.com
Docker image for server: hasura/graphql-engine:pull4269-cda0a3b9

@beerose
Copy link
Contributor Author

beerose commented Apr 2, 2020

@rikinsk It should be fixed now.

@rikinsk rikinsk self-requested a review April 2, 2020 15:50
@beerose beerose assigned wawhal and unassigned wawhal Apr 2, 2020
wawhal
wawhal suggested changes Apr 2, 2020
View reviewed changes
@hasura-bot
Copy link
Contributor

hasura-bot commented Apr 2, 2020

Review app for commit e3d0b16 deployed to Heroku: https://hge-ci-pull-4269.herokuapp.com
Docker image for server: hasura/graphql-engine:pull4269-e3d0b163

@hasura-bot
Copy link
Contributor

hasura-bot commented Apr 2, 2020

Review app for commit ad97e1d deployed to Heroku: https://hge-ci-pull-4269.herokuapp.com
Docker image for server: hasura/graphql-engine:pull4269-ad97e1da

rikinsk
rikinsk reviewed Apr 3, 2020
View reviewed changes
Copy link
Member

@rikinsk rikinsk left a comment

Choose a reason for hiding this comment

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

This seems to work well. Regarding UX, I know we decided to show a spinner for the pagination but it felt a little odd not being able to navigate between pages till the count loads. It would be nice to still be able to navigate to next and prev pages till count loads. Maybe react-table has a way to do this or else can we think of some hack?

@rikinsk
Copy link
Member

rikinsk commented Apr 3, 2020 •
edited
Loading

Also noticed one other thing. The bulk selected rows should be reset when we change pages.

@netlify
Copy link

netlify bot commented Apr 3, 2020 •
edited
Loading

Deploy preview for hasura-docs ready!

Built with commit 823c066

https://deploy-preview-4269--hasura-docs.netlify.com

Aleksandra Sikora added 2 commits April 3, 2020 22:26
@beerose
Copy link
Contributor Author

beerose commented Apr 3, 2020

  1. I implemented the solution with an estimated count (show estimate and then change to the exact count), but it feels kinda odd when the number of pages changes.

  2. I added resetting bulk select on page change.

@hasura-bot
Copy link
Contributor

hasura-bot commented Apr 3, 2020

Review app for commit 1a0733f deployed to Heroku: https://hge-ci-pull-4269.herokuapp.com
Docker image for server: hasura/graphql-engine:pull4269-1a0733fa

@rikinsk rikinsk self-requested a review April 6, 2020 14:42
@rikinsk rikinsk assigned rikinsk and unassigned beerose Apr 6, 2020
@beerose beerose requested a review from a team as a code owner April 6, 2020 14:48
@rikinsk rikinsk assigned wawhal and unassigned rikinsk Apr 6, 2020
@rikinsk
Copy link
Member

rikinsk commented Apr 6, 2020

/heroku deploy

@hasura-bot
Copy link
Contributor

hasura-bot commented Apr 6, 2020

Review app for commit 8928cee deployed to Heroku: https://hge-ci-pull-4269.herokuapp.com
Docker image for server: hasura/graphql-engine:pull4269-8928ceec

wawhal
wawhal suggested changes Apr 6, 2020
View reviewed changes
marionschleifer
marionschleifer approved these changes Apr 6, 2020
View reviewed changes
Copy link
Contributor

@marionschleifer marionschleifer left a comment

Choose a reason for hiding this comment

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

Changelog approved.

@hasura-bot
Copy link
Contributor

hasura-bot commented Apr 6, 2020

Review app for commit 8417dc3 deployed to Heroku: https://hge-ci-pull-4269.herokuapp.com
Docker image for server: hasura/graphql-engine:pull4269-8417dc39

wawhal
wawhal approved these changes Apr 7, 2020
View reviewed changes
Copy link
Contributor

@wawhal wawhal left a comment

Choose a reason for hiding this comment

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

Code looks great.

@hasura-bot
Copy link
Contributor

hasura-bot commented Apr 7, 2020

Review app for commit 823c066 deployed to Heroku: https://hge-ci-pull-4269.herokuapp.com
Docker image for server: hasura/graphql-engine:pull4269-823c0662

@hasura-bot
Copy link
Contributor

hasura-bot commented Apr 7, 2020

Review app for commit 17c2c18 deployed to Heroku: https://hge-ci-pull-4269.herokuapp.com
Docker image for server: hasura/graphql-engine:pull4269-17c2c184

@hasura-bot
Copy link
Contributor

hasura-bot commented Apr 7, 2020

Review app for commit 8162d96 deployed to Heroku: https://hge-ci-pull-4269.herokuapp.com
Docker image for server: hasura/graphql-engine:pull4269-8162d964

@rikinsk rikinsk merged commit f615efd into hasura:master Apr 7, 2020
@hasura-bot
Copy link
Contributor

hasura-bot commented Apr 7, 2020

Review app https://hge-ci-pull-4269.herokuapp.com is deleted

wawhal
wawhal reviewed Apr 27, 2020
View reviewed changes
console/src/components/Common/utils/v1QueryUtils.js
},
});

export const getFetchManualTriggersQuery = tableName => ({
Copy link
Contributor

@wawhal wawhal Apr 27, 2020

Choose a reason for hiding this comment

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

@beerose @rikinsk This should ideally accept a table definition rather than just a table name. Can we assign this to somebody ASAP? 5 min fix.

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

Reviewers

@rikinsk rikinsk rikinsk approved these changes

+2 more reviewers

@wawhal wawhal wawhal approved these changes

@marionschleifer marionschleifer marionschleifer approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

c/console Related to console

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@beerose @hasura-bot @rikinsk @marionschleifer @wawhal