[FIX] Added JSON Formatting check for custom translations field #27873
Conversation
Pradumn27
changed the title
Pradumn27
changed the title
dudanogueira
changed the title
Codecov Report
@@ Coverage Diff @@
## develop #27873 +/- ##
===========================================
+ Coverage 42.03% 43.19% +1.16%
===========================================
Files 842 818 -24
Lines 17604 17152 -452
Branches 2013 1946 -67
===========================================
+ Hits 7399 7409 +10
+ Misses 9939 9476 -463
- Partials 266 267 +1
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
@dudanogueira can you review and merge the PR, Thank you. |
dougfabris
changed the title
There was a problem hiding this comment.
Heeey @Pradumn27 ! Thanks for suggesting these changes
Please check #28600
| @@ -41,6 +52,9 @@ Meteor.methods({ | |||
| case 'multiSelect': | |||
| check(value, Array); | |||
| break; | |||
| case 'code': | |||
| parseToJSON(value); | |||
There was a problem hiding this comment.
Not all code settings are JSON (eg some of them are CSS or HTML). We should check settings' code property to assure they should be a valid JSON.
| @@ -6,6 +6,17 @@ import { hasPermission } from '../../../authorization/server'; | |||
| import { getSettingPermissionId } from '../../../authorization/lib'; | |||
| import { twoFactorRequired } from '../../../2fa/server/twoFactorRequired'; | |||
|
|
|||
| const parseToJSON = (customTranslations) => { | |||
| try { | |||
| JSON.parse(customTranslations); | |||
There was a problem hiding this comment.
We should also accept empty strings as valid values for settings, otherwise they can't be erased.
|
Closing this one in favor of: #28600 |
Proposed changes (including videos or screenshots)
Currently the UI is breaking when an invalid JSON is sent in custom translations. So, I've added the check for valid JSON on the frontend to avoid the UI breaking due to that. Along with that I've also added the valid JSON Formatting check on the saveSettings call at the backend thus ensuring end to end checks for this field.
I've also changed the help text field here to avoid the confusion caused due to that.
Screen.Recording.2023-01-27.at.9.08.06.PM.mov
Issue(s)
Closes #27846
Steps to test or reproduce
Further comments