-
Notifications
You must be signed in to change notification settings - Fork 5
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
[BE] issue255: 스터디 상세 정보 수정 #278
Changes from 7 commits
a6777da
72650c5
c570b61
e67b7da
1269c08
cd806d4
e9c71d8
04f7880
89ea2d5
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 |
---|---|---|
|
@@ -31,7 +31,8 @@ public AuthenticationRequestMatcher authenticationRequestMatcher() { | |
) | ||
.addUpAuthenticationPath(HttpMethod.PUT, | ||
"/api/studies/\\d+/reviews/\\d+", | ||
"/api/studies/\\d+/reference-room/links/\\d+" | ||
"/api/studies/\\d+/reference-room/links/\\d+", | ||
"/api/study/\\d+" | ||
Comment on lines
+34
to
+35
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. AuthRequestMatchConfig가 제법 이제 커졌는데 어떻게 더욱 좋게 관리할 수 있을지 보고서 작성해서 제출하세요 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. 보...고서..?? 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. 어데서 오셨나요..? 보고서요 푸하하
Comment on lines
-34
to
+35
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. 확실히 이제 AuthRequestMatchConfig가 복잡해지긴 했네요 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. 확실히 데모 이후에는 고민해보아야할 것 같습니다ㅠ.ㅠ |
||
) | ||
.addUpAuthenticationPath(HttpMethod.DELETE, | ||
"/api/studies/\\d+/reviews/\\d+", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,13 +3,14 @@ | |
import com.woowacourse.moamoa.auth.config.AuthenticationPrincipal; | ||
import com.woowacourse.moamoa.study.domain.Study; | ||
import com.woowacourse.moamoa.study.service.StudyService; | ||
import com.woowacourse.moamoa.study.service.request.CreatingStudyRequest; | ||
import com.woowacourse.moamoa.study.service.request.StudyRequest; | ||
import java.net.URI; | ||
import javax.validation.Valid; | ||
import lombok.RequiredArgsConstructor; | ||
import org.springframework.http.ResponseEntity; | ||
import org.springframework.web.bind.annotation.PathVariable; | ||
import org.springframework.web.bind.annotation.PostMapping; | ||
import org.springframework.web.bind.annotation.PutMapping; | ||
import org.springframework.web.bind.annotation.RequestBody; | ||
import org.springframework.web.bind.annotation.RequestMapping; | ||
import org.springframework.web.bind.annotation.RestController; | ||
|
@@ -24,9 +25,9 @@ public class StudyController { | |
@PostMapping | ||
public ResponseEntity<Void> createStudy( | ||
@AuthenticationPrincipal final Long githubId, | ||
@Valid @RequestBody(required = false) final CreatingStudyRequest creatingStudyRequest | ||
@Valid @RequestBody(required = false) final StudyRequest studyRequest | ||
) { | ||
final Study study = studyService.createStudy(githubId, creatingStudyRequest); | ||
final Study study = studyService.createStudy(githubId, studyRequest); | ||
return ResponseEntity.created(URI.create("/api/studies/" + study.getId())).build(); | ||
} | ||
|
||
|
@@ -37,4 +38,13 @@ public ResponseEntity<Void> participateStudy(@AuthenticationPrincipal final Long | |
studyService.participateStudy(githubId, studyId); | ||
return ResponseEntity.ok().build(); | ||
} | ||
|
||
@PutMapping("/{study-id}") | ||
public ResponseEntity<Void> updateStudy(@AuthenticationPrincipal final Long githubId, | ||
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. 베루스, 그린론이 바꾼 @AuthenticatedMember로 바꿔도 좋을 것 같아요! 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. 좋습니다~_~ |
||
@PathVariable("study-id") final Long studyId, | ||
@Valid @RequestBody(required = false) final StudyRequest request | ||
) { | ||
studyService.updateStudy(githubId, studyId, request); | ||
return ResponseEntity.ok().build(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,10 +3,12 @@ | |
import static javax.persistence.GenerationType.IDENTITY; | ||
import static lombok.AccessLevel.PROTECTED; | ||
|
||
import com.woowacourse.moamoa.common.exception.UnauthorizedException; | ||
import com.woowacourse.moamoa.study.domain.exception.InvalidPeriodException; | ||
import com.woowacourse.moamoa.study.service.exception.FailureParticipationException; | ||
import java.time.LocalDate; | ||
import java.time.LocalDateTime; | ||
import java.util.Objects; | ||
import javax.persistence.Column; | ||
import javax.persistence.Embedded; | ||
import javax.persistence.Entity; | ||
|
@@ -129,4 +131,20 @@ public MemberRole getRole(final Long memberId) { | |
} | ||
return MemberRole.NON_MEMBER; | ||
} | ||
|
||
public void update(Long memberId, Content content, RecruitPlanner recruitPlanner, AttachedTags attachedTags, | ||
StudyPlanner studyPlanner | ||
) { | ||
checkOwner(memberId); | ||
this.content = content; | ||
this.recruitPlanner = recruitPlanner; | ||
this.attachedTags = attachedTags; | ||
this.studyPlanner = studyPlanner; | ||
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 void checkOwner(Long memberId) { | ||
if (!Objects.equals(getParticipants().getOwnerId(), memberId)) { | ||
throw new UnauthorizedException("스터디 방장만이 스터디를 수정할 수 있습니다."); | ||
} | ||
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. 그 isOwner() 사용하지 않고 여기서 바로 equals 한 이유가 있을까요? 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. participants의 |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ | |
import com.woowacourse.moamoa.common.utils.DateTimeSystem; | ||
import com.woowacourse.moamoa.member.domain.Member; | ||
import com.woowacourse.moamoa.member.domain.repository.MemberRepository; | ||
import com.woowacourse.moamoa.member.service.exception.MemberNotFoundException; | ||
import com.woowacourse.moamoa.study.domain.AttachedTags; | ||
import com.woowacourse.moamoa.study.domain.Content; | ||
import com.woowacourse.moamoa.study.domain.Participants; | ||
|
@@ -12,7 +13,7 @@ | |
import com.woowacourse.moamoa.study.domain.StudyPlanner; | ||
import com.woowacourse.moamoa.study.domain.repository.StudyRepository; | ||
import com.woowacourse.moamoa.study.service.exception.StudyNotFoundException; | ||
import com.woowacourse.moamoa.study.service.request.CreatingStudyRequest; | ||
import com.woowacourse.moamoa.study.service.request.StudyRequest; | ||
import java.time.LocalDate; | ||
import java.time.LocalDateTime; | ||
import java.util.List; | ||
|
@@ -36,7 +37,7 @@ public StudyService(final StudyRepository studyRepository, | |
this.dateTimeSystem = dateTimeSystem; | ||
} | ||
|
||
public Study createStudy(final Long githubId, final CreatingStudyRequest request) { | ||
public Study createStudy(final Long githubId, final StudyRequest request) { | ||
final LocalDateTime createdAt = dateTimeSystem.now(); | ||
final Member owner = findMemberBy(githubId); | ||
|
||
|
@@ -47,7 +48,8 @@ public Study createStudy(final Long githubId, final CreatingStudyRequest request | |
final AttachedTags attachedTags = request.mapToAttachedTags(); | ||
final Content content = request.mapToContent(); | ||
|
||
return studyRepository.save(new Study(content, participants, recruitPlanner, studyPlanner, attachedTags, createdAt)); | ||
return studyRepository.save( | ||
new Study(content, participants, recruitPlanner, studyPlanner, attachedTags, createdAt)); | ||
} | ||
|
||
public void participateStudy(final Long githubId, final Long studyId) { | ||
|
@@ -74,4 +76,14 @@ public void autoUpdateStatus() { | |
study.changeStatus(now); | ||
} | ||
} | ||
|
||
public void updateStudy(Long githubId, Long studyId, StudyRequest request) { | ||
Study study = studyRepository.findById(studyId) | ||
.orElseThrow(StudyNotFoundException::new); | ||
Member member = memberRepository.findByGithubId(githubId) | ||
.orElseThrow(MemberNotFoundException::new); | ||
|
||
study.update(member.getId(), request.mapToContent(), request.mapToRecruitPlan(), request.mapToAttachedTags(), | ||
request.mapToStudyPlanner(LocalDate.now())); | ||
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. update할 때 파라미터가 5개나 필요한데요 그래서 너무 복잡해보이기도 하는데 혹시 어떻게 생각하세요? 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. request 자체를 넘겨서 study.update(request) 로 감추어도 되는데, 도메인 객체로 바로 바꿀 수 있는 |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
package com.woowacourse.acceptance.test.study; | ||
|
||
import static com.woowacourse.acceptance.fixture.TagFixtures.BE_태그_ID; | ||
import static com.woowacourse.acceptance.fixture.TagFixtures.우테코4기_태그_ID; | ||
import static com.woowacourse.acceptance.fixture.TagFixtures.자바_태그_ID; | ||
import static com.woowacourse.acceptance.steps.LoginSteps.짱구가; | ||
import static org.springframework.http.HttpHeaders.ACCEPT; | ||
import static org.springframework.http.HttpHeaders.AUTHORIZATION; | ||
import static org.springframework.restdocs.headers.HeaderDocumentation.headerWithName; | ||
import static org.springframework.restdocs.headers.HeaderDocumentation.requestHeaders; | ||
import static org.springframework.restdocs.restassured3.RestAssuredRestDocumentation.document; | ||
|
||
import com.woowacourse.acceptance.AcceptanceTest; | ||
import com.woowacourse.moamoa.study.service.request.StudyRequest; | ||
import com.woowacourse.moamoa.study.service.request.StudyRequestBuilder; | ||
import io.restassured.RestAssured; | ||
import java.time.LocalDate; | ||
import java.util.List; | ||
import org.junit.jupiter.api.DisplayName; | ||
import org.junit.jupiter.api.Test; | ||
import org.springframework.http.HttpHeaders; | ||
import org.springframework.http.HttpStatus; | ||
import org.springframework.http.MediaType; | ||
|
||
public class UpdatingStudyAcceptanceTest extends AcceptanceTest { | ||
|
||
@DisplayName("스터디 내용을 수정할 수 있다.") | ||
@Test | ||
public void updateStudy() { | ||
final LocalDate 지금 = LocalDate.now(); | ||
final long studyId = 짱구가().로그인하고().자바_스터디를() | ||
.시작일자는(지금).태그는(자바_태그_ID, 우테코4기_태그_ID, BE_태그_ID) | ||
.생성한다(); | ||
final String accessToken = 짱구가().로그인한다(); | ||
|
||
final StudyRequest request = new StudyRequestBuilder().title("변경된 제목") | ||
.description("변경된 설명") | ||
.excerpt("변경된 한 줄 설명") | ||
.thumbnail("변경된 썸네일") | ||
.startDate(LocalDate.now()) | ||
.endDate(LocalDate.now().plusMonths(1)) | ||
.enrollmentEndDate(LocalDate.now().plusDays(5)) | ||
.tagIds(List.of(자바_태그_ID, 우테코4기_태그_ID)) | ||
.build(); | ||
|
||
RestAssured.given(spec).log().all() | ||
.filter(document("studies/update", | ||
requestHeaders(headerWithName("Authorization").description("Bearer Token")))) | ||
.header(AUTHORIZATION, accessToken) | ||
.header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE) | ||
.header(ACCEPT, MediaType.APPLICATION_JSON_VALUE) | ||
.pathParam("study-id", studyId) | ||
.body(request) | ||
.when().log().all() | ||
.put("/api/studies/{study-id}") | ||
.then().statusCode(HttpStatus.OK.value()); | ||
Comment on lines
+46
to
+56
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. 이렇게 인수테스트를 끝내는 것도 방법이긴 한데요! 이러나 저러나 이러쿵 저러쿵 수정된 것을 확인하기 위해서 해당 스터디의 상세 페이지를 불러와서 잘 수정되었는지 확인하는 로직도 추가해보면 어떨까요? 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. 좋습니다! |
||
} | ||
} |
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.
굿