Fix/accented pagination links 21999 #24603
Conversation
Fixes TryGhost#24588 When importing HTML content with CTA cards wrapped in gated blocks via the Admin API using `{source: 'html'}`, the visibility settings were being lost during HTML-to-Lexical conversion. This caused all CTA visibility toggles to show as "on" (visible to everyone) instead of preserving the intended restrictions like "free members only". Root cause: The `@tryghost/kg-html-to-lexical` converter strips HTML comments where gated block visibility settings are stored (e.g., `<\!--kg-gated-block:begin nonMember:false memberSegment:"status:free"-->`). Solution: - Created `gated-block-visibility.js` utility to extract visibility settings from gated block comments before HTML-to-Lexical conversion - Modified posts and pages serializers to preserve visibility during conversion - Added comprehensive tests covering various scenarios The fix ensures that CTA cards (and other supported cards) maintain their visibility restrictions when imported via Admin API, resolving the issue where users would see incorrect "visible to everyone" settings instead of their intended member segment targeting. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
refs TryGhost#21999 When tags contained accented characters (e.g., "sécurité incendie") or spaces, Ghost's pagination links in {{ghost_head}} metadata contained unencoded special characters. This could confuse search engine crawlers and lead to invalid URL variations being attempted, affecting SEO indexing. The getPaginatedUrl function was using the raw data.relativeUrl from the request path without properly encoding special characters when constructing pagination URLs.
WalkthroughThe changes introduce enhancements and new utilities for handling gated block visibility during HTML to Lexical document conversion, and improve URL encoding for paginated URLs containing special or accented characters. Specifically, the Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used🧠 Learnings (5)📓 Common learnings📚 Learning: the pending activity utilities in the ghost activitypub module are covered by tests in the file `app...Applied to files:
📚 Learning: the pending activity utilities in ghost's activitypub module are thoroughly tested in `apps/admin-x-...Applied to files:
📚 Learning: in ghost project, app.js files under core/server/web are intentionally excluded from unit test cover...Applied to files:
📚 Learning: in the ghost test framework, when testing csv exports via the admin api, the response `body` field i...Applied to files:
🔇 Additional comments (2)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
ghost/core/core/frontend/meta/paginated-url.js (1)
20-41: LGTM! Fix ESLint style issue.The URL encoding implementation correctly addresses the pagination link issue by encoding each path segment individually. This approach properly handles accented characters and spaces while preserving URL structure.
However, there's a minor ESLint style issue to fix:
- const encodedSegments = segments.map(segment => { + const encodedSegments = segments.map((segment) => {ghost/core/core/server/api/endpoints/utils/serializers/input/utils/gated-block-visibility.js (2)
14-55: LGTM with minor performance consideration.The function logic is well-structured with proper input validation and clear separation of concerns. The regex handling with
lastIndexreset is correct.Consider the performance implications of deep cloning large Lexical documents using
JSON.parse(JSON.stringify()). For very large documents, this could be expensive. However, this is likely acceptable for the current use case.
94-113: Fix ESLint arrow-parens violation.The recursive traversal logic is correct and properly structured.
Address the ESLint arrow-parens error on line 109:
- node.children.forEach(child => { + node.children.forEach((child) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
ghost/core/core/frontend/meta/paginated-url.js(1 hunks)ghost/core/core/server/api/endpoints/utils/serializers/input/pages.js(2 hunks)ghost/core/core/server/api/endpoints/utils/serializers/input/posts.js(2 hunks)ghost/core/core/server/api/endpoints/utils/serializers/input/utils/gated-block-visibility.js(1 hunks)ghost/core/test/integration/api/cta-visibility-import.test.js(1 hunks)ghost/core/test/unit/frontend/meta/paginated-url.test.js(1 hunks)ghost/core/test/unit/server/api/endpoints/utils/serializers/input/utils/gated-block-visibility.test.js(1 hunks)
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: cathysarisky
PR: TryGhost/Ghost#21982
File: ghost/i18n/locales/hu v2/portal.json:70-70
Timestamp: 2025-01-29T15:23:58.658Z
Learning: Typos and spelling errors within localization files (ghost/i18n/locales/*.json) must be treated as important problems, not nitpicks, as they directly affect the user experience for speakers of that language. These issues should be flagged with high priority during code review.
Learnt from: kevinansfield
PR: TryGhost/Ghost#22101
File: ghost/core/core/server/api/endpoints/utils/serializers/output/utils/post-gating.js:110-117
Timestamp: 2025-02-03T17:14:37.367Z
Learning: The potential issues with gated content blocks (lack of migration path, race conditions during flag transitions, content visibility when disabled) are accepted trade-offs during the early testing phase of the contentVisibility labs feature.
Learnt from: kevinansfield
PR: TryGhost/Ghost#23770
File: apps/admin-x-settings/src/components/settings/email/newsletters/NewsletterDetailModalLabs.tsx:435-450
Timestamp: 2025-06-10T11:07:10.800Z
Learning: In Ghost's newsletter customization features, when promoting a feature from alpha to beta status, the feature flag guards are updated to make the feature available under both the `emailCustomization` (beta) and `emailCustomizationAlpha` (alpha) flags. This is done by either removing conditional guards entirely when the component is already behind both flags, or by updating conditionals to check for either flag.
📚 Learning: the pending activity utilities in the ghost activitypub module are covered by tests in the file `app...
Learnt from: mike182uk
PR: TryGhost/Ghost#22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in the Ghost ActivityPub module are covered by tests in the file `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`.
Applied to files:
ghost/core/test/unit/frontend/meta/paginated-url.test.jsghost/core/core/server/api/endpoints/utils/serializers/input/utils/gated-block-visibility.jsghost/core/test/integration/api/cta-visibility-import.test.jsghost/core/test/unit/server/api/endpoints/utils/serializers/input/utils/gated-block-visibility.test.js
📚 Learning: the pending activity utilities in ghost's activitypub module are thoroughly tested in `apps/admin-x-...
Learnt from: mike182uk
PR: TryGhost/Ghost#22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in Ghost's ActivityPub module are thoroughly tested in `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`, which covers `generatePendingActivityId`, `isPendingActivity`, and `generatePendingActivity` functions.
Applied to files:
ghost/core/test/unit/frontend/meta/paginated-url.test.jsghost/core/core/server/api/endpoints/utils/serializers/input/utils/gated-block-visibility.jsghost/core/test/integration/api/cta-visibility-import.test.jsghost/core/test/unit/server/api/endpoints/utils/serializers/input/utils/gated-block-visibility.test.js
📚 Learning: the existing implementation in `ghost/core/core/server/web/admin/middleware/serve-auth-frame-file.js...
Learnt from: 9larsons
PR: TryGhost/Ghost#21866
File: ghost/core/core/server/web/admin/middleware/serve-auth-frame-file.js:10-19
Timestamp: 2025-04-23T15:44:52.549Z
Learning: The existing implementation in `ghost/core/core/server/web/admin/middleware/serve-auth-frame-file.js` using `path.parse(req.url).base` is secure against path traversal attacks as it properly extracts only the filename component without any directory parts.
Applied to files:
ghost/core/core/frontend/meta/paginated-url.js
📚 Learning: the potential issues with gated content blocks (lack of migration path, race conditions during flag ...
Learnt from: kevinansfield
PR: TryGhost/Ghost#22101
File: ghost/core/core/server/api/endpoints/utils/serializers/output/utils/post-gating.js:110-117
Timestamp: 2025-02-03T17:14:37.367Z
Learning: The potential issues with gated content blocks (lack of migration path, race conditions during flag transitions, content visibility when disabled) are accepted trade-offs during the early testing phase of the contentVisibility labs feature.
Applied to files:
ghost/core/core/server/api/endpoints/utils/serializers/input/pages.jsghost/core/core/server/api/endpoints/utils/serializers/input/utils/gated-block-visibility.jsghost/core/test/integration/api/cta-visibility-import.test.jsghost/core/test/unit/server/api/endpoints/utils/serializers/input/utils/gated-block-visibility.test.js
📚 Learning: the validatecommentdata function in ghost/core/core/server/api/endpoints/comments.js is not duplicat...
Learnt from: 55sketch
PR: TryGhost/Ghost#23894
File: ghost/core/core/server/api/endpoints/comments.js:16-22
Timestamp: 2025-06-18T10:56:19.906Z
Learning: The validateCommentData function in ghost/core/core/server/api/endpoints/comments.js is not duplicated elsewhere in the codebase - it's a unique validation function that ensures either post_id or parent_id is provided for comment creation.
Applied to files:
ghost/core/core/server/api/endpoints/utils/serializers/input/utils/gated-block-visibility.js
📚 Learning: in ghost's newsletter customization features, when promoting a feature from alpha to beta status, th...
Learnt from: kevinansfield
PR: TryGhost/Ghost#23770
File: apps/admin-x-settings/src/components/settings/email/newsletters/NewsletterDetailModalLabs.tsx:435-450
Timestamp: 2025-06-10T11:07:10.800Z
Learning: In Ghost's newsletter customization features, when promoting a feature from alpha to beta status, the feature flag guards are updated to make the feature available under both the `emailCustomization` (beta) and `emailCustomizationAlpha` (alpha) flags. This is done by either removing conditional guards entirely when the component is already behind both flags, or by updating conditionals to check for either flag.
Applied to files:
ghost/core/core/server/api/endpoints/utils/serializers/input/utils/gated-block-visibility.js
📚 Learning: in the ghost test framework, when testing csv exports via the admin api, the response `body` field i...
Learnt from: ErisDS
PR: TryGhost/Ghost#23588
File: ghost/core/test/e2e-api/admin/backup.test.js:136-148
Timestamp: 2025-05-29T10:37:26.369Z
Learning: In the Ghost test framework, when testing CSV exports via the admin API, the response `body` field is empty while the actual CSV data is provided through the `text` field. Tests should use `expectEmptyBody()` and then validate the CSV content via `.expect(({text}) => ...)` - this is not contradictory behavior.
Applied to files:
ghost/core/test/integration/api/cta-visibility-import.test.js
📚 Learning: in ghost project, app.js files under core/server/web are intentionally excluded from unit test cover...
Learnt from: ErisDS
PR: TryGhost/Ghost#23582
File: ghost/core/.c8rc.json:24-24
Timestamp: 2025-05-29T07:45:35.714Z
Learning: In Ghost project, app.js files under core/server/web are intentionally excluded from unit test coverage because they are not easily unit-testable due to being entry points with initialization code and side effects.
Applied to files:
ghost/core/test/unit/server/api/endpoints/utils/serializers/input/utils/gated-block-visibility.test.js
🪛 ESLint
ghost/core/core/frontend/meta/paginated-url.js
[error] 29-29: Expected parentheses around arrow function argument having a body with curly braces.
(arrow-parens)
ghost/core/core/server/api/endpoints/utils/serializers/input/utils/gated-block-visibility.js
[error] 50-50: Expected parentheses around arrow function argument having a body with curly braces.
(arrow-parens)
[error] 109-109: Expected parentheses around arrow function argument having a body with curly braces.
(arrow-parens)
🔇 Additional comments (20)
ghost/core/test/unit/frontend/meta/paginated-url.test.js (1)
168-217: Excellent test coverage for URL encoding scenarios.The new test suite comprehensively covers the URL encoding functionality with well-chosen test cases:
- Accented characters: Properly tests UTF-8 encoding of 'sécurité incendie'
- Spaces: Verifies space encoding as %20 in 'hello world'
- Mixed special characters: Tests complex scenarios with 'café & résumé!'
The expected encoded values are accurate and the test structure follows the existing pattern. This provides good confidence that the encoding implementation works correctly across different character sets and scenarios.
ghost/core/core/server/api/endpoints/utils/serializers/input/utils/gated-block-visibility.js (6)
1-4: LGTM!The import and regex pattern are correctly structured. Reusing
parseGatedBlockParamsfrom the existing post-gating module is a good approach for consistency.
63-86: LGTM!The content identifier extraction logic is correct for CTA cards. The regex patterns properly match the expected HTML structure and the text cleaning with HTML tag removal is appropriate.
121-140: LGTM!The card type checking logic is clean and comprehensive. The list of supported card types covers the major Ghost card types appropriately.
150-168: LGTM!The card matching logic is well-designed. The approach of treating missing identifiers as matches provides good fallback behavior, and the specific CTA matching logic using text content and URLs is appropriate.
176-198: LGTM!The visibility object construction is correct with sensible defaults. The parameter application logic properly handles undefined values and maintains consistency between web and email visibility settings.
200-202: LGTM!Clean module exports exposing only the necessary public interface.
ghost/core/core/server/api/endpoints/utils/serializers/input/posts.js (2)
14-14: LGTM!The import is correctly placed and follows the established patterns in the file.
236-238: LGTM!The integration of gated block visibility preservation is clean and follows the established pattern. The three-step process (convert, enhance, serialize) is logical and maintains existing error handling.
ghost/core/core/server/api/endpoints/utils/serializers/input/pages.js (2)
14-14: LGTM!The import addition is consistent with the posts.js implementation and correctly placed.
194-196: LGTM!The integration matches the posts.js implementation with appropriate adaptation for the pages data structure.
ghost/core/test/unit/server/api/endpoints/utils/serializers/input/utils/gated-block-visibility.test.js (5)
1-3: LGTM!Standard test setup with appropriate imports and assertion library.
6-62: LGTM!This test case provides comprehensive coverage of the core functionality, with realistic HTML and Lexical structures and proper verification of the applied visibility settings.
64-134: LGTM!Important test case that verifies the function correctly distinguishes between gated and non-gated content, ensuring visibility is only applied where appropriate.
136-185: LGTM!Good coverage of multiple gated blocks with different visibility settings, ensuring each block is processed independently and correctly.
187-222: LGTM!Good edge case coverage ensuring the function handles empty inputs gracefully and doesn't modify documents when no gated blocks are present.
ghost/core/test/integration/api/cta-visibility-import.test.js (4)
1-8: LGTM!Proper integration test setup with Ghost environment initialization following established patterns.
11-97: LGTM!Comprehensive integration test with realistic HTML and Lexical structures that closely match actual usage scenarios. Good verification of visibility preservation.
99-163: LGTM!Good integration test coverage for multiple gated blocks with different member segments, providing realistic scenarios for free vs paid member content.
165-220: LGTM!Good integration test coverage for mixed public and gated CTAs, ensuring proper boundary handling in a realistic integration context.
refs TryGhost#21999 Added comprehensive solution for accented character URLs: 1. Fixed ESLint arrow-parens violations in paginated-url.js and gated-block-visibility.js 2. Added normalize-urls middleware to redirect accented taxonomy URLs to normalized versions 3. Integrated middleware into pretty-urls chain for automatic 301 redirects 4. Added comprehensive test coverage for all redirect scenarios This provides a two-layer solution: 301 redirects prevent crawlers from reaching wrong URLs, while the existing pagination URL encoding ensures proper metadata even if accented URLs are accessed directly.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
ghost/core/core/server/api/endpoints/utils/serializers/input/utils/gated-block-visibility.js (2)
50-52: Fix ESLint arrow function parentheses.The static analysis tool correctly identified missing parentheses around the arrow function parameter.
- gatedBlocks.forEach(block => { + gatedBlocks.forEach((block) => { applyVisibilityToMatchingCards(modifiedDoc.root, block); });
67-85: Improve regex robustness and consider edge cases.The regex patterns for extracting content identifiers may not handle all HTML variations and could miss content with different formatting or nested elements.
Consider using a more robust HTML parsing approach:
- const ctaTextMatch = content.match(/<div[^>]*kg-cta-text[^>]*>(.*?)<\/div>/s); + const ctaTextMatch = content.match(/<div[^>]*\bkg-cta-text\b[^>]*>(.*?)<\/div>/s); if (ctaTextMatch) { identifiers.ctaText = ctaTextMatch[1].replace(/<[^>]*>/g, '').trim(); } - const buttonTextMatch = content.match(/<a[^>]*kg-cta-button[^>]*>(.*?)<\/a>/s); + const buttonTextMatch = content.match(/<a[^>]*\bkg-cta-button\b[^>]*>(.*?)<\/a>/s); if (buttonTextMatch) { identifiers.buttonText = buttonTextMatch[1].replace(/<[^>]*>/g, '').trim(); } - const buttonUrlMatch = content.match(/href="([^"]*)"[^>]*kg-cta-button/); + const buttonUrlMatch = content.match(/href="([^"]*)"[^>]*\bkg-cta-button\b/);ghost/core/core/server/web/shared/middleware/normalize-urls.js (3)
29-29: Add missing curly braces for single-statement if blocks.ESLint correctly flagged missing curly braces, which is required by the coding standards.
- if (!segment) return false; + if (!segment) { + return false; + }- if (!segment) return segment; + if (!segment) { + return segment; + }Also applies to: 46-46
67-67: Remove unused variables.The variables
taxonomyTypeanddecodedURIare assigned but never used.- const [, basePath, taxonomyType, slug, remainder] = taxonomyMatch; + const [, basePath, , slug, remainder] = taxonomyMatch;- let decodedURI; - // Only process taxonomy routes (tags, authors) const taxonomyMatch = pathToTest.match(/^(.*\/(tag|author)\/([^\/]+))(\/.*)?$/); if (!taxonomyMatch) { return next(); } const [, basePath, , slug, remainder] = taxonomyMatch; try { - decodedURI = decodeURIComponent(pathToTest); + decodeURIComponent(pathToTest); } catch (err) {Also applies to: 70-70
61-61: Remove unnecessary escape characters in regex.The forward slashes in the regex don't need to be escaped.
- const taxonomyMatch = pathToTest.match(/^(.*\/(tag|author)\/([^\/]+))(\/.*)?$/); + const taxonomyMatch = pathToTest.match(/^(.*(tag|author)\/([^/]+))(\/.*)?$/);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
ghost/core/core/frontend/meta/paginated-url.js(1 hunks)ghost/core/core/server/api/endpoints/utils/serializers/input/utils/gated-block-visibility.js(1 hunks)ghost/core/core/server/web/shared/middleware/index.js(1 hunks)ghost/core/core/server/web/shared/middleware/normalize-urls.js(1 hunks)ghost/core/core/server/web/shared/middleware/pretty-urls.js(1 hunks)ghost/core/test/unit/server/web/shared/middleware/normalize-urls.test.js(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- ghost/core/core/server/web/shared/middleware/index.js
🚧 Files skipped from review as they are similar to previous changes (1)
- ghost/core/core/frontend/meta/paginated-url.js
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: cathysarisky
PR: TryGhost/Ghost#21982
File: ghost/i18n/locales/hu v2/portal.json:70-70
Timestamp: 2025-01-29T15:23:58.658Z
Learning: Typos and spelling errors within localization files (ghost/i18n/locales/*.json) must be treated as important problems, not nitpicks, as they directly affect the user experience for speakers of that language. These issues should be flagged with high priority during code review.
Learnt from: kevinansfield
PR: TryGhost/Ghost#22101
File: ghost/core/core/server/api/endpoints/utils/serializers/output/utils/post-gating.js:110-117
Timestamp: 2025-02-03T17:14:37.367Z
Learning: The potential issues with gated content blocks (lack of migration path, race conditions during flag transitions, content visibility when disabled) are accepted trade-offs during the early testing phase of the contentVisibility labs feature.
Learnt from: kevinansfield
PR: TryGhost/Ghost#23770
File: apps/admin-x-settings/src/components/settings/email/newsletters/NewsletterDetailModalLabs.tsx:435-450
Timestamp: 2025-06-10T11:07:10.800Z
Learning: In Ghost's newsletter customization features, when promoting a feature from alpha to beta status, the feature flag guards are updated to make the feature available under both the `emailCustomization` (beta) and `emailCustomizationAlpha` (alpha) flags. This is done by either removing conditional guards entirely when the component is already behind both flags, or by updating conditionals to check for either flag.
📚 Learning: in the ghost codebase, middleware directories should be included in unit test coverage because they ...
Learnt from: ErisDS
PR: TryGhost/Ghost#23582
File: ghost/core/.c8rc.json:28-28
Timestamp: 2025-05-29T07:47:00.748Z
Learning: In the Ghost codebase, middleware directories should be included in unit test coverage because they contain actual business logic that should be tested, not just routing or setup code.
Applied to files:
ghost/core/core/server/web/shared/middleware/pretty-urls.jsghost/core/test/unit/server/web/shared/middleware/normalize-urls.test.js
📚 Learning: the existing implementation in `ghost/core/core/server/web/admin/middleware/serve-auth-frame-file.js...
Learnt from: 9larsons
PR: TryGhost/Ghost#21866
File: ghost/core/core/server/web/admin/middleware/serve-auth-frame-file.js:10-19
Timestamp: 2025-04-23T15:44:52.549Z
Learning: The existing implementation in `ghost/core/core/server/web/admin/middleware/serve-auth-frame-file.js` using `path.parse(req.url).base` is secure against path traversal attacks as it properly extracts only the filename component without any directory parts.
Applied to files:
ghost/core/core/server/web/shared/middleware/pretty-urls.jsghost/core/core/server/web/shared/middleware/normalize-urls.js
📚 Learning: the pending activity utilities in the ghost activitypub module are covered by tests in the file `app...
Learnt from: mike182uk
PR: TryGhost/Ghost#22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in the Ghost ActivityPub module are covered by tests in the file `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`.
Applied to files:
ghost/core/test/unit/server/web/shared/middleware/normalize-urls.test.jsghost/core/core/server/api/endpoints/utils/serializers/input/utils/gated-block-visibility.js
📚 Learning: the pending activity utilities in ghost's activitypub module are thoroughly tested in `apps/admin-x-...
Learnt from: mike182uk
PR: TryGhost/Ghost#22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in Ghost's ActivityPub module are thoroughly tested in `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`, which covers `generatePendingActivityId`, `isPendingActivity`, and `generatePendingActivity` functions.
Applied to files:
ghost/core/test/unit/server/web/shared/middleware/normalize-urls.test.jsghost/core/core/server/api/endpoints/utils/serializers/input/utils/gated-block-visibility.js
📚 Learning: in ghost admin-x-settings, webhook url validation uses {require_tld: false} to allow localhost urls ...
Learnt from: troyciesco
PR: TryGhost/Ghost#24291
File: apps/admin-x-settings/package.json:45-45
Timestamp: 2025-07-09T18:06:09.856Z
Learning: In Ghost admin-x-settings, webhook URL validation uses {require_tld: false} to allow localhost URLs for local integration testing, while social URL validation uses the default {require_tld: true} since social URLs should be public-facing.
Applied to files:
ghost/core/test/unit/server/web/shared/middleware/normalize-urls.test.jsghost/core/core/server/web/shared/middleware/normalize-urls.js
📚 Learning: the potential issues with gated content blocks (lack of migration path, race conditions during flag ...
Learnt from: kevinansfield
PR: TryGhost/Ghost#22101
File: ghost/core/core/server/api/endpoints/utils/serializers/output/utils/post-gating.js:110-117
Timestamp: 2025-02-03T17:14:37.367Z
Learning: The potential issues with gated content blocks (lack of migration path, race conditions during flag transitions, content visibility when disabled) are accepted trade-offs during the early testing phase of the contentVisibility labs feature.
Applied to files:
ghost/core/core/server/api/endpoints/utils/serializers/input/utils/gated-block-visibility.js
📚 Learning: the validatecommentdata function in ghost/core/core/server/api/endpoints/comments.js is not duplicat...
Learnt from: 55sketch
PR: TryGhost/Ghost#23894
File: ghost/core/core/server/api/endpoints/comments.js:16-22
Timestamp: 2025-06-18T10:56:19.906Z
Learning: The validateCommentData function in ghost/core/core/server/api/endpoints/comments.js is not duplicated elsewhere in the codebase - it's a unique validation function that ensures either post_id or parent_id is provided for comment creation.
Applied to files:
ghost/core/core/server/api/endpoints/utils/serializers/input/utils/gated-block-visibility.js
🪛 ESLint
ghost/core/core/server/web/shared/middleware/normalize-urls.js
[error] 29-29: Expected { after 'if' condition.
(curly)
[error] 46-46: Expected { after 'if' condition.
(curly)
[error] 61-61: Unnecessary escape character: /.
(no-useless-escape)
[error] 67-67: 'taxonomyType' is assigned a value but never used.
(no-unused-vars)
[error] 70-70: 'decodedURI' is assigned a value but never used.
(no-unused-vars)
ghost/core/core/server/api/endpoints/utils/serializers/input/utils/gated-block-visibility.js
[error] 50-50: Expected parentheses around arrow function argument having a body with curly braces.
(arrow-parens)
🔇 Additional comments (6)
ghost/core/core/server/api/endpoints/utils/serializers/input/utils/gated-block-visibility.js (1)
1-202: Inconsistent with PR objectives.This entire file appears to implement gated block visibility functionality, which is unrelated to the stated PR objective of fixing pagination URL encoding for accented characters. This suggests either the PR description is incomplete or this file was included by mistake.
Likely an incorrect or invalid review comment.
ghost/core/core/server/web/shared/middleware/pretty-urls.js (1)
20-22: LGTM! Proper middleware placement.The normalize-urls middleware is correctly positioned after uncapitalise in the pipeline, ensuring URLs are processed in the right order for the pagination URL encoding fix.
ghost/core/test/unit/server/web/shared/middleware/normalize-urls.test.js (2)
1-133: Excellent test coverage!The test suite comprehensively covers all the scenarios mentioned in the PR objectives, including accented characters, spaces, pagination, query parameters, and error handling. The test structure is well-organized and follows good practices.
99-108: Encoded URL normalization behavior is intentional and loop-safeThe middleware’s
needsNormalizationlogic decodes the slug and compares it to its “safe” normalized form, redirecting only when they differ (e.g. already-encoded accents/spaces). Once the URL is normalized (e.g./tag/securite-incendie/),needsNormalizationreturns false, sonext()is called and no further redirects occur. No redirect loop will happen—no changes required.ghost/core/core/server/web/shared/middleware/normalize-urls.js (2)
56-92: Core functionality looks solid for pagination URL encoding fix.The middleware correctly identifies taxonomy URLs, normalizes slugs with accented characters and spaces, and issues proper 301 redirects. This addresses the main PR objective of fixing pagination links with special characters.
84-86: Good security practice using removeOpenRedirectFromUrl.The use of
localUtils.removeOpenRedirectFromUrlproperly prevents open redirect vulnerabilities when constructing redirect URLs.
ghost/core/core/server/api/endpoints/utils/serializers/input/utils/gated-block-visibility.js
Outdated
Show resolved
Hide resolved
refs TryGhost#24603 - Fixed ESLint arrow-parens violations in gated-block-visibility.js - Improved regex robustness with word boundaries in CTA matching - Fixed false positive matching for non-CTA cards - Added missing curly braces for single-statement if blocks in normalize-urls.js - Removed unused variables and unnecessary regex escapes - All syntax checks pass and functionality remains intact
… pages and posts
When using the #get helper to retrieve pages or posts, the show_title_and_feature_image property was being stripped by prepareContextResource, making it inaccessible in templates. This fix preserves the property so it can be accessed directly on the retrieved items.
- Modified get.js to preserve show_title_and_feature_image for pages and posts
- Added comprehensive tests for both pages and posts
- Maintains backward compatibility with existing behavior
- Allows themes to access the property when using {{#get "pages"}} or {{#get "posts"}}
Fixes TryGhost#23316
Why are you making it?
Fixes #21999
When tags contain accented characters (e.g., "sécurité incendie") or spaces, Ghost's pagination links in
{{ghost_head}}metadata containunencoded special characters. This can confuse search engine crawlers who may attempt to crawl invalid URL variations, potentially affecting
SEO indexing and site discoverability.
What does it do?
getPaginatedUrlfunction incore/frontend/meta/paginated-url.jsto applyencodeURIComponent()to each URL segment"sécurité incendie"→"s%C3%A9curit%C3%A9%20incendie""hello world"→"hello%20world""café & résumé!"→"caf%C3%A9%20%26%20r%C3%A9sum%C3%A9!"Why is this something Ghost users or developers need?