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

review: 여섯번째 리뷰 PR #70

Merged
merged 38 commits into from
Nov 10, 2024
Merged

review: 여섯번째 리뷰 PR #70

merged 38 commits into from
Nov 10, 2024

Conversation

5win
Copy link
Contributor

@5win 5win commented Nov 8, 2024

안녕하세요 멘토님!
여섯번째 마지막 리뷰 PR드립니다!

고생 많으셨습니다 :)

5win and others added 30 commits September 23, 2024 18:37
* [#6] feat: JPA 레포지토리 연결

   - save, findAll, findById 구현

* [#6] feat: 활동 리스트, 상세정보 응답 DTO 구현

* [#6] feat: JPA 엔티티 애노테이션 수정

   - GeneratedValue 삭제 : 받아온 데이터의 id를 그대로 사용하기 위함.
   - String 컬럼에 length 제한 적용
   - LocalDateTime 컬럼에 @TeMPOraL 적용

* [#6] feat: 예외 메시지 enum 생성 및 적용

   - 기존 string 하드코딩에서 enum으로 변경

* [#6] feat: Spring security 우회 설정

   - 컨트롤러 테스트를 위해 모든 경로 permitAll로 지정하여 우회함.

* [#6] fix: 컨트롤러 URI 변경

   - program -> activity로 명명 변경에서 놓친 부분 수정

* [#6] fix: 활동 저장 오류 수정

   - 서비스의 잘못된 로직 수정
   - JpaEntity에 기본 생성자 추가

* [#6] refact: DTO 변수를 final로 변경

   - 응답 dto의 변수를 final로 변경
   - 저장 요청 dto에서 @DaTa 에서 @Getter로 변경

* [#6] feat: Activity 예외 수정 및 핸들러 추가

   - common.constant.ErrorCode 추가
   - common.exception.GlobalExceptionHandler 추가
   - activity.exception.ActivityCustomException 수정

* [#6] feat: Activity 관련 클래스 변수 추가 및 삭제

   - onlinePossible 변수 삭제
   - actStartTime, actEndTime 변수 추가

* [#6] feat: 활동 제목 변수 추가

   - actTitle 추가

* [#6] test: Activity 단위 테스트 추가

   - 아래 클래스에 대한 단위 테스트를 추가함.
      - JpaEntity
      - Service
      - JpaRepository
* docs: Update issue templates

이슈 템플릿 생성

* docs: Create pull_request_template.md

- PR 템플릿 작성

* init: 프로젝트 생성

[#2] init: 프로젝트 생성 및 의존성 추가 (#3) (#4)

* weekly: 3주차 작업 내용 머지 (#8)

* [#2] init: 프로젝트 생성 및 의존성 추가 (#3)

- 프로젝트 생성
- 의존성 추가
- H2 연결 및 테스트

* [#6] feat: 봉사활동 조회 기능 추가 (#7)

- 아직 모두 구현하지 못했음

* fix: PR 템플릿 위치 수정 (#14)

* [#5] init: PR 템플릿 위치 변경

   - 템플릿 파일의 위치를 변경함

* rename: 기본 테스트 클래스 이름 변경

* feat: 활동 조회 기능 완성 및 단위테스트 추가 (#15)

* [#6] feat: JPA 레포지토리 연결

   - save, findAll, findById 구현

* [#6] feat: 활동 리스트, 상세정보 응답 DTO 구현

* [#6] feat: JPA 엔티티 애노테이션 수정

   - GeneratedValue 삭제 : 받아온 데이터의 id를 그대로 사용하기 위함.
   - String 컬럼에 length 제한 적용
   - LocalDateTime 컬럼에 @TeMPOraL 적용

* [#6] feat: 예외 메시지 enum 생성 및 적용

   - 기존 string 하드코딩에서 enum으로 변경

* [#6] feat: Spring security 우회 설정

   - 컨트롤러 테스트를 위해 모든 경로 permitAll로 지정하여 우회함.

* [#6] fix: 컨트롤러 URI 변경

   - program -> activity로 명명 변경에서 놓친 부분 수정

* [#6] fix: 활동 저장 오류 수정

   - 서비스의 잘못된 로직 수정
   - JpaEntity에 기본 생성자 추가

* [#6] refact: DTO 변수를 final로 변경

   - 응답 dto의 변수를 final로 변경
   - 저장 요청 dto에서 @DaTa 에서 @Getter로 변경

* [#6] feat: Activity 예외 수정 및 핸들러 추가

   - common.constant.ErrorCode 추가
   - common.exception.GlobalExceptionHandler 추가
   - activity.exception.ActivityCustomException 수정

* [#6] feat: Activity 관련 클래스 변수 추가 및 삭제

   - onlinePossible 변수 삭제
   - actStartTime, actEndTime 변수 추가

* [#6] feat: 활동 제목 변수 추가

   - actTitle 추가

* [#6] test: Activity 단위 테스트 추가

   - 아래 클래스에 대한 단위 테스트를 추가함.
      - JpaEntity
      - Service
      - JpaRepository

* feat: 봉사활동 조회 무한스크롤 기능 및 단위테스트 구현 (#20)

* [#18] feat: 활동 조회 id 기준 QueryDSL 무한스크롤 구현

- QueryDSL 의존성 추가 및 common.config.QueryDslConfig 파일 추가
- Slice 객체 반환하도록 변경
- CustomRepository 만드는 패턴을 통해 QueryDSL과 Spring data JPA 결합

* [#18] rename: 활동 테이블 PK명 id에서 actId로 변경

* [#18] test: 무한스크롤 단위 테스트 추가 및 수정

* [#18] feat: 활동 정렬 기준 상수클래스 생성

- 정렬 기준 상수클래스 생성하여 하드코딩 제거
- common.constant를 activity 패키지로 이동

* [#18] fix: ActivityCustomRepositoryImpl에서의 도메인 엔티티 종속성 제거

- Activity로의 종속성을 제거하고 ActivityJpaEntity만 종속하도록 수정함.

* [#18] rename: 바꾸지 않은 findAll 명명을 모두 findSlice로 변경

* [#18] rename: ActivityCustomException을 ActivityException으로 변경

* [#18] refact: OrderSpecifier 정렬 기준 생성 메서드 리팩토링

불필요한 클래스 삭제
- common.utils.QueryDslUtil
- activity.constant.ActivitySortType

* feat: 봉사 분야 기능, 봉사 조회 필터링 기능 추가 (#23)

* [#22] refact: lombok 사용하여 생성자 제거

* [#22] feat: H2콘솔 위한 X-Frame-Options 비활성화

* [#22] feat: 봉사 분야 컬럼 추가

- enum 형태로 관리
- CategoryConverter를 통해 DB에는 한글로 저장.

* [#22] feat: 봉사활동 조회 필터링 기능 추가

- filter DTO를 추가하여 필터링 정보 전달.
- 파라미터 변경에 따른 클래스, 테스트코드 수정
- ActivityFilterBuilder: 필터링 정보 조합 BooleanBuilder 반환

* [#22] fix: 마감 전 활동만 필터링 하도록 조건 수정

- 마감된 활동만 필터링에서 마감 전 활동 필터링으로 변경

* [#22] test: 활동 조회 정렬, 필터링 기능 단위테스트

- 마감 날짜 오름차순 정렬 조회
- 마감된 공고 중, 마감 날짜 가까운 순 정렬
- 카테고리 필터링
- 청소년 가능 활동만 필터링
- 마감되지 않은 활동만 필터링

* [#22] refact: 기존 활동 조회 단위 테스트 리팩토링

* feat: 아바타 관련 기능 구현

* feat: 아바타 관련 기능 구현
    신규 아바타 저장
    기존 아바타 호출
    기존 아바타 수정

* test: 아바타 관련 단위 테스트

* fix: 아바타 기능 수정

---------

Co-authored-by: Awhn <69659322+Awhn@users.noreply.github.com>
* [#21] fix: AvatarJpaRepositoryTest에 @import 추가

* [#21] refact: Activity 도메인 lombok 수정

- AllArgsConstructor 제거

* [#21] feat: 봉사기관 테이블 생성 및 조회

- 봉사활동:봉사기관 = N:1 (ManyToOne)
- ActivityDetail 응답에 기관 정보 포함

* [#21] test: 봉사기관 관련 단위테스트

* [#21] fix: 리뷰 후 1차 수정 사항

- InstituteDetailResponse로 명명 수정
- 위도, 경도 BigDecimal로 변경
* [#30] feat: 시도, 군구 CR API 구현

- 시도, 군구 영속성 엔티티 생성
- create, read API 구현
- 시도 조회, 군구 조회 별도로 생성

* [#30] fix: 변수명, 제약조건 수정

- gunguCode 에서 sidoGunguCode로 변경
- nullable 제약조건 추가

* [#30] feat: 봉사활동과 시도군구 연관관계 매핑

* [#30] feat: 시도, 시군구 필터링 조회 기능 추가

- 봉사활동 조회 시, 시도, 시군구 필터링 기능 추가
- sidoCode, sidoGunguCode로 queryDsl BooleanBuilder 추가.
infra: Github actions를 사용한 CI (#36)
* feat: 유저 기록 기능 구현

* test: 유저 기록 기능 테스트

* fix: activity 관련 의존성 수정
weekly: 8주차 작업 내용 머지
* feat: 유저 기록 기능 구현

* test: 유저 기록 기능 테스트

* fix: activity 관련 의존성 수정

* fix: 요구사항 변경에 따른 선택적 무한 스크롤

* fix: 아바타 역직렬화 과정에서의 오류 수정
* feat: 유저 기록 기능 구현

* test: 유저 기록 기능 테스트

* fix: activity 관련 의존성 수정

* fix: 요구사항 변경에 따른 선택적 무한 스크롤
* [#37] infra: 배포 CI 파이프라인

* [#37] fix: 브랜치 및 오류 수정
* [#19] feat: 유저 카카오 회원가입/로그인 구현

- JWT + Interceptor를 사용하여 구현

* [#19] feat: 유저, 아바타 연동

- userId로 아바타 생성 기능 추가.
- user와 avatar 1:1 연관관계 매핑.

* [#19] fix: 테스트용 로깅 제거

* [#19] feat: 로그인 시, 아바타 유무 응답 반환

- 헤더의 token : JWT
- 바디의 avatarExists: true/false

* [#19] refact: 아바타 URI 변경

- /api/v1/avatar -> /api/v1/avatars

* [#19] refact: kakao-token 바디에서 헤더로 변경

- @RequesetBody -> @RequestHeader 로 수정

* [#19] refact: 유저 도메인과 영속성 엔티티 분리

- User 클래스 생성

* [#19] test: 유저 테스트 추가 및 다른 테스트코드 수정

- 유저 단위테스트 추가
- 다른 테스트 충돌 부분 해결

* [#19] fix: history URI 수정 및 불필요한 test 삭제

- /api/v1/history -> /api/v1/histories
- ApplicationTest.java 삭제

* [#19] feat: User, Avatar, History 연동 및 수정

- INTERMEDIATE 오타 수정
- 인터셉터 인증 경로에 /api/v1/histories 추가
- NoSuchElement, IllegalArgument 예외 핸들러 추가
- History 관련 QueryDsl 수정
- JWT 파싱 후, 아바타 조회한 뒤 해당 아바타ID로 history 조회하도록 변경

* [#19] fix: History 생성 API의 avatarId 제거

- JWT 인증 토큰을 통해 userId를 받으므로 제거함

* [#19] refact: JWT userId 추출 로직 공통화

- common.utils.ExtractUserIdFromJwt.java

* [#19] feat: 아바타 존재시 아바타 정보 반환

- body에 아바타 객체 정보 담아서 반환.
- 없으면 null

* [#19] test: 테스트 코드 수정
- 카테고리 목록 반환 API 생성
- 클라이언트와 주고받는 카테고리명 한글로 수정
weekly: 9주차 작업 중간 병합
weekly: CI 파이프라인 yml파일 수정 후 중간 병합
* [#37] infra: 압축파일 S3 전송하도록 수정

- tar -cvfz ./build.tar.gz 명령어 추가

* [#37] infra: CD 파이프라인 추가, CI수정

- prod-cd.yml 추가
- codedeploy 위한 appspec.yml 추가
- 배포 script 추가
- prod-ci.yml 수정
weekly: CD 동작 확인용 9주차 중간 병합
weekly: S3 업로드 방식 cp로 변경
deploy: 9주차 CI/CD 테스트용 중간 배포
* [#59] docs: gitignore에 민감 파일 추가

* [#59] infra: 설정 파일 변경 및 배포 스크립트 수정

* [#59] fix: 기본 실행 환경 dev로 수정
- 프로덕션 환경에서 테스트
- AWS RDS 연동 테스트
* feat: 활동 참여 정보 동기화를 위한 기초 코드

* feat: 데이터 업데이트 서비스 및 유틸리티 구현

* fix: json으 로수

* fix: 테스트 코드를 위한 수정

* test: 주소와 활동 데이터 업데이트 테스트 코드(외부 API 사용)

* test: 주소와 활동 데이터 업데이트 테스트 코드(외부 API 사용)
weekly: 9주차 작업 내용 최종 병합
5win and others added 7 commits November 1, 2024 23:38
* [#55] feat: 질문 리스트 반환 API 구현

* [#55] rename: Review에서 Question으로 명명 변경

* [#55] feat: 리뷰 기능 구현 중간 커밋
weekly: 10주차 작업 내용 머지
@5win 5win added the review 리뷰 요청 PR label Nov 8, 2024
@5win 5win requested a review from syndersonlee November 8, 2024 14:25
@5win 5win self-assigned this Nov 8, 2024
Copy link

@syndersonlee syndersonlee left a comment

Choose a reason for hiding this comment

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

고생하셨습니다 테스트 코드는 사실 무의미 한 형태로 짜여져 있는 것 같아요. 그 점은 나중에 알려드릴 기회가 있으면 좋겠네요.

})
.toList();

answers.forEach(answerRepository::save);

Choose a reason for hiding this comment

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

p4; saveAll을 사용하시는 게 나을 거 같아요

@@ -17,21 +23,29 @@ public class HitstoryServiceTest {
.actId(1L)
.build();

ReviewService reviewService = new ReviewService(

Choose a reason for hiding this comment

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

p4; stub으로 생성하는 것 보다는 mock 객체를 사용하셔서 쓰는게 좋을 거 같아요

Suggested change
ReviewService reviewService = new ReviewService(
@Mock
ExistUserRepository mockExistUserRepository;
@Mock
QuestionRepository questionRepository;
@InjectMock
ReviewService reviewService;

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

public class StubReviewRepository implements ReviewRepository {

Choose a reason for hiding this comment

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

p5; 테스트 코드에서 implement를 할 필요성이 잘 안보여요

OptionalDouble averageScore = reviewRepository.findReviews(instituteId, questionId)
.stream()
.flatMap(review -> review.getAnswers().stream())
.filter(answer -> answer.getQuestion().getQuestionId() == questionId)

Choose a reason for hiding this comment

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

p5; 해당 필터 조건은는 answer 내부에서 메소드 선언해도 좋을거 같네요


activityRepository.save(saveRequest.toModel(institute, sidoGungu));
}

public Slice<ActivityFindSliceResponse> findSlice(ActivityFilterRequest request,
Pageable pageable) {
Pageable pageable) {
Slice<Activity> activities = activityRepository.findSlice(request, pageable);
return activities.map(ActivityFindSliceResponse::from);
}

public ActivityDetailResponse findById(Long activityId) {

Choose a reason for hiding this comment

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

p5; 이 부분은 리팩토링 해보려고 했는데, entity depth가 높아서 어렵겠네요. 메소드 네임을 역할에 맞게 수정해주시면 좋을 거 같아요

Activity activity = activityRepository.findById(saveRequest.getActId())
.orElseThrow(NoSuchElementException::new);
.orElseThrow(NoSuchElementException::new);
History history = saveRequest.toModel(avatar, activity);
historyRepository.save(history);
}

public Slice<HistoryFindSliceResponse> findSliceByAvatarId(Long userId, Pageable pageable) {

Choose a reason for hiding this comment

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

p5; 이 부분은 리팩토링 해보려고 했는데, entity depth가 높아서 어렵겠네요. 메소드 네임을 역할에 맞게 수정해주시면 좋을 거 같아요



Map<QuestionResponse, BigDecimal> scores = new HashMap<>();
long instituteId = activity.getInstitute().getInstituteId();

Choose a reason for hiding this comment

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

p5; Input이 대부분 Wrapper class인데 여기만 primitive 인데 통일하면 좋겠어요

@syndersonlee
Copy link

내일 아침에 기상 후 코멘트 추가로 없으면 바로 머지할게요

@syndersonlee syndersonlee merged commit d6ad7e9 into review Nov 10, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review 리뷰 요청 PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants