feat: 스터디원 엑셀 다운로드 API 추가 #806
Conversation
…into feature/787-download-study-excel
|
Caution Review failedThe pull request is closed. Walkthrough
Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (10)
src/main/java/com/gdschongik/gdsc/global/common/constant/WorkbookConstant.java (3)
15-15: WEEKLY_ASSIGNMENT 상수가 적절히 정의되었습니다.주차별 과제를 위한 포맷 문자열이 잘 정의되어 있습니다. 동적으로 주차 번호를 삽입할 수 있어 유연성이 좋습니다.
개선 제안: 상수명을
WEEKLY_ASSIGNMENT_FORMAT으로 변경하면 이 상수가 포맷 문자열임을 더 명확히 나타낼 수 있습니다.
16-16: WEEKLY_ATTENDANCE 상수가 적절히 정의되었습니다.주차별 출석을 위한 포맷 문자열이 잘 정의되어 있습니다.
WEEKLY_ASSIGNMENT와 일관된 접근 방식을 사용하고 있어 좋습니다.개선 제안:
WEEKLY_ASSIGNMENT와 마찬가지로, 상수명을WEEKLY_ATTENDANCE_FORMAT으로 변경하면 이 상수가 포맷 문자열임을 더 명확히 나타낼 수 있습니다.
Line range hint
1-20: 전체적으로 PR 목표에 부합하는 변경사항입니다.이 파일의 변경사항은 스터디원 엑셀 다운로드 API 추가라는 PR의 목표에 잘 부합합니다. 새로 추가된 상수들은 스터디 관련 정보를 엑셀 파일로 생성하는 데 필요한 기초를 제공합니다. 기존 상수들과의 일관성도 잘 유지되고 있습니다.
향후 개선사항: 만약 다양한 유형의 워크북이 추가될 예정이라면, 각 워크북 유형별로 별도의 상수 클래스를 만드는 것을 고려해 보세요. 예를 들어,
StudyWorkbookConstant,MemberWorkbookConstant등으로 분리하면 코드의 구조화와 유지보수성이 향상될 수 있습니다.src/main/java/com/gdschongik/gdsc/domain/study/dao/StudyHistoryRepository.java (1)
23-24: 새로운 메서드가 잘 추가되었습니다.새로 추가된
findAllByStudyId메서드는 기존 메서드들과 일관성 있게 작성되었으며, PR의 목적에 부합합니다. 스프링 데이터 JPA 명명 규칙을 잘 따르고 있습니다.일관성을 위해 기존의
findByStudyId메서드와 새로운findAllByStudyId메서드 사이에 빈 줄을 추가하는 것이 좋겠습니다. 이렇게 하면 코드의 가독성이 향상될 것입니다.src/main/java/com/gdschongik/gdsc/domain/study/dto/response/StudyTodoResponse.java (3)
67-69: 메서드 구현이 적절합니다.
isAttendance()메서드의 구현이 간결하고 명확합니다. 이는StudyTodoResponse의 사용성을 향상시킵니다.일관성을 위해 다음과 같이 메서드 이름을 변경하는 것을 고려해 보세요:
- public boolean isAttendance() { + public boolean isAttendanceType() { return todoType == ATTENDANCE; }이렇게 하면
StudyTodoType열거형의 이름과 더 일치하게 됩니다.
71-73: 메서드 구현이 적절합니다.
isAssignment()메서드의 구현이 간결하고 명확합니다. 이는StudyTodoResponse의 사용성을 향상시키며,isAttendance()메서드와 일관성이 있습니다.일관성을 위해 다음과 같이 메서드 이름을 변경하는 것을 고려해 보세요:
- public boolean isAssignment() { + public boolean isAssignmentType() { return todoType == ASSIGNMENT; }이렇게 하면
StudyTodoType열거형의 이름과 더 일치하게 됩니다.
67-73: 새로운 메서드에 대한 문서화 추가 제안새로 추가된
isAttendance()와isAssignment()메서드는StudyTodoResponse레코드의 기능을 향상시킵니다. 하지만 이 메서드들의 목적과 사용 방법을 명확히 하기 위해 문서화를 추가하는 것이 좋겠습니다.각 메서드 위에 JavaDoc 주석을 추가하는 것을 고려해 보세요. 예를 들면:
/** * 이 StudyTodoResponse가 출석 유형인지 확인합니다. * @return 출석 유형이면 true, 그렇지 않으면 false */ public boolean isAttendance() { return todoType == ATTENDANCE; } /** * 이 StudyTodoResponse가 과제 유형인지 확인합니다. * @return 과제 유형이면 true, 그렇지 않으면 false */ public boolean isAssignment() { return todoType == ASSIGNMENT; }이렇게 하면 코드의 가독성과 유지보수성이 향상될 것입니다.
src/main/java/com/gdschongik/gdsc/domain/study/api/MentorStudyController.java (2)
73-86: Excel 다운로드 기능이 잘 구현되었습니다. 파일명 개선 제안새로운
createStudyWorkbook메서드가 요구사항을 잘 충족시키고 있습니다. ResponseEntity와 헤더 설정이 적절히 이루어져 있어 파일 다운로드 기능이 정상적으로 작동할 것으로 보입니다.개선 제안:
파일명을 "study.xls"로 하드코딩하는 대신, 스터디 ID나 이름을 포함시켜 더 구체적으로 만들면 좋을 것 같습니다. 예를 들어:String filename = String.format("study_%d.xls", studyId); ContentDisposition contentDisposition = ContentDisposition.builder("attachment").filename(filename).build();이렇게 하면 사용자가 여러 스터디의 엑셀 파일을 다운로드할 때 파일을 쉽게 구분할 수 있습니다.
75-86: 예외 처리 개선 제안현재 메서드는
IOException을 던질 수 있다고 선언되어 있지만, 구체적인 예외 처리가 없습니다. 다음과 같은 개선을 제안합니다:
mentorStudyService.createStudyExcel(studyId)메서드에서 발생할 수 있는 구체적인 예외들을 파악하고, 이에 대한 처리를 추가하세요.- 가능하다면 사용자에게 더 명확한 에러 메시지를 제공할 수 있도록 예외를 잡아 처리하는 것을 고려해보세요.
예시:
try { byte[] response = mentorStudyService.createStudyExcel(studyId); // ... (나머지 코드) } catch (StudyNotFoundException e) { return ResponseEntity.notFound().build(); } catch (IOException e) { return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) .body("엑셀 파일 생성 중 오류가 발생했습니다."); }이렇게 하면 클라이언트에게 더 명확한 에러 응답을 제공할 수 있습니다.
src/main/java/com/gdschongik/gdsc/global/util/ExcelUtil.java (1)
75-76: TODO 항목 처리 필요코드에
// todo: 수료 여부 추가라는 주석이 있습니다. 수료 여부 필드가 아직 구현되지 않은 것으로 보입니다.수료 여부 필드 추가 및 관련 구현에 도움이 필요하시면 말씀해주세요. 새로운 GitHub 이슈를 생성하거나 구현을 도와드릴 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
- src/main/java/com/gdschongik/gdsc/domain/study/api/MentorStudyController.java (2 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/application/MentorStudyService.java (4 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/dao/StudyHistoryRepository.java (1 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/dto/response/StudyTodoResponse.java (1 hunks)
- src/main/java/com/gdschongik/gdsc/global/common/constant/WorkbookConstant.java (1 hunks)
- src/main/java/com/gdschongik/gdsc/global/util/ExcelUtil.java (4 hunks)
🧰 Additional context used
🔇 Additional comments (9)
src/main/java/com/gdschongik/gdsc/global/common/constant/WorkbookConstant.java (1)
12-14: STUDY_SHEET_HEADER 상수가 적절히 정의되었습니다.스터디 멤버 정보를 위한 헤더가 잘 정의되어 있습니다. 이름, 학번, 디스코드 정보 등 필요한 정보들이 포함되어 있어 적절해 보입니다.
src/main/java/com/gdschongik/gdsc/domain/study/api/MentorStudyController.java (3)
11-11: 새로운 임포트가 적절히 추가되었습니다.
IOException,ContentDisposition, 그리고MediaType의 임포트는 새로운 Excel 다운로드 기능을 위해 필요하며 올바르게 추가되었습니다.Also applies to: 16-17
Line range hint
1-86: 전반적인 구현이 잘 되었으나 몇 가지 개선 사항이 있습니다새로운 Excel 다운로드 기능이 성공적으로 구현되었습니다. PR의 목적을 잘 달성하고 있으며, 전체적인 구조가 적절합니다. 다만 다음 사항들을 개선하면 더 좋을 것 같습니다:
- 파일명을 더 구체적으로 지정하여 사용자 편의성 향상
- 예외 처리를 더 구체적으로 하여 오류 상황에 대한 명확한 피드백 제공
- 보안 검증 메커니즘 추가 또는 확인하여 무단 접근 방지
이러한 개선사항들을 반영하면 코드의 품질과 안정성이 더욱 향상될 것입니다.
73-75: 보안 검증 추가 필요현재 구현에서는 인증이나 권한 검사가 명시적으로 보이지 않습니다. 다음 사항을 고려해 보시기 바랍니다:
- 이 엔드포인트에 접근하기 위한 인증 메커니즘이 있는지 확인하세요.
- 요청한 사용자가 해당 스터디의 정보를 다운로드할 권한이 있는지 검증하는 로직을 추가하세요.
예를 들어:
@PreAuthorize("@studyAuthorizationChecker.isMentor(#studyId)") @GetMapping("/{studyId}/students/excel") public ResponseEntity<byte[]> createStudyWorkbook(@PathVariable Long studyId) throws IOException { // ... (기존 코드) }이렇게 하면 오직 해당 스터디의 멘토만이 학생 정보를 다운로드할 수 있게 됩니다.
보안 검증이 다른 곳에서 이루어지고 있는지 확인하기 위해 다음 스크립트를 실행해 주세요:
src/main/java/com/gdschongik/gdsc/global/util/ExcelUtil.java (2)
34-35: 메소드 호출 이름 변경 반영 확인메소드 이름이
createSheet에서createMemberSheetByRole로 변경되었으며, 해당 호출 부분도 정확히 수정되어 있습니다.
39-43: 새로운 기능 추가 확인스터디 엑셀 파일을 생성하는
createStudyExcel메소드가 적절하게 추가되었습니다. 기능 구현 및 예외 처리가 올바르게 되어 있습니다.src/main/java/com/gdschongik/gdsc/domain/study/application/MentorStudyService.java (3)
26-26: ExcelUtil import 확인되었습니다ExcelUtil을 올바르게 import하여 Excel 관련 기능을 사용할 수 있게 되었습니다.
48-48: ExcelUtil 의존성 주입 확인ExcelUtil을 멤버 변수로 선언하고,
@RequiredArgsConstructor를 통해 의존성을 주입하여 설정하였습니다.
78-80: 코드 재사용성을 높이기 위한 리팩토링 좋습니다
getStudyAchievementMap,getAttendanceMap,getAssignmentHistoryMap메서드를 활용하여 중복 코드를 줄이고 가독성을 향상시켰습니다.
| private Sheet setUpStudySheet(Workbook workbook, String sheetName, long totalWeek) { | ||
| Sheet sheet = workbook.createSheet(sheetName); | ||
|
|
||
| Row row = sheet.createRow(0); | ||
| IntStream.range(0, STUDY_SHEET_HEADER.length).forEach(i -> { | ||
| Cell cell = row.createCell(i); | ||
| cell.setCellValue(STUDY_SHEET_HEADER[i]); | ||
| }); | ||
|
|
||
| for (int i = 1; i <= totalWeek; i++) { | ||
| Cell cell = row.createCell(row.getLastCellNum()); | ||
| cell.setCellValue(String.format(WEEKLY_ASSIGNMENT, i)); | ||
| } | ||
|
|
||
| for (int i = 1; i <= totalWeek; i++) { | ||
| Cell cell = row.createCell(row.getLastCellNum()); | ||
| cell.setCellValue(String.format(WEEKLY_ATTENDANCE, i)); | ||
| } | ||
| return sheet; | ||
| } |
There was a problem hiding this comment.
헤더 셀 인덱스 관리 방식 개선
setUpStudySheet 메소드에서 헤더를 생성할 때 row.getLastCellNum()을 사용하고 있습니다. 하지만 새로운 행에서 getLastCellNum()은 예상치 못한 값을 반환할 수 있으므로 주의가 필요합니다.
헤더 셀 인덱스를 별도의 변수로 관리하여 정확한 위치에 헤더가 추가되도록 수정하는 것을 권장합니다.
int cellIndex = STUDY_SHEET_HEADER.length;
// 과제 헤더 추가
for (int i = 1; i <= totalWeek; i++) {
Cell cell = row.createCell(cellIndex++);
cell.setCellValue(String.format(WEEKLY_ASSIGNMENT, i));
}
// 출석 헤더 추가
for (int i = 1; i <= totalWeek; i++) {
Cell cell = row.createCell(cellIndex++);
cell.setCellValue(String.format(WEEKLY_ATTENDANCE, i));
}📝 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.
| private Sheet setUpStudySheet(Workbook workbook, String sheetName, long totalWeek) { | |
| Sheet sheet = workbook.createSheet(sheetName); | |
| Row row = sheet.createRow(0); | |
| IntStream.range(0, STUDY_SHEET_HEADER.length).forEach(i -> { | |
| Cell cell = row.createCell(i); | |
| cell.setCellValue(STUDY_SHEET_HEADER[i]); | |
| }); | |
| for (int i = 1; i <= totalWeek; i++) { | |
| Cell cell = row.createCell(row.getLastCellNum()); | |
| cell.setCellValue(String.format(WEEKLY_ASSIGNMENT, i)); | |
| } | |
| for (int i = 1; i <= totalWeek; i++) { | |
| Cell cell = row.createCell(row.getLastCellNum()); | |
| cell.setCellValue(String.format(WEEKLY_ATTENDANCE, i)); | |
| } | |
| return sheet; | |
| } | |
| private Sheet setUpStudySheet(Workbook workbook, String sheetName, long totalWeek) { | |
| Sheet sheet = workbook.createSheet(sheetName); | |
| Row row = sheet.createRow(0); | |
| IntStream.range(0, STUDY_SHEET_HEADER.length).forEach(i -> { | |
| Cell cell = row.createCell(i); | |
| cell.setCellValue(STUDY_SHEET_HEADER[i]); | |
| }); | |
| int cellIndex = STUDY_SHEET_HEADER.length; | |
| // 과제 헤더 추가 | |
| for (int i = 1; i <= totalWeek; i++) { | |
| Cell cell = row.createCell(cellIndex++); | |
| cell.setCellValue(String.format(WEEKLY_ASSIGNMENT, i)); | |
| } | |
| // 출석 헤더 추가 | |
| for (int i = 1; i <= totalWeek; i++) { | |
| Cell cell = row.createCell(cellIndex++); | |
| cell.setCellValue(String.format(WEEKLY_ATTENDANCE, i)); | |
| } | |
| return sheet; | |
| } |
| private void createStudySheet(Workbook workbook, Study study, List<StudyStudentResponse> content) { | ||
| Sheet sheet = setUpStudySheet(workbook, study.getTitle(), study.getTotalWeek()); | ||
|
|
||
| content.forEach(student -> { | ||
| Row studentRow = sheet.createRow(sheet.getLastRowNum() + 1); | ||
| studentRow.createCell(0).setCellValue(student.name()); | ||
| studentRow.createCell(1).setCellValue(student.studentId()); | ||
| studentRow.createCell(2).setCellValue(student.discordUsername()); | ||
| studentRow.createCell(3).setCellValue(student.nickname()); | ||
| studentRow.createCell(4).setCellValue(student.githubLink()); | ||
| // todo: 수료 여부 추가 | ||
| studentRow.createCell(5).setCellValue("X"); | ||
| studentRow.createCell(6).setCellValue(student.isFirstRoundOutstandingStudent() ? "O" : "X"); | ||
| studentRow.createCell(7).setCellValue(student.isSecondRoundOutstandingStudent() ? "O" : "X"); | ||
| studentRow.createCell(8).setCellValue(student.attendanceRate()); | ||
| studentRow.createCell(9).setCellValue(student.assignmentRate()); | ||
| student.studyTodos().stream() | ||
| .filter(StudyTodoResponse::isAssignment) | ||
| .forEach(todo -> { | ||
| studentRow | ||
| .createCell(studentRow.getLastCellNum()) | ||
| .setCellValue(todo.assignmentSubmissionStatus().getValue()); | ||
| }); | ||
| student.studyTodos().stream() | ||
| .filter(StudyTodoResponse::isAttendance) | ||
| .forEach(todo -> { | ||
| studentRow | ||
| .createCell(studentRow.getLastCellNum()) | ||
| .setCellValue(todo.attendanceStatus().getValue()); | ||
| }); | ||
| }); | ||
| } |
There was a problem hiding this comment.
셀 인덱스 관리 방식 개선 필요
createStudySheet 메소드에서 학생 데이터를 셀에 추가할 때 studentRow.getLastCellNum()을 사용하고 있습니다. 그러나 반복문 내에서 이 메소드를 사용하면 셀 인덱스 관리에 문제가 발생할 수 있습니다.
셀 인덱스를 별도의 변수로 관리하여 데이터가 정확한 위치에 입력되도록 수정하는 것을 권장합니다.
int cellIndex = 5; // 초기 셀 인덱스 설정
// 수료 여부
// TODO: 수료 여부 추가
studentRow.createCell(cellIndex++).setCellValue("X");
studentRow.createCell(cellIndex++).setCellValue(student.isFirstRoundOutstandingStudent() ? "O" : "X");
studentRow.createCell(cellIndex++).setCellValue(student.isSecondRoundOutstandingStudent() ? "O" : "X");
studentRow.createCell(cellIndex++).setCellValue(student.attendanceRate());
studentRow.createCell(cellIndex++).setCellValue(student.assignmentRate());
// 과제 상태 입력
student.studyTodos().stream()
.filter(StudyTodoResponse::isAssignment)
.forEach(todo -> {
studentRow.createCell(cellIndex++)
.setCellValue(todo.assignmentSubmissionStatus().getValue());
});
// 출석 상태 입력
student.studyTodos().stream()
.filter(StudyTodoResponse::isAttendance)
.forEach(todo -> {
studentRow.createCell(cellIndex++)
.setCellValue(todo.attendanceStatus().getValue());
});📝 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.
| private void createStudySheet(Workbook workbook, Study study, List<StudyStudentResponse> content) { | |
| Sheet sheet = setUpStudySheet(workbook, study.getTitle(), study.getTotalWeek()); | |
| content.forEach(student -> { | |
| Row studentRow = sheet.createRow(sheet.getLastRowNum() + 1); | |
| studentRow.createCell(0).setCellValue(student.name()); | |
| studentRow.createCell(1).setCellValue(student.studentId()); | |
| studentRow.createCell(2).setCellValue(student.discordUsername()); | |
| studentRow.createCell(3).setCellValue(student.nickname()); | |
| studentRow.createCell(4).setCellValue(student.githubLink()); | |
| // todo: 수료 여부 추가 | |
| studentRow.createCell(5).setCellValue("X"); | |
| studentRow.createCell(6).setCellValue(student.isFirstRoundOutstandingStudent() ? "O" : "X"); | |
| studentRow.createCell(7).setCellValue(student.isSecondRoundOutstandingStudent() ? "O" : "X"); | |
| studentRow.createCell(8).setCellValue(student.attendanceRate()); | |
| studentRow.createCell(9).setCellValue(student.assignmentRate()); | |
| student.studyTodos().stream() | |
| .filter(StudyTodoResponse::isAssignment) | |
| .forEach(todo -> { | |
| studentRow | |
| .createCell(studentRow.getLastCellNum()) | |
| .setCellValue(todo.assignmentSubmissionStatus().getValue()); | |
| }); | |
| student.studyTodos().stream() | |
| .filter(StudyTodoResponse::isAttendance) | |
| .forEach(todo -> { | |
| studentRow | |
| .createCell(studentRow.getLastCellNum()) | |
| .setCellValue(todo.attendanceStatus().getValue()); | |
| }); | |
| }); | |
| } | |
| private void createStudySheet(Workbook workbook, Study study, List<StudyStudentResponse> content) { | |
| Sheet sheet = setUpStudySheet(workbook, study.getTitle(), study.getTotalWeek()); | |
| content.forEach(student -> { | |
| Row studentRow = sheet.createRow(sheet.getLastRowNum() + 1); | |
| studentRow.createCell(0).setCellValue(student.name()); | |
| studentRow.createCell(1).setCellValue(student.studentId()); | |
| studentRow.createCell(2).setCellValue(student.discordUsername()); | |
| studentRow.createCell(3).setCellValue(student.nickname()); | |
| studentRow.createCell(4).setCellValue(student.githubLink()); | |
| int cellIndex = 5; // 초기 셀 인덱스 설정 | |
| // 수료 여부 | |
| // TODO: 수료 여부 추가 | |
| studentRow.createCell(cellIndex++).setCellValue("X"); | |
| studentRow.createCell(cellIndex++).setCellValue(student.isFirstRoundOutstandingStudent() ? "O" : "X"); | |
| studentRow.createCell(cellIndex++).setCellValue(student.isSecondRoundOutstandingStudent() ? "O" : "X"); | |
| studentRow.createCell(cellIndex++).setCellValue(student.attendanceRate()); | |
| studentRow.createCell(cellIndex++).setCellValue(student.assignmentRate()); | |
| // 과제 상태 입력 | |
| student.studyTodos().stream() | |
| .filter(StudyTodoResponse::isAssignment) | |
| .forEach(todo -> { | |
| studentRow.createCell(cellIndex++) | |
| .setCellValue(todo.assignmentSubmissionStatus().getValue()); | |
| }); | |
| // 출석 상태 입력 | |
| student.studyTodos().stream() | |
| .filter(StudyTodoResponse::isAttendance) | |
| .forEach(todo -> { | |
| studentRow.createCell(cellIndex++) | |
| .setCellValue(todo.attendanceStatus().getValue()); | |
| }); | |
| }); | |
| } |
There was a problem hiding this comment.
@uwoobeat 안녕하세요, 말씀하신 부분 확인했습니다. createMemberSheetByRole 메소드에서도 셀 인덱스 관리를 개선할 수 있습니다. 현재 해당 메소드에서는 셀 인덱스를 하드코딩하고 있는데, cellIndex 변수를 사용하여 관리하면 유지보수성과 가독성을 향상시킬 수 있습니다. 아래와 같이 수정해보실 수 있습니다:
private void createMemberSheetByRole(Workbook workbook, String sheetName, @Nullable MemberRole role) {
Sheet sheet = setUpMemberSheet(workbook, sheetName);
memberRepository.findAllByRole(role).forEach(member -> {
Row memberRow = sheet.createRow(sheet.getLastRowNum() + 1);
int cellIndex = 0;
memberRow.createCell(cellIndex++).setCellValue(member.getCreatedAt().toString());
memberRow.createCell(cellIndex++).setCellValue(member.getName());
memberRow.createCell(cellIndex++).setCellValue(member.getStudentId());
memberRow.createCell(cellIndex++)
.setCellValue(Optional.ofNullable(member.getDepartment())
.map(Department::getDepartmentName)
.orElse(""));
memberRow.createCell(cellIndex++).setCellValue(member.getPhone());
memberRow.createCell(cellIndex++).setCellValue(member.getEmail());
memberRow.createCell(cellIndex++).setCellValue(member.getDiscordUsername());
memberRow.createCell(cellIndex++).setCellValue(member.getNickname());
});
}이렇게 수정하면 셀 인덱스를 일관성 있게 관리할 수 있어 코드의 신뢰성을 높일 수 있습니다. 확인 부탁드립니다.
| @@ -208,4 +200,65 @@ private void updateAllStudyDetailCurriculum( | |||
| studyDetailRepository.saveAll(studyDetails); | |||
| log.info("[MentorStudyService] 스터디 상세정보 커리큘럼 작성 완료: studyDetailId={}", studyDetails); | |||
| } | |||
|
|
|||
| @Transactional(readOnly = true) | |||
| public byte[] createStudyExcel(Long studyId) throws IOException { | |||
There was a problem hiding this comment.
IOException 처리 방식 개선 필요
createStudyExcel 메서드에서 IOException을 throws하고 있습니다. 서비스 계층에서는 체크 예외를 직접 throws하기보다는 내부에서 처리하거나 적절한 런타임 예외로 변환하는 것이 좋습니다. IOException을 catch하여 CustomException으로 래핑하면 클라이언트가 에러를 더 명확히 이해할 수 있습니다.
수정 제안:
- public byte[] createStudyExcel(Long studyId) throws IOException {
+ public byte[] createStudyExcel(Long studyId) {예외 처리 추가:
try {
return excelUtil.createStudyExcel(study, content);
} catch (IOException e) {
throw new CustomException(FILE_CREATION_FAILED, e);
}There was a problem hiding this comment.
@uwoobeat 네, 확인하였습니다. 제안드린 사항을 검토해주셔서 감사합니다.
((
( ^.^)
o(_(")(")
| studyHistories.forEach(studyHistory -> { | ||
| List<StudyAchievement> currentStudyAchievements = | ||
| studyAchievementMap.getOrDefault(studyHistory.getStudent().getId(), new ArrayList<>()); | ||
| List<Attendance> currentAttendances = | ||
| attendanceMap.getOrDefault(studyHistory.getStudent().getId(), new ArrayList<>()); | ||
| List<AssignmentHistory> currentAssignmentHistories = | ||
| assignmentHistoryMap.getOrDefault(studyHistory.getStudent().getId(), new ArrayList<>()); | ||
|
|
||
| List<StudyTodoResponse> studyTodos = new ArrayList<>(); | ||
| studyDetails.forEach(studyDetail -> { | ||
| studyTodos.add(StudyTodoResponse.createAttendanceType( | ||
| studyDetail, LocalDate.now(), isAttended(currentAttendances, studyDetail))); | ||
| studyTodos.add(StudyTodoResponse.createAssignmentType( | ||
| studyDetail, getSubmittedAssignment(currentAssignmentHistories, studyDetail))); | ||
| }); | ||
|
|
||
| content.add(StudyStudentResponse.of(studyHistory, currentStudyAchievements, studyTodos)); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
스트림 API 활용으로 코드 간결화 가능
studyHistories를 순회하며 StudyStudentResponse를 생성하는 부분에서 스트림 API를 활용하면 코드를 더욱 간결하게 표현할 수 있습니다.
예시 수정안:
List<StudyStudentResponse> content = studyHistories.stream().map(studyHistory -> {
List<StudyAchievement> currentStudyAchievements =
studyAchievementMap.getOrDefault(studyHistory.getStudent().getId(), new ArrayList<>());
List<Attendance> currentAttendances =
attendanceMap.getOrDefault(studyHistory.getStudent().getId(), new ArrayList<>());
List<AssignmentHistory> currentAssignmentHistories =
assignmentHistoryMap.getOrDefault(studyHistory.getStudent().getId(), new ArrayList<>());
List<StudyTodoResponse> studyTodos = studyDetails.stream().flatMap(studyDetail -> Stream.of(
StudyTodoResponse.createAttendanceType(
studyDetail, LocalDate.now(), isAttended(currentAttendances, studyDetail)),
StudyTodoResponse.createAssignmentType(
studyDetail, getSubmittedAssignment(currentAssignmentHistories, studyDetail))
)).collect(Collectors.toList());
return StudyStudentResponse.of(studyHistory, currentStudyAchievements, studyTodos);
}).collect(Collectors.toList());| private Map<Long, List<StudyAchievement>> getStudyAchievementMap(Long studyId, List<Long> studentIds) { | ||
| List<StudyAchievement> studyAchievements = | ||
| studyAchievementRepository.findByStudyIdAndMemberIds(studyId, studentIds); | ||
| return studyAchievements.stream() | ||
| .collect(groupingBy( | ||
| studyAchievement -> studyAchievement.getStudent().getId())); | ||
| } | ||
|
|
||
| private Map<Long, List<Attendance>> getAttendanceMap(Long studyId, List<Long> studentIds) { | ||
| List<Attendance> attendances = attendanceRepository.findByStudyIdAndMemberIds(studyId, studentIds); | ||
| return attendances.stream() | ||
| .collect(groupingBy(attendance -> attendance.getStudent().getId())); | ||
| } | ||
|
|
||
| private Map<Long, List<AssignmentHistory>> getAssignmentHistoryMap(Long studyId, List<Long> studentIds) { | ||
| List<AssignmentHistory> assignmentHistories = | ||
| assignmentHistoryRepository.findByStudyIdAndMemberIds(studyId, studentIds); | ||
| return assignmentHistories.stream() | ||
| .collect(groupingBy( | ||
| assignmentHistory -> assignmentHistory.getMember().getId())); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
유사한 메서드들의 중복 제거 고려
getStudyAchievementMap, getAttendanceMap, getAssignmentHistoryMap 메서드들이 유사한 로직을 가지고 있습니다. 제네릭을 활용하거나 공통 메서드로 추출하여 중복을 줄일 수 있습니다.
예시 수정안:
private <T, R> Map<Long, List<T>> getEntityMap(Long studyId, List<Long> studentIds,
BiFunction<Long, List<Long>, List<T>> repositoryMethod,
Function<T, R> groupingFunction) {
List<T> entities = repositoryMethod.apply(studyId, studentIds);
return entities.stream()
.collect(groupingBy(entity -> groupingFunction.apply(entity).getId()));
}
// 활용 예시
private Map<Long, List<StudyAchievement>> getStudyAchievementMap(Long studyId, List<Long> studentIds) {
return getEntityMap(studyId, studentIds,
studyAchievementRepository::findByStudyIdAndMemberIds,
studyAchievement -> studyAchievement.getStudent());
}
private Map<Long, List<Attendance>> getAttendanceMap(Long studyId, List<Long> studentIds) {
return getEntityMap(studyId, studentIds,
attendanceRepository::findByStudyIdAndMemberIds,
attendance -> attendance.getStudent());
}
private Map<Long, List<AssignmentHistory>> getAssignmentHistoryMap(Long studyId, List<Long> studentIds) {
return getEntityMap(studyId, studentIds,
assignmentHistoryRepository::findByStudyIdAndMemberIds,
assignmentHistory -> assignmentHistory.getMember());
}
| @@ -208,4 +200,65 @@ private void updateAllStudyDetailCurriculum( | |||
| studyDetailRepository.saveAll(studyDetails); | |||
| log.info("[MentorStudyService] 스터디 상세정보 커리큘럼 작성 완료: studyDetailId={}", studyDetails); | |||
| } | |||
|
|
|||
| @Transactional(readOnly = true) | |||
| public byte[] createStudyExcel(Long studyId) throws IOException { | |||
| private void createStudySheet(Workbook workbook, Study study, List<StudyStudentResponse> content) { | ||
| Sheet sheet = setUpStudySheet(workbook, study.getTitle(), study.getTotalWeek()); | ||
|
|
||
| content.forEach(student -> { | ||
| Row studentRow = sheet.createRow(sheet.getLastRowNum() + 1); | ||
| studentRow.createCell(0).setCellValue(student.name()); | ||
| studentRow.createCell(1).setCellValue(student.studentId()); | ||
| studentRow.createCell(2).setCellValue(student.discordUsername()); | ||
| studentRow.createCell(3).setCellValue(student.nickname()); | ||
| studentRow.createCell(4).setCellValue(student.githubLink()); | ||
| // todo: 수료 여부 추가 | ||
| studentRow.createCell(5).setCellValue("X"); | ||
| studentRow.createCell(6).setCellValue(student.isFirstRoundOutstandingStudent() ? "O" : "X"); | ||
| studentRow.createCell(7).setCellValue(student.isSecondRoundOutstandingStudent() ? "O" : "X"); | ||
| studentRow.createCell(8).setCellValue(student.attendanceRate()); | ||
| studentRow.createCell(9).setCellValue(student.assignmentRate()); | ||
| student.studyTodos().stream() | ||
| .filter(StudyTodoResponse::isAssignment) | ||
| .forEach(todo -> { | ||
| studentRow | ||
| .createCell(studentRow.getLastCellNum()) | ||
| .setCellValue(todo.assignmentSubmissionStatus().getValue()); | ||
| }); | ||
| student.studyTodos().stream() | ||
| .filter(StudyTodoResponse::isAttendance) | ||
| .forEach(todo -> { | ||
| studentRow | ||
| .createCell(studentRow.getLastCellNum()) | ||
| .setCellValue(todo.attendanceStatus().getValue()); | ||
| }); | ||
| }); | ||
| } |
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
🌱 관련 이슈
📌 작업 내용 및 특이사항
📝 참고사항
✨ 수강생 명단 조회 API에 수료 여부 필드 추가 #805 에서 수정하겠습니다~
📚 기타
Summary by CodeRabbit
새로운 기능
버그 수정
문서화