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

Suggestions for admin.py #51

Open
ataylor32 opened this issue May 18, 2017 · 3 comments
Open

Suggestions for admin.py #51

ataylor32 opened this issue May 18, 2017 · 3 comments
Labels

Comments

@ataylor32
Copy link

I noticed that the APIRequestLogAdmin class allows users to add and edit records. If that's not needed, I would suggest making all of the fields read-only and adding this:

def has_add_permission(self, request):
    return False

If it is needed then could it be a setting so that developers can more easily disable the adding and editing of records? Probably two settings, actually (one for adding and one for editing).

As for deleting, I can see why that hasn't been disabled as people may want to delete records from the admin. This might be another thing to be made into a setting, though, since some developers may want to delete records on a schedule instead and disallow deleting from the admin.

@triat
Copy link
Contributor

triat commented May 19, 2017

Hello @ataylor32, actually all this things could be managed through the groups and not a params in the settings.

Or maybe I don't understand correctly what you want, sorry

@avelis
Copy link
Collaborator

avelis commented May 19, 2017

@ataylor32 Admin editability can depend on a per user basis. Dictating it one way probably isn't the best idea. Now are people using the admin to edit track records is a whole other question. For high volume customers this wouldn't make sense much. Be better to handle it via the ORM.

Per app permissions can be handled with Django auth groups/permissions I haven't used that and wouldn't be able to prescribe a solution right away. There is django guardian but that introduces another level of complexity.

If you really need the model edit view to have read only fields you can import the admin class into a file that will be picked up during app load and set as follows:

from rest_framework_tracking.admin import APIRequestLogAdmin

APIRequestLogAdmin.readonly_fields = ('body', )

@ataylor32 All these solutions above can be achieved without new changes to the library. Always open to changes if you have a PR that represents what you are looking for in spirit.

@ataylor32
Copy link
Author

Thanks for the replies! I understand where you're coming from. If you want to leave things the way they are then I respect that. I'd like to explain my thinking, though.

I don't know why anybody—superuser or not—would need to add or edit a drf-tracking record. The purpose of drf-tracking, as I understand it, is to log real requests made to the site's API. If somebody adds or edits a record from the admin then drf-tracking will have inaccurate information since the record won't be a representation of a real request.

I think it makes sense to have the most likely configuration be the default configuration and if somebody wants to override it then they can. So, in my opinion, drf-tracking's default configuration should be that records cannot be added or edited from the admin. If somebody really wants to be able to add or edit drf-tracking records from the admin (and, again, I don't know why they would) then they can enable that themselves. That's just my opinion, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants