Skip to content

[FEAT] [HIGH-66] QA 사항 반영 및 클릭 로깅 API 연동#57

Merged
0Jaemin0 merged 4 commits into
developfrom
feat/HIGH-66-all-pages-review
Aug 7, 2025
Merged

[FEAT] [HIGH-66] QA 사항 반영 및 클릭 로깅 API 연동#57
0Jaemin0 merged 4 commits into
developfrom
feat/HIGH-66-all-pages-review

Conversation

@neungdong

@neungdong neungdong commented Aug 7, 2025

Copy link
Copy Markdown
Contributor

User description

✨ PR 세부 내용

쇼츠 콘텐츠 클릭 시 호출되는 /content/click API 연동 기능 추가
클릭 API를 사용하는 위치(홈 추천 콘텐츠, 쇼츠 타이틀, 검색 결과 등)에 호출 로직 연동 완료
쇼츠 댓글 컴포넌트 리패치 로직 추가 (refetchQueries)
쇼츠 페이지에서 브라우저 뒤로가기를 고려한 처리 추가

☑️ 체크리스트

🛠 기본 검사 항목

  • ESLint 검사 완료
  • Prettier 적용
  • 함수, 변수, 파일 네이밍 검토
  • 코드 작성 순서 (import, 내부 코드) 점검
  • 폴더 구조에 맞는 파일 분리 여부 확인
  • 코드 작성 스타일 점검

✍️ 작성자 선택 항목

📸 스크린샷

✅ 리뷰 요구사항


PR Type

Enhancement, Bug fix


Description

  • Add content click logging API endpoint

  • Introduce useContentClickMutation hook

  • Integrate click logging in key UI components

  • Fix shorts back navigation and comment refetch


Diagram Walkthrough

flowchart LR
  UI["UI components"] -- "mutatePostContentClick" --> Hook["useContentClickMutation"]
  Hook -- "POST /content/click" --> API["postContentClick"]
  UI -- "navigate(-1)" --> Nav["Back navigation fix"]
Loading

File Walkthrough

Relevant files

@neungdong neungdong self-assigned this Aug 7, 2025
@netlify

netlify Bot commented Aug 7, 2025

Copy link
Copy Markdown

Deploy Preview for lead-me ready!

Name Link
🔨 Latest commit 52f24d1
🔍 Latest deploy log https://app.netlify.com/projects/lead-me/deploys/689420d73828f100097cc224
😎 Deploy Preview https://deploy-preview-57--lead-me.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@github-actions

github-actions Bot commented Aug 7, 2025

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Missing import

The new call to navigate(-1) is added without importing useNavigate from react-router-dom, which will cause a runtime ReferenceError.

navigate(-1);
Missing import

You invoke navigate in the overlay and destructure useNavigate(), but there is no import { useNavigate } from "react-router-dom" at the top of the file.

const navigate = useNavigate();
const { mutatePostContentClick } = useContentClickMutation();
Inline handler

An inline arrow function is used directly in the onClick prop. Consider extracting this to a named handler (e.g., handleContentClick) to improve readability and consistency with event handler naming conventions.

onClick={() =>
  mutatePostContentClick({ contentId: content.contentId })
}

@github-actions

github-actions Bot commented Aug 7, 2025

Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Switch to synchronous mutate

Return the synchronous mutate function instead of mutateAsync to fire-and-forget the
click logging without introducing unhandled Promise usage.

src/hooks/queries/content/useContentClickMutation.ts [9-11]

 return {
-  mutatePostContentClick: postContentClickMutation.mutateAsync,
+  mutatePostContentClick: postContentClickMutation.mutate,
 };
Suggestion importance[1-10]: 6

__

Why: Using mutate instead of mutateAsync prevents unhandled promise warnings when the mutation is fire-and-forget and aligns with the existing component usage.

Low
Add error handling to mutation

Attach an onError callback to the mutation to catch and log failures, preventing
silent errors from going unnoticed.

src/hooks/queries/content/useContentClickMutation.ts [5-7]

 const postContentClickMutation = useMutation({
   mutationFn: postContentClick,
+  onError: (error) => console.error('Content click log failed', error),
 });
Suggestion importance[1-10]: 5

__

Why: Attaching an onError callback improves observability by logging failures, but it’s a minor enhancement to the existing mutation setup.

Low

@0Jaemin0 0Jaemin0 merged commit 89de4d6 into develop Aug 7, 2025
5 checks passed
@0Jaemin0 0Jaemin0 deleted the feat/HIGH-66-all-pages-review branch August 8, 2025 01:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants