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

Fixed #27991 -- Add 'obj' kwarg to InlineModelAdmin.has_add_permission() #8304

Closed
wants to merge 3 commits into from

Conversation

icu0755
Copy link

@icu0755 icu0755 commented Apr 6, 2017

@icu0755 icu0755 force-pushed the ticket_27991 branch 2 times, most recently from 32e5d35 to 955cbf6 Compare April 6, 2017 10:48
@@ -2220,6 +2217,21 @@ The ``InlineModelAdmin`` class adds:
inline forms. For example, this may be based on the model instance
(passed as the keyword argument ``obj``).

.. method:: InlineModelAdmin.has_add_permission(self, request, obj=None)

The ``has_add_permission`` method is given the HttpRequest and the parent ``obj``.
Copy link
Member

Choose a reason for hiding this comment

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

Please wrap documentation at 79 characters.

inline.has_change_permission(request, obj) or
inline.has_delete_permission(request, obj)):
continue
if not inline.has_add_permission(request):
if not inline.has_add_permission(request, obj):
Copy link
Member

Choose a reason for hiding this comment

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

As mentioned in the ticket we'll want to use an approach similar to a7c256c to support inline.has_add_permission(request) for a certain time.

@@ -617,7 +617,6 @@ def test_log_actions(self):


class ModelAdminPermissionTests(SimpleTestCase):

Copy link
Member

Choose a reason for hiding this comment

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

Please don't make unrelated whitespace changes.

The ``has_add_permission`` method is given the HttpRequest and the parent ``obj``.
Should return ``True`` if adding an instance is permitted, ``False`` otherwise.

.. method:: InlineModelAdmin.has_change_permission(self, request, obj=None)
Copy link
Member

Choose a reason for hiding this comment

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

Doc changes for has_change_permission/has_delete_permission seem unrelated to the change.

Copy link
Author

Choose a reason for hiding this comment

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

As mentioned in the ticket it is not clear that obj refers to the parent instance, and not the inline instance. The documentation just points to the ModelAdmin.has_add_permission, ModelAdmin.has_change_permission, ModelAdmin.has_delete_permission though the actual use case differs.

@icu0755 icu0755 closed this Apr 7, 2017
@icu0755 icu0755 deleted the ticket_27991 branch April 7, 2017 13:42
@icu0755 icu0755 restored the ticket_27991 branch April 7, 2017 13:47
@icu0755 icu0755 deleted the ticket_27991 branch April 7, 2017 13:54
…ion()

- Backward compatibility for old api
- Deprecated warning for the old api
- Updated tests
- Updated releases/2.0.txt
@icu0755 icu0755 reopened this Apr 7, 2017
…ion()

- Fixed documentations links
- Removed duplicated test
@jdufresne
Copy link
Member

Hi @icu0755 Are you still working on this PR? I'm interested in this feature. If you're too busy to continue or have otherwise moved on, I can continue the work if you'd like. If you'd like to continue the work, that works too.

@icu0755
Copy link
Author

icu0755 commented Feb 21, 2018

Hi @jdufresne. Please feel free to continue or start from scratch. Unfortunately I don't know how to proceed further.

@jdufresne
Copy link
Member

Thanks @icu0755 and thanks for the initial patch. I've created a new PR at #9721.

@timgraham timgraham closed this Feb 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants