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

[ENG-5845] Preprint Institutions Relationship #10659

Conversation

Johnetordoff
Copy link
Contributor

@Johnetordoff Johnetordoff commented Jul 5, 2024

Purpose

To transfer NodeInstitutionsRelationshipList logic to Preprints from nodes with adjoining classes.

Changes

  • WriteOrPublicForRelationshipInstitutions is moved and refactored from being exclusively for nodes into a all resources
  • Adapt NodeInstitutionsRelationshipList view to PreprintInstitutionsRelationshipList
  • Adapt NodeInstitutionsRelationshipSerializer to PreprintsInstitutionsRelationshipSerializer

TODO

  • remove references to -list
  • write POST and DELETE tests
  • move update_institutions to base
  • fix test class name
  • more institutions created
  • more complex test cases with institutional affiliation.
    • 3 affiliation and 2 remain after user removes one.
    • User 2 affiliations with one they don't have.

QA Notes

N/A

Documentation

N/A

Side Effects

N/A

Ticket

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

@Johnetordoff Johnetordoff changed the base branch from develop to feature/preprints-affiliations July 5, 2024 13:56
@Johnetordoff Johnetordoff force-pushed the preprint-insetitutions-relationship-list-api branch from 27db54a to 60bfb27 Compare July 5, 2024 14:28
@Johnetordoff Johnetordoff force-pushed the preprint-insetitutions-relationship-list-api branch 2 times, most recently from 660d92d to e3425bf Compare July 5, 2024 15:01
@Johnetordoff Johnetordoff force-pushed the preprint-insetitutions-relationship-list-api branch from e3425bf to 07bf405 Compare July 5, 2024 15:03
@Johnetordoff Johnetordoff changed the title [WIP] Preprint insetitutions Relationship List api [ENG-5845] Preprint insetitutions Relationship List api Jul 5, 2024
@Johnetordoff Johnetordoff changed the title [ENG-5845] Preprint insetitutions Relationship List api [ENG-5845][WIP] Preprint insetitutions Relationship List Jul 5, 2024
…enterForOpenScience/osf.io into preprint-insetitutions-relationship-list-api

* 'feature/preprints-affiliations' of https://github.com/CenterForOpenScience/osf.io:
  remove literal string permissions
  add docstring to explain permission
  improve preprint tests by creating 404 case
  improve permissions by dropping unsafe method behavior
  remove vestigial serializer

# Conflicts:
#	api/preprints/permissions.py
#	api/preprints/serializers.py
#	api/preprints/views.py
#	api_tests/preprints/views/test_preprint_institutions.py
@Johnetordoff Johnetordoff force-pushed the preprint-insetitutions-relationship-list-api branch from 55f6105 to 268d5c6 Compare July 5, 2024 16:39
@Johnetordoff Johnetordoff changed the title [ENG-5845][WIP] Preprint insetitutions Relationship List [ENG-5845][WIP] Preprint insetitutions Relationship Jul 5, 2024
api/preprints/views.py Outdated Show resolved Hide resolved
@Johnetordoff Johnetordoff changed the title [ENG-5845][WIP] Preprint insetitutions Relationship [ENG-5845][WIP] Preprint Institutions Relationship Jul 8, 2024
@Johnetordoff Johnetordoff force-pushed the preprint-insetitutions-relationship-list-api branch from eff1e35 to 164aa66 Compare July 8, 2024 14:34
@Johnetordoff Johnetordoff force-pushed the preprint-insetitutions-relationship-list-api branch from c54fbb1 to 8adc2df Compare July 12, 2024 18:07
@Johnetordoff Johnetordoff marked this pull request as ready for review July 12, 2024 18:07
Copy link
Contributor

@cslzchen cslzchen left a comment

Choose a reason for hiding this comment

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

Looks great overall and nice work 👍 . Have a few questions + some style nitpicking. First pass done and I will do some local testing locally.

api/preprints/views.py Outdated Show resolved Hide resolved
api/preprints/views.py Show resolved Hide resolved
api/registrations/serializers.py Outdated Show resolved Hide resolved
api/institutions/utils.py Outdated Show resolved Hide resolved
api/institutions/utils.py Outdated Show resolved Hide resolved
api/preprints/serializers.py Show resolved Hide resolved
@Johnetordoff Johnetordoff changed the title [ENG-5845][WIP] Preprint Institutions Relationship [ENG-5845] Preprint Institutions Relationship Jul 16, 2024
Copy link
Contributor

@cslzchen cslzchen left a comment

Choose a reason for hiding this comment

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

Looks great and thanks for the quick response 🎆 I will make a couple of trivial style changes + adding some comments / DocStr and merge it soon.

Copy link
Contributor

@cslzchen cslzchen left a comment

Choose a reason for hiding this comment

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

Pushing my changes now and will merge after CI passes

api/institutions/utils.py Outdated Show resolved Hide resolved
api/preprints/serializers.py Outdated Show resolved Hide resolved
@cslzchen cslzchen merged commit 231b478 into CenterForOpenScience:feature/preprints-affiliations Jul 18, 2024
6 checks passed
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.

None yet

2 participants