Orphan records in oc_group_folders_manage are not cleaned up

[donquixote]

How to use GitHub

• Please use the 👍 reaction to show that you are affected by the same issue.
• Please don't comment if you have no relevant information to add. It's just extra noise for everyone subscribed to this issue.
• Subscribe to receive notifications on status change and new comments.

Steps to reproduce

1. Install groupfolders app.
2. Create a group "tempgroup".
3. Create a user "tempuser", as member of "tempgroup".
4. Create a group folder. Enable advanced permissions, Add the group "tempgroup" and the user "tempuser" to the advanced permission list. (This requires to first add the "tempgroup" as a group).
5. (A) Check the contents of oc_group_folders_manage to see if it contains 'tempgroup' and 'tempuser'. Also check 'oc_groups' and 'oc_users'.
6. Remove the user 'tempuser'.
7. (B) Check the contents of oc_group_folders_manage to see if it contains 'tempgroup' and 'tempuser'.
8. Remove the group 'tempgroup'.
9. (C) Check the contents of oc_group_folders_manage to see if it contains 'tempgroup' and 'tempuser'.
10. (D) Use the API to load the group folder, have a look at the 'manage' key in the response.

Expected behaviour

At (A), the 'tempgroup' and 'tempuser' is found in oc_group_folders_manage, oc_users and/or oc_groups, respectively.
At (B), the 'tempuser' is no longer found in either table.
At (C), the 'tempgroup' is no longer found in either table.
At (D), the response contains {manage: []}.

Actual behaviour

At (B) and (C), an orphan record for 'tempuser' still exists in oc_group_folders_manage, even though it was removed from 'oc_users'.
At (C), an orphan record for 'tempgroup' still exists in oc_group_folders_manage, even though it was removed from 'oc_groups'.
At (D), the response contains {manage: [[]]} (nested empty array).

Workaround

Currently, one way to clean up the orphan records through the API is to unset the ACL checkbox, through POST '/folders/{id}/acl' with acl=false. Then enable it again and restore the records.

Solution

The app must listen to users or groups being removed, and remove the orphan records when that happens.
I don't know how this would be done in Nextcloud, unfortunately :)

[donquixote]

Actually there is already a listener for groups here, in OCA\GroupFolders\AppInfo\Application::boot():

	public function boot(IBootContext $context): void {
		$context->injectFn(function (IMountProviderCollection $mountProviderCollection, CacheListener $cacheListener, IGroupManager $groupManager): void {
			$mountProviderCollection->registerProvider($this->getMountProvider());

			$groupManager->listen('\OC\Group', 'postDelete', function (IGroup $group) {
				$this->getFolderManager()->deleteGroup($group->getGID());
			});
			$cacheListener->listen();
		});
	}

So all that is needed is:

• The ->deleteGroup() method in FolderManager should also remove the respective records from group_folders_manage table.
• In addition to listening to groups, we also need to listen to users, and have a method ->deleteUser() in FolderManager.

[juliusknorr]

This was fixed by #2504

[donquixote]

Are you sure?

In addition to listening to groups, we also need to listen to users, and have a method ->deleteUser() in FolderManager.

Is this happening?
I see a deleteCircle(), not sure what this is, but I assume not the same as a user.

[donquixote]

Also I don't see test coverage. But this is up to you.

[juliusknorr]

Right, sorry I missed that, let me reopen then