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

Fix Issue #4831 - Adds pagination of tokens on user profile page. #4990

Merged
merged 11 commits into from Jul 18, 2019

Conversation

@ahormazabal
Copy link
Contributor

commented Jun 24, 2019

This PR adds pagination to the token section of the user profile page, in order to support a high amount of tokens.

Tested with 8000+ tokens. Loading time was reduced from 20s to ~300ms

The main changes on this PR are:

  • Added a gsp-based paginator which will reload the webpage when a new page is requested.
  • Token delete operation now work through http post instead of ajax in order to correctly display the pagination afterwards.
  • Token create still works through ajax, but it will reload the page afterwards instead of injecting the new record on the table, for the pagination to display correctly.
  • Added a new pagination taglib in order to support gsp pagination using bootstrap markup.

@ahormazabal ahormazabal changed the title Pagination of tokens on user profile page. FIX #4831 - Adds pagination of tokens on user profile page. Jul 1, 2019

@ahormazabal ahormazabal changed the title FIX #4831 - Adds pagination of tokens on user profile page. FIX https://github.com/rundeck/rundeck/issues/4831 - Adds pagination of tokens on user profile page. Jul 1, 2019

@ahormazabal ahormazabal changed the title FIX https://github.com/rundeck/rundeck/issues/4831 - Adds pagination of tokens on user profile page. Fix Issue #4831 - Adds pagination of tokens on user profile page. Jul 1, 2019

@ahormazabal

This comment has been minimized.

Copy link
Contributor Author

commented Jul 1, 2019

Fixes issue: #4831

@ahormazabal ahormazabal requested a review from sjrd218 Jul 2, 2019

@ahormazabal ahormazabal added this to the 3.1.0-RC2 milestone Jul 2, 2019

@ahormazabal

This comment has been minimized.

Copy link
Contributor Author

commented Jul 2, 2019

This still needs to be backported to 3.0.x, i'll create a new PR for that.

@sjrd218
Copy link
Contributor

left a comment

I created 21 tokens. Navigated to the second page, then deleted a token. Instead of removing the paginator, and rendering 20 tokens, it rendered an empty token list with the paginator still visible.

@sjrd218
Copy link
Contributor

left a comment

When the token is inserted into the table with ajax the total token count isn't being updated. Also, the striping on the table stops alternating.

@ahormazabal ahormazabal self-assigned this Jul 10, 2019

@ahormazabal

This comment has been minimized.

Copy link
Contributor Author

commented Jul 10, 2019

finally the behavior will be the following:

  • changed token ordering to newer to older.
  • a row will be added dinamically to the table only if we are at the first page and the addition will not change the number of pages.
  • a row will be deleted dinamically from the table only if we are at the last page and the deletion will not change the number of pages.
  • on all other cases the page will be reloaded.
  • The default page size was setted on 50, and added an option to the gui options to override.

The practical effect of this behavior is that users with a low amount of tokens will only see the dynamic behavior, but as the number of tokens grow the page will switch to the gsp paginator.

Also fixed the table striping bug and the table summary update.

@ahormazabal ahormazabal force-pushed the ahormazabal:dev/userprofile-tokenpaging branch from df9c8a8 to 52b78e5 Jul 10, 2019

@ahormazabal ahormazabal requested a review from sjrd218 Jul 10, 2019

@sjrd218
Copy link
Contributor

left a comment

Looks good to me. I like the way the token insertion works.

@ahormazabal ahormazabal force-pushed the ahormazabal:dev/userprofile-tokenpaging branch from 52b78e5 to 1dd4b5e Jul 17, 2019

ahormazabal pushed a commit that referenced this pull request Jul 17, 2019

ahormazabal pushed a commit to ahormazabal/rundeck that referenced this pull request Jul 17, 2019

ahormazabal pushed a commit to ahormazabal/rundeck that referenced this pull request Jul 17, 2019

@gschueler gschueler merged commit 99198b7 into rundeck:master Jul 18, 2019

20 checks passed

Mergeable Mergeable Run has been Completed!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
security/snyk - build.gradle (rundeck) No manifest changes detected
security/snyk - core/build.gradle (rundeck) No manifest changes detected
security/snyk - plugins/build.gradle (rundeck) No manifest changes detected
security/snyk - plugins/copyfile-plugin/build.gradle (rundeck) No manifest changes detected
security/snyk - plugins/flow-control-plugin/build.gradle (rundeck) No manifest changes detected
security/snyk - plugins/git-plugin/build.gradle (rundeck) No manifest changes detected
security/snyk - plugins/jasypt-encryption-plugin/build.gradle (rundeck) No manifest changes detected
security/snyk - plugins/job-state-plugin/build.gradle (rundeck) No manifest changes detected
security/snyk - plugins/localexec-plugin/build.gradle (rundeck) No manifest changes detected
security/snyk - plugins/orchestrator-plugin/build.gradle (rundeck) No manifest changes detected
security/snyk - plugins/script-plugin/build.gradle (rundeck) No manifest changes detected
security/snyk - plugins/source-refresh-plugin/build.gradle (rundeck) No manifest changes detected
security/snyk - plugins/stub-plugin/build.gradle (rundeck) No manifest changes detected
security/snyk - plugins/upvar-plugin/build.gradle (rundeck) No manifest changes detected
security/snyk - rundeck-storage/build.gradle (rundeck) No manifest changes detected
security/snyk - rundeckapp/build.gradle (rundeck) No manifest changes detected
security/snyk - rundeckapp/grails-spa/package.json (rundeck) No manifest changes detected
security/snyk - rundeckapp/metricsweb/build.gradle (rundeck) No manifest changes detected

gschueler added a commit that referenced this pull request Jul 18, 2019

Merge pull request #5065 from ahormazabal/backports/userprofile-token…
…paging2

Backport PR #4990 - Adds pagination of tokens on user profile
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.