Skip to content

Comments

Feat/#148 add change request fetch api#149

Merged
saokiritoni merged 3 commits intodevelopfrom
feat/#148-add-change-request-fetch-api
Nov 5, 2025
Merged

Feat/#148 add change request fetch api#149
saokiritoni merged 3 commits intodevelopfrom
feat/#148-add-change-request-fetch-api

Conversation

@kwdahun
Copy link
Contributor

@kwdahun kwdahun commented Sep 29, 2025

@kwdahun kwdahun self-assigned this Sep 29, 2025
@kwdahun kwdahun requested a review from a team as a code owner September 29, 2025 12:42
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds functionality to fetch all change requests regardless of their status, extending the existing change request API. Previously, only pending change requests could be retrieved.

  • Adds a new getAllChangeRequests() method to retrieve change requests in all statuses
  • Extends the ChangeRequestResponseDTO to include an adminComment field
  • Creates a new API endpoint /all for fetching all change requests

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
AdminRequestQueryService.java Adds service method to fetch all change requests without status filtering
ChangeRequestResponseDTO.java Extends response DTO to include admin comment field
AdminRequestChangeApi.java Adds API documentation for the new getAllChangeRequests endpoint
AdminRequestChangeController.java Implements new REST endpoint for fetching all change requests

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +68 to +72
public List<ChangeRequestResponseDTO> getAllChangeRequests() {
return changeRequestRepository.findAll().stream()
.map(ChangeRequestResponseDTO::fromEntity)
.collect(Collectors.toList());
}
Copy link

Copilot AI Sep 29, 2025

Choose a reason for hiding this comment

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

The new getAllChangeRequests() method duplicates the logic from the existing getChangeRequests() method. Consider extracting the common stream processing logic into a private helper method to reduce code duplication.

Copilot uses AI. Check for mistakes.
Comment on lines +68 to +69
public List<ChangeRequestResponseDTO> getAllChangeRequests() {
return changeRequestRepository.findAll().stream()
Copy link

Copilot AI Sep 29, 2025

Choose a reason for hiding this comment

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

Using findAll() without pagination could lead to performance issues if there are many change requests. Consider adding pagination parameters or implementing a limit to prevent loading excessive data into memory.

Suggested change
public List<ChangeRequestResponseDTO> getAllChangeRequests() {
return changeRequestRepository.findAll().stream()
public List<ChangeRequestResponseDTO> getAllChangeRequests(int page, int size) {
return changeRequestRepository.findAll(org.springframework.data.domain.PageRequest.of(page, size))
.stream()

Copilot uses AI. Check for mistakes.
Copy link
Member

@saokiritoni saokiritoni left a comment

Choose a reason for hiding this comment

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

수정 사항 한군데? 정도만 고치면 될 거 같습니다! (엔드포인트만) 굿굿 깔끔 ~

* 모든 변경 요청 목록 조회 (관리자용)
* 모든 상태의 ChangeRequest 목록을 반환합니다.
*/
@GetMapping("/all")
Copy link
Member

Choose a reason for hiding this comment

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

/all 말고 될수있으면 그냥 /로만 가는 게 좋을 거 같아요!

.newValue(changeRequest.getNewValue())
.reason(changeRequest.getReason())
.status(changeRequest.getStatus())
.adminComment(changeRequest.getAdminComment())
Copy link
Member

Choose a reason for hiding this comment

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

굿!

.map(ChangeRequestResponseDTO::fromEntity)
.collect(Collectors.toList());
}

Copy link
Member

Choose a reason for hiding this comment

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

일단 페이징은 없이가구 제가 시간 되면 공통 페이징 메서드 추가해보겠습니다!


/** 내 변경 요청 목록 조회 */
public List<ChangeRequestResponseDTO> getMyChangeRequests(Long userId) {
if (!userRepository.existsById(userId)) {
Copy link
Member

Choose a reason for hiding this comment

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

굿!

@saokiritoni saokiritoni merged commit ef56594 into develop Nov 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[feat] Add API that fetches all change requests

2 participants