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.

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

[BE] refactor: 기존 api가 리뷰 그룹 정보를 포함하도록 수정 #1078

Merged
merged 9 commits into from
Feb 11, 2025

Conversation

nayonsoso
Copy link
Contributor

@nayonsoso nayonsoso commented Jan 31, 2025


🚀 어떤 기능을 구현했나요 ?

  • 기존 api 에 리뷰 그룹 정보가 담기도록 했습니다.

🔥 어떻게 해결했나요 ?

  • 회의 내용을 바탕으로 구현하고 다시 PR 열었습니다 ㅎㅎ

📝 어떤 부분에 집중해서 리뷰해야 할까요?

  • ❗️DefaultTemplateService 괜찮나요?❗️

Copy link

github-actions bot commented Jan 31, 2025

Test Results

0 tests   - 157   0 ✅  - 154   0s ⏱️ -4s
0 suites  -  59   0 💤  -   3 
0 files    -  59   0 ❌ ±  0 

Results for commit bc05aac. ± Comparison against base commit 4248c38.

♻️ This comment has been updated with latest results.

Comment on lines 28 to +27
@GetMapping("/v2/groups/summary")
public ResponseEntity<ReviewGroupResponse> getReviewGroupSummary(@RequestParam String reviewRequestCode) {
ReviewGroupResponse response = reviewGroupLookupService.getReviewGroupSummary(reviewRequestCode);
public ResponseEntity<ReviewGroupSummaryResponse> getReviewGroupSummary(@RequestParam String reviewRequestCode) {
ReviewGroupSummaryResponse response = reviewGroupLookupService.getReviewGroupSummary(reviewRequestCode);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

회의에서

  • 이 api 도 reviewGroupId 로 판별할까?🤔
  • 리리코로 리뷰 그룹 정보를 식별해오는게 통일도 안된 것 같고 애매하긴 한데,..
    호환성의 문제도 있고 복잡하니 일단 api 는 그대로 두고 response 이름만 변경하자! 😇

라고 했어서 반영했습니다~

Comment on lines 16 to +18
@GetMapping("/v2/sections")
public ResponseEntity<SectionNamesResponse> getSectionNames(
@ReviewGroupSession ReviewGroup reviewGroup
) {
SectionNamesResponse sectionNames = templateService.getSectionNames(reviewGroup);
public ResponseEntity<SectionNamesResponse> getSectionNames() {
SectionNamesResponse sectionNames = templateService.getSectionNames();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

이 부분은 사실 회의에서 /templates/{templateId}/sections로 하자고 했었는데,
왜 테드가 기존 api 그대로 두고 고정 응답을 하자고 했는지 코드를 보면서 깨달았어요 ㅠ
templateId 를 받게 해봤자, templateId 를 지정해서 생성하는 곳도 없고, 응답으로 내려주는 곳도 없고...
지금 변경을 하더라도 맥락에 안맞는 변경 같더라고요😅

그래서 기존 내용대로 그대로 뒀습니다!
대신 defaultTemplate 관련된 내용이 응집도있게 존재할 수 있도록
DefaultTemplateService 를 만들어봤어요.

@nayonsoso nayonsoso force-pushed the be/refactor/1048-add-group-info-to-api branch from 0418229 to 63222d8 Compare February 10, 2025 15:46
@nayonsoso nayonsoso changed the title [BE] docs: 기존 api가 리뷰 그룹 정보를 포함하도록 수정 [BE] refactor: 기존 api가 리뷰 그룹 정보를 포함하도록 수정 Feb 10, 2025
Copy link
Contributor

@Kimprodp Kimprodp left a comment

Choose a reason for hiding this comment

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

전체적으로 괜찮은 것 같은데, 선푸룹 하고! 리베이스 후 구현하면서 더 자세히 볼게요

@nayonsoso nayonsoso force-pushed the be/refactor/1048-add-group-info-to-api branch from 63222d8 to bc05aac Compare February 11, 2025 14:04
@nayonsoso nayonsoso merged commit 073cec3 into develop Feb 11, 2025
1 check passed
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.

[BE] 회원 개념 도입으로 인해 변경된 것을 기존 api 반영한다.
2 participants