feat: Add Regex string validation when editing Cloud Config parameter#3222
feat: Add Regex string validation when editing Cloud Config parameter#3222mtrezza merged 3 commits intoparse-community:alphafrom
Conversation
|
🚀 Thanks for opening this pull request! |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
📝 WalkthroughWalkthroughThe changes add regex pattern validation to NonPrintableHighlighter (new helper exports and UI), integrate detectRegex/detectNonPrintable settings into ConfigDialog, and add toggles in CloudConfigSettings to persist these detection settings via the server. Changes
Sequence DiagramsequenceDiagram
actor User
participant CloudConfigSettings
participant Server
participant ConfigDialog
participant JsonEditor
participant NonPrintableHighlighter
User->>CloudConfigSettings: Toggle detectRegex/detectNonPrintable
CloudConfigSettings->>Server: Save setting
Server-->>CloudConfigSettings: Acknowledge
User->>ConfigDialog: Open editor
ConfigDialog->>Server: Load settings (detectRegex, detectNonPrintable)
Server-->>ConfigDialog: Return settings
ConfigDialog->>JsonEditor: Open with options (detectRegex, detectNonPrintable, syntaxColors)
JsonEditor->>NonPrintableHighlighter: Render with options
NonPrintableHighlighter->>NonPrintableHighlighter: Run getRegexValidation / getRegexValidationFromJson
NonPrintableHighlighter-->>User: Show regex badge and details panel
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/dashboard/Settings/CloudConfigSettings.react.js (1)
114-134: Consider extracting a shared toggle-save helper to reduce duplication.
handleDetectNonPrintableChangeandhandleDetectRegexChangeare structurally identical — only the state key and notification text differ. A small helper likehandleToggleChange(key, label, value)would eliminate the duplication and make it trivial to add future toggles.♻️ Proposed refactor
- async handleDetectNonPrintableChange(value) { - this.setState({ detectNonPrintable: value }); - // Don't store default value (true) on server - if (await this.saveSettings({ detectNonPrintable: value === true ? undefined : value })) { - this.showNote(`Non-printable character detection ${value ? 'enabled' : 'disabled'}.`); - } else { - this.setState({ detectNonPrintable: !value }); - this.showNote('Failed to save setting.', true); - } - } - - async handleDetectRegexChange(value) { - this.setState({ detectRegex: value }); - // Don't store default value (true) on server - if (await this.saveSettings({ detectRegex: value === true ? undefined : value })) { - this.showNote(`Regex validation display ${value ? 'enabled' : 'disabled'}.`); - } else { - this.setState({ detectRegex: !value }); - this.showNote('Failed to save setting.', true); - } - } + async handleToggleSettingChange(key, label, value) { + this.setState({ [key]: value }); + // Don't store default value (true) on server + if (await this.saveSettings({ [key]: value === true ? undefined : value })) { + this.showNote(`${label} ${value ? 'enabled' : 'disabled'}.`); + } else { + this.setState({ [key]: !value }); + this.showNote('Failed to save setting.', true); + } + }Then bind in the render:
- onChange={this.handleDetectNonPrintableChange.bind(this)} + onChange={this.handleToggleSettingChange.bind(this, 'detectNonPrintable', 'Non-printable character detection')} - onChange={this.handleDetectRegexChange.bind(this)} + onChange={this.handleToggleSettingChange.bind(this, 'detectRegex', 'Regex validation display')}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/Settings/CloudConfigSettings.react.js` around lines 114 - 134, The two handlers handleDetectNonPrintableChange and handleDetectRegexChange are duplicated; extract a shared helper (e.g., handleToggleChange(key, label, value)) that accepts the state key (detectNonPrintable/detectRegex), a human-readable label for notifications, and the new value; inside it call this.setState({ [key]: value }), call this.saveSettings({ [key]: value === true ? undefined : value }), on success call this.showNote using the label to build "label enabled/disabled" and on failure revert state with this.setState({ [key]: !value }) and show the failure note; then replace the two originals to call this.handleToggleChange('detectNonPrintable', 'Non-printable character detection', value) and this.handleToggleChange('detectRegex', 'Regex validation display', value).src/components/NonPrintableHighlighter/NonPrintableHighlighter.react.js (3)
567-569:regexNotesconstruction can be simplified.The current spread pattern creates an array with a single conditional entry:
const regexNotes = [ ...(requiredFlags.length > 0 ? [`${requiredFlags.join(', ')}`] : []), ];This is equivalent to a simpler conditional. Also, the template literal
`${requiredFlags.join(', ')}`is redundant since.join()already returns a string.♻️ Simplification
- const regexNotes = [ - ...(requiredFlags.length > 0 ? [`${requiredFlags.join(', ')}`] : []), - ]; + const regexNotes = requiredFlags.length > 0 + ? [requiredFlags.join(', ')] + : [];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/NonPrintableHighlighter/NonPrintableHighlighter.react.js` around lines 567 - 569, The array construction for regexNotes is needlessly using a spread and a template literal; replace it with a simple conditional assignment that returns an empty array when requiredFlags is empty and otherwise returns an array with requiredFlags.join(', ') as the single entry. Update the declaration for regexNotes accordingly (referencing regexNotes and requiredFlags) so the code no longer uses the spread or the redundant `${...}` template.
537-569: Regex results are recomputed on every render — acceptable for typical payloads but worth noting.
getRegexValidationandgetRegexValidationFromJsonrun synchronously inrender()on every keystroke. For typical Cloud Config values this is fine, but for large JSON payloads with many string values it could become noticeable. If performance ever becomes a concern, memoizing or debouncing the computation would help.Not a blocker — just a heads-up.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/NonPrintableHighlighter/NonPrintableHighlighter.react.js` around lines 537 - 569, The render() of NonPrintableHighlighter currently calls getRegexValidation and getRegexValidationFromJson synchronously on every render (via the detectRegex branch that reads value/isJson), which can be expensive for large payloads; modify the component to avoid recomputing on every keystroke by memoizing or debouncing the regex validation: either memoize results of getRegexValidation/getRegexValidationFromJson keyed by the input value (or serialized JSON) using a stable memo helper (e.g., memoizeOne or a simple cache) and call the memoized functions inside render, or move the computation out of render into lifecycle (componentDidUpdate) or a debounced handler that updates component state with regexResult when value/isJson changes, and then read regexResult from state in render; update references to detectRegex, getRegexValidation, getRegexValidationFromJson, regexResult, and the render() method accordingly.
459-483: Well-designed regex validation with graceful flag fallback.The three-mode approach (no flags →
u→v) with early return on first success is clean and the preference order makes sense. The code gracefully degrades ifvflag is unsupported—earlier modes will catch valid patterns before any potentially misleading "Invalid flags" error surfaces.As of early 2026, the
vflag (unicodeSets) is now standard across all major browsers (Chrome 112+, Firefox 116+, Safari 17+) and Node.js 20+, so the risk of encountering an unsupported runtime is minimal for modern deployments. However, if this dashboard supports older environments or strict legacy requirements, feature-detecting thevflag remains a clean practice:♻️ Optional: feature-detect `v` flag support
+// Feature-detect RegExp 'v' flag support (ES2024 unicodeSets) +let supportsVFlag = false; +try { + new RegExp('', 'v'); + supportsVFlag = true; +} catch { + // Not supported in this environment +} + export function getRegexValidation(str) { // ... const modes = [ { flags: '', flag: null }, { flags: 'u', flag: 'unicode' }, - { flags: 'v', flag: 'unicodeSets' }, + ...(supportsVFlag ? [{ flags: 'v', flag: 'unicodeSets' }] : []), ];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/NonPrintableHighlighter/NonPrintableHighlighter.react.js` around lines 459 - 483, The current getRegexValidation uses trial-and-error to test flags; add a one-time feature-detection for the 'v' (unicodeSets) flag before the modes loop to avoid spurious "Invalid flags" errors on runtimes that don't support it: in getRegexValidation, attempt new RegExp('', 'v') inside a try/catch once to set a boolean like supportsV, then build the modes array conditionally (include the { flags: 'v', flag: 'unicodeSets' } entry only if supportsV is true) and proceed with the existing validation loop so requiredFlag returns 'unicodeSets' only when the runtime actually supports it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/dashboard/Data/Config/ConfigDialog.react.js`:
- Around line 178-196: The method loadDetectRegexSetting is misnamed and its
catch comment is incorrect: rename the method to loadDetectSettings (or
loadValueAnalysisSettings) wherever referenced, update its internal comment to
reflect that it intentionally ignores errors while keeping the current defaults
(which are true for detectNonPrintable and detectRegex), and adjust any call
sites to use the new name; keep the logic that reads from
ServerConfigStorage.getConfig(CONFIG_KEY, this.context.applicationId) and sets
state via this.setState({ detectNonPrintable: ..., detectRegex: ... })
unchanged.
---
Nitpick comments:
In `@src/components/NonPrintableHighlighter/NonPrintableHighlighter.react.js`:
- Around line 567-569: The array construction for regexNotes is needlessly using
a spread and a template literal; replace it with a simple conditional assignment
that returns an empty array when requiredFlags is empty and otherwise returns an
array with requiredFlags.join(', ') as the single entry. Update the declaration
for regexNotes accordingly (referencing regexNotes and requiredFlags) so the
code no longer uses the spread or the redundant `${...}` template.
- Around line 537-569: The render() of NonPrintableHighlighter currently calls
getRegexValidation and getRegexValidationFromJson synchronously on every render
(via the detectRegex branch that reads value/isJson), which can be expensive for
large payloads; modify the component to avoid recomputing on every keystroke by
memoizing or debouncing the regex validation: either memoize results of
getRegexValidation/getRegexValidationFromJson keyed by the input value (or
serialized JSON) using a stable memo helper (e.g., memoizeOne or a simple cache)
and call the memoized functions inside render, or move the computation out of
render into lifecycle (componentDidUpdate) or a debounced handler that updates
component state with regexResult when value/isJson changes, and then read
regexResult from state in render; update references to detectRegex,
getRegexValidation, getRegexValidationFromJson, regexResult, and the render()
method accordingly.
- Around line 459-483: The current getRegexValidation uses trial-and-error to
test flags; add a one-time feature-detection for the 'v' (unicodeSets) flag
before the modes loop to avoid spurious "Invalid flags" errors on runtimes that
don't support it: in getRegexValidation, attempt new RegExp('', 'v') inside a
try/catch once to set a boolean like supportsV, then build the modes array
conditionally (include the { flags: 'v', flag: 'unicodeSets' } entry only if
supportsV is true) and proceed with the existing validation loop so requiredFlag
returns 'unicodeSets' only when the runtime actually supports it.
In `@src/dashboard/Settings/CloudConfigSettings.react.js`:
- Around line 114-134: The two handlers handleDetectNonPrintableChange and
handleDetectRegexChange are duplicated; extract a shared helper (e.g.,
handleToggleChange(key, label, value)) that accepts the state key
(detectNonPrintable/detectRegex), a human-readable label for notifications, and
the new value; inside it call this.setState({ [key]: value }), call
this.saveSettings({ [key]: value === true ? undefined : value }), on success
call this.showNote using the label to build "label enabled/disabled" and on
failure revert state with this.setState({ [key]: !value }) and show the failure
note; then replace the two originals to call
this.handleToggleChange('detectNonPrintable', 'Non-printable character
detection', value) and this.handleToggleChange('detectRegex', 'Regex validation
display', value).
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/dashboard/Data/Config/ConfigDialog.react.js (2)
31-31:CONFIG_KEYis duplicated withCloudConfigSettings.react.jsBoth files independently define
'config.settings'. If this key ever changes in one file but not the other,loadValueAnalysisSettingswill silently read stale/missing data. Extract it to a shared constants module (e.g.src/lib/configKeys.js) and import from both.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/Data/Config/ConfigDialog.react.js` at line 31, The constant CONFIG_KEY = 'config.settings' is duplicated (also in CloudConfigSettings.react.js) which risks divergence; extract this shared key into a new module (e.g. export const CONFIG_KEY from src/lib/configKeys.js) and update both ConfigDialog.react.js and CloudConfigSettings.react.js to import CONFIG_KEY instead of redefining it so loadValueAnalysisSettings and other consumers read the same canonical key.
186-191: Batch the twosetStatecalls into oneIn React 16,
setStateinside async callbacks is not batched — the current code schedules two separate re-renders. Merge them.♻️ Proposed fix
- if (settings?.detectNonPrintable !== undefined) { - this.setState({ detectNonPrintable: !!settings.detectNonPrintable }); - } - if (settings?.detectRegex !== undefined) { - this.setState({ detectRegex: !!settings.detectRegex }); - } + const nextState = {}; + if (settings?.detectNonPrintable !== undefined) { + nextState.detectNonPrintable = !!settings.detectNonPrintable; + } + if (settings?.detectRegex !== undefined) { + nextState.detectRegex = !!settings.detectRegex; + } + if (Object.keys(nextState).length > 0) { + this.setState(nextState); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/Data/Config/ConfigDialog.react.js` around lines 186 - 191, Batch the two separate this.setState calls into one to avoid two re-renders: in the code that checks settings (the block referencing settings?.detectNonPrintable and settings?.detectRegex), replace the two individual this.setState calls with a single this.setState that sets both detectNonPrintable and detectRegex together (use the boolean coercions !!settings.detectNonPrintable and !!settings.detectRegex or an updater function to merge with prior state).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/dashboard/Data/Config/ConfigDialog.react.js`:
- Line 31: The constant CONFIG_KEY = 'config.settings' is duplicated (also in
CloudConfigSettings.react.js) which risks divergence; extract this shared key
into a new module (e.g. export const CONFIG_KEY from src/lib/configKeys.js) and
update both ConfigDialog.react.js and CloudConfigSettings.react.js to import
CONFIG_KEY instead of redefining it so loadValueAnalysisSettings and other
consumers read the same canonical key.
- Around line 186-191: Batch the two separate this.setState calls into one to
avoid two re-renders: in the code that checks settings (the block referencing
settings?.detectNonPrintable and settings?.detectRegex), replace the two
individual this.setState calls with a single this.setState that sets both
detectNonPrintable and detectRegex together (use the boolean coercions
!!settings.detectNonPrintable and !!settings.detectRegex or an updater function
to merge with prior state).
# [9.0.0-alpha.6](9.0.0-alpha.5...9.0.0-alpha.6) (2026-02-17) ### Features * Add Regex string validation when editing Cloud Config parameter ([#3222](#3222)) ([067b9d1](067b9d1))
|
🎉 This change has been released in version 9.0.0-alpha.6 |
Pull Request
Issue
Add Regex string validation when editing Cloud Config parameter.
Summary by CodeRabbit