Opened 8 years ago

Last modified 3 weeks ago

#8936 assigned New feature

admin databrowse (read-only view-only permissions)

Reported by: simon Owned by: PetrDlouhy
Component: contrib.admin Version: master
Severity: Normal Keywords:
Cc: charettes, PetrDlouhy, aarcro, cmawebsite@…, manelclos@…, serhiy.int@…, jeroen@…, olivier.dalang@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Adrian and Jacob on stage at DjangoCon.

Attachments (2)

options.diff (2.5 KB) - added by riccardodivirgilio 5 years ago.
django.contrib.admin.options.py
options.py (63.4 KB) - added by riccardodivirgilio 5 years ago.

Download all attachments as: .zip

Change History (38)

comment:1 Changed 7 years ago by jacob

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Someday/Maybe

comment:2 Changed 7 years ago by thejaswi_puthraya

  • Component changed from Uncategorized to Contrib apps
  • Keywords django pony added

comment:3 Changed 5 years ago by gabrielhurley

  • Component changed from Contrib apps to contrib.databrowse

comment:4 Changed 5 years ago by lukeplant

  • Severity set to Normal
  • Type set to New feature

comment:5 Changed 5 years ago by aaugustin

  • Easy pickings unset
  • UI/UX unset

A summary is here: https://code.djangoproject.com/wiki/SummerOfCode2011#Integratedatabrowseintotheadmin
This project didn't happen in 2011.

comment:6 Changed 5 years ago by julien

See also #15956 for a list of specific problems when trying to use the admin merely for browsing data.

comment:7 Changed 5 years ago by ptone

  • Component changed from contrib.databrowse to contrib.admin
  • Summary changed from Integrate databrowse functionality in to the admin to add databrowse-like functionality to the admin

Since Databrowse is now deprecated, this should be considered as a proposed enhancement to the admin

Databrowse is now deprecated, see #16907

Changed 5 years ago by riccardodivirgilio

django.contrib.admin.options.py

Changed 5 years ago by riccardodivirgilio

comment:8 Changed 5 years ago by anonymous

hi, i've added a small patch to django.contrib.admin.options

i've tested it and it works, it's pretty basic and there is more work to do.

what i've done is to add a method has_view_permission, and check a view permission in every view

i also created a get_list_editable, and i add in get_list_editable and in get_readonly_fields a check, and it returns original read_only and list_editable only if user has got a change_pemission

i also raise a permission error every time there is a post request and user do not have a change_permission

a also ask for view_permission in history_view.

todo:

fix inlines
fix submit_row

add a opts.has_view_permission()

comment:10 Changed 3 years ago by naktinis@…

What is the current status of this one? Is the provided patch of any help or do we need a new patch?

comment:11 Changed 3 years ago by timo

It's probably best to start from scratch on this one. The patch is in a weird format for one, and it's difficult for me to tell what's going on with it. Instead of just diving in and writing code for this, what we really need to move forward with this is a well thought out design document that can be reviewed. See Aymeric's post on the mailing list thread above for more details.

comment:12 Changed 3 years ago by charettes

I think a simple POC could be implemented by doing the following:

  1. Create a view default permission.
  2. Add has_view_permission methods to BaseModelAdmin and InlineModelAdmin.
  3. Use the built-in readonly_fields logic to provide views of models.
  4. Do the same thing with the listing of models.
  5. Add a new icon to link to the read-only version of models.

Then some permission checks will need to be changed in order to fallback to the view mode of models if the user doesn't have the change permission but has the view one.

comment:13 Changed 3 years ago by charettes

  • Cc charettes added

comment:14 Changed 2 years ago by anonymous

It's weird to see a feature that was requested 6 years ago, by multiple people, still not being implemented. Instead, the "admins" refused this a number of times, calling it a pony, and generally ignoring what actual users want. It's very sad.

comment:15 Changed 2 years ago by timo

If you want to see this, make a proposal (comment 12 looks good) and implement it with tests and documentation. That will be more useful than lamenting the fact that no one has done it.

comment:16 Changed 15 months ago by PetrDlouhy

Hi,

I have started to work on this issue. It is not complete yet, so any help with this will be appreciated. The commit is following:
https://github.com/PetrDlouhy/django/commit/19b5582c8604cb8e96914aa382de18f2a1060ea7

  • It should not break current permission model, so change permission also qualifies to view data in admin.
  • All fields are made readonly, "Safe" button is hidden, and POST requests are denied without change permission.
  • There is "change" link on index admin page for all models which links to list view. The name not very accurate, but I didn't rename it nor change the icon.
  • Tests should be written for this.

I am new to Django contributing, so if someone helps me with finishing this and making it part of the project, I would be very glad.

comment:17 Changed 15 months ago by PetrDlouhy

  • Cc PetrDlouhy added
  • Has patch set
  • Patch needs improvement set

Hello,

I have added few corrections to my patch, fixed tests, added new tests and made few updates to the documentation. I don't think, that all issues connected with this are solved yet. Anyway, I would like my work to be reviewed.

The new patches are part of (yet uncreated) pull request:
https://github.com/django/django/compare/master...PetrDlouhy:view_permission_master?expand=1

There few weak spots, that I am feeling that should be discussed with someone more experienced:

  • Model situation: User has defined ModelAdmin with custom has_change_permission() such that it disallows to change (consequently also view) anything.
    • Then adding view permissions allows superuser to view that ModelAdmin.
    • I had to change RowLevelChangePermissionModelAdmin in tests for this.
  • How should change RelatedFieldWidgetWrapper

comment:18 Changed 11 months ago by PetrDlouhy

As advised on Django development mailing list, I have made PR for this ticket:
https://github.com/django/django/pull/5108

comment:19 Changed 11 months ago by timgraham

  • Keywords django pony removed
  • Patch needs improvement unset
  • Triage Stage changed from Someday/Maybe to Accepted
  • Version changed from 1.0 to master

comment:20 Changed 10 months ago by jarshwah

  • Triage Stage changed from Accepted to Ready for checkin

comment:21 Changed 10 months ago by timgraham

  • Patch needs improvement set
  • Triage Stage changed from Ready for checkin to Accepted

I left some comments for improvement on the pull request.

comment:22 Changed 10 months ago by PetrDlouhy

  • Patch needs improvement unset
  • Triage Stage changed from Accepted to Ready for checkin

comment:23 Changed 10 months ago by timgraham

  • Patch needs improvement set
  • Triage Stage changed from Ready for checkin to Accepted

Please uncheck "Patch needs improvement" when you update the pull request, but don't promote it to "Ready for checkin" until someone has reviewed the changes. Thanks!

comment:24 Changed 10 months ago by aarcro

  • Cc aarcro added

comment:25 Changed 9 months ago by PetrDlouhy

  • Owner changed from nobody to PetrDlouhy
  • Patch needs improvement unset
  • Status changed from new to assigned

comment:27 Changed 8 months ago by timgraham

  • Patch needs improvement set

comment:28 Changed 8 months ago by collinanderson

  • Cc cmawebsite@… added
  • Summary changed from add databrowse-like functionality to the admin to admin databrowse (read-only view-only permissions)

comment:29 Changed 8 months ago by collinanderson

This would solve #11561 (raw_id_fields requires that the user has change permissions on the model class that is being linked to)

comment:30 Changed 8 months ago by manelclos

  • Cc manelclos@… added

comment:31 Changed 5 months ago by PetrDlouhy

  • Patch needs improvement unset

comment:32 Changed 4 months ago by int-ua

  • Cc serhiy.int@… added

comment:33 Changed 3 months ago by collinanderson

  • Patch needs improvement set

(I believe this still needs work.)

comment:34 Changed 3 months ago by dekkers

  • Cc jeroen@… added

comment:35 Changed 3 months ago by olivierdalang

  • Cc olivier.dalang@… added

comment:36 Changed 3 weeks ago by olivierdalang

  • Patch needs improvement unset

Hi,

I fixed the merge conflicts on the patch today : https://github.com/olivierdalang/django/commits/view_permission_master
See discussion on the PR :
https://github.com/django/django/pull/5297#issuecomment-223714731

It should apply to master now.

I think we need a core dev to review this, so I unset the "patch needs improvement" tag. Sorry if this is not the right process (I'm new to contributing to django).

Please let me know if there's anything else I can do to help getting those merged into core.

Bests

Last edited 3 weeks ago by olivierdalang (previous) (diff)
Note: See TracTickets for help on using tickets.
Back to Top