Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .coderabbit.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ tone_instructions: |
# 리뷰 설정
reviews:
profile: 'assertive'
high_level_summary: true
high_level_summary: false
review_status: true
changed_files_summary: true
collapse_walkthrough: true
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package com.amp.domain.audience.dto.response;

import com.amp.domain.notice.entity.Bookmark;
import com.amp.domain.notice.entity.NoticeImage;
import com.amp.global.common.dto.response.PaginationResponse;
import lombok.Builder;
import lombok.Getter;

import java.util.Comparator;
import java.util.List;

@Getter
Expand All @@ -22,7 +24,7 @@ public static class SavedAnnouncementDto {
private String categoryName;
private String content;
private String title;
private String imageUrl;
private List<String> imageUrls;

public static SavedAnnouncementDto from(Bookmark bookmark) {
return SavedAnnouncementDto.builder()
Expand All @@ -32,7 +34,10 @@ public static SavedAnnouncementDto from(Bookmark bookmark) {
.festivalTitle(bookmark.getNotice().getFestival().getTitle())
.categoryName(bookmark.getNotice().getFestivalCategory().getCategory().getCategoryName())
.title(bookmark.getNotice().getTitle())
.imageUrl(bookmark.getNotice().getImageUrl())
.imageUrls(bookmark.getNotice().getImages().stream()
.sorted(Comparator.comparingInt(NoticeImage::getImageOrder))
.map(NoticeImage::getImageUrl)
.toList())
.build();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,11 @@
import org.springframework.http.ResponseEntity;
import org.springframework.validation.annotation.Validated;
import org.springframework.web.bind.annotation.*;
import org.springframework.web.multipart.MultipartFile;
import org.springframework.security.access.prepost.PreAuthorize;

import java.util.List;

@RestController
@RequestMapping("/api/v1/festivals")
@Tag(name = "Notice")
Expand All @@ -33,9 +36,10 @@ public class NoticeCreateController {
@PostMapping(path = "/{festivalId}/notices",
consumes = MediaType.MULTIPART_FORM_DATA_VALUE)
public ResponseEntity<BaseResponse<NoticeCreateResponse>> createNotice(
@PathVariable("festivalId") @Positive Long festivalId,
@ModelAttribute @Valid NoticeCreateRequest request) {
NoticeCreateResponse response = noticeService.createNotice(festivalId, request);
@PathVariable @Positive Long festivalId,
@RequestPart("noticeCreateRequest") @Valid NoticeCreateRequest noticeCreateRequest,
@RequestPart(value = "images", required = false) List<MultipartFile> images) {
Comment on lines +40 to +41
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기에는 requestPart적용해서 처리했군요 앞서 삭제기능도 이렇게 처리해도 좋을거같습니다

NoticeCreateResponse response = noticeService.createNotice(festivalId, noticeCreateRequest, images);
return ResponseEntity
.status(SuccessStatus.NOTICE_CREATE_SUCCESS.getHttpStatus())
.body(BaseResponse.create(SuccessStatus.NOTICE_CREATE_SUCCESS.getMsg(), response));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import com.amp.domain.notice.dto.request.NoticeUpdateRequest;
import com.amp.domain.notice.service.organizer.NoticeService;
import com.amp.domain.notice.service.organizer.NoticeUpdateService;
import com.amp.global.annotation.ApiErrorCodes;
import com.amp.global.common.SuccessStatus;
import com.amp.global.response.success.BaseResponse;
Expand All @@ -15,27 +14,30 @@
import org.springframework.http.MediaType;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.*;
import org.springframework.web.multipart.MultipartFile;
import org.springframework.security.access.prepost.PreAuthorize;

import java.util.List;

@RestController
@RequestMapping("/api/v1/notices")
@Tag(name = "Notice")
@RequiredArgsConstructor
@PreAuthorize("hasRole('ORGANIZER')")
public class NoticeUpdateController {

private final NoticeUpdateService noticeUpdateService;
private final NoticeService noticeService;

@Operation(summary = "공지 수정/상단고정", description = "공지 수정 및 상단 고정 여부 선택 api")
@ApiErrorCodes(SwaggerResponseDescription.FAIL_TO_UPDATE_NOTICE)
@PutMapping(path = "/{noticeId}",
consumes = MediaType.MULTIPART_FORM_DATA_VALUE)
public ResponseEntity<BaseResponse<Void>> updateNotice(
@PathVariable("noticeId") @Positive Long noticeId,
@ModelAttribute @Valid NoticeUpdateRequest noticeUpdateRequest
@PathVariable @Positive Long noticeId,
@RequestPart("noticeUpdateRequest") @Valid NoticeUpdateRequest noticeUpdateRequest,
@RequestPart(value = "newImages", required = false) List<MultipartFile> newImages
) {
noticeUpdateService.updateNotice(noticeId, noticeUpdateRequest);
noticeService.updateNotice(noticeId, noticeUpdateRequest, newImages);

return ResponseEntity
.status(SuccessStatus.UPDATE_NOTICE_SUCCESS.getHttpStatus())
Expand All @@ -46,7 +48,7 @@ public ResponseEntity<BaseResponse<Void>> updateNotice(
@ApiErrorCodes(SwaggerResponseDescription.FAIL_TO_DELETE_NOTICE)
@DeleteMapping("/{noticeId}")
public ResponseEntity<BaseResponse<Void>> deleteNotice(
@PathVariable("noticeId") @Positive Long noticeId
@PathVariable @Positive Long noticeId
) {
noticeService.deleteNotice(noticeId);

Expand All @@ -57,4 +59,3 @@ public ResponseEntity<BaseResponse<Void>> deleteNotice(
}

}

Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,12 @@
import jakarta.validation.constraints.NotBlank;
import jakarta.validation.constraints.NotNull;
import jakarta.validation.constraints.Size;
import org.springframework.web.multipart.MultipartFile;

public record NoticeCreateRequest(
@NotBlank(message = "공지 제목은 필수값입니다.")
@Size(max = 50, message = "공지 제목은 최대 50자까지 입력할 수 있습니다.") String title,
@NotNull(message = "공지 카테고리 값은 필수값입니다.") Long categoryId,
MultipartFile image,
@NotBlank(message = "공지 내용은 필수값입니다.") String content,
@NotNull(message = "고정 여부 값은 필수 값입니다.") Boolean isPinned
) {
}

Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,16 @@
import jakarta.validation.constraints.NotBlank;
import jakarta.validation.constraints.NotNull;
import jakarta.validation.constraints.Size;
import org.springframework.web.multipart.MultipartFile;

import java.util.List;

public record NoticeUpdateRequest(
@NotNull(message = "페스티벌 아이디는 필수값입니다.") Long festivalId,
@NotBlank(message = "공지 제목은 필수값입니다.")
@Size(max = 50, message = "공지 제목은 최대 50자까지 입력할 수 있습니다.") String title,
@NotNull(message = "공지 카테고리 값은 필수값입니다.") Long categoryId,
MultipartFile newImage,
List<String> keepImageUrls,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== syncImages 구현 =="
sed -n '294,342p' src/main/java/com/amp/domain/notice/service/organizer/NoticeService.java

echo
echo "== invalid keepImageUrls 검증/테스트 검색 =="
rg -n -C2 'keepImageUrls|keepUrls|containsKey|NOTICE_.*IMAGE' \
  src/main/java/com/amp/domain/notice/service/organizer/NoticeService.java \
  src/test/java

Repository: TEAM-DECIBEL/AMP-SERVER

Length of output: 11996


🏁 Script executed:

# Verify if there are any validation checks elsewhere or in DTOs
rg -n 'keepImageUrls|keepUrls' src/main/java/com/amp/domain/notice --type java -B2 -A2 | head -60

Repository: TEAM-DECIBEL/AMP-SERVER

Length of output: 3754


keepImageUrls에 존재하지 않는 URL이 들어오면 조용히 삭제가 발생합니다

syncImages()keepImageUrls의 각 URL이 현재 이미지에 실제로 존재하는지 검증하지 않습니다. 존재하지 않는 URL은 314-317번 줄의 filter(imageMap::containsKey)에서 조용히 제거되고, 이렇게 필터링된 keepUrls에 포함되지 않는 기존 이미지들이 모두 삭제 대상으로 표시됩니다.

구체적 문제:

  • 사용자가 오타가 난 URL을 keepImageUrls에 실수로 포함시키면 → 그 URL은 무시되고 → 유지하려던 다른 이미지들이 삭제됨
  • 400 에러나 검증 오류 없이 조용히 발생 → 사용자 입력 실수가 의도하지 않은 데이터 손실로 이어짐
  • 요청 DTO에 keepImageUrls 필드에 대한 검증 로직이 없음
  • 존재하지 않는 URL 시나리오를 테스트하는 케이스 부재

권장: 유효하지 않은 keepImageUrls URL이 하나라도 있으면 요청 자체를 거절하는 검증 로직을 추가하세요.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/com/amp/domain/notice/dto/request/NoticeUpdateRequest.java` at
line 14, The syncImages() flow silently drops any keepImageUrls that don't match
existing images (filtered by imageMap::containsKey), causing unexpected
deletions; update validation so that NoticeUpdateRequest's keepImageUrls is
validated before syncImages() runs: in the request validation path, check each
URL in keepImageUrls against current imageMap (or repository lookup used by
syncImages()), and if any URL is not found, reject the request with a 4xx
validation error (include which URL(s) are invalid). Modify syncImages() to
assume validated input (or explicitly fail-fast if mismatch still occurs) and
add unit/integration tests that submit a keepImageUrls list containing a
non-existent URL and assert the request is rejected and no images are deleted.
Ensure the error message names the invalid URL(s) for clarity.

@NotBlank(message = "공지 내용은 필수값입니다.") String content,
boolean isPinned,
String previousImageUrl //기존 첨부이미지에서 이미지 수정 없이 업로드시 기존 이미지url 첨부
@NotNull(message = "고정 여부 값은 필수 값입니다.") Boolean isPinned
) {
}
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
package com.amp.domain.notice.dto.response;

import java.util.List;

public record FestivalNoticeListResponse(
Long noticeId,
String categoryName,
String title,
String content,
String imageUrl,
List<String> imageUrls,
boolean isPinned,
boolean isSaved,
String createdAt
) {
}

Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,19 @@

import com.amp.global.common.dto.CategoryData;

import java.util.List;

public record NoticeDetailResponse(
Long noticeId,
Long festivalId,
String festivalTitle,
CategoryData category,
String title,
String content,
String imageUrl,
List<String> imageUrls,
boolean isPinned,
boolean isSaved,
Author author,
String createdAt
) {
}

17 changes: 11 additions & 6 deletions src/main/java/com/amp/domain/notice/entity/Notice.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,12 @@
import lombok.Builder;
import lombok.Getter;
import lombok.NoArgsConstructor;
import org.hibernate.annotations.BatchSize;
import org.hibernate.annotations.SQLRestriction;

import java.time.LocalDateTime;
import java.util.ArrayList;
import java.util.List;

@Entity
@Table(name = "notice")
Expand Down Expand Up @@ -42,8 +45,9 @@ public class Notice extends BaseTimeEntity {
@Column(nullable = false, columnDefinition = "TEXT")
private String content;

@Column(name = "image_url", length = 500)
private String imageUrl;
@OneToMany(mappedBy = "notice", cascade = CascadeType.ALL, orphanRemoval = true)
@BatchSize(size = 100)
private List<NoticeImage> images = new ArrayList<>();

@Column(name = "is_pinned", nullable = false)
private Boolean isPinned = false;
Expand All @@ -53,30 +57,31 @@ public class Notice extends BaseTimeEntity {

@Builder
public Notice(Festival festival, FestivalCategory festivalCategory, Organizer organizer,
String title, String content, String imageUrl, Boolean isPinned) {
String title, String content, Boolean isPinned) {
this.festival = festival;
this.festivalCategory = festivalCategory;
this.organizer = organizer;
this.title = title;
this.content = content;
this.imageUrl = imageUrl;
this.isPinned = isPinned != null ? isPinned : false;
}

public void delete() {
this.deletedAt = LocalDateTime.now();
}

public void addImage(NoticeImage image) {
this.images.add(image);
}

public void update(
String title,
String content,
String imageUrl,
Boolean isPinned,
FestivalCategory festivalCategory
) {
this.title = title;
this.content = content;
this.imageUrl = imageUrl;
this.isPinned = isPinned;
this.festivalCategory = festivalCategory;
}
Expand Down
42 changes: 42 additions & 0 deletions src/main/java/com/amp/domain/notice/entity/NoticeImage.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package com.amp.domain.notice.entity;

import jakarta.persistence.*;
import lombok.AccessLevel;
import lombok.Getter;
import lombok.NoArgsConstructor;

import static jakarta.persistence.GenerationType.IDENTITY;

@Entity
@Table(name = "notice_images")
@Getter
@NoArgsConstructor(access = AccessLevel.PROTECTED)
public class NoticeImage {
@Id
@GeneratedValue(strategy = IDENTITY)
private Long id;

@ManyToOne(fetch = FetchType.LAZY)
@JoinColumn(name = "notice_id")
private Notice notice;

@Column(nullable = false, length = 500)
private String imageUrl;

@Column(nullable = false)
private int imageOrder;

private NoticeImage(Notice notice, String imageUrl, int imageOrder) {
this.notice = notice;
this.imageUrl = imageUrl;
this.imageOrder = imageOrder;
}

public static NoticeImage of(Notice notice, String imageUrl, int imageOrder) {
return new NoticeImage(notice, imageUrl, imageOrder);
}

public void updateOrder(int imageOrder) {
this.imageOrder = imageOrder;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ public enum NoticeErrorCode implements ErrorCode {
// 400 Bad Request
NOTICE_ALREADY_DELETED(HttpStatus.BAD_REQUEST, "NTC", "001", "이미 삭제된 공지입니다."),
PINNED_NOTICE_LIMIT_EXCEEDED(HttpStatus.BAD_REQUEST, "NTC", "002", "상단 고정 공지는 최대 3개까지 가능합니다."),
NOTICE_IMAGE_LIMIT_EXCEEDED(HttpStatus.BAD_REQUEST, "NTC", "003", "이미지는 최대 20장까지 업로드 가능합니다."),

// 403 Forbidden
NOTICE_DELETE_FORBIDDEN(HttpStatus.FORBIDDEN, "NTC", "001", "작성자 유저만 공지글을 삭제할 수 있습니다."),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package com.amp.domain.notice.repository;

import com.amp.domain.notice.entity.NoticeImage;
import org.springframework.data.jpa.repository.JpaRepository;

public interface NoticeImageRepository extends JpaRepository<NoticeImage, Long> {
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,9 @@
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Query;
import org.springframework.data.repository.query.Param;
import org.springframework.stereotype.Repository;

import java.util.Optional;

@Repository
public interface NoticeRepository extends JpaRepository<Notice, Long> {
Optional<Notice> findById(Long id);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import com.amp.domain.notice.dto.response.FestivalNoticeListResponse;
import com.amp.domain.notice.dto.response.NoticeListResponse;
import com.amp.domain.notice.entity.Notice;
import com.amp.domain.notice.entity.NoticeImage;
import com.amp.domain.notice.exception.NoticeException;
import com.amp.domain.notice.repository.BookmarkRepository;
import com.amp.domain.notice.repository.NoticeRepository;
Expand All @@ -22,10 +23,7 @@
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.*;
import java.util.stream.Collectors;

import static com.amp.global.common.dto.TimeFormatter.formatTimeAgo;
Expand Down Expand Up @@ -57,12 +55,17 @@ public NoticeListResponse getFestivalNoticeList(Long festivalId, Long categoryId

List<FestivalNoticeListResponse> announcements = noticePage.getContent().stream().map(notice -> {
boolean isSaved = savedNoticeIds.contains(notice.getId());
List<String> imageUrls = notice.getImages().stream()
.sorted(Comparator.comparingInt(NoticeImage::getImageOrder))
.map(NoticeImage::getImageUrl)
.toList();
Comment on lines +58 to +61
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

이미지 URL 정렬 로직을 한 곳으로 모아두는 편이 안전합니다.

같은 sorted(...).map(...).toList()NoticeService.getNoticeDetailSavedNoticesResponse.SavedAnnouncementDto.from()에도 반복됩니다. 정렬 규칙이 바뀔 때 한 군데만 누락돼도 응답 순서가 달라지니, 공통 메서드로 추출해서 재사용하는 편이 유지보수에 유리합니다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/com/amp/domain/notice/service/common/FestivalNoticeService.java`
around lines 58 - 61, Extract the repeated sorting-and-mapping logic into a
single reusable method (e.g., a static helper like
NoticeImageUtils.getSortedImageUrls(List<NoticeImage>) or an instance method
like Notice.getSortedImageUrls()) that takes List<NoticeImage> and returns a
List<String> of imageUrls using
Comparator.comparingInt(NoticeImage::getImageOrder) and
.map(NoticeImage::getImageUrl). Replace the inline chains in
FestivalNoticeService (the shown code), NoticeService.getNoticeDetail, and
SavedNoticesResponse.SavedAnnouncementDto.from() to call that new method so all
callers share the same ordering logic.


return new FestivalNoticeListResponse(
notice.getId(),
notice.getFestivalCategory().getCategory().getCategoryName(),
notice.getTitle(),
notice.getContent(),
notice.getImageUrl(),
imageUrls,
notice.getIsPinned(),
isSaved,
formatTimeAgo(notice.getCreatedAt())
Expand Down
Loading