Merged
Conversation
(cherry picked from commit 88c5a17)
(cherry picked from commit cca511b)
(cherry picked from commit 05eb9ec)
(cherry picked from commit 9dd3a9d)
(cherry picked from commit a01ba33)
(cherry picked from commit 4c7919a)
(cherry picked from commit b6dccc4)
(cherry picked from commit 6e37565)
(cherry picked from commit df26d15)
(cherry picked from commit 1e5a342)
(cherry picked from commit 6652520)
(cherry picked from commit e62db0e)
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note
|
| 코호트 / 파일 | 요약 |
|---|---|
설문 및 섹션 엔티티 정리 src/main/java/OneQ/OnSurvey/domain/survey/entity/Survey.java, src/main/java/OneQ/OnSurvey/domain/question/entity/Section.java |
imageUrl 필드 및 매핑 제거, updateSurvey/updateSection 메서드 시그니처에서 imageUrl 파라미터 제거 |
질문 엔티티 및 타입 변경 src/main/java/OneQ/OnSurvey/domain/question/entity/Question.java, src/main/java/OneQ/OnSurvey/domain/question/entity/question/*.java, src/main/java/OneQ/OnSurvey/domain/question/model/QuestionType.java |
모든 질문 타입(Choice, DateAnswer, Image, LongAnswer, NPS, NumberAnswer, Rating, ShortAnswer)에서 nextSection 파라미터 제거, 새로운 IMAGE 타입 추가 |
DTO 및 응답 객체 정리 src/main/java/OneQ/OnSurvey/domain/question/model/dto/..., src/main/java/OneQ/OnSurvey/domain/survey/model/dto/..., src/main/java/OneQ/OnSurvey/domain/survey/model/response/*.java |
ParticipationQuestionResponse, SurveyDetailResponse, SurveyFormResponse, ParticipationInfoResponse 등에서 imageUrl 필드 제거, nextSection 매핑 제거 |
Google 폼 변환 기능 src/main/java/OneQ/OnSurvey/domain/survey/model/formRequest/*.java, src/main/java/OneQ/OnSurvey/domain/survey/service/formRequest/FormCommandService.java, src/main/java/OneQ/OnSurvey/domain/survey/service/formRequest/FormRequestLambda.java |
FormConversionPayload, FormConversionResponse, FormRequestConversionEvent 신규 추가, 폼 변환 비동기 처리 로직 구현 |
Discord 알림 확장 src/main/java/OneQ/OnSurvey/global/infra/discord/*.java, src/main/java/OneQ/OnSurvey/global/infra/discord/notifier/dto/SurveyConversionAlert.java |
설문 변환 결과 알림을 위한 SurveyConversionAlert 및 인터페이스 메서드 추가, 다중 notifier 구현체 업데이트 |
서비스 및 비즈니스 로직 src/main/java/OneQ/OnSurvey/domain/question/service/QuestionCommandService.java, src/main/java/OneQ/OnSurvey/domain/survey/service/command/SurveyCommandService.java, src/main/java/OneQ/OnSurvey/domain/survey/service/query/SurveyQueryService.java |
nextSection 및 imageUrl 처리 제거, Google 폼 변환 후 설문 생성 로직 추가 |
관리자 API 및 설정 src/main/java/OneQ/OnSurvey/domain/admin/api/AdminController.java, src/main/java/OneQ/OnSurvey/domain/admin/api/dto/response/AdminSurveyDetailResponse.java, src/main/java/OneQ/OnSurvey/domain/admin/domain/model/survey/*.java, src/main/java/OneQ/OnSurvey/domain/survey/SurveyErrorCode.java, src/main/java/OneQ/OnSurvey/global/common/config/WebClientConfig.java |
관리자 엔드포인트 경로를 /surveys/{surveyId}/owner로 변경, 응답 DTO에서 imageUrl 제거, FORM_CONVERSION_FAILED 에러 코드 추가, Lambda용 WebClient Bean 추가 |
인프라 및 유틸리티 src/main/java/OneQ/OnSurvey/global/infra/transaction/TransactionHandler.java, src/main/java/OneQ/OnSurvey/domain/question/repository/section/SectionRepositoryImpl.java, src/main/java/OneQ/OnSurvey/domain/survey/repository/SurveyRepositoryImpl.java |
트랜잭션 처리 컴포넌트 신규 추가, SectionDto 조회 쿼리에서 imageUrl 제거 |
요청 및 폼 모델 src/main/java/OneQ/OnSurvey/domain/survey/model/request/*.java |
FreeSurveyFormRequest, SectionRequest, SurveyFormCreateRequest, SurveyFormRequest에서 imageUrl 필드 제거 |
관리 화면 UI src/main/resources/templates/bo/survey.html, src/main/resources/static/bo/scripts/form-request.js, src/main/resources/templates/bo/form-request.html |
contenteditable 기반 인라인 편집 지원, 이미지 업로드/미리보기 기능, 하이퍼링크 편집 팝오버, 폼 변환 요청 입력 UI 추가 |
시퀀스 다이어그램
sequenceDiagram
participant Client as 클라이언트
participant FormCmd as FormCommandService
participant EventPub as EventPublisher
participant Lambda as FormRequestLambda
participant ExtAPI as AWS Lambda<br/>(Google Form API)
participant SurveyService as SurveyCommandService<br/>/QuestionService
participant DB as 데이터베이스
participant Discord as DiscordAlarmService
Client->>FormCmd: POST /form-requests<br/>(form URL)
FormCmd->>DB: FormRequest 저장
DB-->>FormCmd: FormRequest ID 반환
FormCmd->>EventPub: FormRequestConversionEvent 발행
EventPub->>Lambda: convertGoogleFormIntoSurvey()
Lambda->>Lambda: FormConversionPayload 구성
Lambda->>ExtAPI: WebClient POST<br/>(timeout, retry 설정)
ExtAPI-->>Lambda: FormConversionResponse 반환
Lambda->>DB: 요청자 이메일로 멤버 조회
loop 변환 결과 반복
alt 성공
Lambda->>SurveyService: 설문 생성<br/>(제목, 섹션, 질문)
SurveyService->>DB: 설문/섹션/질문 저장
DB-->>SurveyService: surveyId 반환
Lambda->>Lambda: 성공 상세 정보 기록
else 실패
Lambda->>Lambda: 실패 상세 정보 기록
end
end
Lambda->>Discord: SurveyConversionAlert 발행
Discord->>Discord: Discord 메시지 구성<br/>(성공/실패 여부)
Discord-->>Client: Discord Webhook 전송
예상 코드 리뷰 노력
🎯 4 (복잡) | ⏱️ ~75분
관련 가능성 있는 PR
- ✨ OMF-235 폼 변환 자동화 작업 #115: Google 폼 변환 자동화 기능 구현 (FormConversionPayload, FormConversionResponse, FormRequestLambda, SurveyConversionAlert, Discord 알림, TransactionHandler, WebClient Lambda 클라이언트 등 동일한 코드 수준 변경)
- ✨ OMF-149 설문 폼 등록 기능 #99: Google 폼 요청/변환 기능 영역 수정 (SurveyErrorCode, form-request 관련 구조/이벤트/컨트롤러 겹침)
- ✨ OMF-234 이미지, 하이퍼 링크 추가 작업 #116: imageUrl 필드 추가와 제거의 정반대 변경 (동일 도메인 DTO/엔티티/매퍼 수정, 코드 수준에서 충돌 가능성)
제안 레이블
🌟feature
🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (2 warnings)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Description check | PR 설명은 Jira 링크와 간단한 설명만 포함하며, 제공된 템플릿의 필수 섹션들(Task Details, Review Requirements 등)이 누락되어 있습니다. | 제공된 템플릿에 따라 Task Details 섹션에서 구체적인 작업 항목들을 체크박스로 나열하고, 필요시 Review Requirements를 추가하세요. | |
| Docstring Coverage | Docstring coverage is 7.02% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (1 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | PR 제목은 'OMF-235 폼 변환 자동화 작업'이며, 이는 Jira 이슈를 참조하고 주요 변경 사항인 폼 변환 자동화 기능을 명확하게 설명합니다. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
fix/OMF-235
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 help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
src/main/resources/static/bo/scripts/form-request.js (1)
145-152:⚠️ Potential issue | 🟠 MajorXSS 취약점:
formLink값이 이스케이프 없이 onclick 핸들러에 삽입됨
req.formLink에 작은따옴표가 포함되면 JavaScript 문자열이 깨지며 코드 인젝션이 가능합니다. 예:https://evil.com'); alert('XSS🛡️ 제안된 수정
${req.isRegistered && req.registeredSurveyId ? `<button onclick="openSurveyEditor(${req.registeredSurveyId}, ${req.id})" class="px-3 py-1 bg-indigo-600 text-white rounded text-xs font-bold hover:bg-indigo-700"> <i class="fas fa-eye mr-1"></i>설문 조회 </button>` - : `<button onclick="createSurveyFromRequest(${req.id}, '${req.formLink}')" class="px-3 py-1 bg-green-600 text-white rounded text-xs font-bold hover:bg-green-700"> + : `<button onclick="createSurveyFromRequest(${req.id}, '${req.formLink.replace(/'/g, "\\'")}')" class="px-3 py-1 bg-green-600 text-white rounded text-xs font-bold hover:bg-green-700"> <i class="fas fa-plus mr-1"></i>설문 생성 </button>` }또는 data 속성과 이벤트 리스너를 사용하는 것이 더 안전합니다:
: `<button data-request-id="${req.id}" data-form-link="${encodeURIComponent(req.formLink)}" class="create-survey-btn ...">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/static/bo/scripts/form-request.js` around lines 145 - 152, The inline onclick currently injects req.formLink into a JS string (in the createSurveyFromRequest call) causing XSS when formLink contains quotes; replace the inline onclick approach by rendering buttons with safe data attributes (e.g., data-request-id and data-form-link where formLink is URL-encoded via encodeURIComponent) for both branches and remove the onclick; then attach a delegated event listener in your script that reads dataset.requestId and decodes dataset.formLink before calling createSurveyFromRequest(requestId, decodedFormLink) (and similarly call openSurveyEditor using dataset.requestId for the registeredSurveyId button) so no unescaped user data is placed into inline JavaScript.src/main/resources/templates/bo/form-request.html (1)
7-7: 🧹 Nitpick | 🔵 Trivial프로덕션 환경에서 Tailwind CDN 사용 검토 필요
cdn.tailwindcss.com은 개발 환경용으로 권장됩니다. 프로덕션에서는 빌드된 CSS 파일 사용을 고려해 주세요. 성능 및 가용성 측면에서 외부 CDN 의존성을 줄일 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/templates/bo/form-request.html` at line 7, The template currently includes the Tailwind CDN script ("https://cdn.tailwindcss.com") which is intended for development; replace this by linking to a built, versioned CSS asset and remove the CDN script in form-request.html: add a <link> to the compiled Tailwind output (e.g., /assets/css/tailwind.min.css), update your build pipeline to run Tailwind (e.g., npx tailwindcss build -> generate the compiled CSS into your static assets) and ensure the template references that compiled file (and optionally include cache-busting/versioning).src/main/java/OneQ/OnSurvey/domain/survey/model/response/SurveyDetailResponse.java (1)
10-28:⚠️ Potential issue | 🟠 Major
v1응답에서 필드를 제거하면 기존 클라이언트 계약이 깨집니다.
imageUrl을 응답 DTO에서 바로 제거하면, 그 필드를 기대하던 기존 클라이언트는 파싱 실패나 UI 누락을 일으킬 수 있습니다. 클라이언트 마이그레이션이 끝날 때까지는 deprecated 필드로 유지하거나, 호환되지 않는 변경이면 새 버전 응답으로 분리하는 게 안전합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/OneQ/OnSurvey/domain/survey/model/response/SurveyDetailResponse.java` around lines 10 - 28, SurveyDetailResponse currently removes the imageUrl field which breaks v1 client contracts; restore backward compatibility by either reintroducing imageUrl as a deprecated field on SurveyDetailResponse (and populate it in the static factory SurveyDetailResponse.from(Survey, SurveyInfo)) or create a new versioned DTO (e.g., SurveyDetailResponseV2) and leave the current SurveyDetailResponse unchanged so existing clients keep receiving imageUrl until migration completes.src/main/java/OneQ/OnSurvey/domain/admin/api/AdminController.java (1)
62-72:⚠️ Potential issue | 🟠 Major기존 어드민 API 경로를 그대로 깨뜨립니다.
이 변경으로
/v1/admin/{surveyId}/owner를 호출하던 기존 클라이언트는 바로 404를 받습니다. 이미 배포된 소비자가 있다면 이전 경로를 함께 유지하거나, 버전 업으로 분리한 뒤 클라이언트 전환이 끝난 다음 제거하는 편이 안전합니다.호환 경로를 함께 유지하는 최소 수정안
- `@PatchMapping`("/surveys/{surveyId}/owner") + `@PatchMapping`({"/surveys/{surveyId}/owner", "/{surveyId}/owner"})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/OneQ/OnSurvey/domain/admin/api/AdminController.java` around lines 62 - 72, The AdminController changeSurveyOwner endpoint changed its route and broke existing clients; restore backward compatibility by adding the old route mapping alongside the new one (either use `@PatchMapping`(paths = {"/surveys/{surveyId}/owner","/v1/admin/{surveyId}/owner"}) on the changeSurveyOwner method or add a small delegating method annotated with `@PatchMapping`("/v1/admin/{surveyId}/owner") that calls AdminController.changeSurveyOwner(surveyId, request)); ensure you keep the current method name changeSurveyOwner and the adminFacade.changeSurveyOwner call unchanged and do not remove the new path.src/main/java/OneQ/OnSurvey/domain/question/service/QuestionCommandService.java (1)
211-313: 🧹 Nitpick | 🔵 Trivial[선택사항] 질문 타입별 if-else 체인을 팩토리 패턴으로 리팩토링 고려
createQuestion메서드의 if-else 체인이 8개의 질문 타입을 처리하며 길어지고 있습니다. 새로운 질문 타입이 추가될 때마다 이 메서드를 수정해야 합니다.
QuestionType에 팩토리 메서드를 두거나 별도의 팩토리 클래스를 사용하면 OCP(Open-Closed Principle)를 더 잘 준수할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/OneQ/OnSurvey/domain/question/service/QuestionCommandService.java` around lines 211 - 313, The createQuestion method in QuestionCommandService contains a long if-else chain over QuestionType; extract this logic into a factory to follow OCP: implement a QuestionFactory (or add a static factory method on QuestionType) with a single method like QuestionFactory.createQuestion(Long surveyId, QuestionUpsertDto.UpsertInfo upsertInfo) that switches on type once and returns the appropriate Question subtype (DateAnswer, NPS, Rating, Choice, ShortAnswer, LongAnswer, NumberAnswer, Image); replace the body of createQuestion to delegate to that factory and remove the big if-else so adding new QuestionType only requires extending the factory, not changing createQuestion.src/main/resources/templates/bo/survey.html (1)
1694-1729: 🛠️ Refactor suggestion | 🟠 Major
sanitizeDescHtml함수는 적절하게 구현되어 있음이 함수는 표시용 HTML sanitizer로 잘 구현되어 있습니다. 서버에서 데이터를 로드하여 contenteditable이나 innerHTML에 삽입할 때 이 함수를 일관되게 사용하면 XSS 위험을 방지할 수 있습니다.
위에서 지적한 여러 위치에서 이 함수를 활용하시기 바랍니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/templates/bo/survey.html` around lines 1694 - 1729, The review notes that sanitizeDescHtml is correctly implemented as an HTML sanitizer for rendering/preview; ensure all places that insert server-loaded HTML into the DOM (especially where using innerHTML or contenteditable) call sanitizeDescHtml first; locate usages that set innerHTML or assign HTML to editable fields and replace direct insertion with sanitized content via sanitizeDescHtml(html) (reference function sanitizeDescHtml and its wrapper/resulting innerHTML) so server data is always sanitized before DOM insertion.
🤖 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/main/java/OneQ/OnSurvey/domain/survey/model/response/ParticipationQuestionResponse.java`:
- Around line 33-34: The ternary on the second element uses currSection != null
? nextSection : 0 which can return a null nextSection and cause an NPE; in
ParticipationQuestionResponse (constructor/factory/builder where currSection and
nextSection are used) change the condition to check nextSection instead—e.g.,
use nextSection != null ? nextSection : 0 or
Objects.requireNonNullElse(nextSection, 0)—so you return a safe int when
nextSection is null.
In
`@src/main/java/OneQ/OnSurvey/domain/survey/service/formRequest/FormRequestLambda.java`:
- Around line 95-97: The code currently constructs a SurveyConversionAlert with
new ArrayList<>() and then mutates it via alert.details().add(), violating
record immutability; instead, in FormRequestLambda collect
SurveyConversionAlert.SurveyDetails into a local List (e.g., detailsList) while
iterating response.results(), then call
SurveyConversionAlert.success(response.totalCount(), response.successCount(),
detailsList) and pass that immutable-ready alert to
alertNotifier.sendSurveyConversionAsync; remove any later calls that mutate
alert.details() and ensure all additions happen before creating the alert.
- Around line 43-45: The class-level `@Qualifier` on FormRequestLambda is
ineffective for constructor injection with `@RequiredArgsConstructor`; move the
qualifier onto the injected web client field/constructor parameter so Spring can
pick the "lambdaWebClient" bean (i.e., annotate the lambdaWebClient field or its
constructor parameter with `@Qualifier`("lambdaWebClient") in the
FormRequestLambda class and ensure the `@Qualifier` import is present).
- Around line 283-298: The mapQuestionType method currently misses the
"DROPDOWN" mapping causing dropdown questions to return null; update the switch
in mapQuestionType to include case "DROPDOWN" -> QuestionType.CHOICE (alongside
"MULTIPLE_CHOICE", "CHECKBOX") so dropdowns are mapped to Choice like maxChoice
logic expects.
- Around line 76-83: The reactive chain in FormRequestLambda currently throws a
CustomException inside doOnError (which is a side-effect operator), so the
thrown exception won't replace the stream error; instead, keep the
logging/alerting in doOnError (call alertNotifier.sendSurveyConversionAsync with
SurveyConversionAlert) and then append onErrorMap(e -> new
CustomException(SurveyErrorCode.FORM_CONVERSION_FAILED)) to convert the original
error into the desired CustomException (or use onErrorResume to return an
alternative Mono if appropriate) before the terminal .block(); ensure you
reference the existing doOnError, alertNotifier.sendSurveyConversionAsync,
SurveyConversionAlert, onErrorMap/onErrorResume, CustomException and
SurveyErrorCode symbols when making the change.
In `@src/main/java/OneQ/OnSurvey/domain/survey/SurveyErrorCode.java`:
- Around line 32-33: The enum entry FORM_CONVERSION_FAILED in SurveyErrorCode
currently uses the incorrect errorCode "FORM_REQUEST_001"; update its errorCode
to follow the project's TYPE_HTTP_STATUS convention (e.g.,
"FORM_CONVERSION_400") to match HttpStatus.BAD_REQUEST and the pattern used by
other entries like FORM_REQUEST_NOT_FOUND; ensure only the string value for
FORM_CONVERSION_FAILED is changed and keep the existing message and
HttpStatus.BAD_REQUEST.
In `@src/main/java/OneQ/OnSurvey/global/common/config/WebClientConfig.java`:
- Around line 36-46: The method formatting for creating the HttpClient and
returning the WebClient is inconsistent with the existing webClient method chain
style; reformat the block that constructs HttpClient (symbols:
HttpClient.create(), ChannelOption.CONNECT_TIMEOUT_MILLIS, responseTimeout,
doOnConnected, ReadTimeoutHandler, WriteTimeoutHandler) and the return builder
(WebClient.builder(), ReactorClientHttpConnector) to match the deeper
method-chaining indentation used elsewhere in the webClient method for
consistent style across the class.
In `@src/main/java/OneQ/OnSurvey/global/infra/discord/DiscordAlarmService.java`:
- Around line 121-162: sendSurveyConversionAlert builds a potentially very long
description with StringBuilder (symbols: sendSurveyConversionAlert, desc,
SurveyConversionAlert.SurveyDetails) but doesn’t enforce the embed description
length limit; mirror the sendErrorAlert approach by truncating desc.toString()
to MAX_EMBED_DESC (use the existing MAX_EMBED_DESC constant) before calling
post(url, title, ...), optionally appending an ellipsis or summary suffix to
indicate truncation so the resulting payload to post(...) never exceeds Discord
embed limits.
In `@src/main/resources/static/bo/scripts/form-request.js`:
- Around line 53-62: The try/catch should validate the API response more
robustly and surface errors to the user: replace the loose check (response !==
undefined) with a proper validation of the returned object (e.g., ensure
response is not null and contains expected success flag or HTTP status) inside
the block that calls apiCall('/form-requests', 'POST', requestData), and only
call showToast('폼 변환 요청이 등록되었습니다.'), clearFormRequestInputs(), and
loadFormRequests() when that validation passes; also enhance the catch block to
call showToast with a user-friendly error message and log the detailed error via
console.error so users receive feedback if apiCall fails.
In `@src/main/resources/templates/bo/survey.html`:
- Around line 2570-2578: The section template inserts server-side
title/description directly into contenteditable divs (section-title-input,
section-desc-input) similar to addSectionWithoutSave, creating XSS risk; update
the template and the addSectionWithoutSave code path to sanitize or escape the
values before inserting into the contenteditable elements (use the existing
escapeHtml helper or a dedicated sanitize function) or prefer assigning via
textContent/createTextNode instead of raw HTML so that title/description cannot
inject markup; ensure both the hidden inputs (section-title-value,
section-desc-value) and visible divs receive the sanitized/escaped values
consistently.
- Around line 3135-3143: The contenteditable title/description are being set
with raw HTML in addNewQuestion, creating XSS risk; before inserting into the
contenteditable divs (or before calling addNewQuestion in
renderLoadedQuestions), run the incoming q.title and q.description through
sanitizeDescHtml (or a similar sanitizer) and pass the sanitized strings to
addNewQuestion so that the .question-title-input / .question-desc-input
innerHTML is safe; keep escapeHtml for hidden inputs but ensure sanitizeDescHtml
is applied to any value rendered into contenteditable elements (or alternatively
call sanitizeDescHtml inside addNewQuestion right before assigning the
innerHTML).
- Around line 2425-2432: The edit mode assigns untrusted server data directly to
innerHTML for titleDiv and descDiv (and also sets hidden inputs), creating an
XSS risk; change these assignments to use textContent (or run the incoming data
through a strict HTML sanitizer like DOMPurify before assigning) for
document.getElementById('survey-title') and
document.getElementById('survey-description'), keep setting
document.getElementById('survey-title-value').value and
'survey-description-value'.value as-is, and if setupEditableLinkHandlers expects
HTML, ensure you pass sanitized HTML rather than raw data.
- Around line 2406-2414: The code assigns server data directly to innerHTML
(titleDiv.innerHTML and descDiv.innerHTML), risking XSS; instead sanitize or
escape before inserting: for plain text use
titleDiv.textContent/descDiv.textContent, and if rich HTML is required run the
trusted sanitizer (e.g., sanitizeHtmlForSave or a DOMPurify call) on data.title
and data.description before assigning to titleDiv.innerHTML/descDiv.innerHTML;
also ensure the hidden inputs (survey-title-value, survey-description-value)
receive the same sanitized or escaped values and keep calling
setupEditableLinkHandlers(titleDiv) and setupEditableLinkHandlers(descDiv) after
sanitization.
- Around line 1939-1951: The popover position calculation uses
popover.offsetHeight while the element is still hidden, causing offsetHeight to
be 0; change the flow in the click/positioning handler (references: linkElement,
popover, popoverRect, getBoundingClientRect, offsetHeight) so you first make the
popover measurable (e.g., remove the 'hidden' class or temporarily set
visibility/display so it can be measured) then call getBoundingClientRect() and
use popoverRect.height/width (not offsetHeight) to compute top/left, and finally
apply any adjustments and ensure the intended visibility state (re-hide if
needed) to avoid measuring a hidden element.
---
Outside diff comments:
In `@src/main/java/OneQ/OnSurvey/domain/admin/api/AdminController.java`:
- Around line 62-72: The AdminController changeSurveyOwner endpoint changed its
route and broke existing clients; restore backward compatibility by adding the
old route mapping alongside the new one (either use `@PatchMapping`(paths =
{"/surveys/{surveyId}/owner","/v1/admin/{surveyId}/owner"}) on the
changeSurveyOwner method or add a small delegating method annotated with
`@PatchMapping`("/v1/admin/{surveyId}/owner") that calls
AdminController.changeSurveyOwner(surveyId, request)); ensure you keep the
current method name changeSurveyOwner and the adminFacade.changeSurveyOwner call
unchanged and do not remove the new path.
In
`@src/main/java/OneQ/OnSurvey/domain/question/service/QuestionCommandService.java`:
- Around line 211-313: The createQuestion method in QuestionCommandService
contains a long if-else chain over QuestionType; extract this logic into a
factory to follow OCP: implement a QuestionFactory (or add a static factory
method on QuestionType) with a single method like
QuestionFactory.createQuestion(Long surveyId, QuestionUpsertDto.UpsertInfo
upsertInfo) that switches on type once and returns the appropriate Question
subtype (DateAnswer, NPS, Rating, Choice, ShortAnswer, LongAnswer, NumberAnswer,
Image); replace the body of createQuestion to delegate to that factory and
remove the big if-else so adding new QuestionType only requires extending the
factory, not changing createQuestion.
In
`@src/main/java/OneQ/OnSurvey/domain/survey/model/response/SurveyDetailResponse.java`:
- Around line 10-28: SurveyDetailResponse currently removes the imageUrl field
which breaks v1 client contracts; restore backward compatibility by either
reintroducing imageUrl as a deprecated field on SurveyDetailResponse (and
populate it in the static factory SurveyDetailResponse.from(Survey, SurveyInfo))
or create a new versioned DTO (e.g., SurveyDetailResponseV2) and leave the
current SurveyDetailResponse unchanged so existing clients keep receiving
imageUrl until migration completes.
In `@src/main/resources/static/bo/scripts/form-request.js`:
- Around line 145-152: The inline onclick currently injects req.formLink into a
JS string (in the createSurveyFromRequest call) causing XSS when formLink
contains quotes; replace the inline onclick approach by rendering buttons with
safe data attributes (e.g., data-request-id and data-form-link where formLink is
URL-encoded via encodeURIComponent) for both branches and remove the onclick;
then attach a delegated event listener in your script that reads
dataset.requestId and decodes dataset.formLink before calling
createSurveyFromRequest(requestId, decodedFormLink) (and similarly call
openSurveyEditor using dataset.requestId for the registeredSurveyId button) so
no unescaped user data is placed into inline JavaScript.
In `@src/main/resources/templates/bo/form-request.html`:
- Line 7: The template currently includes the Tailwind CDN script
("https://cdn.tailwindcss.com") which is intended for development; replace this
by linking to a built, versioned CSS asset and remove the CDN script in
form-request.html: add a <link> to the compiled Tailwind output (e.g.,
/assets/css/tailwind.min.css), update your build pipeline to run Tailwind (e.g.,
npx tailwindcss build -> generate the compiled CSS into your static assets) and
ensure the template references that compiled file (and optionally include
cache-busting/versioning).
In `@src/main/resources/templates/bo/survey.html`:
- Around line 1694-1729: The review notes that sanitizeDescHtml is correctly
implemented as an HTML sanitizer for rendering/preview; ensure all places that
insert server-loaded HTML into the DOM (especially where using innerHTML or
contenteditable) call sanitizeDescHtml first; locate usages that set innerHTML
or assign HTML to editable fields and replace direct insertion with sanitized
content via sanitizeDescHtml(html) (reference function sanitizeDescHtml and its
wrapper/resulting innerHTML) so server data is always sanitized before DOM
insertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 84892ee4-5140-4d17-a7fa-f236cc138f6f
📒 Files selected for processing (54)
src/main/java/OneQ/OnSurvey/domain/admin/api/AdminController.javasrc/main/java/OneQ/OnSurvey/domain/admin/api/dto/response/AdminSurveyDetailResponse.javasrc/main/java/OneQ/OnSurvey/domain/admin/domain/model/survey/SurveySection.javasrc/main/java/OneQ/OnSurvey/domain/admin/domain/model/survey/SurveySingleViewInfo.javasrc/main/java/OneQ/OnSurvey/domain/admin/infra/mapper/AdminSurveyMapper.javasrc/main/java/OneQ/OnSurvey/domain/question/entity/Question.javasrc/main/java/OneQ/OnSurvey/domain/question/entity/Section.javasrc/main/java/OneQ/OnSurvey/domain/question/entity/question/Choice.javasrc/main/java/OneQ/OnSurvey/domain/question/entity/question/DateAnswer.javasrc/main/java/OneQ/OnSurvey/domain/question/entity/question/Image.javasrc/main/java/OneQ/OnSurvey/domain/question/entity/question/LongAnswer.javasrc/main/java/OneQ/OnSurvey/domain/question/entity/question/NPS.javasrc/main/java/OneQ/OnSurvey/domain/question/entity/question/NumberAnswer.javasrc/main/java/OneQ/OnSurvey/domain/question/entity/question/Rating.javasrc/main/java/OneQ/OnSurvey/domain/question/entity/question/ShortAnswer.javasrc/main/java/OneQ/OnSurvey/domain/question/model/QuestionType.javasrc/main/java/OneQ/OnSurvey/domain/question/model/dto/QuestionUpsertDto.javasrc/main/java/OneQ/OnSurvey/domain/question/model/dto/SectionDto.javasrc/main/java/OneQ/OnSurvey/domain/question/model/dto/type/ChoiceDto.javasrc/main/java/OneQ/OnSurvey/domain/question/model/dto/type/DateDto.javasrc/main/java/OneQ/OnSurvey/domain/question/model/dto/type/DefaultQuestionDto.javasrc/main/java/OneQ/OnSurvey/domain/question/model/dto/type/RatingDto.javasrc/main/java/OneQ/OnSurvey/domain/question/repository/section/SectionRepositoryImpl.javasrc/main/java/OneQ/OnSurvey/domain/question/service/QuestionCommandService.javasrc/main/java/OneQ/OnSurvey/domain/survey/SurveyErrorCode.javasrc/main/java/OneQ/OnSurvey/domain/survey/entity/Survey.javasrc/main/java/OneQ/OnSurvey/domain/survey/model/dto/SurveyDetailData.javasrc/main/java/OneQ/OnSurvey/domain/survey/model/formRequest/FormConversionPayload.javasrc/main/java/OneQ/OnSurvey/domain/survey/model/formRequest/FormConversionResponse.javasrc/main/java/OneQ/OnSurvey/domain/survey/model/formRequest/event/FormRequestConversionEvent.javasrc/main/java/OneQ/OnSurvey/domain/survey/model/request/FreeSurveyFormRequest.javasrc/main/java/OneQ/OnSurvey/domain/survey/model/request/SectionRequest.javasrc/main/java/OneQ/OnSurvey/domain/survey/model/request/SurveyFormCreateRequest.javasrc/main/java/OneQ/OnSurvey/domain/survey/model/request/SurveyFormRequest.javasrc/main/java/OneQ/OnSurvey/domain/survey/model/response/ParticipationInfoResponse.javasrc/main/java/OneQ/OnSurvey/domain/survey/model/response/ParticipationQuestionResponse.javasrc/main/java/OneQ/OnSurvey/domain/survey/model/response/SurveyDetailResponse.javasrc/main/java/OneQ/OnSurvey/domain/survey/model/response/SurveyFormResponse.javasrc/main/java/OneQ/OnSurvey/domain/survey/repository/SurveyRepositoryImpl.javasrc/main/java/OneQ/OnSurvey/domain/survey/service/command/SurveyCommandService.javasrc/main/java/OneQ/OnSurvey/domain/survey/service/formRequest/FormCommandService.javasrc/main/java/OneQ/OnSurvey/domain/survey/service/formRequest/FormRequestLambda.javasrc/main/java/OneQ/OnSurvey/domain/survey/service/query/SurveyQueryService.javasrc/main/java/OneQ/OnSurvey/global/common/config/WebClientConfig.javasrc/main/java/OneQ/OnSurvey/global/infra/discord/DiscordAlarmAsyncFacade.javasrc/main/java/OneQ/OnSurvey/global/infra/discord/DiscordAlarmService.javasrc/main/java/OneQ/OnSurvey/global/infra/discord/notifier/AlertNotifier.javasrc/main/java/OneQ/OnSurvey/global/infra/discord/notifier/DiscordAlertNotifier.javasrc/main/java/OneQ/OnSurvey/global/infra/discord/notifier/NoOpAlertNotifier.javasrc/main/java/OneQ/OnSurvey/global/infra/discord/notifier/dto/SurveyConversionAlert.javasrc/main/java/OneQ/OnSurvey/global/infra/transaction/TransactionHandler.javasrc/main/resources/static/bo/scripts/form-request.jssrc/main/resources/templates/bo/form-request.htmlsrc/main/resources/templates/bo/survey.html
💤 Files with no reviewable changes (9)
- src/main/java/OneQ/OnSurvey/domain/question/model/dto/type/DefaultQuestionDto.java
- src/main/java/OneQ/OnSurvey/domain/question/model/dto/type/DateDto.java
- src/main/java/OneQ/OnSurvey/domain/question/entity/Question.java
- src/main/java/OneQ/OnSurvey/domain/survey/repository/SurveyRepositoryImpl.java
- src/main/java/OneQ/OnSurvey/domain/question/model/dto/QuestionUpsertDto.java
- src/main/java/OneQ/OnSurvey/domain/survey/model/dto/SurveyDetailData.java
- src/main/java/OneQ/OnSurvey/domain/admin/domain/model/survey/SurveySingleViewInfo.java
- src/main/java/OneQ/OnSurvey/domain/question/model/dto/type/ChoiceDto.java
- src/main/java/OneQ/OnSurvey/domain/question/model/dto/type/RatingDto.java
src/main/java/OneQ/OnSurvey/domain/survey/model/response/ParticipationQuestionResponse.java
Outdated
Show resolved
Hide resolved
src/main/java/OneQ/OnSurvey/domain/survey/service/formRequest/FormRequestLambda.java
Show resolved
Hide resolved
src/main/java/OneQ/OnSurvey/domain/survey/service/formRequest/FormRequestLambda.java
Outdated
Show resolved
Hide resolved
src/main/java/OneQ/OnSurvey/domain/survey/service/formRequest/FormRequestLambda.java
Outdated
Show resolved
Hide resolved
src/main/java/OneQ/OnSurvey/domain/survey/service/formRequest/FormRequestLambda.java
Show resolved
Hide resolved
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 by CodeRabbit
릴리스 노트
새로운 기능
변경 사항
/{surveyId}/owner→/surveys/{surveyId}/owner)