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..f3b064b 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.defaultPrTitle(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..517c9b9 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 () => { @@ -3445,14 +3449,23 @@ function SiteRow({ sitePath, initialized, createdAt, label, onInitialized, onSit */} {!prResult.dryRun && ( <> -
- Triage and props live on the ticket, so the link belongs there too. -
+ {/* + Core-only: on Trac the pull request is a link somebody has to + carry back to the ticket, which is why this flow ends there. On + GitHub the pull request is the venue — saying the same thing + would send a Gutenberg contributor looking for a step that does + not exist. + */} + {isTracWorkItem ? ( +
+ Triage and props live on the ticket, so the link belongs there too. +
+ ) : null} {tracTicket ? ( - ) : null} @@ -3520,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} {/* @@ -3545,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
@@ -3558,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, @@ -3573,7 +3594,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 +4239,7 @@ function SiteRow({ sitePath, initialized, createdAt, label, onInitialized, onSit ) : null} {skipInit ? (
-
Trac ticket
+
{workItemConfig.label}
{tracTicket ? ( <>
@@ -4229,7 +4250,7 @@ function SiteRow({ sitePath, initialized, createdAt, label, onInitialized, onSit #{tracTicket} - +
{changesNote && changesNote.placement === 'ticket' ? ( @@ -4297,12 +4318,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 +4396,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 +4410,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={workItem.refPlaceholder} + aria-label={`${workItemConfig.label} number or URL`} />
)} @@ -4932,6 +4960,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={workItem.refPlaceholder} + aria-label={`${workItemConfig.label} number or URL`} />