Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Filter owners and groups dropdown lists #592

Merged
merged 7 commits into from
Dec 9, 2024

Conversation

will-moore
Copy link
Member

@will-moore will-moore commented Sep 23, 2024

See https://forum.image.sc/t/omero-figure-filtering-users-by-group/101933

When you filter the File > Open list by a Group, the owners list is also filtered to only show Owners within that Group.
Also, the columns now show the current status of Filtering:

Screenshot 2024-12-03 at 14 24 15

This PR changes the display of the Owners menu in the File choose dialog. It does not change the actual behaviour of filtering Files.
To clarify the changes in this PR, I'll compare the behaviour BEFORE and AFTER...
NB: The Groups and Owners menus are only populated based on the Groups and Owners of the Figures you can access (that are loaded in the Files dialog). They are not generated based on Groups and Owners listed from OMERO. We also have no info on who is an Admin, so all users will get the same behaviour.

BEFORE:

  • Initially the default filtering is to filter by Owner = YOU and ALL Groups.
  • The Owner drop-down menu would contain All users and the Groups drop-down menu would also contain All Groups that contain Figures (regardless of whether YOU own any Figures in those groups).

Screenshot 2024-11-28 at 11 28 55

  • If you choose to filter by a Group where you have NO Figures, you will see NO Figures.
  • To find Figures that belong to another user, you will first have to Filter by that Owner, then (optionally) filter by Group.

AFTER:

  • Initially the default filtering is to filter by Owner = YOU and ALL Groups. (No Change)

  • The Owner drop-down menu initially contains All users, since we are filtering by ALL groups (No Change). The Groups menu also shows ALL groups that contain figures (No Change)

  • Now, when you filter by Group, the Owners menu only shows users who own figures in that Group.

  • To find Figures that belong to another user, you can directly filter by that Owner (No Change), or filter by Group to reduce the number of Owners, then filter by Owner.

  • If someone (E.g. a PI) wants to find figures belonging to a member of his/her Group, they can filter first by that Group, then choose to filter by Owner (which now ONLY shows owners in the selected Group). Previously this owner list still listed ALL owners.

@imagesc-bot
Copy link

This pull request has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/omero-figure-filtering-users-by-group/101933/2

@will-moore
Copy link
Member Author

@JensWendt would you be able to give this a test? Thanks!

@JensWendt
Copy link
Contributor

Hey @will-moore yes I will. Promised.
But I am swamped right now and did not find the time yet.
I will bump it higher on my prio-list

@JensWendt
Copy link
Contributor

JensWendt commented Oct 8, 2024

eeeehhhmm, stupid question, but.... where do I find files.js in the OMERO.web directory hierarchy to swap it with your adapted one?

@JensWendt
Copy link
Contributor

JensWendt commented Oct 29, 2024

@will-moore *Bump
Would love to test it, but have no idea how to get the changed code onto my test-server ?!

@will-moore
Copy link
Member Author

@JensWendt Do you have a local omero-web dev environment? I thought you would have done for #589 ?
If so, and you have this app installed there pip install -e ., then to include the JS changes, you need to have https://nodejs.org/ installed.
Then you can do:

cd omero_figure
npm install 
npm run build

That will compile the JS and copy it to the installed app static directory.

@JensWendt
Copy link
Contributor

Okay, that is what I lowkey feared :/
Nope, I do not have a dev environment. For #589 I just changed the lines in the "live" .py file and tried it out.
To be honest I cannot promise you a time frame when I come around to work my way into setting this up.
If you guys have a dev environment set up and ready to go, you might be faster with just creating some figures for different users and try it out that way.
And here I was, naively hoping Javascript stuff would not need a compile step 😩

@Tom-TBT
Copy link
Contributor

Tom-TBT commented Nov 20, 2024

I tried it, filtering figures on the production server is much smoother.

After File → Open
I get list of my all my figures in different groups. ✅ (no change)

If I look to filter per User, it lists all users of my OMERO. That's a lot, but expected (there's no filter on the group yet) ✅ (no change)

If I look to filter per Group, it only shows me the group I'm part of ✅ (change for admin user)

After filtering per Group, I now only see in Users the users of that group. ✅ (change for all)

As an admin, if I want to check a figure from a group I'm not part of, I have for the Users to select first "Show all". Then I would see all groups listed. This PR makes it less intuitive for admins to search for a figure of a group (is it an issue? i don't know) 🟧

Could the groups be listed for admins like on the main OMERO.web page ? Group you belong to first, then all groups?

@will-moore
Copy link
Member Author

@Tom-TBT - Thanks for testing.
As an Admin you should still start off from the position of seeing figures from All Owners and Groups (as before), so you shouldn't have to choose "Show All" for an Owner or Group (unless you've previously filtered by Owner or Group)?

We don't have info in hand about which groups an Admin (or anyone else) is actually in. We only know which groups and owners are assigned to the list of Figures, so it's not so easy to replicate the webclient behaviour here.

@will-moore will-moore added this to the 7.2.0 milestone Nov 27, 2024
@will-moore
Copy link
Member Author

Ah - my bad, @Tom-TBT. I was testing with Admin as root user where owner ID is 0, so everywhere that I check if (this.owner...) evaluates to false and filtering never happens for root! This is a newly identified bug which I'll fix...

But even with this fix, I still don't have the info on Admin status of group-membership status to know which groups I'm a member of and this is probably out of scope for this PR.

Copy link
Member

@pwalczysko pwalczysko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@will-moore I believe what @Tom-TBT means is that when you come in to Figure first time as an admin (NOT root, but admin), you will see the following empty canvas. I do not think this is a mild or acceptable change. I will be hitting the below every time in a workshop, as my user is always an admin. imho this should be fixed.

Edit: sorry, see comment #592 (comment) below.
This PR is not changing the behaviour of the workflow as I thought it would. The reason I was seeing the empty canvas is that the dialog always only showed your own figures by default, and my user did not have any own figures.

@pwalczysko
Copy link
Member

pwalczysko commented Nov 27, 2024

@will-moore sorry, I realized the comment #592 (review) needs to be modified. I am seeing as admin an empty canvas, because I have myself no figures. If I create a figure which belongs to me (user-6 which is an admin) I can see that figure and nothing else. I will never see any other figure unless I figure out that I have to click Owner and then Show All (btw clicking Show All under Groups is not going to do it for me, why ?). I am afraid, I still have to insist on this not being right - not intuitive, and not easily guessable either.

A different thing is also the order of columns -> Owner before the Group atm.
But I am filtering by Group which filters the Owners to the left of it. i.e. filtering from right to left.... -> first time I see something like that.
I think this is simply to be redesigned, the menu headers are not what they are intuitively promising to be.

Edit:
See #592 (comment) please. The admin behaviour did not change as a result of this PR for my type of workflow at all.
It is still a valid point in this comment to ponder the order of columns (this matches the comments of @Tom-TBT above about "lets make it like in webclient"), but this is much finer decision and for me not a must.

At this stage, I would suggest that @JensWendt gets a chance to test the behaviour.

@pwalczysko
Copy link
Member

After @will-moore clarified and we tested that actually, before this PR as well as with this PR, you only ever see your own figures by default when you come to the menu in question for the first time (does not matter if you are admin or not), I am much more positive about the PR.
It does not change the behaviour for my workflow as I was afraid it would.

@will-moore
Copy link
Member Author

I've updated the PR description above to clarify the changes in behaviour and also improved the Files dialog to show the current filtering status more clearly (see new screenshots above)

@Tom-TBT
Copy link
Contributor

Tom-TBT commented Nov 28, 2024

I don't think the admin scenario I mentioned is needed very often, so I'm okay without it.

However, I revisited the issue, and the way groups are initially filtered seems problematic to me.

From the description:

The Owner drop-down menu initially contains All users, since we are filtering by ALL groups (No Change) but, the Groups menu ONLY shows groups where YOU own Figures

If someone (E.g. a PI) wants to find figures belonging to a member of his/her Group, they can filter first by that Group, then choose to filter by Owner (which now ONLY shows owners in the selected Group). Previously this owner list still listed ALL owners.

If I understand correctly, the PI won't see the group listed as a filter if they don't have any Figures in it (e.g., all Figures belong to the students). This makes the described workflow inaccurate.
Instead, the workflow would need to be:

  1. Show all users
  2. Select Group (to narrow down the user list)
  3. Select Users (from the filtered list)

We don't have info in hand about which groups an Admin (or anyone else) is actually in.

True, but wouldn’t it be worth retrieving it?

[g.getName() for g in conn.getGroupsMemberOf()]

I haven't dived into the logic of the file filtering so I don't know how many changes that implies.
I'm also ok if you leave it out. This PR is still an improvement IMO.

This was referenced Nov 28, 2024
Copy link
Member

@pwalczysko pwalczysko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is still an improvement IMO.

I concur. After studying it repeatedly, and now when the indication what is being filtered on is implemented (e.g. in the column header, the selection Owner -> user-6 is indicated once you selected filtering for user-6) the PR is a great improvement for my workflows.

I think the biggest say now should come from @JensWendt as we are at the stage of imagining somebody elses problems/workflows which can be tricky.

@JensWendt
Copy link
Contributor

Hello,

First of all, thank you all a lot for writing and testing this change!!
Without having tested this, just by reading the description/discussion my thoughts:

The Owner drop-down menu initially contains All users

Would that mean, that a user who is in 2 of possible dozens of groups will see all users from all groups in that dropdown menu? If so, that is a change to the current status, where you just see users listed which have a Figure in groups you belong to.
I would highly prefer the current option, otherwise it is just too many "irrelevant" users listed.

You can't immediately filter by a Group where you have NO Figures (which would previously show NO Figures)

I agree with Tom here, that this is not good/intuitive for two important use cases.

  1. The "newbie" who just joined a group and takes their first steps in OMERO would expect to be able to filter for their group without having to know a workaround where they have to create an initial blank Figure first, just to have one in the group.
    Follow up question, would the "Group" dropdown menu be empty for someone new, who is in only one group if they don't have a figure in there? That would be even more weird for them.
  2. The admin who actually wants to be able to filter for all possible groups, even though they do not own Figures in most/any of them.

I agree with Toms proposed workflow, as the most likely one for everyday use-cases.

  1. Show all users
  2. Select Group (to narrow down the user list)
  3. Select Users (from the filtered list)

...you only ever see your own figures by default when you come to the menu in question for the first time...

I also like that

The issue @pwalczysko raised with

the order of columns -> Owner before the Group atm.

is a valid point, but a very minor nuisance in my opinion. If it can get changed easily, fine, if not, don't bother.

As I don't have a dev setup for Figure, is there a way I can clone the PR branch and pip install it from there, or does it have to be compiled before it goes on pypi.org normally?

@will-moore
Copy link
Member Author

@JensWendt Did you get my e-mail with testing instructions on our merge-ci server?

@will-moore
Copy link
Member Author

@JensWendt There is NO change to the initial filtering.

Would that mean, that a user who is in 2 of possible dozens of groups will see all users from all groups in that dropdown menu?

No - There is no change here. You will only see figures listed that you can access. The Groups and Owners menus are the Groups and Owners of that list of figures which you can see, so they must belong to groups that you are a member of (unless you are an Admin and can see all Figures).

The "newbie" who just joined a group and takes their first steps in OMERO would expect to be able to filter for their group without having to know a workaround where they have to create an initial blank Figure first, just to have one in the group.

This is not necessary. If you have NO figures, but other users do (and you can access those figure) then you simply need to switch the Owner filter to a different user (or to "All users") and you will see their figures.

When I said "The Owner drop-down menu initially contains All users" I mean "All Owners of figures you can see".
I think this is what you want.

If you can pip install -e . this repo into a python environment and perform the other config steps to run it within omero-web, then to get the JavaScript compiled you need to install node (npm). Then:

cd omero-figure
npm install
npm build

Then you should see the changes when you refresh the figure app.

I'm going to make these changes...

  • Switch the order of the columns. Group | Owners
  • Never filter the Group column - always show All Groups that contain figures that you can see

@will-moore
Copy link
Member Author

Made a few changes and now deployed on merge-ci for testing. I also updated the description with the new behaviour.

@Tom-TBT
Copy link
Contributor

Tom-TBT commented Dec 3, 2024

Thank you Will. I prefer the current version with no filtering on the groups.
Switching Users and Groups dropdown lists also makes it more intuitive IMO.
lgtm !

@will-moore will-moore merged commit d5050d8 into ome:master Dec 9, 2024
1 check passed
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.

5 participants