Skip to content

修复2个问题,在mac上测试通过 - #40

Open
psx516 wants to merge 2 commits into
yuanweize:masterfrom
psx516:master
Open

修复2个问题,在mac上测试通过#40
psx516 wants to merge 2 commits into
yuanweize:masterfrom
psx516:master

Conversation

@psx516

@psx516 psx516 commented Aug 3, 2026

Copy link
Copy Markdown

本次提交包含的修复:

  1. repo.go — 修复 UpdateTarget 使用显式 Select() 强制更新 enabled=false ,解决禁用监控操作无效的问题
  2. server.go — 新增 parseTimeParam() 函数,解析时间后转为本地时区,解决仪表盘历史数据查询返回空的问题
    本次修复均使用AI进行修复,本地功能验证通过,请检查代码后再合并

psx516 and others added 2 commits August 3, 2026 14:30
Updated GeoIP database download URLs for improved accuracy.
- Fix UpdateTarget to use explicit Select() to force bool zero-value (false) updates
  Previously GORM's Updates() silently skipped 'enabled=false' since bool zero
  value was treated as no-change, causing disable operations to be ignored.

- Fix dashboard history query returning empty results due to timezone mismatch
  Frontend sends UTC timestamps via toISOString() but glebarez/sqlite stores
  time in local timezone, causing BETWEEN comparisons to fail. Added parseTimeParam()
  that converts parsed timestamps to local timezone before querying the database.
@coderabbitai

coderabbitai Bot commented Aug 3, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Improved history filtering to support precise RFC3339 and RFC3339Nano timestamps.
    • History results now apply inclusive start and end time boundaries consistently.
    • Corrected target updates so disabled or zero-valued settings are preserved accurately.
    • Parsed history timestamps are converted to local time for clearer display.

Walkthrough

The PR adds shared parsing for RFC3339 timestamps, uses inclusive history bounds, preserves false-valued target fields during updates, and changes the GeoIP database download source.

Changes

History time handling

Layer / File(s) Summary
History timestamp parsing and filtering
internal/api/server.go, pkg/storage/repo.go
handleHistory accepts RFC3339 and RFC3339Nano timestamps and converts them to local time. GetHistory uses explicit inclusive lower and upper timestamp comparisons.

Target update fields

Layer / File(s) Summary
Full target field updates
pkg/storage/repo.go
UpdateTarget explicitly selects all target fields, including enabled, before updating.

Database download sources

Layer / File(s) Summary
Database source URL maintenance
internal/monitor/service.go
The GeoIP download uses the project’s latest-release URL. The ip2region URL remains unchanged, with trailing whitespace removed.

Estimated code review effort: 2 (Simple) | ~10 minutes

Poem

Timestamps align in time,
False flags now persist in line,
GeoIP finds its release trail,
History bounds no longer fail,
Clean URLs complete the run.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed 标题准确概括了修复两个问题并完成 macOS 测试的主要变更。
Description check ✅ Passed 描述明确说明了 UpdateTarget 和历史时间查询的修复内容,并与变更集一致。
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 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 `@internal/api/server.go`:
- Around line 393-401: Update handleHistory’s startStr and endStr parsing to
handle parseTimeParam errors explicitly: when either supplied parameter is
invalid, return HTTP 400 immediately and do not call GetHistory; preserve the
existing parsed values and default bounds for valid or omitted parameters.

In `@internal/monitor/service.go`:
- Around line 622-624: 统一 ensureGeoIPDatabase 与受保护手动更新流程使用的 GeoIP 下载地址:将当前
release URL 提取为共享配置,并让 internal/api/server.go
中的下载逻辑复用该配置,避免两条路径来源不一致;增加集成测试,验证两条路径写入的 MMDB 文件格式兼容且可正常使用。
🪄 Autofix (Beta)

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: ASSERTIVE

Plan: Pro Plus

Run ID: 27ae4c4c-bbc1-4727-a018-0553b747c7a6

📥 Commits

Reviewing files that changed from the base of the PR and between 57b57cd and 7e80c20.

📒 Files selected for processing (3)
  • internal/api/server.go
  • internal/monitor/service.go
  • pkg/storage/repo.go
📜 Review details
🔇 Additional comments (4)
internal/monitor/service.go (1)

639-639: LGTM!

internal/api/server.go (1)

413-426: LGTM!

pkg/storage/repo.go (2)

20-21: LGTM!


65-73: LGTM!

Comment thread internal/api/server.go
Comment on lines +393 to 401
if parsed, err := parseTimeParam(startStr); err == nil {
start = parsed
}
}
if endStr != "" {
if parsed, err := time.Parse(time.RFC3339, endStr); err == nil {
if parsed, err := parseTimeParam(endStr); err == nil {
end = parsed
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Reject invalid time parameters instead of using default bounds.

parseTimeParam returns an error, but handleHistory discards it. An invalid start silently uses the default six-hour window. An invalid end silently uses time.Now(). Return HTTP 400 before calling GetHistory when either supplied parameter is invalid.

Proposed fix
  if startStr != "" {
-   if parsed, err := parseTimeParam(startStr); err == nil {
-     start = parsed
+   parsed, err := parseTimeParam(startStr)
+   if err != nil {
+     c.JSON(http.StatusBadRequest, gin.H{"error": "invalid start time"})
+     return
    }
+   start = parsed
  }
  if endStr != "" {
-   if parsed, err := parseTimeParam(endStr); err == nil {
-     end = parsed
+   parsed, err := parseTimeParam(endStr)
+   if err != nil {
+     c.JSON(http.StatusBadRequest, gin.H{"error": "invalid end time"})
+     return
    }
+   end = parsed
  }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if parsed, err := parseTimeParam(startStr); err == nil {
start = parsed
}
}
if endStr != "" {
if parsed, err := time.Parse(time.RFC3339, endStr); err == nil {
if parsed, err := parseTimeParam(endStr); err == nil {
end = parsed
}
}
parsed, err := parseTimeParam(startStr)
if err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid start time"})
return
}
start = parsed
}
if endStr != "" {
parsed, err := parseTimeParam(endStr)
if err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid end time"})
return
}
end = parsed
}
🤖 Prompt for 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.

In `@internal/api/server.go` around lines 393 - 401, Update handleHistory’s
startStr and endStr parsing to handle parseTimeParam errors explicitly: when
either supplied parameter is invalid, return HTTP 400 immediately and do not
call GetHistory; preserve the existing parsed values and default bounds for
valid or omitted parameters.

Comment on lines +622 to +624
//return downloadGeoIP(path, "https://raw.githubusercontent.com/sapics/ip-location-db/main/dbip-city-mmdb/dbip-city-ipv4.mmdb")
//return downloadGeoIP(path, "https://cdn.jsdelivr.net/gh/Loyalsoldier/geoip@release/Country.mmdb")
return downloadGeoIP(path, "https://github.com/sapics/ip-location-db/releases/download/latest/dbip-city-ipv4.mmdb")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

统一 GeoIP 数据库的下载来源。

ensureGeoIPDatabase 现在使用 release 资产,但 internal/api/server.go Line 792 仍从 main 分支下载同一数据库。启动时自动更新和受保护的手动更新可能使用不同版本,或在来源可用性不一致时产生不同结果。

请将下载地址提取到共享配置,并让两条路径使用同一来源。增加集成测试,确认两条路径写入兼容的 MMDB 文件。

🤖 Prompt for 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.

In `@internal/monitor/service.go` around lines 622 - 624, 统一 ensureGeoIPDatabase
与受保护手动更新流程使用的 GeoIP 下载地址:将当前 release URL 提取为共享配置,并让 internal/api/server.go
中的下载逻辑复用该配置,避免两条路径来源不一致;增加集成测试,验证两条路径写入的 MMDB 文件格式兼容且可正常使用。

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.

1 participant