feat: filter the admin content list by byline - #2312
Draft
MA2153 wants to merge 2 commits into
Draft
Conversation
Adds byline filtering alongside the existing status, author, and date filters. Selecting several bylines matches entries credited to any of them; "No byline assigned" matches entries with no credit. Credits inferred from an entry's author (rendered when an entry has no explicit credit) are excluded unless opted into, so the filter matches assigned bylines by default. No migration is required. The UNIQUE(collection_slug, content_id, byline_id) index from migration 031 covers every filter shape: EXPLAIN QUERY PLAN shows a covering seek for the include, exclude, and no-byline probes while the outer query keeps its sort-ordered composite index, so LIMIT still short-circuits without a temp B-tree. Correlating EXISTS from the content table is what makes that hold — driving from the pivot side cannot use the index for the byline and forces a temp sort — hence the note in applyBylineFilter. Filter values are translation_groups (what the junction has stored since migration 040), so a selection matches a byline across every locale. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 28101af The changes in this PR will be included in the next version bump. This PR includes changesets to release 17 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
Scope checkThis PR changes 674 lines across 12 files. Large PRs are harder to review and more likely to be closed without review. If this scope is intentional, no action needed. A maintainer will review it. If not, please consider splitting this into smaller PRs. See CONTRIBUTING.md for contribution guidelines. |
Contributor
PR template validation failedPlease fix the following issues by editing your PR description:
See CONTRIBUTING.md for the full contribution policy. |
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/registry-verification
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
Contributor
Overlapping PRsThis PR modifies files that are also changed by other open PRs:
This may cause merge conflicts or duplicated work. A maintainer will coordinate. |
Temporary tooling for exercising the byline filter by hand. Not intended to ship with the feature -- revert this commit before the PR goes up. Adds a "Set byline" picker to the existing bulk-selection toolbar. The picked bylines replace each selected entry's credit set rather than merging into it: list items hydrate credits with strict locale matching, so an entry whose byline has no row in the entry's locale comes back with an empty `bylines` array, and a client-side merge silently drops those credits on write. Requests fan out through runBulkAction like the other bulk actions, so failed ids stay selected for a retry. Co-Authored-By: Claude Opus 5 <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
1 participant
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.
What does this PR do?
Adds byline filtering to the admin content list, alongside the existing status, author, and date filters. Selecting one or more bylines matches entries credited to any of them (OR); a No byline assigned option matches entries with no credit at all.
Bylines are searched server-side in the picker rather than listed exhaustively, so the filter works across the whole byline directory rather than one page.
Inferred bylines are excluded by default. An entry with no explicit credit still renders the byline linked to its author (see
hydrateBylinesMany), but filtering usually means "who is credited", not "whose name happens to show". A single Include inferred bylines switch opts into the wider behaviour, and it widens consistently: with it on, "No byline assigned" means nothing is rendered, so entries whose author resolves to a byline drop out too.Filter values are
translation_groups — what_emdash_content_bylines.byline_idhas stored since migration 040 — so a selection matches a byline across every locale it exists in.Important
No Discussion exists for this yet, so the feature checklist line below is deliberately unticked and this is opened as a draft. Happy to open one in Ideas and hold this until it's approved — flagging rather than assuming, given CONTRIBUTING.md's policy on feature PRs.
Closes #
No migration required
Verified before writing any code, with
EXPLAIN QUERY PLANagainst a freshly-migrated DB rather than by inspection. TheUNIQUE(collection_slug, content_id, byline_id)constraint from migration 031 already creates an index of exactly the right shape:Each probe is index-only, and the outer query keeps its sort-ordered composite index, so
LIMITstill short-circuits with no temp B-tree.Two consequences are load-bearing enough that they're recorded in comments rather than just here:
EXISTSshape matters. Driving from the pivot side (FROM _emdash_content_bylines JOIN ec_*) cannot use that index for the byline and addsUSE TEMP B-TREE FOR ORDER BY. Written as anEXISTSfrom the content table, no new index is needed; written the other way, no index rescues it.primary_byline_id. The two agree — both junction write paths stamp the column in the same call — but not atomically (D1 has no transactions), so the junction stays authoritative, mirroring how migration 051 treats the denormalized taxonomy columns as advisory and re-checks on read.Known limitation, deliberately not addressed: the
EXISTSplan walks the collection's sort index and probes per row, so a byline matching very few entries in a very large collection reads a lot before fillingLIMIT— the shape #1834/migration 051 fixed for taxonomies. Making that seek-optimal needs denormalization, i.e. a migration. This is the authenticated admin list rather than the logged-out hot path, and every probe is index-only, so it didn't seem worth paying now. Happy to revisit.No queries were added to any logged-out route.
Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runmessages.pochanges except in translation PRs — a workflow extracts catalogs on merge tomain.AI-generated code disclosure
Screenshots / test output
Verified in the browser against
demos/simple: the filter renders in the existing filter bar as an "All bylines" dropdown containing a search box, the exclusive "No byline assigned" checkbox, the byline list, and the "Include inferred bylines" switch.Also checked in Arabic per the RTL guidance —
document.documentElement.dirisrtl, and the popover, checkboxes, switch, and caret all mirror correctly with no broken directionality. (New strings render in English until the extraction workflow picks them up on merge; nomessages.pochanges are included here.)Screenshots can be attached on request — omitted here since I can't upload to GitHub's CDN from the CLI.
Tests
11 new integration tests (
content-list-byline-filter.test.ts), run against both dialects viadescribeEachDialect, covering single/multi-byline OR matching, the no-byline filter, inferred credits on and off, an explicit credit suppressing author inference, composition with the status filter, andtotalreflecting the filter.One of them caught a real bug during development: the empty-selection guard used
eb.val(false), which better-sqlite3 refuses to bind (SQLite3 can only bind numbers, strings, bigints, buffers, and null). It now emits a literal1 = 0predicate instead.Behaviour verified end-to-end against
demos/simple, filtering a 16-entry collection:totalbylines=<A>bylines=<A>,<B>bylines=nonebylines=<unknown-id>bylines=<A>&status=published6 + 10 = 16 — the include and no-byline filters partition the collection exactly.
Wire contract:
includeInferredBylines=1/true/0/falseincludeInferredBylines=yesVALIDATION_ERRORbylines=(empty)VALIDATION_ERRORThe 25-id cap keeps the
IN (...)clause clear of D1's bound-parameter ceiling once the rest of the list query's placeholders are counted.