Skip to content

fix: make update banner dynamic instead of static at startup#1725

Merged
yottahmd merged 1 commit intomainfrom
upgrade-improve
Mar 5, 2026
Merged

fix: make update banner dynamic instead of static at startup#1725
yottahmd merged 1 commit intomainfrom
upgrade-improve

Conversation

@yottahmd
Copy link
Copy Markdown
Collaborator

@yottahmd yottahmd commented Mar 5, 2026

The update check was only evaluated once at server startup and injected as static values into the HTML template. Long-running servers would never show the update banner even if new versions were released.

Now the template functions read from the cache store on every page load, and a periodic background goroutine refreshes the cache every 24 hours.

Summary by CodeRabbit

  • Improvements
    • Enhanced update checking to run periodically throughout the application lifecycle, ensuring users have access to the latest version information with improved freshness and accuracy.

The update check was only evaluated once at server startup and injected
as static values into the HTML template. Long-running servers would
never show the update banner even if new versions were released.

Now the template functions read from the cache store on every page load,
and a periodic background goroutine refreshes the cache every 24 hours.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 5, 2026

📝 Walkthrough

Walkthrough

The changes refactor update info retrieval from direct fields to an interface-based pattern. A new UpdateChecker interface replaces embedded UpdateAvailable and LatestVersion fields, while a new updateChecker implementation with cache-backed periodic checks replaces immediate initialization logic.

Changes

Cohort / File(s) Summary
Template Interface
internal/service/frontend/templates.go
Introduced UpdateChecker interface with GetUpdateInfo() method. Replaced UpdateAvailable and LatestVersion fields in funcsConfig with single UpdateChecker field. Updated template functions to call interface method with nil checks instead of reading direct fields.
Server Implementation
internal/service/frontend/server.go
Added upgradeStore field to Server struct. Implemented unexported updateChecker type with GetUpdateInfo() method. Removed initial async upgrade check from NewServer. Added startPeriodicUpdateCheck() lifecycle method that performs initial check and schedules periodic updates at upgrade.CacheTTL. Integrated periodic check invocation into Serve() startup flow.

Sequence Diagram(s)

sequenceDiagram
    participant Server as Server
    participant Scheduler as Periodic Scheduler
    participant Checker as updateChecker
    participant Store as upgradeStore (Cache)
    participant Template as Template Functions

    Note over Server,Template: Server Startup
    Server->>Checker: create updateChecker{store: upgradeStore}
    Server->>Server: startPeriodicUpdateCheck(ctx)
    
    Note over Scheduler,Store: Initial Check + Scheduling
    Server->>Checker: GetUpdateInfo()
    Checker->>Store: read cache
    Store-->>Checker: cached values
    Checker-->>Server: (updateAvailable, latestVersion)
    Server->>Store: update cache (initial check)
    
    Server->>Scheduler: schedule periodic checks at CacheTTL
    
    Note over Scheduler,Template: Periodic Update Flow
    loop Every CacheTTL
        Scheduler->>Checker: GetUpdateInfo()
        Checker->>Store: read current cache
        Store-->>Checker: cached values
        Checker-->>Scheduler: (updateAvailable, latestVersion)
        Scheduler->>Store: update cache with latest
    end
    
    Note over Template,Store: Template Rendering
    Template->>Checker: GetUpdateInfo()
    Checker->>Store: read cache
    Store-->>Checker: cached values
    Checker-->>Template: (updateAvailable, latestVersion)
    Template-->>Template: render with update info
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: converting the update banner from a static value computed once at startup to a dynamic value that reflects current cache state on each page load.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch upgrade-improve

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 and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/service/frontend/server.go`:
- Around line 858-859: The call to upgrade.CheckAndUpdateCache currently
discards its returned error (e.g., "_, _ =
upgrade.CheckAndUpdateCache(srv.upgradeStore, config.Version)") so periodic
update-check failures are hidden; change both places where CheckAndUpdateCache
is called to capture the error return, and if non-nil log it (using the server
logger, e.g., srv.logger or appropriate logger in scope) with a clear message
including context (service name/upgradeStore/config.Version) so persistent
failures are visible.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6fdcddfd-225c-48ec-b8de-e5a6f767bf3d

📥 Commits

Reviewing files that changed from the base of the PR and between e9ffc5c and a54fee6.

📒 Files selected for processing (2)
  • internal/service/frontend/server.go
  • internal/service/frontend/templates.go

Comment thread internal/service/frontend/server.go
@ghansham
Copy link
Copy Markdown

ghansham commented Mar 5, 2026

For airgapped systems, should it be made configurable (default value: true) to switch it off permanently.

@yottahmd
Copy link
Copy Markdown
Collaborator Author

yottahmd commented Mar 5, 2026

Yes, let's add it in backlog.

@yottahmd yottahmd merged commit e6e3713 into main Mar 5, 2026
5 checks passed
@yottahmd yottahmd deleted the upgrade-improve branch March 5, 2026 08:26
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 5, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.54%. Comparing base (2364eae) to head (a54fee6).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1725      +/-   ##
==========================================
- Coverage   69.57%   69.54%   -0.03%     
==========================================
  Files         398      398              
  Lines       43877    43877              
==========================================
- Hits        30528    30516      -12     
- Misses      10891    10911      +20     
+ Partials     2458     2450       -8     

see 13 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e9ffc5c...a54fee6. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

2 participants