FEATURE : QA 결과에 따른 ui 및 api 수정 및 리팩토링#43
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
워크스루이 PR은 보고서 페이지의 평판 데이터 구조를 신고 건수/도메인 생성일 중심으로 개선하고, 스캔 결과 정규화 및 라우팅 시스템을 도입하며, 로딩/결과 페이지의 상세 분석 제어 로직을 모듈화합니다. 또한 Non-URL 실행 보안을 강화하고 프로젝트 문서를 업데이트합니다. 변경 사항보고서 페이지 데이터 구조 및 표현 개선
스캔 결과 정규화 및 라우팅 시스템
로딩 페이지 상세 분석 제어 시스템
결과 페이지 위험도 표현 및 URL 통합
Non-URL 실행 보안 강화
QR 스캔 페이지 라우팅 통합
히스토리 위험도 톤 결정
프로젝트 문서 및 CI/빌드 설정
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
src/pages/Loading/lib/loadingDetailResolution.ts (2)
64-67: ⚡ Quick winPromise 동작이 반환 타입과 일치하지 않습니다.
함수가
Promise<LoadingDetailResolutionStatus>를 반환하지만, 404가 아닌 에러의 경우 Line 66에서 예외를 다시 던집니다. 이는 함수가 resolve(상태 반환)와 reject(예외 던지기) 두 가지 방식으로 종료될 수 있음을 의미하며, 반환 타입만으로는 이를 알 수 없습니다.다음 중 하나를 고려하세요:
- 모든 에러를 잡아서 적절한 상태로 반환 (예:
'error'상태 추가)- JSDoc 또는 주석으로 rethrow 동작을 명시적으로 문서화
- 반환 타입을 변경하여 에러 가능성을 표현
🤖 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/pages/Loading/lib/loadingDetailResolution.ts` around lines 64 - 67, The function currently declares Promise<LoadingDetailResolutionStatus> but rethrows non-404 errors (see isApiError check and throw error), causing inconsistent behavior; instead of rethrowing in the non-404 branch, call onFailure with the error message (using error instanceof Error ? error.message : DETAIL_RESOLUTION_ERROR_MESSAGE) and return a LoadingDetailResolutionStatus value representing failure (e.g., 'error'); update the branch around isApiError()/onFailure to swallow the exception and return that error status so the function always resolves to LoadingDetailResolutionStatus rather than rejecting.
60-62: ⚡ Quick win에러 메시지 문자열 기반 체크는 취약합니다.
Line 60에서
error.message === 'SCAN_SESSION_REQUIRED'로 특수 에러를 식별하는데, 이는 문자열 기반 비교로 오타나 메시지 변경에 취약합니다.커스텀 에러 클래스를 사용하는 것을 권장합니다:
🔧 제안하는 개선 방안
새로운 에러 클래스 정의:
export class ScanSessionRequiredError extends Error { constructor() { super('SCAN_SESSION_REQUIRED'); this.name = 'ScanSessionRequiredError'; } }그리고 체크 로직을 다음과 같이 변경:
- if (error instanceof Error && error.message === 'SCAN_SESSION_REQUIRED') { + if (error instanceof ScanSessionRequiredError) { return 'session-required'; }🤖 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/pages/Loading/lib/loadingDetailResolution.ts` around lines 60 - 62, Replace the brittle string comparison in loadingDetailResolution.ts (the block checking error.message === 'SCAN_SESSION_REQUIRED') with a proper custom error class check: define a ScanSessionRequiredError extends Error (name 'ScanSessionRequiredError' and message 'SCAN_SESSION_REQUIRED'), update any places that currently throw that string to throw new ScanSessionRequiredError(), and change the conditional in the function (the one using error instanceof Error) to use error instanceof ScanSessionRequiredError so the special-case branch reliably detects the condition.src/shared/lib/scan-session/scanUrlResolution.test.ts (1)
5-55: ⚡ Quick win테스트 커버리지를 개선하면 좋습니다.
현재 테스트는 주요 시나리오(리다이렉트, 세션 디코딩, 히스토리 폴백)를 잘 다루고 있습니다. 그러나 모든 소스가 실패하고 빈 문자열로 폴백되는 엣지 케이스에 대한 테스트가 없습니다.
다음과 같은 테스트 케이스 추가를 고려하세요:
it('returns empty string when all URL resolution attempts fail', () => { expect( resolveScanUrls({ sources: [{ unrelatedField: 'value' }], }), ).toMatchObject({ scannedUrl: '', destinationUrl: '', originalUrl: '', }); });🤖 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/shared/lib/scan-session/scanUrlResolution.test.ts` around lines 5 - 55, Add a new unit test for resolveScanUrls to cover the edge case where no URL fields are present on any source and all resolution attempts fail: create a test (e.g. "returns empty string when all URL resolution attempts fail") that calls resolveScanUrls({ sources: [{ unrelatedField: 'value' }] }) and asserts that scannedUrl, destinationUrl, and originalUrl are all empty strings; this ensures resolveScanUrls' fallback-to-empty-string behavior is covered alongside the existing redirect/decoded/history tests.src/pages/Loading/hooks/useLoadingPage.ts (2)
159-176: 💤 Low value
onFinal의 후속 분기들이 모두 같은 조기 종료로 귀결됩니다.
!hasResolvedRiskLevel블록의try/catch이후 세 개의return은 모두 동일 결과(함수 종료)를 가집니다.isResolved/detailResolutionFailedRef검사 결과에 따른 후속 동작 차이가 없다면 가독성/노이즈 측면에서 단순화하는 편이 안전합니다.♻️ 제안 diff
onFinal: async (payload) => { setFinalResult(payload); const hasResolvedRiskLevel = resolveResultToneFromSources([payload], null) !== null; if (!hasResolvedRiskLevel) { - let isResolved = false; - try { - isResolved = await resolveDetailAndOpenResult(); + await resolveDetailAndOpenResult(); } catch { - return; + // resolveDetailAndOpenResult 내부 onFailure 가 알림/플래그 갱신을 담당 } - - if (isResolved) { - return; - } - - if (detailResolutionFailedRef.current) { - return; - } - return; }만약 향후
isResolved/detailResolutionFailedRef별 후속 분기가 의도되어 placeholder로 남긴 것이라면 TODO 주석으로 의도를 남겨두는 편이 좋습니다.🤖 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/pages/Loading/hooks/useLoadingPage.ts` around lines 159 - 176, The code in useLoadingPage surrounding hasResolvedRiskLevel contains three successive early returns after calling resolveDetailAndOpenResult (the isResolved check, the detailResolutionFailedRef check, and a final return) which are redundant; simplify by collapsing them into one early exit path—either remove the redundant isResolved/detailResolutionFailedRef checks and directly return when !hasResolvedRiskLevel after the try/catch, or if those checks are placeholders for future branching, add a clear TODO comment above the isResolved/detailResolutionFailedRef checks noting intended future behavior; locate the logic in the same block that calls resolveDetailAndOpenResult and references isResolved, hasResolvedRiskLevel, and detailResolutionFailedRef to apply the change.
32-34: ⚡ Quick win결과 페이지 진입을 풀-페이지 리로드로 전환한 점을 확인하고, 불필요한 조건문을 정리하세요.
Line 33:
useNavigate(line 62의/리다이렉트)와 달리 결과 페이지 진입만window.location.assign으로 풀-페이지 네비게이션을 강제합니다. SPA 내부 상태 손실, 메모리·HTTP 캐시 초기화, 진행 중인 zustand 영속화 flush 타이밍에 영향이 있을 수 있으니, 의도(예: 결과 라우트 lazy chunk/스토어 초기화 필수)를 문서화하거나 TanStack Router의navigateAPI로 통일하는 것을 검토하세요.Lines 168-176 (
onFinal콜백 내):resolveDetailAndOpenResult()호출 후 3개의 조건부 return이 모두 같은 결과(함수 종료)를 가집니다. 각 조건이 표현하는 의도(resolved 여부, 실패 여부, 미활성 상태)를 명확히 하거나, 공통 로직으로 단순화하는 것을 권장합니다.🤖 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/pages/Loading/hooks/useLoadingPage.ts` around lines 32 - 34, The openResultRouteForCurrentSession function currently forces a full-page reload via window.location.assign(resolveScanResultRoute(getScanSessionSnapshot()).href); change this to use your SPA router's navigate API (e.g., TanStack Router's navigate) or, if a full reload is truly required, add an inline comment documenting why (lazy chunk/store init requirement) so intent is explicit; replace the direct location.assign call with navigate(resolveScanResultRoute(getScanSessionSnapshot())) or equivalent. Also, inside the onFinal callback where resolveDetailAndOpenResult() is called, collapse the three identical conditional returns into a single clear guard (or make each branch's intent explicit with separate boolean-named checks) so the logic either returns once on any failure/inactive/resolved case or documents distinct outcomes, referencing resolveDetailAndOpenResult and onFinal to locate the code to simplify.
🤖 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 `@README.md`:
- Line 47: Update the README sentence so it accurately states that the
postinstall script runs during installation and creates .env.local and
.env.server.local from the example files (i.e., change the current wording that
suggests the opposite conditional to an explicit statement like: "During
installation the postinstall script runs and creates .env.local and
.env.server.local from the example files").
In `@src/pages/Report/constants/threatText.ts`:
- Around line 181-188: The entry with englishLabel 'SHORTENED URL' whose names
array includes ['SHORTENED_URL', 'shortened_url', 'short_url', 'url_shortener']
conflicts with an earlier mapping that also uses 'shortened_url', 'short_url',
and 'url_shortener', causing one to overwrite the other; fix by consolidating
aliases so each alias only appears once across the list—either remove the
duplicate aliases from this 'SHORTENED URL' entry and add any missing aliases to
the intended canonical entry, or merge the two entries into a single canonical
entry (keeping title '단축 URL 감지' and description/risk) and ensure the final
names array contains the unique set of aliases.
---
Nitpick comments:
In `@src/pages/Loading/hooks/useLoadingPage.ts`:
- Around line 159-176: The code in useLoadingPage surrounding
hasResolvedRiskLevel contains three successive early returns after calling
resolveDetailAndOpenResult (the isResolved check, the detailResolutionFailedRef
check, and a final return) which are redundant; simplify by collapsing them into
one early exit path—either remove the redundant
isResolved/detailResolutionFailedRef checks and directly return when
!hasResolvedRiskLevel after the try/catch, or if those checks are placeholders
for future branching, add a clear TODO comment above the
isResolved/detailResolutionFailedRef checks noting intended future behavior;
locate the logic in the same block that calls resolveDetailAndOpenResult and
references isResolved, hasResolvedRiskLevel, and detailResolutionFailedRef to
apply the change.
- Around line 32-34: The openResultRouteForCurrentSession function currently
forces a full-page reload via
window.location.assign(resolveScanResultRoute(getScanSessionSnapshot()).href);
change this to use your SPA router's navigate API (e.g., TanStack Router's
navigate) or, if a full reload is truly required, add an inline comment
documenting why (lazy chunk/store init requirement) so intent is explicit;
replace the direct location.assign call with
navigate(resolveScanResultRoute(getScanSessionSnapshot())) or equivalent. Also,
inside the onFinal callback where resolveDetailAndOpenResult() is called,
collapse the three identical conditional returns into a single clear guard (or
make each branch's intent explicit with separate boolean-named checks) so the
logic either returns once on any failure/inactive/resolved case or documents
distinct outcomes, referencing resolveDetailAndOpenResult and onFinal to locate
the code to simplify.
In `@src/pages/Loading/lib/loadingDetailResolution.ts`:
- Around line 64-67: The function currently declares
Promise<LoadingDetailResolutionStatus> but rethrows non-404 errors (see
isApiError check and throw error), causing inconsistent behavior; instead of
rethrowing in the non-404 branch, call onFailure with the error message (using
error instanceof Error ? error.message : DETAIL_RESOLUTION_ERROR_MESSAGE) and
return a LoadingDetailResolutionStatus value representing failure (e.g.,
'error'); update the branch around isApiError()/onFailure to swallow the
exception and return that error status so the function always resolves to
LoadingDetailResolutionStatus rather than rejecting.
- Around line 60-62: Replace the brittle string comparison in
loadingDetailResolution.ts (the block checking error.message ===
'SCAN_SESSION_REQUIRED') with a proper custom error class check: define a
ScanSessionRequiredError extends Error (name 'ScanSessionRequiredError' and
message 'SCAN_SESSION_REQUIRED'), update any places that currently throw that
string to throw new ScanSessionRequiredError(), and change the conditional in
the function (the one using error instanceof Error) to use error instanceof
ScanSessionRequiredError so the special-case branch reliably detects the
condition.
In `@src/shared/lib/scan-session/scanUrlResolution.test.ts`:
- Around line 5-55: Add a new unit test for resolveScanUrls to cover the edge
case where no URL fields are present on any source and all resolution attempts
fail: create a test (e.g. "returns empty string when all URL resolution attempts
fail") that calls resolveScanUrls({ sources: [{ unrelatedField: 'value' }] })
and asserts that scannedUrl, destinationUrl, and originalUrl are all empty
strings; this ensures resolveScanUrls' fallback-to-empty-string behavior is
covered alongside the existing redirect/decoded/history tests.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 531ef085-70f3-485b-b220-53bbeb5a40c5
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (60)
.github/workflows/ci.yml.gitignore.secretlintignoreREADME.mddocs/convention.mddocs/refactor-roadmap.mdpackage.jsonsrc/features/scan-history/lib/historyItemAccess.test.tssrc/features/scan-history/lib/historyItemAccess.tssrc/pages/Loading/hooks/useLoadingPage.tssrc/pages/Loading/lib/loadingDetailPolling.test.tssrc/pages/Loading/lib/loadingDetailPolling.tssrc/pages/Loading/lib/loadingDetailResolution.test.tssrc/pages/Loading/lib/loadingDetailResolution.tssrc/pages/Loading/lib/loadingProgressResolution.test.tssrc/pages/Loading/lib/loadingProgressResolution.tssrc/pages/QRScan/hooks/useQRScanPage.tssrc/pages/Report/ReportPage.tsxsrc/pages/Report/constants/riskDetectionCatalog.test.tssrc/pages/Report/constants/riskDetectionCatalog.tssrc/pages/Report/constants/threatText.tssrc/pages/Report/lib/reportDomainData.test.tssrc/pages/Report/lib/reportDomainData.tssrc/pages/Report/lib/reportPageDataContext.test.tssrc/pages/Report/lib/reportPageDataContext.tssrc/pages/Report/lib/reportReputationData.test.tssrc/pages/Report/lib/reportReputationData.tssrc/pages/Report/lib/reportServerInfoData.test.tssrc/pages/Report/lib/reportServerInfoData.tssrc/pages/Report/lib/toReportPageData.test.tssrc/pages/Report/lib/toReportPageData.tssrc/pages/Report/styles/reportPage.css.tssrc/pages/Report/types/reportPage.types.tssrc/pages/Result/ResultPage.tsxsrc/pages/Result/api/createResultPageFetcher.test.tssrc/pages/Result/api/createResultPageFetcher.tssrc/pages/Result/hooks/useResultPage.tssrc/pages/Result/lib/toResultPageData.test.tssrc/pages/Result/lib/toResultPageData.tssrc/pages/Result/types/resultPage.types.tssrc/pages/Result/ui/ResultStatusPage.tsxsrc/pages/ResultNonUrl/ResultNonUrlPage.tsxsrc/pages/ResultNonUrl/constants/nonUrlActionText.tssrc/pages/ResultNonUrl/lib/resolveNonUrlActionExecution.test.tssrc/pages/ResultNonUrl/lib/resolveNonUrlActionExecution.tssrc/pages/ResultNonUrl/styles/resultNonUrlPage.css.tssrc/shared/lib/browser/openExternalLink.test.tssrc/shared/lib/browser/openExternalLink.tssrc/shared/lib/scan-session/scanIdentity.test.tssrc/shared/lib/scan-session/scanIdentity.tssrc/shared/lib/scan-session/scanResultNormalization.test.tssrc/shared/lib/scan-session/scanResultNormalization.tssrc/shared/lib/scan-session/scanResultRoute.test.tssrc/shared/lib/scan-session/scanResultRoute.tssrc/shared/lib/scan-session/scanUrlResolution.test.tssrc/shared/lib/scan-session/scanUrlResolution.tssrc/shared/store/scanSessionStore.tssrc/shared/store/scanSessionTransitions.test.tssrc/shared/store/scanSessionTransitions.tssrc/test-code/generate-qr-png.mjs
💤 Files with no reviewable changes (2)
- src/pages/ResultNonUrl/styles/resultNonUrlPage.css.ts
- src/pages/ResultNonUrl/constants/nonUrlActionText.ts
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.
Summary
closed #42
QR 분석 결과 처리 흐름의 안정성, 보안성, 유지보수성을 높이기 위해 스캔 응답 정규화, 상태 전환, 로딩/SSE 처리, 리포트 데이터 조립 로직을 단계적으로 리팩토링했습니다.
또한 README와 컨벤션/로드맵 문서를 현재 구조에 맞게 최신화했습니다.
Tasks
typecheck스크립트 및 검증 흐름 보강scanSessionStore상태 전환 로직을 순수 함수로 분리pnpm format,pnpm lint,pnpm test,pnpm typecheck,pnpm security:check,pnpm build통과Summary by CodeRabbit
릴리스 노트
New Features
Bug Fixes
Documentation
Tests
Chores