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

HOTFIX: Prevent sharing of locked samples #1403

Merged
merged 11 commits into from
Nov 4, 2022

Conversation

joshsadam
Copy link
Contributor

@joshsadam joshsadam commented Nov 3, 2022

Description of changes

Currently in IRIDA a locked sample can be shared / moved to another project. This also includes sharing and moving with keeping the locked status. This PR addresses this by completely removing the ability to share or move locked samples.

Related issue

N/A

Checklist

Things for the developer to confirm they've done before the PR should be accepted:

  • CHANGELOG.md (and UPGRADING.md if necessary) updated with information for new change.
  • Tests added (or description of how to test) for any new features.
  • User documentation updated for UI or technical changes.

@joshsadam joshsadam marked this pull request as ready for review November 4, 2022 10:12
@joshsadam joshsadam requested a review from ericenns November 4, 2022 10:12
Copy link
Contributor

@deepsidhu85 deepsidhu85 left a comment

Choose a reason for hiding this comment

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

I still need to test it but the code looks good. Just one change for now.

Also, one thing that I think we may want to address is should a project owner still be allowed to share their samples to their own projects. What if it's the case where a project owner wants to share the samples to another one of their own projects so they can use the sample and write back the results to it, but at the same time want to allow project collaborators to use the sample but not allow them to update any of the sample metadata etc. Since this functionality has been around quite a while we may want to check with some of the users if making this change (preventing the sharing/moving of locked samples) is alright. Thoughts? @joshsadam @ericenns

@ericenns
Copy link
Member

ericenns commented Nov 4, 2022

that I think we may want to address is should a project owner still be allowed to share their samples to their own projects. What if it's the case where a project owner wants to share the samples to another one of their own projects so they can use the sample and write back the result

The original owner of the samples can go to the original project to share them to other projects. If they deleted the samples from the original project, the samples are now effectively locked and cannot be unlocked, since they would no longer be able to share them (which currently exists). This is why I am ok with this change. I doubt many people were sharing locked samples, I don't think there is anything in the documentation talking about this.

@deepsidhu85
Copy link
Contributor

that I think we may want to address is should a project owner still be allowed to share their samples to their own projects. What if it's the case where a project owner wants to share the samples to another one of their own projects so they can use the sample and write back the result

The original owner of the samples can go to the original project to share them to other projects. If they deleted the samples from the original project, the samples are now effectively locked and cannot be unlocked, since they would no longer be able to share them (which currently exists). This is why I am ok with this change. I doubt many people were sharing locked samples, I don't think there is anything in the documentation talking about this.

Thanks @ericenns. Yeah it that case it definitely makes sense. I will leave it with you. I will test it out shortly but code wise it looks good to me

Copy link
Contributor

@deepsidhu85 deepsidhu85 left a comment

Choose a reason for hiding this comment

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

One other change below

@deepsidhu85
Copy link
Contributor

As discussed, it was a little confusing when a user selected both unlocked and locked samples, clicked on the share button, a modal pops up to list the locked samples, then the user clicks ok and it takes you to the share page to continue sharing the unlocked samples. Update it to remove the modal listing the locked samples and rather list these samples on the share -> samples step.

Copy link
Member

@ericenns ericenns left a comment

Choose a reason for hiding this comment

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

👍

@ericenns ericenns merged commit 232bbe3 into master Nov 4, 2022
@ericenns ericenns deleted the hotfix-dont-share-locked-samples branch November 4, 2022 18:38
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.

3 participants