Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughMyBookmark/Remind의 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as 사용자
participant P as Page (MyBookmark/Remind)
participant M as CardEditModal
participant D as datetime 유틸
U->>P: 편집 버튼 클릭
P->>P: articleDetail 설정/변경
Note right of P #D6EAF8: 렌더 시 key = articleDetail.id
P->>M: render CardEditModal (key=articleDetail.id, articleDetail)
alt articleDetail 변경 → key 변경
Note over M #D5F5E3: React가 컴포넌트 재마운트 수행
M->>D: combineDateTime(date, time)
D-->>M: 정규화된 datetime 또는 null
M-->>P: 최신 articleDetail 기반 폼 초기화
else key 동일
M-->>P: 기존 상태 유지
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)apps/client/src/shared/components/cardEditModal/CardEditModal.tsx (1)
🔇 Additional comments (1)
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. Comment |
|
✅ Storybook chromatic 배포 확인: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/client/src/shared/apis/axios.ts (1)
26-28: 요청 URL에서 불필요한?제거 제안현재
'/api/v1/users/acorns?'형태로 남아 있어서 Axios가 최종적으로.../acorns?&now=...같은 문자열을 만들 가능성이 있습니다. 대부분의 서버는 처리하지만 쓸데없는 빈 쿼리 파라미터가 붙어 가독성이 떨어질 수 있으니?를 제거해 두는 편이 깔끔합니다.- const { data } = await apiRequest.get('/api/v1/users/acorns?', { + const { data } = await apiRequest.get('/api/v1/users/acorns', { params: { now }, });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/client/src/pages/myBookmark/MyBookmark.tsx(1 hunks)apps/client/src/pages/remind/Remind.tsx(1 hunks)apps/client/src/shared/apis/axios.ts(1 hunks)
⏰ 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: storybook
🔇 Additional comments (2)
apps/client/src/pages/remind/Remind.tsx (1)
195-198: 모달 상태 초기화 잘 처리됐습니다
key={articleDetail.id}덕분에 아티클을 바꿔 열 때마다CardEditModal이 재마운트되어 내부 상태가 초기화됩니다. 기대한 이슈 방지에 적절한 접근입니다.apps/client/src/pages/myBookmark/MyBookmark.tsx (1)
244-247: 동일한 키 전략으로 북마크 편집도 안전해졌습니다여기서도
key={articleDetail.id}를 부여해 편집 모달이 다른 아티클로 전환될 때마다 초기화되도록 한 점이 좋습니다. Remind 페이지와 일관성 있게 동작하겠습니다.
jjangminii
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
apps/client/src/shared/utils/datetime.ts (1)
13-27: to24Hour: 로케일/포맷 다양성 보강 제안 (AM/PM, 초 지원, 느슨한 공백)현재 정규식은 "오전/오후 HH:mm"만 처리합니다. AM/PM, 초(SS), 공백 변형까지 허용하면 호환성이 높아집니다. 또한 입력이 빈 문자열일 때의 처리도 빠른 리턴으로 명시하는 편이 안전합니다.
아래와 같이 보강을 제안합니다:
-export const to24Hour = (time: string) => { - const match = time.match(/(오전|오후)\s(\d{1,2}):(\d{2})/); - if (!match) return time; - - const [, period, hourStr, minute] = match; - let hour = parseInt(hourStr, 10); - - if (period === '오전' && hour === 12) { - hour = 0; - } else if (period === '오후' && hour !== 12) { - hour += 12; - } - - return `${hour.toString().padStart(2, '0')}:${minute}`; -}; +export const to24Hour = (time: string) => { + if (!time) return ''; + // 허용 예: "오전 9:05", "오후10:30", "AM 9:05", "pm 12:30:10" + const match = time.match( + /^(?:(오전|오후)|(?:(AM|PM)))\s?(\d{1,2}):(\d{2})(?::(\d{2}))?$/i + ); + if (!match) return time; + + const [, koPeriod, enPeriod, hourStr, minute, second] = match; + const isPM = (koPeriod === '오후') || (enPeriod?.toLowerCase() === 'pm'); + let hour = parseInt(hourStr, 10) % 12; // 12 → 0으로 정규화 + if (isPM) hour += 12; + const hh = hour.toString().padStart(2, '0'); + const ss = second ? `:${second}` : ''; + return `${hh}:${minute}${ss}`; +};apps/client/src/shared/components/cardEditModal/CardEditModal.tsx (2)
22-24: date/time 유틸 import 경로 혼재 — 단일 소스(datetime.ts)로 통일 권장동일 목적의 유틸이 서로 다른 모듈에서 임포트되고 있습니다. 한 파일(datetime.ts)로 일원화하면 추후 동작 불일치/이중 유지보수를 줄일 수 있습니다.
-import { combineDateTime } from '@shared/utils/datetime'; -import { updateDate, updateTime } from '@shared/utils/formatDateTime'; +import { combineDateTime, updateDate, updateTime } from '@shared/utils/datetime';
89-90: 유효성 에러/미입력 시 저장 차단 로직 추가 권장현재 date/time에 에러가 있어도 remindTime이 null 또는 잘못된 값으로 저장될 수 있습니다. 저장 전에 에러/미입력 시 early-return 하도록 보강을 권합니다.
예시 구현(참고용):
const saveData = () => { if (!prevData?.id) { /* 그대로 */ } if (isRemindOn) { if (!date || !time || dateError || timeError) { setToastIsOpen(true); return; } } const remindTime = isRemindOn ? combineDateTime(date, time) : null; // 이하 동일 };
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/client/src/pages/myBookmark/MyBookmark.tsx(1 hunks)apps/client/src/shared/components/cardEditModal/CardEditModal.tsx(3 hunks)apps/client/src/shared/utils/datetime.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/client/src/pages/myBookmark/MyBookmark.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
apps/client/src/shared/components/cardEditModal/CardEditModal.tsx (1)
apps/client/src/shared/utils/datetime.ts (1)
combineDateTime(29-54)
⏰ 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: storybook
🔇 Additional comments (2)
apps/client/src/shared/utils/datetime.ts (2)
2-5: updateDate 유틸은 간결하고 목적에 부합합니다단순 포맷 변환 로직으로 문제 없어 보입니다.
8-11: updateTime 유틸 처리 적절HH:mm:ss 입력에 대해 UI 표기용 HH:mm으로 잘라내는 동작이 명확합니다.
apps/client/src/shared/components/cardEditModal/CardEditModal.tsx
Outdated
Show resolved
Hide resolved
| export const combineDateTime = (date: string, time: string) => { | ||
| if (!date || !time) return null; | ||
|
|
||
| // date가 YYYYMMDD 형태라면 가공 필요 | ||
| let formattedDate = date; | ||
| if (/^\d{8}$/.test(date)) { | ||
| const year = date.slice(0, 4); | ||
| const month = date.slice(4, 6); | ||
| const day = date.slice(6, 8); | ||
| formattedDate = `${year}-${month}-${day}`; | ||
| } else { | ||
| formattedDate = date.replace(/\./g, '-'); // YYYY.MM.DD → YYYY-MM-DD | ||
| } | ||
|
|
||
| // time이 HHmm 형태라면 HH:mm으로 변환 | ||
| let normalizedTime = time; | ||
| if (/^\d{4}$/.test(time)) { | ||
| normalizedTime = `${time.slice(0, 2)}:${time.slice(2, 4)}`; | ||
| } | ||
|
|
||
| // 24시간 포맷 변환 함수가 있다면 적용 | ||
| const to24 = to24Hour ? to24Hour(normalizedTime) : normalizedTime; | ||
| const formattedTime = to24.length === 5 ? `${to24}:00` : to24; | ||
|
|
||
| return `${formattedDate}T${formattedTime}`; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
combineDateTime: 타임존/UTC 계약 확인 필요 + 파싱 실패 가드 추가 제안
- 현재 반환값은 "YYYY-MM-DDTHH:mm(:ss)"의 로컬 타임스탬프(타임존 미포함)입니다. 이전 buildUtcIso(UTC ISO 생성) 대비 의미가 바뀌었을 수 있어 서버가 기대하는 시간대(로컬/UTC/오프셋 포함)를 확인해야 합니다. 계약이 UTC라면 now는 UTC(ISOString)인데 remindTime은 로컬(오프셋 없음)이라 불일치합니다.
- to24Hour 존재 여부 체크는 항상 truthy라 의미가 없어지고, 변환 실패 시에도 그대로 문자열을 이어붙이면 유효하지 않은 ISO가 생성될 수 있습니다.
아래는 안전성 향상을 위한 최소 수정입니다(형식 검증 + 명시적 반환 타입 + 불필요 조건 제거):
-export const combineDateTime = (date: string, time: string) => {
+export const combineDateTime = (date: string, time: string): string | null => {
if (!date || !time) return null;
@@
- // 24시간 포맷 변환 함수가 있다면 적용
- const to24 = to24Hour ? to24Hour(normalizedTime) : normalizedTime;
- const formattedTime = to24.length === 5 ? `${to24}:00` : to24;
+ // 24시간 포맷 변환
+ const to24 = to24Hour(normalizedTime);
+ // 유효한 HH:mm(:ss) 아니면 null
+ if (!/^\d{2}:\d{2}(?::\d{2})?$/.test(to24)) return null;
+ const formattedTime = to24.length === 5 ? `${to24}:00` : to24;
return `${formattedDate}T${formattedTime}`;
};서버가 UTC를 요구한다면 아래 대안으로 변환하십시오(로컬 해석 → UTC ISO). 유효하지 않은 날짜면 null 반환:
export const combineDateTimeUTC = (date: string, time: string): string | null => {
const local = combineDateTime(date, time);
if (!local) return null;
const d = new Date(local); // 로컬로 파싱
if (Number.isNaN(d.getTime())) return null;
return d.toISOString(); // UTC ISO (Z 포함)
};검증 스크립트(레포 루트에서 실행):
🏁 Script executed:
#!/bin/bash
# 1) 기존 buildUtcIso 잔존 사용처 확인
rg -nP -C2 '\bbuildUtcIso\s*\('
# 2) EditArticleRequest 내 remindTime 타입/계약 확인
rg -nPU -C3 '(?s)\binterface\s+EditArticleRequest\b.*?\{.*?\n.*remindTime.*\n.*\}'
# 3) remindTime을 서버에 전송하는 쿼리 함수 확인
rg -nP -C3 'usePutEditArticle|remindTime'
# 4) 서버가 기대하는 포맷(UTC/오프셋/로컬) 문서/주석 단서 탐색
rg -nP -C2 'remind|timezone|utc|offset|Z\b|ISO'Length of output: 289459
🏁 Script executed:
#!/bin/bash
set -eo pipefail
echo "=== Show file: apps/client/src/shared/utils/datetime.ts ==="
sed -n '1,240p' apps/client/src/shared/utils/datetime.ts || true
echo
echo "=== Search for to24Hour, combineDateTime, buildUtcIso across repo ==="
rg -n --hidden --no-ignore-vcs 'to24Hour|combineDateTime|buildUtcIso' || true
echo
echo "=== Show remindTimeFormat utility (used by extension imports) if exists ==="
rg -n --hidden --no-ignore-vcs 'remindTimeFormat' -S || true
rg -n --hidden --no-ignore-vcs '@utils/remindTimeFormat' -S || trueLength of output: 2991
combineDateTime: 타임존 계약 확인 + 파싱 실패 방어(클라이언트·익스텐션 동시 수정 필요)
combineDateTime은 타임존/오프셋 정보 없이 로컬 형식으로 반환하며(to24Hour 호출 후 "YYYY-MM-DDTHH:mm(:ss)" 형태), to24Hour 존재 체크는 무의미합니다. 입력 유효성(날짜/시간 파싱 실패) 처리와 명시적 반환 타입을 추가하고, 서버가 UTC를 요구하면 변환 로직을 별도 함수로 제공하세요.
수정 위치(정확한 포인트): apps/client/src/shared/utils/datetime.ts, apps/extension/src/utils/remindTimeFormat.ts
호출부 확인: apps/client/src/shared/components/cardEditModal/CardEditModal.tsx, apps/extension/src/pages/MainPop.tsx
서버 타입 참조: apps/client/src/shared/types/api.ts (remindTime: string | null) — 서버가 UTC(Z 포함)를 기대하는지 확인 필요.
권장 최소 변경(파일 내 combineDateTime 대체안 — 유효성 검증 + 타입 명시):
-export const combineDateTime = (date: string, time: string) => {
+export const combineDateTime = (date: string, time: string): string | null => {
if (!date || !time) return null;
@@
- // 24시간 포맷 변환 함수가 있다면 적용
- const to24 = to24Hour ? to24Hour(normalizedTime) : normalizedTime;
- const formattedTime = to24.length === 5 ? `${to24}:00` : to24;
+ // 24시간 포맷 변환
+ const to24 = to24Hour(normalizedTime);
+ if (!/^\d{2}:\d{2}(?::\d{2})?$/.test(to24)) return null;
+ const formattedTime = to24.length === 5 ? `${to24}:00` : to24;
return `${formattedDate}T${formattedTime}`;
};서버가 UTC ISO(Z)를 요구한다면(로컬 해석 → UTC ISO 변환) 보조 함수 추가:
export const combineDateTimeUTC = (date: string, time: string): string | null => {
const local = combineDateTime(date, time);
if (!local) return null;
const d = new Date(local); // 로컬로 파싱
if (Number.isNaN(d.getTime())) return null;
return d.toISOString(); // UTC ISO (Z 포함)
};검증 요약: 리포지토리에 동일 로직이 client/extension에 존재함을 확인(동기 수정 필요). 서버가 UTC를 요구할 경우 클라이언트에서 toISOString()으로 변환해 전송하거나 서버 계약에 맞게 명확히 합의해야 합니다.
2 participants
📌 Related Issues
📄 Tasks
리마인드 수정시 오류 1 : 리마인드 edit 창을 키고 닫을 때마다 시간이 제멋대로 지정되는 오류 발생
리마인드 수정시 오류 2 : 리마인드 edit 창을 키고 저장할 때마다 시간이 -9시간 저장되는 오류 발생
⭐ PR Point (To Reviewer)
리마인드 수정시 오류 1
원인
React는 컴포넌트의 state를 트리에서의 위치에 묶어서 보존하는데 같은 컴포넌트를 같은 위치에 계속 렌더링하면 이전 state를 그대로 유지해버려요. 그래서 북마크에서 모달을 열 때마다 이전 아티클에서 쓰던 내부 state(날짜/시간)가 carry-over”되는 현상이 발생했습니다.
“같은 위치에 렌더된 동일 컴포넌트는 상태가 보존된다.” 즉, 부모가 모달을 조건부로 토글만 하고, 모달의 위치/타입이 같다면 상태가 초기화되지 않게됩니다.
해결 방법
React는 같은 위치에 같은 타입의 컴포넌트가 렌더되면 기존 인스턴스를 재사용합니다. 그래서 prevData가 바뀌어도 모달은 그대로고 내부 useState 값들이 유지되는 state의 carry-over현상이 발생했어요.
그래서 리액트에서는 다른 대상을 보여줄 때처럼 컴포넌트 트리의 정체성이 바뀌어야 하는 경우 key를 달아 새 인스턴스로 간주시키라고 말하고있어요.
key={articleDetail.id}를 주면, id가 달라질 때마다 다른 컴포넌트로 간주되어 기존 것을 언마운트하며 새로 마운트며 내부 state가 초기화됩니다. ❇️리마인드 수정시 오류 2
.. 얘는 별거 없었어요
읽을 땐 UTC→로컬 변환을 안 했는데, 저장할 땐 로컬→UTC 변환을 해서 매 저장마다 –9시간씩 밀렸습니다
extension과 같은 포맷 방식으로 수정 했습니다
📷 Screenshot
Summary by CodeRabbit
신규 기능
버그 수정
기타