There are at least two strings that I think should use the t() function to make them translatable:

Line 648:

<?php
$n
->log = "Node published by scheduler module. Original creation date was "format_date($old_creation_date, 'custom', $date_format) .".";
?>

Line 706:

<?php
$n
->log = "Node unpublished by scheduler module. Original change date was "format_date($old_change_date, 'custom', $date_format) .".";
?>

Those strings are displayed as the revision comment to users that have the permission. It would be nice to be able to translate them.

I don't know if watchdog messages should be translatable, too.

Files: 
CommentFileSizeAuthor
#10 1898784_10.scheduler.translatable_revision_text-test_only.patch4.79 KBpfrenssen
FAILED: [[SimpleTest]]: [MySQL] 94 pass(es), 2 fail(s), and 0 exception(s).
[ View ]
#10 1898784_10.scheduler.translatable_revision_text.patch7.06 KBpfrenssen
PASSED: [[SimpleTest]]: [MySQL] 96 pass(es).
[ View ]
#9 1898784_9.scheduler.translatable_revision_text.patch2.27 KBjonathan1055
PASSED: [[SimpleTest]]: [MySQL] 52 pass(es).
[ View ]
#7 1898784_7.scheduler.translatable_revision_text.patch1.53 KBjonathan1055
PASSED: [[SimpleTest]]: [MySQL] 52 pass(es).
[ View ]
#3 revision message 1.jpg139.01 KBjonathan1055
#3 revision message 2.jpg147.81 KBjonathan1055
#3 1898784_3.scheduler.translatable_revision_text.d7.patch1.52 KBjonathan1055
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1898784_3.scheduler.translatable_revision_text.d7.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#2 1898784_2.scheduler.translatable_revision_text.d7.patch1.37 KBjonathan1055
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1898784_2.scheduler.translatable_revision_text.d7.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#1 1898784_1.scheduler.translatable_revision_text.d7.patch1.37 KBjonathan1055
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1898784_1.scheduler.translatable_revision_text.d7.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Comments

Title:Make all strings translatableMake log revision text translatable
Version:6.x-1.x-dev» 7.x-1.1
Status:Active» Needs review
StatusFileSize
new1.37 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1898784_1.scheduler.translatable_revision_text.d7.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Hi Yan,
Yes these strings do need to be translated, and the 7.x version needs to be fixed first. Attached is a patch to test. Watchdog calls must not have t() within them, so they are ok.

For our own future reference, the functions changed in this patch and are also changed by the patch in #773510: Integration with Rules module so whichever of these is done first, the other one will need a re-roll.

Jonathan

StatusFileSize
new1.37 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1898784_2.scheduler.translatable_revision_text.d7.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Re-roll after dev 1.1+10

StatusFileSize
new1.52 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1898784_3.scheduler.translatable_revision_text.d7.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
new147.81 KB
new139.01 KB

New patch. Added the current datetime into the log revision message. This is useful because if the node is subsequently edited without a revision being made, then the new date is shown and attached to the Scheduler revision text. This gives confusing information, so adding the date into the text means we have a fixed record of when Scheduler did the processing.

After Scheduler has published via cron

After a subsequent edit, without a revision being made - we can now still see the Scheduler time

Jonathan

Hi Yan,

The patch in #3 still applies ok (with offsets) to the latest dev 7.x-1.1+15 dated 25th Sept.
Would be good to get this tested and then committed.

Jonathan

Version:7.x-1.1» 7.x-1.x-dev

Changed version to 1.x-dev to see if the automated testing status will not be 'ignored'

[edit: yes it now says 'queued for testing', but it will fail as not in a/ b/ format. Needs a re-roll]

Status:Needs review» Needs work

The last submitted patch, 1898784_3.scheduler.translatable_revision_text.d7.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new1.53 KB
PASSED: [[SimpleTest]]: [MySQL] 52 pass(es).
[ View ]

Here is the same code change, in the required diff --git format.

This patch does not affect any lines in the .module coding standards patch in #1566024: Coding Standards and Coder Review #27 so they should both be able to be tested and committed without conflict.

Jonathan

Status:Needs review» Needs work
  1. +++ b/scheduler.module
    @@ -784,7 +784,10 @@ function _scheduler_publish() {
    -      $n->log = "Node published by scheduler module. Original creation date was " . format_date($old_creation_date, 'custom', $date_format) . ".";
    +      $n->log = t('Node published by Scheduler on @now. Previous creation date was @date.', array(
    +        '@now' => format_date(REQUEST_TIME, 'short'),
    +        '@date' => format_date($old_creation_date, 'short'),
    +      ));

    The date format is changed from the user configured custom date format to 'short'. Is this intentional? Can you explain your motivation for this?

    If you have a good reason to do this, then the $date_format variable is unused in the function scope and its definition should be removed as well.

  2. +++ b/scheduler.module
    @@ -858,7 +861,10 @@ function _scheduler_unpublish() {
    -        $n->log = "Node unpublished by scheduler module. Original change date was " . format_date($old_change_date, 'custom', $date_format) . ".";
    +        $n->log = t('Node unpublished by Scheduler on @now. Previous change date was @date.', array(
    +          '@now' => format_date(REQUEST_TIME, 'short'),
    +          '@date' => format_date($old_change_date, 'short'),
    +        ));
           }

    Same as above.

Status:Needs work» Needs review
StatusFileSize
new2.27 KB
PASSED: [[SimpleTest]]: [MySQL] 52 pass(es).
[ View ]

Yes I changed the format of the date in the revision message because the admin-configured format is defined to specify how the user enters the date. It is not always applicable to be used as a display format (although we do that sometimes in other places and those should be looked at too). In particular I noticed this when testing in conjunction with the new 'default time' patch, where the admin can remove the time part completely from the scheduler input format. If this is used for display it will not show the time, which is not very useful in the revision log where the time may be very important. The core date formats all have time included, so this is a better choice here. We could use 'medium' but I thought the 'short' looked ok in the message. See the screen shots in #3 above.

Re-rolled with the two $date_format lines removed, and comments added about using a core date format.

StatusFileSize
new7.06 KB
PASSED: [[SimpleTest]]: [MySQL] 96 pass(es).
[ View ]
new4.79 KB
FAILED: [[SimpleTest]]: [MySQL] 94 pass(es), 2 fail(s), and 0 exception(s).
[ View ]

This is looking good! The patch is RTBC for me. The revisioning was not yet tested, so I have written a test for it.

I have already committed the patch and test in a feature branch. Review of the test is welcome!

Status:Needs review» Reviewed & tested by the community

The tests look good. You have been busy. Patched and run ok. I would not normally RTBC my own patch, but given that you have RTBC'd the original patch, I confirm the tests are ready too, hence changing the issue status.

I can see that we'll need to re-organise the .test file soon because we (you) are adding to it regularly now. Good work, thanks.

Status:Reviewed & tested by the community» Fixed

Thanks!

Merged into 7.x-1.x. Relevant commits: 8477498 and 96a0f77.

Status:Fixed» Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.