feat(ui): [#3] Build registration and login forms#16
Conversation
This commit implements a new shared library (`libs/shared-validation`) and configures the entire monorepo to support it. This was required to share the password validation regex between the frontend and backend. Changes include: - Adding the `@shared/validation` path alias to both the backend and frontend `tsconfig.json` files. - Updating the backend's Jest config (`moduleNameMapper`) to resolve the alias during tests. - Swapping `bcrypt` for `bcryptjs` (a pure-JS library) to resolve a build conflict with Webpack. - Configuring the backend's `nest-cli.json` to use the `webpack` builder, which is required to bundle the aliased `libs` folder for production.
…dundant validations
📝 WalkthroughWalkthroughAdds a complete auth feature: frontend AuthModule (login/register components, service, tests), shared DTOs and validation constants moved into libs, backend switched to Changes
Sequence DiagramsequenceDiagram
participant User as User
participant Frontend as Frontend App
participant AuthService as Frontend AuthService
participant Backend as Backend API
participant DB as Database
User->>Frontend: Fill login/register form & submit
Frontend->>Frontend: Client-side validation (EMAIL_REGEX_STRING / PASSWORD_REGEX_STRING)
alt invalid
Frontend->>User: Show validation errors
else valid
Frontend->>AuthService: call login() or register()
AuthService->>Backend: POST /auth/login or /auth/register
Backend->>Backend: Validate DTOs (shared-dtos) and business logic
Backend->>DB: Query / create user
DB-->>Backend: user result
Backend->>Backend: Hash/verify password (bcryptjs)
alt success
Backend-->>AuthService: AccessTokenDto or UserResponseDto
AuthService-->>Frontend: Observable with response
Frontend->>Frontend: Store token / navigate
Frontend->>User: Show success
else error
Backend-->>AuthService: Error response
AuthService-->>Frontend: Error Observable
Frontend->>User: Display API error message
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Poem
🚥 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)
No actionable comments were generated in the recent review. 🎉 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 |
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (9)
apps/frontend/src/app/app.config.ts (1)
9-9: LGTM! HttpClient provider correctly configured.The addition of
provideHttpClient()is necessary for the AuthService to make HTTP requests to the backend API. This follows the modern Angular standalone API pattern.Optional: Consider HTTP interceptor for JWT token management.
While the basic HTTP client configuration is complete, authentication flows typically benefit from an HTTP interceptor to automatically attach JWT tokens to outgoing requests. If not already implemented elsewhere, consider adding:
provideHttpClient( withInterceptors([authTokenInterceptor]) )This would centralize token management rather than handling it in each service method. However, if this is planned for future work or handled differently, feel free to disregard.
Also applies to: 15-16
apps/frontend/package.json (1)
6-6: The--openflag is a convenience, but consider team preference.Automatically opening the browser on
npm startcan be helpful during development, but some developers prefer to control when the browser opens manually. Consider whether this aligns with your team's workflow preferences.apps/backend/src/auth/auth.service.ts (1)
48-52: Consider using class-transformer for DTO mapping.While the manual object construction is correct, you could leverage the
plainToClassutility fromclass-transformer(already in dependencies) for more maintainable DTO mapping:import { plainToClass } from 'class-transformer'; const response = plainToClass(UserResponseDto, savedUser, { excludeExtraneousValues: true });This provides better type safety and makes it easier to add transformations or exclusions later.
apps/frontend/src/app/app.routes.ts (1)
3-8: LGTM: Proper lazy loading implementation.The lazy-loaded route configuration follows Angular best practices and will improve initial bundle size.
Consider adding a default route and wildcard route for better user experience:
export const routes: Routes = [ { path: '', redirectTo: '/auth/login', pathMatch: 'full' }, { path: 'auth', loadChildren: () => import('./auth/auth-module').then((m) => m.AuthModule), }, { path: '**', redirectTo: '/auth/login' } ];apps/frontend/src/app/auth/components/login/login.ts (1)
49-49: Remove console.log from production code.The console.log statement should be removed or replaced with a proper logging service for production environments.
🔎 Proposed fix
this.errorMessage = null; localStorage.setItem('access_token', response.access_token); - console.log('Login successful!', response); },apps/backend/package.json (1)
29-29: bcryptjs 3.0.3 is the latest stable version; no critical vulnerabilities in the package itself.Version 3.0.3 (released November 2, 2025) is confirmed as the latest stable release. The package has no known direct security vulnerabilities. However, note that bcryptjs follows the bcrypt specification and truncates passwords longer than 72 bytes—ensure your application validates or enforces a maximum password length before hashing to avoid security assumptions.
Regarding the linting pipeline failure mentioned: no evidence of actual linting errors was provided. Verify the pipeline output and address any specific linting issues if they exist.
apps/frontend/src/app/auth/components/register/register.spec.ts (1)
63-78: Add assertion to verify service is not called on invalid form.The test should also verify that
authService.registerwas not called when the form is invalid.🔎 Proposed enhancement
// Assert expect(component.registerForm.invalid).toBeTrue(); expect(component.registerForm.markAllAsTouched).toHaveBeenCalled(); + expect(authService.register).not.toHaveBeenCalled(); });apps/frontend/src/app/auth/components/login/login.spec.ts (1)
63-77: Add assertion to verify service is not called on invalid form.Similar to the register test, this should also verify that
authService.loginwas not called when the form is invalid.🔎 Proposed enhancement
// Assert expect(component.loginForm.invalid).toBeTrue(); expect(component.loginForm.markAllAsTouched).toHaveBeenCalled(); + expect(authService.login).not.toHaveBeenCalled(); });apps/frontend/src/app/auth/components/register/register.ts (1)
50-54: Consider removing console.log and adding navigation.The
console.logstatement should be removed or replaced with proper logging for production. Additionally, consider whether successful registration should navigate the user (e.g., to login page or auto-login and redirect to dashboard).🔎 Suggested improvements
next: (response) => { this.errorMessage = null; - console.log('Registration successful!', response); + // TODO: Navigate to login page or dashboard after successful registration },
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (38)
.github/workflows/lint.ymlapps/backend/eslint.config.mjsapps/backend/nest-cli.jsonapps/backend/package.jsonapps/backend/src/auth/auth.controller.spec.tsapps/backend/src/auth/auth.controller.tsapps/backend/src/auth/auth.service.spec.tsapps/backend/src/auth/auth.service.tsapps/backend/src/auth/dto/access-token.dto.tsapps/backend/src/auth/dto/register-user.dto.tsapps/backend/src/main.tsapps/backend/tsconfig.jsonapps/frontend/eslint.config.jsapps/frontend/package.jsonapps/frontend/src/app/app.config.tsapps/frontend/src/app/app.htmlapps/frontend/src/app/app.routes.tsapps/frontend/src/app/app.spec.tsapps/frontend/src/app/app.tsapps/frontend/src/app/auth/auth-module.tsapps/frontend/src/app/auth/auth-routing-module.tsapps/frontend/src/app/auth/components/login/login.cssapps/frontend/src/app/auth/components/login/login.htmlapps/frontend/src/app/auth/components/login/login.spec.tsapps/frontend/src/app/auth/components/login/login.tsapps/frontend/src/app/auth/components/register/register.cssapps/frontend/src/app/auth/components/register/register.htmlapps/frontend/src/app/auth/components/register/register.spec.tsapps/frontend/src/app/auth/components/register/register.tsapps/frontend/src/app/auth/services/auth.spec.tsapps/frontend/src/app/auth/services/auth.tsapps/frontend/tsconfig.jsonlibs/shared-dtos/src/auth/access-token.dto.tslibs/shared-dtos/src/auth/login-user.dto.tslibs/shared-dtos/src/auth/register-user.dto.tslibs/shared-dtos/src/user/user-response.dto.tslibs/shared-validation/src/email.constants.tslibs/shared-validation/src/password.constants.ts
💤 Files with no reviewable changes (3)
- apps/backend/src/auth/dto/access-token.dto.ts
- apps/backend/src/auth/dto/register-user.dto.ts
- apps/frontend/src/app/app.html
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-19T16:44:42.665Z
Learnt from: Zafar7645
Repo: Zafar7645/syncup PR: 12
File: apps/backend/src/users/users.service.spec.ts:15-17
Timestamp: 2025-10-19T16:44:42.665Z
Learning: In the syncup repository (apps/backend/src/users/users.service.ts), the UsersService is scaffolding for future implementation and currently contains no methods or business logic. Basic "should be defined" smoke tests are sufficient for empty scaffolding services.
Applied to files:
apps/frontend/src/app/auth/services/auth.tsapps/backend/src/auth/auth.service.tsapps/backend/src/auth/auth.service.spec.tsapps/backend/src/auth/auth.controller.spec.tsapps/frontend/src/app/auth/services/auth.spec.ts
🧬 Code graph analysis (9)
apps/frontend/src/app/auth/services/auth.ts (4)
libs/shared-dtos/src/auth/register-user.dto.ts (1)
RegisterUserDto(7-21)libs/shared-dtos/src/user/user-response.dto.ts (1)
UserResponseDto(5-9)libs/shared-dtos/src/auth/login-user.dto.ts (1)
LoginUserDto(3-13)libs/shared-dtos/src/auth/access-token.dto.ts (1)
AccessTokenDto(1-3)
apps/frontend/src/app/auth/components/register/register.ts (4)
apps/frontend/src/app/auth/components/login/login.ts (1)
Component(12-62)libs/shared-validation/src/email.constants.ts (1)
EMAIL_REGEX_STRING(1-2)libs/shared-validation/src/password.constants.ts (2)
PASSWORD_REGEX_STRING(1-2)PASSWORD_VALIDATION_MESSAGE(4-5)libs/shared-dtos/src/auth/register-user.dto.ts (1)
RegisterUserDto(7-21)
libs/shared-dtos/src/auth/register-user.dto.ts (1)
libs/shared-validation/src/password.constants.ts (2)
PASSWORD_REGEX_STRING(1-2)PASSWORD_VALIDATION_MESSAGE(4-5)
apps/frontend/src/app/auth/auth-routing-module.ts (2)
apps/frontend/src/app/app.routes.ts (1)
routes(3-8)apps/frontend/src/app/auth/auth-module.ts (1)
NgModule(7-11)
apps/frontend/src/app/auth/auth-module.ts (1)
apps/frontend/src/app/auth/auth-routing-module.ts (1)
NgModule(22-26)
apps/frontend/src/app/auth/components/login/login.spec.ts (1)
libs/shared-dtos/src/auth/access-token.dto.ts (1)
AccessTokenDto(1-3)
apps/frontend/src/app/auth/components/register/register.spec.ts (1)
libs/shared-dtos/src/user/user-response.dto.ts (1)
UserResponseDto(5-9)
apps/frontend/src/app/app.config.ts (1)
apps/frontend/src/app/app.routes.ts (1)
routes(3-8)
apps/frontend/src/app/auth/services/auth.spec.ts (4)
libs/shared-dtos/src/auth/register-user.dto.ts (1)
RegisterUserDto(7-21)libs/shared-dtos/src/user/user-response.dto.ts (1)
UserResponseDto(5-9)libs/shared-dtos/src/auth/login-user.dto.ts (1)
LoginUserDto(3-13)libs/shared-dtos/src/auth/access-token.dto.ts (1)
AccessTokenDto(1-3)
🪛 ast-grep (0.40.3)
apps/frontend/src/app/auth/components/register/register.ts
[warning] 23-23: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(EMAIL_REGEX_STRING)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 24-24: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(PASSWORD_REGEX_STRING)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
libs/shared-dtos/src/auth/register-user.dto.ts
[warning] 16-16: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(PASSWORD_REGEX_STRING)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
apps/frontend/src/app/auth/components/login/login.ts
[warning] 22-22: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(EMAIL_REGEX_STRING)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 23-23: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(PASSWORD_REGEX_STRING)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 47-47: Sensitive information detected in localStorage. Storing sensitive data like emails, usernames, or personal information in localStorage exposes it to malicious scripts and XSS attacks. Use secure storage alternatives or avoid storing sensitive data client-side.
Context: localStorage.setItem('access_token', response.access_token)
Note: [CWE-312] Cleartext Storage of Sensitive Information [REFERENCES]
- https://cwe.mitre.org/data/definitions/312.html
- https://owasp.org/www-community/vulnerabilities/HTML5_Local_Storage_Security
(local-storage-sensitive-information)
🪛 GitHub Actions: Linting Check
apps/backend/src/auth/auth.service.ts
[error] 1-1: Lint step failed. Command 'eslint "{src,apps,libs,test}/**/*.ts" --fix' exited with code 1.
apps/backend/eslint.config.mjs
[error] 1-1: Lint step failed. Command 'eslint "{src,apps,libs,test}/**/*.ts" --fix' exited with code 1.
apps/backend/src/auth/auth.service.spec.ts
[error] 1-1: Lint step failed. Command 'eslint "{src,apps,libs,test}/**/*.ts" --fix' exited with code 1.
apps/backend/package.json
[error] 1-1: Lint step failed. Command 'eslint "{src,apps,libs,test}/**/*.ts" --fix' exited with code 1.
apps/backend/src/auth/auth.controller.spec.ts
[error] 1-1: Lint step failed. Command 'eslint "{src,apps,libs,test}/**/*.ts" --fix' exited with code 1.
apps/backend/src/main.ts
[error] 1-1: Lint step failed. Command 'eslint "{src,apps,libs,test}/**/*.ts" --fix' exited with code 1.
apps/backend/src/auth/auth.controller.ts
[error] 1-1: Lint step failed. Command 'eslint "{src,apps,libs,test}/**/*.ts" --fix' exited with code 1.
apps/backend/tsconfig.json
[error] 1-1: Lint step failed. Command 'eslint "{src,apps,libs,test}/**/*.ts" --fix' exited with code 1.
apps/backend/nest-cli.json
[error] 1-1: Lint step failed. Command 'eslint "{src,apps,libs,test}/**/*.ts" --fix' exited with code 1.
🪛 GitHub Check: CodeQL
libs/shared-validation/src/email.constants.ts
[failure] 2-2: Useless regular-expression character escape
The escape sequence '.' is equivalent to just '.', so the sequence may still represent a meta-character when it is used in a regular expression.
The escape sequence '.' is equivalent to just '.', so the sequence may still represent a meta-character when it is used in a regular expression.
🔇 Additional comments (32)
.github/workflows/lint.yml (1)
3-4: Security improvement: Explicit minimal permissions correctly applied.Adding explicit
permissionswithcontents: readis a best practice that reduces the attack surface by adhering to the principle of least privilege. This is appropriate for a read-only linting workflow.apps/backend/src/main.ts (1)
1-1: Install ESLint dependencies to resolve the module resolution error.The pipeline is failing because ESLint dependencies are missing, not due to code linting issues. The eslint.config.mjs requires
@eslint/jsand other packages that are not found. Runnpm installto install all dependencies before attempting to lint.Likely an incorrect or invalid review comment.
apps/frontend/src/app/app.spec.ts (2)
2-2: LGTM: Path alias usage is correct.The import now uses the
@/path alias configured intsconfig.json, promoting consistent imports across the frontend codebase.
11-15: LGTM: Test suite correctly reduced to match component changes.The removal of the title rendering test aligns with the simplification of the App component (removal of the title signal property).
apps/frontend/tsconfig.json (1)
17-22: LGTM: Path aliases correctly configured for shared libraries.The baseUrl and paths configuration properly enables imports from shared validation and DTO libraries, supporting the monorepo structure.
apps/backend/tsconfig.json (2)
18-21: LGTM: Path aliases correctly mirror frontend configuration.The shared library aliases are properly configured for consistent cross-project imports.
32-32: [Your rewritten review comment text here]
[Exactly ONE classification tag]apps/backend/nest-cli.json (1)
6-6: LGTM: Webpack builder enables path alias resolution.Adding the webpack builder is the correct approach for handling TypeScript path aliases (e.g.,
@shared/*) in NestJS builds.apps/frontend/src/app/app.ts (1)
1-10: LGTM: App component correctly simplified to routing shell.Removing the title signal and simplifying the component to an empty class aligns with the architectural shift toward lazy-loaded feature modules (e.g., auth module) and routing-driven content.
apps/frontend/eslint.config.js (1)
34-39: LGTM: TypeScript project references correctly configured for type-aware linting.The
parserOptionsproperly reference both app and spec TypeScript configurations, enabling type-aware ESLint rules across the frontend codebase.apps/backend/eslint.config.mjs (1)
23-23: Using standard project-based TypeScript discovery with proper configuration.The switch from
projectServicetoproject: truecorrectly aligns with typescript-eslint 8.x's recommended approach. The configuration properly pairsproject: truewithtsconfigRootDir: import.meta.dirname, and thetsconfig.jsonincludes all required paths (@/*,@shared/*) to support bothsrcand shared library files. This setup is compatible with the import resolver configuration at line 36. The ESLint pipeline integration is properly configured for type-checking against the project.libs/shared-dtos/src/auth/login-user.dto.ts (1)
7-7: LGTM: Definite assignment assertions improve type safety.The definite assignment assertions are appropriate here since the class-validator decorators (
@IsNotEmpty,@IsEmail,@IsString) guarantee these properties will be populated during validation.Also applies to: 12-12
libs/shared-dtos/src/user/user-response.dto.ts (1)
6-8: Excellent:readonlyenforces immutability for response data.Using
readonlyproperties for response DTOs prevents accidental mutation and clearly signals these are read-only data structures returned from the API.apps/backend/src/auth/auth.service.spec.ts (3)
4-4: LGTM: Shared DTO imports align with monorepo architecture.The migration to
@shared/dtos/*path aliases correctly centralizes DTOs for reuse across frontend and backend.Also applies to: 11-12
89-283: Well-structured tests with comprehensive coverage.The test suite effectively covers:
- Happy path registration with transaction handling
- Duplicate email conflict detection
- Generic error rollback
- Salt rounds configuration edge cases
1-1: No linting error detected on this import statement.The
TestingModuleandTestsymbols imported on line 1 are both actively used in the file (e.g., line 54:const module: TestingModule = await Test.createTestingModule(...)). The import statement complies with the ESLint configuration and should not block the pipeline.apps/backend/src/auth/auth.service.ts (1)
6-7: LGTM: Consistent migration to shared DTOs and bcryptjs.The import updates correctly reference shared DTOs and align with the bcryptjs migration tested in the spec file.
Also applies to: 10-10, 14-15
libs/shared-dtos/src/auth/access-token.dto.ts (1)
1-3: LGTM: Simple and appropriate token response DTO.The
readonlymodifier correctly prevents mutation, and theaccess_tokenproperty name follows standard JWT response conventions.apps/backend/src/auth/auth.controller.ts (1)
2-2: LGTM: Import updates align with shared DTO architecture.The controller correctly references shared DTOs via the
@shared/dtos/*path aliases.Also applies to: 4-6
apps/frontend/src/app/auth/auth-module.ts (1)
7-10: The emptydeclarationsarray is intentional and correct.The
LoginandRegistercomponents are standalone (indicated by their@Componentdecorators with theimportsarray). Standalone components should not be declared in theNgModule'sdeclarationsarray. Instead, they are imported directly in theAuthRoutingModuleand used in its route definitions, which is the proper pattern for standalone components in Angular. No changes needed.libs/shared-validation/src/password.constants.ts (1)
1-5: LGTM!The password validation constants are well-defined and provide clear, reusable validation rules across the codebase. The regex pattern correctly enforces strong password requirements, and the accompanying message clearly communicates these requirements to users.
apps/frontend/src/app/auth/auth-routing-module.ts (1)
1-26: LGTM!The routing module is correctly implemented with appropriate routes for login and registration, and a sensible default redirect to the login page.
apps/frontend/src/app/auth/components/register/register.html (1)
1-35: LGTM!The registration form template is well-structured with clear validation feedback and appropriate form controls. The error messages provide helpful guidance to users.
apps/frontend/src/app/auth/components/login/login.ts (1)
48-48: Note: JWT stored in localStorage per acceptance criteria.While the acceptance criteria specify storing the JWT in localStorage, be aware this exposes the token to XSS attacks. Consider implementing httpOnly cookies or more secure storage mechanisms in the future for production deployments.
apps/backend/package.json (1)
84-86: LGTM!The Jest moduleNameMapper configuration correctly maps the shared library paths, enabling tests to resolve imports from
@shared/validationand@shared/dtos.apps/frontend/src/app/auth/components/register/register.spec.ts (1)
1-60: LGTM! Comprehensive test setup and validation coverage.The test setup properly mocks the Auth service, and the validation tests thoroughly cover required fields, email pattern, and password pattern validation with both valid and invalid cases.
apps/frontend/src/app/auth/components/login/login.spec.ts (1)
1-60: LGTM! Well-structured test setup and validation coverage.The test configuration and validation tests properly cover all required scenarios for the Login component.
apps/frontend/src/app/auth/components/register/register.ts (3)
19-27: LGTM! Proper use of shared validation constants.The form setup correctly uses shared validation constants for email and password patterns. The static analysis ReDoS warnings can be safely ignored as these regex patterns are constants (not user input) and are relatively simple.
31-41: LGTM! Standard form control getters.The getters provide convenient access to form controls for validation feedback in the template.
55-64: LGTM! Proper error handling with fallback.The error handling correctly extracts the server error message (handling both string and array formats) and provides a sensible default message when no server error is available.
libs/shared-dtos/src/auth/register-user.dto.ts (1)
1-21: LGTM! Well-structured DTO with proper validation.The RegisterUserDto correctly uses class-validator decorators and shared validation constants to ensure consistent validation rules across frontend and backend. The static analysis ReDoS warning can be safely ignored as PASSWORD_REGEX_STRING is a constant with a simple, safe pattern.
apps/backend/src/auth/auth.controller.spec.ts (1)
4-7: [rewritten comment]
[classification tag]
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/frontend/package.json`:
- Around line 32-33: Remove the unused runtime dependencies "class-transformer"
and "class-validator" from the frontend package.json: locate the dependencies
block where "class-transformer" and "class-validator" are declared and delete
those entries, then run the package manager (npm/yarn/pnpm) to update the
lockfile and node_modules; ensure they remain declared where needed (e.g.,
backend package.json) and run a full frontend build/test to confirm no runtime
imports like plainToInstance(), validate(), or similar are used in the frontend
codebase.
In `@apps/frontend/src/app/auth/components/login/login.ts`:
- Around line 46-49: The success handler in the login component currently writes
the JWT to localStorage and emits a console.log, and it doesn't navigate to the
dashboard; update the next callback used in the login Observable to (1) remove
the console.log('Login successful!', response), (2) avoid direct localStorage
usage by delegating token persistence to an abstraction (e.g., call a new or
existing method like AuthStorageService.setToken/this.authService.saveToken
instead of localStorage.setItem('access_token', ...)), and (3) after saving the
token call the router navigation to the main dashboard (e.g.,
this.router.navigate(['/dashboard'])) so the user is redirected on success;
modify or add the storage abstraction so it can later be swapped for
httpOnly-cookie behavior without changing the login handler.
🧹 Nitpick comments (3)
apps/frontend/angular.json (1)
29-34: Non-standardfileReplacementsdirection.The conventional Angular pattern is for source code to import from a generic
environment.ts, which then gets replaced withenvironment.prod.ts(or similar) in production builds. Here the replacement goes the opposite way — source importsenvironment.development.tsand production swaps it withenvironment.ts. This works but may confuse developers familiar with the standard Angular convention.Consider renaming to follow the typical pattern:
environment.ts→ imported by app code (contains dev defaults)environment.prod.ts→ replacesenvironment.tsin productionapps/frontend/src/app/auth/components/login/login.spec.ts (2)
12-12: Spy created atdescribescope — call history leaks between tests.
authServiceSpyObjis instantiated once and reused across allitblocks. Jasmine spy call trackers accumulate, so a test that assertstoHaveBeenCalledWithcould pass or fail depending on execution order. Move creation intobeforeEach, or addauthService.login.calls.reset()inbeforeEach.Proposed fix (reset in beforeEach)
beforeEach(async () => { + authServiceSpyObj.login.calls.reset(); await TestBed.configureTestingModule({
79-97: Success test doesn't verify token storage.The component's key side-effect on success is
localStorage.setItem('access_token', …). This test assertserrorMessageis null but never checks that the token was actually stored. Adding alocalStoragespy would make this test meaningful for the happy path.Suggested addition
+ spyOn(localStorage, 'setItem'); + // Act component.onSubmit(); // Assert expect(authService.login).toHaveBeenCalledWith(mockUserData); expect(component.errorMessage).toBe(null); + expect(localStorage.setItem).toHaveBeenCalledWith('access_token', 'access_token');
This PR implements the frontend authentication UI and connects it to the NestJS backend, completing the requirements for Issue #3.
Key Changes:
Closes: #3
Summary by CodeRabbit
New Features
Bug Fixes
Chores