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

[KEYCLOAK-2538] - groups pagination and group search #4710

Closed
wants to merge 19 commits into from
Closed

[KEYCLOAK-2538] - groups pagination and group search #4710

wants to merge 19 commits into from

Conversation

levyndot
Copy link
Contributor

@levyndot levyndot commented Nov 16, 2017

Group pagination responding to JIRA KEYCLOAK-2538. Including fixes detected after #4209 validation (JIRA KEYCLOAK-5701).

Wainting for backup from @patriot1burke and/or @stianst.

Thanks :)

@levyndot levyndot changed the title Feature/group search and pagination [KEYCLOAK-2538] - groups pagination and group search Nov 16, 2017
@stianst stianst requested review from ssilvert and removed request for ssilvert November 16, 2017 18:39
@levyndot
Copy link
Contributor Author

levyndot commented Nov 17, 2017

The pages number on each lists is not updated when a search is done. I'm fixing it right now.

@levyndot
Copy link
Contributor Author

It's ok for pages number update.

@stianst stianst added the 4.x label Dec 5, 2017
@levyndot
Copy link
Contributor Author

levyndot commented Dec 6, 2017

Did you think that this PR can be released before February ?
Just to know what kind of answer we have to prepare to our clients on when they can use an official distribution with group pagination. Thanks :)

@stianst
Copy link
Contributor

stianst commented Dec 6, 2017

Due to feature freeze for Keycloak 3.x we're not able to accept this contribution at the moment. We would also really need some tests added here for the admin console as the previous PR for this was quite broken if I remember correctly. If the PR is updated and tests added we should be able to review and include it for the first Keycloak 4.x release which should be some time in February.

@levyndot
Copy link
Contributor Author

levyndot commented Dec 6, 2017

Thanks @stianst for this answer.

@stianst stianst removed the 4.x label Feb 28, 2018
@levyndot
Copy link
Contributor Author

Hi,

I'm coming for news. Can we have any state on test progress for this feature ? Thanks.

@levyndot
Copy link
Contributor Author

@patriot1burke, @stianst, any news please about this feature ? Thanks

@stianst stianst requested review from mposolda and removed request for patriot1burke June 28, 2018 08:13
@stianst
Copy link
Contributor

stianst commented Jun 28, 2018

@mposolda can you take a look at this please?

Copy link
Contributor

@mposolda mposolda left a comment

Choose a reason for hiding this comment

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

@guitaro Thanks for the PR and sorry it took so long to review :( Overally it looks very good. I have few points:

  • Will be good to squash the commits to contain just 1 commit in the PR instead of 14 commits and ensure it is rebases/cherry-picked on top of latest master. We strongly prefer rebase/cherry-pick instead of merging due the cleaner git history. I see there are conflicts in files GroupsResource and UsersResource, but not sure, if you rather don't want to remove any changes from those files entirely (as I see that your PR changes just imports).

  • Is it possible that you add some automated tests to the admin console UI tests and re-check that your changes didn't break existing tests? See: https://github.com/keycloak/keycloak/blob/master/testsuite/integration-arquillian/HOW-TO-RUN.md#admin-console-ui-tests

@levyndot
Copy link
Contributor Author

Hi @mposolda,

Thanks for your review ! I'm working on the points that you described.

Coming soon.

Levente NAGY added 3 commits September 25, 2018 10:17
KEYCLOAK 2538 - UI group pagination - fix duplicate result for search + sort result

KEYCLOAK 2538 - UI group pagination - TU + some code improvement + add mockito dependency

KEYCLOAK 2538 - UI group pagination - Remove junit mocked TUs, add arquillian Tests, delete mockito from poms, fix groups sorting when get result from cache

KEYCLOAK 2538 - UI group pagination :
- Fix group creation bug
- Add pagination on default groups tab
- Add pagination on group membership user page
- Fix cache invalidations

KEYCLOAK 2538 - UI group pagination - update pages number on group search

KEYCLOAK 2538 - UI group pagination - Remove GroupListLoader

KEYCLOAK 2538 - compilation failed

KEYCLOAK 2538 - UI group pagination
…ion' into feature/group-search-and-pagination

# Conflicts:
#	server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java
#	services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java
#	themes/src/main/resources/theme/base/admin/resources/js/controllers/groups.js
#	themes/src/main/resources/theme/base/admin/resources/js/controllers/users.js
#	themes/src/main/resources/theme/base/admin/resources/js/loaders.js
@mposolda
Copy link
Contributor

Closing as we had other PR with this functionality, which added also server-side support.

@mposolda mposolda closed this Nov 15, 2018
@mposolda
Copy link
Contributor

@guitaro Thanks for your PR anyway. You can check if current master (Keycloak 4.7.0.Final) suits your needs.

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.

None yet

4 participants