[Team-21][BE][루이1세] 2주차 세번째 PR #215

rkolx · 2022-06-27T07:43:15Z

인사말

안녕하세요 시그리드! 루이 & 한세입니다 😄
이번에는 본격적으로 기능 구현을 들어갔습니다!

진행 사항

로그인 기능 및 jwt 토큰 생성
BE/FE 배포를 위한 workflows 생성
이슈등록 기능 구현
파일 업로드 기능 구현

API 명세서

업데이트 된 API 명세서

[BE] 백엔드 배포

- 입력값 유효한 경우 커멘트, 매니저, 라벨, 마일스톤 저장 - 작성자 추후 추가 예정

- createDateTime, updateDateTime, status

[BE] 이슈 등록 기능 구현

- 연관관계 편의 메서드 및 샘플 회원 데이터 추가

feat : Issue 등록 기능 사용자 추가

[BE] 파일 업로드 기능 구현

sigridjineth

안녕하세요, Sigrid Jin입니다.
몇 가지 리뷰한 사항을 남겨드렸으니 확인해주시면 감사하겠습니다. 마지막까지 화이팅입니다 💯

sigridjineth · 2022-06-28T15:25:16Z

BE/Dockerfile

+# 변수에 저장된 것을 컨테이너 실행시 이름을 app.jar파일로 변경하여 컨테이너에 저장
+COPY ${JAR_FILE} app.jar
+
+# 빌드된 이미지가 run 될 때 실행할 명령어


정리가 꼼꼼해서 좋네요.

sigridjineth · 2022-06-28T15:41:46Z

BE/src/main/java/louie/hanse/issuetracker/domain/Comment.java

@@ -11,14 +14,12 @@

 @Entity
 @Table(name = "comments")
+@NoArgsConstructor(access = AccessLevel.PROTECTED)


막아두신 것 좋네요.

sigridjineth · 2022-06-28T15:44:49Z

BE/src/main/java/louie/hanse/issuetracker/domain/Issue.java

@@ -33,9 +37,30 @@ public class Issue {
    private List<IssueManager> issueManagers = new ArrayList<>();

    private String title;
-    private LocalDateTime createDateTime;
+    private LocalDateTime createDateTime = LocalDateTime.now();


nit
다른 팀에서 하셨던 이전 프로젝트 리뷰에서 다음의 링크에 나와있는 질문을 드린 적이 있습니다. 한 번 살펴보세요.
참고 할 만한 링크

sigridjineth · 2022-06-28T15:46:03Z

BE/src/main/java/louie/hanse/issuetracker/domain/IssueLabel.java

+        this.issue = issue;
+        issue.addIssueLabel(this);
+        this.label = label;
+        label.addIssueLabel(this);


현재 하신 것이 딱히 나쁘지 않은 접근이긴 합니다만, issue 와 label 이 서로를 양방향으로 꼭 알아야 하나요? 한 쪽만 알게 구현하고, 다른 쪽에서 불러올 때는 DB에서 쿼리를 날려서 불러오도록 하면 어떨까요?

sigridjineth · 2022-06-28T15:48:20Z

BE/src/main/java/louie/hanse/issuetracker/domain/IssueManager.java

@@ -16,5 +20,11 @@ public class IssueManager {

    @ManyToOne
    @JoinColumn
-    private Member manger;
+    private Member manager;


@Inheritance 라는 어노테이션에 대해서도 한 번 알아보세요. Manager를 상속하는 클래스가 되는 것이 자연스러운 접근인 것으로 보입니다.

참고할 만한 링크

sigridjineth · 2022-06-28T15:50:21Z

BE/src/main/java/louie/hanse/issuetracker/service/IssueService.java

+    private final MemberRepository memberRepository;
+    private final LabelRepository labelRepository;
+    private final IssueLabelRepository issueLabelRepository;
+    private final MilestoneRepository milestoneRepository;


어우, 너무 많은 Repository를 상속하고 있네요. 이 쯤 되면 DDD에 대해서도 고민할 법 합니다. 물론 저도 잘 모르지만요. 큰 도메인 안에서 서로 연관된 클래스를 서로 엮어 각각의 entrypoint를 만들고, 각각의 도메인끼리 관계를 맺도록 구현하는 것이 DDD입니다. 지금은 하나의 도메인(Issue)가 지나치게 비대해 여러 가지 다양한 도메인을 다루는 여러 Repository를 모두 사용하고 있지요. 너무 결합도가 높은 설계입니다.

sigridjineth · 2022-06-28T15:55:12Z

BE/src/main/java/louie/hanse/issuetracker/service/IssueService.java

+    private final MilestoneRepository milestoneRepository;
+
+    @Transactional
+    public void register(IssueSaveRequest issueSaveRequest, Long memberId) {


음.. 이 코드를 보고 무슨 생각이 드시나요? 지금 이 register 가 여러 가지 책임을 모두 수행하고 있어, 이 코드를 리팩토링해야 할 일이 있으면 아주 건들기 어렵지 않을까요?
도메인 모델이 비즈니스 객체를 들고 있도록 구현해야 합니다. 그 이유가 무엇일 지 아래 링크를 보고 자신만의 의견을 작성해주세요.

우리가 도메인이 비즈니스 로직의 주도권을 가지고 개발하는 것을 도메인 주도 설계(DDD) 라 합니다. 이렇게 해두면 서비스의 많은 로직이 엔티티로 이동하고, 서비스는 엔티티를 호출하는 정도의 얇은 비즈니스 로직을 가지게 되는데, 이렇게 구현하면 information expert pattern 를 지키면서 구현할 수 있습니다.

시간이 되실 때 클린코드 책도 한 번 읽어보세요.

참고할 만한 링크

sigridjineth · 2022-06-28T15:57:12Z

BE/src/main/java/louie/hanse/issuetracker/service/S3Service.java

+
+    private final AmazonS3Client amazonS3Client;
+
+    @Value("${cloud.aws.s3.bucket}")


@Value 의 사용이 좋습니다.

sigridjineth · 2022-06-28T15:59:53Z

BE/src/main/java/louie/hanse/issuetracker/web/controller/FileUploadController.java

+import org.springframework.web.multipart.MultipartFile;
+
+@RequiredArgsConstructor
+@RequestMapping("/upload-files")


파일 업로드라면 구체적으로 어떤 파일을 업로드할 지 리소스 중심으로 작명해주시면 좋겠습니다.
다음 링크를 참고해주세요.

참고할 만한 링크
참고할 만한 링크 2
참고할 만한 링크 3

sigridjineth · 2022-06-28T16:02:14Z

BE/src/main/java/louie/hanse/issuetracker/service/IssueService.java

+            Comment comment = new Comment(issue, issueSaveRequest.getContents());
+            commentRepository.save(comment);
+        }
+        if (!issueSaveRequest.getManagerIds().isEmpty()) {


if 문이 너무 많아지고 있는데요. 문제점이 있을까요? 만약 문제가 있다면, 어떻게 개선해야 할까요?

참고할 만한 링크
참고할 만한 링크 2

- Querydsl을 통해 이슈 오픈 및 클로즈 개수 구하기 - 입력한 필터에 따라 이슈리스트 Response

[BE] 필터검색 기능 구현

refactor: DTO 패키지 분리

Revert "refactor: DTO 패키지 분리"

refactor : DTO 패키지 구조변경

- findByIdOrThrow 메서드로 분리

[BE] 이슈 제목 편집 및 삭제 기능 구현

- 해당 어노테이션 사용시 LocalDateTime 반환시 배열로 출력되므로 제거

[BE]이슈 상태 변경

[BE] 코멘트 작성 기능 구현

[BE] 코멘트 편집 기능 구현

- 로그아웃 요청시 액세스토큰 재발급과 동일하게 검증

feat: 로그아웃 기능 구현

sigridjineth · 2022-07-07T16:01:40Z

BE/src/main/java/louie/hanse/issuetracker/domain/Member.java

@@ -30,4 +30,9 @@ public Member(String socialId, String avatarImageUrl) {
    public void changeRefreshToken(String refreshToken) {
        this.refreshToken = refreshToken;
    }
+
+    public void deleteRefreshToken() {
+        this.refreshToken = null;


null 값을 직접 넣기 보다는 다른 방법이 있지 않을까요?
저라면 Null Object Pattern을 적용하든, 차라리 빈 문자열을 넣겠어요.
참고할 만한 링크

sigridjineth · 2022-07-07T16:02:07Z

BE/src/main/java/louie/hanse/issuetracker/service/MemberService.java

@@ -42,4 +42,10 @@ private Member findByIdOrThrow(Long id) {
                    throw new MemberNotFoundException("해당 id를 가진 회원이 존재하지 않습니다.");
                });
    }
+
+    @Transactional
+    public void deleteRefreshToken(Long id) {


아무리 delete 라 하더라도 기본적인 응답값을 주는 것이 좋습니다.
여기서는 Member 를 리턴할 수 있겠네요.

- 요청 값 검증 로직 추가

[BE] 라벨 관련 기능 구현

[BE] 마일스톤 기능 구현

Louie-03 and others added 13 commits June 23, 2022 19:54

build: Backend workflow 작성

24b65be

Merge pull request #40 from Louie-03/BE-deploy

bada3f9

[BE] 백엔드 배포

refactor: 패키지 변경

f7879f4

chore: client_id 환경변수 처리

093cd55

feat: 이슈 등록 기능 구현

78aba46

- 입력값 유효한 경우 커멘트, 매니저, 라벨, 마일스톤 저장 - 작성자 추후 추가 예정

refactor: 생성자가 아닌 필드에서 값을 초기화하도록 변경

3b0cdb0

- createDateTime, updateDateTime, status

Merge pull request #47 from Louie-03/BE-feature-46

d213622

[BE] 이슈 등록 기능 구현

feat : Issue 등록 기능 사용자 추가

cd5ca0b

- 연관관계 편의 메서드 및 샘플 회원 데이터 추가

Merge pull request #49 from Louie-03/BE-feature-48

3bee614

feat : Issue 등록 기능 사용자 추가

refactor : UploadFile과 Comment 엔티티의 연관관계 제거

714de81

feat : S3 관련 환경 설정 추가

9697a76

feat : 파일 업로드 기능 구현

42a5cc0

Merge pull request #50 from Louie-03/BE-feature-42

8c3470e

[BE] 파일 업로드 기능 구현

rkolx added the review-BE Improvements or additions to documentation label Jun 27, 2022

rkolx requested a review from sigridjineth June 27, 2022 07:43

rkolx assigned rkolx and Louie-03 Jun 27, 2022

Louie-03 added 4 commits June 27, 2022 16:55

init : Querydsl 환경 설정

6b49372

feat : 로그인 성공 시 avatarImageUrl Response 기능 추가

7112736

fix : CORS 문제 설정 추가

c5000f3

Update gradle.yml

944ea80

sigridjineth requested changes Jun 28, 2022

View reviewed changes

rkolx added 3 commits June 29, 2022 11:03

feat: 필터 검색 동적 쿼리 구현

19de7ac

refactor: 엔티티 @gatter 추가

1053173

feat : 이슈 리스트 구현 1단계

0a940f0

rkolx force-pushed the dev-BE branch from 8c3470e to 24b65be Compare June 29, 2022 02:07

rkolx added 4 commits June 29, 2022 11:20

refactor: Label 엔티티에 IssueLabel 연관관계 제거

3c48b39

feat : 이슈 리스트 구현

e20c259

- Querydsl을 통해 이슈 오픈 및 클로즈 개수 구하기 - 입력한 필터에 따라 이슈리스트 Response

Merge pull request #55 from Louie-03/BE-feature-51

70f68f2

[BE] 필터검색 기능 구현

refactor : Getter 생성 및 Response 이름 변경

5706e03

rkolx and others added 18 commits July 5, 2022 10:50

Merge pull request #61 from Louie-03/BE-feature-59

4a31a0c

refactor: DTO 패키지 분리

Revert "refactor: DTO 패키지 분리"

6d474a0

Merge pull request #62 from Louie-03/revert-61-BE-feature-59

eb2cb3c

Revert "refactor: DTO 패키지 분리"

refactor : DTO 패키지 구조변경

eb9a582

Merge pull request #63 from Louie-03/BE-feature-59

ac9b616

refactor : DTO 패키지 구조변경

refactor: 패키지 구조 변경

337405c

feat: 이슈 제목 편집 및 삭제 구현

1a1e782

- findByIdOrThrow 메서드로 분리

Merge pull request #65 from Louie-03/BE-feature-64

cbdf32b

[BE] 이슈 제목 편집 및 삭제 기능 구현

feat: 이슈 상태 변경 기능 구현

af67f7e

refactor: controller에서 enum을 바로 받을 수 있게 구현 및 Querydsl 상태개수 중복 수정

39b34bb

refactor: EnableWebMvc 어노테이션 제거

3d4a817

- 해당 어노테이션 사용시 LocalDateTime 반환시 배열로 출력되므로 제거

Merge pull request #67 from Louie-03/BE-feature-66

38c6449

[BE]이슈 상태 변경

feat : 코멘트 작성 기능 구현

21a14ed

Merge pull request #69 from Louie-03/BE-feature-68

912a8e9

[BE] 코멘트 작성 기능 구현

feat : 코멘트 편집 기능 구현

7c0fed5

Merge pull request #71 from Louie-03/BE-feature-70

8562cef

[BE] 코멘트 편집 기능 구현

feat: 로그아웃 기능 구현

0dce6c1

- 로그아웃 요청시 액세스토큰 재발급과 동일하게 검증

Merge pull request #73 from Louie-03/BE-feature-72

fd69187

feat: 로그아웃 기능 구현

sigridjineth reviewed Jul 7, 2022

View reviewed changes

Louie-03 and others added 10 commits July 11, 2022 13:51

feat : 라벨 등록 기능 구현

b196c6c

- 요청 값 검증 로직 추가

feat : 라벨 조회 기능 구현

f488fe5

feat : 라벨 수정 기능 구현

5e2bd12

feat : 라벨 삭제 기능 구현

2ea43ec

Merge pull request #76 from Louie-03/BE-feature-75

0e80f58

[BE] 라벨 관련 기능 구현

feat: 마일스톤 등록기능 구현

66ecb64

feat: 마일스톤 조회기능 구현

84b852b

feat: 마일스톤 수정기능 구현

343df5b

feat: 마일스톤 삭제기능 구현

6f15c5d

Merge pull request #77 from Louie-03/BE-feature-74

b5ef834

[BE] 마일스톤 기능 구현

Louie-03 force-pushed the dev-BE branch from 9e59efa to b5ef834 Compare July 23, 2022 06:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Team-21][BE][루이1세] 2주차 세번째 PR #215

[Team-21][BE][루이1세] 2주차 세번째 PR #215

rkolx commented Jun 27, 2022

sigridjineth left a comment •

edited

Loading

sigridjineth Jun 28, 2022

sigridjineth Jun 28, 2022

sigridjineth Jun 28, 2022 •

edited

Loading

sigridjineth Jun 28, 2022

sigridjineth Jun 28, 2022

sigridjineth Jun 28, 2022

sigridjineth Jun 28, 2022 •

edited

Loading

sigridjineth Jun 28, 2022

sigridjineth Jun 28, 2022

sigridjineth Jun 28, 2022

sigridjineth Jul 7, 2022

sigridjineth Jul 7, 2022


		private final AmazonS3Client amazonS3Client;

		@Value("${cloud.aws.s3.bucket}")

[Team-21][BE][루이1세] 2주차 세번째 PR #215

Are you sure you want to change the base?

[Team-21][BE][루이1세] 2주차 세번째 PR #215

Conversation

rkolx commented Jun 27, 2022

인사말

진행 사항

API 명세서

sigridjineth left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sigridjineth Jun 28, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sigridjineth Jun 28, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sigridjineth left a comment •

edited

Loading

sigridjineth Jun 28, 2022 •

edited

Loading

sigridjineth Jun 28, 2022 •

edited

Loading