Skip to content

Fixed lazy URL primary_tag on tied tag sort_order - #29482

Merged
allouis merged 1 commit into
mainfrom
lazy-url-primary-tag-tied-tags
Jul 23, 2026
Merged

Fixed lazy URL primary_tag on tied tag sort_order#29482
allouis merged 1 commit into
mainfrom
lazy-url-primary-tag-tied-tags

Conversation

@allouis

@allouis allouis commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Why

Under a {primary_tag} or {primary_author} permalink, a post's URL is built from its primary tag — the post's first tag, if that tag is public. So the URL depends on the order the tags relation returns.

The lazy URL path and the eager service load a post's tags through different queries, and both order by sort_order with no tie-breaker. When several of a post's tags share a sort_order — the state bulk edits and imports leave behind — MySQL is free to return them in a different order for each query. So the lazy path and eager pick a different primary tag and build a different URL for the same post. In production this surfaced as eager serving /disaster-preparedness/… while the lazy comparison produced /claremont-elmwood/… for the same post.

Eager is authoritative and is being removed, so rather than touch its query this pins only the lazy path. The model's tags() and authors() relations now order by the pivot id (posts_tags.id / posts_authors.id) after sort_order. That's attach order, which is the tag eager already serves, so the lazy path now matches it without eager changing.

The pivot id is pulled into the relation's withPivot because the query uses DISTINCT, which requires ORDER BY columns to be in the SELECT list.

The divergence only happens on MySQL — sqlite resolves the tie consistently — so the added regression test pins the deterministic ordering rather than reproducing the divergence.

ref https://linear.app/ghost/issue/HKG-1822

primary_tag is a post's first tag if it is public, so it depends on the
order the tags relation returns. The lazy URL path loads tags through the
model's tags() relation, which orders by sort_order with no tie-breaker.
When several of a post's tags share a sort_order — the state bulk and import
operations leave behind — MySQL is free to return them in different orders
per query, so the lazy path could resolve a different primary_tag than the
eager service and build a different URL for the same post under a
{primary_tag} permalink (seen in production as eager serving
/disaster-preparedness/ while the serializer produced /claremont-elmwood/).

Order tags()/authors() by posts_tags.id / posts_authors.id (attach order)
after sort_order. That is the tag the eager service already serves, so the
lazy path now matches it without touching eager's own raw-knex query. The
pivot id is added to withPivot because the relation query uses DISTINCT,
which requires ORDER BY columns to appear in the SELECT list.
@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ff21b201-73fe-4c07-9074-c90fca9a9d97

📥 Commits

Reviewing files that changed from the base of the PR and between 08e92f1 and 5d607c9.

📒 Files selected for processing (2)
  • ghost/core/core/server/models/post.js
  • ghost/core/test/integration/url-serializer-parity.test.js

Walkthrough

The Post model now uses pivot IDs as secondary ordering keys for author and tag relations when sort_order values tie. The URL serializer parity integration test adds direct database setup for tied tag ordering and verifies that the generated URL uses the first-attached public tag.

Possibly related PRs

  • TryGhost/Ghost#29480: Both changes address primary_tag behavior in URL resolution and parity comparison.

Suggested reviewers: kevinansfield

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main fix for lazy URL generation when tag sort order ties, though it omits the author-related part.
Description check ✅ Passed The description is directly about the URL ordering bug and the related model/test changes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch lazy-url-primary-tag-tied-tags

Comment @coderabbitai help to get the list of available commands.

@nx-cloud

nx-cloud Bot commented Jul 21, 2026

Copy link
Copy Markdown

🤖 Nx Cloud AI Fix

Ensure the fix-ci command is configured to always run in your CI pipeline to get automatic fixes in future runs. For more information, please see https://nx.dev/ci/features/self-healing-ci


View your CI Pipeline Execution ↗ for commit 5d607c9

Command Status Duration Result
nx run ghost:test:ci:integration ✅ Succeeded 2m 12s View ↗
nx run ghost:test:integration ✅ Succeeded 3m 1s View ↗
nx run ghost:test:legacy ✅ Succeeded 3m 3s View ↗
nx run ghost:test:e2e ✅ Succeeded 2m 41s View ↗
nx run-many -t test:unit -p ghost ✅ Succeeded 25s View ↗
nx run ghost-monorepo:lint:boundaries ✅ Succeeded 20s View ↗
nx run-many -t lint -p ghost,ghost-monorepo ✅ Succeeded 19s View ↗
nx run-many --target=build --projects=tag:publi... ✅ Succeeded 1s View ↗
nx run @tryghost/admin:build ✅ Succeeded 4s View ↗

💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗


☁️ Nx Cloud last updated this comment at 2026-07-21 05:14:59 UTC

@allouis
allouis marked this pull request as ready for review July 22, 2026 07:51
@codecov

codecov Bot commented Jul 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.50%. Comparing base (08e92f1) to head (5d607c9).
⚠️ Report is 344 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #29482      +/-   ##
==========================================
+ Coverage   74.48%   74.50%   +0.01%     
==========================================
  Files        1602     1602              
  Lines      140563   140577      +14     
  Branches    17062    17059       -3     
==========================================
+ Hits       104699   104735      +36     
+ Misses      34845    34789      -56     
- Partials     1019     1053      +34     
Flag Coverage Δ
e2e-tests 76.58% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@allouis
allouis merged commit 93a4261 into main Jul 23, 2026
50 checks passed
@allouis
allouis deleted the lazy-url-primary-tag-tied-tags branch July 23, 2026 01:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants