fix(appapi): map rankings zone name to number and fix period parameter - #13
Conversation
Reviewer's Guide将文本排名区域映射到数值 API 标识符,通过重命名后的辅助函数集中管理排名周期映射,将 CLI 标志和 SDK 接入新的映射逻辑,并添加针对电影和回放排名的查询参数映射的测试,这些测试会对接一个用于验证的 HTTP 测试服务器。 带区域和周期映射的排名请求时序图sequenceDiagram
actor User
participant CLI as rankings_cmd
participant SDK as sdk.Client
participant AppAPI as appapi.Client
participant JavDB as JavDB_App_API
User->>CLI: run javdb rankings movies --type fc2 --period day
CLI->>SDK: RankingsMovies(ctx,"fc2","day")
SDK->>AppAPI: RankingsMovies("fc2","day")
AppAPI->>AppAPI: RankingPeriod("day")
AppAPI->>AppAPI: Zones["fc2"]
AppAPI->>JavDB: GetJSON("/api/v1/rankings",{"type":strconv.Itoa(Zones["fc2"]),"period":RankingPeriod("day")})
User->>CLI: run javdb rankings playback --filter-by western --period month
CLI->>SDK: RankingsPlayback(ctx,"western","month")
SDK->>AppAPI: RankingsPlayback("western","month")
AppAPI->>AppAPI: RankingPeriod("month")
AppAPI->>AppAPI: Zones["western"]
AppAPI->>JavDB: GetJSON("/api/v1/rankings/playback",{"filter_by":strconv.Itoa(Zones["western"]),"period":RankingPeriod("month")})
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your Experience访问你的 dashboard 来:
Getting HelpOriginal review guide in EnglishReviewer's GuideMaps textual rankings zones to numeric API identifiers, centralizes rankings period mapping via a renamed helper, wires CLI flags and SDK to use the new mappings, and adds tests that verify query parameter mapping for movie and playback rankings against a test HTTP server. Sequence diagram for rankings requests with zone and period mappingsequenceDiagram
actor User
participant CLI as rankings_cmd
participant SDK as sdk.Client
participant AppAPI as appapi.Client
participant JavDB as JavDB_App_API
User->>CLI: run javdb rankings movies --type fc2 --period day
CLI->>SDK: RankingsMovies(ctx,"fc2","day")
SDK->>AppAPI: RankingsMovies("fc2","day")
AppAPI->>AppAPI: RankingPeriod("day")
AppAPI->>AppAPI: Zones["fc2"]
AppAPI->>JavDB: GetJSON("/api/v1/rankings",{"type":strconv.Itoa(Zones["fc2"]),"period":RankingPeriod("day")})
User->>CLI: run javdb rankings playback --filter-by western --period month
CLI->>SDK: RankingsPlayback(ctx,"western","month")
SDK->>AppAPI: RankingsPlayback("western","month")
AppAPI->>AppAPI: RankingPeriod("month")
AppAPI->>AppAPI: Zones["western"]
AppAPI->>JavDB: GetJSON("/api/v1/rankings/playback",{"filter_by":strconv.Itoa(Zones["western"]),"period":RankingPeriod("month")})
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Review limit reached
Next review available in: 54 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthrough本次变更统一榜单周期辅助函数名称,转换电影和播放榜单的区域筛选参数,并在 CLI 帮助中加入 Changes榜单参数统一
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
There was a problem hiding this comment.
Hey - 我发现了两个问题,并给出了一些高层次的反馈:
- 目前 CLI 会调用
javdb.RankingPeriod,而 app API client 在内部也会应用RankingPeriod,这会让 period 的映射变得重复;建议在 CLI 层传递原始的 period 值,并把映射逻辑集中到同一层来处理。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- 目前 CLI 会调用 `javdb.RankingPeriod`,而 app API client 在内部也会应用 `RankingPeriod`,这会让 period 的映射变得重复;建议在 CLI 层传递原始的 period 值,并把映射逻辑集中到同一层来处理。
## Individual Comments
### Comment 1
<location path="internal/javdb/appapi/rankings.go" line_range="64-65" />
<code_context>
// RankingsMovies GET /api/v1/rankings
func (c *Client) RankingsMovies(type_, period string) (SearchResult, error) {
+ t := type_
+ if z, ok := Zones[type_]; ok {
+ t = strconv.Itoa(z)
+ }
</code_context>
<issue_to_address>
**suggestion:** 针对 `type_` 的 Zone 到 int 映射存在重复实现,可以集中处理。
这里的 `type_` → `Zones` → `strconv.Itoa` 逻辑与 `RankingsPlayback` 中对 `filterBy` 的处理一致。请提取一个共享的辅助函数(例如 `normalizeZone(string) string`),并在两个地方都使用它,以消除重复代码,并确保当映射发生变化时,区域归一化逻辑保持一致。
建议实现:
```golang
func normalizeZone(zone string) string {
if z, ok := Zones[zone]; ok {
return strconv.Itoa(z)
}
return zone
}
// RankingsMovies GET /api/v1/rankings
func (c *Client) RankingsMovies(type_, period string) (SearchResult, error) {
t := normalizeZone(type_)
```
```golang
var data map[string]json.RawMessage
if err := c.GetJSON("/api/v1/rankings", map[string]string{
"type": t, "period": RankingPeriod(period),
}, &data); err != nil {
return nil, err
}
func (c *Client) RankingsActors(period string) (SearchResult, error) {
var data map[string]json.RawMessage
```
1. 在同一个文件中,将 `RankingsPlayback` 更新为使用 `normalizeZone` 处理其 `filterBy`(或等价)逻辑,用对 `normalizeZone(...)` 的调用替换重复的 `if z, ok := Zones[...] { strconv.Itoa(z) }` 代码块。
2. 确保在 `internal/javdb/appapi/rankings.go` 文件顶部导入了 `strconv`(如果尚未导入),因为 `normalizeZone` 依赖 `strconv.Itoa`。
</issue_to_address>
### Comment 2
<location path="internal/cli/rankings_cmd.go" line_range="102" />
<code_context>
return err
}
- res, err := c.RankingsActors(context.Background(), javdb.ActorPeriod(period))
+ res, err := c.RankingsActors(context.Background(), javdb.RankingPeriod(period))
if err != nil {
return fmt.Errorf("rankings failed: %w", err)
</code_context>
<issue_to_address>
**suggestion:** 当 `RankingsActors` 本身也会对 period 做归一化时,从 CLI 传入 `RankingPeriod(period)` 可能会让人困惑。
CLI 已经通过 `javdb.RankingPeriod` 对 `period` 做了归一化,而 `RankingsActors` 在内部也会应用 `RankingPeriod`。这种重复归一化会让各层期望的 period 格式(`day|week|month` vs. `daily|weekly|monthly`)变得不清晰。请合并这一职责——要么在 actors 的 CLI 层去掉归一化,要么在 app API 层移除归一化,并清晰地文档化每一层期望的 `period` 格式。
建议实现:
```golang
// CLI 传入原始 period("day|week|month"),RankingsActors
// 在内部通过 javdb.RankingPeriod 负责归一化。
res, err := c.RankingsActors(context.Background(), period)
```
1. 确保 `RankingsActors` 在内部调用 `javdb.RankingPeriod`(或等价逻辑),这样归一化职责就明确地归属于 app API 这一层。
2. 如果其他 CLI 命令(例如电影排行榜)也会把 `javdb.RankingPeriod(period)` 传入在 app 层自身会做归一化的排行榜函数,请在这些地方做同样的调整以保持一致性。
3. 考虑在 `RankingsActors` 以及 CLI 的 `period` 标志上添加简短的文档注释,说明 CLI 使用 `day|week|month`,而 app 层负责转换为 `daily|weekly|monthly`。
</issue_to_address>帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈改进后续的代码审查。
Original comment in English
Hey - I've found 2 issues, and left some high level feedback:
- The CLI now calls
javdb.RankingPeriodwhile the app API client also appliesRankingPeriodinternally, which makes the period mapping redundant; consider passing the raw CLI value through and centralizing the mapping in one layer.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The CLI now calls `javdb.RankingPeriod` while the app API client also applies `RankingPeriod` internally, which makes the period mapping redundant; consider passing the raw CLI value through and centralizing the mapping in one layer.
## Individual Comments
### Comment 1
<location path="internal/javdb/appapi/rankings.go" line_range="64-65" />
<code_context>
// RankingsMovies GET /api/v1/rankings
func (c *Client) RankingsMovies(type_, period string) (SearchResult, error) {
+ t := type_
+ if z, ok := Zones[type_]; ok {
+ t = strconv.Itoa(z)
+ }
</code_context>
<issue_to_address>
**suggestion:** Zone-to-int mapping for `type_` is duplicated and could be centralized.
The `type_` → `Zones` → `strconv.Itoa` logic here mirrors the `filterBy` handling in `RankingsPlayback`. Please extract a shared helper (e.g. `normalizeZone(string) string`) and use it in both places to remove duplication and keep the zone normalization consistent if the mapping changes.
Suggested implementation:
```golang
func normalizeZone(zone string) string {
if z, ok := Zones[zone]; ok {
return strconv.Itoa(z)
}
return zone
}
// RankingsMovies GET /api/v1/rankings
func (c *Client) RankingsMovies(type_, period string) (SearchResult, error) {
t := normalizeZone(type_)
```
```golang
var data map[string]json.RawMessage
if err := c.GetJSON("/api/v1/rankings", map[string]string{
"type": t, "period": RankingPeriod(period),
}, &data); err != nil {
return nil, err
}
func (c *Client) RankingsActors(period string) (SearchResult, error) {
var data map[string]json.RawMessage
```
1. In this same file, update `RankingsPlayback` to use `normalizeZone` for its `filterBy` (or equivalent) logic, replacing the duplicated `if z, ok := Zones[...] { strconv.Itoa(z) }` block with a `normalizeZone(...)` call.
2. Ensure `strconv` is imported at the top of `internal/javdb/appapi/rankings.go` (if it is not already), since `normalizeZone` depends on `strconv.Itoa`.
</issue_to_address>
### Comment 2
<location path="internal/cli/rankings_cmd.go" line_range="102" />
<code_context>
return err
}
- res, err := c.RankingsActors(context.Background(), javdb.ActorPeriod(period))
+ res, err := c.RankingsActors(context.Background(), javdb.RankingPeriod(period))
if err != nil {
return fmt.Errorf("rankings failed: %w", err)
</code_context>
<issue_to_address>
**suggestion:** Passing `RankingPeriod(period)` into `RankingsActors` while `RankingsActors` also normalizes period is potentially confusing.
The CLI already normalizes `period` via `javdb.RankingPeriod`, and `RankingsActors` also applies `RankingPeriod` internally. This duplicated normalization obscures which layer expects `day|week|month` vs. `daily|weekly|monthly`. Please consolidate the responsibility—either drop the CLI-side normalization for actors or remove it from the app API and clearly document the expected `period` format at each layer.
Suggested implementation:
```golang
// The CLI passes the raw period ("day|week|month") and RankingsActors
// is responsible for normalizing it via javdb.RankingPeriod internally.
res, err := c.RankingsActors(context.Background(), period)
```
1. Ensure `RankingsActors` internally calls `javdb.RankingPeriod` (or equivalent) so the normalization responsibility clearly resides in the app API layer.
2. If other CLI commands (e.g. movie rankings) also pass `javdb.RankingPeriod(period)` into app-layer ranking functions that themselves normalize, apply the same adjustment there for consistency.
3. Consider adding brief documentation comments on `RankingsActors` and the CLI `period` flag to clarify that the CLI uses `day|week|month` while the app layer handles conversion to `daily|weekly|monthly`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@sdk/rankings.go`:
- Around line 9-10: 保留 sdk 中公开的 ActorPeriod 废弃兼容函数,并将其实现委托给
RankingPeriod,使现有调用方继续编译;不要移除该别名,并按现有弃用约定标注其后续主版本删除计划。
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e7267e24-e2de-4929-b098-1362c6bef846
📒 Files selected for processing (4)
internal/cli/rankings_cmd.gointernal/javdb/appapi/rankings.gointernal/javdb/appapi/rankings_test.gosdk/rankings.go
FlanChanXwO
left a comment
There was a problem hiding this comment.
代码审查未发现阻塞问题。已核对排行榜 zone/period 映射、app API 与公开 SDK 边界、ActorPeriod 向后兼容别名及聚焦测试;本地 go test、race、vet、build、发布脚本、pre-commit 与 LSP 诊断均通过。文档由维护者在后续发布准备中同步。
Summary / 概述
This PR fixes the movie and playback rankings endpoints. Specifically, it maps textual zone names (e.g.,
censored,uncensored,western,fc2) to their corresponding API numeric identifiers before sending requests to the JavDB App API. It also updates the rankings commands to useRankingPeriod(renamed fromActorPeriod) for proper query parameter mapping.Scope and compatibility / 范围与兼容性
javdb rankings movies --typeandjavdb rankings playback --filter-bynow correctly accept and resolve textual zones (including the newly addedfc2option).ActorPeriodtoRankingPeriodin the publicjavdbfacade.Verification / 验证
Successfully ran unit tests verifying query mapping behavior for both movies and playback endpoints:
Release note declaration / Release note 声明
Checklist / 检查清单
~/.javdb-cli/auth.jsoncontents, proxy credentials, private URLs, local state, or private API responses.Summary by Sourcery
修复电影、演员和播放的排行榜 API 查询映射,并更新 CLI 和 SDK 的命名以使用 RankingPeriod。
新功能:
错误修复:
filter_by值。改进:
ActorPeriod辅助工具重命名为RankingPeriod,并在 SDK 中重新导出,以在所有排行榜端点中保持术语一致。测试:
RankingPeriod辅助工具。Original summary in English
Summary by Sourcery
Fix rankings API query mapping for movies, actors, and playback, and update CLI and SDK naming to use RankingPeriod.
New Features:
Bug Fixes:
Enhancements:
Tests:
Summary by CodeRabbit
新功能
fc2选项。改进