Make log revision text translatable

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

Re-roll after dev 1.1+10

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

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]

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

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.

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.

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!

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.

Thanks!

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