From a7955520410ca27acffb7a0554e4fde5655a1f5a Mon Sep 17 00:00:00 2001 From: JuanMa Date: Tue, 11 Aug 2026 12:13:39 +0200 Subject: [PATCH 1/2] [Add] Work on a Gutenberg issue and open a pull request to it (#251) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The last half of the Gutenberg flow: linking a work item and opening a pull request were both Trac- and wordpress-develop-shaped. - src/renderer/github-issue.cjs: the Gutenberg counterpart of trac-ticket.cjs. Accepts `1234`, `#1234` and an issue URL with the noise a copy-paste brings; refuses another repository's issue, another host, and — by name, because it is the obvious mistake — a pull request URL pasted instead of the issue it fixes. Both branches share one bounds check, so a pasted `/issues/0` or a twenty-digit path cannot become a falsy id or a branch named after a float. - src/work-item.cjs: one interface over the two, chosen by the site's project type. The rest of the app asks "parse this", "where does it live", "what is it called" without knowing which kind it is. Defaults to Trac, so a site with no project type behaves exactly as before. - sites:set-ticket validates through the site's provider, so a Gutenberg site accepts an issue where a Core site accepts a ticket. Both parse to a number, so the `ticket/` branch key and every reader of `tracTicket` are unchanged. - github-pr.cjs takes the project's upstream, base branch and branch prefix through the `deps` each helper already receives, and buildPullRequestBody takes the project's citation line. A Gutenberg pull request goes to WordPress/gutenberg on a `fix/issue-` branch and says `Fixes #`, so merging it closes the issue. - The handoff patch header cites the same work item, rather than deriving a Trac URL from the number — a Gutenberg patch must not cite a core.trac ticket that merely shares its issue number. - The work-item card, its input and the browse link follow the project. Both Trac-only surfaces are hidden for a Gutenberg site: the attachments panel, which would offer a Core patch for apply into a Gutenberg checkout, and the "Attach to Trac" destination, whose button would otherwise save a file and then have nowhere to send it. testMode() deliberately keeps reading the environment override rather than the project, so a Gutenberg site is not reported as sandboxed simply for not being wordpress-develop. Co-Authored-By: Claude Opus 4.8 --- src/github-pr.cjs | 84 ++++++++++++++++++------ src/main.js | 35 ++++++++-- src/patch-provenance.cjs | 18 ++++-- src/project-type.cjs | 15 ++++- src/renderer/github-issue.cjs | 107 +++++++++++++++++++++++++++++++ src/renderer/index.jsx | 51 ++++++++++----- src/work-item.cjs | 55 ++++++++++++++++ test/github-issue.test.cjs | 101 +++++++++++++++++++++++++++++ test/github-pr.test.cjs | 98 ++++++++++++++++++++++++++++ test/ipc-wiring.test.cjs | 74 ++++++++++++++++++++- test/patch-provenance.test.cjs | 17 +++++ test/ticket-branch-list.test.cjs | 9 ++- test/work-item.test.cjs | 58 +++++++++++++++++ 13 files changed, 668 insertions(+), 54 deletions(-) create mode 100644 src/renderer/github-issue.cjs create mode 100644 src/work-item.cjs create mode 100644 test/github-issue.test.cjs create mode 100644 test/work-item.test.cjs diff --git a/src/github-pr.cjs b/src/github-pr.cjs index 2c98754..6337c6a 100644 --- a/src/github-pr.cjs +++ b/src/github-pr.cjs @@ -46,15 +46,34 @@ const UPSTREAM_REPO = 'wordpress-develop'; * sandbox needs a `trunk` branch and must not be owned by the signed-in * account, since an account cannot fork its own repository. * + * The site's project type supplies the repository for everything that is not a + * sandbox run (#251), so a Gutenberg site forks and targets WordPress/gutenberg. + * The environment override still wins, because its whole purpose is to redirect + * a real run away from a real upstream. + * + * @param {Object} [project] The project type's `upstream` config. * @return {{owner: string, repo: string}} */ -function upstream() { +function upstream(project) { const raw = process.env.WP_DEV_ENV_GITHUB_UPSTREAM; const match = typeof raw === 'string' && /^([^/\s]+)\/([^/\s]+)$/.exec(raw.trim()); if (match) return { owner: match[1], repo: match[2] }; + if (project && project.owner && project.repo) return { owner: project.owner, repo: project.repo }; return { owner: UPSTREAM_OWNER, repo: UPSTREAM_REPO }; } +/** + * The branch a pull request targets. Both projects call it `trunk` today, but + * reading it from the project type rather than a constant is what keeps a third + * one from needing a second code path. + * + * @param {Object} [project] The project type's `upstream` config. + * @return {string} + */ +function baseBranchFor(project) { + return (project && project.base) || BASE_BRANCH; +} + /** * True when this run stops before opening the pull request. * @@ -77,6 +96,10 @@ function isDryRun() { * @return {{dryRun: boolean, target: string}|null} */ function testMode() { + // Deliberately without a project: this answers "is the environment + // redirecting a real run somewhere else", which is an env-override question. + // Reading a project's own upstream here would report every Gutenberg site as + // sandboxed simply for not being wordpress-develop. const up = upstream(); const target = `${up.owner}/${up.repo}`; const sandboxed = target !== `${UPSTREAM_OWNER}/${UPSTREAM_REPO}`; @@ -176,16 +199,22 @@ const MAX_NOTES_LENGTH = 20000; * @param {number|string} root0.ticketId * @param {string} [root0.handle] * @param {string} [root0.event] - * @param {string} [root0.notes] Free text from the contributor. + * @param {string} [root0.notes] Free text from the contributor. + * @param {Object} [root0.project] The project type's `pr` config plus its work-item URL. * @return {string} */ -function buildPullRequestBody({ ticketId, handle, event, notes } = {}) { +function buildPullRequestBody({ ticketId, handle, event, notes, project } = {}) { const lines = []; const written = typeof notes === 'string' ? notes.trim().slice(0, MAX_NOTES_LENGTH) : ''; if (written) lines.push(written, ''); - lines.push(`Trac ticket: ${ticketUrl(ticketId)}`); + // How a pull request names its work item is the project's own convention + // (#251): Core cites the Trac URL, Gutenberg closes the issue with + // `Fixes #1234`. Defaults to Core's when no project is supplied. + lines.push(project && typeof project.bodyLine === 'function' + ? project.bodyLine(ticketId, project.workItemUrl || ticketUrl(ticketId)) + : `Trac ticket: ${ticketUrl(ticketId)}`); // The same two facts the mentor-handoff header carries (#166), for the same // reason: props follow whoever wrote the patch, and a contributor-day room // is worth naming while it is still happening. @@ -196,14 +225,19 @@ function buildPullRequestBody({ ticketId, handle, event, notes } = {}) { } /** - * A branch name for a ticket, and the alternatives to try if it is taken. + * A branch name for a work item, and the alternatives to try if it is taken. + * + * The prefix comes from the project type (#251) — `trac-` for a Core ticket, + * `fix/issue-` for a Gutenberg issue — so the branch reads correctly in the + * repository it is pushed to. * * @param {number|string} ticketId * @param {number} attempt Zero for the first try. + * @param {string} [prefix] Defaults to Core's. * @return {string} */ -function branchNameFor(ticketId, attempt = 0) { - const base = `trac-${String(ticketId).replace(/[^0-9]/g, '')}`; +function branchNameFor(ticketId, attempt = 0, prefix = 'trac-') { + const base = `${prefix}${String(ticketId).replace(/[^0-9]/g, '')}`; return attempt === 0 ? base : `${base}-${attempt + 1}`; } @@ -224,7 +258,7 @@ async function ensureFork({ token, login }, deps = {}) { const post = deps.post || postJson; const wait = deps.sleep || sleep; const attempts = deps.forkPollAttempts || FORK_POLL_ATTEMPTS; - const up = upstream(); + const up = upstream(deps.project); const forkUrl = `${API}/repos/${login}/${up.repo}`; // A repository under the fork's name is only usable if it actually is a @@ -247,7 +281,7 @@ async function ensureFork({ token, login }, deps = {}) { // surfaces at the very last write — the branch — as an opaque 404. Found // by hand on the first real run against this repository, which is big // enough for that window to be minutes wide. - const readRefs = () => get(`${forkUrl}/git/ref/heads/${BASE_BRANCH}`, { token }); + const readRefs = () => get(`${forkUrl}/git/ref/heads/${baseBranchFor(deps.project)}`, { token }); let existing; try { @@ -323,16 +357,16 @@ async function ensureFork({ token, login }, deps = {}) { async function resolveBase({ token, login, baseSha }, deps = {}) { const get = deps.get || getJson; const post = deps.post || postJson; - const repo = `${API}/repos/${login}/${upstream().repo}`; + const repo = `${API}/repos/${login}/${upstream(deps.project).repo}`; try { // Always fast-forward first, so "the tip" means today's trunk and not // wherever the fork was left. 409 here is a diverged fork, which is a // normal state for someone who has contributed before — not a failure // to report; the branch then bases on the fork's own tip. - await post(`${repo}/merge-upstream`, { branch: BASE_BRANCH }, { token }); + await post(`${repo}/merge-upstream`, { branch: baseBranchFor(deps.project) }, { token }); - const ref = await get(`${repo}/git/ref/heads/${BASE_BRANCH}`, { token }); + const ref = await get(`${repo}/git/ref/heads/${baseBranchFor(deps.project)}`, { token }); if (ref.status !== 200 || !ref.json || !ref.json.object || !ref.json.object.sha) { return failure(ref, 'Could not read your fork’s trunk'); } @@ -374,7 +408,7 @@ async function resolveBase({ token, login, baseSha }, deps = {}) { */ async function staleTouchedPaths({ token, login, tipSha, files }, deps = {}) { const get = deps.get || getJson; - const repo = `${API}/repos/${login}/${upstream().repo}`; + const repo = `${API}/repos/${login}/${upstream(deps.project).repo}`; const clashes = []; try { @@ -418,7 +452,7 @@ async function staleTouchedPaths({ token, login, tipSha, files }, deps = {}) { */ async function createTree({ token, login, baseTreeSha, files }, deps = {}) { const post = deps.post || postJson; - const repo = `${API}/repos/${login}/${upstream().repo}`; + const repo = `${API}/repos/${login}/${upstream(deps.project).repo}`; const entries = []; try { @@ -466,7 +500,7 @@ async function createTree({ token, login, baseTreeSha, files }, deps = {}) { */ async function commitAndBranch({ token, login, ticketId, message, treeSha, parentSha }, deps = {}) { const post = deps.post || postJson; - const repo = `${API}/repos/${login}/${upstream().repo}`; + const repo = `${API}/repos/${login}/${upstream(deps.project).repo}`; try { const commit = await post(`${repo}/git/commits`, { @@ -479,7 +513,7 @@ async function commitAndBranch({ token, login, ticketId, message, treeSha, paren let lastRes = null; for (let attempt = 0; attempt < MAX_BRANCH_ATTEMPTS; attempt++) { - const branch = branchNameFor(ticketId, attempt); + const branch = branchNameFor(ticketId, attempt, deps.branchPrefix); const ref = await post(`${repo}/git/refs`, { ref: `refs/heads/${branch}`, sha }, { token }); if (ref.status === 201) return { ok: true, branch, sha }; // A 404 here is the fork's ref database still initialising — the @@ -520,12 +554,12 @@ async function commitAndBranch({ token, login, ticketId, message, treeSha, paren async function createPullRequest({ token, login, branch, title, body }, deps = {}) { const post = deps.post || postJson; try { - const up = upstream(); + const up = upstream(deps.project); const res = await post(`${API}/repos/${up.owner}/${up.repo}/pulls`, { title, body, head: `${login}:${branch}`, - base: BASE_BRANCH, + base: baseBranchFor(deps.project), maintainer_can_modify: true }, { token }); if (res.status !== 201 || !res.json || !res.json.html_url) return failure(res, 'Could not open the pull request'); @@ -551,11 +585,19 @@ async function createPullRequest({ token, login, branch, title, body }, deps = { * @param {Array} root0.files * @param {string} root0.title * @param {string} root0.body + * @param {Object} [root0.project] The project type's `upstream` + branch prefix. * @param {Function} [root0.onProgress] * @param {Object} [deps] * @return {Promise<{ok: true, url: string, number: number, branch: string, exactBase: boolean}|{ok: false, reason: string, error: string, stage: string}>} */ -async function openPullRequest({ token, login, ticketId, baseSha, files, title, body, onProgress }, deps = {}) { +async function openPullRequest({ token, login, ticketId, baseSha, files, title, body, project, onProgress }, deps = {}) { + // The project type rides in `deps` so every helper below — fork, sync, tree, + // branch, pull request — targets the same repository and base branch without + // each one growing its own parameter (#251). Absent, they all default to + // wordpress-develop, which is what a site with no project type is. + if (project) { + deps = { ...deps, project: project.upstream, branchPrefix: project.branchPrefix }; + } const get = deps.get || getJson; const report = typeof onProgress === 'function' ? onProgress : () => {}; const at = (stage, result) => ({ ...result, stage }); @@ -596,7 +638,7 @@ async function openPullRequest({ token, login, ticketId, baseSha, files, title, // the wrong one silently produces a tree with no history behind it. let baseCommit; try { - baseCommit = await get(`${API}/repos/${login}/${upstream().repo}/git/commits/${base.sha}`, { token }); + baseCommit = await get(`${API}/repos/${login}/${upstream(deps.project).repo}/git/commits/${base.sha}`, { token }); } catch (e) { return at('syncing', { ok: false, reason: 'offline', error: String(e && e.message ? e.message : e) }); } @@ -627,7 +669,7 @@ async function openPullRequest({ token, login, ticketId, baseSha, files, title, return { ok: true, dryRun: true, - url: `https://github.com/${login}/${upstream().repo}/tree/${branched.branch}`, + url: `https://github.com/${login}/${upstream(deps.project).repo}/tree/${branched.branch}`, number: null, branch: branched.branch, exactBase: base.exact diff --git a/src/main.js b/src/main.js index 596cfb3..4cda994 100644 --- a/src/main.js +++ b/src/main.js @@ -66,7 +66,7 @@ const SWITCH_PROGRESS_CHANNEL = 'switch:progress'; // step of an operation, and describing it as progress would have the panel say // "Saving your work…" about trunk — which is the one thing this refuses to do. const CARRIED_WORK_CHANNEL = 'ticket:carried-work'; -const { parseTicketRef } = require('./renderer/trac-ticket.cjs'); +const { workItemProvider } = require('./work-item.cjs'); const { parseHandle } = require('./wporg-handle.cjs'); const { parseEventName, buildProvenanceHeader, handoffFilename } = require('./patch-provenance.cjs'); const { describeRefused } = require('./safe-log'); @@ -619,10 +619,16 @@ ipcMain.handle('git:save-patch', async (_e, sitePath, options) => { const s = await getStore(); const meta = (s.get('siteMeta') || {})[sitePath] || {}; const { wporgHandle: handle = null, contributionEvent: event = null } = s.get('preferences') || {}; + // The work item is named the way this site's project names it + // (#251) — a Gutenberg patch must not cite a core.trac ticket that + // merely shares its number. + const wi = siteWorkItemProvider(meta); header = buildProvenanceHeader({ handle, event, ticketId: meta.tracTicket, + workItemLabel: wi.kind === 'trac' ? 'Ticket' : 'Issue', + workItemUrl: meta.tracTicket ? wi.urlFor(meta.tracTicket) : null, // The base the patch was actually diffed against, which on a // ticket branch is the trunk it was born at — not the site's // current trunk, which "Update to latest trunk" may have moved @@ -766,8 +772,11 @@ ipcMain.handle('github:open-pr', async (event, sitePath, options = {}) => { const s = await getStore(); const meta = (s.get('siteMeta') || {})[sitePath] || {}; const ticketId = meta.tracTicket; + // What the work item is called, and where its pull request goes, both follow + // the site's project (#251). + const projectType = projectTypeForSite(meta); if (!ticketId) { - return { ok: false, reason: 'no-ticket', error: 'Link a Trac ticket to this site first.', stage: 'auth' }; + return { ok: false, reason: 'no-ticket', error: `Link a ${projectType.workItem.label} to this site first — a pull request has to cite one.`, stage: 'auth' }; } const { wporgHandle: handle = null, contributionEvent = null } = s.get('preferences') || {}; @@ -778,9 +787,10 @@ ipcMain.handle('github:open-pr', async (event, sitePath, options = {}) => { return { ok: false, reason: 'error', error: String(e), stage: 'collect' }; } + const provider = siteWorkItemProvider(meta); const title = typeof options.title === 'string' && options.title.trim() ? options.title.trim() - : `Ticket #${ticketId}`; + : `${provider.noun.charAt(0).toUpperCase()}${provider.noun.slice(1)} #${ticketId}`; const result = await openPullRequest({ token: githubToken, @@ -789,7 +799,14 @@ ipcMain.handle('github:open-pr', async (event, sitePath, options = {}) => { baseSha: collected.baseOid, files: collected.files, title, - body: buildPullRequestBody({ ticketId, handle, event: contributionEvent, notes: options.notes }), + project: { upstream: projectType.upstream, branchPrefix: projectType.pr.branchPrefix }, + body: buildPullRequestBody({ + ticketId, + handle, + event: contributionEvent, + notes: options.notes, + project: { bodyLine: projectType.pr.bodyLine, workItemUrl: provider.urlFor(ticketId) } + }), onProgress: (stage) => { if (!event.sender.isDestroyed()) event.sender.send('github:pr:progress', { sitePath, stage }); } @@ -1328,6 +1345,11 @@ const upstreamRepoPath = (meta) => { return `${up.owner}/${up.repo}`; }; +// The work-item provider for a site — Trac tickets for Core, GitHub issues for +// Gutenberg (#251). Defaults to Trac for a site with no project type. +const siteWorkItemProvider = (meta) => + workItemProvider(projectTypeForSite(meta).workItem.provider, upstreamRepoPath(meta)); + ipcMain.handle('git:list-ticket-patches', async (_e, sitePath) => { try { const s = await getStore(); @@ -1869,7 +1891,10 @@ ipcMain.handle('sites:set-ticket', async (event, sitePath, ref, options) => with return { ok: true, ticket: null, branch: TRUNK }; } - const parsed = parseTicketRef(raw); + // What counts as a work item follows the site's project (#251): a Trac ticket + // for Core, a GitHub issue for Gutenberg. Both parse to a number, so the + // `ticket/` branch key and every reader of `tracTicket` are unchanged. + const parsed = siteWorkItemProvider(await readSiteMeta(sitePath)).parseRef(raw); if (!parsed.ok) return { ok: false, error: parsed.error }; const blocked = await midSwitchBlock(sitePath); diff --git a/src/patch-provenance.cjs b/src/patch-provenance.cjs index d528cb8..7265b2c 100644 --- a/src/patch-provenance.cjs +++ b/src/patch-provenance.cjs @@ -124,15 +124,17 @@ function ticketNumber(ticketId) { * recorded. * * @param {Object} details - * @param {string} [details.handle] WordPress.org handle, already validated. - * @param {string} [details.event] Where it was written — a WordCamp, a meetup. + * @param {string} [details.handle] WordPress.org handle, already validated. + * @param {string} [details.event] Where it was written — a WordCamp, a meetup. * @param {number|string} [details.ticketId] + * @param {string} [details.workItemLabel] 'Ticket' (default) or 'Issue'. + * @param {string} [details.workItemUrl] Defaults to the Trac ticket URL. * @param {string} [details.trunkOid] - * @param {string} [details.trunkDate] ISO timestamp of the base commit. - * @param {string} [details.generatedAt] ISO timestamp for "now". + * @param {string} [details.trunkDate] ISO timestamp of the base commit. + * @param {string} [details.generatedAt] ISO timestamp for "now". * @return {string} */ -function buildProvenanceHeader({ handle, event, ticketId, trunkOid, trunkDate, generatedAt } = {}) { +function buildProvenanceHeader({ handle, event, ticketId, workItemLabel, workItemUrl, trunkOid, trunkDate, generatedAt } = {}) { const lines = []; const contributor = field(handle); @@ -145,8 +147,12 @@ function buildProvenanceHeader({ handle, event, ticketId, trunkOid, trunkDate, g const where = field(event); if (where) lines.push(`# Event: ${where}`); + // The work item, named the way its own project names it (#251): a Core patch + // cites a Trac ticket, a Gutenberg one its GitHub issue. `workItemUrl` is + // supplied by the caller, which knows the site's project; without it this + // falls back to Trac, which is what every existing caller meant. const ticket = ticketNumber(ticketId); - if (ticket) lines.push(`# Ticket: ${ticketUrl(ticket)}`); + if (ticket) lines.push(`# ${workItemLabel || 'Ticket'}: ${workItemUrl || ticketUrl(ticket)}`); const oid = field(trunkOid); const based = day(trunkDate); diff --git a/src/project-type.cjs b/src/project-type.cjs index 031e7a1..156b919 100644 --- a/src/project-type.cjs +++ b/src/project-type.cjs @@ -54,7 +54,13 @@ const PROJECT_TYPES = { // src/wp-includes layout (patch-plan.cjs mapToSrcLayout). patch: { layout: 'src-layout' }, - workItem: { provider: 'trac' }, + workItem: { + provider: 'trac', + // What the panel calls it, and where a newcomer goes to find one. + label: 'Trac ticket', + browseUrl: 'https://core.trac.wordpress.org/tickets/good-first-bugs', + browseLabel: 'Browse good first bugs on Trac' + }, pr: { branchPrefix: 'trac-', @@ -94,7 +100,12 @@ const PROJECT_TYPES = { // (packages/…); no src-layout rewrite. patch: { layout: 'repo-relative' }, - workItem: { provider: 'github-issue' }, + workItem: { + provider: 'github-issue', + label: 'GitHub issue', + browseUrl: 'https://github.com/WordPress/gutenberg/issues?q=is%3Aissue+is%3Aopen+label%3A%22Good+First+Issue%22', + browseLabel: 'Browse good first issues on GitHub' + }, pr: { branchPrefix: 'fix/issue-', diff --git a/src/renderer/github-issue.cjs b/src/renderer/github-issue.cjs new file mode 100644 index 0000000..15f98e8 --- /dev/null +++ b/src/renderer/github-issue.cjs @@ -0,0 +1,107 @@ +'use strict'; + +/** + * Reading what a contributor typed when asked which GitHub issue they are + * working on (#251) — the Gutenberg counterpart of trac-ticket.cjs. + * + * Same shape as the Trac parser on purpose: a contributor arrives from a + * browser, so the input is as likely to be a pasted URL — with a comment + * anchor, a trailing slash or a query still attached — as it is a bare `#1234`. + * Keeping the two behind one interface is what lets the rest of the app ask + * "which work item is this site on?" without knowing where the answer lives. + * + * Kept pure and dependency-free so it can be unit tested without a DOM: the + * renderer bundle imports it and `node --test` requires it directly. + */ + +const GITHUB_HOST = 'github.com'; + +// Gutenberg is in the 70,000s. Seven digits leaves decades of headroom while +// still rejecting a pasted timestamp — the same bound, and the same reasoning, +// as the Trac parser's. +const MAX_ISSUE_ID = 9999999; + +/** + * Canonical URL for an issue in a repository. + * + * @param {number|string} id + * @param {string} repoPath `owner/repo`. + */ +function issueUrl(id, repoPath) { + return `https://${GITHUB_HOST}/${repoPath}/issues/${id}`; +} + +/** + * Resolves free-form input to an issue id. Accepts `1234`, `#1234` and an issue + * URL for this site's own repository; rejects everything else with a message + * meant for the contributor, not for a log. + * + * A pull-request URL is rejected by name rather than falling through to the + * generic message: pasting the PR instead of the issue it fixes is the obvious + * mistake here, and "that is a pull request" is the answer that unsticks it. + * + * @param {string} input + * @param {Object} [options] + * @param {string} [options.repoPath] `owner/repo` whose issues this site tracks. + * @return {{ok: true, id: number, url: string}|{ok: false, error: string}} + */ +function parseIssueRef(input, { repoPath = 'WordPress/gutenberg' } = {}) { + const notAnIssue = `Enter an issue number like 1234, or a ${repoPath} issue URL.`; + const raw = typeof input === 'string' ? input.trim() : ''; + if (!raw) return { ok: false, error: 'Enter an issue number or URL.' }; + + // Both branches below go through this, the way the Trac parser routes both of + // its own through fromDigits. A URL's digits are no more trustworthy than a + // typed one's: `/issues/0` would otherwise store a falsy id that every + // `ticketId ? …` reads as "no work item" while the site sits on `ticket/0`, + // and a 20-digit path would become a branch named after a float. + const fromDigits = (digits) => { + const id = Number(digits); + if (!Number.isSafeInteger(id) || id < 1 || id > MAX_ISSUE_ID) { + return { ok: false, error: notAnIssue }; + } + return { ok: true, id, url: issueUrl(id, repoPath) }; + }; + + const bare = raw.replace(/^#/, ''); + if (/^\d+$/.test(bare)) return fromDigits(bare); + + // Anything else has to be a URL. Accept it without a scheme too — copying a + // host out of an address bar often drops it. But `new URL` is lenient + // (`new URL('https://abc')` succeeds), so only take this branch when the + // input actually looks like a URL; otherwise a bare word would be reported + // as a wrong host rather than as not-an-issue. + const hasScheme = /^[a-z][a-z0-9+.-]*:\/\//i.test(raw); + if (!hasScheme && !raw.includes('/') && !raw.includes('.')) { + return { ok: false, error: notAnIssue }; + } + + let parsed; + try { + parsed = new URL(hasScheme ? raw : `https://${raw}`); + } catch { + return { ok: false, error: notAnIssue }; + } + + if (parsed.hostname.toLowerCase() !== GITHUB_HOST) { + return { ok: false, error: `Only ${GITHUB_HOST} issues are supported.` }; + } + + // Reading the id off the path drops ?foo= and #issuecomment- for free. + const match = /^\/([^/]+\/[^/]+)\/(issues|pull)\/(\d+)\/?$/.exec(parsed.pathname); + if (!match) return { ok: false, error: notAnIssue }; + if (match[2] === 'pull') { + return { ok: false, error: 'That is a pull request. Link the issue it fixes instead.' }; + } + if (match[1].toLowerCase() !== String(repoPath).toLowerCase()) { + return { ok: false, error: `Only ${repoPath} issues can be linked here.` }; + } + return fromDigits(match[3]); +} + +module.exports = { + GITHUB_HOST, + MAX_ISSUE_ID, + issueUrl, + parseIssueRef +}; diff --git a/src/renderer/index.jsx b/src/renderer/index.jsx index 5942531..dcfbd52 100644 --- a/src/renderer/index.jsx +++ b/src/renderer/index.jsx @@ -35,7 +35,7 @@ import { pickLatest } from '../latest-patch.cjs'; import { beginSetup, adoptSetupPath, discardSetup, rowPathAfterStatus } from './pending-setup.cjs'; import { parsePrRef } from '../patch-sources.cjs'; import { prStateBadge } from './pr-state.cjs'; -import { ticketUrl, attachUrl } from './trac-ticket.cjs'; +import { workItemProvider } from '../work-item.cjs'; import { adminUrl, adminerUrl } from './site-urls.cjs'; import { ticketBranchRows, ticketListCard } from './ticket-branch-list.cjs'; import { describeSwitchProgress } from '../switch-progress.cjs'; @@ -80,7 +80,6 @@ const PR_FAILURE_MESSAGES = { unauthorized: 'That GitHub sign-in is no longer valid. Sign in again, or save the patch file instead.', 'rate-limited': 'GitHub is rate-limiting this connection. It usually clears within the hour.', offline: 'No connection to GitHub.', - 'no-ticket': 'Link a Trac ticket to this site first — a pull request has to cite one.', empty: 'There are no changes to open a pull request with.' }; // Per-status wording for the update chain card (#94), following the issue's @@ -119,7 +118,6 @@ const TICKET_PATCH_STATUS_MESSAGE = { offline: 'Could not reach GitHub.', error: 'Could not read the pull requests from GitHub.' }; -const TRAC_TICKET_LISTS_URL = 'https://core.trac.wordpress.org/tickets/good-first-bugs'; const CREATE_SITE_MODAL_STYLE_ID = 'create-site-modal-theme'; function formatEmailDate(email) { @@ -1283,6 +1281,12 @@ function SiteRow({ sitePath, initialized, createdAt, label, onInitialized, onSit const allowedScripts = projectBuildConfig.allowedScripts; // `owner/repo` this site's pull requests come from and go to. const upstreamRepoPath = `${projectConfig.upstream.owner}/${projectConfig.upstream.repo}`; + // The work item this site tracks — a Trac ticket for Core, a GitHub issue for + // Gutenberg (#251). `workItem.attachUrlFor` is null where the concept does not + // exist, which is what the attachments panel keys off. + const workItemConfig = projectConfig.workItem; + const workItem = workItemProvider(workItemConfig.provider, upstreamRepoPath); + const isTracWorkItem = workItem.kind === 'trac'; const [statusLoading, setStatusLoading] = useState(true); const [waitingForWatch, setWaitingForWatch] = useState(false); // Trac ticket association (#109) @@ -3297,7 +3301,7 @@ function SiteRow({ sitePath, initialized, createdAt, label, onInitialized, onSit // an attach form with nothing to attach. const saveForTrac = async () => { const filePath = await savePatchFile(); - if (filePath && tracTicket) window.api.openExternal(attachUrl(tracTicket)); + if (filePath && tracTicket && workItem.attachUrlFor) window.api.openExternal(workItem.attachUrlFor(tracTicket)); }; const saveForHandoff = async () => { @@ -3452,7 +3456,7 @@ function SiteRow({ sitePath, initialized, createdAt, label, onInitialized, onSit {prLinkCopied ? 'Link copied' : 'Copy the link'} {tracTicket ? ( - ) : null} @@ -3573,7 +3577,7 @@ function SiteRow({ sitePath, initialized, createdAt, label, onInitialized, onSit ) : (
- No ticket is linked to this site. A pull request has to cite one — link it in the Trac card. + No work item is linked to this site. A pull request has to cite one — link it in the {workItemConfig.label} card.
)} {prStage ? ( @@ -4218,7 +4222,7 @@ function SiteRow({ sitePath, initialized, createdAt, label, onInitialized, onSit ) : null} {skipInit ? (
-
Trac ticket
+
{workItemConfig.label}
{tracTicket ? ( <>
@@ -4229,7 +4233,7 @@ function SiteRow({ sitePath, initialized, createdAt, label, onInitialized, onSit #{tracTicket} - +
{changesNote && changesNote.placement === 'ticket' ? ( @@ -4297,12 +4301,18 @@ function SiteRow({ sitePath, initialized, createdAt, label, onInitialized, onSit ) : null}
- {latestIsAttachment ? ( + {latestIsAttachment && isTracWorkItem ? (
The most recent patch on this ticket is a file attachment, not a pull request — see Trac attachments below.
) : null} + {/* Only Trac carries patch attachments (#251). A GitHub issue's work + arrives as a pull request, which the panel above already lists — + and offering "Show Trac attachments" on a Gutenberg site would + open a core.trac ticket that merely shares its number, then offer + its Core patch for apply into a Gutenberg checkout. */} + {isTracWorkItem ? (
Trac attachments
@@ -4369,11 +4379,12 @@ function SiteRow({ sitePath, initialized, createdAt, label, onInitialized, onSit
) : null}
+ ) : null} ) : ( <>
- Tell the app which ticket you are working on. It is stored with the site, so it survives restarts, and you can change or remove it at any time. + Tell the app which work item you are on. It is stored with the site, so it survives restarts, and you can change or remove it at any time.
@@ -4382,8 +4393,8 @@ function SiteRow({ sitePath, initialized, createdAt, label, onInitialized, onSit onChange={(value) => { setTicketInput(value); setTicketError(''); }} onKeyDown={(event) => { if (event.key === 'Enter') { event.preventDefault(); linkTicket(); } }} disabled={ticketActionsBlocked} - placeholder="Ticket number or URL, e.g. 62281" - aria-label="Trac ticket number or URL" + placeholder={isTracWorkItem ? 'Ticket number or URL, e.g. 62281' : 'Issue number or URL, e.g. 71234'} + aria-label={`${workItemConfig.label} number or URL`} />
)} @@ -4932,6 +4943,13 @@ function SiteRow({ sitePath, initialized, createdAt, label, onInitialized, onSit + {/* Attaching a patch file to the work item is a Trac concept + (#251). A GitHub issue takes no attachments — Gutenberg + work arrives as a pull request, which the destination + above already offers — so this is hidden rather than + rendered with a button that saves the file and then + silently has nowhere to send it. */} + {isTracWorkItem ? ( { setTicketInput(value); setTicketError(''); }} onKeyDown={(event) => { if (event.key === 'Enter') { event.preventDefault(); linkTicket(); } }} disabled={ticketActionsBlocked} - placeholder="Ticket number or URL, e.g. 62281" - aria-label="Trac ticket number or URL" + placeholder={isTracWorkItem ? 'Ticket number or URL, e.g. 62281' : 'Issue number or URL, e.g. 71234'} + aria-label={`${workItemConfig.label} number or URL`} /> @@ -3524,7 +3533,7 @@ function SiteRow({ sitePath, initialized, createdAt, label, onInitialized, onSit /> {!prTitle.trim() ? (
- Left empty, it will be titled Ticket #{tracTicket}. + Left empty, it will be titled {workItem.defaultPrTitle(tracTicket)}.
) : null} {/* @@ -3549,7 +3558,14 @@ function SiteRow({ sitePath, initialized, createdAt, label, onInitialized, onSit this flow ends on the point rather than the postscript, so they are stated before the button, not after the pull request exists. + + Both are false for Gutenberg, where the pull request IS the + review venue and IS what gets merged — and the audience least + able to spot that the app is describing a different project is + exactly this one. Core-only until the Gutenberg counterpart is + written. */} + {isTracWorkItem ? (
How pull requests work in core
@@ -3562,6 +3578,7 @@ function SiteRow({ sitePath, initialized, createdAt, label, onInitialized, onSit >The handbook page on pull requests
+ ) : null} {/* The button says what it will actually do. A dry run's button reading "Open pull request" is the label lying about the mode, @@ -4393,7 +4410,7 @@ function SiteRow({ sitePath, initialized, createdAt, label, onInitialized, onSit onChange={(value) => { setTicketInput(value); setTicketError(''); }} onKeyDown={(event) => { if (event.key === 'Enter') { event.preventDefault(); linkTicket(); } }} disabled={ticketActionsBlocked} - placeholder={isTracWorkItem ? 'Ticket number or URL, e.g. 62281' : 'Issue number or URL, e.g. 71234'} + placeholder={workItem.refPlaceholder} aria-label={`${workItemConfig.label} number or URL`} /> @@ -4969,7 +4986,7 @@ function SiteRow({ sitePath, initialized, createdAt, label, onInitialized, onSit onChange={(value) => { setTicketInput(value); setTicketError(''); }} onKeyDown={(event) => { if (event.key === 'Enter') { event.preventDefault(); linkTicket(); } }} disabled={ticketActionsBlocked} - placeholder={isTracWorkItem ? 'Ticket number or URL, e.g. 62281' : 'Issue number or URL, e.g. 71234'} + placeholder={workItem.refPlaceholder} aria-label={`${workItemConfig.label} number or URL`} />