Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughCardEditModal now computes remindTime via a new datetime utility that builds a UTC ISO string from separate date/time inputs. It replaces inline string concatenation and uses nullish coalescing for categoryId. A new shared datetime module adds parsers and the exported buildUtcIso function. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant C as CardEditModal
participant D as datetime.ts
participant API as Backend API
U->>C: Click "Save"
C->>C: Gather inputs (date, time, remind toggle, category)
alt Remind ON and date/time provided
C->>D: buildUtcIso(dateStr, timeStr)
D-->>C: UTC ISO string or null
note over C: Set payload.remindTime = ISO or null
else Remind OFF or missing inputs
C->>C: Set payload.remindTime = null
end
C->>API: PUT/POST payload with remindTime and categoryId
API-->>C: Response
C-->>U: Close modal / update state
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests
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 배포 확인: |
| -1, | ||
| now: new Date().toISOString(), | ||
| remindTime: isRemindOn ? `${date}T${time}` : null, | ||
| remindTime, |
There was a problem hiding this comment.
오홍 문자열 사이에 T만 붙이면 잘 돌아가는거 확인 되나용? 굿굿
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/client/src/shared/components/cardEditModal/CardEditModal.tsx(2 hunks)apps/client/src/shared/utils/datetime.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/client/src/shared/components/cardEditModal/CardEditModal.tsx (2)
apps/client/src/shared/utils/datetime.ts (1)
buildUtcIso(18-25)apps/client/src/shared/types/api.ts (1)
EditArticleRequest(21-26)
⏰ 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 (4)
apps/client/src/shared/components/cardEditModal/CardEditModal.tsx (3)
22-22: UTC 변환 유틸 도입 좋습니다형식 변환을 컴포넌트에서 분리한 점이 깔끔합니다.
95-96: || → ?? 전환 적절합니다id가 0인 경우도 정상 처리되며, 의미상 null/undefined만 대체합니다. 다만 서버가 -1을 무카테고리로 허용하는지 재확인 부탁드립니다.
98-99: 페이로드에 UTC ISO 반영 OK — 라운드트립 확인저장 형식은 서버 요구와 일치해 보입니다. 다만 초기화(useEffect)에서는 prevData.remindAt를 단순 split/format만 하므로, 서버가 UTC ISO를 내려줄 경우 TZ 보정 없이 그대로 UI에 노출될 수 있습니다. 저장→조회→편집 라운드트립에서 시각이 변형되지 않는지 실제 응답으로 확인해주세요.
apps/client/src/shared/utils/datetime.ts (1)
18-25: DST 갭/롤오버 보호 및 반환 타입 명시 — 서버 요구사항 확인 필요File: apps/client/src/shared/utils/datetime.ts
Lines: 18-25로컬 시각을 Date로 생성해 toISOString으로 UTC 변환하는 방식은 적절하나, DST로 인한 암묵적 롤오버(존재하지 않는 시각) 문제가 있으므로 입력 구성요소 보존 검증을 추가하고 반환 타입을 명시하세요. 서버가 "사용자 로컬 시각을 UTC ISO 문자열로 변환"을 요구하는지 확인해 주세요. 아니라면 입력을 UTC로 간주해 Date.UTC 기반으로 변경해야 합니다.
-export function buildUtcIso(dateStr: string, timeStr: string) { +export function buildUtcIso(dateStr: string, timeStr: string): string | null { const d = parseDateString(dateStr); const t = parseTimeString(timeStr); if (!d || !t) return null; - const dt = new Date(d.Y, d.M - 1, d.D, t.h, t.m, 0, 0); + const dt = new Date(d.Y, d.M - 1, d.D, t.h, t.m, 0, 0); // 로컬 -> UTC 변환을 위해 로컬 생성 + // DST 갭/롤오버 방지: 구성 요소가 보존되었는지 확인 + if ( + dt.getFullYear() !== d.Y || + dt.getMonth() !== d.M - 1 || + dt.getDate() !== d.D || + dt.getHours() !== t.h || + dt.getMinutes() !== t.m + ) { + return null; + } return dt.toISOString(); }
| const remindTime = | ||
| isRemindOn && date && time ? buildUtcIso(date, time) : null; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
유효성 에러를 반영해 리마인드 시간 생성을 제한하세요
현재는 isRemindOn && date && time 만으로 ISO를 만들기 때문에, validate 에러가 있어도 null/잘못된 값이 제출될 수 있습니다(기존 리마인드가 의도치 않게 제거될 위험).
다음처럼 에러 부재 시에만 생성하도록 최소 수정이 가능합니다.
- const remindTime =
- isRemindOn && date && time ? buildUtcIso(date, time) : null;
+ const remindTime =
+ isRemindOn && date && time && !dateError && !timeError
+ ? buildUtcIso(date, time)
+ : null;또한 buildUtcIso가 null을 반환(파싱 실패/DST 갭)하면 저장을 막고 사용자에게 알려주는 가드 추가를 권장합니다.
// 추가 제안: remindTime 산출 직후
if (isRemindOn && remindTime === null) {
setErrorTxt('리마인드 시간 형식이 올바르지 않습니다.');
setIsPopError(true);
setToastIsOpen(true);
return;
}🏁 Script executed:
#!/bin/bash
set -e
# Verify remindTime usage in CardEditModal.tsx
rg -n "const remindTime" -C3 apps/client/src/shared/components/cardEditModal/CardEditModal.tsx || true
# Check presence of dateError/timeError in the same file
rg -n "dateError" -C3 apps/client/src/shared/components/cardEditModal/CardEditModal.tsx || true
rg -n "timeError" -C3 apps/client/src/shared/components/cardEditModal/CardEditModal.tsx || true
# Inspect buildUtcIso usage in CardEditModal.tsx
rg -n "buildUtcIso" -C3 apps/client/src/shared/components/cardEditModal/CardEditModal.tsx || true
# Locate buildUtcIso definition to confirm its return type
rg -R -n "buildUtcIso" -C3 apps/client/src || trueLength of output: 2984
리마인드 시간 생성: 유효성 에러가 없을 때만 ISO 생성하도록 즉시 수정 필요 (apps/client/src/shared/components/cardEditModal/CardEditModal.tsx:89-91)
isRemindOn && date && time만으로 buildUtcIso를 호출하면 dateError/timeError가 있어도 잘못된 값이 제출되어 기존 리마인드가 의도치 않게 제거될 위험이 있습니다. 아래 최소 수정 적용 후, buildUtcIso가 null을 반환할 가능성도 확인해 추가 가드 적용하세요.
- const remindTime =
- isRemindOn && date && time ? buildUtcIso(date, time) : null;
+ const remindTime =
+ isRemindOn && date && time && !dateError && !timeError
+ ? buildUtcIso(date, time)
+ : null;추가 가드 제안(빌드 후 실패 시 저장 차단 및 사용자 알림):
if (isRemindOn && remindTime === null) {
setErrorTxt('리마인드 시간 형식이 올바르지 않습니다.');
setIsPopError(true);
setToastIsOpen(true);
return;
}🤖 Prompt for AI Agents
In apps/client/src/shared/components/cardEditModal/CardEditModal.tsx around
lines 89 to 91, the current remindTime calculation calls buildUtcIso when
isRemindOn && date && time are truthy, but ignores dateError/timeError and the
possibility that buildUtcIso may return null; change the logic to only call
buildUtcIso when isRemindOn is true AND there are no dateError or timeError
values, assign remindTime from that call, then add a guard before saving that
checks if isRemindOn && remindTime === null (or missing) and in that case set
the error text, open the pop error and toast, and return to block the save.
Ensure all branches avoid clearing an existing reminder when validation fails.
| function parseDateString(d: string) { | ||
| const s = d.replace(/[^\d]/g, ''); | ||
| if (s.length !== 8) return null; | ||
| const Y = Number(s.slice(0, 4)); | ||
| const M = Number(s.slice(4, 6)); | ||
| const D = Number(s.slice(6, 8)); | ||
| return { Y, M, D }; | ||
| } |
There was a problem hiding this comment.
날짜 파싱: 범위 검증·반환 타입 보강 필요
월/일 범위(윤년 포함) 검증이 없어 잘못된 값이 그대로 통과되고, 반환 타입 명시가 없습니다. 스케줄/리마인드에 직격이라 저장 시점 오동작 위험이 큽니다.
다음과 같이 안전 검증과 타입을 추가해주세요.
-function parseDateString(d: string) {
+function parseDateString(d: string): { Y: number; M: number; D: number } | null {
const s = d.replace(/[^\d]/g, '');
if (s.length !== 8) return null;
- const Y = Number(s.slice(0, 4));
- const M = Number(s.slice(4, 6));
- const D = Number(s.slice(6, 8));
- return { Y, M, D };
+ const Y = Number(s.slice(0, 4));
+ const M = Number(s.slice(4, 6));
+ const D = Number(s.slice(6, 8));
+ if (!Number.isInteger(Y) || !Number.isInteger(M) || !Number.isInteger(D)) return null;
+ if (M < 1 || M > 12) return null;
+ // UTC 기준으로 말일 체크해 TZ/DST 영향 제거
+ const lastDay = new Date(Date.UTC(Y, M, 0)).getUTCDate();
+ if (D < 1 || D > lastDay) return null;
+ return { Y, M, D };
}📝 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.
| function parseDateString(d: string) { | |
| const s = d.replace(/[^\d]/g, ''); | |
| if (s.length !== 8) return null; | |
| const Y = Number(s.slice(0, 4)); | |
| const M = Number(s.slice(4, 6)); | |
| const D = Number(s.slice(6, 8)); | |
| return { Y, M, D }; | |
| } | |
| function parseDateString(d: string): { Y: number; M: number; D: number } | null { | |
| const s = d.replace(/[^\d]/g, ''); | |
| if (s.length !== 8) return null; | |
| const Y = Number(s.slice(0, 4)); | |
| const M = Number(s.slice(4, 6)); | |
| const D = Number(s.slice(6, 8)); | |
| if (!Number.isInteger(Y) || !Number.isInteger(M) || !Number.isInteger(D)) return null; | |
| if (M < 1 || M > 12) return null; | |
| // UTC 기준으로 말일 체크해 TZ/DST 영향 제거 | |
| const lastDay = new Date(Date.UTC(Y, M, 0)).getUTCDate(); | |
| if (D < 1 || D > lastDay) return null; | |
| return { Y, M, D }; | |
| } |
| function parseTimeString(t: string) { | ||
| const s = t.replace(/[^\d]/g, ''); | ||
| if (s.length !== 4) return null; | ||
| const h = Number(s.slice(0, 2)); | ||
| const m = Number(s.slice(2, 4)); | ||
| return { h, m }; | ||
| } |
There was a problem hiding this comment.
시간 파싱: 시/분 범위 검증·반환 타입 명시 필요
24시간제 기준 범위 체크가 없어 99:99 등도 통과합니다. 타입도 명시해주세요.
-function parseTimeString(t: string) {
+function parseTimeString(t: string): { h: number; m: number } | null {
const s = t.replace(/[^\d]/g, '');
if (s.length !== 4) return null;
const h = Number(s.slice(0, 2));
const m = Number(s.slice(2, 4));
- return { h, m };
+ if (!Number.isInteger(h) || !Number.isInteger(m)) return null;
+ if (h < 0 || h > 23 || m < 0 || m > 59) return null;
+ return { h, m };
}📝 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.
| function parseTimeString(t: string) { | |
| const s = t.replace(/[^\d]/g, ''); | |
| if (s.length !== 4) return null; | |
| const h = Number(s.slice(0, 2)); | |
| const m = Number(s.slice(2, 4)); | |
| return { h, m }; | |
| } | |
| function parseTimeString(t: string): { h: number; m: number } | null { | |
| const s = t.replace(/[^\d]/g, ''); | |
| if (s.length !== 4) return null; | |
| const h = Number(s.slice(0, 2)); | |
| const m = Number(s.slice(2, 4)); | |
| if (!Number.isInteger(h) || !Number.isInteger(m)) return null; | |
| if (h < 0 || h > 23 || m < 0 || m > 59) return null; | |
| return { h, m }; | |
| } |
🤖 Prompt for AI Agents
In apps/client/src/shared/utils/datetime.ts around lines 10 to 16, the
parseTimeString function currently accepts any four digits (e.g. "99:99") and
lacks an explicit return type; update the signature to return a typed value
(e.g. { h: number; m: number } | null), ensure parsed values are numeric
integers and validate that 0 <= h <= 23 and 0 <= m <= 59, and return null for
any invalid or non-integer results; keep the existing non-digit stripping and
length check, but add NaN checks and the range checks before returning the
object.
📌 Related Issues
📄 Tasks
현재 일반 문자열로 보내지는거 서버가 원하는 형식으로 수정했습니다
⭐ PR Point (To Reviewer)
📷 Screenshot
Summary by CodeRabbit