Fix translation for invalid schedule cron + cleanup translations for import modal #1618
Conversation
📝 WalkthroughWalkthroughTranslations for schedule import UI were moved from admin/schedule.* to server/schedule.import_action.*, with some labels hidden. A new 'invalid' translation and import_action group were added. ScheduleResource now wraps cron next-run computation in try/catch to avoid exceptions during rendering. Changes
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🧹 Nitpick comments (6)
lang/en/server/schedule.php (2)
33-34: Good addition: explicit fallback label for invalid cron.
This key enables a safe UI fallback when parsing fails. Consider making the key more specific (e.g., invalid_cron) to avoid generic reuse in unrelated contexts, but this is a nitpick.
109-117: Translations moved and grouped under server/schedule.import_action look consistent. Minor wording/punctuation polish suggested.
The texts read fine; a couple of tiny improvements for clarity and consistency (no spaces inside parentheses, consistent title casing).Apply this diff:
'import_action' => [ 'file' => 'File', 'url' => 'URL', - 'schedule_help' => 'This should be the raw .json file ( schedule-daily-restart.json )', + 'schedule_help' => 'This should be the raw .json file (schedule-daily-restart.json)', 'url_help' => 'URLs must point directly to the raw .json file', - 'add_url' => 'New URL', + 'add_url' => 'Add URL', 'import_failed' => 'Import Failed', - 'import_success' => 'Import Success', + 'import_success' => 'Import Successful', ],app/Filament/Server/Resources/ScheduleResource.php (1)
118-126: Escape placeholders and format the date for better UX and safety.
Small hardening: HTML is rendered via HtmlString and placeholders include user timezone; escaping reduces any residual risk. Also propose a more human-readable timestamp.Apply this diff:
- ->description(function (Get $get) { - try { - $nextRun = Utilities::getScheduleNextRunDate($get('cron_minute'), $get('cron_hour'), $get('cron_day_of_month'), $get('cron_month'), $get('cron_day_of_week'))->timezone(auth()->user()->timezone); - } catch (Exception) { - $nextRun = trans('server/schedule.invalid'); - } - - return new HtmlString(trans('server/schedule.cron_body') . '<br>' . trans('server/schedule.cron_timezone', ['timezone' => auth()->user()->timezone, 'next_run' => $nextRun])); - }) + ->description(function (Get $get) { + try { + $nextRun = Utilities::getScheduleNextRunDate( + $get('cron_minute'), + $get('cron_hour'), + $get('cron_day_of_month'), + $get('cron_month'), + $get('cron_day_of_week') + )->timezone(auth()->user()->timezone); + } catch (Exception) { + $nextRun = trans('server/schedule.invalid'); + } + + $timezone = e(auth()->user()->timezone); + $nextRunText = $nextRun instanceof Carbon + ? e($nextRun->toDayDateTimeString()) + : e($nextRun); + + return new HtmlString( + trans('server/schedule.cron_body') . + '<br>' . + trans('server/schedule.cron_timezone', [ + 'timezone' => $timezone, + 'next_run' => $nextRunText, + ]) + ); + })app/Filament/Components/Actions/ImportScheduleAction.php (3)
46-48: Consider keeping a visible label for accessibility on FileUpload.
hiddenLabel() streamlines the UI, but labels aid screen readers and cognitive load. If you prefer a minimal UI, at least keep an accessible name via the label.Apply this diff:
- FileUpload::make('files') - ->hiddenLabel() + FileUpload::make('files') + ->label(trans('server/schedule.import_action.file')) ->hint(trans('server/schedule.import_action.schedule_help'))
58-61: Same for Repeater: consider a visible (or at least accessible) label.
Hidden labels can reduce usability. If keeping it hidden, ensure a clear context title elsewhere; alternatively show the label.Apply this diff:
- Repeater::make('urls') - ->hiddenLabel() + Repeater::make('urls') + ->label(trans('server/schedule.import_action.url')) ->itemLabel(fn (array $state) => str($state['url'])->afterLast('/schedule-')->before('.json')->headline()) ->hint(trans('server/schedule.import_action.url_help'))
67-73: URL validation: endsWith('.json') may reject valid raw URLs with query strings.
Git providers or CDNs often serve raw JSON via URLs like .../file.json?raw=1. endsWith('.json') would fail those. Consider a regex that permits optional query strings.Apply this diff:
TextInput::make('url') ->live() ->label(trans('server/schedule.import_action.url')) ->url() - ->endsWith('.json') + ->rule('regex:/\.json(\?.*)?$/i') ->validationAttribute(trans('server/schedule.import_action.url')),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
app/Filament/Components/Actions/ImportScheduleAction.php(2 hunks)app/Filament/Server/Resources/ScheduleResource.php(1 hunks)lang/en/admin/schedule.php(0 hunks)lang/en/server/schedule.php(2 hunks)
💤 Files with no reviewable changes (1)
- lang/en/admin/schedule.php
🧰 Additional context used
🧬 Code Graph Analysis (1)
app/Filament/Server/Resources/ScheduleResource.php (1)
app/Helpers/Utilities.php (2)
Utilities(9-56)getScheduleNextRunDate(39-44)
🔇 Additional comments (5)
lang/en/server/schedule.php (1)
109-117: No non-English locale files for server/schedule.php found
I checked under lang/* and the only server/schedule.php file is the English one you’ve updated. There are no other locale copies, so there are no missinginvalidorimport_actionentries in existing translations.If you add this file for other languages later, be sure to include those new keys to avoid fallback-to-key.
app/Filament/Server/Resources/ScheduleResource.php (1)
118-126: Great fix: guard cron parsing to prevent 500s in the form.
Catching Exception and returning a localized fallback resolves the original crash scenario in the cron description. This is exactly what was needed for robustness during live editing.app/Filament/Components/Actions/ImportScheduleAction.php (3)
42-55: Migration to server/schedule.import_action translations looks correct.
Tabs and related strings now consistently reference the server namespace; aligns with the file move.
105-116: Notifications now use the new translation keys. LGTM.
Titles map to server/schedule.import_action.* and align with the moved strings.
42-75: No remaining admin/schedule translation references found
Ranrg -nP "trans\s*\(\s*['\"]admin/schedule" -g '!vendor/**'and confirmed zero matches—all translation keys have been updated.
Closes #1617