-
Notifications
You must be signed in to change notification settings - Fork 0
[TM-2768] add site polygon bulk status update #464
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: staging
Are you sure you want to change the base?
[TM-2768] add site polygon bulk status update #464
Conversation
| @IsOptional() | ||
| @IsString() | ||
| @ApiProperty({ description: "Comment for the status update", required: false, type: String }) | ||
| comment: string | null = null; |
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.
For required: false, this should be comment?: string. If you really want to explicitly accept null, the ApiProperty should have nullable: true.
| @ApiOperation({ | ||
| operationId: "updateSitePolygonStatus", | ||
| summary: "Update the status of a site polygon", | ||
| description: "Update the status of a site polygon" |
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.
These descriptions make it sound like this endpoint operates on a single polygon.
| @JsonApiResponse({ data: SitePolygonLightDto, hasMany: true }) | ||
| @ExceptionResponse(UnauthorizedException, { description: "Authentication failed." }) | ||
| @ExceptionResponse(BadRequestException, { description: "Invalid request data." }) | ||
| @ExceptionResponse(NotFoundException, { description: "Site polygon not found." }) |
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.
This should also be something more like "At least one of the site polygons was not found".
| const userId = this.policyService.userId; | ||
| if (userId == null) { | ||
| throw new UnauthorizedException("User must be authenticated"); | ||
| } |
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.
For a normal endpoint (one that doesn't opt out of auth), the user id is guaranteed to be non null, so instead of this check, you can do:
const userId = this.policyService.userId as number;
Task Link