Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
There are at least two strings that I think should use the t() function to make them translatable:
Line 648:
$n->log = "Node published by scheduler module. Original creation date was ". format_date($old_creation_date, 'custom', $date_format) .".";
Line 706:
$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.
Comments
Comment #1
jonathan1055 CreditAttribution: jonathan1055 commentedHi 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
Comment #2
jonathan1055 CreditAttribution: jonathan1055 commentedRe-roll after dev 1.1+10
Comment #3
jonathan1055 CreditAttribution: jonathan1055 commentedNew 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
Comment #4
jonathan1055 CreditAttribution: jonathan1055 commentedHi 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
Comment #5
jonathan1055 CreditAttribution: jonathan1055 commentedChanged 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]
Comment #7
jonathan1055 CreditAttribution: jonathan1055 commentedHere 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
Comment #8
pfrenssenThe 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.
Same as above.
Comment #9
jonathan1055 CreditAttribution: jonathan1055 commentedYes 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.
Comment #10
pfrenssenThis 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!
Comment #11
jonathan1055 CreditAttribution: jonathan1055 commentedThe 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.
Comment #12
pfrenssenThanks!
Merged into 7.x-1.x. Relevant commits: 8477498 and 96a0f77.