Re: [Django] #8936: Add view (read-only) permission to admin
Django
-------------------------------+--------------------------------------
Reporter: simon | Owner: PetrDlouhy
Type: New feature | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------------------------
Changes (by timgraham):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/8936#comment:38>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: simon | Owner: Petr Dlouhý
Type: New feature | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Olivier):
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/8936#comment:39>
Django
-------------------------------+---------------------------------------
Reporter: simon | Owner: Petr Dlouhý
Type: New feature | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+---------------------------------------
* needs_better_patch: 0 => 1
Comment:
There are comments for improvement from Nick Pope on the PR.
--
Ticket URL: <https://code.djangoproject.com/ticket/8936#comment:40>
Django
-------------------------------+------------------------------------
Type: New feature | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
* owner: Petr Dlouhý => Olivier
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/8936#comment:41>
Django
-------------------------------+------------------------------------
Reporter: simon | Owner: Olivier
Type: New feature | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
* cc: Jeppe Vesterbæk (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/8936#comment:42>
Django
-------------------------------+------------------------------------
Reporter: simon | Owner: Olivier
Type: New feature | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/8936#comment:43>
Django
-------------------------------+------------------------------------
Reporter: simon | Owner: Olivier
Type: New feature | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by viktor25):
I'm copying here my comment from the pull request:
It is important to be able to see the read-only page even if one has
change permission, because long-term the read-only page will be more
helpful than the form. For example, the form shows foreign keys as
dropdowns, while the read-only page could show them as links to the
foreign object. The read-only page is not constrained by the need to show
input widgets, so it is more powerful for visualizing information.
--
Ticket URL: <https://code.djangoproject.com/ticket/8936#comment:44>
Django
-------------------------------+------------------------------------
Reporter: simon | Owner: Olivier
Type: New feature | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
* cc: Carlton Gibson (added)
* needs_better_patch: 1 => 0
Comment:
Unchecking "Patch needs improvement" to add this back to the review queue.
I will review over the next week with a possible inclusion (or not) in 2.1
in mind.
--
Ticket URL: <https://code.djangoproject.com/ticket/8936#comment:45>
Django
-------------------------------+------------------------------------
Reporter: simon | Owner: Olivier
Type: New feature | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Changes (by Carlton Gibson):
* needs_better_patch: 0 => 1
Comment:
Having reviewed, there are a few small changes needed.
Olivier, please uncheck "Patch needs improvement" when you've taken those
on. (Feel free to @menion me on GitHub if you want input or help!)
--
Ticket URL: <https://code.djangoproject.com/ticket/8936#comment:46>
Django
Reporter: simon | Owner: Olivier
Type: New feature | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin
Comment:
This looks ready to go now. A lot of points have been picked up and all
addressed.
It's a big patch, with security implications, so a further review is
justified.
These seem like the main areas:
* Manual testing looks good.
* Tests on permission checks when POSTing have been added.
* Permission checks on accessing inlines are also tested.
--
Ticket URL: <https://code.djangoproject.com/ticket/8936#comment:47>
Django
-------------------------------------+-------------------------------------
Reporter: simon | Owner: Olivier
Type: New feature | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham):
I pushed a commit with some edits. I'm okay with merging this tomorrow,
although I find the tests a bit lacking. In particular, many of the
template changes are untested. I added some `assertContains()` assertions
but more are needed. This could be done after the alpha release, I
suppose.
--
Ticket URL: <https://code.djangoproject.com/ticket/8936#comment:48>
Django
-------------------------------------+-------------------------------------
Reporter: simon | Owner: Olivier
Component: contrib.admin | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"825f0beda804e48e9197fcf3b0d909f9f548aa47" 825f0be]:
{{{
#!CommitTicketReference repository=""
revision="825f0beda804e48e9197fcf3b0d909f9f548aa47"
Fixed #8936 -- Added a view permission and a read-only admin.
Co-authored-by: Petr Dlouhy <petr....@email.cz>
Co-authored-by: Olivier Dalang <olivier...@gmail.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/8936#comment:49>
Django
-------------------------------------+-------------------------------------
Reporter: simon | Owner: Olivier
Type: New feature | Status: closed
Component: contrib.admin | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Olivier):
Replying to [comment:48 Tim Graham]:
> I'm okay with merging this tomorrow, although I find the tests a bit
lacking. In particular, many of the template changes are untested. I added
some `assertContains()` assertions but more are needed. This could be done
after the alpha release, I suppose.
Thanks for the merge ! Yes I think I could spare some time to add some
tests. But would you have some concrete examples of tests you'd like to
have added ?
--
Ticket URL: <https://code.djangoproject.com/ticket/8936#comment:50>
Django
-------------------------------------+-------------------------------------
Reporter: simon | Owner: Olivier
Type: New feature | Status: closed
Component: contrib.admin | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham):
Try reverting various changes in templates and you'll find that no tests
fail.
--
Ticket URL: <https://code.djangoproject.com/ticket/8936#comment:51>
Django
-------------------------------------+-------------------------------------
Reporter: simon | Owner: Olivier
Type: New feature | Status: closed
Component: contrib.admin | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"b30f9b131c9489b9d9f21c311ecb46d0aea91381" b30f9b13]:
{{{
#!CommitTicketReference repository=""
revision="b30f9b131c9489b9d9f21c311ecb46d0aea91381"
Refs #29419, #8936 -- Removed change permission requirement for admin
actions.
Partially reverted 825f0beda804e48e9197fcf3b0d909f9f548aa47.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/8936#comment:52>
Django
-------------------------------------+-------------------------------------
Reporter: simon | Owner: Olivier
Type: New feature | Status: closed
Component: contrib.admin | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"aea0e2b9ca8376c1491f98262e20ed6069c70e5e" aea0e2b9]:
{{{
#!CommitTicketReference repository=""
revision="aea0e2b9ca8376c1491f98262e20ed6069c70e5e"
[2.1.x] Refs #29419, #8936 -- Removed change permission requirement for
admin actions.
Partially reverted 825f0beda804e48e9197fcf3b0d909f9f548aa47.
Backport of b30f9b131c9489b9d9f21c311ecb46d0aea91381 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/8936#comment:53>
Django
-------------------------------------+-------------------------------------
Reporter: simon | Owner: Olivier
Type: New feature | Status: closed
Component: contrib.admin | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"cd790ed1a6dbdf910c41da44c306ddae6ea6fd4d" cd790ed1]:
{{{
#!CommitTicketReference repository=""
revision="cd790ed1a6dbdf910c41da44c306ddae6ea6fd4d"
Refs #8936 -- Added ModelAdmin.has_view_or_change_permission().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/8936#comment:54>
Django
-------------------------------------+-------------------------------------
Reporter: simon | Owner: Olivier
Type: New feature | Status: closed
Component: contrib.admin | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"f5fbff0b4e87973e903c6cc63a4fb6bb682c52c0" f5fbff0b]:
{{{
#!CommitTicketReference repository=""
revision="f5fbff0b4e87973e903c6cc63a4fb6bb682c52c0"
[2.1.x] Refs #8936 -- Added ModelAdmin.has_view_or_change_permission().
Backport of cd790ed1a6dbdf910c41da44c306ddae6ea6fd4d from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/8936#comment:55>