마크다운 에디터 개선 및 그룹 스터디 적용#474
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 Walkthrough작업 개요마크다운 편집기 인프라를 재구성하고, 새로운 이미지 처리 및 자산 해석 유틸리티를 도입하며, 그룹 스터디 API 엔드포인트의 타입을 강화하고, 폼 컴포넌트를 새로운 마크다운 편집기 패턴으로 재설계했습니다. 변경 사항
시퀀스 다이어그램sequenceDiagram
participant User as 사용자
participant Editor as GroupStudyMarkdownEditor
participant Form as 폼 상태
participant API as API 엔드포인트
participant Upload as 이미지 업로드
User->>Editor: 이미지 클립보드 붙여넣기
Editor->>Editor: 이미지 파일 추출
Editor->>Editor: createGroupStudyPendingDescriptionImage() 호출
Editor->>Form: descriptionPendingImages 배열에 추가
User->>Form: 폼 제출 (저장 클릭)
Form->>Form: serializeGroupStudyDescriptionForRequest() 호출
Form->>Form: @@macroFilename@@ 플레이스홀더로 변환
Form->>API: 직렬화된 설명 + pending uploads 전송
API->>API: GroupStudyWriteResponse 반환
API->>API: thumbnailUploadUrl, descriptionImageUploadUrls 제공
Form->>Upload: Promise.allSettled로 모든 이미지 업로드
Upload->>Upload: 각 pending image를 제공된 URL로 업로드
Upload->>Form: 업로드 완료
Form->>Editor: 성공 토스트 표시
Form->>Form: 모달 닫기 및 폼 리셋
코드 리뷰 예상 소요 시간🎯 4 (복잡함) | ⏱️ ~60분 관련 가능성 있는 PR
시 🐰
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/forms/group-study-form.tsx (1)
119-127:⚠️ Potential issue | 🟠 Major
maxMembersCount가 숫자일 때 Next가 영구 비활성화될 수 있습니다.Line 126 조건이
typeof value !== 'string'를 즉시 실패로 처리해서,maxMembersCount가 number로 저장되는 케이스에서 항상 막힙니다. 숫자/문자열 입력 모두 허용하는 분기 분리가 필요합니다.수정 예시
- if ( - field === 'maxMembersCount' || - field === 'startDate' || - field === 'endDate' || - field === 'title' || - field === 'summary' || - field === 'description' - ) { - return typeof value !== 'string' || value.trim() === ''; - } + if (field === 'maxMembersCount') { + if (typeof value === 'number') return value <= 0; + if (typeof value === 'string') return value.trim() === ''; + return true; + } + if ( + field === 'startDate' || + field === 'endDate' || + field === 'title' || + field === 'summary' || + field === 'description' + ) { + return typeof value !== 'string' || value.trim() === ''; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/forms/group-study-form.tsx` around lines 119 - 127, The validation block that checks fields including 'maxMembersCount' treats non-string values as empty because it does a shared check (typeof value !== 'string' || value.trim() === ''), which causes numeric maxMembersCount to always be considered invalid; update the conditional to special-case 'maxMembersCount' so that numeric values are accepted — e.g., for field === 'maxMembersCount' check for null/undefined or an empty string, and for non-strings validate via Number(value) (isNaN or out-of-range) instead of using typeof !== 'string', while keeping the existing string.trim() check for 'startDate', 'endDate', 'title', 'summary', 'description'.src/components/common/modals/group-study-form-modal.tsx (1)
415-421:⚠️ Potential issue | 🟠 Major편집 데이터가 없을 때 모달 본문이 비어 있습니다.
!isGroupStudyLoading && !groupStudyInfo분기가 없어서 조회 실패나 빈 응답이면 헤더만 있는 빈 모달이 남습니다. 이 경우는 인라인 오류와 닫기/재시도 액션을 렌더링해야 사용자가 이유를 알고 흐름을 이어갈 수 있습니다.As per coding guidelines, "For recoverable failures, preserve user flow. Prefer inline error first, use Toast as secondary. Never use browser
alert()— use Toast (useToastStore). For action-required failures, use Modal or in-app confirmation UI."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/modals/group-study-form-modal.tsx` around lines 415 - 421, The modal currently only renders GroupStudyForm when mode === 'edit' && !isGroupStudyLoading && groupStudyInfo, leaving an empty modal when the fetch fails; add a branch for mode === 'edit' && !isGroupStudyLoading && !groupStudyInfo that renders an inline error UI (error message text) plus action buttons for "Close" and "Retry" (wire Close to the existing modal close handler and Retry to re-fetch the group study info), and use useToastStore for any transient notices; locate this logic around the existing conditional that references mode, isGroupStudyLoading, groupStudyInfo, GroupStudyForm, editMethods, and handleSubmitForm and ensure the new UI preserves accessibility and matches existing modal styling.
🧹 Nitpick comments (2)
src/components/common/ui/editor/use-image-upload.ts (1)
18-19: 최대 이미지 수 에러 메시지 중복 제거가 잘 되었습니다.헬퍼 함수로 통일해서 문구 수정 시 누락 위험을 줄였고, 동일 규칙을 여러 경로에서 일관되게 적용하고 있습니다.
Also applies to: 60-62, 105-107, 127-129
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/ui/editor/use-image-upload.ts` around lines 18 - 19, Replace duplicated hardcoded Korean max-image-count messages with the helper function maxImageCountError to avoid divergence: find usages of the same literal at the other mentioned locations (around the ranges 60-62, 105-107, 127-129) and update those to call maxImageCountError(count) instead of embedding the string, ensuring all code paths in use-image-upload.ts reference the single maxImageCountError function so future text changes are centralized.src/types/mentoring/markdown.ts (1)
1-5: 공용 이미지 제약 상수는 중립 레이어로 빼는 편이 안전합니다.
src/types/mentoring/markdown.ts가components/common/ui/editor/image-utils를 직접 바라보면 타입/모델 레이어가 UI 경로에 묶입니다. 지금 PR에서도 그룹 스터디 쪽은 같은 값을 다시 선언하고 있어서, 공통 정책이면서도 한쪽은 복제되고 한쪽은 UI 레이어를 참조하는 상태가 됐습니다.constants나utils같은 중립 모듈로 올려 두 파일이 같은 소스를 보게 하는 편이 유지보수에 안전합니다.Also applies to: 16-20
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/types/mentoring/markdown.ts` around lines 1 - 5, This file imports UI-layer constants MARKDOWN_IMAGE_DEFAULT_ALLOWED_EXTENSIONS, MARKDOWN_IMAGE_DEFAULT_MAX_COUNT, and MARKDOWN_IMAGE_DEFAULT_MAX_FILE_SIZE from a components path which couples the type/model layer to the UI; extract these constants into a neutral shared module (e.g., a new src/constants or src/utils module) and update this file to import the three symbols from that neutral module (also update the other module that currently redeclares them so both consume the same source); ensure exported names remain identical so functions/types in src/types/mentoring/markdown.ts continue to reference MARKDOWN_IMAGE_DEFAULT_ALLOWED_EXTENSIONS, MARKDOWN_IMAGE_DEFAULT_MAX_COUNT, and MARKDOWN_IMAGE_DEFAULT_MAX_FILE_SIZE without further changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@PROJECT_INDEX.md`:
- Around line 15-69: The fenced code block that renders the project directory
tree (the block starting with "src/") is missing a language specifier causing
markdownlint MD040; fix it by changing the opening fence from ``` to ```text (or
```plaintext) so the block is explicitly marked as plain text — update the
fenced block in PROJECT_INDEX.md that contains the directory tree (the "src/"
listing).
In `@src/components/common/modals/group-study-form-modal.tsx`:
- Around line 237-255: The code currently treats missingUploadUrlCount (computed
from uploadPairs) as a benign upload failure and increments failedCount;
instead, when missingUploadUrlCount > 0 you must escalate this to a hard
validation error so the form/modal remains open and the user can retry—inside
the submit flow where uploadPairs and settledResults are processed (references:
uploadPairs, missingUploadUrlCount, uploadThumbnail, failedCount), detect
missingUploadUrlCount > 0 and throw or return an error that halts closing the
modal (or set a form-level error state/message) with a clear user-facing
explanation about mismatched presigned URLs/placeholders; apply the same change
at the other similar blocks mentioned (around the analogous code at lines
~293-304 and ~340-351) so missing presigned URLs never get treated as partial
success.
- Around line 282-289: The code assumes response.content exists when accessing
createdResponse.content.thumbnailUploadUrl /
updatedResponse.content.thumbnailUploadUrl which can crash if content is absent;
instead first validate that createdResponse.content (and updatedResponse.content
in the other block) is present, then safely read thumbnailUploadUrl (e.g., const
thumbUrl = createdResponse.content?.thumbnailUploadUrl) and only call
uploadThumbnail(thumbUrl, values.thumbnailFile) if thumbUrl is defined,
mirroring how descriptionImageUploadUrls are optional-chained; update both the
createdResponse and updatedResponse branches and throw or log a clear error if
content or thumbnailUploadUrl is missing before attempting upload.
In `@src/components/common/ui/editor/markdown-sanitizer.ts`:
- Line 30: The sanitizer currently blocks blob: and data: via ALLOWED_URI_REGEXP
but applyPostSanitizeAttributes restores such URLs using
resolveMarkdownAssetUrl, creating a bypass; update the policy to be consistent
by either (A) adding blob: and/or data: schemes to ALLOWED_URI_REGEXP so
DOMPurify allows those asset preview URLs, or (B) keep ALLOWED_URI_REGEXP
unchanged and add the same allowlist validation inside
applyPostSanitizeAttributes (or before using resolveMarkdownAssetUrl) to reject
any resolved URLs that don't match ALLOWED_URI_REGEXP; reference
ALLOWED_URI_REGEXP, applyPostSanitizeAttributes and resolveMarkdownAssetUrl when
making the change so the sanitizer and post-sanitize restoration are aligned.
In `@src/types/schemas/group-study-form.schema.ts`:
- Line 105: The schema removed the max-length check for description—restore
length validation using the existing GROUP_STUDY_DESCRIPTION_MAX_LENGTH constant
by adding a Zod refine on the description field (instead of a raw .max) that
computes visible/plain text length (strip markdown/HTML) and rejects if >
GROUP_STUDY_DESCRIPTION_MAX_LENGTH; update the description schema expression
(the z.string().trim().min(...) chain) to include .refine(...) with a clear
error message so the form (React Hook Form + Zod) enforces visible-text length
limits while allowing markdown/HTML input.
In `@src/utils/markdown-content-shared.ts`:
- Around line 14-18: The numeric HTML entity replacers can throw RangeError for
out-of-range values; update the two replace callbacks that call
String.fromCodePoint (the decimal handler and the hex handler) to validate
parsed code points are within 0..0x10FFFF and non-negative before calling
String.fromCodePoint, and if invalid return the original matched entity string
(e.g. "&#<num>;" or "&#x<hex>;") as a safe fallback so callers like
getRichContentVisibleText() and getRichContentVisibleTextLength() won't crash.
---
Outside diff comments:
In `@src/components/common/modals/group-study-form-modal.tsx`:
- Around line 415-421: The modal currently only renders GroupStudyForm when mode
=== 'edit' && !isGroupStudyLoading && groupStudyInfo, leaving an empty modal
when the fetch fails; add a branch for mode === 'edit' && !isGroupStudyLoading
&& !groupStudyInfo that renders an inline error UI (error message text) plus
action buttons for "Close" and "Retry" (wire Close to the existing modal close
handler and Retry to re-fetch the group study info), and use useToastStore for
any transient notices; locate this logic around the existing conditional that
references mode, isGroupStudyLoading, groupStudyInfo, GroupStudyForm,
editMethods, and handleSubmitForm and ensure the new UI preserves accessibility
and matches existing modal styling.
In `@src/components/forms/group-study-form.tsx`:
- Around line 119-127: The validation block that checks fields including
'maxMembersCount' treats non-string values as empty because it does a shared
check (typeof value !== 'string' || value.trim() === ''), which causes numeric
maxMembersCount to always be considered invalid; update the conditional to
special-case 'maxMembersCount' so that numeric values are accepted — e.g., for
field === 'maxMembersCount' check for null/undefined or an empty string, and for
non-strings validate via Number(value) (isNaN or out-of-range) instead of using
typeof !== 'string', while keeping the existing string.trim() check for
'startDate', 'endDate', 'title', 'summary', 'description'.
---
Nitpick comments:
In `@src/components/common/ui/editor/use-image-upload.ts`:
- Around line 18-19: Replace duplicated hardcoded Korean max-image-count
messages with the helper function maxImageCountError to avoid divergence: find
usages of the same literal at the other mentioned locations (around the ranges
60-62, 105-107, 127-129) and update those to call maxImageCountError(count)
instead of embedding the string, ensuring all code paths in use-image-upload.ts
reference the single maxImageCountError function so future text changes are
centralized.
In `@src/types/mentoring/markdown.ts`:
- Around line 1-5: This file imports UI-layer constants
MARKDOWN_IMAGE_DEFAULT_ALLOWED_EXTENSIONS, MARKDOWN_IMAGE_DEFAULT_MAX_COUNT, and
MARKDOWN_IMAGE_DEFAULT_MAX_FILE_SIZE from a components path which couples the
type/model layer to the UI; extract these constants into a neutral shared module
(e.g., a new src/constants or src/utils module) and update this file to import
the three symbols from that neutral module (also update the other module that
currently redeclares them so both consume the same source); ensure exported
names remain identical so functions/types in src/types/mentoring/markdown.ts
continue to reference MARKDOWN_IMAGE_DEFAULT_ALLOWED_EXTENSIONS,
MARKDOWN_IMAGE_DEFAULT_MAX_COUNT, and MARKDOWN_IMAGE_DEFAULT_MAX_FILE_SIZE
without further changes.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 13e86b26-f126-4d5b-8b9b-3ee75e853103
📒 Files selected for processing (30)
PROJECT_INDEX.mdsrc/api/endpoints/group-study/create-group-study.tssrc/api/endpoints/group-study/update-group-study.tssrc/app/global.csssrc/components/common/modals/group-study-form-modal.tsxsrc/components/common/ui/editor/clipboard-utils.tssrc/components/common/ui/editor/hljs-setup.tssrc/components/common/ui/editor/image-utils.tssrc/components/common/ui/editor/index.tsxsrc/components/common/ui/editor/markdown-content-assets.tssrc/components/common/ui/editor/markdown-content.tsxsrc/components/common/ui/editor/markdown-editor.tsxsrc/components/common/ui/editor/markdown-sanitizer.tssrc/components/common/ui/editor/markdown-utils.tssrc/components/common/ui/editor/use-image-upload.tssrc/components/common/ui/form/form-field.tsxsrc/components/forms/group-study-form.tsxsrc/components/forms/group-study-steps/group-study-step-introduction.tsxsrc/features/group-study/model/group-study-description.tssrc/features/group-study/model/group-study-markdown.tssrc/features/group-study/ui/group-study-markdown-editor.tsxsrc/features/mentoring/ui/registration/markdown/mentor-markdown-rendering.tssrc/types/api/group-study.types.tssrc/types/mentoring/markdown.tssrc/types/schemas/group-study-form.schema.tssrc/utils/markdown-content-images.tssrc/utils/markdown-content-normalize.tssrc/utils/markdown-content-shared.tssrc/utils/markdown-content-text.tssrc/utils/markdown-content.ts
💤 Files with no reviewable changes (2)
- src/components/common/ui/editor/index.tsx
- src/components/common/ui/editor/markdown-utils.ts
Comment on lines
+237
to
+255
| const missingUploadUrlCount = uploadPairs.filter( | ||
| ({ uploadUrl }) => !uploadUrl, | ||
| ).length; | ||
|
|
||
| const settledResults = await Promise.allSettled( | ||
| uploadPairs | ||
| .filter( | ||
| (pair): pair is typeof pair & { uploadUrl: string } => | ||
| typeof pair.uploadUrl === 'string' && pair.uploadUrl.length > 0, | ||
| ) | ||
| .map(({ uploadUrl, pendingUpload }) => | ||
| uploadThumbnail(uploadUrl, pendingUpload.file), | ||
| ), | ||
| ); | ||
|
|
||
| return { | ||
| failedCount: | ||
| missingUploadUrlCount + | ||
| settledResults.filter((result) => result.status === 'rejected').length, |
There was a problem hiding this comment.
설명 이미지 presigned URL 누락을 부분 성공으로 넘기면 저장된 소개문이 깨집니다.
missingUploadUrlCount는 이미 본문에 넣어 둔 @@...@@ placeholder 수와 서버가 돌려준 업로드 URL 수가 어긋났다는 뜻입니다. 이 상태는 단순 업로드 실패가 아니라 해당 이미지를 더 이상 올릴 수 없는 계약 불일치인데, 지금처럼 failedCount만 올리고 모달을 닫으면 복구 불가능한 placeholder가 저장된 상태로 남습니다. URL 누락은 즉시 예외로 승격해서 폼을 유지하고 다시 제출하게 하는 편이 안전합니다.
Also applies to: 293-304, 340-351
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/common/modals/group-study-form-modal.tsx` around lines 237 -
255, The code currently treats missingUploadUrlCount (computed from uploadPairs)
as a benign upload failure and increments failedCount; instead, when
missingUploadUrlCount > 0 you must escalate this to a hard validation error so
the form/modal remains open and the user can retry—inside the submit flow where
uploadPairs and settledResults are processed (references: uploadPairs,
missingUploadUrlCount, uploadThumbnail, failedCount), detect
missingUploadUrlCount > 0 and throw or return an error that halts closing the
modal (or set a form-level error state/message) with a clear user-facing
explanation about mismatched presigned URLs/placeholders; apply the same change
at the other similar blocks mentioned (around the analogous code at lines
~293-304 and ~340-351) so missing presigned URLs never get treated as partial
success.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.eslintrc.cjs (1)
84-85:padding-line-between-statements규칙이 아무 동작도 하지 않습니다.
padding-line-between-statements규칙은 최소 하나의 statement descriptor({ blankLine, prev, next })가 필요합니다. 현재['error']만 설정되어 있어 실질적으로 아무것도 검사하지 않습니다.위의 주석("특정 코드 블록 사이에 줄바꿈 강제")과 실제 동작이 일치하지 않습니다.
♻️ 규칙을 완전히 비활성화하거나, 원하는 동작을 명시하세요
Option 1: 규칙 비활성화 (의도한 경우)
- // 특정 코드 블록 사이에 줄바꿈 강제 - 'padding-line-between-statements': ['error'], + // 줄바꿈 규칙 비활성화 + 'padding-line-between-statements': 'off',Option 2: 이전 동작 유지 (return 전 빈 줄 강제)
- 'padding-line-between-statements': ['error'], + 'padding-line-between-statements': [ + 'error', + { blankLine: 'always', prev: '*', next: 'return' }, + ],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.eslintrc.cjs around lines 84 - 85, The ESLint rule configuration for 'padding-line-between-statements' is currently a no-op because it's set to ['error'] with no descriptors; update the rule either by disabling it explicitly (set to 'off') if you intend no checks, or by supplying the required statement descriptor objects (e.g., add { blankLine: 'always', prev: '*', next: 'return' } to enforce a blank line before return) so the behavior matches the comment "특정 코드 블록 사이에 줄바꿈 강제"; locate the 'padding-line-between-statements' entry in .eslintrc.cjs and apply one of these two fixes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.eslintrc.cjs:
- Around line 84-85: The ESLint rule configuration for
'padding-line-between-statements' is currently a no-op because it's set to
['error'] with no descriptors; update the rule either by disabling it explicitly
(set to 'off') if you intend no checks, or by supplying the required statement
descriptor objects (e.g., add { blankLine: 'always', prev: '*', next: 'return' }
to enforce a blank line before return) so the behavior matches the comment "특정
코드 블록 사이에 줄바꿈 강제"; locate the 'padding-line-between-statements' entry in
.eslintrc.cjs and apply one of these two fixes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5dc7ca90-d44b-4068-a115-9fbb14b610a5
📒 Files selected for processing (16)
.eslintrc.cjssrc/components/common/modals/group-study-form-modal.tsxsrc/components/common/ui/editor/clipboard-utils.tssrc/components/common/ui/editor/extensions.tssrc/components/common/ui/editor/image-utils.tssrc/components/common/ui/editor/markdown-content-assets.tssrc/components/common/ui/editor/markdown-content.tsxsrc/components/common/ui/editor/markdown-editor.tsxsrc/components/common/ui/editor/markdown-sanitizer.tssrc/components/common/ui/editor/toolbar.tsxsrc/components/common/ui/editor/use-image-upload.tssrc/components/forms/group-study-steps/group-study-step-introduction.tsxsrc/components/pages/group-study-detail-page.tsxsrc/components/pages/premium-study-detail-page.tsxsrc/features/group-study/model/group-study-description.tssrc/features/group-study/model/group-study-markdown.ts
✅ Files skipped from review due to trivial changes (2)
- src/components/common/ui/editor/toolbar.tsx
- src/components/common/ui/editor/extensions.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- src/components/common/ui/editor/markdown-sanitizer.ts
- src/components/common/ui/editor/markdown-content-assets.ts
- src/components/common/ui/editor/image-utils.ts
- src/components/common/ui/editor/clipboard-utils.ts
- src/components/common/ui/editor/use-image-upload.ts
- src/components/common/ui/editor/markdown-editor.tsx
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
src/components/common/ui/editor/markdown-sanitizer.ts (1)
30-31:⚠️ Potential issue | 🟠 Major
data:URL이 sanitize 이후 다시 주입될 수 있습니다.Line 30에서는
data:를 차단하지만, Line 106-112에서resolveMarkdownAssetUrl()결과를 그대로 복원해 동일 정책이 깨집니다.img src복원 전에 같은 allowlist 검사로 필터링하세요.수정 예시
+const ALLOWED_URI_PATTERN = /^(?:https?:\/\/|mailto:|tel:|\/images\/|#|blob:)/i; + export const SANITIZE_OPTIONS: DOMPurifyConfig = { @@ - ALLOWED_URI_REGEXP: /^(?:https?:\/\/|mailto:|tel:|\/images\/|#|blob:)/i, + ALLOWED_URI_REGEXP: ALLOWED_URI_PATTERN, }; @@ - if (resolvedSrc) { + if (resolvedSrc && ALLOWED_URI_PATTERN.test(resolvedSrc)) { imageElement.setAttribute('src', resolvedSrc); + } else { + imageElement.removeAttribute('src'); }Also applies to: 106-112
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/ui/editor/markdown-sanitizer.ts` around lines 30 - 31, The code currently blocks data: in ALLOWED_URI_REGEXP but later unconditionally restores asset URLs returned by resolveMarkdownAssetUrl(), which can reintroduce data: URIs; before restoring or assigning the resolved URL (the logic around resolveMarkdownAssetUrl() on lines ~106-112), validate the resolved value against ALLOWED_URI_REGEXP (e.g., ALLOWED_URI_REGEXP.test(resolvedUrl)) and skip or neutralize any URL that fails the test so only allowlisted schemes are reinserted; update the restore path that assigns img src (the block calling resolveMarkdownAssetUrl()) to perform this check and avoid re-injecting disallowed data: URIs.src/components/common/modals/group-study-form-modal.tsx (1)
287-296:⚠️ Potential issue | 🟠 Major설명 이미지 presigned URL 누락은 부분 성공이 아니라 즉시 실패 처리해야 합니다.
uploadUrls개수가pendingUploads보다 부족한 경우는 단순 업로드 실패가 아니라 요청/응답 계약 불일치입니다. 지금처럼failedCount로만 처리하면 unresolved placeholder가 저장될 수 있습니다.🔧 제안 수정
const uploadDescriptionImages = async ({ uploadUrls, pendingUploads, }: { @@ }) => { if (!pendingUploads || pendingUploads.length === 0) { return { failedCount: 0 }; } + + const missingUploadUrlCount = pendingUploads.length - (uploadUrls?.length ?? 0); + if (missingUploadUrlCount > 0) { + throw new Error('소개 이미지 업로드 URL이 일부 누락되었습니다. 다시 시도해주세요.'); + } const results = await Promise.allSettled( pendingUploads.map((item, i) => { const url = uploadUrls?.[i];Also applies to: 334-343
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/modals/group-study-form-modal.tsx` around lines 287 - 296, The code treats missing presigned upload URLs as individual upload failures (counting towards failedCount) instead of aborting the whole operation; before calling Promise.allSettled in the thumbnail upload block (the block that maps pendingUploads and calls uploadThumbnail) validate that uploadUrls has a URL for every pendingUploads entry (e.g., check lengths or that every uploadUrls[i] exists) and if any are missing throw/return a hard failure (reject the parent flow) with a clear error like "Presigned URL missing for thumbnail upload" so the caller cannot proceed and unresolved placeholders are not saved; apply the same validation fix to the similar block referenced at lines 334-343.
🧹 Nitpick comments (1)
src/utils/markdown-content-shared.ts (1)
14-20: 숫자 엔티티 디코딩 중복 로직은 헬퍼 추출을 권장합니다.Line 14-20의 decimal/hex 분기 로직이 동일해 유지보수 관점에서 한 곳으로 모으는 편이 안전합니다.
리팩터링 예시
+const decodeNumericEntity = ( + match: string, + raw: string, + radix: 10 | 16, +) => { + const cp = Number.parseInt(raw, radix); + return cp >= 0 && cp <= 0x10ffff ? String.fromCodePoint(cp) : match; +}; + export const decodeHtmlEntities = (value: string) => { return value @@ - .replace(/&#(\d+);/gi, (match, decimal: string) => { - const cp = Number.parseInt(decimal, 10); - return cp >= 0 && cp <= 0x10ffff ? String.fromCodePoint(cp) : match; - }) - .replace(/&#x([\da-f]+);/gi, (match, hex: string) => { - const cp = Number.parseInt(hex, 16); - return cp >= 0 && cp <= 0x10ffff ? String.fromCodePoint(cp) : match; - }); + .replace(/&#(\d+);/gi, (match, decimal: string) => + decodeNumericEntity(match, decimal, 10), + ) + .replace(/&#x([\da-f]+);/gi, (match, hex: string) => + decodeNumericEntity(match, hex, 16), + ); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/markdown-content-shared.ts` around lines 14 - 20, Extract the duplicate decimal/hex numeric-entity decoding logic into a single helper (e.g., decodeNumericEntity or parseNumericEntity) and call it from both replace calls that use the regexes /&#(\d+);/gi and /&#x([\da-f]+);/gi; the helper should accept the captured string and radix (10 or 16), parse with Number.parseInt, validate the code point range against 0x10FFFF, and return either String.fromCodePoint(cp) or the original match to preserve current behavior, then replace the inline parsing logic in both places with calls to that helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/common/modals/group-study-form-modal.tsx`:
- Around line 143-145: Do not default groupStudyId to 0 when calling
useUpdateGroupStudyMutation; instead remove the fallback and add a guard in the
submit/edit event handler (the function that calls updateGroupStudy) to check if
groupStudyId is undefined, show a Toast error and return early, then call
mutateAsync: updateGroupStudy only when the ID is present; apply the same guard
for the other occurrence referenced (lines ~372-378) so no update is attempted
with an invalid ID.
- Around line 344-348: The finally block in executeStudySubmit always calls
onFinally(), which closes/resets the modal even on failures and loses user
input; modify executeStudySubmit so that onFinally() (or the close/reset
behavior) is invoked only on successful create/edit (e.g., after await
createStudy or updateStudy resolves) and not in the catch path—keep the catch to
show an inline error and/or toast and return so the modal and form remain open
for retry; apply the same change to the other similar try/catch/finally usages
for the create/edit flows in this component (the other execute/save handlers
referenced in the diff).
In `@src/components/common/ui/editor/markdown-sanitizer.ts`:
- Around line 37-45: Update the JSDoc to match the actual implementations: for
parseSanitizedImageWidth, change the description and examples to reflect that
out-of-range values are clamped to min/max (e.g., '50' -> 80) rather than
returning undefined and show non-numeric input returning undefined; and for the
link/image example (lines ~65-70) correct the example to reflect the actual
behavior of the sanitizer function that controls anchor img attributes
(referencing the function that applies target/rel to anchors/images, e.g.,
sanitizeAnchors or the anchor-processing block) so the example only shows
target/rel being added when the implementation does so — ensure JSDoc text and
examples precisely mirror the code paths in parseSanitizedImageWidth and the
anchor/image sanitizer function.
---
Duplicate comments:
In `@src/components/common/modals/group-study-form-modal.tsx`:
- Around line 287-296: The code treats missing presigned upload URLs as
individual upload failures (counting towards failedCount) instead of aborting
the whole operation; before calling Promise.allSettled in the thumbnail upload
block (the block that maps pendingUploads and calls uploadThumbnail) validate
that uploadUrls has a URL for every pendingUploads entry (e.g., check lengths or
that every uploadUrls[i] exists) and if any are missing throw/return a hard
failure (reject the parent flow) with a clear error like "Presigned URL missing
for thumbnail upload" so the caller cannot proceed and unresolved placeholders
are not saved; apply the same validation fix to the similar block referenced at
lines 334-343.
In `@src/components/common/ui/editor/markdown-sanitizer.ts`:
- Around line 30-31: The code currently blocks data: in ALLOWED_URI_REGEXP but
later unconditionally restores asset URLs returned by resolveMarkdownAssetUrl(),
which can reintroduce data: URIs; before restoring or assigning the resolved URL
(the logic around resolveMarkdownAssetUrl() on lines ~106-112), validate the
resolved value against ALLOWED_URI_REGEXP (e.g.,
ALLOWED_URI_REGEXP.test(resolvedUrl)) and skip or neutralize any URL that fails
the test so only allowlisted schemes are reinserted; update the restore path
that assigns img src (the block calling resolveMarkdownAssetUrl()) to perform
this check and avoid re-injecting disallowed data: URIs.
---
Nitpick comments:
In `@src/utils/markdown-content-shared.ts`:
- Around line 14-20: Extract the duplicate decimal/hex numeric-entity decoding
logic into a single helper (e.g., decodeNumericEntity or parseNumericEntity) and
call it from both replace calls that use the regexes /&#(\d+);/gi and
/&#x([\da-f]+);/gi; the helper should accept the captured string and radix (10
or 16), parse with Number.parseInt, validate the code point range against
0x10FFFF, and return either String.fromCodePoint(cp) or the original match to
preserve current behavior, then replace the inline parsing logic in both places
with calls to that helper.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d8bd099a-c70e-44d7-bc44-f40d7dc78a20
📒 Files selected for processing (3)
src/components/common/modals/group-study-form-modal.tsxsrc/components/common/ui/editor/markdown-sanitizer.tssrc/utils/markdown-content-shared.ts
This was referenced Apr 6, 2026
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
w-[112px]) 토큰화, dead code 제거, 에러 메시지 중복 제거 등 코드 품질 개선Changes
Features
editor/clipboard-utils.tseditor/hljs-setup.tseditor/markdown-sanitizer.tseditor/markdown-content-assets.tsfeatures/group-study/ui/group-study-markdown-editor.tsxfeatures/group-study/model/group-study-markdown.tsutils/markdown-content-{images,normalize,shared,text}.tsBug Fixes
form-field.tsxw-[112px]→w-1400커스텀 토큰으로 교체group-study-form-modal.tsxeditDefaultValuesdead computation 제거, 중복 조건 추출use-image-upload.tsmarkdown-editor.tsxuseMemo([editor, editor?.state])mentor-markdown-rendering.tsnew DOMParser()매 호출 → 모듈 레벨 캐싱Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선사항