Skip to content
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

게시글 조기 마감 기능 구현 #115

Merged
merged 17 commits into from
Aug 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
24057e5
refactor: (#95) 필요없는 테스트 클래스 삭제
tjdtls690 Jul 20, 2023
e65bfab
feat: (#95) 해당 게시글 조기 마감 기능 구현
tjdtls690 Jul 20, 2023
1d93dd7
Merge branch 'dev' of github.com:woowacourse-teams/2023-votogether in…
tjdtls690 Jul 20, 2023
8ab5bf2
refactor: (#95) API 성공 시, swagger 표시를 201에서 200으로 수정
tjdtls690 Jul 21, 2023
c1692e2
refactor: (#95) swagger 500 에러 설명은 생략
tjdtls690 Jul 28, 2023
03fd8bd
refactor: (#95) Post 클래스 마지막 줄 개행
tjdtls690 Jul 28, 2023
7b5e0b7
refactor: (#95) PostService 클래스 마지막 줄 개행
tjdtls690 Jul 28, 2023
50380e4
feat: (#95) origin dev로부터 최신화 진행
tjdtls690 Jul 28, 2023
9b775e3
refactor: (#95) 작성자인 경우만 조기 마감이 가능하도록 구현
tjdtls690 Jul 28, 2023
b4c9b43
refactor: (#95) 조기 마감 할 시, 본인 게시글인지, 마감되지 않은 게시글인지, 마감 시간까지 절반 시간이 지난…
tjdtls690 Jul 28, 2023
e0f4990
test: (#95) 게시글 조기 마감 시, 유효성 검증에 대한 테스트 코드 추가
tjdtls690 Jul 30, 2023
bc66a7b
refactor: (#95) PathVariable 값인 id의 변수명을 postId로 더 명확하게 개선
tjdtls690 Jul 30, 2023
21d10f3
refactor: (#95) path parameter를 사용하여 테스트 코드의 url을 더 직관적으로 개선
tjdtls690 Jul 30, 2023
34e5aa7
refactor: (#95) PostServiceTest의 코드에서 finded 단어를 found로 개선
tjdtls690 Jul 30, 2023
3f0f28a
refactor: (#95) 조기 마감하는 메서드 명들을 더 알맞은 단어로 개선
tjdtls690 Jul 30, 2023
b8d8b56
feat: (#95) origin dev 최신화
tjdtls690 Aug 2, 2023
82c227e
Merge branch 'dev' of github.com:woowacourse-teams/2023-votogether in…
tjdtls690 Aug 2, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

@NoArgsConstructor(access = AccessLevel.PROTECTED)
@EqualsAndHashCode(of = {"id"})
@ToString
@Getter
@Entity
public class Member extends BaseEntity {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.springframework.http.MediaType;
import org.springframework.http.ResponseEntity;
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.RequestMapping;
Expand Down Expand Up @@ -98,5 +99,19 @@ public ResponseEntity<VoteOptionStatisticsResponse> getVoteOptionStatistics(
return ResponseEntity.ok(response);
}

@Operation(summary = "게시글 조기 마감", description = "게시글을 조기 마감한다.")
@ApiResponses(value = {
@ApiResponse(responseCode = "200", description = "게시물이 조기 마감 되었습니다."),
@ApiResponse(responseCode = "400", description = "잘못된 입력입니다.")
})
@PatchMapping("/{postId}/close")
public ResponseEntity<Void> closePostEarly(
@PathVariable final Long postId,
@Auth final Member loginMember
) {
postService.closePostEarlyById(postId, loginMember);
return ResponseEntity.ok().build();
}

}

44 changes: 27 additions & 17 deletions backend/src/main/java/com/votogether/domain/post/entity/Post.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import jakarta.persistence.JoinColumn;
import jakarta.persistence.ManyToOne;
import jakarta.persistence.OneToMany;
import java.time.Duration;
import java.time.LocalDateTime;
import java.util.ArrayList;
import java.util.List;
Expand Down Expand Up @@ -106,10 +107,6 @@ private List<PostOption> toPostOptions(
.toList();
}

public boolean hasPostOption(final PostOption postOption) {
return postOptions.contains(postOption);
}

public void validateDeadlineNotExceedByMaximumDeadline(final int maximumDeadline) {
LocalDateTime maximumDeadlineFromNow = LocalDateTime.now().plusDays(maximumDeadline);
if (this.deadline.isAfter(maximumDeadlineFromNow)) {
Expand All @@ -118,15 +115,11 @@ public void validateDeadlineNotExceedByMaximumDeadline(final int maximumDeadline
}

public void validateWriter(final Member member) {
if (!Objects.equals(this.writer.getId(), member.getId())) {
if (!Objects.equals(this.writer, member)) {
throw new BadRequestException(PostExceptionType.NOT_WRITER);
}
}

public boolean isClosed() {
return deadline.isBefore(LocalDateTime.now());
}

public Vote makeVote(final Member voter, final PostOption postOption) {
validateDeadLine();
validateVoter(voter);
Expand All @@ -140,26 +133,43 @@ public Vote makeVote(final Member voter, final PostOption postOption) {
return vote;
}

public void validateDeadLine() {
if (isClosed()) {
throw new BadRequestException(PostExceptionType.POST_CLOSED);
}
}

private boolean isClosed() {
return deadline.isBefore(LocalDateTime.now());
}

private void validateVoter(final Member voter) {
if (Objects.equals(this.writer.getId(), voter.getId())) {
throw new BadRequestException(PostExceptionType.NOT_VOTER);
}
}

private void validateDeadLine() {
if (isClosed()) {
throw new IllegalStateException("게시글이 이미 마감되었습니다.");
private void validatePostOption(final PostOption postOption) {
if (!hasPostOption(postOption)) {
throw new BadRequestException(PostExceptionType.POST_OPTION_NOT_FOUND);
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3
크게 중요하진 않지만 BadRequestException인데 NotFound를 던지고 있으니 개인적으로는 어색하다는 생각이 듭니다.
NonExistPostOption등의 네이밍으로 변경해보는 것은 어떨까요?

}
}

private void validatePostOption(final PostOption postOption) {
if (!hasPostOption(postOption)) {
throw new IllegalArgumentException("해당 게시글에서 존재하지 않는 선택지 입니다.");
private boolean hasPostOption(final PostOption postOption) {
return postOptions.contains(postOption);
}

public void validateHalfDeadLine() {
final Duration betweenDuration = Duration.between(getCreatedAt(), this.deadline);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Duration...처음 봤습니다 이런 기능이 있었군요!

final LocalDateTime midpoint = getCreatedAt().plus(betweenDuration.dividedBy(2));
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3

Suggested change
final LocalDateTime midpoint = getCreatedAt().plus(betweenDuration.dividedBy(2));
final LocalDateTime earlyClosingTime = getCreatedAt().plus(betweenDuration.dividedBy(2));

변수명을 조금 더 명확하게 해도 좋을 것 같아요 :)


if (midpoint.isAfter(LocalDateTime.now())) {
throw new BadRequestException(PostExceptionType.POST_NOT_HALF_DEADLINE);
}
}

public boolean isWriter(final Member member) {
return Objects.equals(this.writer, member);
public void closeEarly() {
this.deadline = LocalDateTime.now();
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3
클래스 마지막 줄 개행이 필요할 것 같아요 :)


public void addContentImage(final String contentImageUrl) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,12 @@ public enum PostExceptionType implements ExceptionType {
POST_OPTION_NOT_FOUND(1001, "해당 게시글 투표 옵션이 존재하지 않습니다."),
UNRELATED_POST_OPTION(1002, "게시글 투표 옵션이 게시글과 연관되어 있지 않습니다."),
NOT_WRITER(1003, "해당 게시글 작성자가 아닙니다."),
POST_CLOSED(1004, "게시글이 이미 마감되었습니다."),
POST_NOT_HALF_DEADLINE(1005, "게시글이 마감 시간까지 절반의 시간 이상이 지나지 않으면 조기마감을 할 수 없습니다."),
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3
CloseEarlyNotAllowed등의 네이밍을 사용해보는 것은 어떨까요?
개인적으로 한번만에 해석이 잘 되지 않는 것 같아서요..!

NOT_VOTER(1004, "해당 게시글 작성자는 투표할 수 없습니다."),
DEADLINE_EXCEED_THREE_DAYS(1005, "마감 기한은 현재 시간으로부터 3일을 초과할 수 없습니다."),
WRONG_IMAGE(1006, "이미지 저장에 실패했습니다. 다시 시도해주세요.");
WRONG_IMAGE(1006, "이미지 저장에 실패했습니다. 다시 시도해주세요."),
;

private final int code;
private final String message;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,4 +213,15 @@ private String groupAgeRange(final String ageRange) {
return ageRange;
}

@Transactional
public void closePostEarlyById(final Long id, final Member loginMember) {
final Post post = postRepository.findById(id)
.orElseThrow(() -> new IllegalArgumentException("해당 게시글은 존재하지 않습니다."));

Copy link
Collaborator

Choose a reason for hiding this comment

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

P1

  1. 멤버 정보를 바탕으로 본인 게시글인지
  2. 마감되지 않은 게시글인지
  3. 마감 시간까지 절반의 시간이 지났는지

위의 정보에 대한 검증이 필요해 보입니다!

Copy link
Collaborator

Choose a reason for hiding this comment

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

동의합니다 특히나 3번 검증 과정은 정책상 필수적이라는 생각이 드네요!
1번은 멤버를 받으면서 자연스럽게 작성하실 것이라는 생각이 듭니다 ㅎㅎ

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

지금까지 예외 처리에 대한 부분은 고민하지 않고 구현을 했었는데, 이제 Exception 구조가 자리 잡혔으니 바로 적용해보도록 하겠습니다.

post.validateWriter(loginMember);
post.validateDeadLine();
post.validateHalfDeadLine();
post.closeEarly();
Comment on lines +221 to +224
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3
꼼꼼한 검증 좋습니다 👍

}
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3
요기도 마지막 줄 개행이 필요할 것 같아요 :)


}
Original file line number Diff line number Diff line change
Expand Up @@ -297,4 +297,20 @@ void getVoteOptionStatistics() {
assertThat(result).usingRecursiveComparison().isEqualTo(response);
}

@Test
@DisplayName("게시글을 조기 마감 합니다")
void postClosedEarly() {
// given
long postId = 1L;

// when
ExtractableResponse<MockMvcResponse> response = RestAssuredMockMvc.given().log().all()
.when().patch("/posts/{postId}/close", postId)
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍🏻

.then().log().all()
.extract();

// then
assertThat(response.statusCode()).isEqualTo(HttpStatus.OK.value());
}

}
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package com.votogether.domain.post.entity;

import static com.votogether.fixtures.MemberFixtures.MALE_30;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatNoException;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
Expand All @@ -14,6 +13,7 @@
import com.votogether.fixtures.MemberFixtures;
import java.nio.charset.StandardCharsets;
import java.time.LocalDateTime;
import java.time.temporal.ChronoUnit;
import java.util.List;
import java.util.stream.Stream;
import org.junit.jupiter.api.DisplayName;
Expand Down Expand Up @@ -103,41 +103,79 @@ void throwExceptionIsWriter() {
}

@Test
@DisplayName("게시글의 작성자 여부를 확인한다.")
void isWriter() {
@DisplayName("게시글의 마감 여부에 따라 예외를 던질 지 결정한다.")
void throwExceptionIsDeadlinePassed() {
// given
Post post = Post.builder()
.writer(MALE_30.get())
final Member writer = MemberFixtures.MALE_30.get();
ReflectionTestUtils.setField(writer, "id", 1L);

Post post1 = Post.builder()
.writer(writer)
.deadline(LocalDateTime.of(2000, 1, 1, 1, 1))
.build();

// when
boolean result1 = post.isWriter(MALE_30.get());
Post post2 = Post.builder()
.writer(writer)
.deadline(LocalDateTime.of(9999, 1, 1, 1, 1))
.build();

// then
assertThat(result1).isTrue();
// when, then
assertAll(
() -> assertThatThrownBy(post1::validateDeadLine)
.isInstanceOf(BadRequestException.class)
.hasMessage(PostExceptionType.POST_CLOSED.getMessage()),
() -> assertThatNoException()
.isThrownBy(post2::validateDeadLine)
);
}

@Test
@DisplayName("게시글의 마감 여부를 확인한다.")
void isClosed() {
@DisplayName("게시글의 마감까지 절반의 시간을 넘겼는 지에 따라 예외를 던질 지 결정한다.")
void throwExceptionIsHalfToTheDeadline() {
// given
Post postA = Post.builder()
.deadline(LocalDateTime.of(2022, 1, 1, 0, 0))
final Member writer = MemberFixtures.MALE_30.get();
ReflectionTestUtils.setField(writer, "id", 1L);

Post post1 = Post.builder()
.writer(writer)
.deadline(LocalDateTime.of(9999, 1, 1, 1, 1))
.build();
ReflectionTestUtils.setField(post1, "createdAt", LocalDateTime.now());

Post post2 = Post.builder()
.writer(writer)
.deadline(LocalDateTime.now().plus(100, ChronoUnit.MILLIS))
.build();
ReflectionTestUtils.setField(post2, "createdAt", LocalDateTime.now());

// when, then
assertAll(
() -> assertThatThrownBy(post1::validateHalfDeadLine)
.isInstanceOf(BadRequestException.class)
.hasMessage(PostExceptionType.POST_NOT_HALF_DEADLINE.getMessage()),
() -> {
Thread.sleep(50);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Q
Thread.sleep을 사용하신 이유가 궁금합니다!

assertThatNoException()
.isThrownBy(post2::validateHalfDeadLine);

Post postB = Post.builder()
.deadline(LocalDateTime.of(3222, 1, 1, 0, 0))
}
);
}

@Test
@DisplayName("해당 게시글을 조기 마감 합니다.")
void closedEarly() {
// given
LocalDateTime deadline = LocalDateTime.of(2100, 1, 1, 0, 0);
Post post = Post.builder()
.deadline(deadline)
.build();

// when
boolean resultA = postA.isClosed();
boolean resultB = postB.isClosed();
post.closeEarly();

// then
assertAll(
() -> assertThat(resultA).isTrue(),
() -> assertThat(resultB).isFalse()
);
assertThat(post.getDeadline()).isBefore(deadline);
}

}

This file was deleted.

Loading