Skip to content

Conversation

@minzziPark
Copy link
Collaborator

작업 사항

중복으로 사용되던 formatStudyTime 함수를 별도의 유틸 함수로 분리했습니다.
이 함수는 minute을 입력받아 00시간 00분 형식의 문자열을 반환합니다.

기존에는 아래 네 파일에서 동일한 로직이 중복으로 정의되어 있었습니다.

  • src/pages/Admin/ReportList/Page.tsx
  • src/pages/Admin/ReportList/components/GroupReportListView.tsx
  • src/pages/ReportDetail/Page.tsx
  • src/pages/ReportList/Page.tsx

중복 코드를 제거하고 재사용성을 높이기 위해 공통 유틸로 분리했습니다.

Naming

기존 함수명 formatStudyTime은 반환 형태가 명확하지 않고, Study라는 도메인 단어가 포함되어 있어 범용적인 활용이 어렵다고 판단했습니다.

이에 따라 함수명을 아래와 같이 변경했습니다.
formatTimeToHoursAndMinutes

관련 이슈

시간이나 분이 0인 경우는 생략되지 않고 보여집니다. (ex. 0시간 32분, 3시간 0분)
0일때 보여지는 것은 의도된건지 궁금합니다. @ohinhyuk

Copilot

Please review this PR as if you were a senior developer and provide constructive feedback on code quality, structure, and naming.

close #157

@minzziPark minzziPark requested a review from Copilot October 22, 2025 08:53
@coderabbitai
Copy link

coderabbitai bot commented Oct 22, 2025

Summary by CodeRabbit

  • 리팩터링
    • 보고서 페이지 전반에서 시간 포맷팅 유틸리티를 통합했습니다. 여러 페이지의 분산된 포맷팅 로직을 단일 공유 유틸리티로 통합하여 코드 일관성을 개선했습니다.

진행 상황

여러 페이지 컴포넌트에서 로컬로 정의된 시간 포맷팅 함수를 공유 유틸리티 함수로 대체하고, 새로운 포맷팅 유틸리티 파일을 추가했습니다.

변경 사항

코호트 / 파일 변경 요약
시간 포맷팅 유틸리티 추가
src/utils/formatTimeToHoursAndMinutes.ts
분 단위를 "X시간 Y분" 형식의 문자열로 변환하는 새로운 유틸리티 함수 추가
로컬 포맷팅 함수를 유틸로 대체
src/pages/Admin/ReportList/Page.tsx, src/pages/Admin/ReportList/components/GroupReportListView.tsx, src/pages/ReportDetail/Page.tsx, src/pages/ReportList/Page.tsx
로컬 formatStudyTime 함수 제거 및 formatTimeToHoursAndMinutes 유틸리티 임포트 추가. 모든 렌더링 경로에서 새로운 유틸리티 함수 사용으로 업데이트

Pre-merge checks

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed 제목 "중복된 formatStudyTime 함수 유틸화"는 PR의 핵심 변경 내용을 명확하고 간결하게 설명합니다. 네 개의 파일에서 중복되던 시간 포맷팅 함수를 별도의 유틸 함수로 분리한다는 주요 목표를 정확히 전달하며, 변경의 의도와 범위가 명확합니다.
Out of Scope Changes Check ✅ Passed PR의 모든 변경 사항이 이슈 #157의 목표와 직접 관련이 있으며 범위 내입니다. 새로운 유틸 파일 생성, 네 파일의 중복 함수 제거 및 새 유틸 함수 import로의 전환은 모두 코드 중복 제거라는 명확한 목표를 달성하기 위한 변경입니다. 데이터 조회, 상태 관리, 네비게이션 로직 등 다른 영역의 변경은 없습니다.
Description Check ✅ Passed PR 설명은 변경 사항과 명확히 관련이 있으며, 상세한 정보를 제공합니다. 작업 사항, 기존 중복 코드의 위치, 함수명 변경의 이유, 그리고 새로운 함수의 동작 방식을 설명하고 있으며, 이슈 #157과 연결되어 있습니다.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cdedeb3 and 3fb5f32.

📒 Files selected for processing (5)
  • src/pages/Admin/ReportList/Page.tsx (2 hunks)
  • src/pages/Admin/ReportList/components/GroupReportListView.tsx (3 hunks)
  • src/pages/ReportDetail/Page.tsx (2 hunks)
  • src/pages/ReportList/Page.tsx (3 hunks)
  • src/utils/formatTimeToHoursAndMinutes.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/pages/Admin/ReportList/components/GroupReportListView.tsx (1)
src/utils/formatTimeToHoursAndMinutes.ts (1)
  • formatTimeToHoursAndMinutes (1-5)
src/pages/ReportList/Page.tsx (1)
src/utils/formatTimeToHoursAndMinutes.ts (1)
  • formatTimeToHoursAndMinutes (1-5)
src/pages/Admin/ReportList/Page.tsx (1)
src/utils/formatTimeToHoursAndMinutes.ts (1)
  • formatTimeToHoursAndMinutes (1-5)
src/pages/ReportDetail/Page.tsx (1)
src/utils/formatTimeToHoursAndMinutes.ts (1)
  • formatTimeToHoursAndMinutes (1-5)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: e2e-test
🔇 Additional comments (6)
src/pages/ReportDetail/Page.tsx (2)

15-15: 깔끔한 유틸리티 함수 임포트입니다.

중복 제거를 위해 공통 유틸리티 함수를 추가한 것은 좋은 리팩토링입니다.


143-143: 안전한 사용 패턴입니다.

|| 0 폴백 값을 사용하여 undefined 케이스를 처리한 것이 좋습니다.

src/pages/Admin/ReportList/Page.tsx (1)

8-8: 중복 제거가 잘 이루어졌습니다.

공통 유틸리티 함수로 대체하여 코드 중복을 제거한 것이 좋습니다. 모바일과 데스크톱 뷰 모두에서 일관되게 적용되었습니다.

Also applies to: 84-84, 91-91

src/pages/Admin/ReportList/components/GroupReportListView.tsx (1)

8-8: 일관된 리팩토링이 적용되었습니다.

공통 유틸리티 함수로 잘 교체되었습니다. 반응형 레이아웃의 서로 다른 브레이크포인트에서 일관되게 사용되고 있습니다.

Also applies to: 70-70, 81-81

src/pages/ReportList/Page.tsx (1)

10-10: 리팩토링이 성공적으로 완료되었습니다.

공통 유틸리티 함수로 잘 대체되었습니다. 이로써 4개 파일에 분산되어 있던 중복 코드가 하나의 공통 함수로 통합되었습니다.

Also applies to: 93-93, 104-104

src/utils/formatTimeToHoursAndMinutes.ts (1)

1-5: 영이 포함된 경우의 출력 형식을 확인해주세요.

현재 함수는 모든 사용처에서 report.totalMinutes를 전달받으므로 음수나 NaN 입력은 실제로 발생하지 않습니다. 다만 영(0) 값이 포함될 경우 "0시간 0분"이 표시되는 점은 의도된 동작인지 확인이 필요합니다.

예를 들어:

  • 0 → 현재: "0시간 0분" (의도된 표시인가요?)
  • 32 → 현재: "0시간 32분" (분만 표시하는 것이 더 자연스러울까요?)
  • 120 → 현재: "2시간 0분" (시간만 표시하는 것이 더 자연스러울까요?)

PR 설명에서 언급하신 영값 처리 방식을 @ohinhyuk님과 함께 확인한 후, 필요시 다음과 같이 개선할 수 있습니다:

export function formatTimeToHoursAndMinutes(minutes: number) {
  const hours = Math.floor(minutes / 60);
  const mins = minutes % 60;
+ if (hours === 0) return `${mins}분`;
+ if (mins === 0) return `${hours}시간`;
  return `${hours}시간 ${mins}분`;
}

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR extracts a duplicated formatStudyTime function into a reusable utility function formatTimeToHoursAndMinutes. The function converts minutes into a Korean-formatted time string (e.g., "3시간 45분").

Key changes:

  • Created a new utility function formatTimeToHoursAndMinutes in src/utils/formatTimeToHoursAndMinutes.ts
  • Removed duplicate implementations from 4 files and replaced them with imports of the new utility
  • Improved function naming from domain-specific formatStudyTime to more generic formatTimeToHoursAndMinutes

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/utils/formatTimeToHoursAndMinutes.ts New utility file containing the extracted time formatting function
src/pages/ReportList/Page.tsx Removed local function, imported and used the new utility
src/pages/ReportDetail/Page.tsx Removed local function, imported and used the new utility
src/pages/Admin/ReportList/components/GroupReportListView.tsx Removed local function, imported and used the new utility
src/pages/Admin/ReportList/Page.tsx Removed local function, imported and used the new utility

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +1 to +5
export function formatTimeToHoursAndMinutes(minutes: number) {
const hours = Math.floor(minutes / 60);
const mins = minutes % 60;
return `${hours}시간 ${mins}분`;
}
Copy link

Copilot AI Oct 22, 2025

Choose a reason for hiding this comment

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

The function does not validate input. Consider adding a guard clause to handle negative numbers or non-integer values, e.g., if (minutes < 0 || !Number.isFinite(minutes)) throw new Error('Invalid minutes value');

Copilot uses AI. Check for mistakes.
@@ -0,0 +1,5 @@
export function formatTimeToHoursAndMinutes(minutes: number) {
Copy link

Copilot AI Oct 22, 2025

Choose a reason for hiding this comment

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

Add JSDoc documentation to describe the function's purpose, parameter, and return value. For example: /** Formats minutes into Korean time string format (e.g., '3시간 45분') */

Copilot uses AI. Check for mistakes.
@ohinhyuk
Copy link
Contributor

ohinhyuk commented Oct 22, 2025

@minzziPark
작업 하느라 고생하셨습니다!
함수명이 직관적으로 변경되고 공용화를 통해 중복 코드가 줄어들어 좋네요.

그리고 0시간 32분 처럼 시간이 0일 때도 나타나는 것이 의도한 것인지 질문을 주셨는데,
해당 부분은 시간 표시 형식을 일치시키려고 의도한 것이 맞습니다.

감사합니다.

@ohinhyuk ohinhyuk added the refactoring 리팩터링 label Oct 22, 2025
@ohinhyuk ohinhyuk changed the base branch from main to dev-refactoring October 22, 2025 10:48
@ohinhyuk ohinhyuk merged commit c0fffea into dev-refactoring Oct 22, 2025
2 checks passed
@minzziPark minzziPark deleted the refactor/157-add-utility-function-to-format-study-time branch November 1, 2025 04:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactoring 리팩터링

Projects

None yet

Development

Successfully merging this pull request may close these issues.

formatStudyTime 공용 함수화

3 participants