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.

Sign up for GitHub

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

Jump to bottom

Add better logic for routing Dataverse files and their stupid duplicate #9963

Merged
merged 7 commits into from
Jun 23, 2022
Merged

Add better logic for routing Dataverse files and their stupid duplicate #9963

merged 7 commits into from
Jun 23, 2022

Conversation

jwalz
Copy link
Contributor

@jwalz jwalz commented Jun 22, 2022 •
edited
Loading

Purpose

Add the ability to differentiate between versions of Dataverse files on the html link and related routing

Changes

  • Add the datasetVersion as the version query param in the html link on Dataverse files
  • Add logic to pass query params to the resolved get_or_create method in addons.base.views.addon_view_or_download_file
  • Add useful error handling around the get_or_create in addons.base.views.addon_view_or_download_file
  • Subclass the get_or_create logic for DataverseFileNode to validate and use the version query param
    • Also don't create if the FileNode doesn't exist in this case to avoid creating future path conflicts

QA Notes

Deployed to Staging 2 and confirmed that:

  • Dataverse files load correctly through both the Project Overview files widget and the new Files page
  • Dataverse files will not load without an explicit ?version=latest or ?version=latest-published query param
  • Dataverse files will not load with ?version=latest-published query param if no published version exists
  • Files from other addons continue to load normally

Documentation

Side Effects

Ticket

brianjgeiger
brianjgeiger approved these changes Jun 22, 2022
View reviewed changes
Copy link
Collaborator

@brianjgeiger brianjgeiger left a comment

Choose a reason for hiding this comment

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

Looks legit. Worth a try. :)

brianjgeiger
brianjgeiger approved these changes Jun 23, 2022
View reviewed changes
Copy link
Collaborator

@brianjgeiger brianjgeiger left a comment

Choose a reason for hiding this comment

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

Looks good. I like the move to the dataverse model. One tiniest of questions.

osf/models/files.py Outdated
@@ -191,10 +191,14 @@ def create(cls, **kwargs):
return cls(**kwargs)

@classmethod
def get_or_create(cls, target, path):
def get_or_create(cls, target, path, **kwargs):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are you still using these kwargs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Passing the splatted query params universally but only using them in the Dataverse case. Standard linting procedure here would be to prepend unused here. Sound good?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Works for me

@jwalz jwalz merged commit 501983d into CenterForOpenScience:develop Jun 23, 2022
Johnetordoff pushed a commit to Johnetordoff/osf.io that referenced this pull request Jun 26, 2022
Merge branch 'feature/django_upgrade' of https://github.com/CenterFor…
76f054c 
…OpenScience/osf.io into load-lincenses

* 'feature/django_upgrade' of https://github.com/CenterForOpenScience/osf.io:
  remove post-migrate signals from migration stream (CenterForOpenScience#9964)
  Add better logic for routing Dataverse files and their stupid duplicate (CenterForOpenScience#9963)
  [ENG-3872] Fix dataverse 502s and pass version info to FE (CenterForOpenScience#9959)
  Fix File `show_as_unviewed` behavior (CenterForOpenScience#9960)
  Fix OSFUser.has_resources
  ensure VOL users always get read perms (CenterForOpenScience#9952)
  make types correspond more to serializer code (CenterForOpenScience#9955)
  remove special casing from DV permissions (CenterForOpenScience#9947)
Johnetordoff pushed a commit to Johnetordoff/osf.io that referenced this pull request Jun 26, 2022
Merge branch 'feature/django_upgrade' of https://github.com/CenterFor…
f3d0bbf 
…OpenScience/osf.io into signal-storage-region

* 'feature/django_upgrade' of https://github.com/CenterForOpenScience/osf.io:
  remove post-migrate signals from migration stream (CenterForOpenScience#9964)
  Add better logic for routing Dataverse files and their stupid duplicate (CenterForOpenScience#9963)
  [ENG-3872] Fix dataverse 502s and pass version info to FE (CenterForOpenScience#9959)
  Fix File `show_as_unviewed` behavior (CenterForOpenScience#9960)
  Fix OSFUser.has_resources
  ensure VOL users always get read perms (CenterForOpenScience#9952)
  make types correspond more to serializer code (CenterForOpenScience#9955)
  remove special casing from DV permissions (CenterForOpenScience#9947)
  Add a generic demo institution on the test server
  Don't send empty affiliations list to Datacite or Crossref
  Update pre-commit config to specify py version
  Fix DOI conditional
  Bump version and update CHANGELOG
  Update sc to use new SSO IdP
  [ENG-3832] Add redirect based for unauthed files page user (CenterForOpenScience#9948)
  [ENG-3781] Admin Registration Schemas (CenterForOpenScience#9937)
Johnetordoff pushed a commit to Johnetordoff/osf.io that referenced this pull request Jun 27, 2022
Merge branch 'feature/django_upgrade' of https://github.com/CenterFor…
74f70bd 
…OpenScience/osf.io into investigate-waffle-flags

* 'feature/django_upgrade' of https://github.com/CenterForOpenScience/osf.io:
  move createcachetable to post migrate signal (CenterForOpenScience#9944)
  remove post-migrate signals from migration stream (CenterForOpenScience#9964)
  Add better logic for routing Dataverse files and their stupid duplicate (CenterForOpenScience#9963)
  [ENG-3872] Fix dataverse 502s and pass version info to FE (CenterForOpenScience#9959)
  Fix File `show_as_unviewed` behavior (CenterForOpenScience#9960)

# Conflicts:
#	osf/apps.py
#	osf/migrations/0137_transfer_preprint_service_permissions.py
#	osf/migrations/__init__.py
Johnetordoff pushed a commit to Johnetordoff/osf.io that referenced this pull request Jun 27, 2022
Merge branch 'feature/django_upgrade' of https://github.com/CenterFor…
6a7f83a 
…OpenScience/osf.io into signal-deny-list

* 'feature/django_upgrade' of https://github.com/CenterForOpenScience/osf.io:
  move createcachetable to post migrate signal (CenterForOpenScience#9944)
  remove post-migrate signals from migration stream (CenterForOpenScience#9964)
  Add better logic for routing Dataverse files and their stupid duplicate (CenterForOpenScience#9963)
  [ENG-3872] Fix dataverse 502s and pass version info to FE (CenterForOpenScience#9959)
  Fix File `show_as_unviewed` behavior (CenterForOpenScience#9960)

# Conflicts:
#	osf/apps.py
#	osf/migrations/0137_transfer_preprint_service_permissions.py
#	osf/migrations/__init__.py
Johnetordoff pushed a commit to Johnetordoff/osf.io that referenced this pull request Jun 27, 2022
Merge branch 'feature/django_upgrade' of https://github.com/CenterFor…
578f0ce 
…OpenScience/osf.io into signal-deny-list

* 'feature/django_upgrade' of https://github.com/CenterForOpenScience/osf.io:
  move createcachetable to post migrate signal (CenterForOpenScience#9944)
  remove post-migrate signals from migration stream (CenterForOpenScience#9964)
  Add better logic for routing Dataverse files and their stupid duplicate (CenterForOpenScience#9963)
  [ENG-3872] Fix dataverse 502s and pass version info to FE (CenterForOpenScience#9959)
  Fix File `show_as_unviewed` behavior (CenterForOpenScience#9960)

# Conflicts:
#	osf/apps.py
#	osf/migrations/0137_transfer_preprint_service_permissions.py
#	osf/migrations/__init__.py
Johnetordoff pushed a commit to Johnetordoff/osf.io that referenced this pull request Jul 29, 2022
Merge branch 'develop' of https://github.com/CenterForOpenScience/osf.io
9ec5802 
 into fix-mailchimp

* 'develop' of https://github.com/CenterForOpenScience/osf.io: (73 commits)
  [ENG-3898][ENG-3899]Model support for OutcomeArtifact update and delete (CenterForOpenScience#9989)
  Make OutcomeArtifact.identifier nullable (CenterForOpenScience#9986)
  [ENG-3894] Outcome models (CenterForOpenScience#9975)
  revert color picker to working version (CenterForOpenScience#9968)
  Instrument the ORCiD SSO affiliation flow * Existing user with verified ORCiD ID * Existing user confirmation of linking ORCiD ID * New user confirmation of account creation with ORCiD ID
  Add a django command script to handle instn sso email domain changes
  Bump version and CHANGELOG
  Add better logic for routing Dataverse files and their stupid duplicate (CenterForOpenScience#9963)
  [ENG-3872] Fix dataverse 502s and pass version info to FE (CenterForOpenScience#9959)
  Fix File `show_as_unviewed` behavior (CenterForOpenScience#9960)
  Fix OSFUser.has_resources
  ensure VOL users always get read perms (CenterForOpenScience#9952)
  make types correspond more to serializer code (CenterForOpenScience#9955)
  remove special casing from DV permissions (CenterForOpenScience#9947)
  Add a generic demo institution on the test server
  Don't send empty affiliations list to Datacite or Crossref
  Update pre-commit config to specify py version
  Fix DOI conditional
  Bump version and update CHANGELOG
  Update sc to use new SSO IdP
  ...

# Conflicts:
#	website/templates/profile/notifications.mako
Johnetordoff pushed a commit to Johnetordoff/osf.io that referenced this pull request Jul 29, 2022
Merge branch 'develop' of https://github.com/CenterForOpenScience/osf.io
76e3d5f 
 into add-doi-domain

* 'develop' of https://github.com/CenterForOpenScience/osf.io: (126 commits)
  [ENG-3898][ENG-3899]Model support for OutcomeArtifact update and delete (CenterForOpenScience#9989)
  Make OutcomeArtifact.identifier nullable (CenterForOpenScience#9986)
  [ENG-3894] Outcome models (CenterForOpenScience#9975)
  revert color picker to working version (CenterForOpenScience#9968)
  Instrument the ORCiD SSO affiliation flow * Existing user with verified ORCiD ID * Existing user confirmation of linking ORCiD ID * New user confirmation of account creation with ORCiD ID
  Add a django command script to handle instn sso email domain changes
  Bump version and CHANGELOG
  Add better logic for routing Dataverse files and their stupid duplicate (CenterForOpenScience#9963)
  [ENG-3872] Fix dataverse 502s and pass version info to FE (CenterForOpenScience#9959)
  Fix File `show_as_unviewed` behavior (CenterForOpenScience#9960)
  Fix OSFUser.has_resources
  ensure VOL users always get read perms (CenterForOpenScience#9952)
  make types correspond more to serializer code (CenterForOpenScience#9955)
  remove special casing from DV permissions (CenterForOpenScience#9947)
  Add a generic demo institution on the test server
  Don't send empty affiliations list to Datacite or Crossref
  Update pre-commit config to specify py version
  Fix DOI conditional
  Bump version and update CHANGELOG
  Update sc to use new SSO IdP
  ...
Johnetordoff pushed a commit to Johnetordoff/osf.io that referenced this pull request Aug 2, 2022
Merge branch 'develop' of https://github.com/CenterForOpenScience/osf.io
365f4e0 
 into update-README

* 'develop' of https://github.com/CenterForOpenScience/osf.io:
  Only redirect to cas if not logged in
  OSFI: Update Shared SSO and Add MagLab/FSU [ENG-3654]
  [ENG-3898][ENG-3899]Model support for OutcomeArtifact update and delete (CenterForOpenScience#9989)
  Make OutcomeArtifact.identifier nullable (CenterForOpenScience#9986)
  [ENG-3894] Outcome models (CenterForOpenScience#9975)
  revert color picker to working version (CenterForOpenScience#9968)
  Instrument the ORCiD SSO affiliation flow * Existing user with verified ORCiD ID * Existing user confirmation of linking ORCiD ID * New user confirmation of account creation with ORCiD ID
  Add a django command script to handle instn sso email domain changes
  Bump version and CHANGELOG
  Add better logic for routing Dataverse files and their stupid duplicate (CenterForOpenScience#9963)
  [ENG-3872] Fix dataverse 502s and pass version info to FE (CenterForOpenScience#9959)
Johnetordoff pushed a commit to Johnetordoff/osf.io that referenced this pull request Aug 16, 2022
Merge branch 'develop' of https://github.com/CenterForOpenScience/osf.io
5845010 
 into remove-quickfiles-code

* 'develop' of https://github.com/CenterForOpenScience/osf.io: (44 commits)
  Re-add non-anonymized fields removed in CenterForOpenScience#10009 (CenterForOpenScience#10022)
  Update shield logo for colorado (UC Boulder)
  Update description for maglab
  [ENG-3249] Improve registration anonymization (CenterForOpenScience#10009)
  "backport" artifact changes and swap schema_response.justification (CenterForOpenScience#10003)
  Add new instn purdue
  Ensure BitBucket token is string, not bytes
  Only redirect to cas if not logged in
  OSFI: Update Shared SSO and Add MagLab/FSU [ENG-3654]
  [ENG-3898][ENG-3899]Model support for OutcomeArtifact update and delete (CenterForOpenScience#9989)
  Make OutcomeArtifact.identifier nullable (CenterForOpenScience#9986)
  [ENG-3894] Outcome models (CenterForOpenScience#9975)
  revert color picker to working version (CenterForOpenScience#9968)
  Instrument the ORCiD SSO affiliation flow * Existing user with verified ORCiD ID * Existing user confirmation of linking ORCiD ID * New user confirmation of account creation with ORCiD ID
  Add a django command script to handle instn sso email domain changes
  Bump version and CHANGELOG
  Add better logic for routing Dataverse files and their stupid duplicate (CenterForOpenScience#9963)
  [ENG-3872] Fix dataverse 502s and pass version info to FE (CenterForOpenScience#9959)
  Fix File `show_as_unviewed` behavior (CenterForOpenScience#9960)
  Fix OSFUser.has_resources
  ...

# Conflicts:
#	api_tests/files/views/test_file_detail.py
#	osf/models/nodelog.py
#	osf/models/user.py
#	tests/test_addons.py
#	website/static/js/anonymousLogActionsList.json
#	website/static/js/logActionsList.json
Johnetordoff pushed a commit to Johnetordoff/osf.io that referenced this pull request Aug 16, 2022
Merge branch 'develop' of https://github.com/CenterForOpenScience/osf.io
ce4c186 
 into remove-quickfiles-code

* 'develop' of https://github.com/CenterForOpenScience/osf.io: (44 commits)
  Re-add non-anonymized fields removed in CenterForOpenScience#10009 (CenterForOpenScience#10022)
  Update shield logo for colorado (UC Boulder)
  Update description for maglab
  [ENG-3249] Improve registration anonymization (CenterForOpenScience#10009)
  "backport" artifact changes and swap schema_response.justification (CenterForOpenScience#10003)
  Add new instn purdue
  Ensure BitBucket token is string, not bytes
  Only redirect to cas if not logged in
  OSFI: Update Shared SSO and Add MagLab/FSU [ENG-3654]
  [ENG-3898][ENG-3899]Model support for OutcomeArtifact update and delete (CenterForOpenScience#9989)
  Make OutcomeArtifact.identifier nullable (CenterForOpenScience#9986)
  [ENG-3894] Outcome models (CenterForOpenScience#9975)
  revert color picker to working version (CenterForOpenScience#9968)
  Instrument the ORCiD SSO affiliation flow * Existing user with verified ORCiD ID * Existing user confirmation of linking ORCiD ID * New user confirmation of account creation with ORCiD ID
  Add a django command script to handle instn sso email domain changes
  Bump version and CHANGELOG
  Add better logic for routing Dataverse files and their stupid duplicate (CenterForOpenScience#9963)
  [ENG-3872] Fix dataverse 502s and pass version info to FE (CenterForOpenScience#9959)
  Fix File `show_as_unviewed` behavior (CenterForOpenScience#9960)
  Fix OSFUser.has_resources
  ...

# Conflicts:
#	api_tests/files/views/test_file_detail.py
#	osf/models/nodelog.py
#	osf/models/user.py
#	tests/test_addons.py
#	website/static/js/anonymousLogActionsList.json
#	website/static/js/logActionsList.json
Johnetordoff pushed a commit to Johnetordoff/osf.io that referenced this pull request Aug 16, 2022
Merge branch 'develop' of https://github.com/CenterForOpenScience/osf.io
c29881a 
 into remove-quickfiles-code

* 'develop' of https://github.com/CenterForOpenScience/osf.io: (44 commits)
  Re-add non-anonymized fields removed in CenterForOpenScience#10009 (CenterForOpenScience#10022)
  Update shield logo for colorado (UC Boulder)
  Update description for maglab
  [ENG-3249] Improve registration anonymization (CenterForOpenScience#10009)
  "backport" artifact changes and swap schema_response.justification (CenterForOpenScience#10003)
  Add new instn purdue
  Ensure BitBucket token is string, not bytes
  Only redirect to cas if not logged in
  OSFI: Update Shared SSO and Add MagLab/FSU [ENG-3654]
  [ENG-3898][ENG-3899]Model support for OutcomeArtifact update and delete (CenterForOpenScience#9989)
  Make OutcomeArtifact.identifier nullable (CenterForOpenScience#9986)
  [ENG-3894] Outcome models (CenterForOpenScience#9975)
  revert color picker to working version (CenterForOpenScience#9968)
  Instrument the ORCiD SSO affiliation flow * Existing user with verified ORCiD ID * Existing user confirmation of linking ORCiD ID * New user confirmation of account creation with ORCiD ID
  Add a django command script to handle instn sso email domain changes
  Bump version and CHANGELOG
  Add better logic for routing Dataverse files and their stupid duplicate (CenterForOpenScience#9963)
  [ENG-3872] Fix dataverse 502s and pass version info to FE (CenterForOpenScience#9959)
  Fix File `show_as_unviewed` behavior (CenterForOpenScience#9960)
  Fix OSFUser.has_resources
  ...

# Conflicts:
#	api_tests/files/views/test_file_detail.py
#	osf/models/nodelog.py
#	osf/models/user.py
#	tests/test_addons.py
#	website/static/js/anonymousLogActionsList.json
#	website/static/js/logActionsList.json
Johnetordoff pushed a commit to Johnetordoff/osf.io that referenced this pull request Aug 16, 2022
Merge branch 'develop' of https://github.com/CenterForOpenScience/osf.io
1341f59 
 into remove-quickfiles-code

* 'develop' of https://github.com/CenterForOpenScience/osf.io: (44 commits)
  Re-add non-anonymized fields removed in CenterForOpenScience#10009 (CenterForOpenScience#10022)
  Update shield logo for colorado (UC Boulder)
  Update description for maglab
  [ENG-3249] Improve registration anonymization (CenterForOpenScience#10009)
  "backport" artifact changes and swap schema_response.justification (CenterForOpenScience#10003)
  Add new instn purdue
  Ensure BitBucket token is string, not bytes
  Only redirect to cas if not logged in
  OSFI: Update Shared SSO and Add MagLab/FSU [ENG-3654]
  [ENG-3898][ENG-3899]Model support for OutcomeArtifact update and delete (CenterForOpenScience#9989)
  Make OutcomeArtifact.identifier nullable (CenterForOpenScience#9986)
  [ENG-3894] Outcome models (CenterForOpenScience#9975)
  revert color picker to working version (CenterForOpenScience#9968)
  Instrument the ORCiD SSO affiliation flow * Existing user with verified ORCiD ID * Existing user confirmation of linking ORCiD ID * New user confirmation of account creation with ORCiD ID
  Add a django command script to handle instn sso email domain changes
  Bump version and CHANGELOG
  Add better logic for routing Dataverse files and their stupid duplicate (CenterForOpenScience#9963)
  [ENG-3872] Fix dataverse 502s and pass version info to FE (CenterForOpenScience#9959)
  Fix File `show_as_unviewed` behavior (CenterForOpenScience#9960)
  Fix OSFUser.has_resources
  ...

# Conflicts:
#	api_tests/files/views/test_file_detail.py
#	osf/models/nodelog.py
#	osf/models/user.py
#	tests/test_addons.py
#	website/static/js/anonymousLogActionsList.json
#	website/static/js/logActionsList.json
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brianjgeiger brianjgeiger brianjgeiger approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jwalz @brianjgeiger