Refuse Compare when no source site is connected

[acicovic]

Summary

Compare told users an imported post was not imported whenever no source site was connected, because the lookup behind its permission check resolves nothing without an identity. It now refuses with the missing connection named, ahead of the lookup, and treats a connected site URL with no parseable identity the same way.

Fixes #391.
Fixes VIPCMS-2142.

Before

With the connection cleared or saved without a scheme, opening Compare answered "No matching post found in current site." — the one wording the modal shows the user verbatim. The post was usually imported and the cause was configuration, so nothing the message suggested could help. Users without the capability to see others' posts got a bare rejection instead.

After

The refusal names the missing connection and points at the settings page, using the same status the plugin already returns elsewhere for an unconfigured or unparseable connected site URL. The unmapped-post and invalid-ID answers are untouched, and nothing changes while connected. The sync-status check stays silent on a missing connection deliberately: it is a background poll with no error surface, feeding a screen that does not render while disconnected.

Human testing steps

Stacked on #390, so check this branch out.

1. On the destination, open Manage → Posts with the connection working and one row Outdated (npm run seed:update if none is).

2. Break the connection in a terminal, and do not reload the page:

npm run wp-cli option update safe_publish_connected_site_url "host.docker.internal:8889"

3. Click Compare on that row. Expect No source site is connected. Configure a valid connected site URL in the settings page before comparing. On the base branch it reads No matching post found in current site., which points the user at the post rather than the configuration.

4. npm run wp-cli option delete safe_publish_connected_site_url answers the same. Driving the route from the CLI, as in step 3 of Resolve nothing when the source scope is empty #390, now returns 500 {"code":"no_connected_site_url",...}.

5. Restore, and confirm Compare renders a diff again:

npm run wp-cli option update safe_publish_connected_site_url "http://host.docker.internal:8889"

[pkevan]

Refusing upfront is the right call, and the placement relative to the post lookup is correct — nothing about a post imported from a previously connected source can leak, since the check precedes find_local_post() entirely. One ordering problem to fix first.

The refusal runs before any capability check

includes/api/class-safe-publish-api.php:108-119 is the first statement in the permission callback:

public function check_diff_preview_permission(
    WP_REST_Request $request
): bool|WP_Error {
    $source_site_url = Options::get_connected_site_url_with_path();

    if ( '' === $source_site_url ) {
        return new WP_Error(
            'no_connected_site_url',
            ...

The first current_user_can() is at :141, and the route's only gate is this callback (:63, 'permission_callback' => array( $this, 'check_diff_preview_permission' )) — there is no wrapper adding auth earlier, and the route is registered on rest_api_init rather than behind an admin check.

So an anonymous POST /wp-json/safe-publish/v1/diff-preview now receives a 500 naming the missing connection, where before it received a permission denial. That turns the endpoint into a free "is this install configured?" probe: the route list is public via /wp-json/, so a scanner can cheaply partition sites into connected and not-connected.

This also cuts against the pattern the same function documents thirty lines below at :139-149:

if ( is_wp_error( $local_post ) ) {
    // Surface 404 only to users who could reasonably know the post exists.
    if ( current_user_can( 'edit_others_posts' ) ) {

Moving the connection check below a capability gate — returning false for a caller who could not act on the information anyway — resolves it and keeps the behavior identical for the operators the message is written for.

Neither new test covers this: test_diff_preview_endpoint_reports_missing_connection and test_diff_preview_endpoint_reports_unparseable_connection both call wp_set_current_user( $this->admin_user_id ) first. A test asserting an unauthenticated caller does not receive the refusal would pin the fix.

Verified fine

Ordering is correct in every other respect — the check precedes the postId parse, the local-post lookup, and any response carrying post data. Returning a WP_Error from a permission callback serializes to a proper {code, message, data:{status}} envelope rather than a failure inside a success envelope. The message is translatable with the right text domain and does not echo the offending URL or option value. The 400/404/403 branches are untouched, and threading $source_site_url into find_local_post() instead of calling Options:: twice is a clean refactor with no behavior change while connected. The unparseable-connection test is a good addition — it covers the case the sibling PR leaves untested at the lookup layer.

Merge order

This cannot land before #390. Four existing tests in Safe_Publish_API_Test dispatch this route with no connection configured and expect 404/403/400; they stay green only because #390 hoists update_option( 'safe_publish_connected_site_url', … ) into setUp(). Merged on its own, those four would receive the new 500 instead.