Skip to content

Commit dca4bde

Browse files
ryanwelcherryanwelcher
authored
Translate the pull request destination and GitHub sign-in (#643)
## Why The Open a pull request card, its GitHub sign-in, and the errors main sends it were plain English in a translated window. This is the #625 batch of #622, which needed the catalog in main from #634. ## What changes - **The card** (`pull-request-destination.jsx`) wraps every string. Sentences with a link or code in them use `createInterpolateElement`, with logins, branches and repository names passed in as elements so they are never read as markup. The failure messages are a function instead of a module-level constant. - **Per-project text** in `project-type.cjs` (`cards.*`) is getters, read when the card renders. Main's refusal with nothing linked used to splice the work item's label into a sentence; it is now `cards.prNeedsWorkItem`, one sentence per project type. - **Main process:** the sign-in errors in `github-auth.cjs` and the pull request errors in `github-pr.cjs` are wrapped, including the step labels passed to `failure()`, which the issue did not list. The "Trunk has moved" message uses `_n`, and a GitHub failure is composed with `%1$s: %2$s%3$s` so a translator can change the punctuation. GitHub's own reason and the bracketed diagnostic for GitHub support stay as they are. Not wrapped, per #622: the pull request body and the default pull request title, because they leave the app. ## How to test this The new journeys cover the card in the pseudo-locale on both projects. Run them from the repository root on either platform: ``` npx playwright test --project=journeys tests/e2e/journeys/i18n.spec.js -g "pull request card" ``` Both fail without this change. The Core journey walks every state #625 lists: signed out, "Not now" and "Show this again", the device code, signed in with the form and its fold, a `rate-limited` failure, a dry run, and a pull request opened from a checkout behind trunk. The Gutenberg one walks signed out, signed in with the fold open, and the opened pull request, checking the words that are Gutenberg's own. What the journeys cannot reach is a real GitHub. They stub every `github:*` handler, so the main-process errors never reach the screen in them. Unit tests in `github-auth`, `github-pr` and `pr-stage` cover those strings in the pseudo-locale instead. To see one on screen by hand (either platform, current head): 1. From the repository root, run `npx electron . --lang=en-XA`. 2. Open a Core site with a linked ticket and an edit, and click **Review & submit changes**. 3. In **Open a pull request**, click **Sign in with GitHub** and finish in the browser. - Expected: every line of the card is accented and in brackets, apart from your login, your fork's name, the code and the branch. **What must not have happened:** no English an English speaker sees has changed. The English journeys in `open-pull-request.spec.js`, `review-changes.spec.js` and `pr-checkout.spec.js` still pass with their exact strings. ## Risks and limitations - The other open batches (#636, #644, #645, #646) also append tests to the end of `i18n.spec.js`, so whichever merges later resolves that by keeping both. - One finding is deferred: `prFailureMessage` decides between five user-facing strings inside a `.jsx` file the suite cannot load. It predates this PR (it was the old constant map), so moving it to a `.cjs` module with a test is a follow-up. ## Related Fixes #625. Part of #622. Follows #634. --- <details> <summary>Design decisions and alternatives considered</summary> - **Not stacked on #635.** #622 asks for batches that review on their own. Now that #635 has merged, its `unwrapped()` scanner change and the `@wordpress/element` dependency, which this PR also needed, come from trunk. - **The textarea placeholder** is three strings joined with a newline. A single `__()` with `\n` in it is refused by `@wordpress/i18n-no-collapsible-whitespace`. - **"Signed in as …"** is wrapped in its own `<span>`, so the sentence and the **Sign out** button beside it are separate elements, as in #635. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> 2 [fix here] · 1 [follow-up]. Both [fix here] findings are fixed; the follow-up is deferred (see Risks). - 🟡 The Gutenberg journey linked no issue, so it never showed the form, the fold or the loop-back. Fixed: it links one in the record and walks those states. - 🔵 No test showed the main-process strings or the stage labels come out translated. Fixed: pseudo-locale unit tests for the scope refusal, a composed `failure()`, the plural stale message and `prStageLabel`. - Style: three dead entries in the journey's `THEIRS` filter were removed. - **Review:** completed. Fresh agent context, head `11ef362` / base `c34aa45`. `npm run lint` clean, `npm test` 2002 pass / 0 fail, i18n and PR journeys 25 pass. - **Since review:** `11ef362 → ef457fd`. The only change is the test fixes above, checked by the authoring session, not re-reviewed by a fresh context. `src` and `tests` lint clean, unit suite 2005 pass / 0 fail, i18n journeys 12 pass. The review did not run the journeys itself; it was read-only. - **After #635 merged:** `ef457fd → 9559a1f` (trunk merged from GitHub) → `012fca8` (trunk at `e94f507` merged; `i18n.spec.js` resolved as trunk's file plus this PR's two tests, nothing else changed). Checked by the authoring session, not re-reviewed: `src` and `tests` lint clean, unit suite 2036 pass / 0 fail, i18n, open-pull-request, review-changes and pr-checkout journeys 31 pass. </details> <details> <summary>Screenshots or recording</summary> None. Nothing changes on screen in English, and the pseudo-locale journeys are the check for the translated state. </details> 🤖 Generated with [Claude Code](https://claude.com/claude-code)
1 parent e94f507 commit dca4bde

12 files changed

Lines changed: 447 additions & 112 deletions

‎src/github-auth.cjs‎

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
// scope that fails to arrive produces a token that signs in fine and cannot
3232
// push — a failure that surfaces at the last write of the whole flow.
3333
const { postForm, getJson } = require('./github-http.cjs');
34+
const { __, sprintf } = require('@wordpress/i18n');
3435

3536
// The WordPress organisation's OAuth application, "WordPress Contributor
3637
// Toolkit". An empty value here means this build has no sign-in configured,
@@ -94,7 +95,7 @@ async function requestDeviceCode(deps = {}) {
9495
return {
9596
ok: false,
9697
reason: 'not-configured',
97-
error: 'This build has no GitHub application configured, so it cannot sign you in.'
98+
error: __('This build has no GitHub application configured, so it cannot sign you in.')
9899
};
99100
}
100101

@@ -112,11 +113,13 @@ async function requestDeviceCode(deps = {}) {
112113
return {
113114
ok: false,
114115
reason: 'not-configured',
115-
error: 'GitHub does not recognise this application, or device flow is not enabled on it.'
116+
error: __('GitHub does not recognise this application, or device flow is not enabled on it.')
116117
};
117118
}
118119
if (res.status !== 200 || !res.json || !res.json.device_code) {
119-
const detail = res.json && res.json.error_description ? res.json.error_description : `GitHub returned ${res.status}`;
120+
const detail = res.json && res.json.error_description ? res.json.error_description :
121+
// translators: %s: an HTTP status code, such as 500.
122+
sprintf(__('GitHub returned %s'), res.status);
120123
return { ok: false, reason: 'error', error: detail };
121124
}
122125

@@ -163,13 +166,13 @@ async function pollForToken({ deviceCode, interval, expiresAt }, deps = {}) {
163166
let waitSeconds = Number(interval) > 0 ? Number(interval) : DEFAULT_INTERVAL_SECONDS;
164167

165168
for (;;) {
166-
if (isCanceled()) return { ok: false, reason: 'canceled', error: 'Sign-in was canceled.' };
169+
if (isCanceled()) return { ok: false, reason: 'canceled', error: __('Sign-in was canceled.') };
167170
await wait(waitSeconds * 1000);
168-
if (isCanceled()) return { ok: false, reason: 'canceled', error: 'Sign-in was canceled.' };
171+
if (isCanceled()) return { ok: false, reason: 'canceled', error: __('Sign-in was canceled.') };
169172
// Checked after the wait rather than before it, so a code that expires
170173
// mid-sleep is reported as expired instead of being polled once more.
171174
if (now() >= expiresAt) {
172-
return { ok: false, reason: 'expired', error: 'The code expired before it was entered.' };
175+
return { ok: false, reason: 'expired', error: __('The code expired before it was entered.') };
173176
}
174177

175178
let res;
@@ -198,14 +201,15 @@ async function pollForToken({ deviceCode, interval, expiresAt }, deps = {}) {
198201
waitSeconds = Number(json.interval) > 0 ? Number(json.interval) : waitSeconds + SLOW_DOWN_BUMP_SECONDS;
199202
break;
200203
case 'expired_token':
201-
return { ok: false, reason: 'expired', error: 'The code expired before it was entered.' };
204+
return { ok: false, reason: 'expired', error: __('The code expired before it was entered.') };
202205
case 'access_denied':
203-
return { ok: false, reason: 'denied', error: 'The authorization was declined on GitHub.' };
206+
return { ok: false, reason: 'denied', error: __('The authorization was declined on GitHub.') };
204207
default:
205208
return {
206209
ok: false,
207210
reason: 'error',
208-
error: json.error_description || json.error || `GitHub returned ${res.status}`
211+
// translators: %s: an HTTP status code, such as 500.
212+
error: json.error_description || json.error || sprintf(__('GitHub returned %s'), res.status)
209213
};
210214
}
211215
}
@@ -252,10 +256,11 @@ async function fetchViewer(token, deps = {}) {
252256
return { ok: false, reason: 'offline', error: String(e && e.message ? e.message : e) };
253257
}
254258
if (res.status === 401) {
255-
return { ok: false, reason: 'unauthorized', error: 'That sign-in is no longer valid.' };
259+
return { ok: false, reason: 'unauthorized', error: __('That sign-in is no longer valid.') };
256260
}
257261
if (res.status !== 200 || !res.json || !res.json.login) {
258-
return { ok: false, reason: 'error', error: `GitHub returned ${res.status}` };
262+
// translators: %s: an HTTP status code, such as 500.
263+
return { ok: false, reason: 'error', error: sprintf(__('GitHub returned %s'), res.status) };
259264
}
260265
// Only judged when GitHub states the scopes: the header is how OAuth
261266
// tokens carry them, and its absence (some token types omit it) is not
@@ -265,7 +270,12 @@ async function fetchViewer(token, deps = {}) {
265270
return {
266271
ok: false,
267272
reason: 'insufficient-scope',
268-
error: 'GitHub granted this sign-in less access than opening a pull request needs — an older authorization of this app was likely reused. Revoke "WordPress Contributor Toolkit" under github.com → Settings → Applications, then sign in here again.'
273+
error: sprintf(
274+
// translators: 1: the name of this app's GitHub application, WordPress Contributor Toolkit. 2: where to revoke it, github.com → Settings → Applications.
275+
__('GitHub granted this sign-in less access than opening a pull request needs — an older authorization of this app was likely reused. Revoke "%1$s" under %2$s, then sign in here again.'),
276+
'WordPress Contributor Toolkit',
277+
'github.com → Settings → Applications'
278+
)
269279
};
270280
}
271281
return { ok: true, login: String(res.json.login) };

‎src/github-pr.cjs‎

Lines changed: 62 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
* patch file, which is always still there — but not the same explanation.
3333
*/
3434

35+
const { __, _n, sprintf } = require('@wordpress/i18n');
3536
const { getJson, postJson } = require('./github-http.cjs');
3637
const { classifyHttpFailure } = require('./patch-sources.cjs');
3738
const { ticketUrl } = require('./renderer/trac-ticket.cjs');
@@ -159,8 +160,22 @@ function classifyFailure(res) {
159160
* @return {{ok: false, reason: string, error: string}}
160161
*/
161162
function failure(res, what) {
162-
const detail = res.json && res.json.message ? res.json.message : `GitHub returned ${res.status}`;
163-
return { ok: false, reason: classifyFailure(res), error: `${what}: ${detail}${describeResponse(res)}` };
163+
let detail = res.json && res.json.message;
164+
if (!detail) {
165+
// translators: %s: an HTTP status code, such as 500.
166+
detail = sprintf(__('GitHub returned %s'), res.status);
167+
}
168+
return {
169+
ok: false,
170+
reason: classifyFailure(res),
171+
error: sprintf(
172+
// translators: 1: what failed, such as "Could not create the branch". 2: GitHub's reason, which is in English. 3: technical details in square brackets, for GitHub support.
173+
__('%1$s: %2$s%3$s'),
174+
what,
175+
detail,
176+
describeResponse(res)
177+
)
178+
};
164179
}
165180

166181
/**
@@ -276,7 +291,12 @@ async function ensureFork({ token, login }, deps = {}) {
276291
const notAFork = () => ({
277292
ok: false,
278293
reason: 'error',
279-
error: `You already have a repository named ${up.repo} that is not a fork of ${up.owner}/${up.repo}, so there is nowhere to push this. The patch file still works.`
294+
error: sprintf(
295+
// translators: 1: a repository name, such as wordpress-develop. 2: the repository it should be a fork of, such as WordPress/wordpress-develop.
296+
__('You already have a repository named %1$s that is not a fork of %2$s, so there is nowhere to push this. The patch file still works.'),
297+
up.repo,
298+
`${up.owner}/${up.repo}`
299+
)
280300
});
281301

282302
// Ready means the fork's own trunk ref answers — not that the repo
@@ -295,7 +315,7 @@ async function ensureFork({ token, login }, deps = {}) {
295315
return { ok: false, reason: 'offline', error: String(e && e.message ? e.message : e) };
296316
}
297317
if (existing.status === 200 && !isOurFork(existing.json)) return notAFork();
298-
if (existing.status !== 200 && existing.status !== 404) return failure(existing, 'Could not check for your fork');
318+
if (existing.status !== 200 && existing.status !== 404) return failure(existing, __('Could not check for your fork'));
299319

300320
let created = false;
301321
if (existing.status === 404) {
@@ -305,7 +325,11 @@ async function ensureFork({ token, login }, deps = {}) {
305325
} catch (e) {
306326
return { ok: false, reason: 'offline', error: String(e && e.message ? e.message : e) };
307327
}
308-
if (forked.status !== 202 && forked.status !== 200) return failure(forked, `Could not fork ${up.repo}`);
328+
if (forked.status !== 202 && forked.status !== 200) return failure(forked, sprintf(
329+
// translators: %s: a repository name, such as wordpress-develop.
330+
__('Could not fork %s'),
331+
up.repo
332+
));
309333
created = true;
310334
}
311335

@@ -319,7 +343,7 @@ async function ensureFork({ token, login }, deps = {}) {
319343
// 404 is the fork still initialising, the state this loop exists to
320344
// wait out. Anything else — a token revoked mid-wait, a spent rate
321345
// limit — will not resolve by waiting, so it is reported as itself.
322-
if (ref.status !== 404) return failure(ref, 'Could not read your fork');
346+
if (ref.status !== 404) return failure(ref, __('Could not read your fork'));
323347
await wait(FORK_POLL_INTERVAL_MS);
324348
}
325349
} catch (e) {
@@ -329,7 +353,7 @@ async function ensureFork({ token, login }, deps = {}) {
329353
return {
330354
ok: false,
331355
reason: 'error',
332-
error: 'Your fork is still being set up on GitHub — for a repository this size that can take a few minutes. Try again shortly; the patch file still works.'
356+
error: __('Your fork is still being set up on GitHub — for a repository this size that can take a few minutes. Try again shortly; the patch file still works.')
333357
};
334358
}
335359

@@ -374,7 +398,11 @@ async function resolveBase({ token, login, baseSha }, deps = {}) {
374398

375399
const ref = await get(`${repo}/git/ref/heads/${base}`, { token });
376400
if (ref.status !== 200 || !ref.json || !ref.json.object || !ref.json.object.sha) {
377-
return failure(ref, `Could not read your fork’s ${base}`);
401+
return failure(ref, sprintf(
402+
// translators: %s: a branch name, such as trunk.
403+
__('Could not read your fork’s %s'),
404+
base
405+
));
378406
}
379407
const tip = String(ref.json.object.sha);
380408
return { ok: true, sha: tip, exact: tip === baseSha };
@@ -475,13 +503,17 @@ async function createTree({ token, login, baseTreeSha, files }, deps = {}) {
475503
encoding: 'base64'
476504
}, { token });
477505
if (blob.status !== 201 || !blob.json || !blob.json.sha) {
478-
return failure(blob, `Could not upload ${file.path}`);
506+
return failure(blob, sprintf(
507+
// translators: %s: the path of a file in the change.
508+
__('Could not upload %s'),
509+
file.path
510+
));
479511
}
480512
entries.push({ path: file.path, mode: file.mode || DEFAULT_MODE, type: 'blob', sha: blob.json.sha });
481513
}
482514

483515
const tree = await post(`${repo}/git/trees`, { base_tree: baseTreeSha, tree: entries }, { token });
484-
if (tree.status !== 201 || !tree.json || !tree.json.sha) return failure(tree, 'Could not assemble the change');
516+
if (tree.status !== 201 || !tree.json || !tree.json.sha) return failure(tree, __('Could not assemble the change'));
485517
return { ok: true, sha: String(tree.json.sha) };
486518
} catch (e) {
487519
return { ok: false, reason: 'offline', error: String(e && e.message ? e.message : e) };
@@ -514,7 +546,7 @@ async function commitAndBranch({ token, login, ticketId, message, treeSha, paren
514546
tree: treeSha,
515547
parents: [parentSha]
516548
}, { token });
517-
if (commit.status !== 201 || !commit.json || !commit.json.sha) return failure(commit, 'Could not create the commit');
549+
if (commit.status !== 201 || !commit.json || !commit.json.sha) return failure(commit, __('Could not create the commit'));
518550
const sha = String(commit.json.sha);
519551

520552
let lastRes = null;
@@ -530,16 +562,20 @@ async function commitAndBranch({ token, login, ticketId, message, treeSha, paren
530562
return {
531563
ok: false,
532564
reason: 'error',
533-
error: `Your fork is still being set up on GitHub. Try again in a minute — the patch file still works.${describeResponse(ref)}`
565+
error: sprintf(
566+
// translators: %s: technical details in square brackets, for GitHub support.
567+
__('Your fork is still being set up on GitHub. Try again in a minute — the patch file still works.%s'),
568+
describeResponse(ref)
569+
)
534570
};
535571
}
536572
// 422 is how GitHub says the reference already exists — the only
537573
// status worth another name. Anything else is a real failure and
538574
// retrying it nine more times would only slow down the report.
539-
if (ref.status !== 422) return failure(ref, 'Could not create the branch');
575+
if (ref.status !== 422) return failure(ref, __('Could not create the branch'));
540576
lastRes = ref;
541577
}
542-
return failure(lastRes, 'Could not find an unused branch name');
578+
return failure(lastRes, __('Could not find an unused branch name'));
543579
} catch (e) {
544580
return { ok: false, reason: 'offline', error: String(e && e.message ? e.message : e) };
545581
}
@@ -568,7 +604,7 @@ async function createPullRequest({ token, login, branch, title, body }, deps = {
568604
base: baseBranchFor(deps.project),
569605
maintainer_can_modify: true
570606
}, { token });
571-
if (res.status !== 201 || !res.json || !res.json.html_url) return failure(res, 'Could not open the pull request');
607+
if (res.status !== 201 || !res.json || !res.json.html_url) return failure(res, __('Could not open the pull request'));
572608
return { ok: true, url: String(res.json.html_url), number: Number(res.json.number) };
573609
} catch (e) {
574610
return { ok: false, reason: 'offline', error: String(e && e.message ? e.message : e) };
@@ -609,7 +645,7 @@ async function openPullRequest({ token, login, ticketId, baseSha, files, title,
609645
const at = (stage, result) => ({ ...result, stage });
610646

611647
if (!files || files.length === 0) {
612-
return { ok: false, reason: 'empty', error: 'There are no changes to open a pull request with.', stage: 'collect' };
648+
return { ok: false, reason: 'empty', error: __('There are no changes to open a pull request with.'), stage: 'collect' };
613649
}
614650

615651
report('forking');
@@ -633,7 +669,15 @@ async function openPullRequest({ token, login, ticketId, baseSha, files, title,
633669
return {
634670
ok: false,
635671
reason: 'stale',
636-
error: `Trunk has moved under ${stale.clashes.length === 1 ? 'a file you edited' : 'files you edited'} (${stale.clashes.slice(0, 3).join(', ')}${stale.clashes.length > 3 ? ', …' : ''}). Update this site to the latest trunk, check your changes still apply, and try again.`,
672+
error: sprintf(
673+
// translators: %s: up to three file paths, separated by commas.
674+
_n(
675+
'Trunk has moved under a file you edited (%s). Update this site to the latest trunk, check your changes still apply, and try again.',
676+
'Trunk has moved under files you edited (%s). Update this site to the latest trunk, check your changes still apply, and try again.',
677+
stale.clashes.length
678+
),
679+
stale.clashes.slice(0, 3).join(', ') + (stale.clashes.length > 3 ? ', …' : '')
680+
),
637681
stage: 'syncing'
638682
};
639683
}
@@ -649,7 +693,7 @@ async function openPullRequest({ token, login, ticketId, baseSha, files, title,
649693
return at('syncing', { ok: false, reason: 'offline', error: String(e && e.message ? e.message : e) });
650694
}
651695
if (baseCommit.status !== 200 || !baseCommit.json || !baseCommit.json.tree) {
652-
return at('syncing', failure(baseCommit, 'Could not read the base commit'));
696+
return at('syncing', failure(baseCommit, __('Could not read the base commit')));
653697
}
654698

655699
report('committing');

‎src/main.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1291,7 +1291,7 @@ ipcMain.handle('github:sign-in', async (event) => {
12911291

12921292
ipcMain.handle('github:open-pr', async (event, sitePath, options = {}) => {
12931293
if (!githubToken || !githubLogin) {
1294-
return { ok: false, reason: 'unauthorized', error: 'Sign in to GitHub first.', stage: 'auth' };
1294+
return { ok: false, reason: 'unauthorized', error: __('Sign in to GitHub first.'), stage: 'auth' };
12951295
}
12961296

12971297
// The ticket is read from this site's stored metadata rather than taken
@@ -1307,7 +1307,7 @@ ipcMain.handle('github:open-pr', async (event, sitePath, options = {}) => {
13071307
const project = projectTypeForSite(meta);
13081308
const ticketId = meta.tracTicket;
13091309
if (!ticketId) {
1310-
return { ok: false, reason: 'no-ticket', error: `Link a ${project.workItem.label} to this site first. A pull request has to cite one.`, stage: 'auth' };
1310+
return { ok: false, reason: 'no-ticket', error: project.cards.prNeedsWorkItem, stage: 'auth' };
13111311
}
13121312
const ownershipRefusal = await appliedPatchSubmissionRefusal(sitePath);
13131313
if (ownershipRefusal) return { ...ownershipRefusal, stage: 'ownership' };

0 commit comments

Comments
 (0)