Automattic / Automattic/newspack-rolling-coverage

Canonical URL can be lost while editing, and a rejected value clears the stored one

Open
#36 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
PHP
Stars
1
Forks
1
Avg merge
2d 18h
Merged PRs (30d)
3

Description

## Summary

The **Canonical URL** field in the Rolling Coverage block sidebar can drop what an editor types, and the host check added in [#18](https://github.com/Automattic/newspack-rolling-coverage/pull/18) clears the stored value when it rejects one. Both fail without telling anyone, and an empty canonical URL means no notification is sent for that coverage.

From the [#18](https://github.com/Automattic/newspack-rolling-coverage/pull/18) review.

## A rejected URL clears the working one

`sanitize_canonical_url()` returns `''` for anything off-site, so a `sanitize_callback` that rejects input also wipes whatever was stored:

```php
// includes/class-taxonomy.php:214
return strtolower( $url_host ) === strtolower( $site_host ) ? $url : '';
```

Observed on a coverage that already had a valid canonical URL: writing an off-site URL left the meta empty rather than unchanged. Combined with the swallowed write errors below, an editor pasting the wrong link silently turns notifications off for that coverage, and the entry meta box's warning is the only hint, one screen away.

Returning the previous value on rejection, or validating before write and surfacing the rejection, keeps a good value in place.

Two narrower behaviours worth a look while in there: `https://www.example.com/...` is rejected against a bare-host `home_url()`, which bites a site whose canonical host differs from `home_url()`; and relative URLs such as `/live-coverage/` are rejected, so the field requires an absolute URL with no indication that it does.

## Typing a URL does not dirty the post

`pendingCanonicalUrl` is component state and never reaches `setAttributes`, so typing does not mark the post as edited. The only caller of `handleApplyCanonicalUrl` is the effect keyed on `isSavingPost` (`src/blocks/rolling-coverage/edit.tsx:427`); there is no Apply button, blur handler, or debounce, unlike the **Status** control directly above it.

In practice the value does save on most sites, because OneSignal registers its own meta box on pages, which sets `forceIsDirty` and leaves **Save** enabled. Measured both ways:

```
OneSignal active meta boxes: true post dirty: false Save: enabled
OneSignal deactivated meta boxes: false post dirty: false Save: disabled
```

So the reachable failure is narrower than it looks, but real: type a URL, navigate away without pressing Save, and it is discarded with no unsaved-changes warning, because the post was never dirty. An Apply button mirroring the Status control would make the write explicit.

## Write failures are not surfaced

`updateCoverageCanonicalUrl()` swallows the error and returns `false` (`src/blocks/rolling-coverage/utils.ts:130`), and the handler ignores the result, so a rejected write looks the same as a successful one. Writing this term meta needs `manage_categories`. Editors have it, so the common path is fine, but an Author editing a post that carries the block sees the panel, types a URL, and has it dropped by a 403 they never see. The existing `applyNotice` mechanism already covers the Status control and would suit here.

Separately, the canonical URL key has no `auth_callback`, unlike the four keys registered just below it (`includes/class-taxonomy.php:150-190`), so it inherits the permissive default.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with includes/class-taxonomy.php:150-214, src/blocks/rolling-coverage/edit.tsx:427, and src/blocks/rolling-coverage/utils.ts:130. Trace canonical URL sanitization, the pending state and save effect, then compare the existing Status control and applyNotice handling. Done means rejected values preserve valid metadata, edits are explicitly saved and marked dirty, and write failures are visible.

Written by the indexing model from the issue text.

Assessment

Tech stack
php, typescript
Domain
full-stack
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.