Skip to content

fix: webhook validation logic, unsafe type assertion, non-idiomatic loops#36

Merged
bzimmer merged 8 commits into
mainfrom
copilot/review-codebase-for-issues
Apr 27, 2026
Merged

fix: webhook validation logic, unsafe type assertion, non-idiomatic loops#36
bzimmer merged 8 commits into
mainfrom
copilot/review-codebase-for-issues

Conversation

Copilot AI commented Apr 26, 2026

Copy link
Copy Markdown
Contributor
  • Add tests for internal/httpclient (New, Do, ApplyRateLimiter, ApplyHTTPTracing, ApplyTransport, ApplyHTTPClient, ApplyAutoRefresh) — 97.1% coverage
  • Add TestFault, TestOptions, TestFaultFromServer to rwgps (cover Fault.Error(), WithConfig, WithToken, WithRateLimiter, WithTransport, WithHTTPClient) — 88.4% (was 69.3%)
  • Add TestTripUpload to rwgps (cover TripsService.Upload and the uploader adapter)
  • Add TestUploaderUploadAndStatus to rwgps (cover uploader.Upload, uploader.Status)
  • Extend TestOptions in strava (cover WithToken, WithTransport, WithHTTPClient) — 86.2% (was 84.2%)
  • Add TestFault, TestOptions, TestExport to zwift (cover WithToken, WithAutoRefresh, WithRateLimiter, WithTransport, Fault.SetDefaults, Export) — 90.0% (was 80.0%)
  • Add TestWithTransport and TestStreamSets to cyclinganalytics — 86.7% (was 84.7%)

@codecov

codecov Bot commented Apr 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.63804% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.94%. Comparing base (1b41a82) to head (7d9f96a).

Files with missing lines Patch % Lines
cyclinganalytics/rides.go 33.33% 2 Missing and 2 partials ⚠️
cyclinganalytics/model.go 60.00% 1 Missing and 1 partial ⚠️
internal/httpclient/client.go 96.00% 2 Missing ⚠️
strava/model.go 60.00% 1 Missing and 1 partial ⚠️
cyclinganalytics/cyclinganalytics.go 50.00% 0 Missing and 1 partial ⚠️
strava/strava.go 75.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #36      +/-   ##
==========================================
+ Coverage   72.37%   81.94%   +9.56%     
==========================================
  Files          34       35       +1     
  Lines        1564     1429     -135     
==========================================
+ Hits         1132     1171      +39     
+ Misses        309      144     -165     
+ Partials      123      114       -9     
Files with missing lines Coverage Δ
cyclinganalytics/cyclinganalytics_with.go 100.00% <100.00%> (+28.12%) ⬆️
rwgps/model.go 100.00% <100.00%> (+12.50%) ⬆️
rwgps/rwgps.go 81.81% <100.00%> (+6.06%) ⬆️
rwgps/rwgps_with.go 100.00% <100.00%> (+48.88%) ⬆️
rwgps/trips.go 75.47% <100.00%> (+21.69%) ⬆️
strava/activity.go 71.57% <100.00%> (-0.15%) ⬇️
strava/auth.go 100.00% <100.00%> (ø)
strava/strava_with.go 100.00% <100.00%> (+29.16%) ⬆️
strava/webhook.go 73.61% <100.00%> (+8.82%) ⬆️
zwift/activity.go 74.72% <100.00%> (+3.29%) ⬆️
... and 10 more

... and 1 file 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 1b41a82...7d9f96a. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…s, bump Go to 1.26.1

Agent-Logs-Url: https://github.com/bzimmer/activity/sessions/2afd90f3-b4c5-4ff6-a409-a326f0cf44f3

Co-authored-by: bzimmer <12852+bzimmer@users.noreply.github.com>
- Remove all //go:generate genwith directives from strava, rwgps, zwift, cyclinganalytics
- Remove '// Code generated by genwith' headers from all four *_with.go files
- Drop outdated //nolint:golint,structcheck comments from service structs
- Fix mismatched doc comment 'WithAPICredentials' -> 'WithClientCredentials'
- Modernize do() method: interface{} -> any, errors.Is(err, io.EOF),
  eliminate dead-code type switch (always *Fault on HTTP errors)

Agent-Logs-Url: https://github.com/bzimmer/activity/sessions/cf291181-aaea-4089-a988-3570072e0098

Co-authored-by: bzimmer <12852+bzimmer@users.noreply.github.com>
Copilot AI and others added 2 commits April 27, 2026 05:34
…ault]

Create internal/httpclient.Client[F Fault] — a generic base type that
encapsulates the shared HTTP state (HTTP, Token, Config) and the single
Do() implementation. Package-specific Client structs embed a *Client[*Fault]
field, and their *_with.go files become thin wrappers delegating to shared
Apply* helpers (ApplyRateLimiter, ApplyHTTPTracing, ApplyTransport,
ApplyHTTPClient, ApplyAutoRefresh).

Also adds SetDefaults(int, string) to all four Fault types to satisfy the
httpclient.Fault interface constraint used by the generic Do method.

Net result: the four identical 170-line *_with.go files are each ~115 lines
shorter; the single Do() implementation replaces four copies; and all
transport-mutation logic lives in one place.

Agent-Logs-Url: https://github.com/bzimmer/activity/sessions/c39b4139-1c51-43e3-9ff9-e3885d9c5ad7

Co-authored-by: bzimmer <12852+bzimmer@users.noreply.github.com>
Copilot AI requested a review from bzimmer April 27, 2026 06:59
@bzimmer

bzimmer commented Apr 27, 2026

Copy link
Copy Markdown
Owner

@copilot fix failing ci build

Copilot AI commented Apr 27, 2026

Copy link
Copy Markdown
Contributor Author

@copilot fix failing ci build

Fixed in 7005cde. The CI failures were:

  • errname: renamed testFaulttestFaultError in client_test.go
  • goimports: removed trailing blank lines in rwgps/rwgps_with.go, strava/strava_with.go, zwift/zwift_with.go; fixed struct field alignment in rwgps/trips_test.go
  • govet shadow: renamed inner errdecErr in client.go's Do method
  • gosec G704: added //nolint:gosec with explanation on the http.Client.Do call

Copilot AI requested a review from bzimmer April 27, 2026 09:21
@bzimmer bzimmer merged commit 897fb10 into main Apr 27, 2026
7 checks passed
@bzimmer bzimmer deleted the copilot/review-codebase-for-issues branch April 27, 2026 16:33
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