Conversation
StephenHulme
added
the
Accessioning
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #5603 +/- ##
===========================================
- Coverage 87.25% 87.23% -0.02%
===========================================
Files 1460 1460
Lines 32966 32994 +28
Branches 3465 3471 +6
===========================================
+ Hits 28764 28783 +19
- Misses 4181 4190 +9
Partials 21 21 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
spec/models/sample_spec.rb
Outdated
There was a problem hiding this comment.
can there be also a separate context where study2 is a managed study for testing the mixed case?
There was a problem hiding this comment.
Yes, good thinking, will add some more tests
StephenHulme
requested review from
andrewsparkes,
neilsycamore and
yoldas
and removed request for
yoldas
|
Screenshots from @neilsycamore's testing:
|
|
|
||
| unopen_studies = sample.studies_for_accessioning.reject { |study| study.study_metadata.open? } | ||
| accession_warning( | ||
| 'Samples belonging to more than one study can only be accessioned if all accessionable studies are open:', |
There was a problem hiding this comment.
If all of them are managed, they could be accessioned to EGA, because there is no mix of open and managed. This is a discussion point I think.
There was a problem hiding this comment.
EGA does have the extra complexity of having different data policies and contacts. But yes, technically there's no reason why not, but it's a changed of business logic that should be discussed.
| let(:studies) { [managed_with_accession_1, managed_with_accession_2] } | ||
|
|
||
| it 'should not be accessioned' do | ||
| expect(sample.should_be_accessioned?).to be false |
There was a problem hiding this comment.
case: if all are managed, we could allow it.
There was a problem hiding this comment.
There are different policy and contact agreements for EGA studies that ENA studies do not have. If this is a desired change to the business logic, it should be relatively easy to implement.
2 participants
Permits accessioning of samples in multiple studies
Changes proposed in this pull request
Instructions for Reviewers
[All PRs] - Confirm PR template filled
[Feature Branches] - Review code
[Production Merges to
main]- Check story numbers included
- Check for debug code
- Check version