Updated private page access and signup flow#26876
Closed
Conversation
Reworked Ghost core's private page to use the new access dialog, optional subscribe form, accent-aware styling, and a dedicated runtime script without pulling Portal onto the page. Also adds the helper, asset pipeline, and test coverage needed to ship and maintain the new flow.
…g, and i18n gap
- Guard json helper against JSON.stringify(undefined) returning undefined
- Check integrityTokenRes.ok before calling .text() in private.js
- Wrap "Site owner login" in {{t}} for i18n support
- Add locale fixture entries and test coverage for all three fixes
- Moved json, color_to_rgba, contrast_text_color from private-blogging app to core/frontend/helpers/
- Added admin_url to @site template context, updated private.hbs to use {{@site.admin_url}}
- Added gscan cross-reference test to catch helpers missing from gscan knownHelpers
- Updated helpers.test.js allowlist with the three new global helpers
- Replaced brittle exact-string error matching with type-based matching (TooManyRequestsError) and keyword regex (restricted domain) - Removed dead failedMagicLink code path that could never match - Added aria-label to signup email input for screen readers - Trim email whitespace before checkValidity() runs - Reverted unrelated console.error to logging.error change in bin script
…ved tests, fixed gscan test - Removed admin_url helper file (superseded by @site.admin_url data) - Moved json, color_to_rgba, contrast_text_color tests to helpers dir - Fixed gscan cross-reference test: tracks exact pending set so it fails if a new helper is added without updating gscan or the pending list - Added middleware test verifying admin_url in @site data
- Client-side validation failure (checkValidity returns false) - Network error (fetch throws, e.g. offline) - Unparseable error response body (API returns non-JSON)
Removed pendingGscanUpdate workaround — the test now correctly fails for color_to_rgba, contrast_text_color, json until they are added to gscan knownHelpers.
Removed raw and search from internalHelpers exclusion list — they should be in gscan knownHelpers. Test now fails for all 5 helpers pending gscan PR: TryGhost/gscan#742
…gured
urlUtils.urlFor("admin", true) joins the admin URL with "/ghost/",
but when admin:url already contains "/ghost/" this produces a doubled
path like "/ghost/ghost/". Use getAdminUrl() which returns the URL
directly from config, falling back to urlFor for environments where
no separate admin URL is configured.
admin:url should be the base URL (e.g. https://admin.example.com) without /ghost — urlFor adds that automatically. If /ghost is present in the config, all 22 callers of urlFor("admin", true) produce a doubled path like /ghost/ghost/. Added sanitizeAdminUrl to config loader which strips /ghost from the path and logs a warning. Reverted the workaround in update-local-template-options.js since the real fix belongs in config sanitization.
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (24)
WalkthroughThis PR implements a private blogging feature with comprehensive frontend and configuration updates. It adds new template helpers for color conversion and JSON serialization, introduces a private-blogging template with a modal-based access dialog, and includes a client-side runtime script for handling newsletter subscriptions and access code validation. The changes extend CSS styling for private UI elements, add configuration sanitization for admin URLs, register new public asset routes, update the password input helper for 1Password support, and include extensive test coverage alongside localization updates for English and German locales. Possibly related PRs
Suggested labels
Important Merge conflicts detected (Beta)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
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
2 participants
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.
Continuation of #26762 (lost push access to fork). Contains all review fixes plus original work.
Review fixes (9 commits on top of original)
Bug fixes: json helper undefined crash, integrity token ok check, error normalization (type-based matching, removed dead code), admin:url config sanitization (strips /ghost, warns), console.error revert.
Improvements: localized Site owner login, aria-label on email input, email trim, promoted helpers to global, added @site.admin_url, removed redundant admin_url helper, gscan cross-reference test.
Tests: 3 unhappy path tests, error normalization tests, middleware admin_url test, config sanitization tests.
See #26762 review comment for full details: #26762 (review)