Backup: wire the modernized dashboard to real REST + Gates - #48889
Backup: wire the modernized dashboard to real REST + Gates#48889dhasilva wants to merge 11 commits into
Conversation
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
github-actions
Bot
added
[Package] Backup
[Plugin] Backup
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! Jetpack plugin: No scheduled milestone found for this plugin. If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Backup plugin: No scheduled milestone found for this plugin. If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
Code Coverage SummaryCoverage changed in 6 files. Only the first 5 are listed here.
27 files are newly checked for coverage. Only the first 5 are listed here.
Full summary · PHP report · JS report If appropriate, add one of these labels to override the failing coverage check:
Covered by non-unit tests
|
|
This PR has been marked as stale. This happened because:
If this PR is still useful, please do a [trunk merge or rebase](https://github.com/Automattic/jetpack/blob/trunk/docs/git-workflow.md#keeping-your-branch-up-to-date) and otherwise make sure it's up to date and has clear testing instructions. You may also want to ping possible reviewers in case they've forgotten about it. Please close this PR if you think it's not valid anymore — if you do, please add a brief explanation. If the PR is not updated (or at least commented on) in another month, it will be automatically closed. |
Adds the @tanstack/react-query infrastructure with no behavior change. DashboardLayout now wraps children in a QueryClientProvider so every screen in every route shares a module-scope singleton client — caches survive navigation between routes. Phase 2 setup commit. Capability gates and real REST hooks follow. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Introduces the per-feature REST bridges pattern with a thin
Rest_Controller that registers routes only when the modernization
filter is on. Capabilities_Bridge proxies /sites/{id}/rewind to
expose hasBackupPlan + planSlug for the React layer.
Gates mounts inside DashboardLayout between the QueryClientProvider
and the body slot. Top-to-bottom: loading → not-connected →
secondary-admin → no-plan → children. All three routes inherit
gating for free.
Connection state comes from window.JP_CONNECTION_INITIAL_STATE,
emitted inline on the modernized Backup admin page from
class-jetpack-backup.php (the modernized enqueue path skips the
legacy render_script call, so we need to put the global up ourselves).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces useMockActivityLog with useActivityLog backed by an
apiFetch -> bridge -> WPCOM /sites/{id}/activity/rewindable chain.
A small normalize/activity-log.ts maps WPCOM's rewindable-activity
shape (gridicon, content.text, object.backup_stats) onto the
ActivityItem discriminated union the UI already consumes.
The hook returns `totalItems` in addition to `totalPages` because the
trunk ActivityList drives DataViews' `paginationInfo` from both. The
WPCOM endpoint doesn't accept `page` or `search`, so the hook fetches
a 100-item window and filters/paginates client-side — DataViews
remains the single source of truth for the visible slice.
Overview's right-pane lookup swaps from the fixture's
findActivityById to getCachedActivityById, which reads the same
single-window cache the list populates. The default selection
(newest backup) is derived from the cache the same way.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds the File_Browser_Bridge (ls + path-info + file-content), each
proxying WPCOM /sites/{id}/rewind/backup/* via as_user. The file-content
endpoint resolves the signed stream URL server-side and caps the
returned body at 64 KB so previews can't balloon the REST response.
React side: useFileTree replaces useMockFileTree, useFileContents
replaces the static findContents() fixture lookup. The rewindId prop
on FileBrowser is now actually consumed and threaded through every
NodeRow so each folder fetches its own children.
FileInfoCard no longer reads metadata from fixture nodes — usePathInfo
fetches size + mime + last-modified for the open file, and
useFileContents pulls the preview body only when the mime type is
text-shaped. FileNodeFile's metadata fields drop to optional since
WPCOM's /ls endpoint only returns name + type.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
useDownload replaces useMockDownload, driving the same
idle → submitting → progress → success | error state machine off
a TanStack useMutation (initiate) + polled useQuery (status).
The bridge proxies WPCOM's /rewind/backups/{rewindId}/prepare-download
and /rewind/backups/{rewindId}/downloads/{id} endpoints.
The screen now derives its "download point" timestamp directly from
the rewindId (it's unix-seconds) instead of resolving it through the
activity-log fixture — Download / Restore can stand alone without
relying on Overview having populated its cache first.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
useRestore replaces useMockRestore, driving the same state machine off
a TanStack useMutation (initiate) + polled useQuery (status). The
bridge proxies WPCOM's v1 /activity-log/{site}/rewind/to/{rewindId}
and /rewind/{restoreId}/restore-status endpoints, with the verbatim
comment explaining why the rewind endpoint must sign as_user
(blog-token auth is rejected by WPCOM with "That API call is not
allowed for this account").
Same rewindId → unix-seconds derivation as the Download screen, so
Restore stands alone without depending on Overview's cache.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Removes the last mock-mode artifacts: useIsMockMode, the dev banner component, the four mock hooks, the three fixture files, and the <DevModeBanner /> mount inside DashboardLayout. The modernized Backup dashboard now only renders real data via the jetpack/v4/* bridges, with <Gates> handling the not-connected and no-plan fallbacks. This completes Phase 2 of the Backup modernization. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
One test class per bridge: - Rest_Capabilities_Bridge_Test (2 tests, 2 assertions) - Rest_Activity_Log_Bridge_Test (2 tests, 2 assertions) - Rest_File_Browser_Bridge_Test (4 tests, 4 assertions) - Rest_Download_Bridge_Test (3 tests, 4 assertions) - Rest_Restore_Bridge_Test (3 tests, 4 assertions) Each class confirms the routes only register when the modernization filter is on and that the manage_options permission_check rejects subscriber-level access with a 403. Upstream WPCOM calls aren't exercised here — they need network and a real connection. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
REST bridges: - Centralize the not-connected check via Rest_Controller::get_blog_id_or_error so every bridge short-circuits with a 412 instead of building /sites/0/… - Require a user-level WPCOM connection in Rest_Controller::permission_check, matching the jetpack-activity-log package's pattern - Rename Activity_Log_Bridge's route from /jetpack/v4/activity-log → /jetpack/v4/site/rewindable-activity to avoid colliding with the jetpack-activity-log package, which owns that route inside the Jetpack plugin - Cap File_Browser_Bridge's preview proxy at the HTTP transport layer (limit_response_size) so multi-GB blobs can't be buffered into PHP memory before truncation, and validate the signed URL with wp_http_validate_url - Drop the raw upstream WPCOM body from Restore_Bridge's WP_Error envelope — the React layer only reads err.message React layer: - Convert getCachedActivityById / getCachedDefaultBackupRewindId to useQuery-backed hooks so Overview's default selection reconciles to the newest backup the moment the activity-log window resolves - Collapse the activityLog cache key to a single shared activityLogWindow key the hook always uses - Drop the raw error_code fallback in useRestore — never surface machine identifiers like "checksum_mismatch" to users - Refresh stale doc comments (DashboardLayout no longer mentions the dev-mode banner; FileBrowser references useFileTree instead of useMockFileTree) Changelog: - Align the Jetpack plugin entry with Phase 1: Type: other for the flag-gated rollout Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…es + review cleanup
The RestoreItemsChecklist selection was collected into local state on the
Restore and Download screens but never sent to the bridge, so an unchecked
"Plugins" or "Themes" was silently restored anyway. Thread `RestoreItems`
through `useRestore` / `useDownload` → `initiateRestore` / `initiateDownload`
as the mutation variable, and surface it to the bridges as `data: { types }`
(the keys already match WPCOM's `types` payload).
Also clears the review suggestions from the same pass:
* `useFileContents` browser path now UTF-8 encodes via TextEncoder before
btoa, so non-ASCII manifest paths (accented or CJK upload filenames)
no longer throw InvalidCharacterError.
* `useConnection` drops its empty-deps useMemo and reads
JP_CONNECTION_INITIAL_STATE on every render — single property lookup,
removes any risk of capturing a pre-emit empty snapshot if load order
ever shifts.
* `query-client.ts` docstring corrected: each wp-build route is its own
bundle so the cache is per-route, not shared across navigation. The
design doesn't depend on cross-route reuse, but the comment shouldn't
claim otherwise.
* `Restore_Bridge::initiate_restore` returns the same user-facing
"Could not start the backup restore." string in both failure branches
instead of the diagnostic "Restore response missing restore id."
* `Activity_Log_Bridge` drops the declared-but-unused `aggregate`,
`after`, `before` args (and mirrors the cleanup in
`data/api/activity-log.ts`'s FetchArgs) — the hook only ever passes
`number`.
* Download success link gains `download` + `rel="noreferrer"` so the
signed URL triggers a download instead of navigating away when WPCOM
omits Content-Disposition.
PHP + JS tests pass; phan unchanged (only the pre-existing build/ artifact
warning); eslint clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Five fixes raised in /jetpack-review-pr feedback:
* Activity list pagination is now real server-side pagination instead
of a 100-item client-side window. The bridge forwards `number` and
`page` to WPCOM; `useActivityLog` uses `keepPreviousData` for smooth
page transitions and reads `totalItems`/`totalPages` straight from
the envelope. The cache key is per-page (`activityLogPage(page,
pageSize)`); `activityLogRoot()` is the family prefix for cross-page
lookup. Overview lifts view state so `<RightPane>`'s
`useActivityById` subscribes to the same paginated query, falling
through to other cached pages via `getQueriesData` if the row was
loaded on a different page. The 100-item cap was a relic of the
mocked-data phase. DataViews `search` is disabled — WPCOM's
`/rewindable` endpoint doesn't accept a search param and the
rewindable list is homogeneous enough that hand-rolling client-side
search wasn't worth it.
* `useConnection` no longer spins forever when
`JP_CONNECTION_INITIAL_STATE` is missing. The "any key present"
loaded heuristic is replaced with always-loaded + a dev-mode
console warning, so `<Gates>` falls through to
`<NotConnectedScreen>` instead of staying on the skeleton.
* `<FileInfoCard>` surfaces `useFileContents` errors with a distinct
muted message ("Preview could not be loaded for this file. It may
no longer be available in storage."). The mime-mismatch branch
keeps its generic "Preview unavailable" copy. Drops the inert
per-file Download/Restore buttons — per-file actions aren't on
this phase's roadmap and ghost affordances are worse than no
affordance.
* `useRestore` and `useDownload` surface `statusQuery.error` as a
terminal `error` phase ("Lost connection while restoring." /
"Lost connection while preparing download.") instead of leaving
the UI stuck on the last known progress percent with no message
and no way out — important for Restore in particular, which is
irreversible.
Tests + lint + tsc clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
34ea3b9 to
c332789
Compare
dhasilva
added
[Status] Needs Review
Code review — thorough pass (3,954 lines, 3 projects)Verdict: minor issues — no blockers found. The PHP side is well-guarded (capability + user-connection permission checks, flag-gated route registration, careful proxying of WPCOM-signed URLs); everything below is a frontend error-state or consistency issue. Tests were run locally from the PR head: Findings
Test coverage gaps
Checked and clear
Generated by Claude. |
1 participant
Fixes # n/a
Proposed changes
rsm_jetpack_ui_modernization_backup) to real REST endpoints. The mocked-data scaffold from Phase 1 is replaced with TanStack Query 5 hooks talking to a thin set of PHP REST bridges (Capabilities,Activity_Log,File_Browser,Download,Restore) underprojects/packages/backup/src/rest/. The legacy dashboard remains byte-identical when the modernization filter is off.<Gates>mounts inside<DashboardLayout>between theQueryClientProviderand the body: top-to-bottom loading → not-connected → secondary-admin → no-plan → children. Connection state is read fromwindow.JP_CONNECTION_INITIAL_STATE, emitted inline byJetpack_Backup::render_connection_initial_stateonadmin_print_scriptspriority 1 — the modernized enqueue path short-circuits the legacyConnection_Initial_State::render_script()call, so without this<Gates>would sit on its loading skeleton forever. We avoid pulling in@automattic/jetpack-connectionbecause its barrel imports SCSS wp-build can't resolve cleanly.useActivityLog( { page, pageSize } ). Paging is server-side: the/jetpack/v4/site/rewindable-activitybridge forwardsnumberandpageto WPCOM, and the envelope'stotalItems/totalPagesfeed DataViews'paginationInfo— the same pattern the trunkjetpack-activity-logpackage uses against the parent/sites/{id}/activityendpoint.keepPreviousDatakeeps the current page on screen while the next one is in flight instead of flashing a spinner over the list. The right pane resolves its row viauseActivityById, which subscribes to the same page query the list uses and falls through to any other cached pages of the family before returning the "not found" state;useDefaultBackupRewindIdsubscribes to page 1 so first load preselects the newest backup the moment that page resolves (deduped with the list's own fetch when it's on page 1 at the default per-page).useFileTree. WPCOM's/rewind/backup/lsactually returnscontentsas a map keyed by filename, not an array. The real folder/leaf discriminator ishas_children, nottype— inside/wp-content, folders likelanguagesandmu-pluginsare reported astype: 'file'withhas_children: true. The virtualtype: 'wordpress'core-version markers are filtered out.useFileContents. VaultPress addresses file blobs by the per-entryperiod— the timestamp when that file last changed — not the parent backup's rewindId. The bridge query param is namedfile_periodaccordingly, with a long docblock capturing the failure mode if it's reverted (silently signs a URL for a non-existent snapshot, then 400 "File not found" on stream).FileNodeFilecarriesperiodand the volume-prefixedmanifestPath(e.g.f5:/wp-config.php) so the card has everything it needs without a separate metadata call.Capabilities_Bridgeproxies WPCOM's/sites/{id}/rewind/capabilities— the dedicated capabilities endpoint, same one the legacy plugin already uses. The previously-considered/sites/{id}/rewind?force=wpcomreturned a state-shaped response whosecapabilitieskey was missing on multi-product plans (Jetpack Complete), producing a false "no plan" gate.useRestore/useDownload(TanStackuseMutationto kick off + polleduseQueryfor status) with the sameidle → submitting → progress → success | errorstate machine the Phase 1 mocks defined. The Restore bridge signs againstrest/v1 /activity-log/{site}/rewind/to/{rewindId}as_user— blog-token auth is rejected by WPCOM withThat API call is not allowed for this account; comment in the bridge captures the original investigation. Both screens derive their displayed timestamp directly from the rewindId (Unix-seconds), so they don't depend on Overview having populated its cache first./sites/{id}/rewind/backup/path-inforeturns "No file found" for every input variant we synthesized and has no working caller anywhere in the monorepo.lastModifiedcomes from/ls'speriodand mime type is inferred from the file extension instead; the unused endpoint + hook + tests are dropped.item.statsnow reads fromentry.content.text("44 plugins, 23 themes, 1562 uploads, 99 posts, 18 pages") instead ofentry.object.backup_stats(a stringified JSON blob in production that was getting rendered verbatim).?jpb-mock=1developer affordance and the dev-mode banner now that everything is real. Removesuse-is-mock-mode, the fouruse-mock-*hooks, thedev-mode-bannercomponent, the three fixture files, and the<DevModeBanner />mount inDashboardLayout.toIntRewindId(3 cases) +normalize/activity-log.ts(6 cases); phpunit smoke-tests each bridge — confirms routes only register behind the modernization filter and reject subscriber-level access with 403.Related product discussion/links
Pagechassis) and PR3 Backup: modernized dashboard UI — Overview, Restore, Download (mocked) #48853 (Phase 1 — modernized Backup UI with mocked data), all merged.Does this pull request change what data or activity we track or use?
No.
Testing instructions
pnpm installfrom the repo root, thencomposer installinprojects/packages/backup/. Build withjp build packages/backup --deps— on a fresh checkout a barepnpm run buildinside the package fails, because wp-build resolves@automattic/jetpack-componentsfrom itsbuild/output, sojs-packages/components(and in turnjs-packages/boost-score-api,js-packages/number-formatters,js-packages/social-logos) pluspackages/wp-build-polyfillsmust be built first./jetpack/v4/rewind/backup/lscall, clicking a text file (e.g.readme.html) opens theFileInfoCardwith Modified (from/lsperiod), Type (inferred from the extension), and a monospace preview body (≤ 64 KB).periodpredates available content blobs (VaultPress retains manifest entries longer than blob storage) — it's expected, not a regression.<Gates>renders the "no plan" fallback with a CTA tojetpack.com/upgrade/backup/. Tested on a fully-connected Jetpack Complete site too — should pass through to the body since Complete includes Backup.<Gates>renders the "Connect Jetpack" fallback..off, reload). Confirm the legacy Backup admin renders unchanged.🤖 Generated with Claude Code