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

Redesign group list in the user settings view #39262

Merged
merged 1 commit into from
Sep 22, 2021

Conversation

AlexAndBear
Copy link

@AlexAndBear AlexAndBear commented Sep 21, 2021

Description

Enhancement: Redesign group list in the user settings view

Before this PR, the group list settings were confusing as the user needs to
click on a group first before the trash bin icon appears to delete a group.
As well the user count was nondescript.
With this PR, a redesign takes care of those issues and also fixes the problem
that while deleting a group in the group list,
it was still available in the user group list.
Furthermore, after deleting a group, the user will be redirected to the
'Everyone' group summary view.
Also fixes an issue where the 'active'-class on a newly created group
was duplicated.

Related Issue

Motivation and Context

How Has This Been Tested?

  • test environment:
  • test case 1:
  • test case 2:
  • ...

Screenshots (if appropriate):

Before

image

After

image

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:
  • Changelog item, see TEMPLATE

@owncloud owncloud deleted a comment from update-docs bot Sep 21, 2021
@AlexAndBear
Copy link
Author

@pmaier1 @micbar FYI

@AlexAndBear AlexAndBear changed the title Redesign Redesign group list in the user settings view Sep 21, 2021
@AlexAndBear AlexAndBear marked this pull request as ready for review September 21, 2021 13:53
@phil-davis
Copy link
Contributor

phil-davis commented Sep 21, 2021

  • delete a group that has members and click "Yes" to confirm
  • the group is gone from the left panel of groups - good
  • the members of the group are still listed in the main user list, as if they are still in the group
    Group-displayed-after-delete

In the screen-shot I had just deleted group "m123"

"something" (tm) needs to happen after a group is deleted, to refresh the user list to "something". Either go back to listing "Everyone" or just empty the list completely or?

Note: this looks like it is issue #29057 which the stale bot just closed. I will leave it closed, because I have reported here and we can do something reasonable in this PR.

Copy link
Contributor

@phil-davis phil-davis left a comment

Choose a reason for hiding this comment

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

Just minor edits to the changelog, while you look at what tests are failing and need adjusting.

changelog/unreleased/39262 Outdated Show resolved Hide resolved
changelog/unreleased/39262 Outdated Show resolved Hide resolved
changelog/unreleased/39262 Outdated Show resolved Hide resolved
@phil-davis phil-davis self-requested a review September 21, 2021 15:27
@AlexAndBear
Copy link
Author

AlexAndBear commented Sep 22, 2021

Another issue popped up (was pre existing)

  1. Create a new group
  2. Create a second group
  3. Hover over the title
  4. The Popover is displaying the wrong name

Fixed

@AlexAndBear
Copy link
Author

@phil-davis after deletion of a group, we now will be redirected to everyone

@AlexAndBear AlexAndBear force-pushed the redesign-user-settings-groups branch 2 times, most recently from a363bda to 5fe8fd9 Compare September 22, 2021 09:54
@AlexAndBear
Copy link
Author

@phil-davis CI should be green now, ready to have a second look =)

@owncloud owncloud deleted a comment from ownclouders Sep 22, 2021
@owncloud owncloud deleted a comment from ownclouders Sep 22, 2021
@owncloud owncloud deleted a comment from sonarcloud bot Sep 22, 2021
@owncloud owncloud deleted a comment from ownclouders Sep 22, 2021
@owncloud owncloud deleted a comment from ownclouders Sep 22, 2021
@owncloud owncloud deleted a comment from ownclouders Sep 22, 2021
@owncloud owncloud deleted a comment from ownclouders Sep 22, 2021
@owncloud owncloud deleted a comment from ownclouders Sep 22, 2021
@owncloud owncloud deleted a comment from ownclouders Sep 22, 2021
@owncloud owncloud deleted a comment from ownclouders Sep 22, 2021
@owncloud owncloud deleted a comment from ownclouders Sep 22, 2021
@owncloud owncloud deleted a comment from ownclouders Sep 22, 2021
@owncloud owncloud deleted a comment from ownclouders Sep 22, 2021
@owncloud owncloud deleted a comment from ownclouders Sep 22, 2021
@owncloud owncloud deleted a comment from ownclouders Sep 22, 2021
@owncloud owncloud deleted a comment from ownclouders Sep 22, 2021
Comment on lines -918 to 910
public function theUserCountOfGroupShouldNotBeDisplayedOnTheWebui($group) {
$count = $this->usersPage->getUserCountOfGroup($group);
Assert::assertNull(
$count,
"Failed asserting that user count of group $group is not displayed"
);
}
}
Copy link
Author

Choose a reason for hiding this comment

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

We don't need this anymore, as we show the correct amount even 0

@sonarcloud
Copy link

sonarcloud bot commented Sep 22, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

60.6% 60.6% Coverage
0.0% 0.0% Duplication

Copy link
Contributor

@phil-davis phil-davis left a comment

Choose a reason for hiding this comment

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

I can't "break" it any more. I manually tried putting users into various groups, and as subadmin of groups, then deleting the group... and it works nicely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants