Skip to content

Validation of included_resources fails when nested #949

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

Closed
2 tasks done
sliverc opened this issue Jun 5, 2021 · 3 comments · Fixed by #956
Closed
2 tasks done

Validation of included_resources fails when nested #949

sliverc opened this issue Jun 5, 2021 · 3 comments · Fixed by #956
Labels

Comments

@sliverc
Copy link
Member

sliverc commented Jun 5, 2021

Description of the Bug Report

#900 introduced validation of included_resources but causes a regression that when one of included_serializers of the current serializer of called view uses included_resources it will fail.

I assume validation of included_resources should actually be done in the serializer meta class and only query parameters validated in IncludedResourcesValidationMixin.

See discussion #318 where there is also some code to reproduce it.

Checklist

  • Certain that this is a bug (if unsure or you have a question use discussions instead)
  • Code snippet or unit test added to reproduce bug
@sliverc sliverc added the bug label Jun 5, 2021
@sliverc
Copy link
Member Author

sliverc commented Jun 5, 2021

As a follow up I question whether it is a good idea to validate include query parameters in init of a serializer as init can be called anytime. Better to be moved to QueryParameterValidationFilter.

@jking6884
Copy link

@sliverc Is this at all related to this problem I am facing. I just upgraded a django 1.11 app up to latest and upgrded all django related dependencies to latest at the same time.

I am hitting an endpoint that maps to the AccountLicenseViewSet (I included the classes below). It gets and serializes the initial queryset fine. However, when it gets into the restframeworkjsonapi renderer. While initializing the related serializer, field = serializer_class(relation_instance, many=many, context=context), it calls this_serializer_class = view.get_serializer_class() to, but this returns the AccountLicenseSerializer rather than the AccountSerializer. So when gets into the validate_path method, it fails and throws an error because it is trying to look up the related serializer on the AccountLicenseSerializer instead of the AccountSerializer.

This is all working on perfectly on the 1.11 django project that is still running on another computer I have. But it is now failing here. Is this possibly related to the issue you mentioned?

image

AccountLicenseViewSet

class AccountLicenseViewSet(BaseViewSet):
    serializer_class = AccountLicenseSerializer
    model = AccountLicense
    filter_fields = ['account__id']
    permission_classes = [IsAuthenticated, Or(IsReadOnlyRequest, IsAdmin, IsSameUser)]

AccountLicenseSerializer

class AccountLicenseSerializer(serializers.ModelSerializer):
    included_serializers = {
        'account': 'core.serializers.AccountSerializer',
        'app': 'core.serializers.AppSerializer'
    }

    app_id = serializers.SerializerMethodField()

    class Meta:
        model = AccountLicense
        fields = '__all__'

    class JSONAPIMeta:
        included_resources = ['account', 'app']

    def get_app_id(self, obj):
        return obj.app.id
class AccountSerializer(serializers.ModelSerializer):
    included_serializers = {
        'account_account_users': AccountUserSerializer,
        'account_license': AccountLicenseSerializer
    }

    account_account_users = ResourceRelatedField(
        queryset=AccountUser.objects.filter(role="owner"),
        # read_only=True,
        many=True
    )
    account_license = ResourceRelatedField(
        model=AccountLicense,
        read_only=True,
        many=True
    )

    class Meta:
        model = Account
        fields = '__all__'

    class JSONAPIMeta:
        included_resources = ['account_account_users', 'account_license']

@sliverc
Copy link
Member Author

sliverc commented Jun 25, 2021

@jking6884 Most likely this is the same issue you are facing then this one. Best try version DJA 4.1.0 for now till this issue has been addressed.

As a note for Django 3.2 support there were no changes needed so DJA 4.1.0 should also run with the newest Django version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants