Develop - [merged]

[yul9910]

Merges develop -> main

1주차 코드리뷰 MR요청입니다!

[yul9910]

In GitLab by @HyeonSeung on Oct 11, 2024, 20:53

added 3 commits

• 7d42b74a - fix: s3 설정 수정
• 494dc571 - feat: 상품에 이미지 기능 추가
• f06b1893 - Merge branch 'feature/hyeonseung/item-s3-#1' into 'develop'

Compare with previous version

[yul9910]

In GitLab by @GobumE on Oct 11, 2024, 21:11

added 3 commits

• 08a3e8fa - feat: 카테고리 url 경로 한글, 영어 번역, webConfig 경로 추가
• f1d97bf5 - Merge branch 'develop' of...
• dfd60268 - Merge branch 'feature/고범석/category-front_header_url-#39' into 'develop'

Compare with previous version

[yul9910]

In GitLab by @chanjin23 on Oct 12, 2024, 16:59

Commented on src/main/java/com/spring_boots/spring_boots/user/domain/Users.java line 45

nullable= true 코드 제거했습니다..!

[yul9910]

In GitLab by @chanjin23 on Oct 12, 2024, 17:01

Commented on src/main/java/com/spring_boots/spring_boots/user/domain/Users.java line 94

getUsername() 과 같은 메서드가 있으나 dto를 사용하여 요청,응답 이용하고 username 필드의 getter메서드를 사용안할 것같아서 따로 수정은 안했습니다..!!

[yul9910]

In GitLab by @chanjin23 on Oct 12, 2024, 17:02

Commented on src/main/java/com/spring_boots/spring_boots/user/controller/TokenApiController.java line 20

팀원과 상의해서 통일하였습니다..!

[yul9910]

In GitLab by @SiyoungOh on Oct 13, 2024, 01:06

Commented on src/main/java/com/spring_boots/spring_boots/SpringBootsApplication.java line 9

컴포넌트 스캔이 하는 역할이 어떤건가요? 다른 코드와 중복되는 부분이 있는지 확인해보세요~

[yul9910]

In GitLab by @SiyoungOh on Oct 13, 2024, 01:07

Commented on src/main/java/com/spring_boots/spring_boots/item/controller/ItemRestController.java line 25

필드 주입(@Autowired)과 생성자 주입에 대해서 체크해보시고 더 나은 케이스로 바꿔주세요.

[yul9910]

In GitLab by @SiyoungOh on Oct 13, 2024, 01:07

Commented on src/main/java/com/spring_boots/spring_boots/item/controller/ItemRestController.java line 44

컨트롤러 어드바이스를 참고해보세요

[yul9910]

In GitLab by @SiyoungOh on Oct 13, 2024, 01:07

Commented on src/main/java/com/spring_boots/spring_boots/orders/controller/OrdersApiController.java line 30

Users 객체를 직접 사용하고 있습니다. 사용자의 모든 정보를 노출시키는 것은 보안 문제를 일으킬 수 있습니다. 객체를 직접 사용하는 것 대신 다른 방법을 생각해보세요. 이미 다른 부분에서는 쓰고 있는 방법입니다.

[yul9910]

In GitLab by @SiyoungOh on Oct 13, 2024, 01:07

Commented on src/main/java/com/spring_boots/spring_boots/category/controller/EventApiController.java line 30

@Valid 검사가 실패하면 발생하는 예외는 클라이언트에게 의미 있는 메시지를 제공하지 않을 수 있습니다.이 예외를 클라이언트에게 의미 있는 메시지를 주게 바꾸는 것이 필요합니다.

[yul9910]

In GitLab by @SiyoungOh on Oct 13, 2024, 01:07

Commented on src/main/java/com/spring_boots/spring_boots/category/controller/AdminCategoryApiController.java line 56

@PreAuthorize 를 살펴보세요

[yul9910]

In GitLab by @SiyoungOh on Oct 13, 2024, 01:07

Commented on src/main/java/com/spring_boots/spring_boots/category/dto/event/EventDetailDto.java line 25

이런 주석은 나중에 처리하는 걸 놓칠 수 있습니다. 이슈로 만들거나 // TODO 코멘트를 써서 관리해주세요.

[yul9910]

In GitLab by @SiyoungOh on Oct 13, 2024, 01:07

Commented on src/main/java/com/spring_boots/spring_boots/category/controller/EventApiController.java line 63

이벤트가 만료되면 이벤트를 삭제하나요?

[yul9910]

In GitLab by @SiyoungOh on Oct 13, 2024, 01:07

Commented on src/main/java/com/spring_boots/spring_boots/category/entity/Event.java line 66

Category 엔티티에서도 이 Event를 참조하고 있는 경우, Category 엔티티에서도 이 Event를 제거해야하는지 확인해보세요.

[yul9910]

In GitLab by @SiyoungOh on Oct 13, 2024, 01:07

Commented on src/main/java/com/spring_boots/spring_boots/category/controller/CategoryApiController.java line 18

Category 모듈은 전반적으로 패턴을 잘 따라서 구현하고 있습니다. 화이팅입니다.

[yul9910]

In GitLab by @SiyoungOh on Oct 13, 2024, 01:07

Commented on src/main/java/com/spring_boots/spring_boots/item/controller/ItemRestController.java line 45

사용자에게 더 구체적인 에러 메시지를 제공하는 것이 좋을 수 있습니다. 예를 들어, 파일 업로드에 실패했다는 메시지를 포함하는 사용자 정의 예외를 던지는 것이 좋을 수 있습니다.

[yul9910]

In GitLab by @SiyoungOh on Oct 13, 2024, 01:07

Commented on src/main/java/com/spring_boots/spring_boots/orders/controller/OrdersApiController.java line 31

info 레벨은 기본 실행시에도 로깅되므로 나중에 로그가 많아지면 추적하기 어려울 수 있습니다. DEBUG 처럼 로그 레벨을 낮추는 걸 고려해보세요.

[yul9910]

In GitLab by @SiyoungOh on Oct 13, 2024, 01:07

Commented on src/main/java/com/spring_boots/spring_boots/orders/controller/OrdersApiController.java line 37

다른 모듈에서 로깅하는 것과 스타일 차이가 보입니다. 팀원들과 논의해서 로깅을 어떻게 처리할지 전반적으로 통일하면 좋을 것 같습니다.

[yul9910]

In GitLab by @SiyoungOh on Oct 13, 2024, 01:07

Commented on src/main/java/com/spring_boots/spring_boots/orders/dto/OrderDetailsDto.java line 30

가독성을 위해 orderitem 을 Camel 케이스 지켜주세요.

[yul9910]

In GitLab by @SiyoungOh on Oct 13, 2024, 01:07

Commented on src/main/java/com/spring_boots/spring_boots/orders/service/OrdersService.java line 96

나중에 로깅으로 바꾸는 거 잊지 마세요~ 이런 부분은 놓치지 않게 // TODO 주석을 활용하는 것이 좋습니다.

[yul9910]

In GitLab by @SiyoungOh on Oct 13, 2024, 01:07

Commented on src/main/java/com/spring_boots/spring_boots/s3Bucket/controller/S3BucketController.java line 31

Hello World~

[yul9910]

In GitLab by @SiyoungOh on Oct 13, 2024, 01:07

Commented on src/main/java/com/spring_boots/spring_boots/s3Bucket/service/S3BucketService.java line 75

지금 예외는 너무 넓어서 나중에 디버깅하기 어려울 수 있습니다. 좀 더 구체적인 예외를 사용하는 게 좋습니다. 다른 모듈에서 이 경우에 어떤 예외를 사용하는지 확인하고 통일해주세요.

[yul9910]

In GitLab by @SiyoungOh on Oct 13, 2024, 01:07

Commented on src/test/java/com/spring_boots/spring_boots/category/service/CategoryServiceTest.java line 44

테스트 메소드 스타일이 다른 모듈과 다릅니다. 가독성과 유지보수를 위해 팀원들과 논의해 통일해주세요.

[yul9910]

In GitLab by @SiyoungOh on Oct 13, 2024, 01:07

Commented on src/test/java/com/spring_boots/spring_boots/category/service/CategoryServiceTest.java line 295

필요하다면 페이지네이션 테스트도 고려해보세요.

[yul9910]

In GitLab by @SiyoungOh on Oct 13, 2024, 01:07

Commented on src/test/java/com/spring_boots/spring_boots/item/controller/ItemRestControllerTest.java line 28

Autowired 가 필요한지, 이 부분에서 사용할 수 있는 것인지 확인해보세요.

[yul9910]

In GitLab by @SiyoungOh on Oct 13, 2024, 01:07

Commented on src/test/java/com/spring_boots/spring_boots/item/controller/ItemRestControllerTest.java line 50

itemRestService.createItem 의 파라미터 갯수를 확인해보세요

[yul9910]

In GitLab by @SiyoungOh on Oct 13, 2024, 01:07

Commented on src/test/java/com/spring_boots/spring_boots/orders/controller/OrdersApiControllerTest.java line 268

실패할 경우에 대한 테스트케이스도 있나요?

[yul9910]

In GitLab by @SiyoungOh on Oct 13, 2024, 01:07

Commented on src/test/java/com/spring_boots/spring_boots/orders/service/OrdersServiceTest.java line 25

실패하는 경우에 대해서도 테스트해주세요.

[yul9910]

In GitLab by @SiyoungOh on Oct 13, 2024, 01:07

Commented on src/test/java/com/spring_boots/spring_boots/user/controller/TokenApiControllerTest.java line 22

로그인이 실패했을 때의 테스트케이스도 필요해보입니다.

[yul9910]

In GitLab by @SiyoungOh on Oct 13, 2024, 01:07

Commented on src/test/java/com/spring_boots/spring_boots/user/service/UserServiceTest.java line 67

저장된 사용자의 정보가 적절하게 반환되는지 확인할 수 있을까요?

[yul9910]

In GitLab by @GobumE on Oct 13, 2024, 13:44

added 3 commits

• 11caa2fb - feat: category-detail.html, css, js 수정 + category-new-or-best.html, css, js 작성
• bc2e3a42 - Merge branch 'develop' of...
• f1cb3232 - Merge branch 'feature/고범석/category-front_howto_new_best_event_html-#48' into 'develop'

Compare with previous version

[yul9910]

In GitLab by @yeomhyeseon on Oct 13, 2024, 21:58

Commented on src/main/resources/static/account-signout/account-signout.js line 43

api.js의 del함수를 사용할 수 있겠어요~ 필요한 설정이 있다면 del함수를 수정해서 사용해보세요

[yul9910]

In GitLab by @yeomhyeseon on Oct 13, 2024, 21:58

Commented on src/main/resources/static/cart/cart.html line 111

dom이 로드된 뒤에 실행하고 싶어서 body 마지막에 위치시켰다면 head에 선언하고 defer속성을 추가하면 dom이 로드된 후에 js가 실행됩니다

[yul9910]

In GitLab by @yeomhyeseon on Oct 13, 2024, 21:58

Commented on src/main/resources/static/cart/cart.js line 62

파라미터를 스네이크케이스로 정의한 이유가 있을까요?
팀의 컨벤션은 카멜케이스로 보여요~

[yul9910]

In GitLab by @yeomhyeseon on Oct 13, 2024, 21:58

Commented on src/main/resources/static/category-detail/category-detail.js line 61

옵셔널체이닝을 사용해서 category?.displayOrder !== '1'로 작성할 수 있어요

[yul9910]

In GitLab by @yeomhyeseon on Oct 13, 2024, 21:58

Commented on src/main/resources/static/category-detail/category-detail.js line 67

    thirdBreadcrumb.classList.add('is-active');
    return;
  }
  thirdBreadcrumb.style.display = 'none';

early return구문을 사용하면 보다 가독성있는 코드를 작성할 수 있어요

[yul9910]

In GitLab by @yeomhyeseon on Oct 13, 2024, 21:58

Commented on src/main/resources/static/category-new-or-best/category-new-or-best.js line 16

'new-in', 'best'가 조건으로 계속 사용되는 값이기 때문에 상수로 선언하면 유지보수가 용이해지겠네요~

[yul9910]

ordersRepository 메소드에 추가해 수정했습니다!

[yul9910]

In GitLab by @GobumE on Oct 14, 2024, 13:56

Commented on src/main/resources/static/category-new-or-best/category-new-or-best.js line 16

new-in, best 부분을 대소문자 구분하여 따로 상수를 적용했습니다.

[yul9910]

In GitLab by @GobumE on Oct 14, 2024, 13:56

Commented on src/main/resources/static/category-detail/category-detail.js line 67

수정하였습니다.

[yul9910]

In GitLab by @GobumE on Oct 14, 2024, 13:56

Commented on src/main/resources/static/category-detail/category-detail.js line 61

수정하였습니다. 감사합니다~

[yul9910]

In GitLab by @GobumE on Oct 14, 2024, 13:57

Commented on src/test/java/com/spring_boots/spring_boots/category/service/CategoryServiceTest.java line 295

카테고리와 이벤트 모두 페이지네이션 테스트가 필요한 것 같아 추가하였습니다.

[yul9910]

In GitLab by @GobumE on Oct 14, 2024, 13:58

Commented on src/test/java/com/spring_boots/spring_boots/category/service/CategoryServiceTest.java line 44

팀원들의 테스트 스타일에 맞도록 통일하였습니다.

[yul9910]

In GitLab by @GobumE on Oct 14, 2024, 13:59

Commented on src/main/java/com/spring_boots/spring_boots/category/entity/Event.java line 66

Category와 Event 엔티티 모두 서로 참조하던 부분을 제거하였습니다.

[yul9910]

In GitLab by @GobumE on Oct 14, 2024, 14:01

Commented on src/main/java/com/spring_boots/spring_boots/category/controller/EventApiController.java line 63

이벤트 조회 시 마다 이벤트의 만료 여부를 확인하여 만료된 경우에는 soft Delete를 적용하여 이벤트를 표시하지 않도록 하였습니다. (엔티티에 만료 메서드를 추가하고 서비스에서는 조회 마다 만료 여부를 업데이트 후 확인하도록 하였습니다.)

[yul9910]

In GitLab by @GobumE on Oct 14, 2024, 14:01

Commented on src/main/java/com/spring_boots/spring_boots/category/dto/event/EventDetailDto.java line 25

해당 필드를 주석과 함께 제거하였습니다.

[yul9910]

In GitLab by @GobumE on Oct 14, 2024, 14:03

Commented on src/main/java/com/spring_boots/spring_boots/category/controller/AdminCategoryApiController.java line 56

@PreAuthorize("hasRole('ADMIN')")로 대체하여 사용하고 GlobalExceptionHandler에 AccessDeniedException을 추가했습니다.

[yul9910]

In GitLab by @GobumE on Oct 14, 2024, 14:03

Commented on src/main/java/com/spring_boots/spring_boots/category/controller/EventApiController.java line 30

GlobalExceptionHandler에 @RestControllerAdvice와 @ExceptionHandler(MethodArgumentNotValidException.class)를 추가했습니다.

[yul9910]

In GitLab by @GobumE on Oct 14, 2024, 14:04

Commented on src/main/java/com/spring_boots/spring_boots/category/controller/CategoryApiController.java line 18

피드백 해주신 부분 모두 감사합니다!

[yul9910]

 @GetMapping("/api/orders")
    public ResponseEntity<?> getUserOrders() {
        // SecurityContext에서 인증 정보 가져오기
        Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
        Long userId = ((Users) authentication.getPrincipal()).getUserId();  // 필요한 정보만 추출

        log.debug("유저 아이디: {}", userId);

        try {
            List<OrderDto> orders = ordersService.getUserOrders(userId);  // 필요한 정보로만 처리
            return ResponseEntity.ok(orders);
        } catch (Exception e) {
            log.error("사용자 주문을 가져오는 도중 오류 발생: {}", e.getMessage(), e);
            return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body("내부 서버 오류가 발생했습니다.");
        }
    }

이렇게 필요한 정보만 가져와서 수정하면 괜찮을까요?