Skip to content

Conversation

@jwnnoh
Copy link
Member

@jwnnoh jwnnoh commented Dec 30, 2025

1. 무슨 이유로 코드를 변경했나요?

  • 사이트 내에서 동아리 지원서 상태 조회 기능을 구현했어요.
  • 로그인한 사용자 정보를 기준으로 지원서 상태 정보를 가져와요.

2. 어떤 위험이나 장애를 발견했나요?


3. 관련 스크린샷을 첨부해주세요.

image

4. 완료 사항

close #85

5. 추가 사항

@jwnnoh jwnnoh self-assigned this Dec 30, 2025
@jwnnoh jwnnoh linked an issue Dec 30, 2025 that may be closed by this pull request
@jwnnoh jwnnoh requested a review from Copilot December 30, 2025 03:55
Copy link

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 implements a feature for users to check their application (합불) status within the site. The implementation retrieves application status information based on the authenticated user's credentials and conditionally includes interview details when the application status is PASS_PAPER.

  • Adds a new /application/status GET endpoint for authenticated users
  • Implements conditional response logic that includes interview details only for paper screening pass status
  • Includes comprehensive unit tests covering all application status scenarios

Reviewed changes

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

Show a summary per file
File Description
src/main/kotlin/land/leets/domain/application/usecase/GetApplicationStatus.kt Defines the interface for retrieving application status by user UUID
src/main/kotlin/land/leets/domain/application/usecase/GetApplicationStatusImpl.kt Implements the application status retrieval use case with repository lookup and exception handling
src/main/kotlin/land/leets/domain/application/presentation/dto/ApplicationStatusResponse.kt DTO that conditionally includes interview information based on application status
src/main/kotlin/land/leets/domain/application/presentation/ApplicationController.kt Adds new /status endpoint with OpenAPI documentation
src/test/kotlin/land/leets/domain/application/usecase/GetApplicationStatusImplTest.kt Comprehensive test coverage for all status scenarios and error cases

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +16 to +18
if (application.applicationStatus != ApplicationStatus.PASS_PAPER) {
return ApplicationStatusResponse(
id = application.id!!,
Copy link

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

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

The conditional logic for returning interview information based on application status would benefit from a comment explaining the business rule. This would help future maintainers understand why interview details are only included for PASS_PAPER status.

Consider adding a brief comment like: "// Interview details are only shown to applicants who passed the paper screening"

Suggested change
if (application.applicationStatus != ApplicationStatus.PASS_PAPER) {
return ApplicationStatusResponse(
id = application.id!!,
// Interview details are only shown to applicants who passed the paper screening
if (application.applicationStatus != ApplicationStatus.PASS_PAPER) {
return ApplicationStatusResponse(

Copilot uses AI. Check for mistakes.
Comment on lines 18 to 25
id = application.id!!,
status = application.applicationStatus,
interviewDay = null,
interviewTime = null,
)
}
return ApplicationStatusResponse(
id = application.id!!,
Copy link

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

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

The use of the non-null assertion operator (!!) on application.id could throw a NullPointerException if the application entity hasn't been persisted yet. While this is unlikely in this context since we're fetching from the repository, consider using a safer approach or document the assumption that the application will always have an ID at this point.

A safer alternative would be to use the elvis operator with a meaningful exception: id = application.id ?: throw IllegalStateException("Application must have an ID")

Suggested change
id = application.id!!,
status = application.applicationStatus,
interviewDay = null,
interviewTime = null,
)
}
return ApplicationStatusResponse(
id = application.id!!,
id = application.id ?: throw IllegalStateException("Application must have an ID"),
status = application.applicationStatus,
interviewDay = null,
interviewTime = null,
)
}
return ApplicationStatusResponse(
id = application.id ?: throw IllegalStateException("Application must have an ID"),

Copilot uses AI. Check for mistakes.
Comment on lines 16 to 25
if (application.applicationStatus != ApplicationStatus.PASS_PAPER) {
return ApplicationStatusResponse(
id = application.id!!,
status = application.applicationStatus,
interviewDay = null,
interviewTime = null,
)
}
return ApplicationStatusResponse(
id = application.id!!,
Copy link

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

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

The same non-null assertion operator (!!) issue appears here. See the comment on line 18 for the recommended safer approach.

Suggested change
if (application.applicationStatus != ApplicationStatus.PASS_PAPER) {
return ApplicationStatusResponse(
id = application.id!!,
status = application.applicationStatus,
interviewDay = null,
interviewTime = null,
)
}
return ApplicationStatusResponse(
id = application.id!!,
val applicationId = application.id
?: throw IllegalStateException("Application id must not be null")
if (application.applicationStatus != ApplicationStatus.PASS_PAPER) {
return ApplicationStatusResponse(
id = applicationId,
status = application.applicationStatus,
interviewDay = null,
interviewTime = null,
)
}
return ApplicationStatusResponse(
id = applicationId,

Copilot uses AI. Check for mistakes.
Comment on lines 9 to 23
val interviewDay: String?,
val interviewTime: String?,
) {
companion object {
fun from(
application: Application,
): ApplicationStatusResponse {
if (application.applicationStatus != ApplicationStatus.PASS_PAPER) {
return ApplicationStatusResponse(
id = application.id!!,
status = application.applicationStatus,
interviewDay = null,
interviewTime = null,
)
}
Copy link

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

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

The Application entity defines interviewDay and interviewTime as non-nullable fields (nullable = false), but this response conditionally returns null for these fields when the status is not PASS_PAPER. This creates a data model inconsistency.

Since all applications must have interviewDay and interviewTime values (as per the entity definition), consider one of the following approaches:

  1. Always return the interview information regardless of status, since it exists in the database
  2. If interview information should only be shown for PASS_PAPER status, make these fields nullable in the Application entity and update the database schema accordingly

The current implementation assumes these fields can be null conditionally, but the entity enforces them as required.

Suggested change
val interviewDay: String?,
val interviewTime: String?,
) {
companion object {
fun from(
application: Application,
): ApplicationStatusResponse {
if (application.applicationStatus != ApplicationStatus.PASS_PAPER) {
return ApplicationStatusResponse(
id = application.id!!,
status = application.applicationStatus,
interviewDay = null,
interviewTime = null,
)
}
val interviewDay: String,
val interviewTime: String,
) {
companion object {
fun from(
application: Application,
): ApplicationStatusResponse {

Copilot uses AI. Check for mistakes.
Copy link
Member

@rootTiket rootTiket left a comment

Choose a reason for hiding this comment

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

두 군데 개행 누락된 부분 제외하고는 큰 문제 없어보입니다~ 고생하셨습니다!!


interface GetApplicationStatus {
fun execute(uid: UUID): ApplicationStatusResponse
} No newline at end of file
Copy link
Member

Choose a reason for hiding this comment

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

개행 부탁드립니당 👀


return ApplicationStatusResponse.from(application)
}
} No newline at end of file
Copy link
Member

Choose a reason for hiding this comment

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

👀

Copy link
Member

@yechan-kim yechan-kim left a comment

Choose a reason for hiding this comment

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

정완님 고생 많으셨습니다. 코멘트 확인 부탁드려요~

Comment on lines 16 to 30
if (application.applicationStatus != ApplicationStatus.PASS_PAPER) {
return ApplicationStatusResponse(
id = application.id!!,
status = application.applicationStatus,
interviewDay = null,
interviewTime = null,
)
}
return ApplicationStatusResponse(
id = application.id!!,
status = application.applicationStatus,
interviewDay = application.interviewDay,
interviewTime = application.interviewTime,
)
}
Copy link
Member

Choose a reason for hiding this comment

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

저는 현재 로직을 서류 합격 상태일 경우에는 인터뷰 장소와 날짜를 반환하고,
그렇지 않으면 지원 상태(서탈, 최합, 최탈)만 반환하는 구조로 이해했습니다.

다만 저희 서비스에서는
Application엔티티는 지원자가 희망한 면접 정보를 가지고 있고,
운영진이 최종적으로 통보하는 면접 정보는 Interview 엔티티에서 관리하는 것으로 알고 있습니다.

따라서 인터뷰에 대한 정보는 Application 엔티티가 아닌 Interview 엔티티에서 가지고 오는 것이 옳은 로직이라고 생각하는데, 정완님의 의견이 궁금합니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

오아 제가 해당 부분은 완전히 착각하고 코드를 작성해버렸네요..캐치 감사합니다..!!
interview 엔티티에서 정보를 가져오는게 정답이네요 ㅎㅎ

수정하겠습니다 ❤️

@jwnnoh jwnnoh requested a review from yechan-kim December 31, 2025 09:12
Copy link
Member

@yechan-kim yechan-kim left a comment

Choose a reason for hiding this comment

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

고생하셨습니다☺️

@jwnnoh jwnnoh merged commit d59da23 into main Dec 31, 2025
1 check passed
@jwnnoh jwnnoh deleted the feat/#85 branch December 31, 2025 09:22
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] 합불 결과 조회 기능 구현

4 participants