Skip to content

Conversation

FooChao
Copy link

@FooChao FooChao commented Sep 15, 2025

Functionality Added

  • Log in
  • Log out
  • Sign in

Others Changes

  • Add NavBar which will be reused by multiple components
  • Add ai/usage-log.md
  • Add docker-compose.yml
  • Update nginx api gateway
  • Add npm shortcuts on root directory

Known Issues:
The local mongodb yet to be tested. (maybe we can just remove it if we all ok with it)

Yet to do:
Sending verification link to email

Other notes:
Get the .env file in user service from me got the monbodb secret.

@hyc17003 hyc17003 self-requested a review September 16, 2025 04:32
Copy link

@hyc17003 hyc17003 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Log in, log out and signup work as expected, have some super minor comments on validation checks beforehand to handle some exceptions if any ever arise, but u can choose to ignore bc they r vv minor

Running npm audit within user-service shows that there are several dependency vulnerabilities:
8 vulnerabilities (4 low, 3 high, 1 critical)

maybe these can be fixed b4 we develop Peerprep further, running npm audit fix should fix it and abit of testing can be done to make sure the updates didn't break anything

Copy link

@CJianzhi CJianzhi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Tested sign up, login, logout, creating new tabs and everything works as intended.

There is a minor issue with duplicate username creation during signup described below.

Regarding the routing, should we only allow authenticated users to access the landing page? ( Currently unauthenticated users can visit the landing page without logging in)

hyc17003
hyc17003 previously approved these changes Sep 16, 2025
@LemonDrew
Copy link

LemonDrew commented Sep 17, 2025

Review Comments:

  1. Login and signup are all working as intended

Bugs encountered:

  1. Slight bug where clicking on the start button on the home page doesnt cause the navbar to change correspondingly

Future improvements to consider:
API endpoints may require jwt token verification

# Conflicts:
#	frontend/src/app/match/page.tsx
#	frontend/src/app/page.tsx
Copy link

@CJianzhi CJianzhi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will be maintaining use of single domain

Copy link

@LemonDrew LemonDrew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@LemonDrew LemonDrew merged commit 109d08d into master Sep 18, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants