Refactor SafemodeCommand and add language support#47
Open
Conversation
Refactor SafemodeCommand class to improve structure and add language loading.
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRefactors the SafemodeCommand console plugin to use a new implementation and adds language file loading for the safemode console command while keeping the old implementation commented out. Sequence diagram for SafemodeCommand initialization with language loadingsequenceDiagram
actor AdminUser
participant ConsoleApplication
participant SafemodeCommand
participant Factory
participant Application
participant Language
AdminUser->>ConsoleApplication: execute safe:mode
ConsoleApplication->>SafemodeCommand: instantiate SafemodeCommand
activate SafemodeCommand
SafemodeCommand->>Factory: getApplication()
Factory-->>SafemodeCommand: Application instance
SafemodeCommand->>Application: getLanguage()
Application-->>SafemodeCommand: Language instance
SafemodeCommand->>Language: load(plg_console_safemode, JPATH_PLUGINS/console/safemode, en-GB, false, true)
SafemodeCommand-->>ConsoleApplication: constructed with language loaded
deactivate SafemodeCommand
Class diagram for SafemodeCommand language loading refactorclassDiagram
class AbstractCommand
class DatabaseInterface
class Factory
class Application {
getLanguage()
}
class Language {
load(extension, basePath, lang, reload, default)
}
class SafemodeCommand {
+__construct(DatabaseInterface db)
}
SafemodeCommand --|> AbstractCommand
SafemodeCommand ..> DatabaseInterface : uses
SafemodeCommand ..> Factory : uses
Factory ..> Application : returns
Application ..> Language : returns
SafemodeCommand ..> Language : loads language file
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The large block of commented-out legacy SafemodeCommand code can likely be removed entirely now that the refactor is in place, to avoid confusion and keep the file maintainable.
- Instead of hard-coding 'en-GB' in the language->load() call, consider using the current language (e.g. null or $language->getTag()) so the command strings respect the site's active language.
- Since you’re explicitly loading the plugin language file, consider also passing the correct client and extension name (e.g. using the plugin’s extension name constant or helper) to avoid path/name drift if the plugin is relocated or renamed in the future.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The large block of commented-out legacy SafemodeCommand code can likely be removed entirely now that the refactor is in place, to avoid confusion and keep the file maintainable.
- Instead of hard-coding 'en-GB' in the language->load() call, consider using the current language (e.g. null or $language->getTag()) so the command strings respect the site's active language.
- Since you’re explicitly loading the plugin language file, consider also passing the correct client and extension name (e.g. using the plugin’s extension name constant or helper) to avoid path/name drift if the plugin is relocated or renamed in the future.
## Individual Comments
### Comment 1
<location path="src/plugins/console/safemode/src/CliCommand/SafemodeCommand.php" line_range="73-76" />
<code_context>
+ // Load plugin language files
+ $app = Factory::getApplication();
+ $language = $app->getLanguage();
+ $language->load('plg_console_safemode', JPATH_PLUGINS . '/console/safemode', 'en-GB', false, true);
+
// Get DB from DI container or Factory if not injected
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Avoid hardcoding the language tag and rely on the current language instead.
Using `'en-GB'` here forces English strings even when the CLI runs under another locale. Instead, use the current language so Joomla’s fallback chain applies:
```php
$language = $app->getLanguage();
$language->load('plg_console_safemode', JPATH_PLUGINS . '/console/safemode', $language->getTag(), false, true);
```
This keeps messages localized for non-English installations.
```suggestion
// Load plugin language files
$app = Factory::getApplication();
$language = $app->getLanguage();
$language->load('plg_console_safemode', JPATH_PLUGINS . '/console/safemode', $language->getTag(), false, true);
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+73
to
+76
| // Load plugin language files | ||
| $app = Factory::getApplication(); | ||
| $language = $app->getLanguage(); | ||
| $language->load('plg_console_safemode', JPATH_PLUGINS . '/console/safemode', 'en-GB', false, true); |
There was a problem hiding this comment.
suggestion (bug_risk): Avoid hardcoding the language tag and rely on the current language instead.
Using 'en-GB' here forces English strings even when the CLI runs under another locale. Instead, use the current language so Joomla’s fallback chain applies:
$language = $app->getLanguage();
$language->load('plg_console_safemode', JPATH_PLUGINS . '/console/safemode', $language->getTag(), false, true);This keeps messages localized for non-English installations.
Suggested change
| // Load plugin language files | |
| $app = Factory::getApplication(); | |
| $language = $app->getLanguage(); | |
| $language->load('plg_console_safemode', JPATH_PLUGINS . '/console/safemode', 'en-GB', false, true); | |
| // Load plugin language files | |
| $app = Factory::getApplication(); | |
| $language = $app->getLanguage(); | |
| $language->load('plg_console_safemode', JPATH_PLUGINS . '/console/safemode', $language->getTag(), false, true); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refactor SafemodeCommand class to improve structure and add language loading.
Summary by Sourcery
Refactor the SafeMode CLI command implementation and integrate plugin language loading in the console command.
Enhancements: