Merged
Conversation
…latform-client into fix/notification
…latform-client into fix/notification
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 Walkthrough개요세 개의 컴포넌트에 걸쳐 개별 알림 클릭 처리 기능을 추가했습니다. useHasNewNotification 훅을 useReadNotifications으로 교체하고, 각 알림을 클릭할 때 읽음 상태로 표시하는 onNotificationClick 핸들러를 구현했습니다. 변경 사항
시퀀스 다이어그램sequenceDiagram
actor User
participant NotificationDropdown
participant NotificationList
participant API as Read API
User->>NotificationDropdown: 알림 드롭다운 열기
NotificationDropdown->>NotificationList: onNotificationClick 핸들러 전달
User->>NotificationList: 읽지 않은 알림 클릭
NotificationList->>NotificationDropdown: onNotificationClick(notification) 호출
NotificationDropdown->>NotificationDropdown: handleNotificationClick 실행
alt notification.isRead === false
NotificationDropdown->>API: readNotifications([notification.id])
API-->>NotificationDropdown: 읽음 상태 업데이트
NotificationDropdown->>NotificationList: 상태 변경 반영
end
예상 코드 검토 난이도🎯 3 (중간) | ⏱️ ~20분 시 🐰
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/app/`(service)/(my)/notification/page.tsx:
- Around line 65-71: The handler handleNotificationClick currently passes
notification.id to readNotifications but MemberNotificationResponse defines id
as optional, so guard against undefined: check that notification.id is a valid
number before calling readNotifications (or build an array by filtering out
falsy/undefined ids) and only call readNotifications when you have one or more
concrete ids; update handleNotificationClick to validate notification.id (or use
a safe array like [notification.id].filter(Boolean)) so readNotifications never
receives undefined.
In `@src/components/lists/notification-list.tsx`:
- Around line 56-60: The code calls date-fns format with notification.createdAt
which is optional (MemberNotificationResponse.createdAt) and can be undefined,
causing a runtime error; update the render in the component that uses format
(the span showing format(notification.createdAt, 'yyyy.MM.dd HH:mm')) to first
check that notification.createdAt is present and a valid Date (or can be safely
parsed) and only call format when valid, otherwise render a safe fallback (e.g.,
empty string or a placeholder like '-') to prevent passing undefined into
format.
In `@src/components/modals/notification-dropdown.tsx`:
- Around line 37-43: handleNotificationClick calls readNotifications with
notification.id but MemberNotificationResponse.id is optional and may be
undefined; update handleNotificationClick to guard that id is defined before
calling readNotifications (e.g., check notification.id !== undefined or use a
local const id = notification.id and if (!notification.isRead && id)
readNotifications([id])), ensuring you reference the existing
handleNotificationClick function, the readNotifications call, and
MemberNotificationResponse.id so only defined IDs are passed.
🧹 Nitpick comments (2)
src/app/(service)/(my)/notification/page.tsx (1)
65-71:handleNotificationClick로직이notification-dropdown.tsx와 중복됩니다.동일한 함수 구현이
src/components/modals/notification-dropdown.tsx(lines 37-43)에도 존재합니다. 이 로직을 커스텀 훅이나 유틸리티 함수로 추출하여 재사용성을 높이는 것을 고려해 주세요.src/components/lists/notification-list.tsx (1)
50-54: 동적 클래스 구성에clsx또는tailwind-merge사용을 권장합니다.코딩 가이드라인에 따르면 동적 Tailwind 클래스 관리에
clsx,tailwind-merge, 또는class-variance-authority (CVA)를 사용해야 합니다. 현재 템플릿 리터럴을 사용하고 있습니다.♻️ 제안된 수정
+import { clsx } from 'clsx'; ... <span - className={`${notification.isRead ? 'font-designer-13r' : 'font-designer-13b'} text-text-default`} + className={clsx( + notification.isRead ? 'font-designer-13r' : 'font-designer-13b', + 'text-text-default' + )} >As per coding guidelines,
**/*.{tsx,jsx}: Use class composition utilities clsx, tailwind-merge, and class-variance-authority (CVA) for dynamic Tailwind class management.
Comment on lines
+56
to
+60
| <div className="flex items-center gap-100"> | ||
| <span className="font-designer-11r text-text-subtlest whitespace-nowrap"> | ||
| {format(notification.createdAt, 'yyyy.MM.dd HH:mm')} | ||
| </span> | ||
| </div> |
There was a problem hiding this comment.
notification.createdAt이 undefined일 경우 런타임 에러가 발생할 수 있습니다.
MemberNotificationResponse.createdAt은 optional 필드입니다. date-fns의 format 함수에 undefined가 전달되면 런타임 에러가 발생합니다.
🛡️ 제안된 수정
<div className="flex items-center gap-100">
<span className="font-designer-11r text-text-subtlest whitespace-nowrap">
- {format(notification.createdAt, 'yyyy.MM.dd HH:mm')}
+ {notification.createdAt
+ ? format(notification.createdAt, 'yyyy.MM.dd HH:mm')
+ : '-'}
</span>
</div>📝 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
| <div className="flex items-center gap-100"> | |
| <span className="font-designer-11r text-text-subtlest whitespace-nowrap"> | |
| {format(notification.createdAt, 'yyyy.MM.dd HH:mm')} | |
| </span> | |
| </div> | |
| <div className="flex items-center gap-100"> | |
| <span className="font-designer-11r text-text-subtlest whitespace-nowrap"> | |
| {notification.createdAt | |
| ? format(notification.createdAt, 'yyyy.MM.dd HH:mm') | |
| : '-'} | |
| </span> | |
| </div> |
🤖 Prompt for AI Agents
In `@src/components/lists/notification-list.tsx` around lines 56 - 60, The code
calls date-fns format with notification.createdAt which is optional
(MemberNotificationResponse.createdAt) and can be undefined, causing a runtime
error; update the render in the component that uses format (the span showing
format(notification.createdAt, 'yyyy.MM.dd HH:mm')) to first check that
notification.createdAt is present and a valid Date (or can be safely parsed) and
only call format when valid, otherwise render a safe fallback (e.g., empty
string or a placeholder like '-') to prevent passing undefined into format.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🌱 연관된 이슈
☘️ 작업 내용
🍀 참고사항
스크린샷 (선택)
2026-02-04.17.34.36.mov
Summary by CodeRabbit
새 기능
개선사항