Skip to content

B7: html_url on issue + PR responses#309

Merged
mfwolffe merged 5 commits into
trunkfrom
b7/html-url-on-issue-pr
May 17, 2026
Merged

B7: html_url on issue + PR responses#309
mfwolffe merged 5 commits into
trunkfrom
b7/html-url-on-issue-pr

Conversation

@espadonne
Copy link
Copy Markdown
Contributor

Summary

B-audit finding B7 (deferred from S62): the issue and PR REST responses didn't include html_url, so CLI clients had to compose the user-facing URL themselves (and were inconsistent about it).

Fix

  • Add HTMLURL string (json: html_url, omitempty) to both issueResponse and pullResponse.
  • Switch 4 issue handlers (list/get/create/patch) + 5 PR handlers (list/get/create/patch/merge) from resolveAPIRepo to resolveAPIRepoWithLogin so they have the owner login available.
  • Two new helpers: (h *Handlers) issueHTMLURL(owner, repo, number) and pullHTMLURL(...), both composing <BaseURL>/<owner>/<repo>/issues|pulls/<number>.
  • Empty BaseURL (test contexts) yields empty string → omitempty drops the field.

The pointer-vs-value type change (*reposdb.Reporeposdb.Repo) required dropping *repo derefs at policy callsites and wrapping with &repo at repoGitDir callsites — both spots are mechanically obvious in the diff.

Test plan

  • TestIssues_CreateAndGet — extended: created.HTMLURL and fetched.HTMLURL both end in /alice/demo/issues/1.
  • TestPulls_CreateAndGet — extended: created.HTMLURL ends in /alice/demo/pulls/1.
  • apiIssue + apiPull test structs gain HTMLURL string.
  • Full TestIssues + TestPulls suites still green against the integration DB.

Notes

This PR depends on no other open PRs. If #307 (D1) merges first, issueResponse will gain Assignees adjacent to where this PR adds HTMLURL — clean rebase.

@espadonne espadonne closed this May 17, 2026
@espadonne espadonne reopened this May 17, 2026
@espadonne espadonne force-pushed the b7/html-url-on-issue-pr branch from f05477e to 55f1a05 Compare May 17, 2026 21:59
@mfwolffe mfwolffe merged commit d254ca8 into trunk May 17, 2026
1 check passed
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