diff --git a/src/content-script/site-adapters/github/index.mjs b/src/content-script/site-adapters/github/index.mjs index 0db9b8920..b74343e2c 100644 --- a/src/content-script/site-adapters/github/index.mjs +++ b/src/content-script/site-adapters/github/index.mjs @@ -1,5 +1,10 @@ import { cropText, limitedFetch } from '../../../utils' import { config } from '../index.mjs' +import { + hasGitHubPathChanged, + isGitHubIssuePath, + isGitHubPullPath, +} from './path-matching.mjs' const getPatchUrl = async () => { const patchUrl = location.origin + location.pathname + '.patch' @@ -16,13 +21,9 @@ const getPatchData = async (patchUrl) => { return patchData } -const isPull = () => { - return location.href.match(/\/pull\/\d+$/) -} +const isPull = () => isGitHubPullPath(location.pathname) -const isIssue = () => { - return location.href.match(/\/issues\/\d+$/) -} +const isIssue = () => isGitHubIssuePath(location.pathname) function parseGitHubIssueData() { // Function to parse a single comment @@ -125,19 +126,20 @@ function createChatGPtSummaryPrompt(issueData, isIssue = true) { export default { init: async (hostname, userConfig, getInput, mountComponent) => { try { - let oldUrl = location.href + let oldPathname = location.pathname const checkUrlChange = async () => { - if (location.href !== oldUrl) { - oldUrl = location.href - if (isPull() || isIssue()) { - mountComponent('github', config.github) - return - } - - const patchUrl = await getPatchUrl() - if (patchUrl) { - mountComponent('github', config.github) - } + const newPathname = location.pathname + if (!hasGitHubPathChanged(oldPathname, newPathname)) return + + oldPathname = newPathname + if (isPull() || isIssue()) { + mountComponent('github', config.github) + return + } + + const patchUrl = await getPatchUrl() + if (patchUrl) { + mountComponent('github', config.github) } } window.setInterval(checkUrlChange, 500) diff --git a/src/content-script/site-adapters/github/path-matching.mjs b/src/content-script/site-adapters/github/path-matching.mjs new file mode 100644 index 000000000..318dd88db --- /dev/null +++ b/src/content-script/site-adapters/github/path-matching.mjs @@ -0,0 +1,14 @@ +const issuePathPattern = /\/issues\/\d+\/?$/ +const pullPathPattern = /\/pull\/\d+\/?$/ + +export function hasGitHubPathChanged(previousPathname, currentPathname) { + return previousPathname !== currentPathname +} + +export function isGitHubIssuePath(pathname) { + return issuePathPattern.test(pathname) +} + +export function isGitHubPullPath(pathname) { + return pullPathPattern.test(pathname) +} diff --git a/tests/unit/content-script/github-path-matching.test.mjs b/tests/unit/content-script/github-path-matching.test.mjs new file mode 100644 index 000000000..8f118d2dc --- /dev/null +++ b/tests/unit/content-script/github-path-matching.test.mjs @@ -0,0 +1,46 @@ +import assert from 'node:assert/strict' +import { test } from 'node:test' +import { + hasGitHubPathChanged, + isGitHubIssuePath, + isGitHubPullPath, +} from '../../../src/content-script/site-adapters/github/path-matching.mjs' + +test('GitHub thread paths allow an optional trailing slash', () => { + assert.equal(isGitHubIssuePath('/owner/repo/issues/123'), true) + assert.equal(isGitHubIssuePath('/owner/repo/issues/123/'), true) + assert.equal(isGitHubPullPath('/owner/repo/pull/456'), true) + assert.equal(isGitHubPullPath('/owner/repo/pull/456/'), true) +}) + +test('GitHub thread matching ignores query strings and fragments via pathname', () => { + const issueUrl = new URL( + 'https://github.com/owner/repo/issues/123?notification_referrer_id=abc#issuecomment-456', + ) + const pullUrl = new URL('https://github.com/owner/repo/pull/456?foo=bar#discussion_r789') + + assert.equal(isGitHubIssuePath(issueUrl.pathname), true) + assert.equal(isGitHubPullPath(pullUrl.pathname), true) +}) + +test('GitHub navigation refreshes only when the pathname changes', () => { + const original = new URL('https://github.com/owner/repo/issues/123') + const comment = new URL('https://github.com/owner/repo/issues/123#issuecomment-456') + const notification = new URL( + 'https://github.com/owner/repo/issues/123?notification_referrer_id=abc', + ) + const nextIssue = new URL('https://github.com/owner/repo/issues/124') + + assert.equal(hasGitHubPathChanged(original.pathname, comment.pathname), false) + assert.equal(hasGitHubPathChanged(original.pathname, notification.pathname), false) + assert.equal(hasGitHubPathChanged(original.pathname, nextIssue.pathname), true) +}) + +test('GitHub thread matching excludes lists, new issues, and pull subpages', () => { + assert.equal(isGitHubIssuePath('/owner/repo/issues'), false) + assert.equal(isGitHubIssuePath('/owner/repo/issues/new'), false) + assert.equal(isGitHubPullPath('/owner/repo/pulls'), false) + assert.equal(isGitHubPullPath('/owner/repo/pull/456/files'), false) + assert.equal(isGitHubPullPath('/owner/repo/pull/456/commits'), false) + assert.equal(isGitHubPullPath('/owner/repo/pull/456/checks'), false) +})