update_group behaviour not as expected

[bernt-matthias]

The documented behavior of update_group is to substitute previous users. But actually it leads to duplicates of user group associations in the galaxy DB.

This has been tested with version 0.7.0 which is shipped with the current release of galaxy.

[bernt-matthias]

To me more precise:
If the parameter user_ids=[] it works as expected. But if the list contains a user that is already present in this group it leads to a duplication.

[nsoranzo]

@bernt-matthias Thanks for the report! It seems this is due to delete_existing_assocs being False at https://github.com/galaxyproject/galaxy/blob/dev/lib/galaxy/webapps/galaxy/api/groups.py#L128 .
That's been the case since when this Galaxy API call was introduced in 2012, ping author @jmchilton.

[jmchilton]

Well that is unfortunate. I can't speak for distant-past-@jmchilton - that guy made a ton of mistakes and was kind of a moron - but current @jmchilton doesn't think this should work the way it does. Distant-past-@jmchilton implemented this endpoint with only creating groups and roles in mind.

I'm okay with either calling this a bug and just modifying the behavior and hope the backward compatibility doesn't bite anyone or adding a parameter like update_or_append that defaults to append but could be set to update. @nsoranzo do you have a preference for either of these options - I would defer to you on which would be preferable.

[bernt-matthias]

Maybe the main question is if duplicate entries in user_group_accessions in the galaxy DB are a problem. I would vote for the update behavior (as default) since this is documented -- and thereby it should not cause compatibility problems. For the original intended purpose (filling empty groups) update / append make no difference anyway. The additional possibility of append would be a nice addition.

[nsoranzo]

There is already another API call to add a user to a group in https://github.com/galaxyproject/galaxy/blob/dev/lib/galaxy/webapps/galaxy/api/group_users.py :

PUT /api/groups/{encoded_group_id}/users/{encoded_user_id}
Adds a user to a group

Similarly for adding a role to a group.
So I suppose the best way forward is call it a bug, fix it and see if any test fails or anyone shouts.

[bernt-matthias]

Sounds good to me. Unfortunately I can not help with this since I'm not familiar enough with this or the galaxy project.