Repository navigation
[Change] Put the apply panel above the site's other tickets - #472
Merged
Merged
Conversation
The three cards under the header read in the order a session actually goes: the ticket in hand, the work you can bring into it, and only then the other tickets this site is holding. "Other tickets on this site" sitting second pushed the apply panel below a list that grows with every ticket, so on a site with ten of them the field for a PR or a patch was off screen. Only the order changes. Both cards keep their render conditions, their contents and their single call site, so a card still appears exactly when it did before. The layout is pinned at the source rather than in the DOM, so the #240 test that asserted the list sat between the two now asserts it sits below them, and the guide sentence that said "between" says "below". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 2 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Collaborator
Author
|
@coderabbitai review |
|
This was referenced Sep 15, 2026
juanmaguitar
added a commit
that referenced
this pull request
Sep 16, 2026
## Why This is the **stable 1.1.0**. `v1.1.0-beta.1` shipped on 12 September and `v1.1.0-beta.2` on 14 September; between them they were downloaded 33 times across macOS, Windows and Linux, and no issue was filed against either. The v1.1.0 milestone is closed, 21 of 21. Shipping sooner rather than later is itself part of the reasoning: a site created by 1.0 cannot be written to by 1.1, so every day 1.0.1 remains the stable release is another day of sites being created that the next version can only read. Four commits reach this tag that no beta carried: the Linux target list narrowed to the one artifact that is actually published and probed (#467), the apply panel moved above the site's other tickets (#472), the applied-patch record now travelling with carried work (#473), and the sidebar heading renamed (#482). Three of the four are visible on screen, so the artifact gets a manual pass before the release is published rather than the packaging check alone. The version reaches two places a contributor sees, and both must carry the real build: electron-builder embeds it in the artifact names (`wordpress-contributor-toolkit-1.1.0-*`), and `src/logging.js` writes `app <version>` as the log's first line, so a problem report names the build it came from. That is why the bump merges before the tag. ## What changes Only the version. All three package-version fields in `package.json` and `package-lock.json` move together from `1.1.0-beta.2` to `1.1.0`. No dependency versions change, and there is no application behaviour change in this PR. ## How to test this Platforms: any for the suite; macOS or Windows for the artifact check that follows the merge. In the repository root: 1. `npm run lint` is clean and `npm test` passes. 2. `node -e "console.log(require('./package.json').version)"` prints `1.1.0`. 3. `python3 -c "import json;d=json.load(open('package-lock.json'));print(d['version'], d['packages']['']['version'])"` prints `1.1.0 1.1.0`. **What must not have happened:** no dependency version may change. `git diff trunk` shows exactly three changed lines, all of them the root package version. After merge, the release build must produce artifacts named `wordpress-contributor-toolkit-1.1.0-*`, and the first line of a fresh app log must read `app 1.1.0`. ## Risks and limitations No UI change. The risk is a mis-scoped edit in `package-lock.json`, where dependencies also carry version strings; the replacement was confined to the first 2000 bytes of the file, which holds the two root entries, and both files were re-parsed as JSON afterwards. Unlike the betas, this tag becomes `releases/latest`, so it is what the README badge and every "download the toolkit" link resolve to. ## Related Same shape as #397 (v1.0.1), #457 (v1.1.0-beta.1) and #466 (v1.1.0-beta.2). The Gutenberg stack for v1.2.0 is held as a draft at #468 until this tag is pushed, so it cannot reach trunk and ship inside a release it has never been tested in; #479 waits for the same reason. --- <details> <summary>Review outcome (required — see AGENTS.md)</summary> Version-only change, three lines. Verified locally on this branch: `npm run lint` clean, `npm test` 1454 pass, 0 fail, 2 expected skips; `git diff --stat` shows two files, three insertions, three deletions; both JSON files parse. Nothing to fix, nothing deferred. </details> <details> <summary>Screenshots or recording</summary> Nothing on screen changes. </details> 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_0121dia1RzfskutrzA8iSK1K Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
juanmaguitar
added a commit
that referenced
this pull request
Sep 17, 2026
…quests that fix it (#251) (#480) ## Why A Gutenberg site could be built, run and used to try pull requests, but it had no work item: the card that on a Core site holds the Trac ticket said that working on a GitHub issue comes in a later version, the linked-PR search only knew `wordpress-develop`, and a pasted pull-request URL was checked against the wrong repository. This is the PR that gives a Gutenberg site its issue. #251 has the case. ## What changes Two stack-review corrections: Gutenberg's conflict/no-base guidance now preserves the issue branch and offers a backup plus mentor help, instead of instructing deletion followed by an unavailable patch import. Its mentor handoff header now uses the site's provider and links the actual GitHub issue (`# Issue: …`); Core retains its Trac header. **Reading an issue.** `src/renderer/github-issue.cjs` is the counterpart of `trac-ticket.cjs`: a bare number, `#71234` or a pasted issue URL (anchor, query, trailing slash included) resolve to an id; a pull-request URL is refused by name, because pasting the PR instead of the issue it fixes is the obvious mistake; an issue on another repository is refused, so a site cannot cite a number from a project it is not a checkout of; the URL branch gets the same bounds check as the typed one. `src/work-item.cjs` chooses between that parser and Trac's from the site's type, defaulting to Trac so a site made before project types existed keeps its behaviour with no migration. The registry holds what is true of one target (label, browse link, branch namespace); the provider holds what is true of one kind of work item. **Linking one.** `sites:set-ticket` parses through the site's provider. The stored key stays `tracTicket` for both kinds: renaming it touches every read on the card, the switcher and the patch handlers for no behaviour, and the decision is recorded above the handler. The branch is `issue/71234`, not `ticket/71234`: the noun the upstream uses, and a contributor reading `git branch` in their own client sees it. Core keeps `ticket/` untouched, so no existing site moves; every read in `ticket-branches.js` accepts both namespaces (a branch is read by its name alone), only the writers take the prefix, and a test walks the list against the registry and against the changes note's own grammar so a third namespace cannot be added to one side only. **The pull requests that fix it.** The linked-PR search goes to the site's own repository, and a row survives on the citation its work-item kind uses: on Gutenberg the forms GitHub itself reads as "for this issue" (`Fixes #71234`, `Closes WordPress/gutenberg#71234`, or the issue URL), so the list agrees with the issue page's own linked pull requests; a bare number in prose does not count, for the reason it never did on Trac. The cache is keyed per work item, and a work item is a number in a repository: Trac ticket 62281 and Gutenberg issue 62281 are different things. Core keeps the bare key it always had, so nothing cached is thrown away. The pasted-URL guard refuses a pull request from another repository by that repository's name. **Patch layout.** The path rewrite that steers a pre-`src/` Trac patch under `src/` was wordpress-develop's and ran on every patch. The registry's `patch.layout` now picks the mapper: `src-layout` is the rewrite as it was, `repo-relative` leaves every path as named, an unknown value throws rather than guessing. The layout rides from the site through preview, apply and the undo after a failed persist, so what is shown is what is written. On a Gutenberg site nothing reaches it yet (the picker and the attachments are hidden there); it is here so the layer is right before a caller arrives. **The card.** It shows on every site and takes its words from the provider and the registry: **GitHub issue**, *Working on issue #71234*, **Link issue**, **Open on GitHub**, the good-first-issues link. What is Trac's alone stays on Core sites: **Read details from Trac**, the attachments block, **Attach to Trac**, the patch-file picker. The pure copy modules take the site's noun rather than restating it (changes note, tickets card heading, trunk-moved notice and its refusals, next-action hint), the switch sentences name an `issue/` branch by its number, and the applied-PR box says the issue's work returns on revert rather than "returns to trunk". The placeholder card and the picker sentence that said issues come later are gone. **Docs:** `docs/guide/gutenberg-issues.md`, the counterpart of the Trac page, in the nav; the four pages that said issues come in a later version now point at it. **Left for PR 5 on purpose:** the **Open a pull request** card on a Gutenberg site still says it is not available and offers the patch file; `github:open-pr` still refuses a non-Core site; the "Link a Trac ticket to this site first" hint in that card is Core-only until then. `wpct://issue/N` is PR 6. ## How to test this Correction regressions, **either platform**, current head; from the repository root: `node --test tests/unit/ticket-trunk-notice.test.cjs tests/unit/ipc-wiring.test.cjs tests/unit/patch-provenance.test.cjs`. Both corrections were checked red before the fix and green afterwards. The recovery test covers conflict with/without named paths and unknown base; it forbids the unlink/delete/reapply sequence for Gutenberg. The real handoff export test reads the saved file, checks the correct issue URL and absence of Trac, and verifies the patch still parses; its Core counterpart preserves the old header. Platforms: **either**; nothing here spawns anything platform-specific beyond what #471 already covered. Build: the Buildkite artifact for the current head. The Gutenberg journey (`tests/e2e/journeys/gutenberg-site.spec.js`) walks the whole path on a Git fixture: the card asks for an issue, a pull-request URL is refused by name before anything is written, an issue URL links and lands on `issue/71234`, a `wordpress-develop` pull-request URL is refused, a `WordPress/gutenberg` one is checked out, the applied box says the issue's work returns, revert lands back on the issue's branch with its tree restored, and a `wpct://ticket` link is still turned away. `npm run test:e2e`: 30 passed on macOS at the current head. By hand, either platform, on a Gutenberg site (needs the network for GitHub): 1. In the **GitHub issue** card, paste `https://github.com/WordPress/gutenberg/pull/4496` and click **Link issue**. Expected: "That is a pull request. Link the issue it fixes instead."; `git branch` in the checkout is unchanged. 2. Paste `https://github.com/WordPress/wordpress-develop/issues/1` and link. Expected: "Only WordPress/gutenberg issues can be linked here." 3. Type a real open issue's number and link. Expected: *Working on issue #N*, **Open on GitHub** opens it, `git branch` shows `issue/N` and no `ticket/N`; no **Read details from Trac**, no **Trac attachments**. 4. Under **Linked pull requests**, expected: the pull requests the issue page lists under *linked pull requests*, and nothing that merely mentions the number. Pick one with **Apply…**: the preview lists its files, **Apply and rebuild** lands on `pr/NNNN`, the green box reads *PR #NNNN is applied* with *Your issue changes are saved separately and return when you revert this PR.*; **Revert this PR** returns to `issue/N`. 5. In **Check out a pull request**, paste a `wordpress-develop` pull-request URL. Expected: "Only WordPress/gutenberg pull requests can be applied here." 6. **Unlink**, then link the same issue again. Expected: *Your issues on this site* lists it in between; linking again switches back with no new branch. 7. On a Core site, link a Trac ticket, apply a Trac attachment, check out a `wordpress-develop` pull request. Expected: exactly what it was; the linked-PR cache key for a Core ticket is unchanged (`ticketPatches:<id>` in the store). **What must not have happened:** a Core site changing in any way (step 7); a `ticket/` branch appearing on a Gutenberg site (step 3); a `wordpress-develop` pull request cited by a Trac ticket showing under a Gutenberg issue of the same number (step 4, if such a number exists). ## Risks and limitations Gutenberg still has no patch importer or automatic conflict resolver. Recovery deliberately retains the branch and directs the contributor to save a backup and ask a mentor; simply adding import would label the restored work as an applied external patch and block submission as their own work. - Review outcome (self-review against `.github/instructions/code-review.instructions.md`, fresh agent context, head `98650dd` on base `juanmaguitar/251-contribute-to`, completed over the full diff; lint and unit suite clean): 5 [fix here], all fixed in `f96db22` (the noun threaded into the trunk-moved refusals, the action gate, the dirty-trunk question and the patch-review heading; the field's accessible name and the attach URL read from the provider; callerless `defaultPrTitle` and the diff fetch's repo option removed; `bodyCitesIssue` narrowed to GitHub's own rule). 1 [follow-up]: `parseIssueRef`'s default repository duplicates the registry's Gutenberg entry; every real caller threads the repository, so it only matters for a second GitHub-issue target. Two style notes: `showTracCards` now means three things at four call sites (the "no PR flow yet" one gets its own flag in PR 5). CodeRabbit: one review on `bdaf6a5`, 1 low finding (the undo test never sent the undo), fixed in `b354059`. - Found by hand on macOS after opening (2026-09-16), fixed in `6ee65ec`: linking an issue by hand armed #292's auto-read, which opened the Trac window on the Core ticket with the same number and painted its summary, status, milestone and attachments note under the issue. The auto-read and the scraped facts are Trac's alone in the renderer now, and `trac:list-attachments` refuses a non-Trac site before the window can open. Reproduced first by a journey assertion (a stubbed Trac records zero scrapes and none of its facts show) and an ipc-wiring test. - `git:fetch-pr-diff` stays Core-only and says so: nothing has invoked it since pull requests became checkouts (#458). Removing the handler and its preload entry is a follow-up (the packaged smoke test pins the API surface). - The renderer's changes note holds its own copy of the two branch namespaces as a regex: `ticket-branches.js` owns the list but reaches for Git and cannot be bundled. A test walks the list against the regex so they cannot drift apart unnoticed. - `bodyCitesIssue` matches GitHub's own linking rule: a closing keyword, followed by `#N`, `owner/repo#N` or the issue URL. A pull request that says "Related to #N" or drops the URL in prose is not listed, on GitHub or here; a contributor looking for it can paste its URL. - The "Link a Trac ticket to this site first" message in the pull-request card, and the card itself on a Gutenberg site, are PR 5's. - `sites:add` (adopting a folder) still records every adopted checkout as Core; carried over from #476. ## Related Part of #251. Stacked on #476 (rebased with it onto trunk `576d218` on 2026-09-16; the tickets card #472 moved keeps its place and shows on every site). 🤖 Generated with [Claude Code](https://claude.com/claude-code) <details> <summary>Stack correction review — 2026-09-17</summary> 0 remaining [fix here] · 0 new [follow-up] in this correction pass. Both recovery and mentor-attribution findings are resolved. - Current PR head: `7b7db146c92022fd5960d6e677f05406bc97831b`; configured base head: `08315156268f34ab94f6f0c81192f02642d48ce0`. Previous PR head: `4915881a062f65fe8e3e6fe5fac8d6e34e806066`. - Reviewer: fresh Codex context `/root/review_fixes`. Completed five-dimension review of the correction delta from previously reviewed integrated head `bc8c4fda3fb78d51cd6f3abd5fc1b0f1e89c0095` to `c8c86677491012f9c9d4e99fe43b4ceff1c9647b`, then re-reviewed through final stack head `34550eec7a9031d2d5f03116801a60e313fa0559`. The additional runtime-deletion finding was fixed and verified; the original three findings remain resolved. Earlier review records above are historical. - Validation on the combined stack: lint; Node unit/integration/IPC suite (1,547 passed, two runtime skips); Electron suite (1,549 passed); 33 macOS journeys; documentation build. The real Playground restart/deletion regression passed on macOS. Windows CI results must be read on these new heads; this pass does not claim a fresh Windows real-setup run. - CodeRabbit's earlier check is not review coverage for these new heads. This local correction review is the review of record for the delta; no new bot review was requested. </details> <details> <summary>Windows journey synchronization fix — 2026-09-17</summary> The PR checkout journey now waits for the pull-request input to reappear and become enabled after restoration, before inspecting Git, files and persisted settings. All previous assertions remain; no production code changed. Current head: `6b9598c44f171b4899abd945853f816556377519`; configured base: `08315156268f34ab94f6f0c81192f02642d48ce0`. Earlier review records are historical. Fresh reviewer `/root/review_ci_sync` completed the five-dimension review of the four-line correction against `7b7db146c92022fd5960d6e677f05406bc97831b`, published as `6b9598c44f171b4899abd945853f816556377519`: 0 [fix here], 0 [follow-up]. The rebased integrated tree differs from the previously reviewed stack only by those same four test lines. Reproduction: temporarily delay the real leave-PR handler after checkout and before metadata persistence; the original journey fails with expected ticket/60001, received pr/7. With the new wait, the same delayed run passes. The diagnostic delay was removed before committing. On the corrected #480 head, lint and build pass, Node tests report 1,539 passed and 2 runtime skips, and all 31 macOS journeys pass. From the repository root on macOS or Windows at the current head, run `npm run build:once && npm run test:e2e`. New-head macOS and Windows journeys and packaged smoke pass, as do lint, docs and macOS unit tests. The Windows Electron unit suite is still running. No new CodeRabbit review was requested; its check says automatic reviews are disabled. The integrated #486 stack also passes all 33 journeys locally on macOS. </details> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The three cards under the site header read in the order a session actually goes: the ticket in hand, the work you can bring into it, and only then the other tickets this site is holding. Other tickets on this site sitting second pushed Apply a patch or PR below a list that grows with every ticket linked, so on a site with ten of them the field for a PR or a patch is off screen and has to be scrolled to, past rows about work you are not doing right now.
What changes
Only the order of two sibling cards in the site view. Both keep their render conditions, their contents and their single call site, so each card still appears exactly when it did before and says exactly what it said before.
The layout is pinned at the source rather than in the DOM, because
src/renderer/index.jsxmounts at module scope and no test can reach it. So the #240 test that asserted the list sits between the Trac ticket card and the patch card now asserts it sits below both, and the guide sentence that said "between" says "below".How to test this
Platforms: any. Nothing platform-specific is touched: no spawn, no path handling, no IPC.
Starting state:
What must not have happened:
tests/unit/ticket-branch-list.test.cjscovers the order at the source. Its assertion fails on trunk and passes here, and the previous assertion does the reverse, so it is the order that is pinned and not just the three headings being present.Risks and limitations
docs/public/screenshots/site-with-tickets.pngstill shows the old order. It is a hand-captured shot, not one of the slugs inscripts/screenshots/shots.cjs, so it cannot be regenerated by running the capture script. The caption was reworded so it does not assert the old order, but the image and the prose atdocs/guide/ticket-branches.mdstill disagree until someone re-captures it.src/renderer/ticket-branch-list.cjsstill calls that state "the primary way to start", left as written because it describes what the rows mean, not where they sit, but it is the sentence to revisit if the unlinked site turns out to want the opposite order.Related
Follow-up to #240, which gave the list a card of its own in the position this changes.
Design decisions and alternatives considered
Moving the block rather than reordering with CSS: the cards are plain siblings in one children array, so the source order is the reading order and a
order:property would put the truth in two places, with the source-pinned test reading the wrong one.Not touched: the card between the two states, the render conditions, and the single
renderBranchRows(call site that #240 established. Anything more than the move would have made a layout change into a behaviour change.Review outcome (required , see AGENTS.md)
.github/instructions/code-review.instructions.md; reviewed head2dcfbd4/ base56f6857; evidence:npm run lintclean,node --run test1451 pass / 2 skipped / 0 fail; outcome 0 [fix here] · 0 [follow-up], no findings across the five dimensions. Three style notes, two acted on: the stale narration at thebranchRowscall site was rewritten to the new order, and the stale screenshot is recorded under Risks and limitations above. The third (the "primary way to start" comment inticket-branch-list.cjs) was left, with the reason in that same section.2dcfbd4 → 95eedf1, the comment rewrite only, no change to the diff that was reviewed; lint and the suite re-run clean on the new head.@coderabbitai reviewwas requested on head95eedf1at 08:37 UTC on 15 Sep 2026 and answered "Review rate limited" without naming a wait, so no review of this head exists. Automatic review is off in this repository and the free open-source plan covers roughly one review an hour for the whole WordPress organisation. The local agent review above is therefore the review of record for95eedf1.Implementation notes
src/renderer/index.jsx: the{skipInit && ticketsCard ? ... }block moves below the{skipInit && (!pullRequest || isApplying || Boolean(applyError)) ? ... }block. Both are siblings in the same children array, under the sameskipInitgate, andticketsCardandbranchRowsare component-scope values computed well before either position, so no hook order and no conditional scope changes.Screenshots or recording
The change is the order of two cards in the site view; the before/after is the reading order described in How to test this. The guide screenshot that shows the old order is called out under Risks and limitations.
🤖 Generated with Claude Code