Fix/signup duplicate username

[ChaitanyaChute]

Related Issue
Closes: #251

Description
Handled duplicate username signup by checking both email and username before user creation and returning a consistent 400 response instead of a 500. Added a test to cover duplicate username registration.

How Has This Been Tested?

• Unit tests: auth routes (signup duplicate email/username).

Type of Change
[x] Bug fix
[ ] New feature
[ ] Code style update
[ ] Breaking change
[ ] Documentation update

Summary by CodeRabbit

• Improvements

  • Enhanced signup validation to prevent duplicate usernames and emails with improved error handling.

• Bug Fixes

  • Fixed login redirect behavior to route correctly after successful authentication.

[netlify]

✅ Deploy Preview for github-spy ready!

Name	Link
🔨 Latest commit	23f1147
🔍 Latest deploy log	https://app.netlify.com/projects/github-spy/deploys/6a06d8d1edfdc00008932e4a
😎 Deploy Preview	https://deploy-preview-252--github-spy.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[coderabbitai]

📝 Walkthrough

Walkthrough

Backend signup validation now checks both email and username uniqueness before user creation and gracefully handles MongoDB duplicate-key errors. Login redirects to root on success. Docker build stage installs full dependencies before copying remaining files. Includes test for username duplicate rejection.

Changes

Auth validation and frontend build updates

Layer / File(s)	Summary
Signup username validation and error handling backend/routes/auth.js	Signup handler queries both email and username fields using $or operator; returns 400 on pre-existing user. Catch block handles MongoDB duplicate-key errors (11000) by returning 400 instead of 500.
Test for duplicate username rejection spec/auth.routes.spec.cjs	Integration test seeds a user with duplicate username, attempts signup with same username but different email, asserts 400 status with "User already exists" message.
Login success navigation update src/pages/Login/Login.tsx	Post-login navigation target changed from "/home" to "/" when login is successful.
Docker build stage dependency installation Dockerfile.prod	Frontend build stage now runs npm install instead of npm install --production, with file copy order adjusted to copy package.json first, then install, then copy remaining application files.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• GitMetricsLab/github_tracker#91: Both PRs modify the frontend Dockerfile.prod build stage dependency-install flow, including changes to npm install behavior and file-copying order around package.json.

Suggested reviewers

• mehul-m-prajapati

Poem

🐰 Duplicate usernames caused quite a fright,
With 500 errors spoiling the night!
Now validation checks both fields with care,
Docker builds faster, and logins go there! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	The frontend Login.tsx change to redirect to '/' instead of '/home' is out of scope relative to the #251 issue objectives about signup validation.	Either remove the Login.tsx redirect change or explain its necessity in the PR description. Verify if this change aligns with a different issue or requirement.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change—fixing duplicate username handling during signup.
Description check	✅ Passed	The description covers required sections: related issue, clear description of changes, testing approach, and type of change.
Linked Issues check	✅ Passed	The PR fulfills all coding requirements from #251: validates email and username uniqueness before creation, returns 400 on duplicate, handles MongoDB constraint errors, and includes test coverage.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

spec/auth.routes.spec.cjs (1)

60-67: ⚡ Quick win

Assert no DB side-effect on duplicate-username rejection.

This test checks status/message, but not that signup avoided creating an additional user record.

Proposed test hardening

   it('should not sign up a user with existing username', async () => {
     await new User({ username: 'testuser', email: 'test@example.com', password: 'password123' }).save();
+    const beforeCount = await User.countDocuments();
     const res = await request(app)
       .post('/auth/signup')
       .send({ username: 'testuser', email: 'test2@example.com', password: 'password456' });
     expect(res.status).toBe(400);
     expect(res.body.message).toBe('User already exists');
+    const afterCount = await User.countDocuments();
+    expect(afterCount).toBe(beforeCount);
   });

🤖 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 `@spec/auth.routes.spec.cjs` around lines 60 - 67, The test "should not sign up
a user with existing username" currently asserts status and message but not DB
state; update it to verify no additional user record was created by checking the
User collection after the POST (use the User model used in the test) — e.g.,
confirm User.countDocuments({ username: 'testuser' }) remains 1 or assert total
users equals 1 and that no user exists with the second email — and keep the
existing expectations (res.status and res.body.message) unchanged.

🤖 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.

Nitpick comments:
In `@spec/auth.routes.spec.cjs`:
- Around line 60-67: The test "should not sign up a user with existing username"
currently asserts status and message but not DB state; update it to verify no
additional user record was created by checking the User collection after the
POST (use the User model used in the test) — e.g., confirm User.countDocuments({
username: 'testuser' }) remains 1 or assert total users equals 1 and that no
user exists with the second email — and keep the existing expectations
(res.status and res.body.message) unchanged.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: fc21ed71-577d-4ba5-95e6-0ab020c2176b

📥 Commits

Reviewing files that changed from the base of the PR and between 56e17a3 and 23f1147.

📒 Files selected for processing (4)

• Dockerfile.prod
• backend/routes/auth.js
• spec/auth.routes.spec.cjs
• src/pages/Login/Login.tsx