Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

우리 CQRS 패턴으로 작업중이라서 Request/Response Dto 패키지 따로 만들어서 관리해야 해.

Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package ssurent.ssurentbe.domain.users.controller;

import lombok.RequiredArgsConstructor;
import org.springframework.security.core.annotation.AuthenticationPrincipal;
import org.springframework.security.core.userdetails.UserDetails;
import org.springframework.web.bind.annotation.*;
import ssurent.ssurentbe.domain.users.dto.UpdatePhoneNumberRequest;
import ssurent.ssurentbe.domain.users.dto.UserInfoResponse;
import ssurent.ssurentbe.domain.users.dto.UserPenaltyResponse;
import ssurent.ssurentbe.domain.users.entity.Users;
import ssurent.ssurentbe.domain.users.service.UserService;

import java.util.List;

@RestController
@RequiredArgsConstructor
@RequestMapping("/v1/api/users")
public class UserController {
private final UserService userService;

@GetMapping
public UserInfoResponse getMyInfo(@AuthenticationPrincipal UserDetails userDetails) {
return userService.getMyInfo(userDetails.getUsername());
}
Comment on lines +26 to +29
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

getMyInfo 응답이 BaseResponse로 래핑되지 않았습니다.

getMyPenaltiesAuthController의 다른 엔드포인트들은 모두 BaseResponse로 래핑하여 반환하는데, getMyInfoUserInfoResponse를 직접 반환합니다. API 응답 형식의 일관성을 위해 동일한 패턴을 적용하는 것이 좋습니다.

♻️ 수정 제안
     `@GetMapping`
-    public UserInfoResponse getMyInfo(`@AuthenticationPrincipal` UserDetails userDetails) {
-        return userQueryService.getMyInfo(userDetails.getUsername());
+    public ResponseEntity<BaseResponse<UserInfoResponse>> getMyInfo(`@AuthenticationPrincipal` UserDetails userDetails) {
+        UserInfoResponse data = userQueryService.getMyInfo(userDetails.getUsername());
+        SuccessStatus status = SuccessStatus.COMM_SUCCESS_STATUS;
+        return ResponseEntity.status(status.getHttpStatus())
+                .body(BaseResponse.success(status, data));
     }
🤖 Prompt for AI Agents
In `@src/main/java/ssurent/ssurentbe/domain/users/controller/UserController.java`
around lines 26 - 29, The getMyInfo endpoint in UserController currently returns
UserInfoResponse directly; change it to return the consistent wrapper type
(BaseResponse<UserInfoResponse>) and wrap the service result
(userQueryService.getMyInfo(userDetails.getUsername())) in a BaseResponse
success payload (use the same static factory or constructor used by other
endpoints like getMyPenalties/AuthController) and update the method signature
and return statement accordingly so the controller consistently returns
BaseResponse across endpoints.


@PatchMapping("/phone-number")
public void updatePhoneNumber(
@AuthenticationPrincipal UserDetails userDetails,
@RequestBody UpdatePhoneNumberRequest request
) {
userService.updatePhoneNumber(
userDetails.getUsername(),
request.phoneNum()
);
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
@GetMapping("/penalties")
public List<UserPenaltyResponse> getMyPenalties(
@AuthenticationPrincipal UserDetails userDetails
) {
return userService.getMyPenalties(userDetails.getUsername());
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package ssurent.ssurentbe.domain.users.dto;

public record UpdatePhoneNumberRequest(
String phoneNum
) {
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

전화번호 입력값 검증이 누락되었습니다.

phoneNum 필드에 @NotBlank@Pattern 같은 Bean Validation 어노테이션이 없어서, 빈 문자열이나 잘못된 형식의 전화번호가 그대로 저장될 수 있습니다. 컨트롤러에서 @Valid와 함께 사용하려면 여기에 검증 어노테이션이 필요합니다.

제안 수정사항
+import jakarta.validation.constraints.NotBlank;
+import jakarta.validation.constraints.Pattern;
+
 public record UpdatePhoneNumberRequest(
-        String phoneNum
+        `@NotBlank`(message = "전화번호는 필수 입력값입니다.")
+        `@Pattern`(regexp = "^01[016789]-?\\d{3,4}-?\\d{4}$", message = "올바른 전화번호 형식이 아닙니다.")
+        String phoneNum
 ) {
 }
🤖 Prompt for AI Agents
In
`@src/main/java/ssurent/ssurentbe/domain/users/dto/UpdatePhoneNumberRequest.java`
around lines 3 - 6, UpdatePhoneNumberRequest currently exposes phoneNum without
validation; add Bean Validation annotations to the record component (e.g.,
annotate phoneNum with `@NotBlank` and a suitable `@Pattern` regex for your phone
format or a more permissive pattern) so controllers using `@Valid` will reject
empty or malformed numbers; update imports to use javax.validation.constraints.*
(or jakarta.validation.constraints.* depending on project) and ensure the
pattern matches your expected formats (international/local) and error messages
are set if needed.

Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package ssurent.ssurentbe.domain.users.dto;

import ssurent.ssurentbe.domain.users.entity.Users;
import ssurent.ssurentbe.domain.users.enums.Role;
import ssurent.ssurentbe.domain.users.enums.Status;

public record UserInfoResponse(
String studentNum,
String name,
String phoneNum,
Status status,
Role role
) {
public static UserInfoResponse from(Users user) {
return new UserInfoResponse(
user.getStudentNum(),
user.getName(),
user.getPhoneNum(),
user.getStatus(),
user.getRole()
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package ssurent.ssurentbe.domain.users.dto;

import ssurent.ssurentbe.domain.users.entity.UserPanaltyLog;
import ssurent.ssurentbe.domain.users.enums.PanaltyTypes;

import java.time.LocalDateTime;

public record UserPenaltyResponse(
Long penaltyId,
PanaltyTypes penaltyType,
Long itemId,
Long rentalHistoryId,
LocalDateTime createdAt
) {
public static UserPenaltyResponse from(UserPanaltyLog log) {
return new UserPenaltyResponse(
log.getId(),
log.getPanaltyType(),
log.getItemsId() != null ? log.getItemsId().getId() : null,
log.getRentalHistoryId() != null ? log.getRentalHistoryId().getId() : null,
log.getCreatedAt()
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,8 @@ public void withdraw(){
this.phoneNum = null;
this.studentNum = null;
}
public void updatePhoneNumber(String phoneNum) {
this.phoneNum = phoneNum;
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package ssurent.ssurentbe.domain.users.repository;

import org.springframework.data.jpa.repository.JpaRepository;
import ssurent.ssurentbe.domain.users.entity.UserPanaltyLog;
import ssurent.ssurentbe.domain.users.entity.Users;

import java.util.List;
import java.util.Optional;

public interface UserPanaltyLogRepository extends JpaRepository<UserPanaltyLog, Long> {

List<UserPanaltyLog> findByUserIdOrderByCreatedAtDesc(Users user);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -name "UserPanaltyLog.java" -type f

Repository: SSURENT/SSURENT-BE

Length of output: 135


🏁 Script executed:

cat -n ./src/main/java/ssurent/ssurentbe/domain/users/entity/UserPanaltyLog.java

Repository: SSURENT/SSURENT-BE

Length of output: 1330


🏁 Script executed:

# Check if there are other entities with similar naming patterns
rg "private \w+ \w+Id;" --type java -A 1 -B 1 | head -50

Repository: SSURENT/SSURENT-BE

Length of output: 2302


🏁 Script executed:

# Check for "Panalty" vs "Penalty" usage throughout the codebase
rg "Panalty|Penalty" --type java | head -30

Repository: SSURENT/SSURENT-BE

Length of output: 2958


🏁 Script executed:

# Verify how Spring Data JPA resolves the method - check if this pattern works elsewhere in the codebase
rg "findBy.*OrderBy.*Desc\(" --type java -A 1

Repository: SSURENT/SSURENT-BE

Length of output: 551


🏁 Script executed:

# Check the extent of the typo - count usages
rg "Panalty" --type java | wc -l

Repository: SSURENT/SSURENT-BE

Length of output: 64


"Panalty" 오타 수정 필요

클래스명, 엔티티명, 리포지토리에서 Panalty로 표기되어 있으나 올바른 철자는 Penalty입니다. 이 오타는 코드베이스 전반에 걸쳐 16개 위치에서 나타나고 있으므로, 엔티티(UserPanaltyLog), 열거형(PanaltyTypes), 리포지토리 등에서 일관되게 Penalty로 수정하는 것을 권장합니다.

🤖 Prompt for AI Agents
In
`@src/main/java/ssurent/ssurentbe/domain/users/repository/UserPanaltyLogRepository.java`
around lines 10 - 12, The repository and related types use the misspelled
"Panalty" instead of "Penalty" (e.g., UserPanaltyLogRepository, UserPanaltyLog,
PanaltyTypes); rename all occurrences to the correct spelling ("Penalty") across
the codebase: rename classes/interfaces (UserPanaltyLogRepository ->
UserPenaltyLogRepository, UserPanaltyLog -> UserPenaltyLog), enums (PanaltyTypes
-> PenaltyTypes), update file names, imports, JPA entity names/annotations,
repository method signatures (findByUserIdOrderByCreatedAtDesc), tests and any
SQL/migration/table names or JSON fields that reference the old name; use a
project-wide refactor/IDE rename to update the ~16 locations and run/build tests
to ensure no references remain.

}
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,5 @@
public interface UserRepository extends JpaRepository<Users, Long> {
Optional<Users> findByStudentNum(String studentNum);
boolean existsByStudentNum(String studentNum);
Optional<Users> findByStudentNumAndDeletedFalse(String studentNum);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package ssurent.ssurentbe.domain.users.service;

import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import ssurent.ssurentbe.domain.users.entity.UserPanaltyLog;
import ssurent.ssurentbe.domain.users.entity.Users;
import ssurent.ssurentbe.domain.users.repository.UserPanaltyLogRepository;
import ssurent.ssurentbe.domain.users.repository.UserRepository;
import ssurent.ssurentbe.domain.users.dto.*;
import java.util.stream.Collectors;

import java.util.List;

@Service
@RequiredArgsConstructor
@Transactional(readOnly = true)
public class UserService {
private final UserRepository userRepository;
private final UserPanaltyLogRepository userPanaltyLogRepository;

public UserInfoResponse getMyInfo(String username) {
Users user = userRepository.findByStudentNumAndDeletedFalse(username)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

해당 문장이 반복되는거 같으니까 함수화해서 적용하면 좋을 듯

.orElseThrow(() -> new IllegalArgumentException("사용자를 찾을 수 없습니다."));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

커스텀 에러 사용하기스텀 에러 사용하기
.orElseThrow(() -> new GeneralException(ErrorStatus.USER_NOT_FOUND)

UserInfoResponse response = UserInfoResponse.from(user);
return UserInfoResponse.from(user);
}

@Transactional
public void updatePhoneNumber(String username, String phoneNum) {
Users user = userRepository.findByStudentNumAndDeletedFalse(username)
.orElseThrow(() -> new IllegalArgumentException("사용자를 찾을 수 없습니다."));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

동일하게 커스텀 에러 사용
.orElseThrow(() -> new GeneralException(ErrorStatus.USER_NOT_FOUND)


user.updatePhoneNumber(phoneNum);
}

public List<UserPenaltyResponse> getMyPenalties(String username) {
Users user = userRepository.findByStudentNum(username)
.orElseThrow(() -> new RuntimeException("User not found"));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

soft-delete 필터링 누락 및 예외 타입 불일치

두 가지 일관성 문제가 있습니다:

  1. Soft-delete 필터 누락: getMyInfoupdatePhoneNumberfindByStudentNumAndDeletedFalse를 사용하지만, getMyPenaltiesfindByStudentNum을 사용하여 삭제된 사용자의 징계 내역도 조회될 수 있습니다.
  2. 예외 타입 불일치: 다른 메서드는 IllegalArgumentException을 던지지만 여기서는 RuntimeException을 사용합니다. 에러 메시지도 영어/한국어가 혼재되어 있습니다.
🔧 수정 제안
     public List<UserPenaltyResponse> getMyPenalties(String username) {
-        Users user = userRepository.findByStudentNum(username)
-                .orElseThrow(() -> new RuntimeException("User not found"));
+        Users user = userRepository.findByStudentNumAndDeletedFalse(username)
+                .orElseThrow(() -> new IllegalArgumentException("사용자를 찾을 수 없습니다."));
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public List<UserPenaltyResponse> getMyPenalties(String username) {
Users user = userRepository.findByStudentNum(username)
.orElseThrow(() -> new RuntimeException("User not found"));
public List<UserPenaltyResponse> getMyPenalties(String username) {
Users user = userRepository.findByStudentNumAndDeletedFalse(username)
.orElseThrow(() -> new IllegalArgumentException("사용자를 찾을 수 없습니다."));
🤖 Prompt for AI Agents
In `@src/main/java/ssurent/ssurentbe/domain/users/service/UserService.java` around
lines 37 - 39, getMyPenalties currently uses userRepository.findByStudentNum and
throws RuntimeException with mixed-language text; change it to use the
soft-delete-aware repository method (same as getMyInfo/updatePhoneNumber) i.e.
userRepository.findByStudentNumAndDeletedFalse(...) and replace the
RuntimeException with IllegalArgumentException using a consistent message style
(match other methods' language/format). Ensure you update the lookup in
getMyPenalties to mirror the soft-delete check and the exception type and
message used by getMyInfo/updatePhoneNumber.


List<UserPanaltyLog> logs = userPanaltyLogRepository.findByUserIdOrderByCreatedAtDesc(user);

return logs.stream()
.map(UserPenaltyResponse::from)
.collect(Collectors.toList());
}


}