Fix: 후원자 정렬#136
Merged
Merged
Conversation
mjy926
commented
Apr 23, 2026
mjy926
commented
Contributor
- ruff isort 설정을 import문이 단일 라인을 유지하도록 수정했습니다.
- 후원자 목록 조회에 정렬, 연도별 필터링을 추가했습니다.
📝 WalkthroughWalkthrough후원자 조회 엔드포인트에 정렬( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
wacruit/src/apps/sponsor/repositories.py (2)
40-41:EXTRACT(YEAR ...)사용에 대한 운영 조언.
extract("year", Sponsor.sponsored_date) == year형태의 필터는sponsored_date컬럼에 인덱스가 있더라도 함수 적용으로 인해 인덱스를 사용하지 못할 가능성이 높습니다(DB 엔진에 따라 다름). 후원자 수가 많아질 경우를 대비해 다음 중 하나를 고려해 주세요.
- 범위 조건으로 전환:
date(year, 1, 1) <= sponsored_date < date(year+1, 1, 1)—sponsored_date인덱스 활용 가능.- 또는
sponsored_date에 함수 기반(expression) 인덱스 생성.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wacruit/src/apps/sponsor/repositories.py` around lines 40 - 41, The current filter using extract("year", Sponsor.sponsored_date) == year can prevent the DB from using an index on Sponsor.sponsored_date; change the filter in the repository (where the stmt is built, e.g., the stmt.where call referencing extract and Sponsor.sponsored_date) to a range comparison using start and end dates for the year (e.g., sponsored_date >= date(year,1,1) AND sponsored_date < date(year+1,1,1)) so the sponsorship_date index can be used, or alternatively note that you can create a function-based/expression index on extract(year, sponsored_date) if you prefer to keep the extract expression. Ensure you update the code paths that set the year condition (the block that checks if year is not None) to use the range logic or document index creation.
35-55: 정렬/필터 로직에 대한 소소한 개선 제안.현재 구현은 동작상 문제는 없습니다(
SponsorOrder가str서브클래스이므로order_map.get(order)조회가 정상 동작). 다만 다음을 고려해볼 수 있습니다.
order_map을 메서드 외부(모듈 상수)로 추출: 호출마다 dict를 재생성하지 않고, 매핑을SponsorOrderenum 멤버 키로 직접 만들면 문자열 리터럴 의존을 제거할 수 있습니다.- 명시적 키 사용:
order_map.get(order)대신order_map.get(order.value)또는 enum 멤버를 키로 사용하면,SponsorOrder가str을 상속하지 않게 리팩터링되더라도 깨지지 않습니다.- 방어적 처리 제거 가능:
order는 이미Optional[SponsorOrder]로 타입이 좁혀져 있으므로order_map에 모든 멤버가 존재하는 한sort_condition is not None체크는 불필요합니다(누락 시 코드로 바로 드러나는 편이 안전).- 기본 정렬 고려:
order가None일 때 결과 순서가 DB에 따라 비결정적일 수 있습니다. 페이지네이션 도입 시 이슈가 될 수 있으니Sponsor.id또는sponsored_date기준 기본 정렬을 두는 것을 권장합니다.♻️ 제안 diff
+_ORDER_MAP = { + SponsorOrder.AMOUNT_ASC: Sponsor.amount.asc(), + SponsorOrder.AMOUNT_DESC: Sponsor.amount.desc(), + SponsorOrder.DATE_ASC: Sponsor.sponsored_date.asc(), + SponsorOrder.DATE_DESC: Sponsor.sponsored_date.desc(), +} + def get_all_sponsors( self, order: Optional[SponsorOrder] = None, year: Optional[int] = None ) -> Sequence[Sponsor]: stmt = select(Sponsor) if year is not None: stmt = stmt.where(extract("year", Sponsor.sponsored_date) == year) if order is not None: - order_map = { - "amount": Sponsor.amount.asc(), - "-amount": Sponsor.amount.desc(), - "date": Sponsor.sponsored_date.asc(), - "-date": Sponsor.sponsored_date.desc(), - } - - sort_condition = order_map.get(order) - if sort_condition is not None: - stmt = stmt.order_by(sort_condition) + stmt = stmt.order_by(_ORDER_MAP[order]) + else: + stmt = stmt.order_by(Sponsor.id.asc()) return self.session.execute(stmt).scalars().all()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wacruit/src/apps/sponsor/repositories.py` around lines 35 - 55, Refactor get_all_sponsors to move the order_map out of the function into a module-level constant keyed by SponsorOrder enum members (or their .value) instead of string literals, update the lookup to use the enum member (or order.value) so it won't break if SponsorOrder stops inheriting str, remove the redundant sort_condition is not None defensive check (since order is typed Optional[SponsorOrder] and the map should cover all members), and add a deterministic default order_by (e.g., Sponsor.id.asc() or Sponsor.sponsored_date.asc()) when order is None to guarantee stable results for pagination.wacruit/src/apps/sponsor/views.py (1)
41-48: 쿼리 파라미터 설명 문자열이 실제 enum 값과 불일치합니다.
SponsorOrderenum의 멤버 이름은AMOUNT_ASC/AMOUNT_DESC/DATE_ASC/DATE_DESC이지만, FastAPI/Pydantic은str서브클래스 Enum을 값("amount","-amount","date","-date")으로 직렬화/역직렬화합니다. 따라서 현재 description은 값 기준으로 올바르게 작성되어 있어 일관적이지만, OpenAPI 스키마에는 enum 값이 이미 노출되므로 description에서 예시를 중복 나열하기보다는"'-' prefix means descending order"정도로 간결하게 정리하는 것이 더 유지보수하기 좋습니다.또한
year에 대해 합리적 범위 검증(ge=2000,le=current_year등)을 추가하면 잘못된 입력(예: 음수, 5자리 연도)으로 인해 불필요한 DB 쿼리가 실행되는 것을 방지할 수 있습니다.♻️ 제안 예시
- year: Annotated[Optional[int], Query(description="sponsors by year")] = None, + year: Annotated[ + Optional[int], + Query(description="filter by sponsored year", ge=1900, le=2100), + ] = None,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wacruit/src/apps/sponsor/views.py` around lines 41 - 48, description for the "order" query should be simplified to avoid duplicating enum values: update the Query description for the order parameter (Annotated Optional[SponsorOrder], named order) to something like "'-' prefix means descending order" instead of listing example enum values; also add range validation to the year parameter (Annotated[Optional[int], Query(...)], named year) by adding ge=2000 and le=current_year (compute current_year at runtime) to the Query so invalid years are rejected before DB queries run.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@wacruit/src/apps/sponsor/repositories.py`:
- Around line 40-41: The current filter using extract("year",
Sponsor.sponsored_date) == year can prevent the DB from using an index on
Sponsor.sponsored_date; change the filter in the repository (where the stmt is
built, e.g., the stmt.where call referencing extract and Sponsor.sponsored_date)
to a range comparison using start and end dates for the year (e.g.,
sponsored_date >= date(year,1,1) AND sponsored_date < date(year+1,1,1)) so the
sponsorship_date index can be used, or alternatively note that you can create a
function-based/expression index on extract(year, sponsored_date) if you prefer
to keep the extract expression. Ensure you update the code paths that set the
year condition (the block that checks if year is not None) to use the range
logic or document index creation.
- Around line 35-55: Refactor get_all_sponsors to move the order_map out of the
function into a module-level constant keyed by SponsorOrder enum members (or
their .value) instead of string literals, update the lookup to use the enum
member (or order.value) so it won't break if SponsorOrder stops inheriting str,
remove the redundant sort_condition is not None defensive check (since order is
typed Optional[SponsorOrder] and the map should cover all members), and add a
deterministic default order_by (e.g., Sponsor.id.asc() or
Sponsor.sponsored_date.asc()) when order is None to guarantee stable results for
pagination.
In `@wacruit/src/apps/sponsor/views.py`:
- Around line 41-48: description for the "order" query should be simplified to
avoid duplicating enum values: update the Query description for the order
parameter (Annotated Optional[SponsorOrder], named order) to something like "'-'
prefix means descending order" instead of listing example enum values; also add
range validation to the year parameter (Annotated[Optional[int], Query(...)],
named year) by adding ge=2000 and le=current_year (compute current_year at
runtime) to the Query so invalid years are rejected before DB queries run.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: eac3af9e-f537-4c36-928c-e5ccc1848d03
📒 Files selected for processing (6)
pyproject.tomlwacruit/src/apps/common/enums.pywacruit/src/apps/sponsor/repositories.pywacruit/src/apps/sponsor/schemas.pywacruit/src/apps/sponsor/services.pywacruit/src/apps/sponsor/views.py
snu-BuMinSeong
approved these changes
Apr 24, 2026
mjy926
added a commit
that referenced
this pull request
Apr 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.