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

Share #19

Merged
merged 9 commits into from
Mar 18, 2021
Merged

Share #19

merged 9 commits into from
Mar 18, 2021

Conversation

pwalczysko
Copy link
Member

A suggestion which tries to solve ome/omero-guides#80

Happy to move this to a better place if this is what we want. As it is laid out now, we can link to the particular Share paragraph specifically

cc @jburel @joshmoore

@pwalczysko pwalczysko mentioned this pull request Feb 24, 2021
@will-moore
Copy link
Member

Shares are completely broken now (see https://forum.image.sc/t/omero-sharing-data-with-users-you-are-not-in-a-group-with/48520) - You can't actually see any user, apart from those that can already see your data.
So, instead of marking "deprecated" as suggested in ome/omero-web#179, maybe we should just remove this completely. We are way past the "deprecating" stage now!

@pwalczysko
Copy link
Member Author

pwalczysko commented Feb 25, 2021

Shares are completely broken now (see https://forum.image.sc/t/omero-sharing-data-with-users-you-are-not-in-a-group-with/48520) - You can't actually see any user, apart from those that can already see your data.
So, instead of marking "deprecated" as suggested in ome/omero-web#179, maybe we should just remove this completely. We are way past the "deprecating" stage now!

But this PR is not about the fate of the feature, it just adds a pointer to warning and a suggestion of a workaround to docs.
The fact that Shares are broken is not in conflict with the formulations here imho. It actually adds a safety - no possiblity to create new Shares (partially). The explanations and warning are valid - except that the screenshot and concrete pointer to the OMERO.web tab might become obsolete soon - but all depends on timing on the OMERO.web front - so I would think we could go ahead with this PR anyway ?

@will-moore
Copy link
Member

OK, let's not hold this PR up, waiting on ome/omero-web#179. But even without any change, I think the "Shares are deprecated" message is already out of date. It should be a "Known issue: Shares are broken" and will be removed soon.

@pwalczysko
Copy link
Member Author

The message I was envisaging is saying "Do not use this. Do not lose your time. We will not fix this." You are right, it seems best not to go ahead with this PR, as the messsage now has too many unknowns and would have to be watered down, which might again lose time of the users trying to explore the feature. cc @jburel

Shares are deprecated
~~~~~~~~~~~~~~~~~~~~~

Duplicating and moving the data into another group as described in :ref:`Duplicate feature<Duplicatecli>` is the recommended way you can share objects in OMERO with users who are not members of the group where the data are located. Please **do not use the** ``Shares`` tab |image6| above the left-hand pane in OMERO.web to create new Shares as this feature is deprecated and not supported and tested any more. The deprecation warning will be added to OMERO.web in future.
Copy link
Member

Choose a reason for hiding this comment

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

"Do not use the Shares tab....to create new Shares..." Shares aren't created in this tab. They are created with the "World" button in the Tree toolbar.

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed in 5f19952

@pwalczysko
Copy link
Member Author

I think I reformulated the text here to adjust for new realities. I guess we wait with merging here until we have ome/omero-web#179 sorted. Then, we could release in sync ?

@pwalczysko
Copy link
Member Author

@will-moore I can see that in ome/omero-web#265 you are going for wording "Not supported" where I use here "discontinued". Do you want to suggest reformulations here accordingly ?

@will-moore
Copy link
Member

I don't think we need to worry about "Not supported" vv "discontinued". Both are fine and interchangeable

@pwalczysko
Copy link
Member Author

thanks @will-moore , the #19 (comment) is fixed in 2b11212

@joshmoore
Copy link
Member

👍

@pwalczysko
Copy link
Member Author

The commit c6be04a fixes the comment of @will-moore on ome/www.openmicroscopy.org#447 (comment)

docs/data-management.rst Outdated Show resolved Hide resolved
pwalczysko and others added 2 commits March 1, 2021 19:10
Co-authored-by: Josh Moore <j.a.moore@dundee.ac.uk>
Shares (discontinued feature)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Previously created Shares can still be viewed in the ``Shares`` tab |image6| above the left-hand side pane of OMERO.web. Nevertheless, the Shares feature is discontinued. **It is not posssible to use the** ``World`` icon |image7| above the left-hand pane in OMERO.web to create new Shares anymore. Shares have been deprecated in OMERO 5.3.0 and are discontinued in OMERO.web xxx and later. You need to use the OMERO standard permissions to share Images, by :ref:`moving the data<Movedowners>` into the appropriate group. If you also want to retain the Images in their current group, you can first duplicate them as described in :ref:`Duplicate feature<Duplicatecli>`. Nevertheless, this :ref:`Duplicate<Duplicatecli>` and :ref:`Move<Movedowners>` workflow has following limitations:
Copy link
Member

Choose a reason for hiding this comment

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

OMERO.web xxx is 5.9.0.

Copy link
Member

Choose a reason for hiding this comment

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

It was actually OMERO.server 5.6.1 that 'broke' shares (meant you couldn't see users who weren't in your own groups). But probably don't need that detail here.

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed in 7be0a68


- :ref:`Duplicate<Duplicatecli>` is only available on the Command Line Interface (CLI)
- You need to already be in a non-private group with the user you wish to share with
- You will also be sharing the data with all the other members of that group
Copy link
Member

Choose a reason for hiding this comment

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

At least on https://github.com/pwalczysko/omero-guide-introduction/blob/share/docs/data-management.rst page, these haven't formatted correctly. They have an extra space before -

Copy link
Member Author

Choose a reason for hiding this comment

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

the rendering on https://github.com/pwalczysko/omero-guide-introduction/blob/share/docs/data-management.rst is not relevant. Try to complare the rendering of an existing link (not changed in this PR) on github with the correct rendering on the released guide in

The rendering of an existing link (not changed in this PR) on github
Screenshot 2021-03-08 at 12 46 29

This renders correctly in https://omero-guides.readthedocs.io/projects/introduction/en/latest/data-management.html?highlight=%22Duplicate%20feature%22#manage-images-in-datasets-projects
Screenshot 2021-03-08 at 12 44 51

The markdown on the line matches to what was there before in places such as screenshotted above. It also renders correctly if you do make clean html with local sphinx.

@jburel jburel mentioned this pull request Mar 8, 2021
@jburel
Copy link
Member

jburel commented Mar 12, 2021

Looks good. To be merged when 5.9.0 is out

@pwalczysko
Copy link
Member Author

Maybe we should merge and release, as all the other pieces are in place and deployed now ?

@jburel jburel merged commit 7adb05b into ome:master Mar 18, 2021
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.

4 participants