Skip to content
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

피드 신고 기능 구현 #285

Merged
merged 9 commits into from
Oct 1, 2024
Merged

피드 신고 기능 구현 #285

merged 9 commits into from
Oct 1, 2024

Conversation

dbscks97
Copy link
Collaborator

🌱 관련 이슈

📌 작업 내용

  • 신고 목록 조회 API, 피드 신고 기능 API 구현

🙏 리뷰 요구사항

  • 신고 목록을 관리하는 부분을 figma확인 후 enum으로 관리하고있는데, 이 부분에 대해서 피드백주시면 감사하겠습니다

📚 레퍼런스

@dbscks97 dbscks97 added the ✨ feature 새로운 기능 추가 및 수정 label Sep 30, 2024
@dbscks97 dbscks97 added this to the 4차 MVP milestone Sep 30, 2024
@dbscks97 dbscks97 self-assigned this Sep 30, 2024
@dbscks97 dbscks97 linked an issue Sep 30, 2024 that may be closed by this pull request
Copy link
Member

@char-yb char-yb left a comment

Choose a reason for hiding this comment

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

Request, Response DTO에 Swagger Schema 어노테이션도 같이 추가 부탁드릴게용🙏

Comment on lines 24 to 30
@Operation(summary = "신고 사유 목록 조회", description = "신고 사유 목록을 가져옵니다.")
@GetMapping("/reasons")
public ResponseEntity<List<ReportReasonResponse>> getReportReasons() {
List<ReportReasonResponse> reasons = reportService.getReportReasons();
return ResponseEntity.ok(reasons);
}

Copy link
Member

Choose a reason for hiding this comment

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

굳이 reasons 엔드포인트를 작성하지 않아도 /reports 만으로 충분해보입니당

Comment on lines 38 to 45

Report report =
Report.builder()
.missionRecord(missionRecord)
.member(member)
.reportReason(reportReason)
.details(reportRequest.details())
.build();
Copy link
Member

Choose a reason for hiding this comment

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

도메인 entity에서 정적 팩토리 메서드를 사용하면 좋을 거 같아용
제가 comment 엔티티에 적용했듯이 사용하면 좋을 듯 합니당

Comment on lines 62 to 64
// report
INVALID_REPORT_REASON(HttpStatus.NOT_FOUND, "해당 신고 목록을 찾을 수 없습니다.");
private final HttpStatus httpStatus;
Copy link
Member

Choose a reason for hiding this comment

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

신고 목록보다는 신고 사유를 찾을 수 없습니다.가 맞지 않을까용???

Comment on lines 40 to 47

@Builder
private Report(MissionRecord missionRecord, Member member, String reason, String details) {
this.missionRecord = missionRecord;
this.member = member;
this.reason = reason;
this.details = details;
}
Copy link
Member

Choose a reason for hiding this comment

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

Builder 패턴을 외부에서 사용하지 못하도록
@Builder(access = AccessLevel.PRIVATE)으로 변경부탁드릴게용

Comment on lines +36 to +38

private String reason;

Copy link
Member

Choose a reason for hiding this comment

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

@Enumerated(EnumType.STRING) 사용해서 Enum 타입으로 정의하지 않는 이유가 있을까용??

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

EnumType.STRING을 사용하면 db에 enum의 name값이들어가서 convert를 사용하거나 String을사용해서 value를 저장하려했습니다. 혹시 좋은 방법이 있을까요

Copy link
Member

Choose a reason for hiding this comment

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

아니면 이건 기획 의도따라 다르겠지만 굳이 ReportReason을 enum으로 정의하지 않고,
일반적인 String 값으로 받는 건 어떨까요??

신고하려는 이유가 너무 불특정하게 계속 생길 수도 있고, enum에 대한 길이도 계속 길어지다보니 굳이 enum 자체를 Request 받지 않고 String으로 Request 받아도 될 듯해용

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

음.. figma를 봤을 땐 기획상 신고 사유가 많아지더라도 지금 수준이랑 비슷할 것 같다고 생각했어요! 그리고 기타라는 항목도 있기에 크게 이유가 불특정하게 enum값이 생길 것 같다고 생각을 안했습니다.

저는 Enum을 사용해서 관리하는 부분이 좀 더 낫다고 생각하는데 어떻게 생각하시나요?

추가로 현재는 Enum을 name-value형식으로 되어있는데 @convert를 사용해서 Enum을 사용하되, 원하는 value값을 저장하는 방식으로 코드를 수정하는건 어떨까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

위의 댓글 취소!,,, 윤범님이 말씀하신대로 클라이언트한테 request를 String으로받고 하는식으로 진행하도록 하겠습니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

수정된 부분 확인한번 부탁드릴게요~

Comment on lines 31 to 37
@Operation(summary = "신고하기", description = "특정 피드를 신고한다.")
@PostMapping
public ResponseEntity<Void> reportFeed(@RequestBody ReportRequest reportRequest) {
reportService.reportFeed(reportRequest);
return ResponseEntity.ok().build();
}
}
Copy link
Member

Choose a reason for hiding this comment

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

사실 예전부터 컨벤션 정의할 때부터 맞췄어야 했는데 Entity Resource 생성 시 ResponseEntity 201 Status Code로 return 하도록 하는 것이 가독성 측면과 RestFul하다고 하더라고용

Copy link
Member

@char-yb char-yb left a comment

Choose a reason for hiding this comment

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

회의 내용 때 나왔던 내용으로
저희 슬랙 신고에 대한 채널을 생성해서 신고 생성 시 슬랙 알림이 갈 수 있도록 다음 이슈에 진행부탁드릴게용

Copy link

Copy link
Member

@char-yb char-yb left a comment

Choose a reason for hiding this comment

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

LGTM

@dbscks97 dbscks97 merged commit 37d2457 into develop Oct 1, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ feature 새로운 기능 추가 및 수정
Projects
None yet
Development

Successfully merging this pull request may close these issues.

✨ 피드 신고기능 구현
2 participants