Skip to content

Feat: 개발 QA 1차(수정하기 모달창 데이트타임 포맷) #118

Merged
jjangminii merged 1 commit intodevelopfrom
fix/#116/remind-modal-sever
Sep 19, 2025
Merged

Feat: 개발 QA 1차(수정하기 모달창 데이트타임 포맷) #118
jjangminii merged 1 commit intodevelopfrom
fix/#116/remind-modal-sever

Conversation

@jjangminii
Copy link
Collaborator

@jjangminii jjangminii commented Sep 19, 2025

📌 Related Issues

관련된 Issue를 태그해주세요. (e.g. - close #25)

📄 Tasks

  • 아티클 리마인드 시간 수정
    현재 일반 문자열로 보내지는거 서버가 원하는 형식으로 수정했습니다

⭐ PR Point (To Reviewer)

📷 Screenshot

image

Summary by CodeRabbit

  • 신규 기능
    • 없음
  • 버그 수정
    • 알림 사용 시 입력한 날짜/시간이 UTC 기준 ISO 형식으로 정확히 저장되어 기기/지역에 따라 시간 오차가 발생하던 문제를 방지했습니다.
    • 유효하지 않은 날짜/시간이거나 알림이 꺼진 경우, 알림 시간이 빈 값으로 처리되어 불필요한 알림 전송을 막습니다.
    • 카테고리 선택이 없는 경우에도 기본값이 안정적으로 적용됩니다.
  • 리팩터링
    • 날짜·시간 파싱과 ISO 문자열 생성을 공용 유틸로 통합해 일관성을 높였습니다.

@jjangminii jjangminii linked an issue Sep 19, 2025 that may be closed by this pull request
@vercel
Copy link

vercel bot commented Sep 19, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
pinback-client-client Ready Ready Preview Comment Sep 19, 2025 7:25am

@coderabbitai
Copy link

coderabbitai bot commented Sep 19, 2025

Walkthrough

CardEditModal 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

Cohort / File(s) Summary
Card Edit Modal
apps/client/src/shared/components/cardEditModal/CardEditModal.tsx
Import buildUtcIso; compute remindTime via UTC ISO helper; replace inline ${date}T${time}; use ?? -1 for categoryId.
Datetime Utils
apps/client/src/shared/utils/datetime.ts
Add parseDateString, parseTimeString, and exported buildUtcIso(dateStr, timeStr) that returns ISO string or null.

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
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • constantly-dev
  • jllee000

Poem

새벽의 토끼, 시계를 톡— 딱 맞춰 보며,
숫자만 골라 시간의 껍질을 벗기네.
로컬의 안개는 걷히고, UTC는 반짝-
카드의 메모엔 별빛 같은 ISO 한 줄.
깡총! 저장하면 시간도 정렬된다네. ⏱️🐰

Pre-merge checks and finishing touches

❌ 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 (4 passed)
Check name Status Explanation
Linked Issues Check ✅ Passed 코드 변경은 remind 시간을 서버가 요구하는 UTC ISO 문자열로 변환하는 유틸(buildUtcIso)과 모달 연동을 추가하여 Issue #116의 코딩 요구사항을 충족합니다. 변경 범위는 CardEditModal.tsx와 utils/datetime.ts로 한정되며 Issue #25(Progress 컴포넌트)는 본 PR과 무관하므로 영향을 받지 않습니다. 따라서 연결된 이슈들 중 본 PR 대상 이슈([#116])에 대해 적절히 구현되었습니다.
Out of Scope Changes Check ✅ Passed 변경된 파일과 요약을 보면 리마인드 시간 포맷 관련 유틸 및 모달 수정 외에 다른 기능(예: progress bar, 디자인 시스템) 변경은 포함되어 있지 않습니다. 카테고리 id 추출 시 null 병합 연산자 사용과 같은 소규모 개선도 기능 범위 내의 합리적 변경으로 보입니다. 따라서 범위 외(out-of-scope) 변경은 없습니다.
Description Check ✅ Passed PR 설명은 레포지토 템플릿 구조(관련 이슈, 작업 내용, PR 포인트, 스크린샷)를 따르고 있으며 'Related Issues'에 close #116가 명시되어 있습니다. 'Tasks'에 리마인드 시간 형식 수정 요약이 포함되고 스크린샷도 첨부되어 있어 리뷰에 필요한 주요 정보가 제공됩니다. PR 포인트가 비어있으나 템플릿상 필수 항목은 아니므로 설명은 대부분 완전합니다.
Title Check ✅ Passed 제목은 "수정하기 모달창 데이트타임 포맷" 변경이라는 핵심을 직접적으로 나타내어 PR 목표(리마인드 시간 포맷을 서버 요구 형식으로 변환)와 일치합니다. 다만 "개발 QA 1차(정민
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/#116/remind-modal-sever

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.

@github-actions github-actions bot added the fix 버그 수정하라 러브버그 label Sep 19, 2025
@github-actions
Copy link

✅ Storybook chromatic 배포 확인:
🐿️ storybook

Copy link
Collaborator

@jllee000 jllee000 left a comment

Choose a reason for hiding this comment

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

빠른수정굿! 확인했습니당!

-1,
now: new Date().toISOString(),
remindTime: isRemindOn ? `${date}T${time}` : null,
remindTime,
Copy link
Collaborator

Choose a reason for hiding this comment

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

오홍 문자열 사이에 T만 붙이면 잘 돌아가는거 확인 되나용? 굿굿

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: 3

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f2303de and 958a3e9.

📒 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();
 }

Comment on lines +89 to +91
const remindTime =
isRemindOn && date && time ? buildUtcIso(date, time) : null;

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

🧩 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 || true

Length 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.

Comment on lines +1 to +8
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 };
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

날짜 파싱: 범위 검증·반환 타입 보강 필요

월/일 범위(윤년 포함) 검증이 없어 잘못된 값이 그대로 통과되고, 반환 타입 명시가 없습니다. 스케줄/리마인드에 직격이라 저장 시점 오동작 위험이 큽니다.

다음과 같이 안전 검증과 타입을 추가해주세요.

-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.

Suggested change
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 };
}

Comment on lines +10 to +16
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 };
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

시간 파싱: 시/분 범위 검증·반환 타입 명시 필요

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.

Suggested change
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.

@jjangminii jjangminii changed the title feat: 리마인드시간 포맷 Feat: 개발 QA 1차(정민 | 수정하기 모달창 데이트타임 포맷) Sep 19, 2025
@jjangminii jjangminii merged commit 88b0212 into develop Sep 19, 2025
10 checks passed
@jjangminii jjangminii changed the title Feat: 개발 QA 1차(정민 | 수정하기 모달창 데이트타임 포맷) Feat: 개발 QA 1차(수정하기 모달창 데이트타임 포맷) Sep 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix 버그 수정하라 러브버그

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Fix] sp1 QA 수정하기 모달 1차 반영

2 participants