-
Notifications
You must be signed in to change notification settings - Fork 438
Fix AttributeError when AUDITLOG_LOGENTRY_MODEL is not set (#788) #789
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #789 +/- ##
=======================================
Coverage 95.92% 95.92%
=======================================
Files 35 35
Lines 1251 1252 +1
=======================================
+ Hits 1200 1201 +1
Misses 51 51 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
A second approach was to simply change from django.conf import settingsto from auditlog.conf import settingsinside |
2ykwang
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've confirmed this fix resolves the issue!
One minor concern with the first approach: the default value would be managed in two places, which increases maintenance overhead.
In my opinion, the second option might be better. Since settings is conventionally associated with django.conf.settings, using an alias could help avoid confusion:
from auditlog.conf import settings as auditlog_settingsWhat do you think?
|
Thanks for taking a look! I agree about the duplication. How confident are you in the alternative approach? I'm not a regular package contributor and I would hate for that change to cause someone an issue downstream. I just don't know the implications of that change, so was hoping for someone with Django package experience to ok it. |
|
@hramezani do you have any other ideas on this? |
I hope I'm not stepping on toes with this, but I was following the issue thread and thought I'd give it a go. This is Claude Opus 4.5 assisted. I have verified that this fix works with my Django project. The Claude summary is below.
Summary
AttributeError: 'Settings' object has no attribute 'AUDITLOG_LOGENTRY_MODEL'that occurs when upgrading to 3.4.0+ without explicitly settingAUDITLOG_LOGENTRY_MODELProblem
When Django's admin autodiscover imports
auditlog/admin.py→mixins.py, theget_logentry_model()function is called at module load time beforeconf.pyhas a chance to apply the default value.Solution
Use
getattr()with the default value directly inget_logentry_model():This ensures the default is available regardless of import order, while still respecting user-defined custom values.
Changes
auditlog/__init__.py: Addgetattr()fallback inget_logentry_model()auditlog_tests/tests.py: Add regression testtest_logentry_model_default_when_setting_missingFixes #788