fix: pad username suggestion suffixes to a consistent width#29571
Conversation
generateUsernameSuggestion built its numeric suffix with
String(rand).padStart(4 - rand.toString().length, "0"). padStart's first
argument is the target total length, not the number of pad characters, so the
expression only padded single-digit numbers: rand=1 became "001" but rand=42
became "42" and rand=999 stayed "999". Suggested usernames therefore had
inconsistent suffix widths, and the magic 4 didn't match the 3-wide output the
existing test pins ("johnny001").
Pull the suffix into a small helper that always pads to 3 digits, so collisions
and the returned value share one definition and every suggestion is uniformly
zero-padded. Add unit tests for the first-attempt path and for two- and
three-digit suffixes; the two-digit case fails on the old code.
|
Welcome to Cal.diy, @yashs33244! Thanks for opening this pull request. A few things to keep in mind:
A maintainer will review your PR soon. Thanks for contributing! |
📝 WalkthroughWalkthrough
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![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 (1)
packages/lib/server/username.test.ts (1)
60-85: ⚡ Quick winAdd a 1-character username regression test for suffix width.
Current tests don’t exercise the
username.length < 2path, so the fixed-width guarantee isn’t protected there. Add one test that forces a generated value>= 1000and asserts whether width must remain 3 (per intended contract).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/lib/server/username.test.ts` around lines 60 - 85, Add a regression test to the generateUsernameSuggestion test suite that exercises the username.length < 2 code path. Create a test that uses a single-character username (e.g., "a"), mocks Math.random to force generation of a suffix value >= 1000, and asserts that the result maintains the intended fixed-width padding contract. This ensures the zero-padding behavior is consistent and protected for short usernames where the suffix limit is higher than the username.length >= 2 case.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/lib/server/username.ts`:
- Around line 59-65: The limit value for short usernames (length < 2) is set to
9999 on line 59, which allows rand to reach values like 1000 or higher on line
64. When these larger values are passed to the suffix function on line 65, they
produce 4+ digit strings instead of the promised 3-digit fixed-width format.
Change the limit from 9999 to 999 for short usernames so that the maximum rand
value is 999, ensuring the padStart(3, "0") on the suffix function always
produces exactly 3 digits and maintains the fixed-width contract consistently
across all username lengths.
---
Nitpick comments:
In `@packages/lib/server/username.test.ts`:
- Around line 60-85: Add a regression test to the generateUsernameSuggestion
test suite that exercises the username.length < 2 code path. Create a test that
uses a single-character username (e.g., "a"), mocks Math.random to force
generation of a suffix value >= 1000, and asserts that the result maintains the
intended fixed-width padding contract. This ensures the zero-padding behavior is
consistent and protected for short usernames where the suffix limit is higher
than the username.length >= 2 case.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ca403a53-b558-4a95-8b5e-89c78970c5fa
📒 Files selected for processing (2)
packages/lib/server/username.test.tspackages/lib/server/username.ts
bandhan-majumder
left a comment
bandhan-majumder
left a comment
There was a problem hiding this comment.
please create an issue first and link it to the PR.
|
This one's already linked to #29577 ( |
bandhan-majumder
added
ready-for-e2e
run-ci
bandhan-majumder
added
ready-for-e2e
run-ci
Closes #29577.
generateUsernameSuggestionusedpadStart(4 - len, "0"), butpadStart's first arg is the target length, not the pad count — so only single-digit suffixes got padded (1→001, but42→42,999→999).Pulled the padding into a small
suffix()helper (it was duplicated) that pads to a consistent 3 digits, matching the existingjohnny001test, and added the two/three-digit cases.yarn viteston the file: 6/6, red-green checked.