-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Do not show deactivated users in sharees and contacts #10543
Conversation
Signed-off-by: Thomas Citharel <tcit@tcit.fr>
96b4811
to
b14cfa9
Compare
The second part of fixes lacks tests into |
@tcitworld ready for reviews? |
Signed-off-by: Thomas Citharel <tcit@tcit.fr>
b14cfa9
to
03f1fef
Compare
@skjnldsv Yes, it just lacks one more test. |
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.
Code is ok 👍
@@ -106,6 +106,7 @@ public function testUpdateAndDeleteUser() { | |||
$user->method('getUID')->willReturn('test-user'); | |||
$user->method('getCloudId')->willReturn('cloudId'); | |||
$user->method('getDisplayName')->willReturn('test-user'); | |||
$user->method('isEnabled')->willReturn(true); |
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.
would be great to have a test for the false
condition as well 😉
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.
@ChristophWurst How's that ?
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 think you just pushed what I was asking for 😉
I feel like this could probably be handled better :/ Signed-off-by: Thomas Citharel <tcit@tcit.fr>
@ChristophWurst so good to go from you? |
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.
Makes sense 🚀
First part to fix #5608 : remove deactivated users from system address book.