Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates partnership persistence so that PaperContent.category is populated from the single selected goods item (improving downstream display/usage), and adjusts the STAMP notification preview text formatting.
Changes:
- Populate
PaperContent.categorywith the goods name when an option contains exactly one goods item (both update and manual-create flows). - Add mutability to
PaperContentto allow updatingcategoryduring save. - Update STAMP notification preview message to remove the newline.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/main/java/com/assu/server/domain/partnership/service/PartnershipServiceImpl.java |
Sets category from single-goods options during partnership update/manual creation. |
src/main/java/com/assu/server/domain/partnership/entity/PaperContent.java |
Adds Lombok @Setter to allow updating category before persisting. |
src/main/java/com/assu/server/domain/notification/service/NotificationCommandServiceImpl.java |
Adjusts STAMP notification preview text formatting. |
Comments suppressed due to low confidence (1)
src/main/java/com/assu/server/domain/partnership/service/PartnershipServiceImpl.java:323
- Same index-coupled
options()↔contentsmutation logic is duplicated here. Consider extracting a small helper (or handling it during DTO→entity mapping) so future changes don’t have to be applied in multiple places.
List<PaperContent> contents = request.toPaperContents(paper);
for (int i = 0; i < contents.size(); i++) {
var opt = request.options().get(i);
if (opt.goods() != null && opt.goods().size() == 1) {
contents.get(i).setCategory(opt.goods().get(0).goodsName());
}
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @Entity | ||
| @Getter | ||
| @Setter | ||
| @NoArgsConstructor | ||
| @Builder | ||
| @AllArgsConstructor |
There was a problem hiding this comment.
Adding @Setter at the entity class level makes all fields (including id, paper, and enum fields) mutable from anywhere, which can lead to unintended state changes on managed JPA entities. If the goal is only to update category, prefer a field-level @Setter on category (or a dedicated mutator like updateCategory(...)) rather than enabling setters for every field.
|
|
||
| for (int i = 0; i < newContents.size(); i++) { | ||
| var option = request.options().get(i); | ||
| if (option.goods() != null && option.goods().size() == 1) { | ||
| newContents.get(i).setCategory(option.goods().get(0).goodsName()); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
request.options().get(i) is accessed via index alignment with newContents. This works only because toPaperContents() currently preserves ordering/size; consider iterating over options() directly (or setting category inside toPaperContents() / PartnershipOptionRequestDTO.toPaperContent) to avoid fragile index-based coupling and to keep the transformation in one place.
| for (int i = 0; i < newContents.size(); i++) { | |
| var option = request.options().get(i); | |
| if (option.goods() != null && option.goods().size() == 1) { | |
| newContents.get(i).setCategory(option.goods().get(0).goodsName()); | |
| } | |
| } | |
| List<?> options = request.options(); | |
| int count = Math.min(newContents.size(), options.size()); | |
| for (int i = 0; i < count; i++) { | |
| var option = options.get(i); | |
| if (option instanceof PartnershipOptionRequestDTO partnershipOption | |
| && partnershipOption.goods() != null | |
| && partnershipOption.goods().size() == 1) { | |
| newContents.get(i).setCategory(partnershipOption.goods().get(0).goodsName()); | |
| } | |
| } |
#️⃣연관된 이슈
📝작업 내용
🔎코드 설명(스크린샷(선택))
💬고민사항 및 리뷰 요구사항 (Optional)
비고 (Optional)