-
Notifications
You must be signed in to change notification settings - Fork 0
[CMAT-47] feat: 관리자용 컨텐츠 삭제 API 구현, 직무별 컨텐츠 조회 API 수정 #29
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
Conversation
Walkthrough이 변경 사항은 콘텐츠 관리 기능을 개선하기 위한 것입니다. Changes
Sequence DiagramsequenceDiagram
participant Client
participant ContentController
participant ContentService
participant ContentRepository
Client->>ContentController: DELETE /contents/{contentId}
ContentController->>ContentService: deleteContent(contentId)
ContentService->>ContentRepository: findById(contentId)
alt Content exists
ContentService->>ContentRepository: delete(content)
ContentController-->>Client: Success Response
else Content not found
ContentService-->>ContentController: Throw GeneralException
ContentController-->>Client: Error Response
end
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/main/java/UMC/career_mate/domain/content/service/ContentService.java (1)
43-49: 트랜잭션 격리 수준 지정 검토컨텐츠 삭제 작업은 데이터 일관성을 위해 명시적인 트랜잭션 격리 수준이 필요할 수 있습니다.
다음과 같이
@Transactional어노테이션에 격리 수준을 지정하는 것을 고려해보세요:- public void deleteContent(Long contentId) { + @Transactional(isolation = Isolation.READ_COMMITTED) + public void deleteContent(Long contentId) {src/main/java/UMC/career_mate/domain/content/controller/ContentController.java (2)
46-58: 응답 메시지 개선 필요삭제된 컨텐츠의 ID를 응답에 포함하면 클라이언트 측에서 더 유용하게 활용할 수 있습니다.
다음과 같이 개선하는 것을 추천드립니다:
- public ApiResponse<String> deleteContent(@PathVariable Long contentId) { - contentService.deleteContent(contentId); - return ApiResponse.onSuccess("컨텐츠 삭제 완료"); + public ApiResponse<Map<String, Object>> deleteContent(@PathVariable Long contentId) { + contentService.deleteContent(contentId); + Map<String, Object> response = new HashMap<>(); + response.put("message", "컨텐츠 삭제 완료"); + response.put("deletedContentId", contentId); + return ApiResponse.onSuccess(response);
62-64: API 문서 보완 필요API 문서에 인증 요구사항과 발생 가능한 예외 상황에 대한 설명이 누락되어 있습니다.
다음과 같이 문서를 보완하는 것을 추천드립니다:
@Operation( - summary = "로그인한 사용자의 직무별 컨텐츠 조회 API", - description = """ - 로그인한 사용자의 직무 ID에 해당하는 컨텐츠를 조회합니다. - 이때 사용자가 해당 컨텐츠를 스크랩했는지 여부가 담겨 조회됩니다. + summary = "로그인한 사용자의 직무별 컨텐츠 조회 API (인증 필요)", + description = """ + 로그인한 사용자의 직무 ID에 해당하는 컨텐츠를 조회합니다. + 이때 사용자가 해당 컨텐츠를 스크랩했는지 여부가 담겨 조회됩니다. + + ### 인증 요구사항 + - 로그인된 사용자만 접근 가능 + - 유효한 직무가 설정된 사용자만 접근 가능 + + ### 발생 가능한 예외 + - 401: 인증되지 않은 사용자 + - 404: 사용자의 직무 정보가 없는 경우
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/UMC/career_mate/domain/content/controller/ContentController.java(1 hunks)src/main/java/UMC/career_mate/domain/content/service/ContentService.java(2 hunks)src/main/resources/application.yml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
src/main/resources/application.yml
[error] 3-3: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (2)
src/main/java/UMC/career_mate/domain/content/service/ContentService.java (1)
13-14: LGTM!예외 처리를 위한 적절한 import문이 추가되었습니다.
src/main/java/UMC/career_mate/domain/content/controller/ContentController.java (1)
75-75: LGTM!서비스 계층의 변경사항과 일관성 있게 수정되었습니다.
| spring: | ||
| profiles: | ||
| active: dev | ||
| active: dev |
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.
🛠️ Refactor suggestion
파일 끝에 개행 문자를 추가해주세요.
YAML 파일의 끝에는 항상 개행 문자가 있어야 합니다. 이는 POSIX 표준을 준수하고 다른 도구들과의 호환성을 보장하기 위함입니다.
다음과 같이 수정해주세요:
spring:
profiles:
- active: dev
+ active: dev
+📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| active: dev | |
| spring: | |
| profiles: | |
| active: dev | |
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 3-3: no new line character at the end of file
(new-line-at-end-of-file)
💡 Codebase verification
프로덕션 환경 설정의 심각한 문제점이 확인되었습니다
현재 'dev' 프로필이 여러 위치에 하드코딩되어 있어 프로덕션 환경에서 심각한 문제가 될 수 있습니다:
application.yml에active: dev하드코딩Dockerfile의ENTRYPOINT명령어에-Dspring.profiles.active=dev하드코딩
권장 수정사항:
application.yml에서 기본 프로필 설정 제거Dockerfile의 프로필 설정을 환경변수로 대체:ENTRYPOINT ["java","-jar","-Dspring.profiles.active=${SPRING_PROFILES_ACTIVE}","/app.jar"]
- 환경별 설정 파일 (
application-prod.yml,application-dev.yml) 추가 - 배포 시 환경변수
SPRING_PROFILES_ACTIVE를 통해 프로필 주입
🔗 Analysis chain
프로덕션 환경 설정 관리 방안을 검토해주세요.
버전 관리 시스템에 'dev' 프로필이 하드코딩되어 있는 것이 확인됩니다. 프로덕션 환경에서는 환경 변수나 외부 설정을 통해 프로필을 주입하는 것이 더 안전한 방법일 수 있습니다.
다음 스크립트를 실행하여 프로필 설정이 다른 설정 파일에서도 하드코딩되어 있는지 확인해보시겠습니까?
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: 프로필 설정이 하드코딩된 YAML 파일 검색
echo "프로필 설정이 하드코딩된 YAML 파일 검색 중..."
rg -l "active:\s*(dev|prod|test)" --type yaml
Length of output: 151
Script:
#!/bin/bash
# Search for profile-specific configuration files and deployment configs
echo "프로필별 설정 파일 및 배포 관련 파일 검색..."
echo -e "\n1. 프로필별 설정 파일 검색:"
fd "application-.*\.yml" "src/"
echo -e "\n2. 도커 및 쿠버네티스 설정 파일 검색:"
fd "(Dockerfile|docker-compose.*\.yml|\.k8s|kubernetes)" .
echo -e "\n3. 배포 스크립트 검색:"
fd "(deploy.*\.sh|\.env.*)" .
Length of output: 382
Script:
#!/bin/bash
# Check Docker files for environment configuration
echo "도커 설정 파일 내용 확인..."
echo -e "\n1. Dockerfile 내용:"
cat Dockerfile
echo -e "\n2. docker-compose.yml 내용:"
cat docker-compose.yml
Length of output: 575
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 3-3: no new line character at the end of file
(new-line-at-end-of-file)
| public PageResponseDTO<List<ContentResponseDTO>> getContentsByJobId(int page, int size, Member member) { | ||
|
|
||
| // 로그인한 사용자의 직무 ID 확인 | ||
| Long jobId = member.getJob().getId(); | ||
| if (jobId == null) { | ||
| throw new GeneralException(CommonErrorCode.NOT_FOUND_JOB); | ||
| } |
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.
🛠️ Refactor suggestion
직무 정보 검증 로직 개선 필요
현재 구현은 직무 ID가 null인 경우만 처리하고 있습니다. 사용자의 직무 정보에 대한 더 구체적인 유효성 검사가 필요합니다.
다음과 같이 개선하는 것을 추천드립니다:
- Long jobId = member.getJob().getId();
- if (jobId == null) {
+ Job job = member.getJob();
+ if (job == null) {
+ throw new GeneralException(CommonErrorCode.NOT_FOUND_JOB);
+ }
+ Long jobId = job.getId();
+ if (jobId == null || jobId <= 0) {
throw new GeneralException(CommonErrorCode.NOT_FOUND_JOB);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public PageResponseDTO<List<ContentResponseDTO>> getContentsByJobId(int page, int size, Member member) { | |
| // 로그인한 사용자의 직무 ID 확인 | |
| Long jobId = member.getJob().getId(); | |
| if (jobId == null) { | |
| throw new GeneralException(CommonErrorCode.NOT_FOUND_JOB); | |
| } | |
| public PageResponseDTO<List<ContentResponseDTO>> getContentsByJobId(int page, int size, Member member) { | |
| // 로그인한 사용자의 직무 ID 확인 | |
| Job job = member.getJob(); | |
| if (job == null) { | |
| throw new GeneralException(CommonErrorCode.NOT_FOUND_JOB); | |
| } | |
| Long jobId = job.getId(); | |
| if (jobId == null || jobId <= 0) { | |
| throw new GeneralException(CommonErrorCode.NOT_FOUND_JOB); | |
| } |
jpark0506
left a comment
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.
LGLGTM~
jjeongdong
left a comment
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.
LGTM~!
#️⃣ 요약 설명
📝 작업 내용
// 핵심 코드를 붙여넣기 해주세요코드에 대한 간단한 설명 부탁드립니다.
동작 확인
작동화면은 아래와 같습니다.
💬 리뷰 요구사항(선택)
Summary by CodeRabbit
새로운 기능
버그 수정
개선 사항