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

refactor: StudyTodoResponse 이름 변경 #812

Merged
merged 2 commits into from
Oct 30, 2024

Conversation

Sangwook02
Copy link
Member

@Sangwook02 Sangwook02 commented Oct 29, 2024

🌱 관련 이슈

📌 작업 내용 및 특이사항

📝 참고사항

📚 기타

Summary by CodeRabbit

  • 새로운 기능

    • "StudyTodoResponse"가 "StudyTaskResponse"로 이름이 변경되어, 학습 관련 작업 처리에서 더 명확한 용어 사용.
  • 버그 수정

    • 관련 메서드에서 "StudyTodoResponse"의 모든 참조가 "StudyTaskResponse"로 업데이트됨.
  • 문서화

    • 엑셀 생성 및 시트 처리 메서드에서 "StudyTaskResponse"를 사용하도록 변경됨.

@Sangwook02 Sangwook02 added the ♻️ refactor 기능 개선 label Oct 29, 2024
@Sangwook02 Sangwook02 self-assigned this Oct 29, 2024
@Sangwook02 Sangwook02 requested a review from a team as a code owner October 29, 2024 07:23
Copy link

coderabbitai bot commented Oct 29, 2024

Walkthrough

이 변경 사항은 StudyTodoResponse 클래스를 StudyTaskResponse로 이름을 변경하는 것을 중심으로 이루어졌습니다. 이로 인해 관련된 여러 메서드와 필드의 타입이 업데이트되었으며, StudentStudyDetailController, MentorStudyService, StudentStudyDetailService, StudyStudentResponse, ExcelUtil 등 여러 클래스에서 해당 변경이 반영되었습니다. 전반적으로 제어 흐름이나 로직은 변경되지 않았으며, 단지 데이터 구조와 관련된 명칭이 변경되었습니다.

Changes

파일 경로 변경 요약
src/main/java/com/gdschongik/gdsc/domain/study/api/StudentStudyDetailController.java StudyTodoResponseStudyTaskResponse로 변경, 메서드 반환 타입 및 내부 변수 업데이트
src/main/java/com/gdschongik/gdsc/domain/study/application/MentorStudyService.java StudyTodoResponseStudyTaskResponse로 변경, 메서드 내 응답 리스트 및 호출 업데이트
src/main/java/com/gdschongik/gdsc/domain/study/application/StudentStudyDetailService.java 메서드 이름 getStudyTodoListgetStudyTaskList로 변경, 반환 타입 및 내부 변수 업데이트
src/main/java/com/gdschongik/gdsc/domain/study/dto/response/StudyStudentResponse.java studyTodos 필드를 studyTasks로 변경, 관련 메서드 시그니처 및 내부 로직 업데이트
src/main/java/com/gdschongik/gdsc/domain/study/dto/response/StudyTaskResponse.java 클래스 이름 변경 및 관련 메서드 업데이트, StudyTodoTypeStudyTaskType으로 변경
src/main/java/com/gdschongik/gdsc/global/util/ExcelUtil.java StudyTodoResponseStudyTaskResponse로 변경, 메서드 내 필터링 로직 업데이트

Assessment against linked issues

Objective Addressed Explanation
StudyTodoResponse 이름 변경 (795)

Possibly related PRs

Suggested reviewers

  • uwoobeat
  • AlmondBreez3

🐰 변화의 바람이 불어와
할 일 목록이 작업으로 변해가네
이름이 바뀌어도 내용은 같아
모두 함께 달려가자, 기쁨의 길로! 🌼


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

Job Summary for Gradle

Check Style and Test to Develop :: build-test
Gradle Root Project Requested Tasks Gradle Version Build Outcome Build Scan®
gdsc check 8.5 Build Scan published

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Outside diff range and nitpick comments (8)
src/main/java/com/gdschongik/gdsc/domain/study/dto/response/StudyTaskResponse.java (4)

14-22: Schema 설명을 더 자세히 보완하면 좋을 것 같습니다.

현재 설명이 명확하지만, 다음과 같은 추가 정보를 포함하면 더 유용할 것 같습니다:

  • taskType의 가능한 값들 (ATTENDANCE, ASSIGNMENT)
  • deadLine이 null일 수 있는 경우

예시:

-@Schema(description = "할일 타입") StudyTaskType taskType,
+@Schema(description = "할일 타입 (ATTENDANCE: 출석, ASSIGNMENT: 과제)") StudyTaskType taskType,
-@Schema(description = "마감 시각") LocalDateTime deadLine,
+@Schema(description = "마감 시각 (취소된 경우 null)") LocalDateTime deadLine,

Line range hint 23-42: 매개변수 유효성 검사를 추가하면 좋을 것 같습니다.

팩토리 메서드들이 잘 구현되어 있지만, 다음과 같은 유효성 검사를 추가하면 더 안전할 것 같습니다:

 public static StudyTaskResponse createAttendanceType(StudyDetail studyDetail, LocalDate now, boolean isAttended) {
+    if (studyDetail == null || now == null) {
+        throw new IllegalArgumentException("필수 매개변수가 null일 수 없습니다");
+    }
     // ... 나머지 코드
 }

 public static StudyTaskResponse createAssignmentType(StudyDetail studyDetail, AssignmentHistory assignmentHistory) {
+    if (studyDetail == null) {
+        throw new IllegalArgumentException("studyDetail은 null일 수 없습니다");
+    }
     // ... 나머지 코드
 }

Also applies to: 44-64


66-72: 메서드 문서화를 추가하면 좋을 것 같습니다.

간단한 메서드이지만, 문서화를 추가하면 더 명확할 것 같습니다.

+/**
+ * @return 현재 응답이 출석 타입인지 여부
+ */
 public boolean isAttendance() {
     return taskType == ATTENDANCE;
 }

+/**
+ * @return 현재 응답이 과제 타입인지 여부
+ */
 public boolean isAssignment() {
     return taskType == ASSIGNMENT;
 }

Line range hint 76-81: 열거형 문서화를 추가하면 좋을 것 같습니다.

열거형과 각 상수에 대한 문서화를 추가하면 더 명확할 것 같습니다.

+/**
+ * 스터디 과제의 유형을 나타내는 열거형
+ */
 public enum StudyTaskType {
+    /** 출석 체크 유형 */
     ATTENDANCE("출석"),
+    /** 과제 제출 유형 */
     ASSIGNMENT("과제");

     private final String value;
 }
src/main/java/com/gdschongik/gdsc/domain/study/dto/response/StudyStudentResponse.java (2)

65-73: 헬퍼 메서드의 가독성을 개선할 수 있습니다.

메서드의 기능은 정확하지만, 람다 표현식을 더 간단하게 만들 수 있습니다.

다음과 같이 변수명을 더 명확하게 수정하는 것을 제안합니다:

 private static long countAssignmentByStatus(
     List<StudyTaskResponse> assignments, AssignmentSubmissionStatusResponse status) {
     return assignments.stream()
-        .filter(studyTaskResponse -> studyTaskResponse.assignmentSubmissionStatus() == status)
+        .filter(task -> task.assignmentSubmissionStatus() == status)
         .count();
 }

 private static long countAttendanceByStatus(
     List<StudyTaskResponse> attendances, AttendanceStatusResponse status) {
     return attendances.stream()
-        .filter(studyTaskResponse -> studyTaskResponse.attendanceStatus() == status)
+        .filter(task -> task.attendanceStatus() == status)
         .count();
 }

API 엔드포인트와 메서드 이름에서 'Todo'가 'Task'로 완전히 변경되지 않았습니다.

다음 위치에서 일관성 없는 네이밍이 발견되었습니다:

  • StudentStudyDetailController.java:

    • /todo 엔드포인트 경로
    • getStudyTodoList() 메서드명
  • StudentStudyDetailService.java:

    • getStudyTodoList() 메서드명

응답 DTO는 StudyTaskResponse로 변경되었지만, 관련 API 엔드포인트와 메서드 이름은 여전히 'Todo'를 사용하고 있어 일관성이 없습니다. 이러한 불일치는 코드의 가독성과 유지보수성을 저하시킬 수 있습니다.

🔗 Analysis chain

Line range hint 1-80: 전체 코드베이스에서 이름 변경이 일관되게 적용되었는지 확인이 필요합니다.

StudyTodoResponse에서 StudyTaskResponse로의 변경이 다른 관련 클래스에서도 올바르게 적용되었는지 확인해야 합니다.

다음 스크립트를 실행하여 변경되지 않은 부분이 있는지 확인하겠습니다:


Let's analyze the usage patterns to ensure consistency in the naming changes.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify complete migration from todo to task naming
echo "Checking for remaining 'todo' references..."
rg -i "todo" --type java

echo "Checking for any mismatched StudyTodoResponse usage..."
rg "StudyTodoResponse" --type java

Length of output: 3912


Script:

#!/bin/bash
# Let's check for any references to StudyTask and related patterns
echo "Checking StudyTask related patterns..."
rg "StudyTask" --type java -A 2

echo "Checking specific method names and patterns..."
rg "getStudyTodo|StudyTodo" --type java -A 2

Length of output: 14860

src/main/java/com/gdschongik/gdsc/global/util/ExcelUtil.java (1)

93-102: 스트림 연산이 올바르게 업데이트되었습니다.

StudyTaskResponse로의 변경이 잘 이루어졌으며, 기존 로직이 정상적으로 유지되고 있습니다.

다만, 가독성 향상을 위해 스트림 체인을 좀 더 명확하게 분리하는 것을 고려해보시는 건 어떨까요?

다음과 같은 형태로 리팩토링하면 더 읽기 쉬울 것 같습니다:

-            student.studyTasks().stream()
-                    .filter(StudyTaskResponse::isAssignment)
-                    .forEach(task -> studentRow
-                            .createCell(cellIndex.getAndIncrement())
-                            .setCellValue(task.assignmentSubmissionStatus().getValue()));
+            student.studyTasks().stream()
+                    .filter(StudyTaskResponse::isAssignment)
+                    .forEach(task -> {
+                        Cell cell = studentRow.createCell(cellIndex.getAndIncrement());
+                        cell.setCellValue(task.assignmentSubmissionStatus().getValue());
+                    });
src/main/java/com/gdschongik/gdsc/domain/study/application/MentorStudyService.java (1)

스터디 관련 메서드와 변수명에서 'todo' 참조가 여전히 존재합니다.

아래 위치에서 'todo'를 'task'로 변경이 필요합니다:

  • StudentStudyDetailController.java: getStudyTodoList() 메서드명
  • StudentStudyDetailService.java: getStudyTodoList() 메서드명

테스트 파일에서 발견된 'TODO' 주석은 실제 코드 로직과 관련이 없는 일반적인 개발자 주석이므로 현재 리네이밍 작업과는 무관합니다.

🔗 Analysis chain

Line range hint 90-235: 전체적인 리네이밍 작업이 잘 수행되었습니다만, 추가 검증이 필요합니다.

코드베이스 전체에서 이전 이름인 'todo'가 완전히 제거되었는지 확인이 필요합니다.

다음 스크립트를 실행하여 남아있는 'todo' 관련 참조를 확인해주세요:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify if there are any remaining references to 'todo' in the codebase
# Expected: No occurrences of StudyTodoResponse or related 'todo' terms in variable names

echo "Checking for remaining 'todo' references..."
rg -i "studytodo|todo.*response" --type java

echo "Checking for potentially missed renames in test files..."
rg -i "todo" --type java --type kotlin --glob "**/*Test.{java,kt}"

Length of output: 1285

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 014cf8c and 5cca03d.

📒 Files selected for processing (6)
  • src/main/java/com/gdschongik/gdsc/domain/study/api/StudentStudyDetailController.java (2 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/application/MentorStudyService.java (3 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/application/StudentStudyDetailService.java (1 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/dto/response/StudyStudentResponse.java (4 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/dto/response/StudyTaskResponse.java (6 hunks)
  • src/main/java/com/gdschongik/gdsc/global/util/ExcelUtil.java (2 hunks)
🔇 Additional comments (10)
src/main/java/com/gdschongik/gdsc/domain/study/api/StudentStudyDetailController.java (2)

7-7: import 문이 올바르게 변경되었습니다.

리팩토링 목적에 맞게 StudyTaskResponse로 정상적으로 변경되었습니다.


36-38: 서비스 계층과 다른 파일들의 변경 사항을 확인해주세요.

메서드 이름 변경에 따른 영향도를 확인해야 합니다.

다음 스크립트를 실행하여 관련 메서드 호출을 확인해주세요:

✅ Verification successful

변경이 필요한 부분이 없음을 확인했습니다.

getStudyTodoList 메서드는 다음 파일들에서만 일관되게 사용되고 있습니다:

  • StudentStudyDetailController: API 엔드포인트와 서비스 호출
  • StudentStudyDetailService: 서비스 구현

나머지 'todo' 관련 용어들은 모두 주석에서 사용된 TODO 마커이며, 실제 코드의 메서드나 변수명과는 관련이 없습니다.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of getStudyTodoList method and todo-related terms

# Check for getStudyTodoList method calls
echo "Checking for getStudyTodoList method usage:"
rg "getStudyTodoList"

# Check for remaining todo terms in study-related files
echo -e "\nChecking for remaining 'todo' terms in study-related files:"
rg -i "todo" "src/main/java/com/gdschongik/gdsc/domain/study"

Length of output: 2273

src/main/java/com/gdschongik/gdsc/domain/study/dto/response/StudyTaskResponse.java (1)

3-4: 변경된 이름이 일관성 있게 적용되었습니다.

정적 임포트와 클래스 이름이 올바르게 변경되었습니다.

Also applies to: 14-14

src/main/java/com/gdschongik/gdsc/domain/study/dto/response/StudyStudentResponse.java (2)

Line range hint 5-24: 필드 이름 변경이 일관성 있게 적용되었습니다.

todo에서 task로의 이름 변경이 import문과 레코드 필드에 올바르게 반영되었습니다.


Line range hint 28-53: 팩토리 메서드의 매개변수와 내부 로직이 적절히 수정되었습니다.

StudyTaskResponse로의 타입 변경과 함께 스트림 필터링 조건도 올바르게 수정되었습니다.

src/main/java/com/gdschongik/gdsc/global/util/ExcelUtil.java (2)

11-11: import 문이 올바르게 변경되었습니다.

StudyTodoResponse에서 StudyTaskResponse로의 import 변경이 PR의 목적에 맞게 잘 이루어졌습니다.


93-102: 코드베이스 전체의 이름 변경 검증이 필요합니다.

StudyTodoResponse에서 StudyTaskResponse로의 변경이 다른 파일에서도 모두 올바르게 이루어졌는지 확인이 필요합니다.

✅ Verification successful

Let me check the actual content of these files to ensure the renaming was done consistently.


Let me check the remaining StudyTodo references to ensure they are just in method/endpoint names and not in actual implementation.


이름 변경이 올바르게 완료되었습니다

StudyTodoResponse에서 StudyTaskResponse로의 변경이 코드베이스 전체에서 일관되게 이루어졌습니다. 남아있는 "StudyTodo" 문자열은 단순히 API 엔드포인트 경로(/todo)와 메서드 이름(getStudyTodoList)에만 존재하며, 이는 비즈니스 로직상의 명명이므로 변경이 불필요합니다.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# StudyTodoResponse 참조가 남아있는지 확인
echo "Checking for remaining StudyTodoResponse references..."
rg "StudyTodoResponse" -l

# StudyTaskResponse로 변경된 파일들 확인
echo "Checking files with StudyTaskResponse..."
rg "StudyTaskResponse" -l

Length of output: 749


Script:

#!/bin/bash
# Check the content of files containing StudyTaskResponse
echo "=== Checking StudyTaskResponse usage in files ==="
for file in $(rg "StudyTaskResponse" -l); do
  echo "=== $file ==="
  rg "StudyTaskResponse" -C 2 "$file"
done

# Double check for any remaining StudyTodo (without Response) references
echo -e "\n=== Checking for any remaining StudyTodo references ==="
rg "StudyTodo" -l

Length of output: 10107


Script:

#!/bin/bash
# Check the actual content of files with StudyTodo references
echo "=== Checking content of files with StudyTodo references ==="
for file in $(rg "StudyTodo" -l); do
  echo "=== $file ==="
  rg "StudyTodo" -C 3 "$file"
done

Length of output: 1538

src/main/java/com/gdschongik/gdsc/domain/study/application/StudentStudyDetailService.java (2)

65-65: 리팩토링이 정상적으로 적용되었습니다.

StudyTodoResponse에서 StudyTaskResponse로의 변경이 다음 부분에서 올바르게 적용되었습니다:

  • 응답 리스트 타입 (65번 줄)
  • 출석 타입 생성 (70-71번 줄)
  • 과제 타입 생성 (77-78번 줄)

코드의 기능적 변경 없이 이름만 변경되어 안전한 리팩토링이 이루어졌습니다.

Also applies to: 70-71, 77-78


57-80: 변경된 응답 타입의 사용처 확인이 필요합니다.

이 서비스를 사용하는 컨트롤러나 다른 컴포넌트들에서도 타입 변경이 필요할 수 있습니다.

다음 스크립트로 관련 사용처를 확인해보겠습니다:

✅ Verification successful

Let me analyze the search results. I can see that:

  1. There is no StudyTodoResponse found in the codebase, indicating this is likely a new response type.
  2. StudyTaskResponse is properly used across the codebase in:
    • The DTO definition itself
    • StudentStudyDetailController
    • StudentStudyDetailService
    • MentorStudyService
    • StudyStudentResponse
    • ExcelUtil

Since we can see all the usages and they are consistent, I can generate the final response:

응답 타입이 올바르게 사용되고 있습니다.

코드베이스 전체를 검토한 결과, StudyTaskResponse 타입이 모든 관련 컴포넌트에서 일관되게 사용되고 있습니다. 추가적인 타입 변경이 필요하지 않습니다.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of StudyTaskResponse in other components

# Test: Search for StudyTodoResponse usage that might need to be updated
echo "Searching for potential StudyTodoResponse usages that need to be updated..."
rg "StudyTodoResponse" --type java

# Test: Verify StudyTaskResponse is properly used in controllers
echo "Verifying StudyTaskResponse usage in controllers..."
rg "StudyTaskResponse" --type java

Length of output: 5638

src/main/java/com/gdschongik/gdsc/domain/study/application/MentorStudyService.java (1)

24-24: import 문이 올바르게 업데이트되었습니다.

StudyTodoResponse에서 StudyTaskResponse로의 변경이 정확하게 반영되었습니다.

Comment on lines +36 to 38
public ResponseEntity<List<StudyTaskResponse>> getStudyTodoList(@RequestParam(name = "studyId") Long studyId) {
List<StudyTaskResponse> response = studentStudyDetailService.getStudyTodoList(studyId);
return ResponseEntity.ok(response);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

메서드 이름도 함께 변경이 필요합니다.

반환 타입은 StudyTaskResponse로 변경되었지만, 메서드 이름은 여전히 getStudyTodoList를 사용하고 있습니다. 일관성을 위해 메서드 이름도 변경하는 것이 좋겠습니다.

다음과 같이 변경을 제안드립니다:

-    public ResponseEntity<List<StudyTaskResponse>> getStudyTodoList(@RequestParam(name = "studyId") Long studyId) {
-        List<StudyTaskResponse> response = studentStudyDetailService.getStudyTodoList(studyId);
+    public ResponseEntity<List<StudyTaskResponse>> getStudyTaskList(@RequestParam(name = "studyId") Long studyId) {
+        List<StudyTaskResponse> response = studentStudyDetailService.getStudyTaskList(studyId);
         return ResponseEntity.ok(response);
     }

또한, @Operation 어노테이션의 summary와 description도 함께 업데이트하는 것이 좋겠습니다:

-    @Operation(summary = "내 할일 리스트 조회", description = "해당 스터디의 내 할일 리스트를 조회합니다")
+    @Operation(summary = "내 작업 리스트 조회", description = "해당 스터디의 내 작업 리스트를 조회합니다")

Committable suggestion was skipped due to low confidence.

@@ -54,27 +54,27 @@ studyDetail, getSubmittedAssignment(assignmentHistories, studyDetail)))
}

@Transactional(readOnly = true)
public List<StudyTodoResponse> getStudyTodoList(Long studyId) {
public List<StudyTaskResponse> getStudyTodoList(Long studyId) {
Copy link

@coderabbitai coderabbitai bot Oct 29, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

메서드 이름도 일관성 있게 변경이 필요합니다.

getStudyTodoList 메서드의 반환 타입은 StudyTaskResponse로 변경되었지만, 메서드 이름은 여전히 "Todo"를 포함하고 있습니다. 일관성을 위해 메서드 이름도 변경하는 것이 좋겠습니다.

다음과 같이 수정을 제안합니다:

-public List<StudyTaskResponse> getStudyTodoList(Long studyId)
+public List<StudyTaskResponse> getStudyTaskList(Long studyId)
📝 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.

Suggested change
public List<StudyTaskResponse> getStudyTodoList(Long studyId) {
public List<StudyTaskResponse> getStudyTaskList(Long studyId) {

Copy link
Member

Choose a reason for hiding this comment

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

@Sangwook02 이거 체크점여

Copy link

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link
Member Author

Choose a reason for hiding this comment

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

이건 api가 '/todo'라서 뒀어요.
실제로 api 용도도 todo 조회가 맞아서 그냥 둬도 될 것 같습니다~

Comment on lines +90 to +98
List<StudyTaskResponse> studyTasks = new ArrayList<>();
studyDetails.forEach(studyDetail -> {
studyTodos.add(StudyTodoResponse.createAttendanceType(
studyTasks.add(StudyTaskResponse.createAttendanceType(
studyDetail, LocalDate.now(), isAttended(currentAttendances, studyDetail)));
studyTodos.add(StudyTodoResponse.createAssignmentType(
studyTasks.add(StudyTaskResponse.createAssignmentType(
studyDetail, getSubmittedAssignment(currentAssignmentHistories, studyDetail)));
});

response.add(StudyStudentResponse.of(studyHistory, currentStudyAchievements, studyTodos));
response.add(StudyStudentResponse.of(studyHistory, currentStudyAchievements, studyTasks));
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

StudyTaskResponse로의 변경이 일관성 있게 적용되었습니다.

변수명과 메서드 호출이 새로운 명명 규칙에 맞게 잘 변경되었습니다. 다만, 아래 사항을 확인해 주시면 좋겠습니다:

  1. 코드의 가독성을 위해 studyTasks.add() 호출을 별도의 메서드로 추출하는 것을 고려해 보세요.
  2. 현재 구현에서는 매번 새로운 ArrayList를 생성하고 있는데, 초기 용량을 지정하여 성능을 개선할 수 있습니다.

다음과 같은 리팩토링을 제안드립니다:

-List<StudyTaskResponse> studyTasks = new ArrayList<>();
+List<StudyTaskResponse> studyTasks = new ArrayList<>(studyDetails.size() * 2);  // 각 studyDetail마다 2개의 태스크가 생성되므로

+private void addStudyTasks(List<StudyTaskResponse> studyTasks, StudyDetail studyDetail,
+                          List<Attendance> currentAttendances,
+                          List<AssignmentHistory> currentAssignmentHistories) {
+    studyTasks.add(StudyTaskResponse.createAttendanceType(
+            studyDetail, LocalDate.now(), isAttended(currentAttendances, studyDetail)));
+    studyTasks.add(StudyTaskResponse.createAssignmentType(
+            studyDetail, getSubmittedAssignment(currentAssignmentHistories, studyDetail)));
+}

Committable suggestion was skipped due to low confidence.

Comment on lines +227 to +235
List<StudyTaskResponse> studyTasks = new ArrayList<>();
studyDetails.forEach(studyDetail -> {
studyTodos.add(StudyTodoResponse.createAttendanceType(
studyTasks.add(StudyTaskResponse.createAttendanceType(
studyDetail, LocalDate.now(), isAttended(currentAttendances, studyDetail)));
studyTodos.add(StudyTodoResponse.createAssignmentType(
studyTasks.add(StudyTaskResponse.createAssignmentType(
studyDetail, getSubmittedAssignment(currentAssignmentHistories, studyDetail)));
});

content.add(StudyStudentResponse.of(studyHistory, currentStudyAchievements, studyTodos));
content.add(StudyStudentResponse.of(studyHistory, currentStudyAchievements, studyTasks));
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

createStudyExcel 메서드의 변경 사항이 getStudyStudents와 중복됩니다.

두 메서드에서 동일한 로직이 반복되고 있습니다. 코드 중복을 제거하고 재사용성을 높이기 위해 공통 로직을 별도의 메서드로 추출하는 것이 좋겠습니다.

다음과 같은 리팩토링을 제안드립니다:

+private List<StudyTaskResponse> createStudyTasks(
+        StudyDetail studyDetail,
+        List<Attendance> currentAttendances,
+        List<AssignmentHistory> currentAssignmentHistories) {
+    List<StudyTaskResponse> studyTasks = new ArrayList<>(2);
+    studyTasks.add(StudyTaskResponse.createAttendanceType(
+            studyDetail, LocalDate.now(), isAttended(currentAttendances, studyDetail)));
+    studyTasks.add(StudyTaskResponse.createAssignmentType(
+            studyDetail, getSubmittedAssignment(currentAssignmentHistories, studyDetail)));
+    return studyTasks;
+}

이 메서드를 getStudyStudentscreateStudyExcel 양쪽에서 재사용할 수 있습니다.

Committable suggestion was skipped due to low confidence.

Copy link
Member

@uwoobeat uwoobeat left a comment

Choose a reason for hiding this comment

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

마이너한 제안사항 첨부합니다
미리 어푸할게요

@@ -54,27 +54,27 @@ studyDetail, getSubmittedAssignment(assignmentHistories, studyDetail)))
}

@Transactional(readOnly = true)
public List<StudyTodoResponse> getStudyTodoList(Long studyId) {
public List<StudyTaskResponse> getStudyTodoList(Long studyId) {
Copy link
Member

Choose a reason for hiding this comment

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

@Sangwook02 이거 체크점여

Long studyDetailId,
@Schema(description = "현 주차수") Long week,
@Schema(description = "할일 타입") StudyTodoType todoType,
@Schema(description = "할일 타입") StudyTaskType taskType,
Copy link
Member

Choose a reason for hiding this comment

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

Todo -> Task로 네이밍 변경했다면 "할일"을 사용하는 한글 네이밍도 변경해주면 좋을듯 합니다

Copy link

Job Summary for Gradle

Check Style and Test to Develop :: build-test
Gradle Root Project Requested Tasks Gradle Version Build Outcome Build Scan®
gdsc check 8.5 Build Scan published

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (1)
src/main/java/com/gdschongik/gdsc/domain/study/dto/response/StudyTaskResponse.java (1)

Line range hint 76-81: 열거형 상수의 한글 설명을 더 구체적으로 개선하면 좋을 것 같습니다.

현재 "출석"과 "과제"로 되어있는 설명을 "출석 확인", "과제 제출" 등으로 더 명확하게 표현하면 어떨까요?

 public enum StudyTaskType {
-    ATTENDANCE("출석"),
-    ASSIGNMENT("과제");
+    ATTENDANCE("출석 확인"),
+    ASSIGNMENT("과제 제출");

     private final String value;
 }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 5cca03d and 9780bf0.

📒 Files selected for processing (1)
  • src/main/java/com/gdschongik/gdsc/domain/study/dto/response/StudyTaskResponse.java (6 hunks)
🔇 Additional comments (4)
src/main/java/com/gdschongik/gdsc/domain/study/dto/response/StudyTaskResponse.java (4)

3-4: 정적 임포트 변경이 올바르게 적용되었습니다.

StudyTodoType에서 StudyTaskType으로의 변경이 일관성 있게 반영되었습니다.


14-21: 이전 리뷰 의견에 동의합니다.

이전 리뷰어가 지적한 대로, Todo에서 Task로 변경된 만큼 한글 설명도 일관성 있게 변경되어야 합니다.


Line range hint 23-63: 팩토리 메서드들이 잘 구현되어 있습니다.

취소된 경우의 처리와 정상적인 경우의 처리가 명확하게 구분되어 있으며, 각 필드의 null 처리가 적절합니다.


65-72: 유틸리티 메서드들이 간결하고 명확합니다.

isAttendanceisAssignment 메서드가 단일 책임 원칙을 잘 준수하고 있습니다.

Copy link
Member

@kckc0608 kckc0608 left a comment

Choose a reason for hiding this comment

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

lgtm

@Sangwook02 Sangwook02 merged commit 6460f58 into develop Oct 30, 2024
1 check passed
@Sangwook02 Sangwook02 deleted the refactor/795-rename-study-todo-response branch October 30, 2024 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
♻️ refactor 기능 개선
Projects
None yet
Development

Successfully merging this pull request may close these issues.

♻️ StudyTodoResponse 이름 변경
3 participants