feat(be): implement admin announcement module #1270
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
backend/apps/admin/src/announcement/announcement.service.spec.ts
Outdated
Show resolved
Hide resolved
Lee-won-hyeok
force-pushed
the
1047-implement-admin-announcement-module
branch
from
7549e51 to
710f50c
Compare
…thub.com/skkuding/codedang into 1047-implement-admin-announcement-module
| @Query(() => [Announcement], { name: 'announcement' }) | ||
| findAll() { | ||
| return this.announcementService.findAll() | ||
| @Query(() => [Announcement], { name: 'getAnnouncements' }) |
There was a problem hiding this comment.
이거 Query name을 따로 'getAnnouncements'로 지정하지 않아도 밑에 메소드 이름으로 적용될거에요!
| } | ||
|
|
||
| @Mutation(() => Announcement) | ||
| removeAnnouncement(@Args('id', { type: () => Int }) id: number) { | ||
| return this.announcementService.remove(id) | ||
| async removeAnnouncement(@Args('id', { type: () => Int }) id: number) { |
There was a problem hiding this comment.
진짜진짜 사소한건데... CRUD니까 remove대신 delete 어떨까요 ㅎㅎ..
| const announcement = await this.prisma.announcement.findFirst({ | ||
| where: { | ||
| ...(announcementInput.problemId && { | ||
| problemId: announcementInput.problemId | ||
| }), | ||
| contestId: announcementInput.contestId, | ||
| content: announcementInput.content | ||
| } | ||
| }) | ||
| if (announcement) throw new DuplicateFoundException('announcement') |
There was a problem hiding this comment.
create에 announcement를 중복체크하는 로직이 필요있을까요??
content까지 중복되게 create할 일이 없을 것 같아서 저는 없애도 괜찮을 것 같아요!
그리고 만약 중복되게 create한데도 create할때마다 중복체크하는 것보다는 중복으로 잘못만든 것을 delete하는게 더 좋은 것 같은데 혹시 어떻게 생각하시나요!
| @@ -20,7 +20,7 @@ | |||
| "paths": { | |||
| "@admin/*": ["./apps/admin/src/*"], | |||
| "@client/*": ["./apps/client/src/*"], | |||
| "@generated": ["./apps/admin/src/@generated"], | |||
| "@generated": ["apps/admin/src/@generated"], | |||
There was a problem hiding this comment.
호옥시 path는 왜 바꿨을까요...??! 궁금해서 여쭤봅니다 !
|
|
||
| constructor(private readonly prisma: PrismaService) {} | ||
|
|
||
| async createAnnouncement(announcementInput: AnnouncementInput) { |
There was a problem hiding this comment.
혹시 보다가 생각난건데 createAnnouncement 할 때 요청을 보낸 user가 해당 contest에서 열리는 group의 리더인지 검증하는 로직이 필요할까요??
이렇게 구현한다면 group 1 에 대해서 contest 1을 개최했을 때, group 1에 속하지 않은 다른 Admin 계정도 contest 1에서 annoucement를 작성하는 게 가능할 것 같아서요!
There was a problem hiding this comment.
여쭤봤는데.. 아직 group에 대해서 기획 확실히 정해진게 없어서 기획팀께 문의해보기로 하였습니다 🥹
| }) | ||
| .catch((error) => { | ||
| if (error.name == 'NotFoundError') { | ||
| throw new EntityNotExistException('contestProblem') |
There was a problem hiding this comment.
혹시 에러 return할 때 contestProblem으로 EntityNotExistException을 보내도 괜찮을까요?
ContestProblem이 하나의 Entity라고 인식하기 어려울 수 있을까 싶어서요! (problem, contest 등에 비해)
그리고 만약 problemId를 주지않고 contestId로만 announcement를 생성한다면 이때 contest가 없어서 뜨는 에러에서 ContestProblem이라고 뜨면 부자연스러울 것 같은데 혹시 어떻게 생각하시나요?
더 자세히 에러를 적는게 나을까요 아니면 이렇게 contestProblem으로 에러를 보내도 사용자가 이해하기 쉬울까요..?!
| @@ -0,0 +1,38 @@ | |||
| meta { | |||
| name: Error: Duplicated Announcement | |||
There was a problem hiding this comment.
혹시 그 에러 이름 양식 맞춰서 작성해주실 수 있을까요?
UNPROCESSABLE (1) (2) ... (에러 네임) 이렇게 적기로 저번 겨울 방학에 정했는데, 관련 회의록 찾아보고 있으면 드릴게요!
| ### Args | ||
| | 이름 | 타입 | 설명| | ||
| |--|--|--| | ||
| |id|int|조회할 Announcement의 id| |
There was a problem hiding this comment.
앗 Args는 그 bruno schema 연동하면 args 다 보여주기 때문에 굳이 적으실 필요는 없어요!
그래도 수고하셨습니다 ... !!
Description
closes #1047
Admin announcement 모듈을 구현하였습니다.
Additional context
생성하려는 announcment의 problem Id, content 쌍이 이미 존재한다면 Duplicated Error를 발생시키고 있습니다. 이 때의 에러 메시지를 수정할 필요가 있어 보입니다.
Before submitting the PR, please make sure you do the following
fixes #123).