-
Notifications
You must be signed in to change notification settings - Fork 7.9k
lazy populate the treeview for groups #21520
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
Conversation
ssilvert
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I'm going to try it out later.
ghost
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unreported flaky test detected, please review
Unreported flaky test detectedIf the below flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR. org.keycloak.testsuite.model.session.OfflineSessionPersistenceTest#testPersistenceMultipleNodesClientSessionsAtRandomNodeKeycloak CI - Store Model Tests |
ssilvert
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't fix the problem for me. Did you try to partial import the json attached to the issue? Import works fine (you just have to let it finish). But then I try to go to the Groups section and it hangs.
On a more minor note, the show/hide works well, but I think the "show" icon should be a right arrow instead of a hamburger menu.
ssilvert
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tested the latest revision on this PR and it works great. Still needs code review.
|
@ssilvert I think the tests are still failing on this one, but I will do quick rebase to make sure. |
ghost
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unreported flaky test detected, please review
Unreported flaky test detectedIf the below flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR. org.keycloak.testsuite.webauthn.account.WebAuthnTransportLocaleTest#multipleTransportsKeycloak CI - WebAuthn IT (chrome) |
|
Looks like it has to do with the changes introduced by Erik, we'll have to wait until he's back next week. |
|
We have been working on the same idea with a slightly different direction by expanding out the storage implementations and then updating the surrounding utility and resource classes to account for pagination. It isn't in a final state yet but since comments mention issue not being solved curious if it would be welcome here: main...Redhat-Alice:keycloak:subgroup-model-expansion |
1 flaky tests on run #8403 ↗︎Details:
|
|||||||||||||||||||||
| Test | Artifacts | |
|---|---|---|
| Authentication test > should add a condition |
Output
Screenshots
|
|
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.
|
Just a heads up. I had a few issues in testing this branch. The pagination on the left side wasn't working when I'd switch the count to 10 per page. I also don't get the caret on the dropdowns for groups with subgroups until I click on the group. |
I believe this is solved now and we intend to merge the PR soon. If you want to suggest an improvement, go ahead and create an enhancement request or discussion. |
This reverts commit 451e96a652915cecbb2ca3d3901acdeb1a20104e.
This reverts commit b8efdf2e550dd92dab0b9244314e438f933c14f1.
This reverts commit a39f9b60e1ae4f3e59339b6878aecbbe8d90a6f7.
* added lazy parameter fixes: keycloak#19954 * changed to only have the parameter * fixed merge errors * removed the `lazy` and now add subgroups on select * lint * fixed prettier * fixed nullpointer * fixed member tab
Fetching of the subgroups is now paginates in the tree and the detail view
subgroup pagination in the treeview

tree is now lazy populated on select
fixes: #19954