refactor(GroupInterface): Cleanup the group backends - #61745
Open
CarlSchwan wants to merge 2 commits into
Open
Conversation
- Use as much typing as possible - Remove unused interface in ILDAPGroupPlugin - Mark group backends as implementing a few more interface Signed-off-by: Carl Schwan <carlschwan@kde.org>
CarlSchwan
requested review from
Altahrim,
come-nc,
leftybournes and
salmart-dev
and removed request for
a team
July 3, 2026 09:03
Signed-off-by: Carl Schwan <carlschwan@kde.org>
susnux
added
the
♻️ refactor
Contributor
its not removed, no? |
come-nc
requested changes
Jul 6, 2026
Comment on lines
-1324
to
1257
| /** | ||
| * Gets group details | ||
| * | ||
| * @param string $gid Name of the group | ||
| * @return array|false | ||
| * @throws Exception | ||
| */ | ||
| public function getGroupDetails($gid) { | ||
| if ($this->groupPluginManager->implementsActions(GroupInterface::GROUP_DETAILS)) { | ||
| return $this->groupPluginManager->getGroupDetails($gid); | ||
| #[\Override] | ||
| public function getGroupDetails(string $gid): array { | ||
| $displayName = $this->getDisplayName($gid); | ||
| if ($displayName !== '') { | ||
| return ['displayName' => $displayName]; | ||
| } | ||
| throw new Exception('Could not get group details in LDAP backend.'); | ||
|
|
||
| return []; | ||
| } |
Contributor
There was a problem hiding this comment.
Here you remove the support for overriding getGroupDetails from a user_ldap plugin?
Comment on lines
-141
to
-171
|
|
||
| /** | ||
| * get the number of all users matching the search string in a group | ||
| * @param string $gid ID of the group | ||
| * @param string $search query string | ||
| * @return int|false | ||
| * @throws \Exception | ||
| */ | ||
| public function countUsersInGroup($gid, $search = '') { | ||
| $plugin = $this->which[GroupInterface::COUNT_USERS]; | ||
|
|
||
| if ($plugin) { | ||
| return $plugin->countUsersInGroup($gid, $search); | ||
| } | ||
| throw new \Exception('No plugin implements countUsersInGroup in this LDAP Backend.'); | ||
| } | ||
|
|
||
| /** | ||
| * get an array with group details | ||
| * @param string $gid | ||
| * @return array|false | ||
| * @throws \Exception | ||
| */ | ||
| public function getGroupDetails($gid) { | ||
| $plugin = $this->which[GroupInterface::GROUP_DETAILS]; | ||
|
|
||
| if ($plugin) { | ||
| return $plugin->getGroupDetails($gid); | ||
| } | ||
| throw new \Exception('No plugin implements getGroupDetails in this LDAP Backend.'); | ||
| } |
Contributor
There was a problem hiding this comment.
Why do you remove support for these? Because it’s unused?
|
|
||
| namespace OCA\User_LDAP; | ||
|
|
||
| use OCP\AppFramework\Attribute\Consumable; |
Contributor
There was a problem hiding this comment.
Forgot to actually add the attribute?
| @@ -182,7 +163,7 @@ public function removeUser(IUser $user): void { | |||
| $this->emitter->emit('\OC\Group', 'preRemoveUser', [$this, $user]); | |||
| } | |||
| foreach ($this->backends as $backend) { | |||
| if ($backend->implementsActions(\OC\Group\Backend::REMOVE_FROM_GOUP) && $backend->inGroup($user->getUID(), $this->gid)) { | |||
| if ($backend->implementsActions(\OCP\GroupInterface::REMOVE_FROM_GOUP) && $backend->inGroup($user->getUID(), $this->gid)) { | |||
Contributor
There was a problem hiding this comment.
Suggested change
| if ($backend->implementsActions(\OCP\GroupInterface::REMOVE_FROM_GOUP) && $backend->inGroup($user->getUID(), $this->gid)) { | |
| if ($backend->implementsActions(\OCP\GroupInterface::REMOVE_FROM_GROUP) && $backend->inGroup($user->getUID(), $this->gid)) { |
| @@ -35,7 +38,6 @@ interface GroupInterface { | |||
|
|
|||
| /** | |||
| * @since 12.0.0 | |||
| * @deprecated 29.0.0 | |||
Contributor
There was a problem hiding this comment.
Please put that back. Or you can even remove the const I guess.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
TODO
Checklist
3. to review, feature component)stable32)AI (if applicable)