Skip to content

[TSK-56-147] 고전독서인증 진행도 계산 시 기준 초과 도서 제외#299

Merged
goldm0ng merged 11 commits intomainfrom
2Jin1031/TSK-56-147
Mar 8, 2026
Merged

[TSK-56-147] 고전독서인증 진행도 계산 시 기준 초과 도서 제외#299
goldm0ng merged 11 commits intomainfrom
2Jin1031/TSK-56-147

Conversation

@2Jin1031
Copy link
Copy Markdown
Contributor

@2Jin1031 2Jin1031 commented Mar 4, 2026

Summary

고전독서인증 진행도 계산 시 각 영역별 최대 인정 권수를 초과한 도서를 제외하도록 개선했습니다. 학교 시스템에서 "7/4권"처럼 기준을 초과한 데이터가 와도 자동으로 최대 인정 권수(4권)로 제한됩니다.

Changes

  1. ClassicsArea enum 추가
  • 4개 고전독서 영역을 enum으로 관리 (서양/동양/동서양문학/과학)
  • 각 영역별 최대 인정 권수(maxRecognizedCount) 정의
    • 서양의 역사와 사상: 4권
    • 동양의 역사와 사상: 2권
    • 동·서양의 문학: 3권
    • 과학 사상: 1권
  • findByLabel(String): 라벨로 영역 검색 (Optional 반환)
  • getRecognizedCount(int): 실제 권수를 최대 인정 권수로 제한
  1. GraduationClassicsCertFetcher 리팩토링
  • "X/Y권" 형태 파싱 지원: "5/4권"에서 완료된 권수(5) 추출
  • EnumMap 활용: if-else 체인을 제거하고 enum 기반으로 개선
  • 최대 권수 제한 로직 추가: 실제 권수가 최대 인정 권수를 초과하면 자동으로 제한
    • 예: 서양역사와사상 5권 → 4권으로 제한

3. 테스트 추가

  • ClassicsAreaTest: enum 기본 기능 및 최대값 제한 로직 검증
  • GraduationClassicsCertFetcherTest:
    • "X/Y권" 및 "X권" 형태 파싱
    • 알 수 없는 영역 무시
    • 숫자가 아닌 값 처리 (N/A, - 등)
    • 최대 인정 권수 초과 시 제한 적용

Test plan

  • ClassicsAreaTest 전체 통과 (10개 테스트)
  • GraduationClassicsCertFetcherTest 전체 통과 (6개 테스트)
  • 기존 고전독서 인증 관련 테스트 통과

Before & After

Before

  • 학교 시스템에서 "7/4권"을 가져오면 7권으로 저장
  • 실제 기준(4권)을 초과한 도서도 모두 카운트에 포함

After

  • 학교 시스템에서 "7/4권"을 가져오면 최대 인정 권수인 4권으로 자동 제한
  • 사용자 흐름에 부합하는 정확한 진행도 표시 (8/10 → 7/10)

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 4, 2026

Test Results

187 tests   187 ✅  25s ⏱️
 43 suites    0 💤
 43 files      0 ❌

Results for commit 93c602e.

♻️ This comment has been updated with latest results.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bada53c949

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +12 to +15
WESTERN("서양의 역사와 사상", 4),
EASTERN("동양의 역사와 사상", 2),
EASTERN_AND_WESTERN("동·서양의 문학", 3),
SCIENCE("과학 사상", 1);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Derive recognized-count caps from criteria instead of constants

Avoid hard-coding per-area caps here, because this commit now truncates fetched counts to 4/2/3/1 before GraduationCertService evaluates completion against admission-year criteria loaded from DB (GraduationCertService#createOrUpdate), and those criteria are synced from sheet data (AdminGraduationSyncService#syncClassicCertCriteria). If any year’s required counts change (e.g., western > 4), affected students will be permanently undercounted and may be marked as not satisfied even when the source system reports enough books.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member

@boyekim boyekim left a comment

Choose a reason for hiding this comment

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

리뷰 전에 하나 여쭤보고싶은게 있어요!!
사용자에게 밑 사진에서 보이는 "시험 통과 권수"도 보정되어서 보이게 될 것 같은데 맞을까요?!
Image

저는 사용자가 보는 시험 통과 개수는 초과한건 초과한대로 보이고, "인증 권수"만 보정이 되는게 훨씬 사용자에게 필요한 정보라고 생각하는데요, 어떻게 생각하시나요??

@2Jin1031
Copy link
Copy Markdown
Contributor Author

2Jin1031 commented Mar 5, 2026

@boyekim

저는 사용자가 보는 시험 통과 개수는 초과한건 초과한대로 보이고, "인증 권수"만 보정이 되는게 훨씬 사용자에게 필요한 정보라고 생각하는데요, 어떻게 생각하시나요??
- 참조

이 방식이 혼선을 줄일 수 있을 것 같네요~ 말씀해주신 부분을 적용하여 수정했습니다!!
감사합니다!!

@2Jin1031
Copy link
Copy Markdown
Contributor Author

2Jin1031 commented Mar 5, 2026

모두 한번 확인해주세요!!
덱스씨의 리뷰를 보고 논의 지점이라는 생각이 들었습니다!

리뷰 요약
고전 독서 기준 권 수가 두 곳에서 관리되고 있다 : enum, DB
하나를 선택할 것

개인적으로는 아래 이유로 enum 방식이 좀 더 적절하지 않을까 생각하고 있어요!

  1. 현재 로직 기준으로 학번 분기가 없고, 실제로 값이 바뀐 이력도 없어서 DB로 관리할 이점이 크지 않을 것 같습니다.
  2. 영역 분류 자체가 바뀌면 로직도 함께 바뀌어야 하는 값이라, 코드 안에서 함께 관리하는 게 더 자연스러울 것 같기도 하고요.

물론 다른 의견도 언제나 환영입니다! 논의 후 하나로 정리하고 머지하겠습니다 😊

@boyekim
Copy link
Copy Markdown
Member

boyekim commented Mar 5, 2026

요 부분은 금서와 해윤이의 의견에 더 중점을 두긴해야겠네요.
우선 저도 Enum관리에 한표이긴 합니다.
구글 시트 관리 비용도 줄고, 학번마다 추가해줄 필요가 없을 것 같긴해요. 구글 시트로 관리한다는 것의 장점은 다른 인증 요건들과 똑같은 구성으로 관리할 수 있다는 것이 생각나는데, 전 그래도 DB 안타는 것의 이점이 더 클 것 같습니다.

@goldm0ng
Copy link
Copy Markdown
Member

goldm0ng commented Mar 6, 2026

모두 한번 확인해주세요!!
덱스씨의 리뷰를 보고 논의 지점이라는 생각이 들었습니다!

리뷰 요약
고전 독서 기준 권 수가 두 곳에서 관리되고 있다 : enum, DB
하나를 선택할 것

개인적으로는 아래 이유로 enum 방식이 좀 더 적절하지 않을까 생각하고 있어요!

현재 로직 기준으로 학번 분기가 없고, 실제로 값이 바뀐 이력도 없어서 DB로 관리할 이점이 크지 않을 것 같습니다.
영역 분류 자체가 바뀌면 로직도 함께 바뀌어야 하는 값이라, 코드 안에서 함께 관리하는 게 더 자연스러울 것 같기도 하고요.
물론 다른 의견도 언제나 환영입니다! 논의 후 하나로 정리하고 머지하겠습니다 😊

저도 수민님의 의견과 동일합니다!
영어인증이나 코딩인증과 같은 졸업인증 기준 데이터들을 구글 시트에서 관리하고 있으니 일관성 있게 구글 시트에서 관리하는 것도 좋아보이지만,
입학연도에 따라 달라지는 기준이 아니다보니 ENUM으로 관리해도 괜찮을 것 같습니다!
-> 불필요한 DB 조회를 안 해도 되니까 굿일거같아요.
근데 혹시혹시혹시나 추후에 입학연도에 따라 고전독서 기준 데이터에 대한 정책이 바뀌거나 하지 않을지 걱정이 되긴 합니다!!
이 부분에 대해서는 어떻게 생각하시나요~?

Copy link
Copy Markdown
Member

@goldm0ng goldm0ng 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 +79 to +82
if (text.contains("/")) {
String completedPart = text.split("/")[0].trim();
return Integer.parseInt(completedPart);
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

이 메서드는 대휴칼에서 파싱하는 로직인 걸로 이해했는데,
혹시 해당 로직이 필요한 이유가 있을까요?

n/n 와 같은 포맷이 보이지 않아서요!
현재로서는, n권에 대해서 n을 파싱하는 것으로 보입니다!
혹시 제가 모르는 부분이 있다면 말씀해주세용

Image

Copy link
Copy Markdown
Member

@goldm0ng goldm0ng left a comment

Choose a reason for hiding this comment

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

수고 많으셨어요!!
한가지 코멘트 남겼는데 의견 및 반영 부탁드립니다 ~!
그리고 CI가 터지네용
좀만 힘내십쇼..!!!!
👏🏻

CodingCertCriteriaResponse codingCriteria = buildCodingCriteria(admissionYear, codingTargetType);

return GraduationCertCriteriaResponse.of(criteriaTarget, certPolicy, englishCriteria, classicCriteria,
return GraduationCertCriteriaResponse.of(criteriaTarget, certPolicy, englishCriteria,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

아래 사진에서 사용되는 부분이 해당 졸업인증 기준 데이터 조회 API 응답의 고전독서 부분에서 가져오는 걸로 알고 있는데요!
따라서, 고전독서 기준데이터를 내려주는 건 필요하다고 생각합니다!
기존 DB에서 가져와서 응답을 만드는 로직에서 ENUM 활용 로직으로 충분히 내려줄 수 있지 안을까용? 어떻게 생각하시는지 궁금합니다!

Image

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

헉 왜 response 필드도 지웠지
꼼꼼히 확인해주셔서 감사합니다!! 한번 더 물어보길 잘햇다

Copy link
Copy Markdown
Member

@goldm0ng goldm0ng left a comment

Choose a reason for hiding this comment

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

확인했습니다!!
머지 후 배포하겠습니당

@goldm0ng goldm0ng merged commit ad925e6 into main Mar 8, 2026
3 checks passed
@goldm0ng goldm0ng deleted the 2Jin1031/TSK-56-147 branch March 8, 2026 06:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants