Repository navigation
fix(onboarding): accept periods in person names and surface pre-fill validation errors - #10170
Emreceyhnn wants to merge 2 commits into
Conversation
…validation errors Fixes makeplane#10164 GitLab sign-in pre-fills the onboarding profile Name field with the GitLab account name, which is often username-shaped and contains a period (e.g. john.doe). validatePersonName rejected periods, and because the form uses mode:"onChange", react-hook-form marked the prefilled field invalid without populating errors — leaving users stuck on a disabled Continue with no explanation. Changes: - PERSON_NAME_REGEX now allows periods (J. R. R. Tolkien, john.doe). Periods are a legitimate character in names and not an injection-risk character; XSS/SQL risk characters are still blocked by hasInjectionRiskChars. - Onboarding profile forms (steps/profile/root.tsx and onboarding/profile-setup.tsx) now run trigger on first_name/last_name on mount, so any invalid pre-filled value shows its validation message immediately instead of silently disabling Continue.
|
|
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Allow periods in person names packages/utils/src/validation.ts |
PERSON_NAME_REGEX and the validation message allow periods. The example includes a name with initials. |
Validate prefilled onboarding names apps/web/components/onboarding/profile-setup.tsx, apps/web/components/onboarding/steps/profile/root.tsx |
Both forms trigger validation for first_name and last_name when a user is present and either name changes. |
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix · Severity of issue fixed: Medium
Possibly related PRs
- makeplane/plane#10165: Changes the same name validator and onboarding validation behavior for names containing periods.
- makeplane/plane#10174: Changes the shared validator to accept periods.
- makeplane/plane#10147: Adds optional last-name validation that uses the shared validator.
Suggested reviewers: dheeru0198
Merge Risk: 
07a2a
-
Names containing periods are accepted, and onboarding now requests validation of prefilled names. No actionable merge-blocking issue remains identified; normal checks should complete before merging.
Pre-merge checks: 
-
Check name Status Explanation Title check The title names the two main changes: periods are now accepted in person names, and pre-fill validation errors are surfaced. Both match the diff, which changes PERSON_NAME_REGEX and adds trigger calls… Description check The description covers the core of the template. It explains the problem and root cause, lists the changes to validation.ts and the two onboarding components, and gives test notes. It links the issue … Linked Issues check Issue #10164requires two behaviors. First, names with periods must be accepted. Second, an invalid pre-filled name must show its validation message at once. At the reviewed head, `packages/utils/src/…Out of Scope Changes check All three changed files implement #10164.validation.tschanges the person-name regex and its error text to accept periods. Both onboarding profile forms add the same mount-time validation for pre-f…Docstring Coverage No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Full details: Title check
-
Explanation
The title names the two main changes: periods are now accepted in person names, and pre-fill validation errors are surfaced. Both match the diff, which changes PERSON_NAME_REGEX and adds trigger calls on mount in the two onboarding profile components. The title is long at 86 characters, but it stays specific and accurate.
Full details: Description check
-
Explanation
The description covers the core of the template. It explains the problem and root cause, lists the changes to validation.ts and the two onboarding components, and gives test notes. It links the issue with 'Fixes
#10164'. The template's 'Type of Change' checkboxes are not marked, and the 'Screenshots and Media' section is missing. The 'References' section is filled only in the form of 'Fixes#10164'. These are non-critical gaps, so the description is mostly complete.
Full details: Linked Issues check
-
Explanation
Issue
#10164requires two behaviors. First, names with periods must be accepted. Second, an invalid pre-filled name must show its validation message at once. At the reviewed head,packages/utils/src/validation.tssetsPERSON_NAME_REGEXto/^[\p{L}\s'.-]+$/u.validatePersonNamestill callshasInjectionRiskChars, which blocks<>'"{}[]*^!#%. Sojohn.doeandJ. R. R. Tolkienpass, andJohn<script>fails. Second,apps/web/components/onboarding/steps/profile/root.tsxandapps/web/components/onboarding/profile-setup.tsxboth importuseEffectfrom React. Both calltrigger(["first_name", "last_name"])in an effect whenuserexists. The effect depends onuser?.first_nameanduser?.last_name. This fixes the prior FAIL, which came from a missinguseEffectimport inprofile-setup.tsx. Note: the PR testing note listsO'Brienas valid. The unchangedhasInjectionRiskCharspattern still rejects'. This note is inaccurate, but the behavior is pre-existing and is not part of#10164. No automated UI test is requested by the issue. Build or typecheck was not run in this review, so compile success rests on source reading.
Full details: Out of Scope Changes check
-
Explanation
All three changed files implement
#10164.validation.tschanges the person-name regex and its error text to accept periods. Both onboarding profile forms add the same mount-time validation for pre-filled names. Theprofile-setup.tsxchange applies the same fix to a second onboarding profile form. The issue scope covers onboarding profile name entry, so this change is related. No unrelated behavior change was found in the reviewed diff summary. The injection-risk checks and other validators are unchanged.
Full details: Docstring Coverage
-
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 3 files.
-
Finishing Touches
-
Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
Hi maintainers — the onboarding name/period fixes are in and the useEffect import flagged in earlier review is already resolved in the current head. Ready for review — happy to adjust per feedback. Thanks! |
|
✅ CLA has been signed, CodeRabbit review is complete, and all checks have passed. The PR is ready to be merged. Please take a look when a maintainer is ready to merge — thanks! |
Summary
Fixes #10164
When signing in with GitLab, the onboarding Create your profile step pre-fills the Name field with the GitLab account name — often username-shaped with a period (e.g.
john.doe).validatePersonNamerejected periods, and because the form usesmode: "onChange", react-hook-form marked the pre-filled field invalid without populatingerrors. The result: Continue stays disabled with no visible error, and users have to guess that the period was the problem.Root cause
PERSON_NAME_REGEX(packages/utils/src/validation.ts) did not allow periods.Changes
packages/utils/src/validation.ts—PERSON_NAME_REGEXnow allows periods (/^[\p{L}\s'.-]+$/u). Periods are legitimate in real names (J. R. R. Tolkien) and are not an injection-risk character. All XSS/SQL/template-injection characters are still blocked byhasInjectionRiskChars.apps/web/components/onboarding/steps/profile/root.tsxandapps/web/components/onboarding/profile-setup.tsx— runtrigger(["first_name", "last_name"])on mount so any invalid pre-filled value surfaces its validation message immediately, instead of leaving the user stuck on a disabled Continue.Testing
Happy to adjust wording or add the validation-message behavior to other forms if the maintainers prefer.
Summary by CodeRabbit