Skip to content

[ENG-8148] Add ArtifactOutcome in annotations to linked nodes #11158

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
Merged
merged 2 commits into from
May 30, 2025
Merged

[ENG-8148] Add ArtifactOutcome in annotations to linked nodes #11158

merged 2 commits into from
May 30, 2025

Conversation

Johnetordoff
Copy link
Contributor

@Johnetordoff Johnetordoff commented May 29, 2025 •
edited
Loading

Purpose

  • Add ArtifactOutcome attributes to linked registrations

Changes

  • add annotations to linked registration views
  • add test cases
  • split up tests to keep < 500 ln, clean them up and make more atomic

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-8148

@Johnetordoff Johnetordoff force-pushed the add-has-data-annotations-to-linked-nodes branch 7 times, most recently from bc4a980 to 1b22ecf Compare May 29, 2025 19:58
@Johnetordoff Johnetordoff force-pushed the add-has-data-annotations-to-linked-nodes branch from 1b22ecf to cddab29 Compare May 29, 2025 20:29
@Johnetordoff Johnetordoff marked this pull request as ready for review May 29, 2025 21:05
brianjgeiger
brianjgeiger reviewed May 30, 2025
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.

Just one question about non-registration linked nodes.

api/base/views.py

return (
self.get_node().linked_nodes
.annotate(**resource_annotations.make_open_practice_badge_annotations())
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since this is in the BaseLinkedList rather than a registration specific one (which I'm guessing doesn't exist), I presume we do have test for linked projects not just linked registrations that are working fine even with this annotation, yes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only two new test case changes test_linked_by_registrations_links_registrations_artifacts and test_node_registrations_links_registrations_artifacts, but I can add test cases for the LinkedNodeList and preprints as well.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If there aren't already any tests for the linked nodes, then yeah, let's add a case or two just to make sure, please.

@Johnetordoff Johnetordoff force-pushed the add-has-data-annotations-to-linked-nodes branch from 5fc50b4 to 4778068 Compare May 30, 2025 14:30
@Johnetordoff
Copy link
Contributor Author

@brianjgeiger Since all the classes where just overriding the get_queryset anyway and invalidating any re-usability of the BaseLinkedList class, instead of add annotations which would be custom for each sub-classes get_queryset method I just made them no longer subclasses of BaseLinkedList. I can understand how this would seem like a non-solution, but I wanted change less test behavior then I had to.

@brianjgeiger brianjgeiger merged commit e000c5b into CenterForOpenScience:feature/pbs-25-10 May 30, 2025
6 checks passed
Johnetordoff added a commit to Johnetordoff/osf.io that referenced this pull request Jun 2, 2025
@Johnetordoff
Merge branch 'feature/pbs-25-10' of https://github.com/CenterForOpenS…
b4f1c3e 
…cience/osf.io into feature/notification-refactor-phase-1

* 'feature/pbs-25-10' of https://github.com/CenterForOpenScience/osf.io:
  fix issue where trying another already confirmed email threw an uncaught exception (CenterForOpenScience#11161)
  [ENG-8148] Add ArtifactOutcome in annotations to linked nodes  (CenterForOpenScience#11158)
  [ENG-7966] Add "collected-in" relationship for Nodes (CenterForOpenScience#11140)
  fix issue where not having any external identities caused a 500

# Conflicts:
#	api_tests/nodes/views/test_node_detail.py
#	api_tests/nodes/views/test_node_linked_registrations.py
Johnetordoff added a commit to Johnetordoff/osf.io that referenced this pull request Jun 3, 2025
@Johnetordoff
Merge branch 'feature/pbs-25-10' of https://github.com/CenterForOpenS…
358091c 
…cience/osf.io into add-new-notifications-data-model

* 'feature/pbs-25-10' of https://github.com/CenterForOpenScience/osf.io:
  fix issue where trying another already confirmed email threw an uncaught exception (CenterForOpenScience#11161)
  [ENG-8148] Add ArtifactOutcome in annotations to linked nodes  (CenterForOpenScience#11158)
Johnetordoff added a commit to Johnetordoff/osf.io that referenced this pull request Jun 3, 2025
@Johnetordoff
Merge branch 'feature/pbs-25-10' of https://github.com/CenterForOpenS…
e8d1370 
…cience/osf.io into refactor-notifications

* 'feature/pbs-25-10' of https://github.com/CenterForOpenScience/osf.io:
  fix issue where trying another already confirmed email threw an uncaught exception (CenterForOpenScience#11161)
  [ENG-8148] Add ArtifactOutcome in annotations to linked nodes  (CenterForOpenScience#11158)
  [ENG-7966] Add "collected-in" relationship for Nodes (CenterForOpenScience#11140)
  fix issue where not having any external identities caused a 500
  [ENG-7965] Add v2 email token confirmation endpoints (CenterForOpenScience#11139)
  [ENG-8052] Fixed FilterMixin issue with multiple values of notification subscription field (CenterForOpenScience#11150)
  support related_counts for view_only links (CenterForOpenScience#11148)
  allow admins change registration providers (CenterForOpenScience#11145)
  [ENG-7927] Improved logging for embargo termination (CenterForOpenScience#11137)
  [ENG-7873] CLONE - SPAM - When Hamming a Spammed user, preprints and registrations remain private (CenterForOpenScience#11125)
  Update changelog and package.json
  fix TypeError when check stucked registration
  revert async email sending (CenterForOpenScience#11134)
  [ENG-7921] Add scopes for applications to full_read and full_write scopes (CenterForOpenScience#11126)

# Conflicts:
#	api_tests/nodes/views/test_node_detail.py
#	api_tests/nodes/views/test_node_linked_registrations.py
#	framework/auth/oauth_scopes.py
#	tests/test_registrations/test_retractions.py
Johnetordoff added a commit to Johnetordoff/osf.io that referenced this pull request Aug 4, 2025
@Johnetordoff
Merge branch 'feature/pbs-25-10' of https://github.com/CenterForOpenS…
91f4eb8 
…cience/osf.io into add-has-data-annotations-to-linked-nodes

* 'feature/pbs-25-10' of https://github.com/CenterForOpenScience/osf.io:
  Revert "[ENG-8048] Remove caching to avoid incorrect results for ascendants (…"
  Revert "fixed None issue when iterate (CenterForOpenScience#11192)"
  fixed None issue when iterate (CenterForOpenScience#11192)
  [ENG-8048] Remove caching to avoid incorrect results for ascendants (CenterForOpenScience#11169)
  [ENG-7870] Crossref DOIs not minting with _v1, OSF is displaying DOI versions with _v1 (CenterForOpenScience#11154)
  Update changelog and package.json
  [ENG-8145] [ENG-8147] Manual DOI and GUID for Preprints & Registrations - BE (CenterForOpenScience#11174)
  Update hybrid values for new workflow check (CenterForOpenScience#11166)
  fix issue where trying another already confirmed email threw an uncaught exception (CenterForOpenScience#11161)
  [ENG-8148] Add ArtifactOutcome in annotations to linked nodes  (CenterForOpenScience#11158)
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
@Johnetordoff @brianjgeiger