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

[BE] 기존 api 중 폴링으로 인한 변경 반영, studyId로 참여코드 조회 api 구현 #569

Conversation

jaehee329
Copy link
Collaborator

관련 이슈

close #567

구현 기능 및 변경 사항

  • 기존 api 중 폴링으로 인한 변경 반영
  • studyId로 참여코드 조회 api 구현

@jaehee329 jaehee329 added BE 백엔드 작업 feature labels Sep 25, 2023
@jaehee329 jaehee329 added this to the 6차 스프린트 milestone Sep 25, 2023
@jaehee329 jaehee329 self-assigned this Sep 25, 2023
Copy link
Collaborator

@woosung1223 woosung1223 left a comment

Choose a reason for hiding this comment

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

고생하셨읍니다. 레거시 전문가 👍

private final ParticipantCodeService participantCodeService;

@Operation(summary = "스터디 아이디로 참여 코드 조회")
@GetMapping("/api/participantCodes")
Copy link
Collaborator

Choose a reason for hiding this comment

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

낙타(camel) 케이스가 들어 갔네요

@@ -12,7 +12,7 @@ public class FilterConfig {
public FilterRegistrationBean<CachingFilter> contentCachingFilter(){
FilterRegistrationBean<CachingFilter> registrationBean = new FilterRegistrationBean<>();
registrationBean.setFilter(new CachingFilter());
registrationBean.addUrlPatterns("/api/studies/*", "/api/temp/*", "/api/auth/*", "/api/me/*");
registrationBean.addUrlPatterns("/api/participantCodes/*", "/api/studies/*", "/api/temp/*", "/api/auth/*", "/api/me/*");
Copy link
Collaborator

Choose a reason for hiding this comment

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

여기도 API 변경되면 같이 바뀌어야 겠네용

import harustudy.backend.study.domain.Study;

public record CreateStudyResponse(@JsonIgnore Long studyId, String participantCode) {
public record CreateStudyResponse(Long studyId) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

studyId를 Location 헤더에 지정해주고 있으니 따로 응답 안보내고 204 no Content로 보내도 될 것 같은데 어떻게 생각하시나여?

Copy link
Collaborator

Choose a reason for hiding this comment

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

studyId만 남아서 이 dto자체는 없어져도 괜찮고 나중에 응답에 무언가가 추가될 수 있으니 남아도 괜찮다고 생각합니다.
그런데 테오가 이야기한 204는 이해가 안가네용. Location 헤더에 지정해주는거면 201 그대로 반환하는게 맞다고 생각하는데 조금 더 설명해주실 수 있나요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

저도 당장 사용되지 않는 dto는 삭제해도 좋을 것 같아요~ 응답코드는 Location헤더로 URI가 내려가니 201이 더 적절해 보이기는 합니다.

Copy link
Collaborator

Choose a reason for hiding this comment

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

생각해보니 201이네요 ;; 굿

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

어렴풋이 하나만 응답하는 경우에도 DTO로 감싸자는 얘기를 했던 것 같아서 이렇게 했는데 기억나는 분 있으신가요?
프론트에서도 일관되게 감싼걸로 입력받고 파싱할 것 같은데 다른 예시가 있나 찾아보니 여기밖에 없네요 ㅎㅎ;
결정되는 대로 바꿀게요~

@@ -6,19 +6,19 @@
import java.time.LocalDateTime;

public record StudyResponse(Long studyId, String name, Integer totalCycle, Integer timePerCycle, Integer currentCycle,
String studyStep, String progress, LocalDateTime createdDateTime) {
String studyStep, String progressStep, LocalDateTime createdDate, LocalDateTime lastModifiedDate) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

progressStep를 쓸지 progress를 쓸지 저희끼리 결정하고 가면 좋을 듯 하네여!(나중에 컨벤션 통일시키려면)

Copy link
Collaborator

@MoonJeWoong MoonJeWoong Sep 25, 2023

Choose a reason for hiding this comment

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

이거 progress로 사용하기로 해서 마코가 merge 하기 전에 수정했던 것 같은데 표현이 혼재되어 있었네용... 저는 개인적으로 step 붙여주는게 통일감 있어서 좋기는 합니다! 그리고 In-Progress 상태에서 단계를 나타낸다는 의미로 progressStep 네이밍도 좋은 것 같아욤

Copy link
Collaborator

@aak2075 aak2075 left a comment

Choose a reason for hiding this comment

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

테스트까지 빠르게 잘 구현해 주셨네요.

고쳐야할 만한 부분은 없는것 같아서 approve 하겠습니다.

import harustudy.backend.study.domain.Study;

public record CreateStudyResponse(@JsonIgnore Long studyId, String participantCode) {
public record CreateStudyResponse(Long studyId) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

studyId만 남아서 이 dto자체는 없어져도 괜찮고 나중에 응답에 무언가가 추가될 수 있으니 남아도 괜찮다고 생각합니다.
그런데 테오가 이야기한 204는 이해가 안가네용. Location 헤더에 지정해주는거면 201 그대로 반환하는게 맞다고 생각하는데 조금 더 설명해주실 수 있나요?

@@ -80,8 +80,7 @@ void setUp() {
@Test
void 회원으로_스터디를_진행한다() throws Exception {
LoginResponse 로그인_정보 = 구글_로그인을_진행한다();
String 참여_코드 = 스터디를_개설한다(로그인_정보);
Long 스터디_아이디 = 스터디를_조회한다(로그인_정보, 참여_코드);
Long 스터디_아이디 = 스터디를_개설한다(로그인_정보);
Copy link
Collaborator

Choose a reason for hiding this comment

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

스터디를 개설하는 사람은 플로우에서 참여 코드가 필요 없어졌군요.
참여하는 사람의 플로우는 참여 코드로 스터디를 조회하는 과정이 필요할텐데 이 시나리오도 나중에 추가되면 좋겠네요.

Copy link
Collaborator

Choose a reason for hiding this comment

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

이거 까먹을 것 같은데 이슈로 만들어둘게용

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

만들까 했는데 남의 참여코드로 참여까지만 하면 되서 전체 플로우 만들기보다 그냥 서비스 테스트정도만 만들었어요!
간단하니까 참여하는 부분까지만 인수조건으로 해서 테스트 추가할게요~

.isAlphabetic()
.isUpperCase()
.hasSize(6);
assertThat(response.studyId()).isPositive();
Copy link
Collaborator

Choose a reason for hiding this comment

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

id 값을 예측 할 수 없으니 있는지만 검증하는군요. 좋습니다 👍

Copy link
Collaborator

@MoonJeWoong MoonJeWoong left a comment

Choose a reason for hiding this comment

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

고생하셨습니다. 폴링 빠르게 구현해보죠

import harustudy.backend.study.domain.Study;

public record CreateStudyResponse(@JsonIgnore Long studyId, String participantCode) {
public record CreateStudyResponse(Long studyId) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

저도 당장 사용되지 않는 dto는 삭제해도 좋을 것 같아요~ 응답코드는 Location헤더로 URI가 내려가니 201이 더 적절해 보이기는 합니다.

@@ -6,19 +6,19 @@
import java.time.LocalDateTime;

public record StudyResponse(Long studyId, String name, Integer totalCycle, Integer timePerCycle, Integer currentCycle,
String studyStep, String progress, LocalDateTime createdDateTime) {
String studyStep, String progressStep, LocalDateTime createdDate, LocalDateTime lastModifiedDate) {
Copy link
Collaborator

@MoonJeWoong MoonJeWoong Sep 25, 2023

Choose a reason for hiding this comment

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

이거 progress로 사용하기로 해서 마코가 merge 하기 전에 수정했던 것 같은데 표현이 혼재되어 있었네용... 저는 개인적으로 step 붙여주는게 통일감 있어서 좋기는 합니다! 그리고 In-Progress 상태에서 단계를 나타낸다는 의미로 progressStep 네이밍도 좋은 것 같아욤

@@ -65,6 +65,7 @@ private StudiesResponse findStudyByMemberId(Long memberId) {
return StudiesResponse.from(studies);
}

// TODO: N+1
Copy link
Collaborator

Choose a reason for hiding this comment

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

꼼꼼함... 좋네요 👍

@@ -80,8 +80,7 @@ void setUp() {
@Test
void 회원으로_스터디를_진행한다() throws Exception {
LoginResponse 로그인_정보 = 구글_로그인을_진행한다();
String 참여_코드 = 스터디를_개설한다(로그인_정보);
Long 스터디_아이디 = 스터디를_조회한다(로그인_정보, 참여_코드);
Long 스터디_아이디 = 스터디를_개설한다(로그인_정보);
Copy link
Collaborator

Choose a reason for hiding this comment

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

이거 까먹을 것 같은데 이슈로 만들어둘게용

@jaehee329
Copy link
Collaborator Author

close #571
스터디 아이디로 참여 코드 조회 시나리오 테스트 추가했습니다

Copy link
Collaborator

@aak2075 aak2075 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다~

@woosung1223 woosung1223 merged commit 5be8362 into be/feature/566-polling Sep 26, 2023
@woosung1223 woosung1223 deleted the be/feature/567-polling-related-legacy-refactor branch September 26, 2023 02:58
aak2075 added a commit that referenced this pull request Sep 26, 2023
* feat: Polling 관련 패키지 구성

* [BE] 기존 api 중 폴링으로 인한 변경 반영, studyId로 참여코드 조회 api 구현 (#569)

* feat: progress 삭제 및 study 정보 변경 관련 api 수정

* feat: studyId로 참여 코드 조회 api 구현

* fix: CachingFilter를 /api/participantCodes 에 적용

* feat: 스터디 아이디로 참여 코드 조회 시나리오 테스트 추가

* fix: 참여 코드 관련 필터 로직 수정

* refactor: 스터디 생성 시 Location 헤더로만 응답을 반환하도록 변경

* [BE] 대기방 페이지 폴링 기능 구현하기 (#575)

* feat: 대기방 폴링 api 구현

* refactor: 스터디 참여 로직 수정

* test: 참여 관련 테스트 추가

* refactor: 생성자 체이닝 사용

* refactor: import 와일드카드 제거

* refactor: 메서드 네이밍 변경

* test: 메서드 네이밍 참여를 참여자 정보로 수정

* test: 메서드 네이밍 폴링 키워드 제거

* refactor: dto 변환 로직 dto로 이동

* refactor: 개행 제거

* refactor: 정적 팩터리 메서드 도입 및 생성자 제거로 인한 테스트 수정사항 반영

* test: expected를 given으로 이동

* refactor: 정적 팩터리 메서드 네이밍 수정

* test: expected를 given으로 이동

* [BE] 이미 시작한 스터디 신규 참여 차단 로직 구현 (#578)

* feat: 이미 시작한 스터디 참여 시 에러 반환 로직 구현

* refactor: assertThrows -> assertThatThrownBy

다른 테스트와 동일한 방식으로 변경

* [BE] 진행 페이지 폴링 기능 구현하기 (#577)

* feat: polling API 구현을 위한 view 패키지 및 기본 컨트롤러, 서비스 클래스 생성

* feat: PollingController 구현

* feat: 폴링 서비스 로직 및 테스트 구현

* feat: 스터디 상태 값만 조회하도록 구현

* feat: controller 단에 authMember 추가

* feat: 기타 수정 사항 반영

* feat: 스터디에서 step 조회 시 Enum으로 조회하도록 수정

* feat: StudyRepository 메서드 네이밍 수정

* style: 잘못된 개행 수정

* fix: resolve merge conflict

---------

Co-authored-by: aak2075 <[email protected]>

* [BE] 제출한 인원 조회하는 기능 구현하기 (#579)

* feat: 제출 인원 조회 기능 및 테스트 구현

* test: 스터디 종료 상태 테스트 케이스 추가

* refactor: DTO에서 정적 팩토리 메소드를 사용하도록 변경

* refactor: Controller -> RestController로 변경

* chore: ExceptionMapper Exception 추가

* chore: 클래스명 수정

* fix: resolve merge conflict

---------

Co-authored-by: aak2075 <[email protected]>

---------

Co-authored-by: Gyuseong Lee <[email protected]>
Co-authored-by: hiiro <[email protected]>
Co-authored-by: teo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BE 백엔드 작업 feature
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

4 participants