Skip to content

Commit 4d88536

Browse files
josh-padnickclaude
andcommitted
[Wails] M5: Address UpdateService PR review feedback
- Fix Check() doc comment: StartAutoCheck → RunAutoCheck (flows through to the regenerated TS binding) - Add User-Agent header to GitHub Releases API requests (GitHub API requires one for every request) - Drain response body before Close so the TCP connection can be reused - Drop dead Draft/Prerelease filter; /releases/latest already excludes both by definition - Wrap table-driven test cases in t.Run for named subtests - Swap Download icon → ExternalLink in UpdateBanner to match the behaviour (opens the release page in the external browser) Co-Authored-By: Claude Opus 4.7 <[email protected]>
1 parent f351afc commit 4d88536

4 files changed

Lines changed: 24 additions & 20 deletions

File tree

services/update_service.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"encoding/json"
66
"errors"
77
"fmt"
8+
"io"
89
"log/slog"
910
"net/http"
1011
"strings"
@@ -84,7 +85,7 @@ const (
8485
// Check performs a single release lookup and returns the result. Safe
8586
// to call from the frontend (e.g. a "Check for updates" menu item in
8687
// a future milestone); does not emit the `update:available` event —
87-
// callers that want the banner behaviour should use StartAutoCheck.
88+
// callers that want the banner behaviour should use RunAutoCheck.
8889
func (s *UpdateService) Check(ctx context.Context) (*UpdateInfo, error) {
8990
return s.check(ctx)
9091
}
@@ -166,12 +167,17 @@ func (s *UpdateService) check(ctx context.Context) (*UpdateInfo, error) {
166167
}
167168
req.Header.Set("Accept", "application/vnd.github+json")
168169
req.Header.Set("X-GitHub-Api-Version", "2022-11-28")
170+
req.Header.Set("User-Agent", fmt.Sprintf("gruntbooks/%s", s.currentVersion))
169171

170172
resp, err := s.httpClient.Do(req)
171173
if err != nil {
172174
return nil, fmt.Errorf("fetch latest release: %w", err)
173175
}
174-
defer func() { _ = resp.Body.Close() }()
176+
defer func() {
177+
// Drain the body so the underlying TCP connection is reusable.
178+
_, _ = io.Copy(io.Discard, resp.Body)
179+
_ = resp.Body.Close()
180+
}()
175181

176182
if resp.StatusCode == http.StatusNotFound {
177183
return &UpdateInfo{
@@ -187,14 +193,8 @@ func (s *UpdateService) check(ctx context.Context) (*UpdateInfo, error) {
187193
if err := json.NewDecoder(resp.Body).Decode(&release); err != nil {
188194
return nil, fmt.Errorf("decode release: %w", err)
189195
}
190-
if release.Draft || release.Prerelease {
191-
return &UpdateInfo{
192-
Available: false,
193-
CurrentVersion: s.currentVersion,
194-
LatestVersion: release.TagName,
195-
ReleaseURL: release.HTMLURL,
196-
}, nil
197-
}
196+
// GitHub's /releases/latest endpoint excludes drafts and prereleases
197+
// by definition, so release.Draft/Prerelease filtering is unnecessary.
198198

199199
currentCanonical := normalizeSemver(s.currentVersion)
200200
latestCanonical := normalizeSemver(release.TagName)

services/update_service_test.go

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,11 @@ func TestNormalizeSemver(t *testing.T) {
2121
{"not-a-version", ""},
2222
}
2323
for _, c := range cases {
24-
if got := normalizeSemver(c.in); got != c.want {
25-
t.Errorf("normalizeSemver(%q) = %q, want %q", c.in, got, c.want)
26-
}
24+
t.Run(c.in, func(t *testing.T) {
25+
if got := normalizeSemver(c.in); got != c.want {
26+
t.Errorf("normalizeSemver(%q) = %q, want %q", c.in, got, c.want)
27+
}
28+
})
2729
}
2830
}
2931

@@ -39,9 +41,11 @@ func TestIsReleaseVersion(t *testing.T) {
3941
{"unknown", false},
4042
}
4143
for _, c := range cases {
42-
s := NewUpdateService(adapters.NewNoopEmitter(), c.version)
43-
if got := s.isReleaseVersion(); got != c.want {
44-
t.Errorf("isReleaseVersion(%q) = %v, want %v", c.version, got, c.want)
45-
}
44+
t.Run(c.version, func(t *testing.T) {
45+
s := NewUpdateService(adapters.NewNoopEmitter(), c.version)
46+
if got := s.isReleaseVersion(); got != c.want {
47+
t.Errorf("isReleaseVersion(%q) = %v, want %v", c.version, got, c.want)
48+
}
49+
})
4650
}
4751
}

web/src/bindings/github.com/gruntwork-io/runbooks/services/updateservice.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ import * as $models from "./models.js";
3030
* Check performs a single release lookup and returns the result. Safe
3131
* to call from the frontend (e.g. a "Check for updates" menu item in
3232
* a future milestone); does not emit the `update:available` event —
33-
* callers that want the banner behaviour should use StartAutoCheck.
33+
* callers that want the banner behaviour should use RunAutoCheck.
3434
*/
3535
export function Check(): $CancellablePromise<$models.UpdateInfo | null> {
3636
return $Call.ByID(374472365).then(($result: any) => {

web/src/components/UpdateBanner.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { useEffect, useState } from 'react'
22
import { Browser, Events } from '@wailsio/runtime'
3-
import { Download, X } from 'lucide-react'
3+
import { ExternalLink, X } from 'lucide-react'
44
import type { UpdateInfo } from '../bindings/github.com/gruntwork-io/runbooks/services/models'
55
import { isDesktop } from '../lib/wails'
66

@@ -61,7 +61,7 @@ export function UpdateBanner() {
6161
onClick={handleDownload}
6262
className="inline-flex items-center gap-1.5 rounded-md bg-primary px-3 py-1 text-primary-foreground hover:opacity-90 cursor-pointer"
6363
>
64-
<Download className="h-3.5 w-3.5" />
64+
<ExternalLink className="h-3.5 w-3.5" />
6565
Download
6666
</button>
6767
<button

0 commit comments

Comments
 (0)