-
Notifications
You must be signed in to change notification settings - Fork 0
User module docs and logs #267
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
76955e2
0d37649
3932439
11b5bdc
aecad73
34e75b6
a1d6804
d93b28b
29c297a
20f1fc0
c0cc8e6
657accb
e689fe9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,11 @@ | ||||||||||||||||||||||||||||||||||||||||||
| import { ApiProperty } from '@nestjs/swagger'; | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
| import { ApiProperty } from '@nestjs/swagger'; |
Copilot
AI
Jan 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing @ApiProperty decorators on the class properties. While the ApiProperty import is present, the properties lack decorators, which means they won't appear in the Swagger documentation with proper descriptions and types. Each property should have an @ApiProperty decorator to document the expected structure in Swagger UI.
| user!: { | |
| userId: string, | |
| position: UserStatus, | |
| email: string | |
| }; | |
| @ApiProperty({ | |
| type: 'object', | |
| properties: { | |
| userId: { type: 'string' }, | |
| position: { enum: Object.values(UserStatus) }, | |
| email: { type: 'string', format: 'email' }, | |
| }, | |
| }) | |
| user!: { | |
| userId: string, | |
| position: UserStatus, | |
| email: string | |
| }; | |
| @ApiProperty({ enum: UserStatus }) |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,60 +1,202 @@ | ||||||
| import { Controller, Get, Post, Body, Param, UseGuards } from "@nestjs/common"; | ||||||
| import { Controller, Get, Patch, Delete, Body, Param, UseGuards, Req } from "@nestjs/common"; | ||||||
| import { UserService } from "./user.service"; | ||||||
| import { User } from "../../../middle-layer/types/User"; | ||||||
| import { UserStatus } from "../../../middle-layer/types/UserStatus"; | ||||||
| import { VerifyAdminRoleGuard, VerifyUserGuard } from "../guards/auth.guard"; | ||||||
| import { VerifyAdminRoleGuard, VerifyUserGuard, VerifyAdminOrEmployeeRoleGuard } from "../guards/auth.guard"; | ||||||
|
||||||
| import { VerifyAdminRoleGuard, VerifyUserGuard, VerifyAdminOrEmployeeRoleGuard } from "../guards/auth.guard"; | |
| import { VerifyAdminRoleGuard, VerifyAdminOrEmployeeRoleGuard } from "../guards/auth.guard"; |
Copilot
AI
Jan 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent spacing in ApiParam decorator. There's a space after the comma between properties (e.g., "ApiParam , ApiBearerAuth"), which is inconsistent with the rest of the codebase and standard formatting conventions.
| import { ApiResponse, ApiParam , ApiBearerAuth} from "@nestjs/swagger"; | |
| import { ApiResponse, ApiParam, ApiBearerAuth } from "@nestjs/swagger"; |
Copilot
AI
Jan 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Breaking API change: Changed from POST to PATCH and modified request signature. The frontend code still uses POST method with the old signature that includes requestedBy in the body (see ApprovedUserCard.tsx:38-53 and PendingUserCard.tsx:33-44). This will break existing functionality until the frontend is updated to use PATCH and remove requestedBy from the request body.
Copilot
AI
Jan 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Breaking API change: Changed from POST to DELETE and modified from body parameters to URL parameter. The frontend code still uses POST method with a request body containing user and requestedBy (see ApprovedUserCard.tsx:81-92 and PendingUserCard.tsx:64-75). This will break existing functionality until the frontend is updated to use DELETE with the userId as a URL parameter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new VerifyAdminOrEmployeeRoleGuard doesn't attach user information to the request object like VerifyAdminRoleGuard does (lines 77-82). While this guard is not currently used in endpoints that need req.user, it's inconsistent with the other guard and could lead to bugs if it's used in endpoints that expect req.user to be populated.