In https://github.com/mozilla/addons-server/pull/14159 I had to copy some code from a pull request I made to DRF to fix an issue around passing code from a failed permission to the exception being raised.
My pull request has been merged into DRF so once they release a new version (should be 3.12) with those changes we can remove the copy/pasting. The code that should be removed this block in src/olympia/signing/views.py:
# When DRF 3.12 is released, we can remove the custom check_permissions()
# and permission_denied() as it will contain the fix for
# https://github.com/encode/django-rest-framework/issues/7038
def check_permissions(self, request):
"""
Check if the request should be permitted.
Raises an appropriate exception if the request is not permitted.
(Lifted from DRF, but also passing the code argument down to the
permission_denied() call if that property existed on the failed
permission class)
"""
for permission in self.get_permissions():
if not permission.has_permission(request, self):
self.permission_denied(
request, message=getattr(permission, 'message', None),
code=getattr(permission, 'code', None),
)
def permission_denied(self, request, message=None, code=None):
"""
If request is not permitted, determine what kind of exception to raise.
(Lifted from DRF, but also passing the optional code argument to
the PermissionDenied exception)
"""
if request.authenticators and not request.successful_authenticator:
raise exceptions.NotAuthenticated()
raise exceptions.PermissionDenied(
detail=message, code=code)
3.12 is released and we're on 3.12.1 now.
If this is your first contribution, please refer to https://wiki.mozilla.org/Add-ons/Contribute/Code on how to get started.
Mentor: @eviljeff
@eviljeff Can I pick this up ?
@eviljeff Can I pick this up ?
okay
So basically , we just have to remove these two functions check_permissions() and permission_denied() from src/olympia/signing/views.py as it already contain the fix in #7078 . I don't think so , but if any other file need to be change then please mentor me .
yes. It's an easy fix - that's why it's a good first bug.
Thanks ! I will land a PR today.
Hey @Sparsh-Bansal, thanks for your enthusiasm! Now that you've gotten a feel for the codebase, why don't you try tackling some of the harder issues labeled contrib: welcome once this PR is merged?
Yeah, sure.
Please add STR if QA is required, otherwise add a qa:not needed label.
Thanks
Most helpful comment
yes. It's an easy fix - that's why it's a good first bug.