-
Notifications
You must be signed in to change notification settings - Fork 0
[TSK-69] 어드민 공지사항 관리 CRUD 기능 구현 #309
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 11 commits
722444e
9b482ab
4b79bca
c440c35
27ae010
979eea8
a341e4c
ed71ea0
fd83c52
cee4753
60bb57b
6c5affa
b445714
0b78d1a
96d52ef
eb1dd87
9d9f639
66f736c
6cf169b
8e4b713
55ec91b
a316c0c
7f4f279
c3684b0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| package kr.allcll.backend.admin.notice; | ||
|
|
||
| import jakarta.servlet.http.HttpServletRequest; | ||
| import jakarta.validation.Valid; | ||
| import kr.allcll.backend.admin.AdminRequestValidator; | ||
| import kr.allcll.backend.admin.notice.dto.CreateNoticeRequest; | ||
| import kr.allcll.backend.admin.notice.dto.CreateNoticeResponse; | ||
| import kr.allcll.backend.admin.notice.dto.NoticesResponse; | ||
| import kr.allcll.backend.admin.notice.dto.UpdateNoticeRequest; | ||
| import kr.allcll.backend.admin.notice.dto.UpdateNoticeResponse; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.http.HttpStatus; | ||
| import org.springframework.http.ResponseEntity; | ||
| import org.springframework.web.bind.annotation.DeleteMapping; | ||
| import org.springframework.web.bind.annotation.GetMapping; | ||
| import org.springframework.web.bind.annotation.PatchMapping; | ||
| import org.springframework.web.bind.annotation.PathVariable; | ||
| import org.springframework.web.bind.annotation.PostMapping; | ||
| import org.springframework.web.bind.annotation.RequestBody; | ||
| import org.springframework.web.bind.annotation.RestController; | ||
|
|
||
| @RestController | ||
| @RequiredArgsConstructor | ||
| public class AdminNoticeApi { | ||
|
|
||
| private final AdminNoticeService adminNoticeService; | ||
| private final AdminRequestValidator validator; | ||
|
|
||
| @GetMapping("/api/admin/notices") | ||
| public ResponseEntity<NoticesResponse> getAllNotice(HttpServletRequest request) { | ||
| if (validator.isRateLimited(request) || validator.isUnauthorized(request)) { | ||
| return ResponseEntity.status(401).build(); | ||
| } | ||
| NoticesResponse response = adminNoticeService.getAllNotice(); | ||
| return ResponseEntity.ok(response); | ||
| } | ||
|
|
||
| @PostMapping("/api/admin/notices") | ||
| public ResponseEntity<CreateNoticeResponse> createNotice( | ||
| HttpServletRequest request, | ||
| @Valid @RequestBody CreateNoticeRequest createNoticeRequest | ||
| ) { | ||
| if (validator.isRateLimited(request) || validator.isUnauthorized(request)) { | ||
| return ResponseEntity.status(401).build(); | ||
| } | ||
| CreateNoticeResponse response = adminNoticeService.createNewNotice(createNoticeRequest); | ||
| return ResponseEntity.status(HttpStatus.CREATED).body(response); | ||
| } | ||
|
|
||
| @PatchMapping("/api/admin/notices/{id}") | ||
| public ResponseEntity<UpdateNoticeResponse> modifyNotice( | ||
| HttpServletRequest request, | ||
| @PathVariable Long id, | ||
| @Valid @RequestBody UpdateNoticeRequest updateNoticeRequest | ||
| ) { | ||
| if (validator.isRateLimited(request) || validator.isUnauthorized(request)) { | ||
| return ResponseEntity.status(401).build(); | ||
| } | ||
| UpdateNoticeResponse response = adminNoticeService.updateNotice(id, updateNoticeRequest); | ||
| return ResponseEntity.ok(response); | ||
| } | ||
|
|
||
| @DeleteMapping("/api/admin/notices/{id}") | ||
| public ResponseEntity<Void> deleteNotice( | ||
| HttpServletRequest request, | ||
| @PathVariable Long id | ||
| ) { | ||
| if (validator.isRateLimited(request) || validator.isUnauthorized(request)) { | ||
| return ResponseEntity.status(401).build(); | ||
| } | ||
| adminNoticeService.deleteNotice(id); | ||
| return ResponseEntity.status(HttpStatus.NO_CONTENT).build(); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| package kr.allcll.backend.admin.notice; | ||
|
|
||
| import java.util.List; | ||
| import kr.allcll.backend.admin.notice.dto.CreateNoticeRequest; | ||
| import kr.allcll.backend.admin.notice.dto.CreateNoticeResponse; | ||
| import kr.allcll.backend.admin.notice.dto.NoticesResponse; | ||
| import kr.allcll.backend.admin.notice.dto.UpdateNoticeRequest; | ||
| import kr.allcll.backend.admin.notice.dto.UpdateNoticeResponse; | ||
| import kr.allcll.backend.domain.notice.Notice; | ||
| import kr.allcll.backend.domain.notice.NoticeRepository; | ||
| import kr.allcll.backend.support.exception.AllcllErrorCode; | ||
| import kr.allcll.backend.support.exception.AllcllException; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.stereotype.Service; | ||
| import org.springframework.transaction.annotation.Transactional; | ||
|
|
||
| @Service | ||
| @Transactional(readOnly = true) | ||
| @RequiredArgsConstructor | ||
| public class AdminNoticeService { | ||
|
|
||
| private final NoticeRepository noticeRepository; | ||
|
|
||
| public NoticesResponse getAllNotice() { | ||
| List<Notice> allNotices = noticeRepository.findAllOrderedByCreatedAt(); | ||
| return NoticesResponse.from(allNotices); | ||
| } | ||
|
|
||
| @Transactional | ||
| public CreateNoticeResponse createNewNotice(CreateNoticeRequest createNoticeRequest) { | ||
| Notice notice = noticeRepository.save(Notice.of( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. DTO 내부에서 도메인으로 변환하는 로직을 캡슐화한다면, 같은 로직이 layer 층에서 반복되더라도 변경 지점이 줄 것 같아 제안해봅니다~!
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 해당 부분 이전에도 제안해주셨던 것으로 기억하는데요, DTO 내부에서 도메인을 생성하는게 과연 적절한지 쫌 의문입니다.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 제가 좀 끄적여봤는데, 상당히 길이가 길어서 읽기 리소스가 보통이 아니더라구요 DTO ↔ 도메인 변환 책임을 어디에 둘 것인가배경
무엇이 신경 쓰이는가이 코드에서 가장 자주 바뀌는 지점은 DTO의 필드, 그 다음이 엔티티의 필드라고 봅니다. 그런데 지금 구조에서는 그 변경이 다음과 같이 번집니다. DTO에 필드가 하나 추가되면
Service는 "저장한다 / 갱신한다"만 알면 충분한데, 엔티티의 조립 시그니처를 매번 따라다녀야 하는 셈입니다. 캡슐화 관점에서도 도메인 필드들이 Service 본문에 그대로 나열되어 노출됩니다. 제안DTO가 자기 자신을 도메인으로 변환하는 책임을 가지면 좋겠습니다. public record CreateNoticeRequest(...) {
public Notice toEntity() {
return Notice.of(title, content, operationType);
}
}
public record UpdateNoticeRequest(...) {
public void applyTo(Notice notice) {
notice.update(title, content, operationType);
}
}public CreateNoticeResponse createNewNotice(CreateNoticeRequest request) {
Notice notice = noticeRepository.save(request.toEntity());
return CreateNoticeResponse.from(notice);
}
public UpdateNoticeResponse updateNotice(Long id, UpdateNoticeRequest request) {
Notice notice = noticeRepository.findActiveById(id)
.orElseThrow(() -> new AllcllException(AllcllErrorCode.NOTICE_NOT_FOUND, id));
request.applyTo(notice);
noticeRepository.flush();
return UpdateNoticeResponse.from(notice);
}이렇게 했을 때의 이점
우려에 대한 답
지금은 요약
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 좋네요 해당 부분 반영했습니다~ |
||
| createNoticeRequest.title(), | ||
| createNoticeRequest.content(), | ||
| createNoticeRequest.operationType() | ||
| )); | ||
| return CreateNoticeResponse.from(notice); | ||
| } | ||
|
|
||
| @Transactional | ||
| public UpdateNoticeResponse updateNotice(Long id, UpdateNoticeRequest updateNoticeRequest) { | ||
| Notice notice = noticeRepository.findActiveById(id) | ||
| .orElseThrow(() -> new AllcllException(AllcllErrorCode.NOTICE_NOT_FOUND, id)); | ||
| notice.update( | ||
| updateNoticeRequest.title(), | ||
| updateNoticeRequest.content(), | ||
| updateNoticeRequest.operationType() | ||
| ); | ||
| return UpdateNoticeResponse.from(notice); | ||
| } | ||
|
|
||
| @Transactional | ||
| public void deleteNotice(Long id) { | ||
| Notice notice = noticeRepository.findActiveById(id) | ||
| .orElseThrow(() -> new AllcllException(AllcllErrorCode.NOTICE_NOT_FOUND, id)); | ||
| softDeleteNotice(notice); | ||
| } | ||
|
|
||
| private void softDeleteNotice(Notice notice) { | ||
| notice.delete(); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| package kr.allcll.backend.admin.notice.dto; | ||
|
|
||
| import jakarta.validation.constraints.NotBlank; | ||
| import jakarta.validation.constraints.NotNull; | ||
| import jakarta.validation.constraints.Size; | ||
| import kr.allcll.backend.domain.operationPeriod.OperationType; | ||
|
|
||
| public record CreateNoticeRequest( | ||
| @NotBlank | ||
| @Size(max = 250) | ||
| String title, | ||
|
|
||
| @NotBlank | ||
| @Size(max = 1000) | ||
| String content, | ||
|
|
||
| @NotNull | ||
| OperationType operationType | ||
| ) { | ||
|
|
||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| package kr.allcll.backend.admin.notice.dto; | ||
|
|
||
| import java.time.LocalDateTime; | ||
| import kr.allcll.backend.domain.notice.Notice; | ||
| import kr.allcll.backend.domain.operationPeriod.OperationType; | ||
|
|
||
| public record CreateNoticeResponse( | ||
| long id, | ||
| String title, | ||
| String content, | ||
| OperationType operationType, | ||
| LocalDateTime createdAt | ||
| ) { | ||
|
|
||
| public static CreateNoticeResponse from(Notice notice) { | ||
| return new CreateNoticeResponse( | ||
| notice.getId(), | ||
| notice.getTitle(), | ||
| notice.getContent(), | ||
| notice.getOperationType(), | ||
| notice.getCreatedAt() | ||
| ); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| package kr.allcll.backend.admin.notice.dto; | ||
|
|
||
| import java.time.LocalDateTime; | ||
| import kr.allcll.backend.domain.notice.Notice; | ||
| import kr.allcll.backend.domain.operationPeriod.OperationType; | ||
|
|
||
| public record NoticeResponse( | ||
| long id, | ||
| String title, | ||
| String content, | ||
| OperationType operationType, | ||
| LocalDateTime createdAt | ||
| ) { | ||
|
|
||
| public static NoticeResponse from(Notice notice) { | ||
| return new NoticeResponse( | ||
| notice.getId(), | ||
| notice.getTitle(), | ||
| notice.getContent(), | ||
| notice.getOperationType(), | ||
| notice.getCreatedAt() | ||
| ); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| package kr.allcll.backend.admin.notice.dto; | ||
|
|
||
| import java.util.List; | ||
| import kr.allcll.backend.domain.notice.Notice; | ||
|
|
||
| public record NoticesResponse( | ||
| List<NoticeResponse> notices | ||
| ) { | ||
|
|
||
| public static NoticesResponse from(List<Notice> allNotices) { | ||
| return new NoticesResponse( | ||
| allNotices.stream() | ||
| .map(NoticeResponse::from) | ||
| .toList() | ||
| ); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| package kr.allcll.backend.admin.notice.dto; | ||
|
|
||
| import jakarta.validation.constraints.Size; | ||
| import kr.allcll.backend.domain.operationPeriod.OperationType; | ||
|
|
||
| public record UpdateNoticeRequest( | ||
| @Size(max = 250) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. CreateNoticeRequest에서는
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. PATCH 요청이라 수정하지 않은 필드는 보내지 않을 것으로 보고 있어서, 해당 값들은 null로 들어올 수 있다고 판단했습니다.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 아,, 맞네요 |
||
| String title, | ||
|
|
||
| @Size(max = 1000) | ||
| String content, | ||
|
Comment on lines
+7
to
+11
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The update DTO only enforces Useful? React with 👍 / 👎. |
||
|
|
||
| OperationType operationType | ||
| ) { | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| package kr.allcll.backend.admin.notice.dto; | ||
|
|
||
| import java.time.LocalDateTime; | ||
| import kr.allcll.backend.domain.notice.Notice; | ||
| import kr.allcll.backend.domain.operationPeriod.OperationType; | ||
|
|
||
| public record UpdateNoticeResponse( | ||
| long id, | ||
| String title, | ||
| String content, | ||
| OperationType operationType, | ||
| LocalDateTime createdAt | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이 부분은 공지 작성 시각보다는,
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 흠...그러네요 BaseEntity에 updatedAt을 추가했는데 어떠신가요? (-> 추후 추가 작업사항: CrawlerBaseEntity에도 똑같이 만들어주기) |
||
| ) { | ||
|
|
||
| public static UpdateNoticeResponse from(Notice notice) { | ||
| return new UpdateNoticeResponse( | ||
| notice.getId(), | ||
| notice.getTitle(), | ||
| notice.getContent(), | ||
| notice.getOperationType(), | ||
| notice.getCreatedAt() | ||
| ); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| package kr.allcll.backend.domain.notice; | ||
|
|
||
| import jakarta.persistence.Column; | ||
| import jakarta.persistence.Entity; | ||
| import jakarta.persistence.EnumType; | ||
| import jakarta.persistence.Enumerated; | ||
| import jakarta.persistence.GeneratedValue; | ||
| import jakarta.persistence.GenerationType; | ||
| import jakarta.persistence.Id; | ||
| import jakarta.persistence.Table; | ||
| import kr.allcll.backend.domain.operationPeriod.OperationType; | ||
| import kr.allcll.backend.support.entity.BaseEntity; | ||
| import lombok.AccessLevel; | ||
| import lombok.Getter; | ||
| import lombok.NoArgsConstructor; | ||
|
|
||
| @Table(name = "notices") | ||
| @Entity | ||
| @Getter | ||
| @NoArgsConstructor(access = AccessLevel.PROTECTED) | ||
| public class Notice extends BaseEntity { | ||
|
|
||
| @Id | ||
| @GeneratedValue(strategy = GenerationType.IDENTITY) | ||
| private Long id; | ||
|
|
||
| @Column(nullable = false, length = 250) | ||
| private String title; | ||
|
|
||
| @Column(nullable = false, length = 1000) | ||
| private String content; | ||
|
|
||
| @Enumerated(EnumType.STRING) | ||
| private OperationType operationType; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 음 그렇네요. 추가했습니다 |
||
|
|
||
| private Notice(String title, String content, OperationType operationType) { | ||
| this.title = title; | ||
| this.content = content; | ||
| this.operationType = operationType; | ||
| } | ||
|
|
||
| public static Notice of(String title, String content, OperationType operationType) { | ||
| return new Notice( | ||
| title, | ||
| content, | ||
| operationType | ||
| ); | ||
| } | ||
|
|
||
| public void update(String title, String content, OperationType operationType) { | ||
| if (title != null) { | ||
| this.title = title; | ||
| } | ||
| if (content != null) { | ||
| this.content = content; | ||
| } | ||
| if (operationType != null) { | ||
| this.operationType = operationType; | ||
| } | ||
| } | ||
|
|
||
| public void delete() { | ||
| super.delete(); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| package kr.allcll.backend.domain.notice; | ||
|
|
||
| import java.util.List; | ||
| import java.util.Optional; | ||
| import org.springframework.data.jpa.repository.JpaRepository; | ||
| import org.springframework.data.jpa.repository.Query; | ||
|
|
||
| public interface NoticeRepository extends JpaRepository<Notice, Long> { | ||
|
|
||
| @Query(""" | ||
| select n from Notice n | ||
| where n.isDeleted = false | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이것도 @SQLRestriction("deleted_at IS NULL") 이걸 도메인 위의 어노테이션으로 붙이면서 까묵지 않고 처리할 수 있게 될 것 같아 제안드려봅니당
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이러면 조건을 항상 추가해줄 수 있군요?! 엔티티 전체적으로 반영하면 좋을 것 같아서
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 좋습니다~! |
||
| order by n.createdAt desc | ||
| """) | ||
| List<Notice> findAllOrderedByCreatedAt(); | ||
|
|
||
| @Query(""" | ||
| select n from Notice n | ||
| where n.id = :id | ||
| and n.isDeleted = false | ||
| """) | ||
| Optional<Notice> findActiveById(Long id); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding
@Validhere makes invalid payloads throwMethodArgumentNotValidException, butGlobalExceptionHandlercurrently has no dedicated handler and its catch-allExceptionpath returnsSERVER_ERROR(500). That means a bad notice create/update request is reported as a server fault instead of a client input error, which will break client-side error handling and monitoring by inflating 5xx rates for ordinary validation mistakes.Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ㅇㅈ ExceptionHandler 추가하겠습니다