Skip to content

🐛 Fixed @site.admin_url missing /ghost/ when admin URL is configured#26960

Merged
ErisDS merged 1 commit intomainfrom
fix-site-admin-url
Mar 25, 2026
Merged

🐛 Fixed @site.admin_url missing /ghost/ when admin URL is configured#26960
ErisDS merged 1 commit intomainfrom
fix-site-admin-url

Conversation

@ErisDS
Copy link
Copy Markdown
Member

@ErisDS ErisDS commented Mar 25, 2026

Problem

@site.admin_url was using getAdminUrl() which returns the base admin domain (e.g. https://admin.example.com/) without /ghost/. This means the "Site owner login" link on the private page pointed to the domain root instead of the admin panel for any site with a separate admin URL configured (including all Ghost(Pro) sites).

Fix

Changed update-local-template-options.js to use urlFor('admin', true) which correctly appends /ghost/ to the admin URL.

Test

Added a test that stubs getAdminUrl() with a separate domain (https://admin.example.com/) and asserts the resulting admin_url ends with /ghost/. This test fails on main and passes with the fix.

getAdminUrl() returns the base admin domain without /ghost/, so using
it directly for @site.admin_url produced links to the domain root
instead of the admin panel. Use urlFor("admin", true) which correctly
appends /ghost/ to the admin URL.

Added test that stubs getAdminUrl() with a separate domain to verify
admin_url always ends with /ghost/.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 25, 2026

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: 9390dcfc-a807-4756-8f3b-2e938d4f3aa9

📥 Commits

Reviewing files that changed from the base of the PR and between 31e09b5 and 4583dd3.

📒 Files selected for processing (2)
  • ghost/core/core/frontend/services/theme-engine/middleware/update-local-template-options.js
  • ghost/core/test/unit/frontend/services/theme-engine/middleware.test.js

Walkthrough

The admin URL construction in the theme engine middleware was modified to consistently use urlUtils.urlFor('admin', true) instead of conditionally falling back to urlUtils.getAdminUrl(). This simplifies the middleware's handling of the siteData.admin_url property. Corresponding unit tests were updated to verify the new behavior, including assertions that the admin URL ends with /ghost/, and explicit test completion handlers were added for improved test control flow.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly references a bug fix related to @site.admin_url and the missing /ghost/ path, which directly aligns with the main changeset objective.
Description check ✅ Passed The description provides relevant context about the problem, the fix applied, and how it was tested, all directly related to the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 fix-site-admin-url

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

@ErisDS ErisDS added the deploy-to-staging Optionally deploy PR to staging label Mar 25, 2026
@ErisDS ErisDS merged commit b08f266 into main Mar 25, 2026
40 checks passed
@ErisDS ErisDS deleted the fix-site-admin-url branch March 25, 2026 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deploy-to-staging Optionally deploy PR to staging

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant