-
Notifications
You must be signed in to change notification settings - Fork 1
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
[FIX] 네이버 검색 결과 태그 제거, 카테고리 대분류 추출 #86
Conversation
Walkthrough이 풀 리퀘스트는 Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this 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/utils/parseSearchResult.ts (1)
3-5
: 정규식 패턴의 견고성 개선이 필요합니다현재 HTML 태그 제거 정규식이 일부 복잡한 HTML 구조에서 문제를 일으킬 수 있습니다.
다음과 같이 더 견고한 정규식으로 개선하는 것을 추천드립니다:
- return result.replace(/<\/?[^>]+(>|$)/g, ''); + return result.replace(/<[^>]*>/g, '').trim();
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
src/hooks/common/useInput.tsx
(1 hunks)src/pages/MapView.tsx
(2 hunks)src/utils/CustomMarker.ts
(1 hunks)src/utils/parseSearchResult.ts
(1 hunks)
🔇 Additional comments (2)
src/pages/MapView.tsx (2)
19-19
: 새로운 유틸리티 함수 import가 적절히 추가되었습니다.
검색 결과를 정제하기 위한 parseSearchResult
유틸리티 함수가 올바르게 import 되었습니다.
58-58
: 검색 결과 처리 로직 검증이 필요합니다.
parseSearchResult
함수를 통한 데이터 정제 처리가 추가되었습니다. 다음 사항들을 확인해주세요:
- 빈 검색 결과에 대한 처리
- HTML 태그가 포함된 다양한 케이스에 대한 처리
- 카테고리 정보가 없는 경우의 처리
다음 스크립트를 실행하여 parseSearchResult
함수의 구현을 확인해보세요:
✅ Verification successful
parseSearchResult
함수의 구현이 안전하게 되어있습니다
parseSearchResult
함수의 구현을 검토한 결과, 우려했던 모든 엣지 케이스들이 적절히 처리되고 있습니다:
- HTML 태그 제거:
removeHtmlTags
함수가 정규식을 사용하여 모든 HTML 태그를 안전하게 제거합니다 - 카테고리 처리:
- 문자열 타입 체크로 잘못된 입력 방지
- 카테고리가 문자열이 아닌 경우 빈 문자열 반환
split('>')
으로 첫 번째 카테고리만 추출
- 데이터 구조 보존: spread 연산자로 원본 데이터의 다른 필드들은 유지
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# parseSearchResult 함수의 구현 확인
ast-grep --pattern 'export function parseSearchResult($_) {
$$$
}'
# 함수의 테스트 코드 확인
fd -e test.ts -e spec.ts parseSearchResult
Length of output: 115
Script:
#!/bin/bash
# 전체 코드베이스에서 parseSearchResult 함수 검색
rg -A 10 "parseSearchResult"
# 관련 파일 찾기
fd parseSearchResult
# utils 또는 helpers 디렉토리에서 검색
fd -t f . src/utils src/helpers
# 테스트 파일 검색 (다른 패턴)
fd -e test.tsx -e spec.tsx -e test.ts -e spec.ts
Length of output: 2120
Script:
#!/bin/bash
# parseSearchResult 관련 유틸리티 함수들 확인
rg -A 5 "removeHtmlTags|extractCategory" src/utils/
# Place 타입 정의 확인
rg -A 10 "type Place"
# 실제 데이터 처리 로직 자세히 확인
cat src/utils/parseSearchResult.ts
Length of output: 1848
export const parseSearchResult = (data: Place[]) => { | ||
return data.map((item) => ({ | ||
...item, | ||
title: removeHtmlTags(item.title), | ||
category: typeof item.category === 'string' ? extractCategory(item.category) : '', | ||
})); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
타입 안전성 강화가 필요합니다
Place
타입의 필수 필드에 대한 검증이 부족합니다.
다음과 같이 타입 가드와 필수 필드 검증을 추가하는 것을 추천드립니다:
export const parseSearchResult = (data: Place[]) => {
+ if (!Array.isArray(data)) return [];
return data.map((item) => ({
...item,
- title: removeHtmlTags(item.title),
+ title: item.title ? removeHtmlTags(item.title) : '',
category: typeof item.category === 'string' ? extractCategory(item.category) : '',
}));
};
📝 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.
export const parseSearchResult = (data: Place[]) => { | |
return data.map((item) => ({ | |
...item, | |
title: removeHtmlTags(item.title), | |
category: typeof item.category === 'string' ? extractCategory(item.category) : '', | |
})); | |
}; | |
export const parseSearchResult = (data: Place[]) => { | |
if (!Array.isArray(data)) return []; | |
return data.map((item) => ({ | |
...item, | |
title: item.title ? removeHtmlTags(item.title) : '', | |
category: typeof item.category === 'string' ? extractCategory(item.category) : '', | |
})); | |
}; |
const extractCategory = (category: string) => { | ||
return category.split('>')[0]; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
카테고리 추출 시 예외 처리가 필요합니다
빈 문자열이나 구분자가 없는 경우에 대한 처리가 없습니다.
다음과 같이 예외 처리를 추가하는 것을 추천드립니다:
- return category.split('>')[0];
+ return category?.split('>')?.[0]?.trim() ?? category ?? '';
📝 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.
const extractCategory = (category: string) => { | |
return category.split('>')[0]; | |
}; | |
const extractCategory = (category: string) => { | |
return category?.split('>')?.[0]?.trim() ?? category ?? ''; | |
}; |
@@ -7,7 +7,7 @@ export const useInput = (initialValue = '', validation: string[] = ['https', 'ww | |||
|
|||
const onChange: ChangeEventHandler<HTMLInputElement> = useCallback((e) => { | |||
const value = e.target.value; | |||
setState(value); | |||
setState(value.trim()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
입력값 트리밍 시점 재고가 필요합니다
onChange 이벤트에서 즉시 트리밍하는 것은 사용자 경험을 해칠 수 있습니다. 사용자가 입력 중에 공백을 사용하는 것이 불가능해집니다.
다음과 같이 onBlur 이벤트에서 트리밍하는 것을 추천드립니다:
- setState(value.trim());
+ setState(value);
그리고 onBlur 함수에서 트리밍을 수행:
const onBlur = useCallback(() => {
+ setState(state.trim());
if (state.length > 0) {
setIsValid(validation.some((criteria) => state.includes(criteria)));
}
}, [state, validation]);
Committable suggestion skipped: line range outside the PR's diff.
<span style="display: block; font-size: 12px; font-weight: 600; color: black; text-align: center; white-space: normal; margin-top: 4px;"> | ||
${shouldShowTitle ? title : ''} | ||
</span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
보안 취약점 개선이 필요합니다
문자열 템플릿을 통한 직접적인 HTML 삽입은 XSS 공격에 취약할 수 있습니다.
다음과 같이 HTML 이스케이프 함수를 추가하고 적용하는 것을 추천드립니다:
+const escapeHtml = (str: string) => {
+ return str
+ .replace(/&/g, '&')
+ .replace(/</g, '<')
+ .replace(/>/g, '>')
+ .replace(/"/g, '"')
+ .replace(/'/g, ''');
+};
<span style="display: block; font-size: 12px; font-weight: 600; color: black; text-align: center; white-space: normal; margin-top: 4px;">
- ${shouldShowTitle ? title : ''}
+ ${shouldShowTitle ? escapeHtml(title) : ''}
</span>
📝 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.
<span style="display: block; font-size: 12px; font-weight: 600; color: black; text-align: center; white-space: normal; margin-top: 4px;"> | |
${shouldShowTitle ? title : ''} | |
</span> | |
const escapeHtml = (str: string) => { | |
return str | |
.replace(/&/g, '&') | |
.replace(/</g, '<') | |
.replace(/>/g, '>') | |
.replace(/"/g, '"') | |
.replace(/'/g, '''); | |
}; | |
<span style="display: block; font-size: 12px; font-weight: 600; color: black; text-align: center; white-space: normal; margin-top: 4px;"> | |
${shouldShowTitle ? escapeHtml(title) : ''} | |
</span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
src/components/common/Input/index.tsx (1)
6-6
: 사용하지 않는 Caption import 제거 필요Caption 컴포넌트가 더 이상 사용되지 않으므로 import 문에서 제거하는 것이 좋습니다.
-import { Body4, Caption } from '../Typography'; +import { Body4 } from '../Typography';
<Body4 className="text-[14px] text-primary mt-1"> | ||
{errorMessage || '유투브 링크 혹은 지도 링크만 가능합니다.'} | ||
</Caption> | ||
</Body4> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Typography 스타일이 중복되어 있습니다
Body4 컴포넌트는 이미 text-body4
클래스를 통해 14px 폰트 크기를 정의하고 있습니다. Tailwind 설정에서 text-body4
가 14px
로 정의되어 있으므로, text-[14px]
클래스는 불필요한 중복입니다. 해당 클래스를 제거하여 Typography 시스템의 일관성을 유지하시기 바랍니다.
🔗 Analysis chain
Typography 스타일 중복 확인 필요
Body4 컴포넌트가 이미 기본 폰트 크기를 정의하고 있다면, text-[14px] 클래스는 중복될 수 있습니다. Typography 시스템의 일관성을 위해 확인이 필요합니다.
Let me check the Tailwind configuration to see if text-body4
class defines a specific font size.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if Body4 component already defines font size
rg -A 5 "Body4" src/components/common/Typography
Length of output: 2007
Script:
#!/bin/bash
# Check Tailwind configuration for text-body4 class definition
fd -e js -e ts "tailwind.config" --exec cat {}
Length of output: 785
Update: 2024년 12월 04일 22시 51분 25초 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/components/features/LinkForm/LinkInput.tsx (1)
Based on the shell script results, I can now generate the final response. The code shows that the URL validation logic in
useInput
hook is using a simple string inclusion check with default validation criteria of['https', 'www', 'youtu']
, which may not be sufficient for properly validating Naver URLs.네이버 URL 검증 로직 개선 필요
현재 URL 검증 로직이 너무 단순합니다.
useInput
훅에서 단순히 'https', 'www', 'youtu'와 같은 문자열 포함 여부만 확인하고 있어, 네이버 검색 결과 URL에 대한 적절한 검증이 이루어지지 않을 수 있습니다.
src/hooks/common/useInput.tsx
:
- 현재 검증 로직:
validation.some((criteria) => state.includes(criteria))
- 네이버 도메인(naver.com)과 검색 결과 URL 패턴에 대한 구체적인 검증이 필요합니다.
- URL 형식 자체에 대한 기본적인 유효성 검사도 추가되어야 합니다.
🔗 Analysis chain
Line range hint
28-38
: 입력값 검증 로직 확인 필요네이버 검색 결과와 관련된 링크 처리를 위해, 입력값 검증 로직이 적절한지 확인이 필요합니다.
다음 스크립트를 실행하여 입력값 검증 로직을 확인해주세요:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify input validation logic in useInput hook # Expected: Find validation logic specific to Naver URLs # Search for useInput hook implementation ast-grep --pattern 'export const useInput = $_' # Search for URL validation patterns rg -A 5 'isValid|validation|validate'Length of output: 10036
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
src/components/features/LinkForm/LinkInput.tsx
(1 hunks)
🔇 Additional comments (2)
src/components/features/LinkForm/LinkInput.tsx (2)
28-28
: 자동 포커스 기능 추가 승인
사용자 경험 향상을 위한 autoFocus
속성 추가는 적절한 개선사항입니다. 사용자가 페이지에 진입했을 때 즉시 링크를 입력할 수 있게 되어 사용성이 향상됩니다.
Line range hint 31-37
: 상태 관리 및 에러 처리 구현 승인
상태 관리와 에러 처리가 잘 구현되어 있습니다:
- 입력값 유효성 검사
- 컨텍스트와 로컬 상태의 동기화
- 초기화 기능 구현
관련 이슈
close #85
📑 작업 내용
💬 리뷰 중점 사항/기타 참고 사항
Summary by CodeRabbit
새로운 기능
스타일