Conversation
hoan301298
left a comment
There was a problem hiding this comment.
This PR needs to be fixed and should cover all of following rules.
…n-information-about-the-daily-emotions-for-the-past-week
hoan301298
left a comment
There was a problem hiding this comment.
There are a few issues needed to fix in this PR. But in overall, it looks good to me. Approved!
src/player/player.service.ts
Outdated
| * @returns - A classic tuple setup [boolean, ServiceError[]] indicating if an entry for today exists. | ||
| */ | ||
| async checkIfEmotionSentToday(playerId: string): Promise<IServiceReturn<EmotionCheckDto>> { | ||
| const player = await this.model |
There was a problem hiding this comment.
I suggest, it's better if you can use BasicService functions.
So maybe this function should be return <IServiceReturn<boolean>>.
There was a problem hiding this comment.
Addressed in my latest commit.
| @Post('/emotion') | ||
| @UniformResponse(ModelName.PLAYER, PlayerDto) | ||
| @Authorize({ action: Action.create, subject: PlayerDto }) | ||
| public async setDailyEmotion( |
There was a problem hiding this comment.
This controller function should return nothing, since you return 201 status_code (means No Content).
There was a problem hiding this comment.
Addressed in my latest commit.
|
@hoan301298 or @Iir0R you can check and approve once more before I hit merge on this, thanks! |
Brief description
The UI Team lead Niko opened an issue last year regarding adding emotions to the backend API. This has now been implemented, unit tested and tested on Postman as well on this PR.
Change list
Added
PlayerEmotionenum and updatedPlayerSchemawith a history array.Implemented
GET /player/emotioncheckto let the client know if the daily popup is needed.Implemented
POST /player/emotionto record daily sentiment (limited to once per day).Fix: Resolved path alias issues in tests by switching to relative imports in the
Schema.Fix: Resolved controller routing collision by reordering static routes above dynamic parameters.
Fix: Aligned controller with
req.user.player_idto ensure correct database targeting.