workspace icon indicating copy to clipboard operation
workspace copied to clipboard

GeneralManager is able to add users to WM-

Open zak39 opened this issue 2 years ago • 2 comments

If the "share with gorup members only" is checked, the GeneralManager is not able to add users to the Workspace Manager group.

@smarinier, don't accept my PR for the moment, please :pray:

I didn't test it yet ^^'

zak39 avatar Jun 16 '23 13:06 zak39

Oui, mon point avec Philippe était justement une Optimisation. On est bien d'accord que ce cas à tester est très rare. Avec le test "en amont" de getUsersFromGroupsOnly, tu 'loades' les groupes de l'utilisateur deux fois (si ce n'est pas un GM, donc dans la majorité des cas).

Une pour tester si c'est un GM, puis après dans getUsersFromGroupsOnly, la première chose que l'on fait c'est getUserGroups() (donc la même chose).

Il suffirait peut être de tester que GENERAL_MANAGER n'est pas dans les groupes obtenus. Et donc de faire le filtrage dans la fonction, pour que ce soit sans impact de performance

smarinier avatar Jun 16 '23 13:06 smarinier

En complément, je changerais le nom de "getUsersFromGroupsOnly", car il n'est pas exact qu'il fasse un "get", vu qu'il reçoit des $users qu'il renvoie identiquement ou diminué, en "filterUsersFromGroupsOnly"

Et voici ce que je mettrais (après $groupsOfUserSession = ...)

	// don't filter users for GeneralManagers
	if (array_reduce($groupsOfUserSession, function (bool $isWSMgr, IGroup $group) {
		return $isWSMgr ? true : $group->getGID() === ManagersWorkspace::GENERAL_MANAGER;
	}, false)) {
		return $users;
	}

(cf la discussion précédente : bouger le test "isGeneralManager" dans la fonction getUsers...)

smarinier avatar Jun 26 '23 07:06 smarinier