[678] feat: Add form field standardizer module - #167
Conversation
- Add form field standardizer module to standardize form field names\n- Create utility functions for field name standardization\n- Add comprehensive test suite for standardization logic\n- Register module in app.js
📝 Walkthrough""" WalkthroughA new module, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ApostropheCMS
participant form-field-standardizer
participant standardizeFieldNames
User->>ApostropheCMS: Submit form
ApostropheCMS->>form-field-standardizer: @apostrophecms/form:beforeSave event
form-field-standardizer->>standardizeFieldNames: standardizeFieldNames(doc)
standardizeFieldNames-->>form-field-standardizer: Standardized doc
form-field-standardizer-->>ApostropheCMS: Proceed with saving standardized doc
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (4)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
website/utils/standardizeFieldNames.js (1)
5-22: Consider documenting the mutation behavior and adding error handling.The function correctly implements field name standardization with good defensive programming. However, consider these improvements:
- Mutation: The function mutates the input document, which might be unexpected. Consider adding JSDoc to clarify this behavior.
- Error handling: No validation that
STANDARD_FORM_FIELD_NAMESis properly structured.- Return value: Consider returning a boolean or count indicating if any changes were made.
Add JSDoc documentation:
+/** + * Standardizes field names in a document's contents.items array. + * Mutates the input document in-place. + * @param {Object} doc - The document to standardize + */ const standardizeFieldNames = (doc) => {Consider adding basic validation:
const standardizeFieldNames = (doc) => { if (!doc) return; const fieldNames = Object.values(STANDARD_FORM_FIELD_NAMES); + if (!Array.isArray(fieldNames) || fieldNames.length === 0) return;website/modules/form-field-standardizer/index.js (1)
10-18: Good integration pattern with room for error handling.The module correctly follows ApostropheCMS conventions using the
improvepattern and registers the event handler appropriately. Consider adding error handling for robustness:Add error handling to prevent save failures:
handlers(self) { return { '@apostrophecms/form:beforeSave': { standardizeFieldNames(req, doc) { - standardizeFieldNames(doc); + try { + standardizeFieldNames(doc); + } catch (error) { + self.apos.util.error('Failed to standardize field names:', error); + // Continue with save - don't block form saving due to standardization errors + } }, }, }; },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
website/app.js(1 hunks)website/modules/form-field-standardizer/index.js(1 hunks)website/utils/standardizeFieldNames.js(1 hunks)website/utils/standardizeFieldNames.test.js(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
website/utils/standardizeFieldNames.test.js (1)
website/utils/standardizeFieldNames.js (2)
require(1-3)standardizeFieldNames(5-22)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: e2e-tests
- GitHub Check: lint
- GitHub Check: security-scan
- GitHub Check: unit-tests
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (actions)
🔇 Additional comments (1)
website/app.js (1)
104-104: LGTM! Proper module registration.The module is correctly registered following ApostropheCMS conventions and is appropriately placed near other form-related modules.
- Add instructions field to form module with field order guidance\n- Remove heading toolbar from form widget for consistency\n- Update form styles for instructions field
823184f
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
website/modules/@apostrophecms/form-widget/index.js(1 hunks)website/modules/@apostrophecms/form/index.js(1 hunks)website/modules/asset/ui/src/scss/_form.scss(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- website/modules/asset/ui/src/scss/_form.scss
- website/modules/@apostrophecms/form-widget/index.js
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: security-scan
- GitHub Check: e2e-tests
- GitHub Check: lint
- GitHub Check: unit-tests
- GitHub Check: Analyze (actions)
- GitHub Check: Analyze (javascript-typescript)
|



Uh oh!
There was an error while loading. Please reload this page.