Skip to content

Create pds for others#5402

Merged
jburel merged 18 commits intoome:developfrom
will-moore:create_pds_for_others
Aug 14, 2017
Merged

Create pds for others#5402
jburel merged 18 commits intoome:developfrom
will-moore:create_pds_for_others

Conversation

@will-moore
Copy link
Copy Markdown
Member

@will-moore will-moore commented Jul 31, 2017

What this PR does

Allows users with writeOwned privilege to create P/D/S belonging to other users:
https://trello.com/c/0icOEIqM/12-rfe-enable-creation-of-p-d-s-on-behalf-of-others-in-web

Also fixes linking of objects in tree so that when user with "writeOwned" moves items in the tree, the new links created belong to the same owner as the parent object.

screen shot 2017-07-31 at 11 15 20

NB: Creation of P/D/S for other users when viewing "All Members" is not yet supported. To keep this PR manageable and within time constraints of m3, support for "All Members" should be in follow-up PR.

TODO:

  • Robot tests!
  • Integration tests.

Testing this PR

  • Log in as a restricted Admin with at least writeOwned privilege.
  • When viewing your own data, Create P/D/S should be identical to regular user (objects belong to you)
  • Viewing another User's data, Create P/D/S should simply indicate that the data will belong to that user (users without writeOwned should have P/D/S creation disabled as before).
  • Viewing "All Members" data, Create P/D/S is only enabled if you are a member of the Group (data will belong to you). See NB above.
  • When a user with writeOwned copies/cut/paste or drag/drops data in the tree, new links should belong to the same user as the Parent object. Can be tested by that user trying to cut links in a read-ann / read-only group. (NB: above, for new object creation, link belongs to same owner as the new data, not the parent)?
  • Test creation of new Dataset with orphaned images selected.

cc @pwalczysko

@jburel
Copy link
Copy Markdown
Member

jburel commented Jul 31, 2017

./components/tools/OmeroWeb/omeroweb/webclient/views.py:2565:50: E128 continuation line under-indented for visual indent

@jburel jburel added the develop label Jul 31, 2017
@pwalczysko
Copy link
Copy Markdown
Member

NB: above, for new object creation, link belongs to same owner as the new data, not the parent)?

Yes, this makes some sense.

@pwalczysko
Copy link
Copy Markdown
Member

pwalczysko commented Aug 2, 2017

Bug1:

  • light admin with only "Write data" privilege
  • light admin logs in and goes to private group, filtering for user-2
  • light admin does not have icons for creation of P/D/S enabled until the user node is selected -> is this intended ?
  • after the P/D/S creation icons are enabled in this way, the first creation of a Project causes also the right-click menu on this Project to be enabled, but not on other Projects or Datasets -> is this intended ?
  • then a crashes can be elicited as these newly created Datasets are being dragged and dropped to the other Projects

I guess the intention here was not to enable any icons in the Private group in the first place when filtering for others' data ?

@pwalczysko
Copy link
Copy Markdown
Member

pwalczysko commented Aug 2, 2017

Bug2: In Read-annotate group, of which the light admin is a member.

  • light admin has "WriteData" but nothing else
  • light admin goes to "All members" view
  • light admin drags and drops a Dataset of somebody else abd is linked to somebody else's Project to their (light admin's) project
  • observe error

Edit: I think the error is connected to the fact that light admin was trying to sever the original link by trying to drag and drop (for which they have no permissions, as they have no "Delete Data"). Note that the creation of the new link is successful, whilst the original link is not severed.

@pwalczysko
Copy link
Copy Markdown
Member

Except for Bug1 and Bug2 all tests in the header of this PR passed.

@will-moore
Copy link
Copy Markdown
Member Author

@pwalczysko Those 2 bugs should be fixed (Delete link failure is silent now).
I changed the create enabled logic a bit because it was previously only allowing creation if canLink was True on the selected object which didn't make much sense (E.g. could on create Screen if selected Dataset was canLink).
Actually, if createOwned is True then we don't care about canLink since the new link is owned by the same owner as the parent, so we can always link!
Need to clarify all the use cases we are supporting.

@will-moore
Copy link
Copy Markdown
Member Author

TODO: remove linking logic from this PR (only handle creation of new obj).

@pwalczysko
Copy link
Copy Markdown
Member

Created https://trello.com/c/lEZQ4DdA/104-light-crash-in-web-when-d-d for the "delete link when you have no rights" crash. This was a pre-existing (pre-this-PR, not pre-5.4.0) bug which we missed when fixing the light crashes in web.

@pwalczysko
Copy link
Copy Markdown
Member

Retest of all functionalities went all as expected, light admin which can or cannot delete and can or cannot write (being a group member or not), as well as normal user were tested. Checked also the Orphaned images workflow, works fine. Checked links ownership and this is as expected as well.

Ready to merge.

@jburel
Copy link
Copy Markdown
Member

jburel commented Aug 8, 2017

Methods in webclient_gateway will need to be deprecated instead of removed

@will-moore
Copy link
Copy Markdown
Member Author

@jburel TODOs to be done in this PR or a follow-up?

@jburel
Copy link
Copy Markdown
Member

jburel commented Aug 9, 2017

Too late for 5.4.0-m3
I will do the todos in this PR
minimally the tests should be added

@will-moore
Copy link
Copy Markdown
Member Author

Robot and web integration tests added.

@snoopycrimecop
Copy link
Copy Markdown
Member

Conflicting PR. Removed from build OMERO-DEV-merge-push#767. See the console output for more details.
Possible conflicts:

@will-moore
Copy link
Copy Markdown
Member Author

@jburel I tried to add tests without conflicting with #5347 but it obviously didn't work.
Need to get that PR in first probably.

@snoopycrimecop snoopycrimecop mentioned this pull request Aug 12, 2017
@will-moore
Copy link
Copy Markdown
Member Author

@jburel If you're happy with this, it would be nice to merge so @rgozim can fix merge conflicts in #5428, thanks.

@jburel
Copy link
Copy Markdown
Member

jburel commented Aug 14, 2017

Thanks for adding the tests and marking the methods back.
Merging so @rgozim can move forward

@jburel jburel merged commit bb0ef7c into ome:develop Aug 14, 2017
@sbesson sbesson added this to the 5.4.0 milestone Oct 6, 2017
@will-moore will-moore deleted the create_pds_for_others branch February 18, 2019 04:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants