search

jazzband / django-authority

A Django app that provides generic per-object-permissions for Django's auth app and helpers to create custom permission checks.
http://django-authority.readthedocs.org
BSD 3-Clause "New" or "Revised" License
292 stars 57 forks source link

Fixes for generic foreign key raw id widget #69

Open mikebq opened 4 years ago

mikebq commented 4 years ago

I am porting a Django app to Django 1.11 and found some issues in the latest version of django-authority while doing so.

The class GenericForeignKeyRawIdWidget has ForeignKeyRawIdWidget as a base class but its __init__ method calls forms.TextInput.__init__(I am guessing that this is why its class name starts with the word "Generic"). The following PR is my attempt to address errors when attempting to view an individual permission from the permissions list view.

The first commit on this branch addresses an issue that occurs when render is called on this widget. During render a call is made to forms.TextInput.render which then calls self.get_context. Unfortunately this goes to ForeignKeyRawIdWidget.get_context which then blows up as it the instance of the class is missing members that would have been added by ForeignKeyRawIdWidget.__init__. In order to avoid this I have added a get_context which simply delegates to forms.TextInput.get_context (resolves up; but it is calling into something that has been initialised).

The second commit addresses an issue that occurs when clicking the anchor of class related-lookup from the rendered widget. In the app that I am porting from Django 1.6.11, using django-authority 0.10 to Django 1.11 using the latest version of django-authority there are differences in the form of the URL where the widget is rendered. With the old version of Django and Django-Authority the path of the URL is like this admin/authority/permission/21852/ where as with Django 1.11 and latest Django-Authority I am seeing admin/authority/permission/21852/change/. Thus having related_url hard coded to "../../../" is not going to work for both paths. What I have done is past the request object into the widget so that it can work out how many ../ it should use to get back to the root of the "related" path.

If you think this PR is off course or wrong could you please let me know :) thank you.

codecov[bot] commented 4 years ago

Codecov Report

Merging #69 into master will increase coverage by 4.64%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #69      +/-   ##
==========================================
+ Coverage      39%   43.64%   +4.64%     
==========================================
  Files           7        7              
  Lines         423      433      +10     
  Branches       78       79       +1     
==========================================
+ Hits          165      189      +24     
+ Misses        250      233      -17     
- Partials        8       11       +3
Impacted Files Coverage Δ
authority/widgets.py 87.17% <100%> (+52.69%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 3677b42...be36893. Read the comment docs.