fix(core): exclude unpublished entries from taxonomy counts#593
fix(core): exclude unpublished entries from taxonomy counts#593maikunari wants to merge 1 commit intoemdash-cms:mainfrom
Conversation
Issue emdash-cms#581: Unpublished posts were included in category/tag counts, causing inaccurate public-facing sidebar counts. Changes: - countEntriesWithTerm now defaults to status='published' - Added optional status parameter: 'all', 'published', 'draft', etc. - Uses raw SQL with sql.ref() and validateIdentifier for dynamic ec_{collection} table joins (Kysely type system limitation) - Updated existing test to reflect new default behavior - Added 4 new test cases covering all status filtering scenarios
|
There was a problem hiding this comment.
Pull request overview
Fixes incorrect public taxonomy term counts by excluding unpublished entries from counts by default, with an option to count entries across any status.
Changes:
- Updated
TaxonomyRepository.countEntriesWithTermto default to counting onlypublishedentries and to support an'all'mode or a specific status filter. - Implemented status-aware counting via raw SQL joins against dynamic
ec_*content tables with identifier validation. - Updated and expanded unit tests to cover default behavior and status-filtering scenarios.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/core/src/database/repositories/taxonomy.ts | Changes taxonomy term counting to default to published-only and adds status filtering with dynamic ec_* joins. |
| packages/core/tests/unit/taxonomies/taxonomies.test.ts | Updates existing test expectation and adds new test cases for status filtering behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| WHERE ct.taxonomy_id = ${taxonomyId} | ||
| AND ${sql.ref(tableName)}.status = ${effectiveStatus} |
There was a problem hiding this comment.
The status-filtered COUNT query doesn't exclude soft-deleted entries. Since ContentRepository.delete() only sets deleted_at (and this repo doesn't appear to clear content_taxonomies on delete), deleted-but-still-tagged entries will still be counted as long as their status matches. Add a ${sql.ref(tableName)}.deleted_at IS NULL predicate (and consider also adding ct.collection = ${collection} inside the loop to avoid scanning all content_taxonomies rows for the taxonomy on every per-collection count).
| WHERE ct.taxonomy_id = ${taxonomyId} | |
| AND ${sql.ref(tableName)}.status = ${effectiveStatus} | |
| WHERE ct.taxonomy_id = ${taxonomyId} | |
| AND ct.collection = ${collection} | |
| AND ${sql.ref(tableName)}.status = ${effectiveStatus} | |
| AND ${sql.ref(tableName)}.deleted_at IS NULL |
| // For status filtering, we need raw SQL to join dynamic ec_* tables. | ||
| // Get distinct collections for this taxonomy term first. | ||
| const collections = await this.db | ||
| .selectFrom("content_taxonomies") | ||
| .select((eb) => eb.fn.count("entry_id").as("count")) | ||
| .select("collection") | ||
| .distinct() | ||
| .where("taxonomy_id", "=", taxonomyId) | ||
| .executeTakeFirst(); | ||
| .execute(); | ||
|
|
||
| if (collections.length === 0) return 0; | ||
|
|
||
| // Count entries matching status by joining each collection's content table | ||
| let totalCount = 0; | ||
| for (const { collection } of collections) { | ||
| const tableName = `ec_${collection}`; | ||
| validateIdentifier(tableName, "content table"); | ||
| const result = await sql<{ count: number }>` | ||
| SELECT COUNT(ct.entry_id) AS count |
There was a problem hiding this comment.
This implementation adds 1 query to fetch collections plus 1 COUNT query per collection, per taxonomy term. In handleTermList (api/handlers/taxonomies.ts) this method is called inside a loop over all terms, which can turn into hundreds/thousands of DB round-trips and be a noticeable regression. Consider building a single UNION ALL query across the relevant ec_* tables (similar to fetchRecentItems in api/handlers/dashboard.ts) or adding a bulk repo method that returns counts for many taxonomy IDs in one call so the handler can fetch all counts at once.
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/blocks
@emdash-cms/cloudflare
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@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-forms
@emdash-cms/plugin-webhook-notifier
commit: |
Issue #581: Unpublished posts were included in category/tag counts, causing inaccurate public-facing sidebar counts.
Changes:
What does this PR do?
Fixes #581 — unpublished posts were being counted in taxonomy term totals, inflating the public-facing category and tag counts shown in sidebars and archive pages.
Problem
TaxonomyRepository.countEntriesWithTerm counted all content_taxonomies rows matching a given taxonomy term, regardless of whether the linked entries were published. Draft, scheduled, and other non-published entries showed up in public counts, so a category labeled "12 posts" might only have 5 actually published.
Change
countEntriesWithTerm now defaults to status: 'published'
Added an optional status parameter accepting 'all', 'published', 'draft', etc.
Status-aware counting implemented via raw SQL joins against dynamic ec_{collection} content tables, with validateIdentifier for safety (Kysely's type system can't handle the dynamic table names)
Updated the existing test to reflect new default behavior
Added 4 new test cases covering the status filtering scenarios
Scope
Intentionally narrow. Default behavior change is the fix; the status parameter is additive for callers that need draft counts (admin UI, etc.).
Closes #581
Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runpnpm locale:extracthas been run (if applicable)AI-generated code disclosure
Screenshots / test output