fix(admin-delegation): Prevent delegation to group if delegation already

[printminion-co]

• Resolves: [Bug]: occ admin-delegation:add allows adding the same delegation multiple times #46609

Before

Multiple delegation to same group is possible

$ php occ admin-delegation:add OCA\\Settings\\Settings\\Admin\\Overview admin
 [OK] Administration of OCA\Settings\Settings\Admin\Overview delegated to admin.                                        

$ php occ admin-delegation:add OCA\\Settings\\Settings\\Admin\\Overview admin
 [OK] Administration of OCA\Settings\Settings\Admin\Overview delegated to admin.                                        

$ php occ admin-delegation:add OCA\\Settings\\Settings\\Admin\\Overview admin
 [OK] Administration of OCA\Settings\Settings\Admin\Overview delegated to admin.                                        

$ php occ admin-delegation:show 

Current delegations
===================

Section: overview
-----------------

 ------------------------- -------------------------------------- --------------------- 
  Name                      SettingId                              Delegated to groups  
 ------------------------- -------------------------------------- --------------------- 
  Security & setup checks   OCA\Settings\Settings\Admin\Overview   admin, admin, admin  
 ------------------------- -------------------------------------- --------------------- 

After fix

Fixes issue with delegation via occ and web.
No multiple entries in DB.

php occ admin-delegation:add OCA\\Settings\\Settings\\Admin\\Overview admin

 [OK] Administration of OCA\Settings\Settings\Admin\Overview delegated to admin.                                        

php occ admin-delegation:add OCA\\Settings\\Settings\\Admin\\Overview admin

 [WARNING] The OCA\Settings\Settings\Admin\Overview is already delegated to admin.                                      

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
• Backports requested where applicable (ex: critical bugfixes)
• Labels added where applicable (ex: bug/enhancement, 3. to review, feature component)
• Milestone added for target branch/version (ex: 32.x for stable32)

[Copilot]

Pull request overview

This PR prevents duplicate admin delegation assignments by adding validation to check if a group is already delegated to a specific settings class before creating a new assignment.

Key Changes:

• Introduces duplicate detection logic in AuthorizedGroupService::create() that throws ConflictException when attempting to assign the same group to the same class twice
• Updates the CLI command to handle the conflict gracefully with a warning message and appropriate exit code
• Adds comprehensive unit and integration tests to verify duplicate prevention behavior

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
apps/settings/lib/Service/ConflictException.php	New exception class for signaling duplicate assignment conflicts
apps/settings/lib/Service/AuthorizedGroupService.php	Adds duplicate check before creating assignments, throwing ConflictException when duplicates are detected
apps/settings/lib/Command/AdminDelegation/Add.php	Catches ConflictException and displays warning message with exit code 4
apps/settings/tests/Service/AuthorizedGroupServiceTest.php	Unit tests verifying duplicate prevention and allowed multi-group/multi-class scenarios
apps/settings/tests/Command/AdminDelegation/AddTest.php	Unit test for CLI command's duplicate handling behavior
apps/settings/tests/Integration/DuplicateAssignmentIntegrationTest.php	Integration tests validating end-to-end duplicate prevention workflow
apps/settings/composer/composer/autoload_static.php	Registers new ConflictException class in autoloader
apps/settings/composer/composer/autoload_classmap.php	Adds ConflictException to class map

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The condition if ($existing) is redundant. The findByGroupIdAndClass method will either return an entity or throw DoesNotExistException. If it returns, the entity will always be truthy. The exception should be thrown immediately after the method call without the conditional check.

Suggested change

	$existing = $this->mapper->findByGroupIdAndClass($groupId, $class);
	if ($existing) {
	throw new ConflictException('Group is already assigned to this class');
	}
	$this->mapper->findByGroupIdAndClass($groupId, $class);
	throw new ConflictException('Group is already assigned to this class');

[Copilot]

The test expects DoesNotExistException on line 111 but catches NotFoundException on line 114. These are different exception types. The expectException should be removed since the test is manually catching and handling the exception, or the caught exception type should match the expected one.

Suggested change

$this->expectException(\OCP\AppFramework\Db\DoesNotExistException::class);

[come-nc]

It would be a bit cleaner to add a unique constraint on the table and simply ignore the exception on insert if it’s a REASON_UNIQUE_CONSTRAINT_VIOLATION I think. But that would require a migration to remove existing duplicates and stuff, not sure it’s worth it.