Conversation
WalkthroughSlugs changed to category/article segment format (no locale/base). A CMS-configured basePath (parent.slug) is derived and threaded through frontend, services, and mappers to build full URLs; Zendesk mappers drop locale-based URL logic and now emit segment-only slugs. Changes
Sequence Diagram(s)sequenceDiagram
participant Frontend
participant CMS
participant SDK
participant Service
participant Mapper
Frontend->>CMS: read block parent.slug (optional)
Frontend->>SDK: request.searchArticles({ basePath: parent?.slug })
SDK->>Service: searchArticles(query with basePath)
Service->>Mapper: mapArticles(articles, basePath)
Mapper->>Mapper: prefix slugs with basePath -> produce full URLs
Mapper-->>Service: Article payloads (slug/url)
Service-->>Frontend: response (blocks with parent & URLs)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/blocks/category/src/api-harmonization/category.mapper.ts (1)
13-58:⚠️ Potential issue | 🟠 MajorNormalize basePath/slug joins to prevent malformed slugs.
basePathcan be empty or already include slashes, causing/slugor//slug. This is a routing/slug-matching risk across category/article pages.🛠️ Proposed fix (shared join helper)
export const mapCategory = ( @@ - const basePath = cms.parent?.slug ?? ''; + const basePath = cms.parent?.slug?.replace(/^\/+|\/+$/g, '') ?? ''; @@ - data: articles.data.map((article) => mapArticle(article, cms, _locale, basePath)), + data: articles.data.map((article) => mapArticle(article, cms, _locale, basePath)), }, }, }; }; @@ export const mapCategoryArticles = ( @@ - const basePath = cms.parent?.slug ?? ''; + const basePath = cms.parent?.slug?.replace(/^\/+|\/+$/g, '') ?? ''; @@ - data: articles.data.map((article) => mapArticle(article, cms, _locale, basePath)), + data: articles.data.map((article) => mapArticle(article, cms, _locale, basePath)), }, }; }; @@ const mapArticle = ( @@ - basePath: string, + basePath: string, ) => { return { ...article, - slug: `${basePath}/${article.slug}`, + slug: basePath ? `${basePath}/${article.slug.replace(/^\/+/, '')}` : article.slug.replace(/^\/+/, ''), createdAt: Utils.Date.formatDateRelative(article.createdAt, _locale, cms.labels.today, cms.labels.yesterday), updatedAt: Utils.Date.formatDateRelative(article.updatedAt, _locale, cms.labels.today, cms.labels.yesterday), }; };packages/integrations/zendesk/src/modules/articles/zendesk-article.service.ts (1)
224-259:⚠️ Potential issue | 🟠 MajorNormalize category slugs before comparison (basePath-aware).
If the UI now passes basePath-prefixed slugs (e.g.,
help/category-slug),mapCategory(cat).slug === options.id(and resolveCategoryId comparisons) will never match, breaking category pages and filters.🛠️ Proposed fix (normalize to last path segment)
+ private normalizeSlug(slug: string): string { + const segments = slug.split('/').filter(Boolean); + return segments[segments.length - 1] ?? slug; + } @@ - const categoryId = Number(options.id); + const normalizedId = this.normalizeSlug(options.id); + const categoryId = Number(normalizedId); @@ - const category = categories.find((cat) => { - const mapped = mapCategory(cat); - return mapped.slug === options.id || mapped.id === options.id; - }); + const category = categories.find((cat) => { + const mapped = mapCategory(cat); + return mapped.slug === normalizedId || mapped.id === normalizedId; + }); @@ - return mapCategory(category); + return mapCategory(category); }), @@ - private resolveCategoryId(categoryIdOrSlug: string, zendeskLocale: string): Observable<number | undefined> { + private resolveCategoryId(categoryIdOrSlug: string, zendeskLocale: string): Observable<number | undefined> { + const normalized = this.normalizeSlug(categoryIdOrSlug); // If it's already a numeric ID, return it - const numericId = Number(categoryIdOrSlug); + const numericId = Number(normalized); if (!isNaN(numericId)) { return of(numericId); } @@ - const mapped = mapCategory(category); - if (mapped.slug === categoryIdOrSlug || mapped.id === categoryIdOrSlug) { + const mapped = mapCategory(category); + if (mapped.slug === normalized || mapped.id === normalized) { return category.id; }Also applies to: 442-455
🤖 Fix all issues with AI agents
In `@apps/api-harmonization/src/modules/page/page.mapper.ts`:
- Around line 118-120: The breadcrumb URL construction using basePath is
creating double slashes when basePath is "/" or has a trailing slash; update the
logic that builds categoryUrl and articleUrl in page.mapper.ts to normalize
basePath first (e.g., compute a normalizedBasePath by removing a trailing slash
unless basePath === "/"), then use `${normalizedBasePath}/${category.slug}` and
`${normalizedBasePath}/${category.slug}/${article.slug}` (or the existing
categoryUrl/articleUrl variables) so URLs never contain duplicate slashes;
ensure normalization is applied wherever basePath is used to build routes.
In `@apps/docs/docs/integrations/articles/zendesk/features.md`:
- Around line 147-172: The two fenced code blocks under "Article slug format:"
and "Category slug format:" in features.md are missing language identifiers,
causing markdownlint MD040; update both fences to include a language (e.g.,
change the three-backtick openings for the article slug and the category slug
blocks to use ```text) so the slug examples like
"{category-id}-{category-name}/{article-id}-{article-title}" and
"{category-id}-{category-name}" are marked as text.
In `@packages/blocks/category-list/src/api-harmonization/category-list.mapper.ts`:
- Around line 10-20: The slug join in the CategoryListBlock mapping can produce
leading or double slashes because basePath (from cms.parent?.slug) may be empty
or end with a slash; update the items mapping in the function that builds the
CategoryListBlock so it sanitizes basePath and category.slug and conditionally
joins them (e.g., trim trailing slash from basePath, trim leading slash from
category.slug, and if basePath is empty return category.slug only) before
assigning slug, ensuring the resulting slug has a single slash separator and no
leading double slashes.
🧹 Nitpick comments (2)
packages/blocks/article-search/src/api-harmonization/article-search.mapper.ts (1)
20-26: Potential URL format inconsistency with other mappers.When
basePathis falsy, the URL becomes justarticle.slug(e.g.,"some-article"). However, inarticle-list.mapper.ts, the pattern${basePath}/${article.slug}with emptybasePathproduces"/some-article"(with leading slash).Consider aligning the behavior for consistency:
♻️ Optional: Consistent URL handling
export const mapArticles = (articles: Articles.Model.Articles, basePath?: string): ArticleList => { return { articles: articles.data.map((article) => ({ label: article.title, - url: basePath ? `${basePath}/${article.slug}` : article.slug, + url: `${basePath ?? ''}/${article.slug}`, })), }; };apps/docs/docs/integrations/articles/zendesk/usage.md (1)
276-278: Add language specification to fenced code block.The code block showing the slug format template should have a language specified for consistency and to satisfy linting rules.
📝 Proposed fix
-``` +```text {category-id}-{category-name}/{article-id}-{article-title}</details> </blockquote></details> </blockquote></details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| const basePath = cms.parent?.slug ?? ''; | ||
|
|
||
| return { | ||
| __typename: 'CategoryListBlock', | ||
| id: cms.id, | ||
| title: cms.title, | ||
| description: cms.description, | ||
| items: categories, | ||
| items: categories.map((category) => ({ | ||
| ...category, | ||
| slug: `${basePath}/${category.slug}`, | ||
| })), |
There was a problem hiding this comment.
Normalize basePath/slug joins to avoid leading or double slashes.
When cms.parent?.slug is empty or already contains a trailing slash, ${basePath}/${category.slug} yields /slug or //slug, which can break slug matching and routing.
🛠️ Proposed fix (sanitize + conditional join)
- const basePath = cms.parent?.slug ?? '';
+ const basePath = cms.parent?.slug?.replace(/^\/+|\/+$/g, '') ?? '';
return {
@@
- items: categories.map((category) => ({
- ...category,
- slug: `${basePath}/${category.slug}`,
- })),
+ items: categories.map((category) => {
+ const categorySlug = category.slug.replace(/^\/+/, '');
+ return {
+ ...category,
+ slug: basePath ? `${basePath}/${categorySlug}` : categorySlug,
+ };
+ }),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const basePath = cms.parent?.slug ?? ''; | |
| return { | |
| __typename: 'CategoryListBlock', | |
| id: cms.id, | |
| title: cms.title, | |
| description: cms.description, | |
| items: categories, | |
| items: categories.map((category) => ({ | |
| ...category, | |
| slug: `${basePath}/${category.slug}`, | |
| })), | |
| const basePath = cms.parent?.slug?.replace(/^\/+|\/+$/g, '') ?? ''; | |
| return { | |
| __typename: 'CategoryListBlock', | |
| id: cms.id, | |
| title: cms.title, | |
| description: cms.description, | |
| items: categories.map((category) => { | |
| const categorySlug = category.slug.replace(/^\/+/, ''); | |
| return { | |
| ...category, | |
| slug: basePath ? `${basePath}/${categorySlug}` : categorySlug, | |
| }; | |
| }), |
🤖 Prompt for AI Agents
In `@packages/blocks/category-list/src/api-harmonization/category-list.mapper.ts`
around lines 10 - 20, The slug join in the CategoryListBlock mapping can produce
leading or double slashes because basePath (from cms.parent?.slug) may be empty
or end with a slash; update the items mapping in the function that builds the
CategoryListBlock so it sanitizes basePath and category.slug and conditionally
joins them (e.g., trim trailing slash from basePath, trim leading slash from
category.slug, and if basePath is empty return category.slug only) before
assigning slug, ensuring the resulting slug has a single slash separator and no
leading double slashes.
… to prevent double slashes
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Coverage Report for packages/configs/vitest-config
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
What does this PR do?
Related Ticket(s)
Key Changes
How to test
Media (Loom or gif)
Summary by CodeRabbit
New Features
Refactor
Documentation
Chores