02-20 17:08 브랜치 최신화

[Eseas]

바로 MERGE하지 마세요! 검토 후 MERGE해야합니다!

✅ 최근 작업 주제 (하나 이상의 주제를 선택해주세요.)

• 기능 추가
• 리뷰 반영 수정
• 리팩토링
• 버그 수정
• 컨벤션 수정
• 브랜치 최신화

🏆 구현 목표

ex)
bucket post 및 bucket list view 초기 구현

📋 구현 사항 설명 (작업한 내용을 상세하게 기록합니다.)

ex)

1. bucket post 기본 기능 구현
2. bucket list view 기본 기능 구현

🔍 테스트 방법 (변경 사항을 확인하기 위한 테스트 방법을 기술합니다.)

ex)

1. npm test 실행
2. Postman으로 API 호출 테스트

🛠️ 쓰이는 모듈

ex)

• corsheaders
• jwt

💬 기타 질문 및 특이 사항

ex) 백프 배포 nginx 검토

[github-actions]

Code review by ChatGPT

[github-actions]

코드 패치에 대한 간략한 리뷰를 하겠습니다.

변경 사항 요약

1. 여러 DTO import를 간소화하기 위해 *를 사용하여 import했습니다.
2. getAdminDashboard 메서드를 추가하여 관리 대시보드와 관련된 프로젝트 정보 목록을 반환합니다.

개선 제안 및 버그 리스크

1. Import 문 정리: 와일드카드(*) 를 사용하여 import하는 방식은 코드의 가독성을 떨어뜨리고, import되는 클래스의 수를 파악하기 어렵게 만듭니다. 가능하면 필요한 클래스를 명시적으로 import하는 것이 좋습니다.

2. 트랜잭셔널 어노테이션: @Transactional(readOnly = true)를 사용하여 읽기 전용 트랜잭션을 선언하는 것은 좋은 접근법입니다. 하지만, 이 메서드 내에서 DB에 변경을 가하는 코드가 추가될 경우 문제가 발생할 수 있습니다. 따라서 이 메서드는 읽기 전용 용도로 사용되는 것이 바람직합니다.

3. 스트림 사용: processingProjects.stream().map(...).toList();의 사용은 Java 16 이상에서 동작합니다. 사용하는 Java 버전을 확인하고, 호환성 문제를 피하기 위해 구버전을 사용할 경우 collect(Collectors.toList())로 변경할 수 있습니다.

4. 예외 처리: getAdminDashboard 메서드 내부에서 adminProjectService.getProcessingProjects() 또는 adminProjectTagService.getAllProjectTags(project.getId()) 메서드에서 예외가 발생할 경우 이를 처리할 로직이 필요합니다. 적절한 예외 처리를 추가하여 안정성을 높이는 것이 좋습니다.

5. 메서드 명: getAdminDashboard라는 메서드 이름은 기능을 잘 설명하고 있지만, 반환되는 자료구조와 관련된 부분을 좀 더 명확하게 명명하는 것도 좋습니다. 예를 들어, getProcessingProjectsForAdminDashboard와 같은 형태로.

이상입니다. 전체적으로 코드의 개선 여지가 있지만, 기본적인 기능은 잘 구현된 것으로 보입니다.

[github-actions]

코드 리뷰 결과는 다음과 같습니다:

코드 변경 사항

1. List 클래스를 임포트하였고, 새롭게 /main 엔드포인트를 추가하였습니다. 이 엔드포인트는 getAdminMain 메소드를 통해 관리자의 대시보드 정보를 반환합니다.

코드 개선 사항 및 버그 위험

1. 입력 검증 부족:

   • getAdminMain 메소드에서 adminProjectFacade.getAdminDashboard()가 null을 반환할 경우 APIResponse.success() 메서드 호출에서 NullPointerException이 발생할 수 있습니다. 이를 방지하기 위해 null 체크를 추가해야 합니다.

2. 상태 코드 확인:

   • SuccessCode.OK.getStatusCode() 메서드가 적절한 상태 코드를 반환하는지 확인이 필요합니다. 변수명과 메서드명에서 상태 코드를 반환하는 방법에 대해 명확히 이해하고 있어야 합니다.

3. 응답 형식 일관성:

   • API 응답 형식의 일관성을 유지하기 위해, 성공과 실패에 대한 응답 처리 로직이 일관되어야 합니다. 실패한 경우의 처리 로직이 추가되지 않은 것 같습니다. 적절한 오류 처리 로직을 추가하는 것이 좋습니다.

4. 문서화:

   • 새로운 엔드포인트(/main)에 대한 API 문서를 작성하여, 다른 개발자가 해당 API의 사용 방법을 쉽게 이해할 수 있도록 하는 것이 중요합니다.

5. 케싱 (선택 사항):

   • 대시보드 정보를 자주 조회할 것으로 예상되는 경우, 이를 캐싱하여 응답 성능을 향상시킬 수 있습니다.

이와 같은 점들을 고려하여 코드의 안정성과 가독성을 높일 수 있습니다. 추가적으로, 테스트 케이스를 작성하여 새로운 메소드에 대한 테스트를 수행하는 것도 좋겠습니다.

[github-actions]

이 코드 패치에 대한 간략한 검토를 해보겠습니다. 다음은 몇 가지 버그 위험 및 개선 제안입니다:

코드 검토

1. 불필요한 @NoArgsConstructor:

   • Response 클래스는 정적 생성자를 사용해서만 인스턴스를 생성하므로, 기본 생성자는 필요하지 않아 보입니다. @NoArgsConstructor를 제거하는 것이 더 나아 보입니다.

2. 불변성 유지:

   • 객체의 상태가 변하지 않도록 하기 위해, Response 클래스의 모든 필드를 final로 지정하는 것을 고려해 볼 수 있습니다. 그럴 경우, 생성자에서 모든 필드가 초기화되어야 하며, 이 클래스는 불변 객체로 만들 수 있습니다.

3. Null 처리:

   • Project와 List<ProjectTag>는 null일 가능성이 있습니다. null 검사를 추가하여 NPE(Null Pointer Exception)를 방지하는 것이 좋습니다.

4. 접근 제어자:

   • 모든 필드가 private로 선언되어 있지만, getter 메서드가 있어서 외부에서 접근 가능하게 되어 있습니다. 공개 API의 일관성을 유지하기 위해 필요에 따라 접근 제어자를 조정할 수 있습니다.

5. Java 11의 List::toList 사용:

   • toList() 메서드는 Java 16부터 추가된 메서드입니다. 따라서 Java 11에서 이 코드를 사용하는 경우, Collectors.toList()를 사용하도록 변경해야 합니다.

6. Response 클래스의 위치:

   • 특정 DTO 클래스는 보통 별도의 파일로 분리하는 것이 좋습니다. 현재 패치에서는 GetAdminDashboard 클래스 안에 중첩되어 있지만, 이를 별도의 파일로 분리하면 가독성을 향상시킬 수 있습니다.

개선된 코드(예시)

package com.seveneleven.project.dto;

import com.seveneleven.entity.project.Project;
import com.seveneleven.entity.project.ProjectTag;
import com.seveneleven.entity.project.constant.ProjectStatusCode;
import lombok.Getter;

import java.time.LocalDate;
import java.util.List;
import java.util.Objects;
import java.util.stream.Collectors;

public class GetAdminDashboard {

    @Getter
    public static class Response {
        private final Long id;
        private final String projectName;
        private final LocalDate startDate;
        private final LocalDate endDate;
        private final Long customerId;
        private final String customerCompanyName;
        private final Long developerId;
        private final String developerCompanyName;
        private final String currentStepName;
        private final ProjectStatusCode projectStatus;
        private final List<String> projectTags;

        private Response(Project project, List<ProjectTag> projectTags) {
            Objects.requireNonNull(project, "Project must not be null");
            this.id = project.getId();
            this.projectName = project.getProjectName();
            this.startDate = project.getPlannedStartDate();
            this.endDate = project.getPlannedEndDate();
            this.customerId = project.getCustomer() != null ? project.getCustomer().getId() : null;
            this.customerCompanyName = project.getCustomer() != null ? project.getCustomer().getCompanyName() : null;
            this.developerId = project.getDeveloper() != null ? project.getDeveloper().getId() : null;
            this.developerCompanyName = project.getDeveloper() != null ? project.getDeveloper().getCompanyName() : null;
            this.currentStepName = project.getCurrentProjectStep();
            this.projectTags = projectTags.stream().map(ProjectTag::getTag).collect(Collectors.toList());
            this.projectStatus = project.getProjectStatusCode();
        }

        public static Response toDto(Project project, List<ProjectTag> projectTags) {
            return new Response(project, projectTags);
        }
    }
}

요약하자면, Код╝의 정리와 안정성을 위해 몇 가지 개선을 고려해보는 것이 좋습니다.

[github-actions]

코드 패치를 살펴본 결과, 몇 가지 개선 사항과 잠재적인 버그 리스크를 발견했습니다.

1. Null 체크: getProcessingProjects 메서드는 adminProjectRepository.findAllByProjectStatusCode를 호출하는데, 만약 이 메서드가 null을 반환할 경우, 호출하는 쪽에서 NullPointerException이 발생할 수 있습니다. 이러한 경우를 방지하기 위해, 반환값을 List가 아닌 Collections.emptyList()로 초기화하는 방식도 고려할 수 있습니다.

2. 해당 메서드의 이름: getProcessingProjects 메서드는 '처리 중인 프로젝트'라는 의미를 가지고 있습니다. 이 메서드가 어떤 특정한 상태의 프로젝트를 가져오는 것인지(예: IN_PROGRESS) 명확히 문서화하거나, 메서드 이름에 명시하는 것이 좋습니다.

3. 로깅 및 예외 처리: 새로운 메서드에서 발생할 수 있는 예외나 상황을 처리하기 위해 로깅을 추가하는 것도 좋습니다. 예를 들어, 처리 중인 프로젝트가 없을 경우의 로깅 등을 고려해볼 수 있습니다.

4. 테스트 케이스 추가: 새로 추가된 getProcessingProjects 메서드에 대해 충분한 단위 테스트를 작성하는 것이 중요합니다. 특히, 다양한 상태의 프로젝트가 있는 경우와 없는 경우를 테스트하여 메서드의 안정성을 보장해야 합니다.

5. 문서화: 클래스와 메서드에 대한 Javadoc 주석을 추가하면 외부에서 이 코드를 이해하는 데 많은 도움이 될 것입니다. 특히 비즈니스 로직을 설명하는 부분에 대한 설명이 중요합니다.

이러한 점들을 고려하여 코드를 리팩토링하면, 안정성과 가독성을 높일 수 있을 것입니다.

[github-actions]

코드 패치에 대해 간단한 리뷰를 제공하겠습니다.

개선 사항 및 버그 위험

1. 메서드 명명 규칙: findByCustomerOrDeveloper 메서드의 이름이 약간 모호합니다. 더 명확하게 findByCustomerOrDeveloperCompany와 같이 고치면 더욱 이해하기 쉬울 것 같습니다.

2. Null 체크: findAllByProjectStatusCode 메서드는 ProjectStatusCode가 null인 경우를 처리하지 않습니다. 입력 값이 null일 경우 NullPointerException을 방지하기 위해 입력 값을 체크하는 로직을 추가하는 것이 좋습니다.

3. 결과 검증: findByIdAndProjectStatusCodeNot 메서드는 Optional을 반환하고 있지만, 사용자가 결과를 사용할 때 제대로 처리하지 않으면 문제를 일으킬 수 있습니다. 호출하는 측에서 Optional의 유무를 체크하는 패턴을 명확히 하는 것이 좋습니다.

4. 표준화된 예외 처리: 데이터베이스에서 데이터를 찾지 못했을 경우에 대한 처리가 명시적으로 필요합니다. 예를 들어, 특정 프로젝트가 없는 경우 적절한 예외를 던지는 것이 좋을 수 있습니다.

5. Javadoc 주석 추가: 새로운 메서드(findAllByProjectStatusCode)에 대해 메서드의 기능과 사용 방법을 설명하는 주석을 추가하면 이해하기가 더 쉬워질 것입니다.

이 외에도, 테스트 커버리지나 성능 관점에서의 검토(예: 대량의 데이터를 다룰 경우)가 필요할 수 있습니다. 위의 사항들을 고려하면 코드 품질을 한층 높일 수 있을 것입니다.

[github-actions]

코드 패치에 대한 간단한 리뷰를 제공하겠습니다.

1. 기능 추가 검토: getProcessingProjects() 메소드가 추가되었습니다. 이 메소드가 무엇을 수행하는지에 대한 주석이나 문서화가 필요할 것입니다. 코드의 가독성과 유지보수성을 높이기 위해서 추가된 기능에 대한 설명이 필요합니다.

2. 메소드명: getProcessingProjects()라는 메소드명이 구체적인 의미를 전달하고 있는지 확인해볼 필요가 있습니다. "Processing"이 어떤 상태의 프로젝트를 의미하는지 명확히 해주는 것이 좋습니다. 예를 들어, 진행 중인지, 처리 중인 것인지에 대한 정의가 필요합니다.

3. 예외 처리: 추가된 메소드에서 발생할 수 있는 예외 상황에 대한 처리가 필요할 수 있습니다. 예를 들어, 요청된 프로젝트가 없거나 시스템 오류가 발생했을 때 적절히 대응할 수 있도록 고려해야 합니다.

4. 통합 테스트: 새로운 메소드로 인해 기존의 동작에 영향이 갈 수도 있으므로, 통합 테스트를 통해 새로운 코드가 기존 시스템과 잘 통합되는지 확인하는 것이 중요합니다.

5. 종속성 주입: 만약 getProcessingProjects()에서 특정 의존성을 사용할 경우, 해당 의존성이 적절히 주입될 수 있는지 검토해야 합니다. 의존성 관리에 대한 측면도 중요합니다.

이 외에도 코드에 대한 전체적인 테스트 커버리지를 확인하고 기존의 메소드들과 일관성을 유지하는 것이 중요합니다. 추가된 코드에 대해 잘 문서화하고, 필요한 경우 코드 리뷰를 통해 팀원과 상의하는 것도 좋은 접근 방식입니다.

[github-actions]

코드 패치에 대한 리뷰입니다. 다음은 몇 가지 버그 리스크와 개선 제안입니다:

1. 코드 가독성: 코드의 가독성을 높이기 위해 불필요한 줄 개행을 줄이는 것이 좋습니다. 클래스 또는 메소드 사이에 주석을 추가하여 해당 기능의 목적을 설명하는 것도 좋은 방법입니다.

2. 예외 처리: getProcessingProjects 메소드에서 adminProjectReader.getProcessingProjects()에서 발생할 수 있는 예외를 처리하는 로직이 없습니다. 예외 처리를 추가하여 API에서 발생할 수 있는 오류를 관리하는 것이 좋습니다.

3. Null 체크: getProcessingProjects 메소드의 반환값에 대한 Null 체크가 필요할 수 있습니다. 만약 adminProjectReader.getProcessingProjects()가 null을 반환할 가능성이 있다면, 이에 대한 처리가 필요합니다.

4. 기능 테스트: 추가된 메소드에 대한 유닛 테스트가 포함되어 있는지 확인해야 합니다. 새로운 기능이 성공적으로 작동하는지 검증하기 위해 테스트가 필요합니다.

5. 문서화: 새로 추가된 메소드에 대한 Javadoc 주석을 추가하여 이 메소드의 기능과 사용법을 설명하는 것이 좋습니다.

이 외에도 코드의 전반적인 유지보수성을 고려해야 하므로, 패키지 구조 및 클래스 설계의 일관성도 검토할 수 있습니다.

[github-actions]

코드 리뷰를 진행하겠습니다.

변경사항 요약

1. setChecklistApplication() 메서드에서 checklistStatus가 APPLICATION_WAITING인 경우에만 상태를 APPROVE_WAITING으로 변경하던 것을, 추가로 REJECTED 상태일 때도 같은 처리를 하도록 수정했습니다.
2. rejectChecklist() 메서드에서 checklistStatus가 APPROVE_WAITING인 경우에 APPLICATION_WAITING으로 상태를 변경하던 것을, 이제 상태를 REJECTED로 변경하도록 수정했습니다.

버그 리스크

1. 상태 관리: checklistStatus의 상태 변경 로직이 추가되면서, 다른 메서드에서 상태를 체크할 때 예상치 못한 동작이 발생할 수 있습니다. 특히, 상태를 REJECTED로 변경하는 부분에서, 기존의 비즈니스 로직과의 충돌이 발생할 위험이 있습니다.

2. 상태 전이: 만약 setChecklistApplication()이 호출된 다음에 rejectChecklist()가 호출된다면 이제 checklistStatus는 REJECTED에 있게 되는데, 이 경우 비즈니스 로직이 어떻게 작동할지가 명확하지 않습니다. 모든 상태 전이에 대한 완전한 테스트가 필요합니다.

개선 제안

1. 상태 전이 메서드: 상태 전이를 보다 명확하게 관리하기 위해, 상태 변경 로직을 별도의 메서드로 분리하는 것이 좋습니다. 이를 통해 각 메서드의 책임이 명확해지고, 변경 사항이 발생할 때 수정하기 용이해집니다.

2. 상태 유효성 검사: 두 메서드에서 상태 변경 시, 해당 상태가 유효한지 확인하는 추가적인 체크를 구현하는 것이 좋습니다. 예를 들어, checklistStatus가 특정 상태지만 메서드가 호출되었을 때 적절한 에러를 발생시키는 로직을 추가하면 좋습니다.

3. 상태 다이어그램: 체크리스트의 상태가 복잡해질 경우, 상태 다이어그램을 작성하여 각 상태 간의 전이를 시각적으로 표현하는 것이 좋습니다. 이는 코드 유지보수에 큰 도움이 될 수 있습니다.

이러한 피드백을 반영하여 코드의 안정성과 가독성을 높일 수 있을 것입니다.

[github-actions]

코드 패치를 검토한 결과는 다음과 같습니다:

1. 버그 리스크:

   • LocalDate 타입으로 필드 startDate와 endDate를 변경한 것은 날짜 및 시간 관리에서 더 나은 방식입니다. 그러나 project.getStartDate()와 project.getEndDate() 메소드가 LocalDate 타입을 반환하는지 확인해야 합니다. 만약 반환 타입이 다르면, ClassCastException이 발생할 수 있습니다.

2. 유효성 검사:

   • CompanyProject 생성자에서 project의 startDate와 endDate가 null일 가능성이 있습니다. 만약 null이라면 NullPointerException을 피하기 위해 적절한 검사가 필요합니다. 예를 들어, null인 경우 기본값을 설정하거나 예외를 발생시킬 수 있습니다.

3. 코드 정리:

   • CustomDateFormatter를 사용하던 부분이 제거되었고, LocalDate를 활용하게 되면서 의존성이 줄어든 점은 긍정적입니다. 향후 LocalDate와 관련된 포맷팅이 필요할 경우 별도의 유틸리티 클래스를 고려할 수도 있습니다.

4. 주석 및 문서화:

   • 코드 변경 사항에 대한 주석이 없기 때문에, 동일한 코드에 익숙하지 않은 개발자는 변경 이유를 이해하기 어려울 수 있습니다. 코드 변경의 이유와 의도를 설명하는 주석을 추가하는 것이 좋습니다.

5. Lombok 사용:

   • Lombok을 사용하고 있지만, 클래스에 @DaTa 또는 @value를 추가하는 것도 고려해볼 수 있습니다. 이는 getter, setter, toString, equals, hashCode 등의 메소드를 자동 생성해 코드량을 줄일 수 있습니다.

6. API 문서화:

   • toDto 메소드가 외부에서 호출되는 경우, 이 메소드에 대한 API 문서화가 필요할 수 있습니다. 어떤 형식의 데이터가 입력되는지, 반환 데이터의 구조는 어떤지를 명확히 해두자는 것입니다.

이러한 관점을 통해 코드를 개선할 수 있으며, 안정성과 가독성을 높일 수 있습니다. 추가 질문이나 특정 부분에 대한 더 깊은 분석이 필요하다면 말씀해 주세요.

[github-actions]

코드 패치를 검토한 결과는 다음과 같습니다:

1. 타입 변경: startDate와 endDate의 타입을 String에서 LocalDate로 변경한 것은 좋은 선택입니다. 이렇게 하면 날짜와 관련된 연산을 더 안전하게 처리할 수 있고, 데이터의 정확성을 높일 수 있습니다.

2. 날짜 포맷팅 제거: CustomDateFormatter.formatDate 메소드를 제거한 것은 코드의 간결성을 높여줍니다. 그러나, project.getPlannedStartDate()와 project.getPlannedEndDate()가 LocalDate 타입인지 확실히 확인해야 합니다. 만약 이 메소드들이 String 타입을 반환하는 경우라면, 이후에 문제가 발생할 수 있습니다.

3. null 체크 필요성: project, project.getCustomer(), 그리고 project.getDeveloper()가 null일 수 있으므로, 이를 체크하는 로직이 필요합니다. 이 부분에 대한 예외 처리가 없다면 NullPointerException이 발생할 위험이 있습니다.

4. 메소드 역할 명확화: GetMyProjectResponseInfo의 생성자에서 데이터를 초기화하는 로직이 포함되어 있습니다. 만약 이 클래스가 데이터 전송 객체(DTO)라면, 더 가독성을 높이기 위해 매개변수를 사용하는 방식으로 초기화를 진행하거나, 빌더 패턴을 고려해보는 것 또한 좋은 방법입니다.

5. Lombok 활용: Lombok을 사용하고 있으므로, @Data 또는 @Builder 어노테이션을 추가하여 boilerplate 코드를 줄이는 것도 고려해볼 수 있습니다. 이렇게 하면 코드의 유지보수성과 가독성을 높일 수 있습니다.

6. 문서화 및 주석: 코드를 문서화하거나 주요 로직에 주석을 추가하는 것도 좋습니다. 특히 다른 개발자들이 코드의 의도를 이해하는 데 도움을 줄 수 있습니다.

요약하자면, 타입 변경은 좋은 결정이나, null 체크와 관련된 예외 처리, 코드 가독성을 높이기 위한 추가 개선이 필요합니다.