Multiple instances of DjangoAdmin share the same model LogEntry

[Jacob Rief]

If someone uses two or more instances of the Django Admin interface,

the sidebar on the right containing the "Recent actions", shares the same

information for all instances of the Django Admin interface.

This is because the model LogEntry is a singleton and shared across all

instances of the Django Admin interface.

As a consequence this may be confusing to some users, since they might

expect that the log entries are bound to the current user interface.

I had a look at the code and although it is possible to create separate entries

for those logs, this is really complicated and requires a lot of work. Therefore

I wanted to ask, if it would make sense to allow each instance of a Django

Admin to specify its own model used to log entries? The default would of

course be to fall back on the current behaviour.

I have some ideas on how to achieve this. An instantiation

of a Django Admin could specify its own model similar to LogEntry

which itself should inherit from a class named AbstractLogEntry (which

doesn't yet exist). This would also allow integrators to extend this class with

additional columns for extra log entries.

– Jacob

[Carlton Gibson]

Hi Jacob. 

Can I ask you to explain your use-case a little more please? 

As I understand it, separate admin sites may register different models, but I'm only going to see the History for instances that are visible to the current admin no? 

Maybe you mean the Recent actions > My actions bit of the index page? (I didn't check but, if not already, could that list be filtered by the content type of registered models for the current admin? 🤔)

C.

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/38c055d3-58ef-4a73-9784-be6877e31b2cn%40googlegroups.com.

[Jacob Rief]

Hi Carlton,

As I understand it, separate admin sites may register different models, but I'm only going to see the History for instances that are visible to the current admin no? 

The Django admin interface uses a templatetag named get_admin_log. It renders the content of all entries of model LogEntry filtered by the current user.

Check here for details: https://github.com/django/django/blob/34b328814976a2e2f7907361a494202763649f3f/django/contrib/admin/templatetags/log.py#L14

If you instantiate more than one Django-Admin interface, the logged in user therefore sees all his actions, regardless of the chosen admin instance.

My proposal would be to add another column to LogLevel containing the name of the admin interface. That column then shall be used as an additional filter. 

It should also be possible to swap that model against a compatible implementation, for instance if someone separates the admin interface because he also

separated the underlying databases.

 

Maybe you mean the Recent actions > My actions bit of the index page? (I didn't check but, if not already, could that list be filtered by the content type of registered models for the current admin? 🤔)

Yes, that's what I wrote and intended.

– Jacob

[Carlton Gibson]

Thanks for clarifying Jacob, great. 

So, the registered content_types are in the available_apps key to the context passed to that tag. (Currently that needs reconstructing in a filter/comprehension, but it could feasibly be added to the `AdminSite.each_context` method.) Adding the matching `__in` lookup to the filter call there should allow retricting to only registered models no?

I'm personally sceptical that a "I added the same model to multiple AdminSites and need to see only actions for each model from the AdminSite that it occurred on" use-case is one we should cover. That's pretty niche I'd say. 

Kind Regards,

Carlton

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/bfcd58e8-bbf1-42bc-a7dc-25a788e47b1en%40googlegroups.com.

[Jacob Rief]

Hi Carlton,
that proposal makes a lot of sense.

This means that one could for instance add a custom function to that context which, if set, would be called

by the AdminLogNode.render method and then overrides the context[self.varname].

How would you name that extra context variable, maybe "render_log_entries"?

Another approach would be to move the functionality of method render into the class AdminSite. This would

allow Django implementers to override the way, log entries are rendered.

This would be a minimalistic change to the existing code base but would allow to change the rendering

of log entries in a much more flexible way.

Shall I create a ticket and a proof of concept?

– Jacob

[Carlton Gibson]

Hey Jacob. 

I had in mind literally **just** adding the content-types so that the line in that render call would be: 

    # this could be via a proposed context["site_content_types"] or via a map from
    # "available_apps"
    site_content_types: [ContentType] = ...
    entries = LogEntry.objects.filter(
        user__pk=user_id,
        content_type__in=site_content_types
    )

This I can sort of see: OK, folks use multiple AdminSites, and having non-registered models show up is a cause of uncertainty, plausibly, yes. 

You want more than that thought right? (I'm still not certain I've grasped exactly what use-case you have in mind.)

Passing in a callable seems an extra step of complexity up. 

(IDK, maybe it's not, but I'm not seeing it, although I'm known to be slow at times.) 

It would be undocumented magic. 

I think I'd lean towards suggesting a custom template and tag in that case. 

(But maybe that's just me.) 

A diff of what you actually have in mind would likely help (to at least be clear). 

C.

--

You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/e50bce39-d1dd-44b5-9fae-fc1bc8c9c864n%40googlegroups.com.

[Adam Johnson]

I'm also skeptical we need to support this use case. Certainly some users prefer shared log messages, so all actions are shown regardless of which interface was used.

Just to comment on the DB side: storing the same few names over and over in such a column would be a waste of disk space. It would be better to have a FK to an "admin site" model, though we'd also need to consider query performance. The LogEntry table can get quite big in busy orgs.

There is also the question of what to use as the name (full import path?), and how to retain it even as the admin site object is renamed or moved.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAJwKpyQqW6wLUCHFQiFvReF-67_Ker8XoFHhFDvg4tnkwFLd_w%40mail.gmail.com.

[Jacob Rief]

Hi Adam,

I'm currently preparing a pull request on this. No changes will be made to the model LogEntry. It will just allow implementers to filter

log entries by registered models (of that instantiated admin). Not doing this, in my opinion is unexpected behaviour anyway.

Thanks to Carlton for pointing this out.

We therefore should change the subject of this post to "Optionally filter LogEntry by registered model".

– Jacob