Multicheck now saves information correctly to the database #183
Conversation
WalkthroughAdds a new computed property Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
assets/src/admin/components/Fields.vue (1)
55-55: Normalize checkbox values to strings (avoids checked-state mismatches) and optionally align name to array semantics.Vue compares checkbox v-model values strictly. If option keys can be numbers or booleans from the schema/backend, coercing to string prevents subtle mismatch when restoring selections. Optional: switch the name attribute to use [] to reflect array semantics (useful if the DOM is ever serialized).
- <input type="checkbox" class="checkbox" :id="sectionId + '[' + fieldData.name + '][' + optionKey + ']'" :name="sectionId + '[' + fieldData.name + '][' + optionKey + ']'" v-model="multicheckValue" :value="optionKey" /> + <input + type="checkbox" + class="checkbox" + :id="sectionId + '[' + fieldData.name + '][' + optionKey + ']'" + :name="sectionId + '[' + fieldData.name + '][]'" + v-model="multicheckValue" + :value="String(optionKey)" + />assets/js/admin.js (1)
31-31: Avoid eval() in distributed bundles; use non-eval source maps for production.The bundle uses eval-based modules (likely devtool: eval-source-map). This triggers security linters and can conflict with CSP. Ship production builds with devtool set to 'source-map' (or false) and without eval.
Also applies to: 43-43
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
assets/js/admin.js(2 hunks)assets/src/admin/components/Fields.vue(2 hunks)
🧰 Additional context used
🪛 Biome (2.1.2)
assets/js/admin.js
[error] 31-31: eval() exposes to security risks and performance issues.
See the MDN web docs for more details.
Refactor the code so that it doesn't need to call eval().
(lint/security/noGlobalEval)
[error] 43-43: eval() exposes to security risks and performance issues.
See the MDN web docs for more details.
Refactor the code so that it doesn't need to call eval().
(lint/security/noGlobalEval)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
assets/src/admin/components/Fields.vue (1)
202-236: LGTM: Legacy migration + normalization are handled wellGetter/settter correctly migrate object-map → array, normalize to strings, and dedupe. This addresses prior feedback.
🧹 Nitpick comments (3)
assets/src/admin/components/Fields.vue (3)
53-59: Add a stable key for v-forAvoids Vue warnings and improves patching performance.
Apply this diff:
- <label :for="sectionId + '[' + fieldData.name + '][' + optionKey + ']'"> + <label :key="String(optionKey)" :for="sectionId + '[' + fieldData.name + '][' + optionKey + ']'">
221-226: Filter to allowed option keys and drop nullish valuesPrevents persisting unknown/tainted keys and avoids accidental "undefined"/empty entries. Keeps UI and saved data consistent when options change.
Apply this diff:
- // Normalize to strings and dedupe to keep v-model stable - const normalized = Array.from(new Set(raw.map(String))); - if (normalized.length !== raw.length || normalized.some((v, i) => v !== raw[i])) { - this.$set(this.fieldValue, this.fieldData.name, normalized); - } - return normalized; + // Normalize (drop nullish), dedupe, and filter to allowed keys + const normalized = Array.from( + new Set( + raw + .filter(v => v !== undefined && v !== null && v !== '') + .map(v => String(v)) + ) + ); + const allowedKeys = Array.isArray(this.fieldData.options) + ? this.fieldData.options.map((_, i) => String(i)) + : Object.keys(this.fieldData.options || {}); + const finalNormalized = normalized.filter(v => allowedKeys.includes(v)); + if (finalNormalized.length !== raw.length || finalNormalized.some((v, i) => v !== raw[i])) { + this.$set(this.fieldValue, this.fieldData.name, finalNormalized); + } + return finalNormalized; @@ - const normalized = Array.isArray(newValue) - ? Array.from(new Set(newValue.map(String))) - : []; - this.$set(this.fieldValue, this.fieldData.name, normalized); + const normalized = Array.isArray(newValue) + ? Array.from( + new Set( + newValue + .filter(v => v !== undefined && v !== null && v !== '') + .map(v => String(v)) + ) + ) + : []; + const allowedKeys = Array.isArray(this.fieldData.options) + ? this.fieldData.options.map((_, i) => String(i)) + : Object.keys(this.fieldData.options || {}); + const finalNormalized = normalized.filter(v => allowedKeys.includes(v)); + this.$set(this.fieldValue, this.fieldData.name, finalNormalized);If backend sanitizes these already, feel free to skip—but please verify.
Also applies to: 229-234
202-236: Optional: avoid side-effects inside computed gettersSetting reactive state inside a computed getter can cause extra recomputations. Consider moving normalization/migration into watchers on
fieldValue[fieldData.name]andfieldData.options.Example (outside this block):
watch: { 'fieldValue[fieldData.name]': { handler(val) { /* normalize/filter then this.$set(...) */ }, deep: false, }, 'fieldData.options': { handler() { /* re-filter saved values against new options */ }, deep: false, } }
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
1 participant
When using the multicheck option for configuration, the information is not being saved correctly in the database. With this change, it works as expected.
Summary by CodeRabbit
Bug Fixes
Refactor