Skip to content

PM-1504 create edit scorecard #20

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

Merged
merged 3 commits into from
Aug 20, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 16 additions & 6 deletions src/api/scorecard/scorecard.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
Param,
Query,
UseInterceptors,
ValidationPipe,
} from '@nestjs/common';
import {
ApiTags,
Expand All @@ -32,6 +33,8 @@ import { ChallengeTrack } from 'src/shared/enums/challengeTrack.enum';
import { ScoreCardService } from './scorecard.service';
import { PaginationHeaderInterceptor } from 'src/interceptors/PaginationHeaderInterceptor';
import { $Enums } from '@prisma/client';
import { User } from 'src/shared/decorators/user.decorator';
import { JwtUser } from 'src/shared/modules/global/jwt.service';

@ApiTags('Scorecard')
@ApiBearerAuth()
Expand All @@ -54,9 +57,11 @@ export class ScorecardController {
})
@ApiResponse({ status: 403, description: 'Forbidden.' })
async addScorecard(
@Body() body: ScorecardRequestDto,
@Body(new ValidationPipe({ whitelist: true, transform: true }))
body: ScorecardRequestDto,
@User() user: JwtUser,
): Promise<ScorecardWithGroupResponseDto> {
return await this.scorecardService.addScorecard(body);
return await this.scorecardService.addScorecard(body, user);
}

@Put('/:id')
Expand All @@ -81,9 +86,11 @@ export class ScorecardController {
@ApiResponse({ status: 404, description: 'Scorecard not found.' })
async editScorecard(
@Param('id') id: string,
@Body() body: ScorecardWithGroupResponseDto,
@Body(new ValidationPipe({ whitelist: true, transform: true }))
body: ScorecardRequestDto,
@User() user: JwtUser,
): Promise<ScorecardWithGroupResponseDto> {
return await this.scorecardService.editScorecard(id, body);
return await this.scorecardService.editScorecard(id, body, user);
}

@Delete(':id')
Expand Down Expand Up @@ -248,7 +255,10 @@ export class ScorecardController {
})
@ApiResponse({ status: 403, description: 'Forbidden.' })
@ApiResponse({ status: 404, description: 'Scorecard not found.' })
async cloneScorecard(@Param('id') id: string): Promise<ScorecardResponseDto> {
return this.scorecardService.cloneScorecard(id);
async cloneScorecard(
@Param('id') id: string,
@User() user: JwtUser,
): Promise<ScorecardResponseDto> {
return this.scorecardService.cloneScorecard(id, user);
}
}
178 changes: 100 additions & 78 deletions src/api/scorecard/scorecard.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,18 @@ import {
} from '@nestjs/common';
import { Prisma } from '@prisma/client';
import {
mapScorecardRequestForCreate,
mapScorecardRequestToDto,
// ScorecardGroupBaseDto,
ScorecardGroupBaseDto,
ScorecardPaginatedResponseDto,
ScorecardQueryDto,
// ScorecardQuestionBaseDto,
ScorecardQuestionBaseDto,
ScorecardRequestDto,
ScorecardResponseDto,
// ScorecardSectionBaseDto,
ScorecardSectionBaseDto,
ScorecardWithGroupResponseDto,
} from 'src/dto/scorecard.dto';
import { JwtUser } from 'src/shared/modules/global/jwt.service';
import { PrismaService } from 'src/shared/modules/global/prisma.service';

@Injectable()
Expand All @@ -28,9 +30,16 @@ export class ScoreCardService {
*/
async addScorecard(
body: ScorecardRequestDto,
user: JwtUser,
): Promise<ScorecardWithGroupResponseDto> {
const data = await this.prisma.scorecard.create({
data: mapScorecardRequestToDto(body),
data: {
...(mapScorecardRequestForCreate({
...body,
createdBy: user.isMachine ? 'System' : (user.userId as string),

Choose a reason for hiding this comment

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

Consider using a type guard or type assertion to ensure user.userId is of type string before casting, as casting without checking can lead to runtime errors if userId is not a string.

updatedBy: user.isMachine ? 'System' : (user.userId as string),

Choose a reason for hiding this comment

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

Similar to the previous comment, ensure user.userId is of type string before casting to avoid potential runtime errors.

}) as any),
},
include: {
scorecardGroups: {
include: {
Expand All @@ -44,7 +53,7 @@ export class ScoreCardService {
},
});

return data as ScorecardWithGroupResponseDto;
return data as unknown as ScorecardWithGroupResponseDto;

Choose a reason for hiding this comment

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

Casting data as unknown before casting to ScorecardWithGroupResponseDto is generally unnecessary unless there is a specific reason to bypass TypeScript's type checking. Consider directly casting data to ScorecardWithGroupResponseDto if possible, or provide a rationale for this approach in the documentation or code comments.

}

/**
Expand All @@ -54,12 +63,17 @@ export class ScoreCardService {
*/
async editScorecard(
id: string,
body: ScorecardWithGroupResponseDto,
body: ScorecardRequestDto,
user: JwtUser,
): Promise<ScorecardWithGroupResponseDto> {
const data = await this.prisma.scorecard
.update({
where: { id },
data: mapScorecardRequestToDto(body),
data: mapScorecardRequestToDto({

Choose a reason for hiding this comment

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

The type assertion as any is used here. Consider refining the types to avoid using any, which can lead to runtime errors and makes the code less type-safe. Ensure that mapScorecardRequestToDto returns the correct type.

...body,
createdBy: user.isMachine ? 'System' : (user.userId as string),
updatedBy: user.isMachine ? 'System' : (user.userId as string),
}) as any,
include: {
scorecardGroups: {
include: {
Expand All @@ -81,7 +95,7 @@ export class ScoreCardService {
});
});

return data as ScorecardWithGroupResponseDto;
return data as unknown as ScorecardWithGroupResponseDto;
}

/**
Expand Down Expand Up @@ -202,79 +216,87 @@ export class ScoreCardService {
};
}

// eslint-disable-next-line @typescript-eslint/no-unused-vars, @typescript-eslint/require-await
async cloneScorecard(id: string): Promise<ScorecardResponseDto> {
// const original = await this.prisma.scorecard.findUnique({
// where: { id },
// include: {
// scorecardGroups: {
// include: {
// sections: {
// include: {
// questions: true,
// },
// },
// },
// },
// },
// });
async cloneScorecard(
id: string,
user: { userId?: string; isMachine: boolean },

Choose a reason for hiding this comment

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

The user parameter is destructured to access userId and isMachine, but userId is marked as optional. Consider handling the case where userId might be undefined to avoid potential runtime errors.

): Promise<ScorecardResponseDto> {
const original = await this.prisma.scorecard.findUnique({
where: { id },
include: {
scorecardGroups: {
include: {
sections: {
include: {
questions: true,
},
},
},
},
},
});

// if (!original) {
// throw new NotFoundException({ message: `Scorecard not found.` });
// }
if (!original) {
throw new NotFoundException({ message: `Scorecard not found.` });
}

// // Remove id fields from nested objects for cloning
// const cloneGroups = original.scorecardGroups.map(
// (group: ScorecardGroupBaseDto) => ({
// ...group,
// id: undefined,
// createdAt: undefined,
// updatedAt: undefined,
// scorecardId: undefined,
// sections: group.sections.map((section: ScorecardSectionBaseDto) => ({
// ...section,
// id: undefined,
// createdAt: undefined,
// updatedAt: undefined,
// scorecardGroupId: undefined,
// questions: section.questions.map(
// (question: ScorecardQuestionBaseDto) => ({
// ...question,
// id: undefined,
// createdAt: undefined,
// updatedAt: undefined,
// sectionId: undefined,
// scorecardSectionId: undefined,
// }),
// ),
// })),
// }),
// );
const auditFields = {
createdBy: user.isMachine ? 'System' : (user.userId as string),
updatedBy: user.isMachine ? 'System' : (user.userId as string),
createdAt: undefined,
updatedAt: undefined,
};

// const clonedScorecard = await this.prisma.scorecard.create({
// data: {
// ...original,
// id: undefined,
// name: `${original.name} (Clone)`,
// createdAt: undefined,
// updatedAt: undefined,
// scorecardGroups: {
// create: cloneGroups,
// },
// },
// include: {
// scorecardGroups: {
// include: {
// sections: {
// include: {
// questions: true,
// },
// },
// },
// },
// },
// });
const clonedScorecard = {};
// Remove id fields from nested objects for cloning
const cloneGroups = original.scorecardGroups.map(
(group: ScorecardGroupBaseDto) => ({
...group,
id: undefined,
...auditFields,
scorecardId: undefined,
sections: {
create: group.sections.map((section: ScorecardSectionBaseDto) => ({
...section,
id: undefined,
...auditFields,
scorecardGroupId: undefined,
questions: {
create: section.questions.map(
(question: ScorecardQuestionBaseDto) => ({
...question,
id: undefined,
...auditFields,
sectionId: undefined,
scorecardSectionId: undefined,
}),
),
},
})),
},
}),
) as any;

Choose a reason for hiding this comment

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

The as any type assertion is used here. Consider refining the types to avoid using any, which can lead to less type safety and potential runtime errors.


const clonedScorecard = await this.prisma.scorecard.create({
data: {
...original,
id: undefined,
name: `${original.name} (Clone)`,
...auditFields,
scorecardGroups: {
create: cloneGroups,
},
},
include: {
scorecardGroups: {
include: {
sections: {
include: {
questions: true,
},
},
},
},
},
});

return clonedScorecard as ScorecardResponseDto;
}
Expand Down
Loading