Feature proposal: override default manager for a model

[DrCuriosity]

I'd like to explore the possibility of setting a specific manager (or possibly queryset?) for a tracked model, to override the default value.

If we can find a good approach, I may be able to submit to a PR for this.

Why?

I've run into an issue where the default managers on some of our models can disable certain records, and make it an explicit action to look at disabled records, while they are masked out of the default queryset for the majority of operations.

When updates are made on those objects (e.g. when re-enabling them), the comparisons recorded in the LogEntry essentially treat it as a new record.

This seems to happen because of the use of sender._default_manager.filter(pk=instance.pk).first() in the receivers.

Working around the problem for now?

I could go with audit_objects = [Model]AuditManager and set the model's default manager to that instead of using the standard objects = [Model]Manager.

This would solve the current problem, but that may also raise problems down the line for other system components that are expecting our standard default manager behaviours.

Possible fixes: add a setting in registry configuration?

Adding a "manager" key in AUDITLOG_INCLUDE_TRACKING_MODELS that can nominate a specific manager to use for a model.

A stricter implementation of this could check in the model's ._meta.managers_map to ensure that the value was an appropriate manager object:

AUDITLOG_INCLUDE_TRACKING_MODELS = (
    {
        "model": "<appname>.<model>",
        "manager": "audit_objects",
        ...
    },
)

Alternately, a looser model could support a queryset callable:

AUDITLOG_INCLUDE_TRACKING_MODELS = (
    {
        "model": "<appname>.<model>",
        "queryset": "<appname>.<model>.get_full_queryset",
        ...
    },
)

In each case, the default behaviour would continue to use the existing sender._default_manager for filtering.

Questions

• Are there any problems with doing this that I might have missed?
• Is there any preference for stricter nomination of model managers vs. the looser queryset callable approach?

Thanks for your consideration :-)

[DrCuriosity]

Considering this further, an implementation where the configuration can only nominate managers registered against the model (from ._meta.managers_map) is likely to be a lot simpler, and avoids the risk of weird side-effects or security risks that the loose callable approach could introduce.

Then we can have something like this highly simplified example:

class MyModelManager(models.Manager):
    """Only gets enabled records by default."""
    def get_queryset(self):
        return super().get_queryset().filter(disabled=False)

class MyModelAuditManager(models.Manager):
    """Gets all records for full change auditing."""
    def get_queryset(self):
        return super().get_queryset()

class MyModel(models.Model):
    objects = MyModelManager()
    audit_objects = MyModelAuditManager()

    disabled = models.BooleanField(...)

And select in config whether we want to use "objects" or "audit_objects" or just use whichever is the default from sender._default_manager.

[2ykwang]

given the purpose of an audit log, I believe we should default to using _base_manager rather than the default manager. Auditing is about recording facts, not enforcing visibility/permission filters, and _base_manager better reflects that goal.

Note: Django uses the _base_manager when accessing related objects to bypass visibility filters.
(Docs: https://docs.djangoproject.com/en/5.1/topics/db/managers/#django.db.models.Model._base_manager)

(As for the history of why used objects in the past: I couldn’t find a definitive reason. My guess is that it was simply the most straightforward change at the time.)

Instead of per-model configuration, what do you think about making _base_manager the default? We could also offer an opt-out to choose between _default_manager and _base_manager for backward compatibility.

@DrCuriosity @hramezani

[DrCuriosity]

Instead of per-model configuration, what do you think about making _base_manager the default? We could also offer an opt-out to choose between _default_manager and _base_manager for backward compatibility.

Somehow I missed that option when I was first exploring the code. Yes, that does sound like a much more elegant solution - a _base_manager default aligns well with the purpose of auditing. That said, I wouldn't want to see it breaking expected behaviours on upgrade, so adding a setting like AUDITLOG_USE_BASE_MODEL_MANAGERS to opt into the change for those who wanted it seems like the better option.

If we could have that in the next release, I'd be very happy.

I don't know if it's easier for existing contributors to make the change, but I'm happy to try putting together a pull request. I am expecting that it would need:

• a new setting with the current behaviour as default in auditlog/conf.py
• documentation for the setting in docs/sources/usage.rst, explaining the difference between base and default managers
• changes in receivers.py and models.py to check the setting and switch to the _base_manager when set
• An extra test in ModelManagerTest (or a separate ModelBaseManagerTest?) confirming the change in behavour with an override_settings decorator

How does that sound?