[Team#04][BE] 3주차 PR입니다 #45

JJONSOO · 2023-08-11T02:35:36Z

What is this PR? 👓

3주차 진행상황에 대한 PR입니다.

레이블, 마일스톤 CRUD
Github Action, Docker를 통한 CI/CD
Validator 추가
Client의 이미지를 받아 AWS S3에 이미지 업로드 후 경로 반환
AOP를 활용한 History 구현
로그아웃
테스트 코드

…olor는 직접 찾도록 수정한다.

…undColor는 직접 찾도록 수정한다.

…factoring [BE] 요구사항에 따른 코드 래픽토링

[BE] 레이블 CRUD API 개발

[BE] CORS설정

…lestone [BE] 마일스톤 CRUD API 개발

…gout [BE] 로그아웃 구현

…/issue-tracker-max into test/#31-issuepage-test

…lidator [BE] Validation 추가

…inpage-modify [BE] 메인페이지 및 마일스톤 API 수정

…inpage-modify [BE] 이슈, 마일스톤 title API 수정

…bel-milestone-duplicate-check [BE] label, milestone 이름 중복

kses1010

고생하셨습니다. 이번 PR에서 느낀점은 코드들이 대부분 깔끔하게 작성되었다는걸 느꼈습니다.

다만 validator를 만드셔서 적용하고 있는데 걱정스러운 점은 서비스 코드에 validator코드가 잔뜩 늘어나면 어떡하면 좋을까 생각했습니다. 이땐 어떻게 처리하면 좋을지 고민하시는걸 추천드립니다.

kses1010 · 2023-08-12T10:06:32Z

be/src/main/java/com/issuetrackermax/config/HistoryAspect.java

+	@Around("execution(* com.issuetrackermax.service.issue.IssueService.post(..)) && args(request,writerId)")
+	public Object save(ProceedingJoinPoint joinPoint, IssuePostRequest request, Long writerId) throws Throwable {
+		IssuePostResponse issuePostResponse = (IssuePostResponse)joinPoint.proceed();
+		Long issueId = issuePostResponse.getId();
+
+		historyService.save(HistoryRequest.builder()
+			.issueId(issueId)
+			.editor(memberRepository.findById(writerId).get().getNickName())
+			.issueIsOpen(true)
+			.build());
+		return issuePostResponse;
+	}
+
+	@Around("execution(* com.issuetrackermax.service.issue.IssueService.updateStatus(..)) && args(request, memberId)")


aspect를 직접 사용하셧군요 changeStatus에는 왜 사용하셨나요?

이슈 상태를 바꾸는 것도 History에 저장하기 위해 사용하였습니다.
ex) A에 의해 5분전에 변경되었습니다.

kses1010 · 2023-08-12T10:08:39Z

be/src/main/java/com/issuetrackermax/controller/filter/dto/response/AssigneeResponse.java

@@ -21,7 +22,7 @@ public AssigneeResponse(Long id, String name) {

 	public static List<AssigneeResponse> convertToAssigneeResponseList(String assigneeIds, String assigneeNames) {
 		if (assigneeIds == null) {
-			return null;
+			return new ArrayList<>();


null에서 빈리스트를 돌려주기로 했군요

넵 null로 반환하는게 어색하다고 생각하여 빈 리스트를 반환하도록 코드를 수정하였습니다.

kses1010 · 2023-08-12T10:09:12Z

be/src/main/java/com/issuetrackermax/controller/filter/dto/response/LabelResponse.java

+	public static LabelResponse from(Label label) {
+		return LabelResponse.builder()
+			.id(label.getId())
+			.title(label.getTitle())
+			.textColor(label.getTextColor())
+			.backgroundColor(label.getBackgroundColor())
+			.build();


전체적으로 dto쪽이 깔끔해지고 있네요

kses1010 · 2023-08-12T10:10:24Z

be/src/main/java/com/issuetrackermax/controller/issue/IssueController.java

+	public ApiResponse<Void> updateStatus(@RequestBody IssuesStatusRequest request, HttpServletRequest servletRequest) {
+		Integer memberId = (Integer)servletRequest.getAttribute(MEMBER_ID);
+		issueService.updateStatus(request, memberId.longValue());
 		return ApiResponse.success();


질문있습니다. memberId가 Long으로 캐시팅되는것 같은데 나중에 Long으로 바꾸거나 할 생각이 있으신가요?

추가적으로 IssueStatusRequest도 나중에 validation이 들어가나요?

JwtService에서 28line
Jwt jwt = jwtProvider.createJwt(generateMemberClaims(member.getId()));
Member Long id를 저장하지만 Filter를 통과할 때 claim.get(MEMBER_ID)를 할 떄, Interger로 변환되는 것 같습니다.
HttpServletRequest.setAttribute 할 때, Long 타입으로 변환하여 저장하도록 코드를 수정하겠습니다.

IssueStatusRequest의 List<Long> ids에 @NotNull을 붙였어야 했는데 누락됐습니다. 다음번에 수정하겠습니다. String issueStatus는 서비스단에서 if문으로 처리하여 따로 validation은 추가하지 않으려고 합니다.

kses1010 · 2023-08-12T10:12:59Z

be/src/main/java/com/issuetrackermax/controller/milestone/MilestoneController.java

+		Long labelSize = (long)labelService.getLabelList().size();
+		List<MilestoneDetailResponse> milestones = milestoneService.getOpenMilestone();
+		Long closedMilestoneCount = (long)milestoneService.getClosedMilestone().size();
+


이렇게 라벨 사이즈, 닫은 마일스톤 사이즈를 체크해서 가져오는 코드인데 문제는 만약 라벨이 많아지고 닫은 마일스톤도 많아지면 계속해서 size()를 통해서 가져오나요?
따로 count를 해서 바로 알수 있는 방법은 없을까요?

객체를 객체 내부에서 생성하도록 하면 컨트롤러단에서 호출만 하면 되겠죠?

넵 Count만 가져오도록 수정하겠습니다.

kses1010 · 2023-08-12T10:28:18Z

be/src/main/java/com/issuetrackermax/service/issue/IssueService.java

+		if (request.getIds() == null) {
+			return;


요건 request 내부에서 validation해야하는데 서비스까지 내려온 것 같네요

request내부에서 validation하도록 수정하겠습니다.

kses1010 · 2023-08-12T10:28:41Z

be/src/main/java/com/issuetrackermax/service/issue/IssueService.java

+		if (request.getIds() == null) {
+			return;


여기도 마찬가지

request내부에서 validation하도록 수정하겠습니다.

kses1010 · 2023-08-12T10:31:59Z

be/src/test/java/com/issuetrackermax/controller/ControllerTestSupport.java

+
+	@MockBean
+	protected MilestoneService milestoneService;
+


계속해서 abstract class에 새로은 서비스가 추가되는 군요
차라리 도메인별로 만드는건 어떤가요? 이름이 ControllerTestSupport 이다 보니 나중에 새로운 컨트롤러, 새로운 도메인이 생길때마다 계속해서 MockBean을 추가해야하는 번거로움이 보입니당

도메인 별로 만들면 Spring Boot가 실행되는 횟수가 많아져서
Spring Boot가 뜨는 횟수를 줄이기 위해 하나의 abstract class에서 처리하도록 코드를 작성하였습니다.
또한 현재 하나의 도메인에 하나의 Controller만 있는 상태라 도메인 별로 Controller를 만들면 abstract class가 의미가 떨어지는 거 같아서, 하나의 ControllerTestSupport를 만들어서 사용하고 있는데,
추후에 하나의 도메인에 여러 Controller가 생긴다면 도메인으로 묶어 abstract class를 만들어보겠습니다.

kses1010 · 2023-08-12T10:33:09Z

be/src/test/java/com/issuetrackermax/domain/issue/IssueRepositoryTest.java

+		Boolean isOpen = true;
+		String title = "새로운 이슈";
+		Long milestoneId = 1L;
+		Long writerId = 1L;
+		issue = makeIssue(isOpen, title, milestoneId, writerId);
+		issueId = issueRepository.save(issue);


새로운 객체 만들다가 필드를 새로 지정한 이유가 무엇인가요? 이렇게 설정하면 어떤 이점이 있던가요?

setup() 메서드 생성 전에 사용했던 필드를 그대로 남긴 건데 사용할 필요가 없어 보입니다. 필드는 지우고 setup() 메서드 안에서 객체를 만들도록 변경하겠습니다.

kses1010 · 2023-08-12T10:34:00Z

be/src/test/java/com/issuetrackermax/service/milestone/MilestoneServiceTest.java

+	@Test
+	void getClosedMilestone() {
+	}


지난번에도 말씀드렸지만 안에 아무것도 없는 테스트는 삭제해주세요 필요할 때마다 추가하시는걸 추천합니다.

넵 삭제하겠습니다.

Dev be/feature/#15

kses1010

고생하셨습니다. PR Merge하겠습니다.

JJONSOO and others added 30 commits August 4, 2023 19:13

feat #37 : issue TextColor, BackgroundColor 추가

83db405

fix #37 : 에러 메세지를 ErrorResponse가 가지도록 수정

69575b7

fix #37 : FilterMapper에서 Label ID만 가져오고 title, textColor, backgroundC…

627b4e4

…olor는 직접 찾도록 수정한다.

fix #37 : LabelRepository 에서 Label ID만 가져오고 title, textColor, backgro…

fc42634

…undColor는 직접 찾도록 수정한다.

fix #37 : null이 아닌 빈 List가 반환되도록 한다.

feae50c

fix #37 : 코드 리뷰 반영

4988ab0

Merge pull request #42 from codesquad-team4-issue-tracker/feat/#37-re…

f9ceea5

…factoring [BE] 요구사항에 따른 코드 래픽토링

fix #43 : Label Test 코드 작성

93b182b

fix #43 : Label CRUD 코드 작성

2186a57

fix #43 : milestone 개수 가져오기

4236397

fix #43 : 비밀번호 길이 두 번 검증은 취소한다.

2dbbd12

Merge pull request #44 from codesquad-team4-issue-tracker/feat/#43-label

d654d33

[BE] 레이블 CRUD API 개발

chore #46: Dockerfile-dev생성

7066c9e

chore #46: cors설정

de57a53

Merge pull request #48 from codesquad-team4-issue-tracker/chore/#46-cors

20ba97b

[BE] CORS설정

feat #45 : nginx 경로 설정을 위해 모든 Path앞에 /api 추가

4b37305

feat #45 : milestone 테스트 작성

efc9d66

feat #45 : milestone API

74de407

Merge pull request #51 from codesquad-team4-issue-tracker/feat/#45-mi…

20d7820

…lestone [BE] 마일스톤 CRUD API 개발

test #31: repository 메서드 일부 테스트코드 추가

5caf076

fix #54 : url method path 수정

1624345

feat #54 : 로그아웃 API

5be72c9

feat #54 : 로그아웃 API

096f3b9

fix #54 : @transactional 추가

624ff3b

refactor #31: 메서드 분리

7c8f326

Merge pull request #56 from codesquad-team4-issue-tracker/faet/#54-lo…

76bb90a

…gout [BE] 로그아웃 구현

Merge branch 'be' of https://github.com/codesquad-team4-issue-tracker…

493bcbb

…/issue-tracker-max into test/#31-issuepage-test

refactor #31: service class method 호출하도록 변경

caa2dd3

refactor #31: 오타 수정

df301d8

refactor #31: 오타 수정

56751a0

JJONSOO and others added 14 commits August 9, 2023 18:28

feat #61 : member validator 추가

1872ede

feat #61 : label validator 추가

bc7cc96

feat #61 : Jwt validator 추가

64d43c7

feat #61 : History validator 추가

fb0048b

feat #61 : Assignee validator 추가

59cb563

feat #61 : Filter validator 추가

1f6bd77

feat #61 : Comment validator 추가

81f0f28

Merge pull request #62 from codesquad-team4-issue-tracker/feat/#61-va…

b207575

…lidator [BE] Validation 추가

feat : cors preflight가 jwt필터를 타지 않도록 수정

54be6bc

feat #69 : mileStoneCount -> milestoneCount로 네이밍 수정

52fe03f

feat #69 : milestone open/closed 을 opposite로 통일

181ea54

Merge pull request #71 from codesquad-team4-issue-tracker/feat/#69-ma…

ebd4d37

…inpage-modify [BE] 메인페이지 및 마일스톤 API 수정

feat #69 : issue, milestone name을 title로 수정

af41b45

Merge pull request #73 from codesquad-team4-issue-tracker/feat/#69-ma…

311feff

…inpage-modify [BE] 이슈, 마일스톤 title API 수정

JJONSOO requested a review from kses1010 August 11, 2023 02:36

JJONSOO assigned JJONSOO, yhpark95 and Ojeegu Aug 11, 2023

JJONSOO added the review-be label Aug 11, 2023

JJONSOO and others added 7 commits August 11, 2023 12:09

feat #78 : issue 저장시 name을 title로 수정

0e0e2d6

feat #78 : 마일스톤 저장시 name을 title로 수정

13edeb9

feat #78 : 같은 이름의 Issue 존재시 예외를 발생시킨다.

699f125

feat #78 : 같은 이름의 Milestone 존재시 예외를 발생시킨다.

56d2d5f

feat #78 : 같은 이름의 Issue 존재시 예외를 발생시키는 테스트.

7b7e565

feat #78 : 같은 이름의 Milestone 존재시 예외를 발생시키는 테스.

c802731

Merge pull request #79 from codesquad-team4-issue-tracker/feat/#78-la…

ca2a15a

…bel-milestone-duplicate-check [BE] label, milestone 이름 중복

kses1010 reviewed Aug 12, 2023

View reviewed changes

kses1010 pushed a commit that referenced this pull request Aug 13, 2023

Merge pull request #45 from codesquad-issue-team-05/dev-be/feature/#15

6c7780a

Dev be/feature/#15

kses1010 approved these changes Aug 13, 2023

View reviewed changes

kses1010 merged commit 43c63a5 into codesquad-members-2023:team-04 Aug 13, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Team#04][BE] 3주차 PR입니다 #45

[Team#04][BE] 3주차 PR입니다 #45

JJONSOO commented Aug 11, 2023

kses1010 left a comment

kses1010 Aug 12, 2023

JJONSOO Aug 12, 2023

kses1010 Aug 12, 2023

JJONSOO Aug 12, 2023

kses1010 Aug 12, 2023

kses1010 Aug 12, 2023

kses1010 Aug 12, 2023

JJONSOO Aug 12, 2023

Ojeegu Aug 12, 2023 •

edited

Loading

kses1010 Aug 12, 2023

kses1010 Aug 12, 2023

JJONSOO Aug 12, 2023

kses1010 Aug 12, 2023

JJONSOO Aug 12, 2023

kses1010 Aug 12, 2023

JJONSOO Aug 12, 2023

kses1010 Aug 12, 2023

JJONSOO Aug 12, 2023

kses1010 Aug 12, 2023

Ojeegu Aug 12, 2023

kses1010 Aug 12, 2023

JJONSOO Aug 12, 2023

kses1010 left a comment

[Team#04][BE] 3주차 PR입니다 #45

[Team#04][BE] 3주차 PR입니다 #45

Conversation

JJONSOO commented Aug 11, 2023

What is this PR? 👓

kses1010 left a 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

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

Ojeegu Aug 12, 2023 • 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kses1010 left a comment

Choose a reason for hiding this comment

Ojeegu Aug 12, 2023 •

edited

Loading