#21113 closed New feature (fixed)
django_admin_log table stores messages in different languages depending on which language was active while editing.
Reported by: | Owned by: | ||
---|---|---|---|
Component: | contrib.admin | Version: | dev |
Severity: | Normal | Keywords: | admin logs i18n |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
In my project (in development) there are 3 different languages supported. Admin site supposed to have 3 main superusers, each is a native speaker of one of the languages. If one superuser edited an object, another one is not able to read what his fellow superuser previously did, because "Change history" page doesn't respect the current user language and displays the messages in the language that was current when the changes took place.
Maybe change_message field should be removed and the messages generated each time they are displayed.
So is object_repr, because object's representation may be dependant on the current language too.
Thank you.
Change History (24)
comment:1 by , 11 years ago
Triage Stage: | Unreviewed → Accepted |
---|---|
Version: | 1.6-beta-1 → master |
comment:2 by , 11 years ago
Type: | Uncategorized → New feature |
---|
comment:3 by , 10 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:4 by , 10 years ago
So at this point I understand how to change this so all subsequent log entries behave correctly. But currently the translated strings are being saved away as part of the object, and I think (?) this means we've lost the information of the starting language, so the only way I can think of fixing *existing* histories is by writing a script and having the user run it, specifying their relevant languages?
comment:5 by , 10 years ago
Fixing existing history might be impossible to achieve. I wouldn't set that as a goal for this ticket.
follow-up: 7 comment:6 by , 10 years ago
I can't get ugettext to translate "Changed" for some reason, even though there are bits of code where it does it successfully and I'm not sure what I'm doing wrong.
comment:7 by , 10 years ago
Replying to anonymous:
I can't get ugettext to translate "Changed" for some reason, even though there are bits of code where it does it successfully and I'm not sure what I'm doing wrong.
Whoops, that was me. Didn't realize I was logged out.
follow-up: 10 comment:9 by , 10 years ago
Has patch: | set |
---|---|
Needs tests: | set |
Owner: | removed |
Patch needs improvement: | set |
Status: | assigned → new |
https://github.com/django/django/pull/3345
Here's a partial fix for this bug. It changes the log messages so that they're saved in the database in English and then parsed and translated when they need to be displayed. However, due to the way the change messages were written upon being entered into the database, I wasn't able to properly distinguish between different types of save messages. For example, if an object (for example, a Poll or Choice) contained a period or quotation marks in the body, I could not tell the difference between the end of the message and the punctuation within the message body. So I'm only submitting a partial fix. From what I could test, it works pretty well for Users. I was also having some trouble getting certain words to translate.
If having history messages displayed in different languages is a priority, I would recommend restructuring the logging functionality entirely, and have the messages be saved in an unambiguously delimited format (yaml? json? something!) so that they can be formed into translatable sentences after they are extracted from the database. I would be willing to work on this if an issue gets made!
comment:10 by , 10 years ago
Replying to jooyous:
(...)
If having history messages displayed in different languages is a priority, I would recommend restructuring the logging functionality entirely, and have the messages be saved in an unambiguously delimited format (yaml? json? something!) so that they can be formed into translatable sentences after they are extracted from the database. I would be willing to work on this if an issue gets made!
Yes, I think that this would be the right approach.
comment:12 by , 9 years ago
Patch needs improvement: | set |
---|
Left comments for improvement on the pull request.
comment:14 by , 9 years ago
Patch needs improvement: | unset |
---|
comment:15 by , 9 years ago
I know this is very late to the party, and it's not that critical of an issue, but generally it is considered good form [citation-needed] to ensure the top level of a JSON structure is an object/dictionary, rather than a list, string, int, etc. django.http.response.JsonResponse
for example considers anything except a dict to be 'unsafe'.
The change is simple... just use {'entries':[ ...]}
, or {'additions':[...], 'deletions':[...], ...}
rather than simply [...]
, but this does involve changing the entire patch. 😦
comment:16 by , 9 years ago
Changing the entire patch is not an issue at all.
I would appreciate though if you could find some reference about this design issue.
comment:17 by , 9 years ago
From a bit of searching, I guess the "good form" is to mitigate JSON hijacking? Those concerns probably aren't relevant here.
comment:18 by , 9 years ago
Patch needs improvement: | set |
---|
Left a few more comments on the pull request.
comment:19 by , 9 years ago
Patch needs improvement: | unset |
---|
comment:20 by , 9 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
comment:22 by , 9 years ago
Possible flaky test seen on Jenkins:
admin_utils.test_logentry.LogEntryTests.test_logentry_change_message Stacktrace Traceback (most recent call last): File "/mnt/jenkinsdata/workspace/django-master/database/mysql/python/python3.5/tests/admin_utils/test_logentry.py", line 65, in test_logentry_change_message self.assertEqual(logentry.get_change_message(), 'Changed title and hist.') AssertionError: 'Changed something' != 'Changed title and hist.' - Changed something + Changed title and hist.
comment:23 by , 9 years ago
Probably an issue with .latest()
, on MySQL, not by chance! We may find another way to get the target logentry based on another criterion...
Accepted. I'd like messages to be language-agnostic and translatable.