Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

✨ [Feature] 지원자 목록 확인 API #804

Merged
merged 18 commits into from
Apr 5, 2024
Merged

✨ [Feature] 지원자 목록 확인 API #804

merged 18 commits into from
Apr 5, 2024

Conversation

middlefitting
Copy link
Member

📌 개요

  • 지원자 목록 확인 API

💻 작업사항

  • 지원자 목록 확인 API 기능 작성
  • 삭제시 자식 엔티티에 쿼리가 생성되지 않는 이슈로 ApplicationAnswer 자식에 존재하는 id 컬럼 제거

💡Issue 번호

@middlefitting middlefitting self-assigned this Apr 2, 2024
@middlefitting middlefitting added the enhancement New feature or request label Apr 2, 2024
@codecov-commenter
Copy link

codecov-commenter commented Apr 3, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.22%. Comparing base (c2dff0a) to head (264a5d5).

Additional details and impacted files 
@@                Coverage Diff                @@
##             recruit-dev     #804      +/-   ##
=================================================
- Coverage          75.72%   75.22%   -0.50%     
+ Complexity          1086     1085       -1     
=================================================
  Files                366      366              
  Lines               4787     4804      +17     
  Branches             390      397       +7     
=================================================
- Hits                3625     3614      -11     
- Misses               994     1013      +19     
- Partials             168      177       +9
Flag Coverage Δ
integrationTest 69.77% <100.00%> (-0.04%) ⬇️
unitTest 33.07% <100.00%> (-3.59%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Kimhan-nah
Kimhan-nah reviewed Apr 3, 2024
View reviewed changes
Copy link
Member

@Kimhan-nah Kimhan-nah left a comment

Choose a reason for hiding this comment

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

확인부탁드립니당

gg-data/src/main/java/gg/data/recruit/application/ApplicationAnswerText.java Show resolved Hide resolved
...admin/controller/response/GetRecruitmentApplicationDto$Form$CheckListForm$MapStructImpl.java Outdated Show resolved Hide resolved
gg-recruit-api/src/main/java/gg/recruit/api/admin/controller/RecruitmentAdminController.java Outdated Show resolved Hide resolved
gg-recruit-api/src/main/java/gg/recruit/api/admin/controller/RecruitmentAdminController.java Show resolved Hide resolved
wken5577
wken5577 reviewed Apr 3, 2024
View reviewed changes
gg-data/src/main/java/gg/data/recruit/application/ApplicationAnswerText.java
@@ -12,10 +11,6 @@
@NoArgsConstructor(access = lombok.AccessLevel.PROTECTED)
@DiscriminatorValue("TEXT")
public class ApplicationAnswerText extends ApplicationAnswer {

@Id
private Long id;
Copy link
Contributor

Choose a reason for hiding this comment

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

여기 ID빠지면 flyway도 수정해줘야 하지 않나요?? 여기 ID값이 있으면 applicationAnswer삭제시에 연결된 applicationAnswerText나 applicationAnswerCheckList에 삭제쿼리가 같이 안나간다는거죠?

Copy link
Member Author

@middlefitting middlefitting Apr 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

넵 그렇습니다 우선 id 컬럼 자체는 부모 엔티티에 정의되어 있으면 자동상속이 이루어지는것 같습니다. 문제가 되는 부분이라면 원래대로 롤백할게요. flyway는 동일하게 작동합니다

wken5577
wken5577 reviewed Apr 3, 2024
View reviewed changes
gg-data/src/main/java/gg/data/recruit/application/Application.java
@@ -44,6 +48,9 @@ public class Application extends BaseTimeEntity {
@Column(length = 15, nullable = false)
private ApplicationStatus status;

@OneToMany(mappedBy = "application", fetch = FetchType.LAZY)
private Set<ApplicationAnswer> applicationAnswers = new HashSet<>();
Copy link
Contributor

Choose a reason for hiding this comment

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

이건 set으로 하신 이유가 따로 있나요?

Copy link
Member Author

Choose a reason for hiding this comment

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

n+1 때문에 엔티티 그래프로 "applicationAnswers", "applicationAnswers.question.checkLists" 데이터를 조인하는 과정에서 여러 리스트 자료를 명시하는게 허용이 안되더라구요 checkLists는 이미 구현된 내용이고 applicationAnswers는 이번에 양방향을 추가한 내용이기 때문에 해당 부분을 set으로 대체했습니다.

@middlefitting middlefitting force-pushed the real-지원자목록-확인-API branch from 2de3f8b to 268410e Compare April 5, 2024 07:27
Kimhan-nah
Kimhan-nah previously approved these changes Apr 5, 2024
View reviewed changes
@middlefitting middlefitting force-pushed the real-지원자목록-확인-API branch from 264a5d5 to d17abe4 Compare April 5, 2024 11:59
@Kimhan-nah Kimhan-nah merged commit 66fb0cf into recruit-dev Apr 5, 2024
2 checks passed
@Kimhan-nah Kimhan-nah deleted the real-지원자목록-확인-API branch April 5, 2024 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kimhan-nah Kimhan-nah Kimhan-nah approved these changes

@wken5577 wken5577 Awaiting requested review from wken5577

Assignees

@middlefitting middlefitting

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@middlefitting @codecov-commenter @Kimhan-nah @wken5577