Fix missing lexicon entries#16960
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 3.x #16960 +/- ##
=========================================
Coverage 21.67% 21.68%
- Complexity 10786 10787 +1
=========================================
Files 566 566
Lines 33149 33156 +7
=========================================
+ Hits 7186 7189 +3
- Misses 25963 25967 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
f79e396 to
4ccc702
Compare
smg6511
left a comment
There was a problem hiding this comment.
Thomas: Thanks for putting this together—I promise I'm not trying to be a PITA, but after going through this very carefully I do have a bunch of requests. Most are easily applied, but there are a couple which would need a change to files not currently in this PR.
| $_lang['context'] = 'Context'; | ||
| $_lang['context_add'] = 'Add Context'; | ||
| $_lang['context_data'] = 'Context Data'; | ||
| $_lang['context_duplicate_confirm'] = 'Are you sure you want to duplicate this context?'; |
There was a problem hiding this comment.
This entry appears to be a legacy one—there is no confirmation dialog when duplicating a Context. The controller (context.view.php) and view in which this key was found (manager/assets/modext/sections/context/view.js) also look to be unused and should be removed as there's no such view in 3.x.
| $_lang['context_duplicate_confirm'] = 'Are you sure you want to duplicate this context?'; |
| $_lang['user_male'] = 'Male'; | ||
| $_lang['user_management_msg'] = 'Here you can choose which user you wish to edit.'; | ||
| $_lang['user_mobile'] = 'Mobile phone number'; | ||
| $_lang['user_new'] = 'Create User'; |
There was a problem hiding this comment.
This already exists via the key new_user; so, update the key in widgets/security/modx.panel.user.js to match.
| $_lang['user_new'] = 'Create User'; |
| $_lang['add_tv'] = 'Create TV'; | ||
| $_lang['add_weblink'] = 'Create Weblink'; | ||
| $_lang['add_symlink'] = 'Create Symlink'; | ||
| $_lang['alert'] = 'Alert'; |
There was a problem hiding this comment.
It certainly doesn't hurt to have this but, from what I found, there are nuances in three places where this key is used:
downloadFilemethod inmodx.tree.directory.jsandmodx.browser.js. Of note here is that theMODx.msg.alertcall seems to be overridden by something and is not actually used, although the presence of the failure listener is required, even if its declared like so:failure: { fn: Ext.emptyFn }processResponseinmodx.tree.treeloader.jswhere the msg.alert is actually used. Here however, "Error" or "Warning" is probably the better dialog title rather than "Alert," in which case we should change the key, i.e.:MODx.msg.alert(_('error'), json.message);
| $_lang['backup'] = 'Backup'; | ||
| $_lang['bk_manager'] = 'Backup'; | ||
| $_lang['bulk_actions'] = 'Bulk Actions'; | ||
| $_lang['cache_err_write'] = 'Cannot cache the file to download!'; |
There was a problem hiding this comment.
The class this is called in is specific to the Console, so we should be more clear about that. Further we should change the key to be more clear as well (e.g., console_download_cache_err
| $_lang['cache_err_write'] = 'Cannot cache the file to download!'; | |
| $_lang['console_download_cache_err'] = 'Could not download the file due to a problem caching the Console’s content for output!'; |
Note that there are two additional todos to complete this fix:
- Change the key in
Processors/System/DownloadOutput.phpfromcache_err_writetoconsole_download_cache_err - Add an empty failure to the modx.console.js file, i.e., add the following to the
downloadmethod's listeners:failure: { fn: Ext.emptyFn },; otherwise the failure message that this Lex is used in never gets shown.
| $_lang['category_remove'] = 'Delete Category'; | ||
| $_lang['chunk'] = 'Chunk'; | ||
| $_lang['chunks'] = 'Chunks'; | ||
| $_lang['class_err_ns'] = 'Class not specified.'; |
There was a problem hiding this comment.
I understand that this was the original key, but it is exclusively used in relation to Derivatives (in the current codebase) so a more specific key and message would be better:
| $_lang['class_err_ns'] = 'Class not specified.'; | |
| $_lang['derivatives_parent_class_err_empty'] = 'Can not get this object’s child classes because its class was not specified.'; |
Again, a couple changes would be needed to the calling class (System/Derviatives/GetList.php) at ~L49:
- Change the Lex key, and
- Add a return to the statement
So, return $this->failure($this->modx->lexicon('derivatives_parent_class_err_empty'));
Without the return, the error will never show in the front end.
| $_lang['user_password_email'] = '<h2>Set up your password</h2><p>We received a request to set up your MODX Revolution password. You can set up your password by clicking the button below and following the instructions on screen.</p><p class="center"><a href="[[+url_scheme]][[+http_host]][[+manager_url]]?modhash=[[+hash]]" class="btn">Set up my password</a></p><p class="small">If you did not send this request, please ignore this email.</p>'; | ||
|
|
||
| // Aliases | ||
| $_lang['user_group_user_create'] = $_lang['user_group_user_add']; |
There was a problem hiding this comment.
Just adding a couple aliases for re-keyed items...
| $_lang['user_group_user_create'] = $_lang['user_group_user_add']; | |
| $_lang['user_err_nf'] = $_lang['user_err_not_found']; | |
| $_lang['user_profile_err_nf'] = $_lang['user_profile_err_not_found']; |
| $this->user = $this->modx->user; | ||
| if (!$this->user) { | ||
| return $this->modx->lexicon('user_err_not_found'); | ||
| return $this->modx->lexicon('user_err_nf'); |
There was a problem hiding this comment.
Let's keep this as it was, but re-key as suggested in the Lex file:
| return $this->modx->lexicon('user_err_nf'); | |
| return $this->modx->lexicon('user_err_not_found'); |
Note that the alias I added in the Lex suggestion would cover any existing usage of the old key, but we could also update the key in the other two places it's found: the security/profile.class.php and security/user/update.class.php (below) controllers
| $this->profile = $this->modx->user->getOne('Profile'); | ||
| if ($this->profile === null) { | ||
| return $this->modx->lexicon('user_profile_err_not_found'); | ||
| return $this->modx->lexicon('user_profile_err_nf'); |
There was a problem hiding this comment.
Again, let's keep the newer key style, and re-key the Lex entry
| return $this->modx->lexicon('user_profile_err_nf'); | |
| return $this->modx->lexicon('user_profile_err_not_found'); |
| $_lang['user_photo_message'] = 'Enter the image URL for this user or use the insert button to select or upload an image file on the server.'; | ||
| $_lang['user_prevlogin'] = 'Previous Login'; | ||
| $_lang['user_prevlogin_desc'] = 'The previous time that the user successfully logged in.'; | ||
| $_lang['user_profile_err_nf'] = 'FATAL ERROR: User profile not found.'; |
There was a problem hiding this comment.
Re-key to match the Profile/Update processor. While we're doing so, no need for the error type in the message
| $_lang['user_profile_err_not_found'] = 'User profile not found.'; |
| $name = $this->getProperty('name'); | ||
| if (empty($name)) { | ||
| $this->addFieldError('name', $this->modx->lexicon('namespace_err_ns_name')); | ||
| $this->addFieldError('name', $this->modx->lexicon($this->objectType . '_err_ns')); |
There was a problem hiding this comment.
This is exclusive to one object, so no use for dynamic Lex key IMO
| $this->addFieldError('name', $this->modx->lexicon($this->objectType . '_err_ns')); | |
| $this->addFieldError('name', $this->modx->lexicon('namespace_err_ns')); |
What changed and why
Add missing lexicon entries and rename keys to use existing lexicon entries
Related issue(s)/PR(s)
#16955