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.

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

Fix user list infinite loading state in user settings #25879

Merged
merged 2 commits into from
Sep 3, 2021
Merged

Fix user list infinite loading state in user settings #25879

merged 2 commits into from
Sep 3, 2021

Conversation

julien-nc
Copy link
Member

@julien-nc julien-nc commented Mar 2, 2021 •
edited
Loading

Fix #25133
This is the same as #25195 but rebased on master with resolved conflicts.

"No users in here" message is shown when there actually are displayed users.

This is because $state.loaded() is not called when the retrieved user list length is less than the 25 limit. It has to be called at least once otherwise vue-infinite-loading shows the no-results slot.

$state.loaded() is now called if we actually fetch users.

There is still one problem with this approach:
If we receive users but the displayed filtered list is empty, the no-results slot will not be displayed because we called $state.loaded().
We could face this problem with the "Disabled" filter but thankfully the related navigation item does not appear when there is no disabled user.
But it might appear in the future with a new filter that can be applied while it leads to an empty displayed user list.

@julien-nc julien-nc added this to the Nextcloud 21.0.1 milestone Mar 2, 2021
PVince81
PVince81 previously requested changes Mar 11, 2021
View reviewed changes
apps/settings/src/components/UserList.vue Outdated Show resolved Hide resolved
@PVince81 PVince81 mentioned this pull request Mar 17, 2021
Closed
@rullzer rullzer mentioned this pull request Mar 26, 2021
Merged
2 tasks
@rullzer
Copy link
Member

rullzer commented Mar 26, 2021

master is 22. Backport once merge if needed.

This was referenced May 20, 2021
Merged
Merged
@blizzz blizzz mentioned this pull request Jun 2, 2021
Merged
57 tasks
@skjnldsv skjnldsv modified the milestones: Nextcloud 22, Nextcloud 23 Jun 2, 2021
@szaimen
Copy link
Contributor

szaimen commented Jul 2, 2021

Any update here?

@julien-nc

This comment has been minimized.

Sign in to view
@julien-nc julien-nc changed the title [WIP] Fix user list infinite loading state in user settings Fix user list infinite loading state in user settings Jul 27, 2021
@julien-nc julien-nc added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jul 27, 2021
@julien-nc julien-nc force-pushed the fix/user-list-infinite-loading branch from 5a6f497 to 71b4db1 Compare July 27, 2021 08:56
@julien-nc
Copy link
Member Author

/compile amend /

@nextcloud-bot nextcloud-bot force-pushed the fix/user-list-infinite-loading branch from 71b4db1 to 2c23829 Compare July 27, 2021 09:03
@julien-nc julien-nc force-pushed the fix/user-list-infinite-loading branch from 2c23829 to 14f2863 Compare July 27, 2021 10:26
@julien-nc
Copy link
Member Author

@PVince81 You're right, #25195 works better than my solution (except one small remaining problem mentioned at the end of this PR's description).

So this PR is now the same as #25195, rebased on master, conflicts resolved.

@julien-nc julien-nc force-pushed the fix/user-list-infinite-loading branch 2 times, most recently from 865a626 to 52be852 Compare August 23, 2021 11:47
@julien-nc
Copy link
Member Author

I don't know why /drone/src/tests/acceptance/features/users.feature:48 scenario is failing. I followed it manually on a local test instance and it works fine...

@julien-nc julien-nc force-pushed the fix/user-list-infinite-loading branch from 52be852 to 517e399 Compare August 23, 2021 13:26
@szaimen
Copy link
Contributor

szaimen commented Aug 31, 2021

/rebase

@szaimen
Copy link
Contributor

szaimen commented Aug 31, 2021

/compile amend /

@szaimen
Copy link
Contributor

szaimen commented Aug 31, 2021

/rebase

@szaimen
Copy link
Contributor

szaimen commented Aug 31, 2021 •
edited
Loading

@eneiluj please rebase

Julien Veyssier added 2 commits September 1, 2021 17:23
avoid 'No users in here' in user list when there actually are users
c11689b 
Signed-off-by: Julien Veyssier <eneiluj@posteo.net>
compile settings
073a673 
Signed-off-by: Julien Veyssier <eneiluj@posteo.net>
@julien-nc julien-nc force-pushed the fix/user-list-infinite-loading branch from f56553f to 073a673 Compare September 1, 2021 15:25
@julien-nc
Copy link
Member Author

@szaimen done

szaimen
szaimen approved these changes Sep 3, 2021
View reviewed changes
Copy link
Contributor

@szaimen szaimen left a comment

Choose a reason for hiding this comment

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

LGTM :)

@szaimen szaimen merged commit 46837e2 into master Sep 3, 2021
@szaimen szaimen deleted the fix/user-list-infinite-loading branch September 3, 2021 09:43
@szaimen
Copy link
Contributor

szaimen commented Sep 3, 2021

/backport to stable22

@szaimen
Copy link
Contributor

szaimen commented Sep 3, 2021

/backport to stable21

@szaimen
Copy link
Contributor

szaimen commented Sep 3, 2021

/backport to stable20

@backportbot-nextcloud
Copy link

The backport to stable22 failed. Please do this backport manually.

@backportbot-nextcloud
Copy link

The backport to stable20 failed. Please do this backport manually.

@backportbot-nextcloud
Copy link

The backport to stable21 failed. Please do this backport manually.

@szaimen
Copy link
Contributor

szaimen commented Sep 3, 2021

/backport to stable22

@backportbot-nextcloud
Copy link

The backport to stable22 failed. Please do this backport manually.

This was referenced Sep 3, 2021
Merged
Merged
@julien-nc
Copy link
Member Author

I don't know why but this bug does not affect stable20 although the implementation is the same. Anyway, no need to backport to stable20 😁.

@szaimen
Copy link
Contributor

szaimen commented Sep 3, 2021

Thanks for the backports! :)

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

@Pytal Pytal Pytal approved these changes

@szaimen szaimen szaimen approved these changes

@ChristophWurst ChristophWurst Awaiting requested review from ChristophWurst

@GretaD GretaD Awaiting requested review from GretaD

@skjnldsv skjnldsv Awaiting requested review from skjnldsv

@PVince81 PVince81 Awaiting requested review from PVince81

@artonge artonge Awaiting requested review from artonge

@CarlSchwan CarlSchwan Awaiting requested review from CarlSchwan

Assignees
No one assigned
Labels
3. to review Waiting for reviews bug feature: settings
Projects
None yet
Milestone
Nextcloud 23
Development

Successfully merging this pull request may close these issues.

20.0.5 - groups display at buttom "Keine Benutzer vorhanden" even if there are users
6 participants
@julien-nc @rullzer @szaimen @skjnldsv @PVince81 @Pytal