Skip to content

refactor(notifications): generate notification URL via the adapter#2886

Open
afonsojramos wants to merge 1 commit into
mainfrom
refactor-notification-url-adapter
Open

refactor(notifications): generate notification URL via the adapter#2886
afonsojramos wants to merge 1 commit into
mainfrom
refactor-notification-url-adapter

Conversation

@afonsojramos
Copy link
Copy Markdown
Member

Summary

Follow-up to #2874. notifications/url.ts imported getHtmlUrl directly from forges/github/client and called it for every notification — including ones from Gitea accounts. That was a sharp adapter leak.

  • Replace the direct getHtmlUrl import with adapter.followUrl<{ html_url: string }>(account, url). The Gitea adapter already implements followUrl (via giteaGetJson), so this works for both forges.
  • Rename generateGitHubWebUrlgenerateNotificationWebUrl to match what it actually does.
  • Drop the now-unused getHtmlUrl helper from forges/github/client.ts plus its test, and the GitHubHtmlUrlResponse type.
  • Tests now spy on githubAdapter.followUrl instead of the deleted helper.

Test plan

  • pnpm exec tsc --noEmit — clean
  • pnpm lint — clean
  • pnpm exec vitest --run src/renderer/utils/notifications/url.test.ts src/renderer/utils/forges/github/client.test.ts src/renderer/utils/system/links.test.ts src/renderer/utils/system/native.test.ts — passes

@github-actions github-actions Bot added the refactor Refactoring of existing feature label May 12, 2026
@sonarqubecloud
Copy link
Copy Markdown

Copy link
Copy Markdown
Member

@setchy setchy left a comment

Choose a reason for hiding this comment

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

Octokit client has inbuilt caching the typing. I wouldn't want to stray from that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor Refactoring of existing feature

Development

Successfully merging this pull request may close these issues.

2 participants