SAK-52234 Profile accept all types of formatting for mobile number validation #14294
Conversation
WalkthroughReplaced validator-based mobile check with custom validation: strip non-digits to count digits and enforce allowed characters (digits, spaces, hyphens, parentheses, plus). On failure the component sets Changes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 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 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
webcomponents/tool/src/main/frontend/packages/sakai-account/src/SakaiAccount.js (1)
187-196: Fix scrollIntoView called on string instead of DOM element.Line 194 calls
scrollIntoView()on themobilevariable, which is a string value (from line 187), not a DOM element. This will cause a runtime error. The same issue exists at line 183 for email validation.Apply this diff to fix both the mobile and email scrollIntoView calls:
_saveContactInfo() { + const emailInput = this.renderRoot.querySelector("#email-input"); - const email = this.renderRoot.querySelector("#email-input").value; + const email = emailInput.value; if (!isEmail(email)) { this._currentError = this._i18n.invalid_email; this._displayContactInfoErrorBanner = true; this._emailInvalid = true; - email.scrollIntoView(); + emailInput.scrollIntoView(); return; } + const mobileInput = this.renderRoot.querySelector("#mobile-input"); - const mobile = this.renderRoot.querySelector("#mobile-input").value; + const mobile = mobileInput.value; // Remove common formatting characters for validation, but keep original format for storage const mobileForValidation = mobile ? mobile.replace(/[\s\-().]/g, "") : mobile; if (mobile && !isMobilePhone(mobileForValidation, "any")) { this._currentError = this._i18n.invalid_mobile; this._displayContactInfoErrorBanner = true; this._mobileInvalid = true; - mobile.scrollIntoView(); + mobileInput.scrollIntoView(); return; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
webcomponents/tool/src/main/frontend/packages/sakai-account/src/SakaiAccount.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.js
📄 CodeRabbit inference engine (.cursor/rules/frontend.mdc)
**/*.js: Use clean, standard modern JavaScript in Sakai frontends
Update jQuery code to modern JavaScript when making changes, if the changes are minimal
Prefer ES6+ features (arrow functions, template literals, destructuring, etc.) in JavaScript
Write modular, reusable JavaScript components in Sakai frontends
Minimize use of global variables and functions (Avoid Global Scope) in JavaScript
Files:
webcomponents/tool/src/main/frontend/packages/sakai-account/src/SakaiAccount.js
**/*.{js,html,jsp,vm,xhtml,xml}
📄 CodeRabbit inference engine (.cursor/rules/frontend.mdc)
Use the web components in the webcomponents/ directory when possible in Sakai frontends
Files:
webcomponents/tool/src/main/frontend/packages/sakai-account/src/SakaiAccount.js
webcomponents/**/*.js
📄 CodeRabbit inference engine (.cursor/rules/webcomponents-rule.mdc)
webcomponents/**/*.js: Use double quotes instead of single quotes in Sakai Webcomponents
Do not use trailing spaces in Sakai Webcomponents code
Use the i18n property for internationalization; never use an OR switch to also provide English text
Do not attempt to manipulate dates and times based on the browser's timezone; use the Sakai portal object which includes the user's preferred timezone
Prefix internal component state properties with an underscore (e.g.,_points) and expose getters if external read access is required; use Lit{ state: true }properties and prefer reactive declarative template bindings over imperative DOM access
Files:
webcomponents/tool/src/main/frontend/packages/sakai-account/src/SakaiAccount.js
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Target evergreen browsers; assume ES2022+ features and browser APIs likefetchkeepalive are present; avoid legacy branches, UA sniffing, or fallbacks unless a specific evergreen gap is documented
Replace jQuery with modern DOM APIs when touching code; new work should not add jQuery dependencies
Compose Lit components, ES modules, and encapsulated helpers; keep state local and explicit with modular code
Prefer module scope or class fields; expose intentional APIs instead of incidental globals; avoid global side channels
Files:
webcomponents/tool/src/main/frontend/packages/sakai-account/src/SakaiAccount.js
**/*/*.{js,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Internal reactive state in Lit components should stay prefixed with
_and is only surfaced through getters/setters when required
Files:
webcomponents/tool/src/main/frontend/packages/sakai-account/src/SakaiAccount.js
**/*.{html,jsp,jspx,xml,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Prefer kebab-case for values of HTML class and id attributes
Files:
webcomponents/tool/src/main/frontend/packages/sakai-account/src/SakaiAccount.js
🧠 Learnings (1)
📓 Common learnings
Learnt from: danielmerino
Repo: sakaiproject/sakai PR: 0
File: :0-0
Timestamp: 2025-09-12T09:45:31.330Z
Learning: The SakaiDateTimePicker's DateHelper.normalize() method handles space-separated datetime formats but falls back to JavaScript's built-in Date parsing for ISO8601 formats with 'T' and timezone. This causes inconsistent parsing between "+02" and "+02:00" timezone formats. The fix is to normalize timezone formats in the RSF template before passing to SakaiDateTimePicker.
Learnt from: ottenhoff
Repo: sakaiproject/sakai PR: 0
File: :0-0
Timestamp: 2025-10-07T15:11:27.298Z
Learning: In samigo’s Total Scores view (samigo/samigo-app/src/webapp/jsf/evaluation/totalScores.jsp), mailto links were hidden after commit dee05746 (PR #12312, SAK-49674) added a render check requiring email.fromEmailAddress to be non-empty; PR #14154 (SAK-52058) restores visibility by checking only description.email.
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: maven-build
- GitHub Check: sakai-deploy
- GitHub Check: maven-build
webcomponents/tool/src/main/frontend/packages/sakai-account/src/SakaiAccount.js
Outdated
Show resolved
Hide resolved
1 participant
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.