--
Ticket URL: <https://code.djangoproject.com/ticket/21113>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
* needs_better_patch: => 0
* stage: Unreviewed => Accepted
* version: 1.6-beta-1 => master
* needs_tests: => 0
* needs_docs: => 0
Comment:
Accepted. I'd like messages to be language-agnostic and translatable.
--
Ticket URL: <https://code.djangoproject.com/ticket/21113#comment:1>
* type: Uncategorized => New feature
--
Ticket URL: <https://code.djangoproject.com/ticket/21113#comment:2>
* owner: nobody => jooyous
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/21113#comment:3>
Comment (by jooyous):
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?
--
Ticket URL: <https://code.djangoproject.com/ticket/21113#comment:4>
Comment (by claudep):
Fixing existing history might be impossible to achieve. I wouldn't set
that as a goal for this ticket.
--
Ticket URL: <https://code.djangoproject.com/ticket/21113#comment:5>
Comment (by 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/21113#comment:6>
Comment (by jooyous):
Replying to [comment:6 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/21113#comment:7>
Comment (by jooyous):
Hellooo I am still working on this.
--
Ticket URL: <https://code.djangoproject.com/ticket/21113#comment:8>
* status: assigned => new
* needs_better_patch: 0 => 1
* has_patch: 0 => 1
* needs_tests: 0 => 1
* owner: jooyous =>
Comment:
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!
--
Ticket URL: <https://code.djangoproject.com/ticket/21113#comment:9>
Comment (by claudep):
Replying to [comment:9 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/21113#comment:10>
* needs_better_patch: 1 => 0
* needs_tests: 1 => 0
Comment:
[https://github.com/django/django/pull/5879 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/21113#comment:11>
* needs_better_patch: 0 => 1
Comment:
Left comments for improvement on the pull request.
--
Ticket URL: <https://code.djangoproject.com/ticket/21113#comment:12>
Comment (by Claude Paroz <claude@…>):
In [changeset:"35c41987ecfaad849019d09468ce322fec86cd39" 35c4198]:
{{{
#!CommitTicketReference repository=""
revision="35c41987ecfaad849019d09468ce322fec86cd39"
Moved LogEntry-related tests to their own test case
Thanks Tim Graham for reviewing and contributing to the patch.
Refs #21113.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21113#comment:13>
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/21113#comment:14>
Comment (by chmarr):
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 `{'changes':[ ...]}` rather than simply
`[...]`, but this does involve changing the entire patch. 😦
--
Ticket URL: <https://code.djangoproject.com/ticket/21113#comment:15>
Comment (by claudep):
Changing the entire patch is not an issue at all.
I would appreciate though if you could find some reference about this
design issue.
--
Ticket URL: <https://code.djangoproject.com/ticket/21113#comment:16>
Comment (by timgraham):
From a bit of searching, I guess the "good form" is to mitigate
[http://haacked.com/archive/2009/06/25/json-hijacking.aspx/ JSON
hijacking]? Those concerns probably aren't relevant here.
--
Ticket URL: <https://code.djangoproject.com/ticket/21113#comment:17>
* needs_better_patch: 0 => 1
Comment:
Left a few more comments on the pull request.
--
Ticket URL: <https://code.djangoproject.com/ticket/21113#comment:18>
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/21113#comment:19>
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/21113#comment:20>
* status: new => closed
* owner: => Claude Paroz <claude@…>
* resolution: => fixed
Comment:
In [changeset:"cf7894be88f6c27680ef80796b883f6e3b709b8b" cf7894be]:
{{{
#!CommitTicketReference repository=""
revision="cf7894be88f6c27680ef80796b883f6e3b709b8b"
Fixed #21113 -- Made LogEntry.change_message language independent
Thanks Tim Graham for the review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21113#comment:21>
Comment (by timgraham):
Possible flaky test seen [http://djangoci.com/job/django-master/1754/ 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.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21113#comment:22>
Comment (by claudep):
Probably an issue with `.latest()`, on MySQL, not by chance! We may find
another way to get the target logentry based on another criterion...
--
Ticket URL: <https://code.djangoproject.com/ticket/21113#comment:23>
Comment (by Claude Paroz <claude@…>):
In [changeset:"4a03e6f27207ec7d9bb12d229da570633c086a92" 4a03e6f2]:
{{{
#!CommitTicketReference repository=""
revision="4a03e6f27207ec7d9bb12d229da570633c086a92"
Refs #21113 -- Updated test to allow for bad MySQL time resolution
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21113#comment:24>