Skip to content

user: allow setting default group#199

Open
joshmoore wants to merge 4 commits intoome:masterfrom
joshmoore:default-group
Open

user: allow setting default group#199
joshmoore wants to merge 4 commits intoome:masterfrom
joshmoore:default-group

Conversation

@joshmoore
Copy link
Member

@joshmoore joshmoore commented Mar 18, 2020

migrate code from openmicroscopy and fix flake8

see: ome/openmicroscopy@8442eed
https://forum.image.sc/t/set-user-default-group-from-command-line/35171


Adding a parameter to set a user's default group when joining a new group, and a helper function to set the default group for a user.

Ideally the CLI should also allow an ad-hoc "set group" command on it's own, but I see this as an incremental improvement towards that goal, and it certainly answers the use case I have for adding users to a group and setting that to be the default group in one command.

What this PR does

Allows a CLI user to set the default group of a user when adding the user to a group.

Testing this PR

  1. required setup

Apply the diff in this PR.

  1. actions to perform

Identify a group which your test user does not belong to.

Issue a CLI command with the new --as-default flag like so, with 103 as an example:

omero user joingroup --group-id 103 --as-default

Then issue a further

omero user joingroup --group-id 103
  1. expected observations
    Output like the following should appear, if the user was not already a member of the group.
Set the default group of user 793 to group 153

Then

793 is already in group 103

Related reading

Link to cards, tickets, other PRs: https://trello.com/c/sjEk7VQG/487-cli-manage-group-and-user-missing-features

  1. background for understanding this PR

  2. what this PR assists, fixes, or otherwise affects

The missing ability to set the default group of a user by the CLI.

migrate code from openmicroscopy and fix flake8

see:
ome/openmicroscopy@8442eed
@joshmoore
Copy link
Member Author

Some thoughts of mine from: ome/openmicroscopy#5914 (comment)


I don't know if I will have time to make a commit today but on top of this I would imagine the following:

  • (possibly) use just the first group listed as the default? (as-default becomes set-default)
  • a bin/omero group default [--set] $user [$group] method to get/show the default
  • python tests

@joshmoore
Copy link
Member Author

Reopening with pytest fix

@joshmoore joshmoore closed this Apr 3, 2020
@joshmoore joshmoore reopened this Apr 3, 2020
@sbesson sbesson added this to the 5.12.0 milestone May 24, 2022
@will-moore
Copy link
Member

Clearly (as mentioned) it would be useful to set the default group to a group that the user is already a member of. This would allow a regular user to set their own default group, without having to be an Admin, which is required by omero user joingroup
However, this otherwise works as described, so 👍

@joshmoore
Copy link
Member Author

👍 also happy to take PRs and/or suggestions on which of the API improvements to use.

@will-moore
Copy link
Member

Looking for PRs that could go in a release soon. I this "done" for now?

@joshmoore
Copy link
Member Author

Tests are missing and there's no final word on what we want the API to be. Feel free to go ahead without it.

@sbesson sbesson removed this from the 5.12.0 milestone Feb 22, 2024
@sbesson
Copy link
Member

sbesson commented Mar 28, 2025

Trying to review historical PRs in preparation of the next OMERO.py 5.20.0 milestone, what is the consensus here @will-moore @joshmoore. From the last comments, it looks like the new option in omero user joingroup is a reasonable addition and what is required here are tests and a final API review?

@will-moore
Copy link
Member

Wow, the original code is from 2018!
It's a nice-to-have feature, but I'm not sure that this has enough priority to move it to the top of my TODO list (or anyone else's?), so one option is to close until priority/demand picks up again?

@joshmoore
Copy link
Member Author

No rush from my side, though during TiM @Tom-TBT and I probably would have used this. 😄

@sbesson
Copy link
Member

sbesson commented Mar 31, 2025

I would propose we either assign a reviewer or close (and reopen when it's ready for review)

@joshmoore joshmoore requested a review from Tom-TBT March 31, 2025 11:11
@joshmoore
Copy link
Member Author

@Tom-TBT, are you up for giving it a try?

@Tom-TBT
Copy link
Contributor

Tom-TBT commented Mar 31, 2025

Some tests I've done

Normal account already in groups test1 & test2, default group is test:

  • 🟡 omero user joingroup --group-name test2 --as-default2 is already in group 53 not changing the default group.
  • 🟢 omero user joingroup --group-name test3 --as-defaultCurrent user is neither admin nor group-leader for the given user(s)/group(s)

From a normal user perspective, this command is not useful: "I can't set my default group."

Same thing from an admin account (still on the user:id:2):

  • 🟡 omero user joingroup --id 2 --group-name test2 --as-default2 is already in group 53 not changing the default group.
  • 🟢 omero user joingroup --id 2 --group-name test3 --as-default → user added to test3 and default set correctly
  • 🟢 omero user joingroup --id 2 --group-name test3 test4 --as-default--as-default requires a single group ID to be supplied. no change because more than one group provided
  • 🔴 omero user joingroup --id 2 --group-name test2 test3 --as-default → user added to test3 set as default though there's more than one group in the input:
2 is already in group 53
Added 2 to group 54
Set the default group of user 2 to group 54

Beside the bug I found, I also feel that not being able to change the default group when a user is already in the group is limiting. But it's also logical given that the name of the command is joingroup.

I see two options:

  • Allow setting default group when already in the group and change the doc of omero user --help to make it clear that it can be used for that: joingroup Join one or more groups and set default group
  • Add the command omero user defaultgroup

@jburel
Copy link
Member

jburel commented Jan 27, 2026

@Tom-TBT Are you taking ownership of this PR following your latest review?

@jburel
Copy link
Member

jburel commented Mar 24, 2026

Discussed today during the PR review meeting.
This is not a functionality that we (Dundee and GS) will use. The user is then assigned to groups via Webadmin that offers more flexibility than CLI. Unless there is a strong need from your side. We are proposing to close this old PR

@joshmoore
Copy link
Member Author

Sorry. I had overlooked Tom's joshmoore#15

I've now merged it and included the mainline. Let's see how the tests do.

@Tom-TBT
Copy link
Contributor

Tom-TBT commented Mar 25, 2026

lgtm but I've done the fix. Here's how I tested it

State before running the command below:

  • Groups test1, test2, test3
  • admin_user (ID:1) and normal_user (ID:2) in groups [test1, test2], default group is test1

The command ensures that only one group is provided when using --as-default, regardless of prior assigned groups:

  • omero user joingroup --id 1 --group-name test1 test2 --as-default
  • omero user joingroup --id 1 --group-name test2 test3 --as-default

Admin can add user to a group and set it as new default:

  • omero user joingroup --id 2 --group-name test3 --as-default

Normal users can change their default group:

  • omero user joingroup --group-name test2 --as-default

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants