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

[Django Upgrade] [ ENG-3948] Fix Draft Registration Serializer #10058

Conversation

cslzchen
Copy link
Contributor

@cslzchen cslzchen commented Sep 13, 2022

Purpose

Replaces #10034
Reference PR: #10057

Changes

Fixes for DRF MRO

For details of the DRF MRO change, refer to this issue and PR for DRF.

  • DraftRegistrationDetailSerializer inherits from DraftRegistrationSerializer and DraftRegistrationDetailLegacySerializer). Both of them define (or further inherits) registration_schema = RegistrationSchemaRelationshipField(...) with different params. The former inherits from DraftRegistrationLegacySerializer where required=True and read_only=False. The latter uses the default (both False). The DRF MRO upgrade led to DraftRegistrationDetailSerializer picking the wrong registration_schema to inherit and thus the fix is to define it here with the correct params.

  • Similarly, id = IDField(...) needs to be defined the same way since parents/grandparents have different definitions of the same field (eg. id = IDField(source='_id', required=True) vs id = IDField(source='_id', read_only=True). As expected, the wrong one is inherited after the DRF upgrade and the fix is to define it here.

Other fixes

  • Fixed an issue where .get_node() is not available from `self.context.get['view']

QA Notes

Documentation

Side Effects

Ticket

cslzchen and others added 3 commits September 13, 2022 13:28
Co-authored-by: John Tordoff <johnetordoff@gmail.com>
Recent DRF upgrade fixed a day-one bug which affects MRO. Have to
explicity define registration_schema to prevent this property from
inheriting from the wwrong parent.

Co-authored-by: John Tordoff <johnetordoff@gmail.com>
This fixed a similar bug caused by DRF MRO change.

Co-authored-by: John Tordoff <johnetordoff@gmail.com>
@cslzchen cslzchen force-pushed the fix-draft-registration-serializers branch from 979ef09 to 0bb5fc5 Compare September 13, 2022 17:59
@cslzchen cslzchen marked this pull request as ready for review September 13, 2022 18:49
@cslzchen cslzchen merged commit d69128c into CenterForOpenScience:feature/django_upgrade Sep 13, 2022
@cslzchen cslzchen changed the title [Django Upgrade] [ ENG-3498] Fix Draft Registration Serializer [Django Upgrade] [ ENG-3948] Fix Draft Registration Serializer Oct 3, 2022
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

1 participant