Repository navigation
06.26.최신화 - 동적 사이트맵 및 회원가입 약관 동의 업데이트 - #420
Conversation
Feat/380 dynamic sitemap
…sitemap conflict using Promise.allSettled)
…dal-URL Feat/414 terms agreement modal url
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe PR makes sitemap generation async and paginated for story, club, and news routes, and rewrites the marketing, privacy, third-party consent, and terms-agreement screens to render from structured consent data constants. ChangesDynamic sitemap routes
Consent documents
Sequence Diagram(s)sequenceDiagram
participant sitemapRoute
participant storyService
participant clubService
participant newsService
participant storySitemapEndpoint
participant clubSitemapEndpoint
participant newsSitemapEndpoint
sitemapRoute->>storyService: fetchAllSitemapItems()
sitemapRoute->>clubService: fetchAllSitemapItems()
sitemapRoute->>newsService: fetchAllSitemapItems()
storyService->>storySitemapEndpoint: GET cursorId + limit
clubService->>clubSitemapEndpoint: GET cursorId + limit
newsService->>newsSitemapEndpoint: GET cursorId + limit
storySitemapEndpoint-->>storyService: SitemapListResponse pages
clubSitemapEndpoint-->>clubService: SitemapListResponse pages
newsSitemapEndpoint-->>newsService: SitemapListResponse pages
sitemapRoute->>sitemapRoute: Promise.allSettled() merges staticRoutes + dynamic routes
Estimated review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Code Review
This pull request implements dynamic sitemap generation by fetching stories, clubs, and news data in parallel, and refactors the legal and marketing consent pages to use centralized constants instead of the deprecated TermsMarkdown component. The review feedback highlights several areas for improvement to prevent runtime errors, specifically recommending defensive checks in the sitemap fetching loop and replacing non-null assertions with safe fallback values in the API services.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const response = await fetchFn(currentCursor, 5000); // 백엔드 최대 허용 limit | ||
| if (response.items) { | ||
| allItems = allItems.concat(response.items); | ||
| } | ||
| hasMore = response.hasNext; | ||
| if (hasMore && response.nextCursor) { | ||
| currentCursor = response.nextCursor; | ||
| } else { | ||
| hasMore = false; | ||
| } |
There was a problem hiding this comment.
fetchFn이 예기치 않게 undefined 또는 null을 반환할 경우, response.items에 접근할 때 런타임 에러(TypeError: Cannot read properties of undefined)가 발생할 수 있습니다. response가 유효한지 먼저 확인하는 방어적 코드를 추가하고, 잘못된 들여쓰기(indentation)도 함께 수정하는 것이 좋습니다.
const response = await fetchFn(currentCursor, 5000); // 백엔드 최대 허용 limit
if (!response) {
break;
}
if (response.items) {
allItems = allItems.concat(response.items);
}
hasMore = response.hasNext;
if (hasMore && response.nextCursor) {
currentCursor = response.nextCursor;
} else {
hasMore = false;
}| getClubsSitemap: async (cursorId?: number, limit: number = 1000): Promise<SitemapListResponse> => { | ||
| const res = await apiClient.get<ApiResponse<SitemapListResponse>>(CLUBS.sitemap, { | ||
| params: { cursorId, limit }, | ||
| }); | ||
| return res.result!; | ||
| }, |
There was a problem hiding this comment.
API 응답에서 res.result가 없거나 실패했을 경우, Non-null assertion(!)을 사용하면 런타임에 undefined가 반환되어 이를 호출하는 곳에서 에러가 발생할 수 있습니다. 안전하게 기본값을 반환하거나 방어 코드를 작성하는 것이 좋습니다.
| getClubsSitemap: async (cursorId?: number, limit: number = 1000): Promise<SitemapListResponse> => { | |
| const res = await apiClient.get<ApiResponse<SitemapListResponse>>(CLUBS.sitemap, { | |
| params: { cursorId, limit }, | |
| }); | |
| return res.result!; | |
| }, | |
| getClubsSitemap: async (cursorId?: number, limit: number = 1000): Promise<SitemapListResponse> => { | |
| const res = await apiClient.get<ApiResponse<SitemapListResponse>>(CLUBS.sitemap, { | |
| params: { cursorId, limit }, | |
| }); | |
| return res.result || { items: [], hasNext: false, nextCursor: null, pageSize: limit }; | |
| }, |
| getNewsSitemap: async (cursorId?: number, limit: number = 1000): Promise<SitemapListResponse> => { | ||
| const response = await apiClient.get<ApiResponse<SitemapListResponse>>( | ||
| NEWS_ENDPOINTS.SITEMAP, | ||
| { params: { cursorId, limit } } | ||
| ); | ||
| return response.result!; | ||
| }, |
There was a problem hiding this comment.
response.result!와 같이 Non-null assertion을 사용하면 API 응답에 result가 없을 때 런타임 에러가 발생할 수 있습니다. 안전하게 기본값을 제공하도록 수정하는 것을 권장합니다.
| getNewsSitemap: async (cursorId?: number, limit: number = 1000): Promise<SitemapListResponse> => { | |
| const response = await apiClient.get<ApiResponse<SitemapListResponse>>( | |
| NEWS_ENDPOINTS.SITEMAP, | |
| { params: { cursorId, limit } } | |
| ); | |
| return response.result!; | |
| }, | |
| getNewsSitemap: async (cursorId?: number, limit: number = 1000): Promise<SitemapListResponse> => { | |
| const response = await apiClient.get<ApiResponse<SitemapListResponse>>( | |
| NEWS_ENDPOINTS.SITEMAP, | |
| { params: { cursorId, limit } } | |
| ); | |
| return response.result || { items: [], hasNext: false, nextCursor: null, pageSize: limit }; | |
| }, |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
src/services/clubService.ts (1)
17-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer
extractResultover the!non-null assertion for consistency.
storyService.getStoriesSitemapusesextractResult(...), which throws a clear error whenresultis missing. Hereres.result!silently asserts non-null and would propagateundefineddownstream intofetchAllSitemapItemsif the backend omitsresult. Align with the story service to keep failure handling uniform.♻️ Suggested change
- getClubsSitemap: async (cursorId?: number, limit: number = 1000): Promise<SitemapListResponse> => { - const res = await apiClient.get<ApiResponse<SitemapListResponse>>(CLUBS.sitemap, { - params: { cursorId, limit }, - }); - return res.result!; - }, + getClubsSitemap: async (cursorId?: number, limit: number = 1000): Promise<SitemapListResponse> => { + const res = await apiClient.get<ApiResponse<SitemapListResponse>>(CLUBS.sitemap, { + params: { cursorId, limit }, + }); + return extractResult(res); + },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/clubService.ts` around lines 17 - 22, The getClubsSitemap method is using a non-null assertion on res.result instead of the shared extractResult helper, which can let undefined slip through. Update getClubsSitemap in clubService to match storyService.getStoriesSitemap by calling extractResult on the apiClient.get response and returning that value, so missing result data fails consistently and clearly.src/app/sitemap.ts (1)
9-30: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider a timeout/abort around the paginated fetches.
fetchAllSitemapItemsloops overapiClient.getcalls during route revalidation with no timeout. A slow/hung backend would stall sitemap (re)generation. Since failures are already tolerated per-entity viaPromise.allSettled, adding a request timeout (orAbortController) would let a stuck entity fail fast and still emit static + other dynamic routes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/sitemap.ts` around lines 9 - 30, `fetchAllSitemapItems` currently paginates via repeated `fetchFn` calls with no timeout, so a slow or hung backend can block sitemap generation. Update the `fetchAllSitemapItems` loop (and the `apiClient.get` path it delegates to) to use a request timeout or `AbortController`, and make sure a timed-out page fetch fails fast so the rest of the sitemap work can continue. Keep the pagination behavior intact by preserving `currentCursor`, `hasMore`, and the `SitemapListResponse` flow while adding the abort/timeout handling.src/services/newsService.ts (1)
9-15: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUnsafe non-null assertion on an optional
result.
ApiResponsehere declaresresult?: T, soresponse.result!masks a genuinely possibleundefined. If the backend returns noresult, this resolves toundefined, which then flows intofetchAllSitemapItems(the.itemsguard hides it, buthasNextbecomesundefined). UseextractResult(...)as instoryServiceto fail fast with a clear message instead.♻️ Suggested change
- return response.result!; + return extractResult(response);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/newsService.ts` around lines 9 - 15, The getNewsSitemap method is using an unsafe non-null assertion on ApiResponse.result, which can allow undefined to leak into fetchAllSitemapItems. Update getNewsSitemap to use extractResult(...) like storyService does, so the NewsService response is validated and fails fast with a clear error when result is missing. Keep the change localized to NewsService and preserve the existing API call and return type behavior.src/constants/setting/marketing.ts (1)
1-35: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: extract a shared
ConsentSectiontype.
MARKETING_DATA,PRIVACY_DATA, andTHIRD_PARTY_DATAall share the{ title: string; content: string | string[] }shape and are assigned toTERMS_CONTENT_MAPwith that exact type. Declaring and applying a shared type (e.g.satisfies ConsentSection[]) would catch typos in content shape at the source rather than only at the map assignment.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/constants/setting/marketing.ts` around lines 1 - 35, Extract and apply a shared ConsentSection type for the consent content arrays used by MARKETING_DATA, PRIVACY_DATA, and THIRD_PARTY_DATA so shape errors are caught where the data is defined. Update MARKETING_DATA to conform to that shared type (for example via a typed array declaration or a satisfies ConsentSection[] assertion) while preserving the existing title/content structure, and keep TERMS_CONTENT_MAP relying on the same type.src/app/support/v1/marketing-consent/page.tsx (1)
25-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated section-rendering logic across consent pages.
This
MARKETING_DATA.map(...)block — theArray.isArray(term.content)branch, theDocumentListvs newline-split paragraph rendering, and the key strategy — is byte-for-byte identical inprivacy/page.tsxandthird-party-consent/page.tsx, and near-identical inTermsAgreement.tsx. Consider extracting a smallConsentSectionscomponent (or arenderConsentSectionhelper) that takes the section array, so future formatting changes happen in one place.♻️ Sketch
// src/components/base-ui/PublicDocument/ConsentSections.tsx export function ConsentSections({ data }: { data: { title: string; content: string | string[] }[] }) { return data.map((term) => ( <DocumentSection key={term.title} title={term.title}> {Array.isArray(term.content) ? ( <DocumentList items={term.content} /> ) : ( <div className="flex flex-col gap-2"> {term.content.split("\n").map((line, index) => ( <p key={`${line.trim()}-${index}`}>{line.trim()}</p> ))} </div> )} </DocumentSection> )); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/support/v1/marketing-consent/page.tsx` around lines 25 - 37, The section rendering logic in this `MARKETING_DATA.map(...)` block is duplicated across the consent pages, so extract it into a shared `ConsentSections` component or `renderConsentSection` helper and reuse it here. Move the `Array.isArray(term.content)` branching, `DocumentList` vs newline-split paragraph rendering, and the existing key strategy into that shared helper so `marketing-consent/page.tsx`, `privacy/page.tsx`, `third-party-consent/page.tsx`, and `TermsAgreement` all consume the same implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/sitemap.ts`:
- Around line 16-27: The pagination loop in sitemap generation can stop early
when `response.nextCursor` is `0` or another falsy value, even if
`response.hasNext` is still true. Update the `while (hasMore)` flow in
`sitemap.ts` to advance based on an explicit null/undefined check for
`response.nextCursor` instead of truthiness, using `fetchFn`, `currentCursor`,
and `hasMore` as the key control points. Also add a safeguard in the same loop
to break if the backend returns the same cursor again so `hasNext: true` cannot
cause an endless loop.
In `@src/components/base-ui/Join/steps/TermsAgreement/TermsAgreement.tsx`:
- Around line 15-27: `TermsAgreement.tsx` is using agreement IDs that don’t
match the raw `agreements` keys used by `SignupTerms.tsx` and `SignupContext`,
so the required selections may never resolve correctly. Update `TERMS_DATA` in
`TermsAgreement` to use the same standard IDs as the signup flow (`privacy` and
`terms` instead of `servicePrivacy` and `termsOfUse`), and keep
`TERMS_CONTENT_MAP` aligned with those identifiers so the checked state and
Next-button enablement work consistently.
---
Nitpick comments:
In `@src/app/sitemap.ts`:
- Around line 9-30: `fetchAllSitemapItems` currently paginates via repeated
`fetchFn` calls with no timeout, so a slow or hung backend can block sitemap
generation. Update the `fetchAllSitemapItems` loop (and the `apiClient.get` path
it delegates to) to use a request timeout or `AbortController`, and make sure a
timed-out page fetch fails fast so the rest of the sitemap work can continue.
Keep the pagination behavior intact by preserving `currentCursor`, `hasMore`,
and the `SitemapListResponse` flow while adding the abort/timeout handling.
In `@src/app/support/v1/marketing-consent/page.tsx`:
- Around line 25-37: The section rendering logic in this
`MARKETING_DATA.map(...)` block is duplicated across the consent pages, so
extract it into a shared `ConsentSections` component or `renderConsentSection`
helper and reuse it here. Move the `Array.isArray(term.content)` branching,
`DocumentList` vs newline-split paragraph rendering, and the existing key
strategy into that shared helper so `marketing-consent/page.tsx`,
`privacy/page.tsx`, `third-party-consent/page.tsx`, and `TermsAgreement` all
consume the same implementation.
In `@src/constants/setting/marketing.ts`:
- Around line 1-35: Extract and apply a shared ConsentSection type for the
consent content arrays used by MARKETING_DATA, PRIVACY_DATA, and
THIRD_PARTY_DATA so shape errors are caught where the data is defined. Update
MARKETING_DATA to conform to that shared type (for example via a typed array
declaration or a satisfies ConsentSection[] assertion) while preserving the
existing title/content structure, and keep TERMS_CONTENT_MAP relying on the same
type.
In `@src/services/clubService.ts`:
- Around line 17-22: The getClubsSitemap method is using a non-null assertion on
res.result instead of the shared extractResult helper, which can let undefined
slip through. Update getClubsSitemap in clubService to match
storyService.getStoriesSitemap by calling extractResult on the apiClient.get
response and returning that value, so missing result data fails consistently and
clearly.
In `@src/services/newsService.ts`:
- Around line 9-15: The getNewsSitemap method is using an unsafe non-null
assertion on ApiResponse.result, which can allow undefined to leak into
fetchAllSitemapItems. Update getNewsSitemap to use extractResult(...) like
storyService does, so the NewsService response is validated and fails fast with
a clear error when result is missing. Keep the change localized to NewsService
and preserve the existing API call and return type behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 74989e20-c6ca-42a4-a819-9b0c75870986
📒 Files selected for processing (17)
src/app/sitemap.tssrc/app/support/v1/marketing-consent/page.tsxsrc/app/support/v1/privacy/page.tsxsrc/app/support/v1/third-party-consent/page.tsxsrc/components/base-ui/Join/steps/TermsAgreement/TermsAgreement.tsxsrc/components/common/TermsMarkdown.tsxsrc/constants/setting/marketing.tssrc/constants/setting/privacy.tssrc/constants/setting/thirdParty.tssrc/constants/signupTerms.tssrc/lib/api/endpoints/Clubs.tssrc/lib/api/endpoints/bookstory.tssrc/lib/api/endpoints/news.tssrc/services/clubService.tssrc/services/newsService.tssrc/services/storyService.tssrc/types/sitemap.ts
💤 Files with no reviewable changes (2)
- src/constants/signupTerms.ts
- src/components/common/TermsMarkdown.tsx
| while (hasMore) { | ||
| const response = await fetchFn(currentCursor, 5000); // 백엔드 최대 허용 limit | ||
| if (response.items) { | ||
| allItems = allItems.concat(response.items); | ||
| } | ||
| hasMore = response.hasNext; | ||
| if (hasMore && response.nextCursor) { | ||
| currentCursor = response.nextCursor; | ||
| } else { | ||
| hasMore = false; | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Early termination when nextCursor is 0 (or falsy) while hasNext is true.
if (hasMore && response.nextCursor) treats nextCursor === 0 as "no more pages" and stops, silently dropping the rest of the sitemap. If cursor IDs can be 0, prefer an explicit null/undefined check. Also consider guarding against a non-advancing cursor to avoid a potential unbounded loop if the backend repeatedly returns hasNext: true.
♻️ Suggested change
- hasMore = response.hasNext;
- if (hasMore && response.nextCursor) {
- currentCursor = response.nextCursor;
- } else {
- hasMore = false;
- }
+ if (response.hasNext && response.nextCursor != null) {
+ currentCursor = response.nextCursor;
+ } else {
+ hasMore = false;
+ }📝 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.
| while (hasMore) { | |
| const response = await fetchFn(currentCursor, 5000); // 백엔드 최대 허용 limit | |
| if (response.items) { | |
| allItems = allItems.concat(response.items); | |
| } | |
| hasMore = response.hasNext; | |
| if (hasMore && response.nextCursor) { | |
| currentCursor = response.nextCursor; | |
| } else { | |
| hasMore = false; | |
| } | |
| } | |
| while (hasMore) { | |
| const response = await fetchFn(currentCursor, 5000); // 백엔드 최대 허용 limit | |
| if (response.items) { | |
| allItems = allItems.concat(response.items); | |
| } | |
| if (response.hasNext && response.nextCursor != null) { | |
| currentCursor = response.nextCursor; | |
| } else { | |
| hasMore = false; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/sitemap.ts` around lines 16 - 27, The pagination loop in sitemap
generation can stop early when `response.nextCursor` is `0` or another falsy
value, even if `response.hasNext` is still true. Update the `while (hasMore)`
flow in `sitemap.ts` to advance based on an explicit null/undefined check for
`response.nextCursor` instead of truthiness, using `fetchFn`, `currentCursor`,
and `hasMore` as the key control points. Also add a safeguard in the same loop
to break if the backend returns the same cursor again so `hasNext: true` cannot
cause an endless loop.
| const TERMS_DATA = [ | ||
| { id: "servicePrivacy", label: "서비스 이용을 위한 필수 개인정보 수집·이용 동의 (필수)", required: true }, | ||
| { id: "termsOfUse", label: "책모 이용약관 동의 (필수)", required: true }, | ||
| { id: "thirdParty", label: "개인정보 제3자 제공 동의 (선택)", required: false }, | ||
| { id: "marketing", label: "마케팅 및 이벤트 정보 수신 동의 (선택)", required: false }, | ||
| ]; | ||
|
|
||
| const TERMS_CONTENT_MAP: Record<string, { title: string; content: string | string[] }[]> = { | ||
| servicePrivacy: PRIVACY_DATA, | ||
| termsOfUse: TERMS_OF_USE_DATA, | ||
| thirdParty: THIRD_PARTY_DATA, | ||
| marketing: MARKETING_DATA, | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find SignupContext definition and all readers of agreement ids
fd -t f 'SignupContext'
rg -nP "agreements\s*[\.\[]|'(privacy|terms|servicePrivacy|termsOfUse|thirdParty|marketing)'|\"(privacy|terms|servicePrivacy|termsOfUse)\"" src -g '!**/*.test.*' -C2Repository: checkmo2025/FE
Length of output: 4662
🏁 Script executed:
#!/bin/bash
# Inspect SignupContext to find how agreements state is initialized and updated
cat -n src/contexts/SignupContext.tsx | head -150Repository: checkmo2025/FE
Length of output: 6286
Fix agreement ID mismatch between TermsAgreement.tsx and SignupTerms.tsx
The agreements state in SignupContext is a raw map without key normalization. TermsAgreement.tsx uses IDs servicePrivacy and termsOfUse, while SignupTerms.tsx uses privacy and terms. If the flow relies on SignupTerms to set agreement state, or if the backend initializes these keys, TermsAgreement will fail to detect them, keeping the "Next" button disabled.
Align TERMS_DATA IDs in TermsAgreement.tsx to match the standard used in SignupTerms.tsx:
View diff
const TERMS_DATA = [
{ id: "privacy", label: "서비스 이용을 위한 필수 개인정보 수집·이용 동의 (필수)", required: true },
{ id: "terms", label: "책모 이용약관 동의 (필수)", required: true },
{ id: "thirdParty", label: "개인정보 제3자 제공 동의 (선택)", required: false },
{ id: "marketing", label: "마케팅 및 이벤트 정보 수신 동의 (선택)", required: false },
];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/base-ui/Join/steps/TermsAgreement/TermsAgreement.tsx` around
lines 15 - 27, `TermsAgreement.tsx` is using agreement IDs that don’t match the
raw `agreements` keys used by `SignupTerms.tsx` and `SignupContext`, so the
required selections may never resolve correctly. Update `TERMS_DATA` in
`TermsAgreement` to use the same standard IDs as the signup flow (`privacy` and
`terms` instead of `servicePrivacy` and `termsOfUse`), and keep
`TERMS_CONTENT_MAP` aligned with those identifiers so the checked state and
Next-button enablement work consistently.
📌 개요 (Summary)
🛠️ 변경 사항 (Changes)
📸 스크린샷 (Screenshots)
(UI 변경 사항이 있다면 첨부해주세요)
✅ 체크리스트 (Checklist)
pnpm build)pnpm lint)Summary by CodeRabbit
New Features
Bug Fixes