fix: 릴리즈 리뷰 반영 (옵션 그룹 계약 설명 3건 정정) - #293
Conversation
세 건 다 클라이언트가 그대로 따르면 유효한 주문을 막거나 무효한 조합을 만든다. - optionRequiresDescription / optionRequiresImage — "선택지에 설명·이미지를 필수로 만든다"고 적었으나, 옵션 아이템 생성은 null이어도 통과한다. 실제로는 order-checkout.service.ts:243-250이 이 플래그가 켜진 그룹의 선택을 통째로 거절한다(OPTION_CUSTOMIZATION_REQUIRED, 커스텀 체크아웃 확장 전 잠정 가드 — 이슈 #212). 판매자가 설명만 보고 켜면 그 옵션을 못 팔게 된다 - sortOrder 기본값 — "미지정 시 맨 뒤"라 적었으나 옵션 그룹·선택지·커스텀 토큰은 input.sortOrder ?? 0이라 0이 들어간다. 상품 이미지만 ?? count로 맨 뒤에 붙는다 (seller-product-image.service.ts:73). 이미지 문구를 나머지 셋에 복사한 것이고 네 자리 중 셋이 틀렸다 - isRequired / minSelect / maxSelect — order-checkout.service.ts:230-235가 count를 [min_select, max_select]로만 보고 is_required는 0개 허용 여부만 가른다. isRequired=true라도 minSelect=0이면 0개가 유효하고, min=max=2면 정확히 2개다. "true면 최소 1개", "minSelect와 같으면 단일 선택"은 둘 다 틀렸다 전체 1,874건 통과.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
🧹 knip — dead-code 리포트전체 리포트
|
🩺 NestJS Doctor — 90/100 (Excellent)진단 313건 (error 0).
architecture / security 상위 항목
|
기계적 대조로 두 건을 더 찾았다. - SellerUpdateOptionGroupInput.isRequired만 '필수 선택 여부.'로 남아 있었다. 같은 필드의 다른 두 자리는 앞 커밋에서 '미선택 허용 여부'로 고쳤는데 표현 변형이 셋이라 하나를 놓쳤다. - SellerProductListInput.isActive는 SDL에 기본값이 없고 서비스가 ?? true로 기본을 적용한다. '기본 true'가 SDL 기본값처럼 읽혀 출처를 명시했다. 점검 방법: 설명이 말하는 '기본 X'와 SDL defaultValue를 전수 대조(불일치 1건), nullable 필드에 무조건 필수라 적은 것 탐색(2건, 그중 1건이 위 잔여분).
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d93e36a19
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| """그룹 안내 문구(상품 상세 옵션 섹션 인트로).""" | ||
| description: String | ||
| """필수 선택 여부. true면 이 그룹에서 최소 1개를 골라야 주문이 된다.""" | ||
| """미선택 허용 여부. false면 0개도 유효하고, true면 선택 개수가 minSelect~maxSelect 범위 안이어야 한다.""" |
There was a problem hiding this comment.
When clients rely on the field summary, 미선택 허용 여부 states the opposite polarity from the actual contract: false is what permits zero selections, while true requires the count to fall within the configured range. The following clause describes the implementation correctly, but the label makes true appear to allow omission; rename it to something like 필수 선택 여부 or 미선택 금지 여부. The same inversion is repeated for the create input at line 338.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
반영: 내가 만든 오류가 맞다. true가 필수인데 라벨을 "미선택 허용 여부"로 바꿔 극성이 뒤집혔다. 뒤에 붙인 설명은 맞지만 라벨과 서로 반대를 가리킨다.
원래 라벨("필수 선택 여부")은 문제가 없었다. 틀린 건 그 뒤의 "최소 1개를 골라야 한다"였는데, 라벨까지 바꾸면서 새 오류를 넣었다. 라벨은 되돌리고 설명만 정확히 적는다.
"필수 선택 여부. true면 선택 개수가 minSelect~maxSelect 범위 안이어야 하고, false면 0개 선택도 허용된다."
지적한 338행 외에 361행에도 같은 반전이 있었고, 함께 고쳤다.
덧붙여 product-detail.graphql의 구매자 쪽 isRequired·maxSelect가 옛 설명 그대로 남아 있는 것도 이번에 발견했다(seller만 고치고 구매자를 놓쳤다). 같은 계약이므로 문구를 통일했다.
앞 커밋에서 라벨을 '미선택 허용 여부'로 바꾸면서 극성을 뒤집었다. true가 필수인데 라벨은 반대를 가리킨다. 원래 라벨은 문제가 없었고 틀린 건 뒤의 '최소 1개를 골라야 한다'였는데, 라벨까지 손대며 새 오류를 넣었다. '필수 선택 여부'로 되돌리고 설명만 정확히 적는다 — true면 선택 개수가 minSelect~maxSelect 범위 안, false면 0개도 허용. 함께 발견: product-detail.graphql의 구매자 쪽 isRequired·maxSelect가 옛 설명 그대로 남아 있었다. seller만 고치고 구매자를 놓친 것이라 같은 계약으로 통일했다.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 26d4016725
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| """구매자 커스텀 입력(설명)이 필요한 그룹으로 표시한다.""" | ||
| optionRequiresDescription: Boolean | ||
| """true면 이 그룹의 선택지에 이미지가 필수가 된다.""" | ||
| """구매자 커스텀 입력(이미지)이 필요한 그룹으로 표시한다.""" | ||
| optionRequiresImage: Boolean |
There was a problem hiding this comment.
When a seller client calls sellerUpdateOptionGroup and sets either customization flag to true, this description only says that the group is being marked as requiring buyer input, even though checkout currently rejects every order selecting an option from that group. The create-input and output descriptions added by this commit include that warning, but update callers can still unknowingly disable ordering; add the same temporary checkout-rejection warning to both update fields.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
반영: 맞다. 생성 입력과 출력 타입에는 경고를 넣고 수정 입력에만 빠뜨렸다. 수정으로 플래그를 켜는 경로도 똑같이 주문을 막는데, 그 경로 호출자만 모르는 상태였다.
바로 앞 답글에서 "같은 개념이 여러 파일·자리에 흩어져 있으니 개념 단위로 전부 훑어야 한다"고 적어 놓고 같은 실수를 한 자리다. optionRequires*가 나오는 6자리(출력 2 · 생성 2 · 수정 2)를 전수로 맞췄다.
Coverage report
Test suite run success1874 tests passing in 225 suites. Report generated by 🧪jest coverage report action from 7a05ad4 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
생성 입력·출력 타입에는 '켜면 주문이 거절된다' 경고를 넣고 수정 입력에만 빠뜨렸다. 수정으로 켜는 경로도 동일하게 주문을 막는데 그 호출자만 모르는 상태였다. optionRequiresDescription/Image가 나오는 6자리(출력 2 · 생성 2 · 수정 2)를 전수로 맞췄다. 같은 개념이 여러 자리에 흩어져 있으면 한 곳만 고쳤을 때 설명끼리 어긋난다.
1 participant
릴리즈 PR #289 리뷰 추가분. 세 건 다 클라이언트가 그대로 따르면 유효한 주문을 막거나 무효한 조합을 만든다.
optionRequiresDescription/Imageorder-checkout.service.ts:243-250)sortOrder생략?? 0→ 0. 맨 뒤는 상품 이미지(?? count)만isRequired/minSelect/maxSelectcount를[min, max]로만 보고is_required는 0개 허용 여부만 가른다 (order-checkout.service.ts:230-235)각 건의 실제 피해
커스텀 플래그 — 판매자가 "선택지에 설명을 필수로 걸자"고 켜면, 그 그룹의 옵션은 주문 자체가 거절된다. 커스텀 체크아웃 확장 전까지의 잠정 가드다(이슈 #212). 설명이 정반대 인상을 준다.
sortOrder — 상품 이미지 문구를 나머지 셋에 복사했다. 네 자리 중 셋이 틀렸다.
선택 개수 —
isRequired: true, minSelect: 0이면 0개도 유효하고min=max=2면 정확히 2개다. 클라이언트가 이 설명으로 라디오/체크박스를 고르면 유효한 주문을 막는다.검증
전체 225 suites 1,874건 통과.
tsc·docs:check통과.