Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: push notification 15min Modal and Alert #379

Merged
merged 11 commits into from
Nov 18, 2024

Conversation

enc2586
Copy link
Member

@enc2586 enc2586 commented Oct 14, 2024

아직 작업이 완료되지 않았습니다. 완료되었습니다.

  • TODO: uncomment 'CSLink' component
  • TODO: implement push notification sending logic

Summary by CodeRabbit

Summary by CodeRabbit

  • 신규 기능

    • 푸시 알림 전송을 위한 경고를 표시하는 SendPushAlarm 컴포넌트 추가.
    • 공지에 게시 날짜를 나타내는 publishedAt 속성 추가.
    • sendNoticeAlarm 기능 추가로 공지 알림 전송 가능.
    • 알림 전송 관련 새로운 번역 항목 추가 (영어 및 한국어).
    • 스펠 체크 구성에 "Swal" 단어 추가.
  • 수정 사항

    • MypageButton의 클릭 가능 링크 변경.
    • Footer 컴포넌트의 버그 보고 렌더링 로직 수정.
    • WritePage의 조건문 간소화.

TODO: uncomment 'CSLink' component
TODO: implement push notification sending logic
TODO: uncomment 'CSLink' component
TODO: implement push notification sending logic
@enc2586 enc2586 linked an issue Oct 14, 2024 that may be closed by this pull request
Copy link
Contributor

coderabbitai bot commented Oct 14, 2024

📝 Walkthrough
📝 Walkthrough

Walkthrough

이 풀 리퀘스트에서는 여러 파일에 대한 변경 사항이 포함되어 있습니다. .vscode/settings.json 파일에 "Swal"이라는 단어가 추가되어 맞춤법 검사 구성이 강화되었습니다. src/api/notice/notice.ts 파일에서는 Notice 인터페이스에 publishedAt 속성이 추가되었으며, src/api/notice/send-alarm.ts 파일에는 sendNoticeAlarm 함수가 새로 도입되었습니다. 또한, SendPushNotificationAlert라는 새로운 React 컴포넌트가 추가되어 푸시 알림 전송과 관련된 기능을 수행합니다. 마지막으로, 여러 언어 파일에서 푸시 알림 관련 번역 항목이 추가되었습니다.

Changes

파일 경로 변경 요약
.vscode/settings.json cSpell.words 배열에 "Swal" 추가.
src/api/notice/notice.ts Notice 인터페이스에 `publishedAt: dayjs.Dayjs
src/api/notice/send-alarm.ts 새로운 함수 sendNoticeAlarm({ id }: Pick<Notice, 'id'>) 추가.
src/app/[lng]/(with-page-layout)/(with-sidebar-layout)/notice/[id]/SendPushNotificationAlert.tsx 새로운 React 컴포넌트 SendPushNotificationAlert 추가. id, author, publishedAt 속성 수용, useSession 훅 사용, 푸시 알림 전송 기능 포함.
src/app/[lng]/write/page.tsx CSS 임포트 재조정 및 조건문에서 불필요한 괄호 제거.
src/app/i18next/locales/en/translation.json "sentPushNotificationAlert" 섹션 추가, 여러 새로운 키 추가.
src/app/i18next/locales/ko/translation.json "sentPushNotificationAlert" 섹션 추가, 여러 새로운 키 추가.
src/app/[lng]/(with-page-layout)/(with-sidebar-layout)/notice/[id]/getTimeDiff.ts isServer 상수 및 getTimeDiff 함수 추가.
src/app/[lng]/(with-page-layout)/(with-sidebar-layout)/notice/[id]/page.tsx SendPushAlarm 컴포넌트 추가.
src/app/[lng]/(with-page-layout)/(without-sidebar-layout)/mypage/MypageButton.tsx MypageButtons 함수 시그니처를 비동기 함수로 변경.

Suggested reviewers

  • dohyun-ko
  • 2paperstar

🐰 푸시 알림의 기쁨,
새로운 컴포넌트가 왔어요!
알림을 보내는 재미,
국제화도 잊지 않죠.
클릭할 수 없던 버튼,
이제는 독립적으로,
모두 함께 즐겨요,
변화의 춤을 추어요! 🎉


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between c19a24f and 10a46d1.

📒 Files selected for processing (1)
  • src/app/[lng]/(with-page-layout)/(with-sidebar-layout)/notice/[id]/SendPushNotificationAlert.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/app/[lng]/(with-page-layout)/(with-sidebar-layout)/notice/[id]/SendPushNotificationAlert.tsx

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@enc2586
Copy link
Member Author

enc2586 commented Oct 14, 2024

@dohyun-ko 아직 완성되진 않았지만, 코드 제공하며 여쭤볼 게 있어서 PR올렸습니다

  • 알림 보내기 요청을 어떤 api로 보내야 하는지
  • 알림 보내기 기능 추가에 따른 isEditable 로직 변경 가능성
    (현재는 createdAt에서 15분이 지나면 막는 중 > 알림을 보냈을 경우도 막아야 할 것으로 보임 > how?)

위 두 사항이 궁금합니다!

Copy link

Copy link
Contributor

@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

🧹 Outside diff range and nitpick comments (3)
src/app/[lng]/(with-page-layout)/(with-sidebar-layout)/notice/[id]/SendPushNotificationAlert.tsx (1)

42-52: 렌더링 로직 승인 및 접근성 개선 제안

컴포넌트의 렌더링 로직과 스타일링이 적절히 구현되었습니다. 국제화도 올바르게 적용되었습니다.

접근성 개선을 위해 다음 사항을 고려해보세요:

  1. 클릭 가능한 span 대신 button 요소를 사용하세요. 이는 키보드 접근성을 향상시킵니다.
  2. 푸시 알림 전송 액션에 대한 설명을 aria-label로 추가하세요.

예시:

<button
  className="cursor-pointer font-bold underline"
  onClick={handleSendPushNotification}
  aria-label={t('zabo.sentPushNotificationAlert.ariaLabel')}
>
  {t('zabo.sentPushNotificationAlert.action')}
</button>
src/app/[lng]/(with-page-layout)/(with-sidebar-layout)/notice/[id]/page.tsx (2)

75-76: SendPushNotificationAlert 컴포넌트 사용 확인 및 최적화 제안

SendPushNotificationAlert 컴포넌트의 배치가 적절해 보입니다. 그러나 성능 최적화를 위해 다음 사항을 고려해 보시기 바랍니다:

  1. 컴포넌트가 전체 notice 객체를 필요로 하지 않는다면, 필요한 속성만 전달하는 것이 좋습니다.
  2. 푸시 알림 전송 권한이 있는 사용자에게만 이 컴포넌트를 조건부로 렌더링하는 것을 고려해 보세요.

다음과 같이 코드를 최적화할 수 있습니다:

- <SendPushNotificationAlert {...notice} lng={lng} />
+ <SendPushNotificationAlert 
+   id={notice.id}
+   title={notice.title}
+   lng={lng}
+ />

이 변경은 필요한 props만 전달하여 컴포넌트의 불필요한 리렌더링을 방지할 수 있습니다.


푸시 알림 기능 구현 미완료

현재 SendPushNotificationAlert 컴포넌트에 푸시 알림을 전송하는 기능이 구현되지 않았습니다. handleSendPushNotification 함수에 TODO 주석이 남아 있어 실제 푸시 알림 전송 로직이 필요합니다.

추가적으로 다음 사항을 고려해 주세요:

  • 푸시 알림을 전송하기 위한 API 엔드포인트를 구현하고, handleSendPushNotification 함수에서 호출하도록 수정.
  • 푸시 알림 전송 성공 및 실패에 대한 사용자 피드백을 추가하여 사용자 경험을 향상시킬 것.
🔗 Analysis chain

Line range hint 1-114: 전반적인 변경 사항 요약 및 평가

이 PR의 변경 사항은 다음과 같습니다:

  1. SendPushNotificationAlert 컴포넌트를 가져오고 사용하는 것.
  2. DetailedNoticePage 컴포넌트의 기존 구조를 유지하면서 새로운 기능을 추가한 것.

이러한 변경 사항은 PR의 목표인 푸시 알림 기능 구현과 잘 일치합니다. 코드는 깔끔하고 기존 기능을 방해하지 않으면서 새로운 기능을 통합하고 있습니다.

다음 단계로 고려해볼 사항:

  1. SendPushNotificationAlert 컴포넌트의 내부 구현을 검토하여 예상대로 작동하는지 확인하세요.
  2. 푸시 알림 전송을 위한 API 엔드포인트가 결정되면, 해당 정보를 SendPushNotificationAlert 컴포넌트에 전달하는 방법을 고려해 보세요.
  3. 필요한 경우, isEditable 로직을 조정하여 푸시 알림 기능과 잘 통합되도록 하세요.

SendPushNotificationAlert 컴포넌트의 구현을 확인하기 위해 다음 스크립트를 실행해 주세요:

이 스크립트를 통해 SendPushNotificationAlert 컴포넌트의 구현을 검토하고, API 호출 및 isEditable 로직의 존재 여부를 확인할 수 있습니다.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# SendPushNotificationAlert 컴포넌트의 구현 확인

# SendPushNotificationAlert 컴포넌트 파일 내용 출력
echo "SendPushNotificationAlert 컴포넌트 내용:"
cat src/app/[lng]/\(with-page-layout\)/\(with-sidebar-layout\)/notice/[id]/SendPushNotificationAlert.tsx

# API 호출 관련 코드 검색
echo "\nAPI 호출 관련 코드:"
grep -n "api" src/app/[lng]/\(with-page-layout\)/\(with-sidebar-layout\)/notice/[id]/SendPushNotificationAlert.tsx

# isEditable 로직 검색
echo "\nisEditable 로직:"
grep -n "isEditable" src/app/[lng]/\(with-page-layout\)/\(with-sidebar-layout\)/notice/[id]/SendPushNotificationAlert.tsx

Length of output: 2402

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between fa25640 and 21f44c7.

📒 Files selected for processing (7)
  • src/app/[lng]/(with-page-layout)/(with-sidebar-layout)/notice/[id]/SendPushNotificationAlert.tsx (1 hunks)
  • src/app/[lng]/(with-page-layout)/(with-sidebar-layout)/notice/[id]/page.tsx (2 hunks)
  • src/app/[lng]/(with-page-layout)/(without-sidebar-layout)/mypage/MypageButton.tsx (1 hunks)
  • src/app/[lng]/write/NoticeEditor.tsx (1 hunks)
  • src/app/components/layout/Footer/Footer.tsx (1 hunks)
  • src/app/i18next/locales/en/translation.json (2 hunks)
  • src/app/i18next/locales/ko/translation.json (2 hunks)
🧰 Additional context used
🪛 Biome
src/app/components/layout/Footer/Footer.tsx

[error] 62-62: Missing key property for this element in iterable.

The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.

(lint/correctness/useJsxKeyInIterable)

🔇 Additional comments (12)
src/app/[lng]/(with-page-layout)/(with-sidebar-layout)/notice/[id]/SendPushNotificationAlert.tsx (3)

1-12: LGTM: 임포트 및 타입 정의가 적절합니다.

필요한 모듈들이 올바르게 임포트되었고, SendPushNotificationAlertProps 타입이 잘 정의되어 있습니다. Pick 타입을 사용하여 NoticeDetail에서 필요한 속성만 선택한 것은 좋은 방식입니다.


38-40: isEditable 로직 검토 필요

PR 목표에서 언급된 대로, isEditable 로직에 대한 변경이 필요한지 검토가 필요합니다. 현재 구현은 남은 시간을 기준으로 편집 가능 여부를 결정하고 있습니다. 이 로직이 푸시 알림 기능 추가와 관련하여 적절한지 확인해주세요.

추가로, 알림 전송 가능 시간이 편집 가능 시간과 다를 수 있는지 고려해보시기 바랍니다. 만약 다르다면, 별도의 isNotificationSendable 같은 변수를 추가하는 것이 좋을 수 있습니다.

이 로직에 대해 추가적인 요구사항이나 명확히 해야 할 부분이 있다면 알려주세요.


1-55: 전체 평가: 잘 구현되었으나 몇 가지 개선 필요

SendPushNotificationAlert 컴포넌트가 전반적으로 잘 구현되었습니다. 주요 기능과 구조가 PR 목표에 부합하며, 타입 안전성과 국제화가 적절히 고려되었습니다.

개선이 필요한 주요 사항:

  1. 푸시 알림 전송 로직 구현 (TODO 항목)
  2. isEditable 로직 검토 및 필요시 조정
  3. 시간 업데이트 로직 최적화
  4. 접근성 개선 (버튼 사용 및 ARIA 레이블 추가)

이러한 개선사항들을 적용하면 컴포넌트의 기능성, 성능, 그리고 접근성이 더욱 향상될 것입니다.

제안된 변경사항에 대해 추가 논의나 명확히 해야 할 점이 있다면 알려주세요.

src/app/components/layout/Footer/Footer.tsx (1)

61-62: ⚠️ Potential issue

PR 목표와 일치하지 않는 변경사항

PR 목표에 따르면 'CSLink' 컴포넌트의 주석을 제거해야 하지만, 이 변경사항은 오히려 'CSLink'를 주석 처리하고 있습니다. 이는 의도된 변경인지 확인이 필요합니다.

다음 사항들을 고려해 주세요:

  1. PR 목표와 일치하도록 'CSLink' 컴포넌트의 주석을 제거할 것인지
  2. 'bugReport' 링크를 제거하는 것이 의도된 것인지
  3. 만약 'bugReport' 링크를 제거하려는 것이라면, 빈 프래그먼트 대신 해당 항목을 완전히 제거하는 것이 좋을 수 있습니다.

'bugReport' 링크의 사용 여부를 확인하기 위해 다음 스크립트를 실행해 주세요:

🧰 Tools
🪛 Biome

[error] 62-62: Missing key property for this element in iterable.

The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.

(lint/correctness/useJsxKeyInIterable)

src/app/[lng]/(with-page-layout)/(with-sidebar-layout)/notice/[id]/page.tsx (1)

12-12: LGTM: SendPushNotificationAlert 컴포넌트 가져오기

새로운 SendPushNotificationAlert 컴포넌트를 가져오는 것이 적절해 보입니다. 이는 PR의 목표인 푸시 알림 기능 구현과 일치합니다.

src/app/i18next/locales/ko/translation.json (3)

129-132: 알림 전송 관련 새로운 번역이 추가되었습니다.

새로운 sentPushNotificationAlert 섹션이 추가되었습니다. 이는 PR의 목적인 푸시 알림 기능 구현과 일치합니다.


313-313: 국제화된 추가 공지 복사 기능에 대한 번역이 추가되었습니다.

copyInternationalAdditionalNotice 항목이 추가되었습니다. 이는 PR 목적에 직접적으로 언급되지 않았지만, 관련 기능으로 보입니다.


314-314: 푸시 알림 지연에 대한 안내 메시지가 추가되었습니다.

pushWillDelayedNotice 항목이 추가되었습니다. 이는 PR의 목적인 푸시 알림 기능 구현과 관련이 있으며, 사용자에게 중요한 정보를 제공합니다.

다음 스크립트를 실행하여 이 메시지가 애플리케이션에서 적절히 사용되고 있는지 확인해 주세요:

✅ Verification successful

다음 스크립트를 실행하여 pushWillDelayedNotice 메시지가 애플리케이션에서 적절히 사용되고 있는지 확인해 주세요:


pushWillDelayedNotice 메시지가 애플리케이션에서 적절히 사용되고 있습니다.

  • src/app/[lng]/write/NoticeEditor.tsx 파일에서 사용 확인됨.
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 설명: pushWillDelayedNotice 메시지의 사용을 확인합니다.

# 테스트: pushWillDelayedNotice 메시지가 사용되는 위치를 검색합니다.
rg --type typescript --type javascript "pushWillDelayedNotice"

Length of output: 97


Script:

#!/bin/bash
# 설명: pushWillDelayedNotice 메시지의 사용을 확인합니다.

# 테스트: pushWillDelayedNotice 메시지가 사용되는 위치를 검색합니다.
rg --type ts --type js "pushWillDelayedNotice"

Length of output: 136

src/app/i18next/locales/en/translation.json (3)

138-141: 새로운 푸시 알림 관련 번역이 추가되었습니다.

새로운 sentPushNotificationAlert 섹션이 추가되어 푸시 알림 기능을 지원합니다. 이는 PR 목표와 일치합니다.

다음 사항을 고려해 보세요:

  1. 메시지가 사용자에게 명확한지 확인하세요.
  2. 다른 언어 파일(예: 한국어)에도 동일한 번역이 추가되었는지 확인하세요.

327-327: 국제 추가 공지 복사 텍스트가 수정되었습니다.

copyInternationalAdditionalNotice 항목이 "Copy En. add. notice"로 변경되었습니다. 이는 더 간결하고 명확해 보입니다.


328-328: 푸시 알림 지연에 대한 새로운 메시지가 추가되었습니다.

새로운 pushWillDelayedNotice 항목이 추가되어 푸시 알림의 지연 시간과 긴급 알림 옵션에 대해 설명합니다. 이는 PR 목표와 일치하며 사용자에게 유용한 정보를 제공합니다.

다음 사항을 확인해 주세요:

  1. 메시지가 기술적으로 정확한지 확인하세요 (예: 15분 지연이 정확한 시간인지).
  2. 긴급 알림 버튼의 정확한 위치와 이름을 언급하는 것이 도움이 될 수 있습니다.
src/app/[lng]/write/NoticeEditor.tsx (1)

186-190: 사용자에게 푸시 알림 지연에 대한 정보 제공

사용자에게 공지사항 제출 시 푸시 알림이 지연될 것임을 알리는 알림이 적절하게 추가되었습니다.

@enc2586 enc2586 changed the title feat: push notification Modal and Alert feat: push notification 15min Modal and Alert Oct 16, 2024
Copy link

@dohyun-ko
Copy link
Member

@dohyun-ko 아직 완성되진 않았지만, 코드 제공하며 여쭤볼 게 있어서 PR올렸습니다

  • 알림 보내기 요청을 어떤 api로 보내야 하는지
  • 알림 보내기 기능 추가에 따른 isEditable 로직 변경 가능성
    (현재는 createdAt에서 15분이 지나면 막는 중 > 알림을 보냈을 경우도 막아야 할 것으로 보임 > how?)

위 두 사항이 궁금합니다!

세상에 이걸 못봤었다니 죄송합니다 ㅠㅠ
2번째는 어제 해결됐고 1번째는 api 문서에서 찾으신거죠??

Copy link
Contributor

@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: 4

🧹 Outside diff range and nitpick comments (4)
src/app/[lng]/(with-page-layout)/(with-sidebar-layout)/notice/[id]/SendPushNotificationAlert.tsx (1)

16-23: prop 유효성 검사 추가 제안

필수 props에 대한 유효성 검사를 추가하면 좋을 것 같습니다.

const SendPushNotificationAlert = ({
  id,
  author,
  publishedAt,
  lng,
}: PropsWithLng<SendPushNotificationAlertProps>): JSX.Element | null => {
  const { t } = useTranslation(lng);
+ 
+ if (!id || !author || !publishedAt) {
+   console.warn('SendPushNotificationAlert: Required props missing');
+   return null;
+ }
src/app/i18next/locales/ko/translation.json (2)

129-132: 알림 전송 후 수정 불가능 경고 메시지 검토

경고 메시지가 명확하고 사용자에게 중요한 정보를 전달하고 있습니다. 하지만 사용자 경험을 개선하기 위해 좀 더 자세한 설명이 필요할 수 있습니다.

다음과 같이 메시지를 더 상세하게 수정하는 것을 제안드립니다:

   "sentPushNotificationAlert": {
-    "title": "알림을 보내면 수정이 불가능합니다.",
+    "title": "알림을 보내면 공지 내용을 더 이상 수정할 수 없습니다. 계속하시겠습니까?",
     "action": "알림 보내기"
   }

314-317: 알림 관련 메시지 일관성 검토

알림 관련 메시지들이 잘 구성되어 있습니다. 다만 메시지 끝의 구두점 사용이 일관적이지 않습니다.

다음과 같이 구두점을 일관되게 수정하는 것을 제안드립니다:

-      "pushWillDelayedNotice": "15분 뒤에 사용자들에게 푸시알람이 전송됩니다. 긴급한 경우 내 공지 > 알림 보내기 기능을 사용하세요.",
+      "pushWillDelayedNotice": "15분 뒤에 사용자들에게 푸시알람이 전송됩니다. 긴급한 경우 내 공지 > 알림 보내기 기능을 사용하세요",
       "sendPushNotice": "모든 사용자들에게 알림이 전송됩니다. 정말 지금 알림을 보낼까요?",
-      "sendingAlarmNotice":"알림 보내는 중...", 
+      "sendingAlarmNotice": "알림 보내는 중...",
-      "sendPushNoticeFail": "알림 전송에 실패했습니다"
+      "sendPushNoticeFail": "알림 전송에 실패했습니다."

또한 sendingAlarmNotice 키 앞의 공백이 누락되어 있습니다.

src/app/i18next/locales/en/translation.json (1)

327-331: 알림 메시지의 명확성을 개선하면 좋겠습니다.

pushWillDelayedNotice 메시지에서 긴급한 경우의 대안을 더 명확하게 설명하면 좋을 것 같습니다.

다음과 같이 수정하는 것을 제안드립니다:

-      "pushWillDelayedNotice": "Users will receive a push notification after 15 minutes. If it's urgent, you can use Send Notification button in your notice page.",
+      "pushWillDelayedNotice": "Users will receive a push notification after 15 minutes. For urgent notices, you can send immediate notifications using the 'Send Notification' button on your notice page.",
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 21f44c7 and ee16def.

📒 Files selected for processing (7)
  • .vscode/settings.json (1 hunks)
  • src/api/notice/notice.ts (1 hunks)
  • src/api/notice/send-alarm.ts (1 hunks)
  • src/app/[lng]/(with-page-layout)/(with-sidebar-layout)/notice/[id]/SendPushNotificationAlert.tsx (1 hunks)
  • src/app/[lng]/write/page.tsx (2 hunks)
  • src/app/i18next/locales/en/translation.json (2 hunks)
  • src/app/i18next/locales/ko/translation.json (2 hunks)
✅ Files skipped from review due to trivial changes (2)
  • .vscode/settings.json
  • src/app/[lng]/write/page.tsx
🔇 Additional comments (6)
src/api/notice/send-alarm.ts (1)

1-2: 적절한 의존성 가져오기입니다!

타입 안전성을 위해 Notice 타입을 가져오고 API 호출을 위해 ziggleApi를 가져오는 것이 적절합니다.

src/app/[lng]/(with-page-layout)/(with-sidebar-layout)/notice/[id]/SendPushNotificationAlert.tsx (2)

1-15: 코드가 깔끔하고 잘 구성되어 있습니다!

타입스크립트의 Pick 유틸리티 타입을 활용하여 인터페이스를 명확하게 정의했습니다.


24-55: ⚠️ Potential issue

알림 데이터 업데이트 로직 구현 필요

알림 전송 후 데이터 업데이트 로직이 아직 구현되지 않았습니다. 다음 사항들을 고려해주세요:

  1. 알림 전송 성공 시 사용자에게 성공 메시지 표시
  2. 데이터 리프레시를 위한 상태 관리 구현 (예: React Query 사용)
src/api/notice/notice.ts (1)

47-47: publishedAt 속성의 타입 및 문서화 개선이 필요합니다.

publishedAt 속성이 공지사항의 게시 시간을 나타내는 것으로 보입니다. 다음 사항들을 고려해주세요:

  1. 임시저장된 공지사항의 경우를 대비하여 null 값을 허용하는 것이 좋을 것 같습니다.
  2. 속성의 목적과 사용 사례를 문서화하면 좋겠습니다.

다음과 같이 수정하는 것을 제안드립니다:

- publishedAt: dayjs.Dayjs | string;
+ /** 공지사항이 실제로 게시된 시간입니다. 임시저장 상태인 경우 null입니다. */
+ publishedAt: dayjs.Dayjs | string | null;

푸시 알림 컴포넌트에서 이 속성이 어떻게 사용되는지 확인이 필요합니다:

src/app/i18next/locales/ko/translation.json (1)

129-132: 번역키 일관성 검증

새로 추가된 알림 관련 번역 키들이 다른 언어 파일에도 존재하는지 확인이 필요합니다.

Also applies to: 314-317

✅ Verification successful

번역키가 모든 언어 파일에서 일관되게 존재함을 확인했습니다.

영어 번역 파일에서도 다음 키들이 모두 존재하는 것을 확인했습니다:

  • sentPushNotificationAlert
  • pushWillDelayedNotice
  • sendPushNotice
  • sendingAlarmNotice
  • sendPushNoticeFail
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: 새로 추가된 번역 키들이 영어 번역 파일에도 존재하는지 확인

# 영어 번역 파일에서 새로 추가된 키들을 검색
echo "영어 번역 파일에서 키 검색 중..."
rg -A 1 '"sentPushNotificationAlert"|"pushWillDelayedNotice"|"sendPushNotice"|"sendingAlarmNotice"|"sendPushNoticeFail"' src/app/i18next/locales/en/translation.json

Length of output: 716

src/app/i18next/locales/en/translation.json (1)

138-141: 알림 전송 관련 메시지가 명확하게 작성되었습니다!

사용자에게 알림 전송 후 편집이 불가능하다는 점을 명확하게 전달하고 있습니다.

Copy link

Copy link

Copy link
Contributor

@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: 4

🧹 Outside diff range and nitpick comments (1)
src/app/[lng]/(with-page-layout)/(with-sidebar-layout)/notice/[id]/SendPushNotificationAlert.tsx (1)

14-22: Props 유효성 검사 추가 필요

컴포넌트의 안정성을 높이기 위해 props에 대한 유효성 검사가 필요합니다.

const SendPushAlarm = ({
  id,
  author,
  publishedAt,
  lng,
}: PropsWithLng<SendPushAlarmProps>): JSX.Element | null => {
+ if (!id || !author || !publishedAt || !lng) {
+   console.warn('SendPushAlarm: Required props missing');
+   return null;
+ }
  const { t } = useTranslation(lng);
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between ee16def and 58aad3e.

📒 Files selected for processing (4)
  • src/app/[lng]/(with-page-layout)/(with-sidebar-layout)/notice/[id]/SendPushNotificationAlert.tsx (1 hunks)
  • src/app/[lng]/(with-page-layout)/(with-sidebar-layout)/notice/[id]/getTimeDiff.ts (1 hunks)
  • src/app/[lng]/(with-page-layout)/(with-sidebar-layout)/notice/[id]/page.tsx (2 hunks)
  • src/app/[lng]/(with-page-layout)/(without-sidebar-layout)/mypage/MypageButton.tsx (2 hunks)
🔇 Additional comments (6)
src/app/[lng]/(with-page-layout)/(without-sidebar-layout)/mypage/MypageButton.tsx (2)

5-5: 번역 기능 관련 import 추가가 적절합니다!

비동기 번역 처리를 위한 createTranslation과 타입 안전성을 위한 PropsWithLng import가 올바르게 추가되었습니다.


43-44: 비동기 컴포넌트 변환에 대한 검증이 필요합니다.

비동기 컴포넌트로의 전환이 서버 사이드 렌더링(SSR)에 미치는 영향을 확인해주세요:

  • 페이지 로딩 성능
  • 하이드레이션 과정
  • SEO 영향
✅ Verification successful

Let me analyze the results and generate a final response in Korean.

비동기 컴포넌트 변환은 적절하게 구현되었습니다.

검증 결과:

  • 다수의 컴포넌트들이 이미 동일한 비동기 패턴을 사용하고 있습니다 (예: Zabo, ResultZabo, Footer 등)
  • 클라이언트 컴포넌트('use client')와 서버 컴포넌트가 적절히 분리되어 있어 하이드레이션 과정에 문제가 없습니다
  • SEO와 관련된 주요 컴포넌트들이 서버 컴포넌트로 유지되어 있어 성능상 이점을 유지하고 있습니다
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for other async components and potential SSR implications

# Look for other async components in the codebase
rg "const \w+ = async.*Props" -g "*.tsx"

# Check for RSC usage markers
rg "use client" -g "*.tsx"

Length of output: 3400

src/app/[lng]/(with-page-layout)/(with-sidebar-layout)/notice/[id]/page.tsx (2)

12-12: 임포트 구문이 깔끔하게 작성되었습니다!

컴포넌트 임포트가 파일의 기존 임포트 구조와 잘 어울립니다.


75-76: SendPushAlarm 컴포넌트 구현 검증이 필요합니다.

PR 설명에 따르면 알림 발송 API에 대한 명확한 이해가 필요한 상태입니다. 다음 사항들을 확인해주세요:

  1. notice 객체에서 전달되는 모든 props가 SendPushAlarm 컴포넌트에서 올바르게 사용되는지
  2. API 연동 부분이 구현되었는지
  3. 알림 발송 로직이 정상적으로 작동하는지

API 연동이나 알림 발송 로직 구현에 도움이 필요하시다면 말씀해 주세요. 함께 해결방안을 찾아보겠습니다.

✅ Verification successful

Let me check the implementation details of the SendPushAlarm component and the API integration.


SendPushAlarm 컴포넌트가 정상적으로 구현되어 있습니다.

SendPushAlarm 컴포넌트와 API 연동이 모두 적절하게 구현되어 있음을 확인했습니다:

  • 필요한 props(id, author, publishedAt)가 올바르게 전달되고 사용됨
  • /notice/${id}/alarm API 엔드포인트를 통한 알림 발송 기능 구현
  • 사용자 확인 및 에러 처리가 포함된 알림 발송 로직 구현
  • 작성자 본인만 알림을 발송할 수 있도록 제한
  • 시간 제한 로직이 적절히 구현됨
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# SendPushAlarm 컴포넌트의 구현 상태를 확인합니다
ast-grep --pattern 'export default function SendPushAlarm' ./src/app/[lng]

# 알림 발송 API 사용 여부를 확인합니다
rg -l "sendNoticeAlarm" 

Length of output: 228


Script:

#!/bin/bash
# Check SendPushAlarm component implementation
cat src/app/[lng]/\(with-page-layout\)/\(with-sidebar-layout\)/notice/[id]/SendPushNotificationAlert.tsx

# Check API implementation
cat src/api/notice/send-alarm.ts

Length of output: 3681

src/app/[lng]/(with-page-layout)/(with-sidebar-layout)/notice/[id]/SendPushNotificationAlert.tsx (2)

59-61: 세션 상태 처리 개선 필요

이전 리뷰 코멘트에서 언급된 세션 상태 처리 개선이 아직 적용되지 않았습니다. 세션 로딩 및 인증되지 않은 상태에 대한 처리가 필요합니다.


93-117: 구현이 잘 되었습니다!

컴포넌트의 가시성 전환 애니메이션과 접근성 기능이 적절하게 구현되었습니다.

@enc2586 enc2586 self-assigned this Nov 17, 2024
Copy link

Copy link

@enc2586
Copy link
Member Author

enc2586 commented Nov 18, 2024

  • BE이슈가 해결되면 머지 예정

Copy link

Copy link
Contributor

@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: 2

🧹 Outside diff range and nitpick comments (4)
src/app/[lng]/(with-page-layout)/(with-sidebar-layout)/notice/[id]/SendPushNotificationAlert.tsx (2)

14-22: 타입 안전성 개선이 필요합니다

NoticeDetail 인터페이스에서 필요한 필드만 선택적으로 사용하는 것은 좋은 접근이지만, 타입 안전성을 더욱 강화할 수 있습니다.

-interface SendPushAlarmProps
-  extends Pick<NoticeDetail, 'id' | 'author' | 'publishedAt'> {}
+interface SendPushAlarmProps {
+  id: NoticeDetail['id'];
+  author: Required<Pick<NoticeDetail['author'], 'uuid'>>;
+  publishedAt: NonNullable<NoticeDetail['publishedAt']>;
+}

111-135: 접근성과 애니메이션 성능 개선이 필요합니다

현재 구현된 접근성 기능과 애니메이션은 좋지만, 다음과 같은 개선사항을 제안합니다:

 <div
   className={`transform transition-all duration-1000 ease-in-out ${
     showComponent ? 'max-h-screen' : 'max-h-0 overflow-hidden'
   }`}
+  role="alert"
+  aria-live="polite"
 >
   <div
     className={`inline-flex w-full items-start justify-start gap-1.5 rounded-[15px] bg-[#fff4f0] px-5 py-[15px] font-normal text-primary`}
+    style={{ willChange: 'transform' }}
   >
     <span>{t('zabo.sentPushNotificationAlert.title')} </span>
     <span
       className="cursor-pointer font-bold underline"
       onClick={handleSendPushNotification}
       tabIndex={0}
+      role="button"
+      aria-label={t('zabo.sentPushNotificationAlert.action')}
       onKeyDown={(e) => {
         if (e.key === 'Enter' || e.key === ' ') {
           handleSendPushNotification();
         }
       }}
     >
src/app/i18next/locales/ko/translation.json (1)

316-316: 메시지 형식의 일관성 개선이 필요합니다.

다른 메시지들과의 일관성을 위해 콜론(:) 뒤에 공백을 추가해주세요.

-      "sendingAlarmNotice":"알림 보내는 중...", 
+      "sendingAlarmNotice": "알림 보내는 중...", 
src/app/i18next/locales/en/translation.json (1)

327-332: 푸시 알림 관련 메시지들이 잘 구성되어 있습니다

알림 전송의 전체 흐름(지연 알림, 확인, 전송 중, 성공, 실패)을 잘 커버하고 있습니다. 다만 다음과 같은 개선사항을 제안드립니다:

  1. "sendPushNotice" 메시지에 알림이 전송될 사용자 범위를 더 구체적으로 명시하면 좋을 것 같습니다.
  2. "sendingAlarmNotice" 메시지 끝에 불필요한 공백이 있습니다.

다음과 같이 수정을 제안드립니다:

-      "sendPushNotice": "Notifications will be sent to all users. Are you sure you want to send them now?",
+      "sendPushNotice": "Push notifications will be sent to all Ziggle users. Are you sure you want to proceed?",
-      "sendingAlarmNotice":"Sending notifications...", 
+      "sendingAlarmNotice": "Sending notifications..."
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between ba2672e and c19a24f.

📒 Files selected for processing (3)
  • src/app/[lng]/(with-page-layout)/(with-sidebar-layout)/notice/[id]/SendPushNotificationAlert.tsx (1 hunks)
  • src/app/i18next/locales/en/translation.json (2 hunks)
  • src/app/i18next/locales/ko/translation.json (2 hunks)
🔇 Additional comments (3)
src/app/i18next/locales/ko/translation.json (2)

129-132: 알림 전송 관련 메시지가 명확하게 작성되었습니다!

사용자에게 알림 전송 후 수정이 불가능하다는 점을 명확하게 전달하고 있습니다.


314-318: 알림 상태 메시지가 체계적으로 구성되었습니다!

알림 전송의 전체 흐름(지연 알림, 확인, 전송 중, 성공, 실패)이 사용자에게 잘 전달될 수 있도록 구성되어 있습니다.

src/app/i18next/locales/en/translation.json (1)

138-141: 알림 전송 관련 경고 메시지가 명확하게 작성되었습니다

사용자에게 알림 전송 후 수정이 불가능하다는 점을 명확하게 전달하고 있습니다.

…[id]/SendPushNotificationAlert.tsx

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link

@enc2586 enc2586 merged commit 257c5f1 into master Nov 18, 2024
3 checks passed
@enc2586 enc2586 deleted the 342-feature-push-noti-is-now-delayed-15-min branch November 18, 2024 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] push noti is now delayed 15 min
2 participants