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

sql: bugfix: return total resource count correctly #236

Merged
merged 6 commits into from
Jul 5, 2024

Conversation

moio
Copy link
Contributor

@moio moio commented Jul 5, 2024

Problem: when browsing to a paginated list, pagination controls do not appear at the bottom of the page:

Screenshot 2024-07-05 at 11 34 15

Root cause: Steve is returning the total resource count incorrectly (returning the number of returned resources, usually one page size or less, instead of the total count).

Solution: leverage new data returned by lasso with rancher/lasso#83 to return the right number.

Note this PR should be merged together with rancher/lasso#83 or functionality will break.

Result: pagination controls appear and work as expected:
fixed

FWIW I have also run the integration tests and they are all green rancher/rancher#46015

Best reviewed commit-by-commit.

Please note this contains commits from #235, disregard the first commit.

Please note this PR should be merged after https://github.com/rancher/lasso/84, and the last TEMP commit should be replaced with a lasso bump. The TEMP commit is there to show that tests pass once both changes are merged.

@moio
Copy link
Contributor Author

moio commented Jul 5, 2024

JFYI @richard-cox @brudnak

Copy link
Contributor

@MbolotSuse MbolotSuse left a comment

Choose a reason for hiding this comment

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

LGTM pending the lasso changes to complete the work.

@tomleb tomleb self-requested a review July 5, 2024 16:05
Copy link
Contributor

@tomleb tomleb left a comment

Choose a reason for hiding this comment

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

LGTM, just gotta make sure to update lasso first

@tomleb tomleb merged commit 0841e03 into rancher:master Jul 5, 2024
1 check passed
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