feat(users): Store and load a user's manager by ChristophWurst · Pull Request #38013 · nextcloud/server

ChristophWurst · 2023-05-02T07:02:03Z

Resolves: Assign managers to users and map value in the system address book #37963

Summary

Allow (sub) admins to set a user's manager.

TODO

Backend for the new user property
Frontend for the new user property
System address book mapping
Trigger update event for assigned admin so the system address book card gets regenerated

Screenshot

Checklist

Code is properly formatted
Sign-off message is added to all commits
Tests (unit, integration, api and/or acceptance) are included
Screenshots before/after for front-end changes
Documentation (manuals or wiki) has been updated or is not required
- Admins feat(usermanual): Add user manager property description documentation#10353
- Devs feat(devmanual): Add user managers doc documentation#10355
Backports requested where applicable (ex: critical bugfixes)

kesselb · 2023-05-02T08:22:32Z

Nice to have: remove the uid when the manager is deleted. A listener for user deleted is already there: https://github.com/nextcloud/server/blob/master/apps/provisioning_api/lib/Listener/UserDeletedListener.php

tcitworld · 2023-05-02T09:42:12Z

+ *
+ * @return string[]
+ */
+	public function getManagerUids(): array;


thought: Those are very specific new methods in IUser, just for metadata elements. Maybe make it more generic so that it can absorb every kind of contact information we could give it?

I thought so too but that's tricky with types. Some properties are string, some are nullable string, this is a string array.

hamza221 · 2023-05-03T16:40:11Z

nimishavijay

Looks great design-wise!

ChristophWurst · 2023-05-04T11:45:24Z

Frontend for the new user property

Is that done @hamza221 or do we miss anything?

hamza221 · 2023-05-04T11:47:00Z

Frontend for the new user property
Is that done @hamza221 or do we miss anything?

Done

ChristophWurst · 2023-05-04T12:35:08Z

Mapping is done too. ~~The only missing piece is change detection and regeneration of the card when the admin property is updated.~~ done

ChristophWurst · 2023-05-04T18:17:47Z

/compile /

ChristophWurst · 2023-05-04T18:31:20Z

 :show-config="showConfig"
 :sub-admins-groups="subAdminsGroups"
 :user="user"
+ :users="users"


BUG this only looks at the local users. If you go to a group you can only assign a manger from the same group. If you have thousands of users and only the first x are visible you can only pick from those x.
The multiselect has to search users on the backend

@hamza221 could you address this please? You will have to use the components async search callback feature and fetch users from the server

Seems addressed. Users are looked up on the server now 👍

ChristophWurst · 2023-05-09T07:57:14Z

BUG can't set myself as manager of another user

Turns out uid!=displayname for my test user. searching by display name works 👍

ChristophWurst · 2023-05-10T15:05:25Z

Files sharing acceptance test fails. Most likely unrelated?

ChristophWurst · 2023-05-11T07:18:40Z

Took a random other PR and it's the same tests that fail: #38188 https://drone.nextcloud.com/nextcloud/server/33791/36/3

😌

The tests are broken in general. Everything is fine.

ChristophWurst · 2023-05-11T07:22:45Z

acceptance-app-files-sharing is suspicious

ChristophWurst · 2023-05-11T12:30:38Z

Rebased because I can't restart Drone

ChristophWurst · 2023-05-11T15:01:35Z

acceptance-app-files-sharing fails reliably

danxuliu · 2023-05-12T04:08:24Z

When locally running the acceptance tests the Nextcloud logs show (docker exec nextcloud-local-test-acceptance tail -f /var/www/html/data/nextcloud.log) the error ...POST","url":"/ocs/v2.php/apps/files_sharing/api/v1/shares","message":"Class OCA\\Notifications\\FakeUser contains 2 abstract methods and must therefore be declared abstract or implement the remaining methods (OCP\\IUser::getManagerUids, OCP\\IUser::setManagerUids) at /nextcloud/apps/notifications/lib/FakeUser.php#30"....

The Notifications app is installed when setting up the acceptance tests environment, but if present the existing app will be used without downloading it again. The failing acceptance tests pass after adding the following to an existing apps/notifications/lib/FakeUser.php:

	public function getManagerUids(): array {
throw new \RuntimeException('Not implemented');
}
public function setManagerUids(array $uids): void {
throw new \RuntimeException('Not implemented');
}

ChristophWurst · 2023-05-12T06:06:44Z

You are the best, honestly.

ChristophWurst · 2023-05-12T07:01:13Z

Alright. I'm giving this PR a final rebase. nextcloud/notifications#1544 is pending. If it gets merged soon the tests will pass here.

blizzz · 2023-05-12T07:54:23Z

seems some rebuilds are needed. fyi, there'll be a nc/vue bump following as well.

ChristophWurst · 2023-05-12T07:55:02Z

OK can you tell me if I should rebase already or wait for the vue bump merge @blizzz? Cheers

blizzz · 2023-05-12T08:27:32Z

OK can you tell me if I should rebase already or wait for the vue bump merge @blizzz? Cheers

Wait until #38214 is merged

nickvergessen · 2023-05-12T08:31:59Z

 <div v-if="showConfig.showUserBackend" class="userBackend" />
 <div v-if="showConfig.showLastLogin" class="lastLogin" />
+ <div :class="{'icon-loading-small': loading.manager}" class="modal__item managers">
+ <NcMultiselect ref="manager"


NcMultiselect is actually deprecated because it is not accessible, but I see it's used in this flie all over, so okay for now.

ChristophWurst · 2023-05-12T09:44:45Z

showtime

ChristophWurst · 2023-05-12T09:47:23Z

showtime

there are not just bundle conflicts. we have to adjust the front-end after #37870

ChristophWurst · 2023-05-12T10:04:25Z

the table looks bad but that's coming from #37870 (comment)

the feature works

Co-Authored-By: hamza221 <hamzamahjoubi221@gmail.com> Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>

ChristophWurst · 2023-05-12T12:21:52Z

Psalm failure is unrelated

ChristophWurst added 2. developing Work in progress feature: users and groups integration labels May 2, 2023

ChristophWurst added this to the Nextcloud 27 milestone May 2, 2023

ChristophWurst self-assigned this May 2, 2023

tcitworld reviewed May 2, 2023
View reviewed changes

skjnldsv mentioned this pull request May 3, 2023
27.0.0 beta 1 #38038
Merged

hamza221 mentioned this pull request May 3, 2023
Assign managers to users and map value in the system address book #37963
Closed

nimishavijay approved these changes May 3, 2023
View reviewed changes

ChristophWurst added 3. to review Waiting for reviews and removed 2. developing Work in progress labels May 4, 2023

ChristophWurst marked this pull request as ready for review May 4, 2023 13:17

ChristophWurst requested a review from miaulalala as a code owner May 4, 2023 13:17

ChristophWurst commented May 4, 2023
View reviewed changes

ChristophWurst marked this pull request as draft May 4, 2023 18:34

ChristophWurst added 2. developing Work in progress and removed 3. to review Waiting for reviews labels May 4, 2023

ChristophWurst mentioned this pull request May 5, 2023
feat(dav): expose system address book #37734
Merged
5 tasks

skjnldsv mentioned this pull request May 9, 2023
27.0.0 beta 2 #38137
Closed

skjnldsv mentioned this pull request May 9, 2023
27.0.0 beta 2 #38139
Merged

ChristophWurst commented May 9, 2023
View reviewed changes

Comment threadapps/settings/src/components/UserList.vue

skjnldsv modified the milestones: Nextcloud 27, Nextcloud 28 May 9, 2023

ChristophWurst force-pushed the feat/users/store-load-manager-uid branch from 960893d to 9ebbe1fCompare May 10, 2023 14:01

ChristophWurst force-pushed the feat/users/store-load-manager-uid branch from 9ebbe1f to 3a1bb8bCompare May 11, 2023 06:38

This comment was marked as resolved.
Sign in to view

ChristophWurst force-pushed the feat/users/store-load-manager-uid branch from 3a1bb8b to 549be4cCompare May 11, 2023 12:30

ChristophWurst mentioned this pull request May 12, 2023
fix: Adapt fake user class to server changes nextcloud/notifications#1544
Merged

ChristophWurst force-pushed the feat/users/store-load-manager-uid branch from 1c19b59 to f502666Compare May 12, 2023 07:02

nickvergessen reviewed May 12, 2023
View reviewed changes

ChristophWurst force-pushed the feat/users/store-load-manager-uid branch from f502666 to e89971dCompare May 12, 2023 10:01

feat(users): Store and load a user's manager
1381c4c
Co-Authored-By: hamza221 <hamzamahjoubi221@gmail.com> Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>

ChristophWurst force-pushed the feat/users/store-load-manager-uid branch from e89971d to 1381c4cCompare May 12, 2023 11:56

ChristophWurst disabled auto-merge May 12, 2023 13:01

ChristophWurst merged commit 2440314 into masterMay 12, 2023

ChristophWurst deleted the feat/users/store-load-manager-uid branch May 12, 2023 13:01

Uh oh!

Uh oh!

Conversation

ChristophWurst commented May 2, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

TODO

Screenshot

Checklist

Uh oh!

kesselb commented May 2, 2023

Uh oh!

tcitworldMay 2, 2023

Choose a reason for hiding this comment

Uh oh!

ChristophWurstMay 2, 2023

Choose a reason for hiding this comment

Uh oh!

hamza221 commented May 3, 2023

Uh oh!

nimishavijay left a comment

Choose a reason for hiding this comment

Uh oh!

ChristophWurst commented May 4, 2023

Uh oh!

hamza221 commented May 4, 2023

Uh oh!

ChristophWurst commented May 4, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ChristophWurst commented May 4, 2023

Uh oh!

ChristophWurstMay 4, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ChristophWurstMay 5, 2023

Choose a reason for hiding this comment

Uh oh!

ChristophWurstMay 9, 2023

Choose a reason for hiding this comment

Uh oh!

ChristophWurstMay 9, 2023

Choose a reason for hiding this comment

Uh oh!

ChristophWurst commented May 9, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

ChristophWurst commented May 10, 2023

Uh oh!

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

ChristophWurst commented May 11, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ChristophWurst commented May 11, 2023

Uh oh!

ChristophWurst commented May 11, 2023

Uh oh!

ChristophWurst commented May 11, 2023

Uh oh!

danxuliu commented May 12, 2023

Uh oh!

ChristophWurst commented May 12, 2023

Uh oh!

ChristophWurst commented May 12, 2023

Uh oh!

blizzz commented May 12, 2023

Uh oh!

ChristophWurst commented May 12, 2023

Uh oh!

blizzz commented May 12, 2023

Uh oh!

nickvergessenMay 12, 2023

Choose a reason for hiding this comment

Uh oh!

ChristophWurstMay 12, 2023

Choose a reason for hiding this comment

ChristophWurst commented May 2, 2023•
edited
Loading

ChristophWurst commented May 4, 2023•
edited
Loading

ChristophWurstMay 4, 2023•
edited
Loading

ChristophWurst commented May 9, 2023•
edited
Loading

ChristophWurst commented May 11, 2023•
edited
Loading

ChristophWurst commented May 12, 2023•
edited
Loading