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

Second DRS Fix #297

Open
wants to merge 25 commits into
base: master
Choose a base branch
from
Open

Second DRS Fix #297

wants to merge 25 commits into from

Conversation

willronchetti
Copy link
Member

  • Remove validation of drs_id

@willronchetti willronchetti requested a review from aschroed June 26, 2024 17:31
Copy link
Member

@aschroed aschroed left a comment

Choose a reason for hiding this comment

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

Tests need updating? The fixture uses old version of uri - may not matter but for consistency?

if 'accession' not in self.schema['properties']:
return keys
keys.setdefault('accession', []).extend(properties.get('alternate_accessions', []))
if properties.get('status') != 'replaced' and 'accession' in properties:
Copy link
Member

Choose a reason for hiding this comment

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

Is the and 'accession' in properties clause needed - if you get here won't it definitely be there even if only an empty list from the line above?

And will this not be problematic for 'replaced' files as now this file that has the old accession as an alternative_accession will have a unique key conflict with the replaced file which is still accessible and in the db?

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