Skip to content

[ENG-3872] Fix dataverse 502s and pass version info to FE#9959

Merged
jwalz merged 8 commits into
CenterForOpenScience:developfrom
Johnetordoff:dataverse-file-list-take-2
Jun 22, 2022
Merged

[ENG-3872] Fix dataverse 502s and pass version info to FE#9959
jwalz merged 8 commits into
CenterForOpenScience:developfrom
Johnetordoff:dataverse-file-list-take-2

Conversation

@Johnetordoff
Copy link
Copy Markdown
Contributor

@Johnetordoff Johnetordoff commented Jun 21, 2022

Purpose

Currently Dataverse/WB send two versions of datverse files with the same path to osf.io, this is problematic because our FE/API can't distinguish which file to retrieve when asked to list files at that path. This PR adds a special case for dataverse so both files are listed at that path and passes the versioning infomation to the FE so they can style the files accordingly.

This also removes an used branch of code by changing the logic of the info queryset retrieval code. Those are the changes in api/nodes/views.py.

Changes

  • add special MultipleObjectsReturned case for dataverse only
  • passes extra version info to FE
  • add tests

QA Notes

Please make verification statements inspired by your code and what your code touches.

  • Verify
  • Verify

What are the areas of risk?

Any concerns/considerations/questions that development raised?

Documentation

Side Effects

Ticket

https://openscience.atlassian.net/browse/ENG-3872

@Johnetordoff Johnetordoff marked this pull request as ready for review June 21, 2022 12:37
Comment thread api/base/views.py Outdated
Comment thread api/base/views.py Outdated
Comment thread api/base/views.py Outdated
Comment thread api/base/views.py Outdated
)
for dataverse_file in dataverse_files:
dataverse_file.update(None, attrs, user=self.request.user, save=False)
file_objs.append(dataverse_file)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I see that this is maintaining legacy behavior, but is it worth considering a follow-up fix to only return one version (based on the perms of request.user?)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's unclear, we need to discuss it more, kind of Product question as to how drafts are to be displayed.

@Johnetordoff Johnetordoff force-pushed the dataverse-file-list-take-2 branch from e4472ba to c6413f0 Compare June 21, 2022 17:32
Comment thread api_tests/files/views/test_file_list.py Outdated
def test_disambiguate_dataverse_paths_retrieve(self, app, user, node, dataverse, dataverse_draft_filenode, dataverse_published_filenode):
'''
This test is for retrieving files from Dataverse and disambiguating thier corresponding OSF filenodes and
ensures their `extra` info is passed along to the front-end.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Add note about why we still need to provide the data via WB even though FileNodes exist?

Comment thread api/base/views.py Outdated
@Johnetordoff Johnetordoff force-pushed the dataverse-file-list-take-2 branch from 52c885d to 9276bcd Compare June 21, 2022 19:10
@Johnetordoff Johnetordoff force-pushed the dataverse-file-list-take-2 branch from 4c14c64 to 2a39334 Compare June 22, 2022 14:32
Comment thread api/files/serializers.py Outdated
extras['downloads'] = obj.get_download_count()

if obj.provider == 'dataverse':
extras.update(obj.history[0]['extra'])
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why history[0] instead of history[-1]?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's just a dumb bug, sorry!

Comment thread api/nodes/views.py Outdated

# overrides ListAPIView
def get_queryset(self):
def param_queryset(self, query_params, default_queryset):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

My concern with this change is that we're taking the effort to evaluate the default_queryset and pass it in here only to ignore it in the info case -- and then we're going to try to annotate the result unnecessarily/incorrectly when it gets returned to get_queryset.

The old logic was a weird short-circuit for an arguably unneeded use case, but at least it was a short-circuit.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point I hadn't considered that the default QS would have to be queried anyway, I'll just take out that branch of logic.

@Johnetordoff Johnetordoff force-pushed the dataverse-file-list-take-2 branch from 7729b7e to 6039d3e Compare June 22, 2022 15:43
@jwalz jwalz merged commit 5f3c24b into CenterForOpenScience:develop Jun 22, 2022
Johnetordoff pushed a commit to Johnetordoff/osf.io that referenced this pull request Jun 26, 2022
…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
…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
…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
…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
…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
 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
 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
 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
 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
 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
 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
 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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants