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

Remove test dependency on django guardian #7989

Closed
wants to merge 1 commit into from
Closed

Remove test dependency on django guardian #7989

wants to merge 1 commit into from

Conversation

jmsmkn
Copy link
Contributor

@jmsmkn jmsmkn commented May 16, 2021

Description

An ObjectPermissionBackend is added which is similar to the one
in guardian.backends.ObjectPermissionBackend for test support.
The differences are that:

  • Permissions can only be assigned to groups
  • The object permissions are stored in a defaultdict
  • Permissions can only be assigned using app_label.codename

This allows DRF to be released without waiting for django-guardian
to update, refs #3427, refs #7927, refs django-guardian/django-guardian#747.

An ObjectPermissionBackend is added which is similar to the one
in guardian.backends.ObjectPermissionBackend for test support.
The differences are that:
  - Permissions can only be assigned to groups
  - The object permissions are stored in a defaultdict
  - Permissions can only be assigned using app_label.codename

This allows DRF to be released without waiting for django-guardian
to update.
Copy link
Collaborator

@carltongibson carltongibson left a comment

Choose a reason for hiding this comment

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

I suspect this is probably a good idea. We check that we didn't break anything from the contract point of view. If anything actually were to break with Guardian, I imagine we'd hear about it.

@jmsmkn
Copy link
Contributor Author

jmsmkn commented May 17, 2021

The list view object permissions tests were moved to https://github.com/rpkilby/django-rest-framework-guardian/blob/master/tests/test_permissions.py when the DjangoObjectPermissionsFilter was extracted from DRF. That might be the logical place to add the other object permissions tests to check on integration with Guardian itself.

@johnthagen
Copy link
Contributor

This may no longer be necessary as django-guardian just published a release: #7927 (comment)

@tomchristie
Copy link
Member

I don't think we need this anymore, but thanks for your time on it all the same.

@tomchristie tomchristie closed this Sep 3, 2021
@jmsmkn jmsmkn deleted the remove_django_guardian branch September 3, 2021 13:51
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.

4 participants