Added localization for translated cron-string#168
Conversation
📝 WalkthroughWalkthroughAdds locale and 24-hour time-format options to cron translation: the interface and BaseScheduledCommand method signatures now accept Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #168 +/- ##
=========================================
Coverage 84.66% 84.66%
Complexity 329 329
=========================================
Files 33 33
Lines 1337 1337
=========================================
Hits 1132 1132
Misses 205 205 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Entity/BaseScheduledCommand.php (1)
329-336:⚠️ Potential issue | 🟡 MinorUpdate
ScheduledCommandInterfaceto match the implementation signature.The interface at
Entity/ScheduledCommandInterface.php:89declaresgetCronExpressionTranslated(): stringwithout the$localeand$timeFormat24hoursparameters that the implementation inBaseScheduledCommand.php:329now includes. While PHP allows optional parameters in implementations beyond the interface, callers typed against the interface cannot pass locale or time-format arguments. Update the interface to maintain consistency:public function getCronExpressionTranslated(string $locale = 'en', bool $timeFormat24hours = false): string;
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@Entity/ScheduledCommandInterface.php`:
- Line 89: Update the composer constraint for lorisleiva/cron-translator so
Composer can install it: replace the non-existent "^0.5.0" constraint with a
valid released version (e.g. "^0.4.6") in composer.json, then run composer
update; the interface method getCronExpressionTranslated(string $locale = 'en',
bool $timeFormat24hours = false) and BaseScheduledCommand already match the
library API so no other code changes are needed.
| public function getNextRunDate(bool $checkExecuteImmediately = true): ?DateTime; | ||
|
|
||
| public function getCronExpressionTranslated(): string; | ||
| public function getCronExpressionTranslated(string $locale = 'en', bool $timeFormat24hours = false): string; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all classes implementing ScheduledCommandInterface
echo "=== Implementations of ScheduledCommandInterface ==="
rg -n 'implements\s+ScheduledCommandInterface' --type=php
echo ""
echo "=== Classes extending BaseScheduledCommand ==="
rg -n 'extends\s+BaseScheduledCommand' --type=phpRepository: Dukecity/CommandSchedulerBundle
Length of output: 475
🌐 Web query:
lorisleiva/cron-translator version 0.5.0 translate method signature API
💡 Result:
I couldn’t find any lorisleiva/cron-translator v0.5.0 release/tag on Packagist or in the GitHub releases list (latest shown there is v0.4.6, released June 25, 2025). [1]
For the currently documented API, CronTranslator::translate is a static method that takes:
CronTranslator::translate(string $expression, string $locale = 'en', bool $use24HourTime = false): string- 1st arg: cron expression
- 2nd arg (optional): locale (e.g.
'fr') - 3rd arg (optional): use 24-hour time formatting (
true/false) [2]
Sources: [1] [2]
🏁 Script executed:
#!/bin/bash
# Check the actual CronTranslator call in BaseScheduledCommand
echo "=== BaseScheduledCommand getCronExpressionTranslated implementation ==="
sed -n '320,340p' Entity/BaseScheduledCommand.php
echo ""
echo "=== Composer dependency for cron-translator ==="
rg -A2 -B2 'cron-translator' composer.jsonRepository: Dukecity/CommandSchedulerBundle
Length of output: 994
Dependency version constraint ^0.5.0 for lorisleiva/cron-translator does not exist and will cause composer to fail.
The interface signature change itself is correct and consistent with BaseScheduledCommand, which properly forwards the new optional parameters to CronTranslator::translate(). The parameter order (expression, locale, use24HourTime) matches the library's API.
However, composer.json specifies "lorisleiva/cron-translator": "^0.5.0", but v0.5.0 has never been released. The latest available version is v0.4.6 (June 25, 2025). The constraint ^0.5.0 must be corrected to ^0.4.6 or higher if a newer version becomes available.
No other implementations of ScheduledCommandInterface exist in the codebase that require updating — only BaseScheduledCommand implements it, and its signature already matches.
🤖 Prompt for AI Agents
In `@Entity/ScheduledCommandInterface.php` at line 89, Update the composer
constraint for lorisleiva/cron-translator so Composer can install it: replace
the non-existent "^0.5.0" constraint with a valid released version (e.g.
"^0.4.6") in composer.json, then run composer update; the interface method
getCronExpressionTranslated(string $locale = 'en', bool $timeFormat24hours =
false) and BaseScheduledCommand already match the library API so no other code
changes are needed.
|
Thanks. |
|
I did update lorisleiva/cron-translator in another PR before i did see your PR. |
|
I tested the changes in a real application and it worked as expected. I'm overwriting the templates in my application, so the only change I needed to make was to provide the parameters, but as the changed translate()-function uses defaults, this should work for anyone, even if they overwrite the templates. |
|
Adding paramters to a method in an Interface, looks like a BC Break to me. What version of symfony and PHP do you use? |
Even if defaults are provided? 🤔
PHP 8.5.1, Symfony 8.0.5
That's probably a good idea. Do you want me to remove the template-change from my PR so users can decide whether they want the text translated? |
|
I guess now changes are needed at this PR for now. |
2 participants
I added the default values for locale (en) and 24h time format (false) to the Entity and the locale from the request in the template. If you overwrite your template, you can also set the time format.
I also bumped to the latest version of lorisleiva/cron-translator
(CodeRabbit now complains that the composer-constraint is not valid, but version 0.5 is available ... ?
https://packagist.org/packages/lorisleiva/cron-translator#v0.5.0)
Summary by CodeRabbit
New Features
Chores