Allow "absolute" (relative to content/) paths in index.mds (#57583)
This commit is contained in:
@@ -5,13 +5,23 @@ import { addError } from 'markdownlint-rule-helpers'
|
||||
import { getFrontmatter } from '../helpers/utils'
|
||||
|
||||
function isValidArticlePath(articlePath, currentFilePath) {
|
||||
// Article paths in recommended are relative to the current page's directory
|
||||
const relativePath = articlePath.startsWith('/') ? articlePath.substring(1) : articlePath
|
||||
const ROOT = process.env.ROOT || '.'
|
||||
|
||||
// Strategy 1: Always try as an absolute path from content root first
|
||||
const contentDir = path.join(ROOT, 'content')
|
||||
const normalizedPath = articlePath.startsWith('/') ? articlePath.substring(1) : articlePath
|
||||
const absolutePath = path.join(contentDir, `${normalizedPath}.md`)
|
||||
|
||||
if (fs.existsSync(absolutePath) && fs.statSync(absolutePath).isFile()) {
|
||||
return true
|
||||
}
|
||||
|
||||
// Strategy 2: Fall back to relative path from current file's directory
|
||||
const currentDir = path.dirname(currentFilePath)
|
||||
const fullPath = path.join(currentDir, `${relativePath}.md`)
|
||||
const relativePath = path.join(currentDir, `${normalizedPath}.md`)
|
||||
|
||||
try {
|
||||
return fs.existsSync(fullPath) && fs.statSync(fullPath).isFile()
|
||||
return fs.existsSync(relativePath) && fs.statSync(relativePath).isFile()
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
|
||||
13
src/content-linter/tests/fixtures/landing-recommended/test-absolute-only.md
vendored
Normal file
13
src/content-linter/tests/fixtures/landing-recommended/test-absolute-only.md
vendored
Normal file
@@ -0,0 +1,13 @@
|
||||
---
|
||||
title: Test Absolute Only Path
|
||||
layout: product-landing
|
||||
versions:
|
||||
fpt: '*'
|
||||
recommended:
|
||||
- /article-two
|
||||
---
|
||||
|
||||
# Test Absolute Only Path
|
||||
|
||||
This tests /article-two which exists in content/article-two.md but NOT in the current directory.
|
||||
If relative paths were tried first, this would fail.
|
||||
13
src/content-linter/tests/fixtures/landing-recommended/test-absolute-priority.md
vendored
Normal file
13
src/content-linter/tests/fixtures/landing-recommended/test-absolute-priority.md
vendored
Normal file
@@ -0,0 +1,13 @@
|
||||
---
|
||||
title: Test Absolute Path Priority
|
||||
layout: product-landing
|
||||
versions:
|
||||
fpt: '*'
|
||||
recommended:
|
||||
- /article-one
|
||||
- /subdir/article-three
|
||||
---
|
||||
|
||||
# Test Absolute Path Priority
|
||||
|
||||
Testing that absolute paths are prioritized over relative paths.
|
||||
17
src/content-linter/tests/fixtures/landing-recommended/test-path-priority.md
vendored
Normal file
17
src/content-linter/tests/fixtures/landing-recommended/test-path-priority.md
vendored
Normal file
@@ -0,0 +1,17 @@
|
||||
---
|
||||
title: Test Path Priority Resolution
|
||||
layout: product-landing
|
||||
versions:
|
||||
fpt: '*'
|
||||
recommended:
|
||||
- /article-one
|
||||
---
|
||||
|
||||
# Test Path Priority Resolution
|
||||
|
||||
This tests that /article-one resolves to the absolute path:
|
||||
tests/fixtures/fixtures/content/article-one.md (absolute from fixtures root)
|
||||
NOT the relative path:
|
||||
tests/fixtures/landing-recommended/article-one.md (relative to this file)
|
||||
|
||||
The absolute path should be prioritized over the relative path.
|
||||
14
src/content-linter/tests/fixtures/landing-recommended/test-priority-validation.md
vendored
Normal file
14
src/content-linter/tests/fixtures/landing-recommended/test-priority-validation.md
vendored
Normal file
@@ -0,0 +1,14 @@
|
||||
---
|
||||
title: Test Priority Validation
|
||||
layout: product-landing
|
||||
versions:
|
||||
fpt: '*'
|
||||
recommended:
|
||||
- /article-one
|
||||
- /nonexistent-absolute
|
||||
---
|
||||
|
||||
# Test Priority Validation
|
||||
|
||||
This tests that /article-one resolves correctly AND that /nonexistent-absolute fails properly.
|
||||
The first should pass (exists in absolute path), the second should fail (doesn't exist anywhere).
|
||||
@@ -1,4 +1,4 @@
|
||||
import { describe, expect, test } from 'vitest'
|
||||
import { describe, expect, test, beforeAll, afterAll } from 'vitest'
|
||||
|
||||
import { runRule } from '@/content-linter/lib/init-test'
|
||||
import { frontmatterLandingRecommended } from '@/content-linter/lib/linting-rules/frontmatter-landing-recommended'
|
||||
@@ -10,6 +10,12 @@ const DUPLICATE_RECOMMENDED =
|
||||
'src/content-linter/tests/fixtures/landing-recommended/duplicate-recommended.md'
|
||||
const INVALID_PATHS = 'src/content-linter/tests/fixtures/landing-recommended/invalid-paths.md'
|
||||
const NO_RECOMMENDED = 'src/content-linter/tests/fixtures/landing-recommended/no-recommended.md'
|
||||
const ABSOLUTE_PRIORITY =
|
||||
'src/content-linter/tests/fixtures/landing-recommended/test-absolute-priority.md'
|
||||
const PATH_PRIORITY = 'src/content-linter/tests/fixtures/landing-recommended/test-path-priority.md'
|
||||
const ABSOLUTE_ONLY = 'src/content-linter/tests/fixtures/landing-recommended/test-absolute-only.md'
|
||||
const PRIORITY_VALIDATION =
|
||||
'src/content-linter/tests/fixtures/landing-recommended/test-priority-validation.md'
|
||||
|
||||
const ruleName = frontmatterLandingRecommended.names[1]
|
||||
|
||||
@@ -17,6 +23,15 @@ const ruleName = frontmatterLandingRecommended.names[1]
|
||||
const fmOptions = { markdownlintOptions: { frontMatter: null } }
|
||||
|
||||
describe(ruleName, () => {
|
||||
const envVarValueBefore = process.env.ROOT
|
||||
|
||||
beforeAll(() => {
|
||||
process.env.ROOT = 'src/fixtures/fixtures'
|
||||
})
|
||||
|
||||
afterAll(() => {
|
||||
process.env.ROOT = envVarValueBefore
|
||||
})
|
||||
test('landing page with recommended articles passes', async () => {
|
||||
const result = await runRule(frontmatterLandingRecommended, {
|
||||
files: [VALID_LANDING],
|
||||
@@ -75,4 +90,61 @@ describe(ruleName, () => {
|
||||
})
|
||||
expect(result[VALID_LANDING]).toEqual([])
|
||||
})
|
||||
|
||||
test('absolute paths are prioritized over relative paths', async () => {
|
||||
// This test verifies that when both absolute and relative paths exist with the same name,
|
||||
// the absolute path is chosen over the relative path.
|
||||
//
|
||||
// Setup:
|
||||
// - /article-one should resolve to src/fixtures/fixtures/content/article-one.md (absolute)
|
||||
// - article-one (relative) would resolve to src/content-linter/tests/fixtures/landing-recommended/article-one.md
|
||||
//
|
||||
// The test passes because our logic prioritizes the absolute path resolution first
|
||||
const result = await runRule(frontmatterLandingRecommended, {
|
||||
files: [ABSOLUTE_PRIORITY],
|
||||
...fmOptions,
|
||||
})
|
||||
expect(result[ABSOLUTE_PRIORITY]).toEqual([])
|
||||
})
|
||||
|
||||
test('path priority resolution works correctly', async () => {
|
||||
// This test verifies that absolute paths are prioritized over relative paths
|
||||
// when both files exist with the same name.
|
||||
//
|
||||
// Setup:
|
||||
// - /article-one could resolve to EITHER:
|
||||
// 1. src/fixtures/fixtures/content/article-one.md (absolute - should be chosen)
|
||||
// 2. src/content-linter/tests/fixtures/landing-recommended/article-one.md (relative - should be ignored)
|
||||
//
|
||||
// Our prioritization logic should choose #1 (absolute) over #2 (relative)
|
||||
// This test passes because the absolute path exists and is found first
|
||||
const result = await runRule(frontmatterLandingRecommended, {
|
||||
files: [PATH_PRIORITY],
|
||||
...fmOptions,
|
||||
})
|
||||
expect(result[PATH_PRIORITY]).toEqual([])
|
||||
})
|
||||
|
||||
test('absolute-only paths work when no relative path exists', async () => {
|
||||
// This test verifies that absolute path resolution works when no relative path exists
|
||||
// /article-two exists in src/fixtures/fixtures/content/article-two.md
|
||||
// but NOT in src/content-linter/tests/fixtures/landing-recommended/article-two.md
|
||||
// This test would fail if we didn't prioritize absolute paths properly
|
||||
const result = await runRule(frontmatterLandingRecommended, {
|
||||
files: [ABSOLUTE_ONLY],
|
||||
...fmOptions,
|
||||
})
|
||||
expect(result[ABSOLUTE_ONLY]).toEqual([])
|
||||
})
|
||||
|
||||
test('mixed valid and invalid absolute paths are handled correctly', async () => {
|
||||
// This test has both a valid absolute path (/article-one) and an invalid one (/nonexistent-absolute)
|
||||
// It should fail because of the invalid path, proving our absolute path resolution is working
|
||||
const result = await runRule(frontmatterLandingRecommended, {
|
||||
files: [PRIORITY_VALIDATION],
|
||||
...fmOptions,
|
||||
})
|
||||
expect(result[PRIORITY_VALIDATION]).toHaveLength(1)
|
||||
expect(result[PRIORITY_VALIDATION][0].errorDetail).toContain('nonexistent-absolute')
|
||||
})
|
||||
})
|
||||
|
||||
8
src/fixtures/fixtures/article-one.md
Normal file
8
src/fixtures/fixtures/article-one.md
Normal file
@@ -0,0 +1,8 @@
|
||||
---
|
||||
title: Article One (ABSOLUTE VERSION - Should be used)
|
||||
---
|
||||
|
||||
# Article One (Absolute Version)
|
||||
|
||||
This is the ABSOLUTE version in the fixtures root directory.
|
||||
If this file is being resolved, the prioritization is CORRECT!
|
||||
8
src/fixtures/fixtures/content/article-one.md
Normal file
8
src/fixtures/fixtures/content/article-one.md
Normal file
@@ -0,0 +1,8 @@
|
||||
---
|
||||
title: Article One (ABSOLUTE VERSION - Should be used)
|
||||
---
|
||||
|
||||
# Article One (Absolute Version)
|
||||
|
||||
This is the ABSOLUTE version in fixtures/content directory.
|
||||
If this file is being resolved, the prioritization is CORRECT!
|
||||
7
src/fixtures/fixtures/content/article-two.md
Normal file
7
src/fixtures/fixtures/content/article-two.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
title: Article Two
|
||||
---
|
||||
|
||||
# Article Two
|
||||
|
||||
This is the second test article.
|
||||
7
src/fixtures/fixtures/content/subdir/article-three.md
Normal file
7
src/fixtures/fixtures/content/subdir/article-three.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
title: Article Three
|
||||
---
|
||||
|
||||
# Article Three
|
||||
|
||||
This is the third test article in a subdirectory.
|
||||
11
src/fixtures/fixtures/test-discovery-landing.md
Normal file
11
src/fixtures/fixtures/test-discovery-landing.md
Normal file
@@ -0,0 +1,11 @@
|
||||
---
|
||||
title: Test Discovery Landing Page
|
||||
intro: This is a test discovery landing page
|
||||
recommended:
|
||||
- /get-started/quickstart
|
||||
- /actions/learn-github-actions
|
||||
---
|
||||
|
||||
# Test Discovery Landing Page
|
||||
|
||||
This page has recommended articles that should be resolved.
|
||||
@@ -95,6 +95,8 @@ class Page {
|
||||
public rawIncludeGuides?: string[]
|
||||
public introLinks?: Record<string, string>
|
||||
public rawIntroLinks?: Record<string, string>
|
||||
public recommended?: string[]
|
||||
public rawRecommended?: string[]
|
||||
|
||||
// Derived properties
|
||||
public languageCode!: string
|
||||
@@ -211,6 +213,7 @@ class Page {
|
||||
this.rawLearningTracks = this.learningTracks
|
||||
this.rawIncludeGuides = this.includeGuides as any
|
||||
this.rawIntroLinks = this.introLinks
|
||||
this.rawRecommended = this.recommended
|
||||
|
||||
// Is this the Homepage or a Product, Category, Topic, or Article?
|
||||
this.documentType = getDocumentType(this.relativePath)
|
||||
|
||||
@@ -43,6 +43,7 @@ import currentProductTree from './context/current-product-tree'
|
||||
import genericToc from './context/generic-toc'
|
||||
import breadcrumbs from './context/breadcrumbs'
|
||||
import glossaries from './context/glossaries'
|
||||
import resolveRecommended from './resolve-recommended'
|
||||
import renderProductName from './context/render-product-name'
|
||||
import features from '@/versions/middleware/features'
|
||||
import productExamples from './context/product-examples'
|
||||
@@ -267,6 +268,7 @@ export default function (app: Express) {
|
||||
app.use(asyncMiddleware(glossaries))
|
||||
app.use(asyncMiddleware(generalSearchMiddleware))
|
||||
app.use(asyncMiddleware(featuredLinks))
|
||||
app.use(asyncMiddleware(resolveRecommended))
|
||||
app.use(asyncMiddleware(learningTrack))
|
||||
|
||||
if (ENABLE_FASTLY_TESTING) {
|
||||
|
||||
165
src/frame/middleware/resolve-recommended.ts
Normal file
165
src/frame/middleware/resolve-recommended.ts
Normal file
@@ -0,0 +1,165 @@
|
||||
import type { ExtendedRequest, ResolvedArticle } from '@/types'
|
||||
import type { Response, NextFunction } from 'express'
|
||||
import findPage from '@/frame/lib/find-page'
|
||||
import { renderContent } from '@/content-render/index'
|
||||
|
||||
import { createLogger } from '@/observability/logger/index'
|
||||
|
||||
const logger = createLogger('middleware:resolve-recommended')
|
||||
|
||||
/**
|
||||
* Build an article path by combining language, optional base path, and article path
|
||||
*/
|
||||
function buildArticlePath(currentLanguage: string, articlePath: string, basePath?: string): string {
|
||||
const pathPrefix = basePath ? `/${currentLanguage}/${basePath}` : `/${currentLanguage}`
|
||||
const separator = articlePath.startsWith('/') ? '' : '/'
|
||||
return `${pathPrefix}${separator}${articlePath}`
|
||||
}
|
||||
|
||||
/**
|
||||
* Try to resolve an article path using multiple resolution strategies
|
||||
*/
|
||||
function tryResolveArticlePath(
|
||||
rawPath: string,
|
||||
pageRelativePath: string | undefined,
|
||||
req: ExtendedRequest,
|
||||
): any {
|
||||
const { pages, redirects } = req.context!
|
||||
const currentLanguage = req.context!.currentLanguage || 'en'
|
||||
|
||||
// Check if we have the required dependencies
|
||||
if (!pages || !redirects) {
|
||||
return undefined
|
||||
}
|
||||
|
||||
// Strategy 1: Try content-relative path (add language prefix to raw path)
|
||||
const contentRelativePath = buildArticlePath(currentLanguage, rawPath)
|
||||
let foundPage = findPage(contentRelativePath, pages, redirects)
|
||||
|
||||
if (foundPage) {
|
||||
return foundPage
|
||||
}
|
||||
|
||||
// Strategy 2: Try page-relative path if page context is available
|
||||
if (pageRelativePath) {
|
||||
const pageDirPath = pageRelativePath.split('/').slice(0, -1).join('/')
|
||||
const pageRelativeFullPath = buildArticlePath(currentLanguage, rawPath, pageDirPath)
|
||||
foundPage = findPage(pageRelativeFullPath, pages, redirects)
|
||||
|
||||
if (foundPage) {
|
||||
return foundPage
|
||||
}
|
||||
}
|
||||
|
||||
// Strategy 3: Try with .md extension if not already present
|
||||
if (!rawPath.endsWith('.md')) {
|
||||
const pathWithExtension = `${rawPath}.md`
|
||||
|
||||
// Try Strategy 1 with .md extension
|
||||
const contentRelativePathWithExt = buildArticlePath(currentLanguage, pathWithExtension)
|
||||
foundPage = findPage(contentRelativePathWithExt, pages, redirects)
|
||||
|
||||
if (foundPage) {
|
||||
return foundPage
|
||||
}
|
||||
|
||||
// Try Strategy 2 with .md extension
|
||||
if (pageRelativePath) {
|
||||
const pageDirPath = pageRelativePath.split('/').slice(0, -1).join('/')
|
||||
const pageRelativeFullPathWithExt = buildArticlePath(
|
||||
currentLanguage,
|
||||
pathWithExtension,
|
||||
pageDirPath,
|
||||
)
|
||||
foundPage = findPage(pageRelativeFullPathWithExt, pages, redirects)
|
||||
|
||||
if (foundPage) {
|
||||
return foundPage
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return foundPage
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the href for a page from its permalinks
|
||||
*/
|
||||
function getPageHref(page: any, currentLanguage: string = 'en'): string {
|
||||
if (page.permalinks?.length > 0) {
|
||||
const permalink = page.permalinks.find((p: any) => p.languageCode === currentLanguage)
|
||||
return permalink ? permalink.href : page.permalinks[0].href
|
||||
}
|
||||
return '' // fallback
|
||||
}
|
||||
|
||||
/**
|
||||
* Middleware to resolve recommended articles from rawRecommended paths and legacy spotlight field
|
||||
*/
|
||||
async function resolveRecommended(
|
||||
req: ExtendedRequest,
|
||||
res: Response,
|
||||
next: NextFunction,
|
||||
): Promise<void> {
|
||||
try {
|
||||
const page = req.context?.page
|
||||
const rawRecommended = (page as any)?.rawRecommended
|
||||
const spotlight = (page as any)?.spotlight
|
||||
|
||||
// Collect article paths from both rawRecommended and spotlight
|
||||
const articlePaths: string[] = []
|
||||
|
||||
// Add paths from rawRecommended
|
||||
if (rawRecommended && Array.isArray(rawRecommended)) {
|
||||
articlePaths.push(...rawRecommended)
|
||||
}
|
||||
|
||||
// Add paths from spotlight (legacy field)
|
||||
if (spotlight && Array.isArray(spotlight)) {
|
||||
const spotlightPaths = spotlight
|
||||
.filter((item: any) => item && typeof item.article === 'string')
|
||||
.map((item: any) => item.article)
|
||||
articlePaths.push(...spotlightPaths)
|
||||
}
|
||||
|
||||
if (articlePaths.length === 0) {
|
||||
return next()
|
||||
}
|
||||
|
||||
const currentLanguage = req.context?.currentLanguage || 'en'
|
||||
const resolved: ResolvedArticle[] = []
|
||||
|
||||
for (const rawPath of articlePaths) {
|
||||
try {
|
||||
const foundPage = tryResolveArticlePath(rawPath, page?.relativePath, req)
|
||||
|
||||
if (foundPage) {
|
||||
const href = getPageHref(foundPage, currentLanguage)
|
||||
const category = foundPage.relativePath
|
||||
? foundPage.relativePath.split('/').slice(0, -1).filter(Boolean)
|
||||
: []
|
||||
|
||||
resolved.push({
|
||||
title: foundPage.title,
|
||||
intro: await renderContent(foundPage.intro, req.context),
|
||||
href,
|
||||
category,
|
||||
})
|
||||
}
|
||||
} catch (error) {
|
||||
logger.warn(`Failed to resolve recommended article: ${rawPath}`, { error })
|
||||
}
|
||||
}
|
||||
|
||||
// Replace the rawRecommended with resolved articles
|
||||
if (page) {
|
||||
;(page as any).recommended = resolved
|
||||
}
|
||||
} catch (error) {
|
||||
logger.error('Error in resolveRecommended middleware:', { error })
|
||||
}
|
||||
|
||||
next()
|
||||
}
|
||||
|
||||
export default resolveRecommended
|
||||
@@ -33,10 +33,22 @@ describe('content files', () => {
|
||||
return file.endsWith('.md') && !file.includes('README')
|
||||
},
|
||||
) as string[]
|
||||
const orphanedFiles = contentFiles.filter((file: string) => !relativeFiles.includes(file))
|
||||
|
||||
const orphanedFiles = contentFiles.filter((file) => !relativeFiles.includes(file))
|
||||
|
||||
// Filter out intentional test fixture files that are meant to be orphaned
|
||||
const allowedOrphanedFiles = [
|
||||
path.join(contentDir, 'article-one.md'),
|
||||
path.join(contentDir, 'article-two.md'),
|
||||
path.join(contentDir, 'subdir', 'article-three.md'),
|
||||
]
|
||||
const unexpectedOrphanedFiles = orphanedFiles.filter(
|
||||
(file) => !allowedOrphanedFiles.includes(file),
|
||||
)
|
||||
|
||||
expect(
|
||||
orphanedFiles.length,
|
||||
`${orphanedFiles} orphaned files found on disk but not in site tree`,
|
||||
unexpectedOrphanedFiles.length,
|
||||
`${unexpectedOrphanedFiles} orphaned files found on disk but not in site tree`,
|
||||
).toBe(0)
|
||||
},
|
||||
)
|
||||
|
||||
265
src/frame/tests/resolve-recommended.test.ts
Normal file
265
src/frame/tests/resolve-recommended.test.ts
Normal file
@@ -0,0 +1,265 @@
|
||||
import { describe, test, expect, vi, beforeEach } from 'vitest'
|
||||
import type { Response, NextFunction } from 'express'
|
||||
import type { ExtendedRequest } from '@/types'
|
||||
import findPage from '@/frame/lib/find-page'
|
||||
import resolveRecommended from '../middleware/resolve-recommended'
|
||||
|
||||
// Mock the findPage function
|
||||
vi.mock('@/frame/lib/find-page', () => ({
|
||||
default: vi.fn(),
|
||||
}))
|
||||
|
||||
// Mock the renderContent function
|
||||
vi.mock('@/content-render/index', () => ({
|
||||
renderContent: vi.fn((content) => `<p>${content}</p>`),
|
||||
}))
|
||||
|
||||
describe('resolveRecommended middleware', () => {
|
||||
const mockFindPage = vi.mocked(findPage)
|
||||
|
||||
const createMockRequest = (pageData: any = {}, contextData: any = {}): ExtendedRequest =>
|
||||
({
|
||||
context: {
|
||||
page: pageData,
|
||||
pages: {
|
||||
'/test-article': {
|
||||
title: 'Test Article',
|
||||
intro: 'Test intro',
|
||||
relativePath: 'test/article.md',
|
||||
},
|
||||
},
|
||||
redirects: {},
|
||||
...contextData,
|
||||
},
|
||||
}) as ExtendedRequest
|
||||
|
||||
const mockRes = {} as Response
|
||||
const mockNext = vi.fn() as NextFunction
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
})
|
||||
|
||||
test('should call next when no context', async () => {
|
||||
const req = {} as ExtendedRequest
|
||||
|
||||
await resolveRecommended(req, mockRes, mockNext)
|
||||
|
||||
expect(mockNext).toHaveBeenCalled()
|
||||
expect(mockFindPage).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
test('should call next when no page', async () => {
|
||||
const req = { context: {} } as ExtendedRequest
|
||||
|
||||
await resolveRecommended(req, mockRes, mockNext)
|
||||
|
||||
expect(mockNext).toHaveBeenCalled()
|
||||
expect(mockFindPage).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
test('should call next when no pages collection', async () => {
|
||||
const req = createMockRequest({ rawRecommended: ['/test-article'] }, { pages: undefined })
|
||||
|
||||
await resolveRecommended(req, mockRes, mockNext)
|
||||
|
||||
expect(mockNext).toHaveBeenCalled()
|
||||
expect(mockFindPage).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
test('should call next when no rawRecommended', async () => {
|
||||
const req = createMockRequest()
|
||||
|
||||
await resolveRecommended(req, mockRes, mockNext)
|
||||
|
||||
expect(mockNext).toHaveBeenCalled()
|
||||
expect(mockFindPage).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
test('should resolve recommended articles when they exist', async () => {
|
||||
const testPage: Partial<import('@/types').Page> = {
|
||||
mtime: Date.now(),
|
||||
title: 'Test Article',
|
||||
rawTitle: 'Test Article',
|
||||
intro: 'Test intro',
|
||||
rawIntro: 'Test intro',
|
||||
relativePath: 'copilot/tutorials/article.md',
|
||||
fullPath: '/full/path/copilot/tutorials/article.md',
|
||||
languageCode: 'en',
|
||||
documentType: 'article',
|
||||
markdown: 'Test content',
|
||||
versions: {},
|
||||
applicableVersions: ['free-pro-team@latest'],
|
||||
permalinks: [
|
||||
{
|
||||
languageCode: 'en',
|
||||
pageVersion: 'free-pro-team@latest',
|
||||
title: 'Test Article',
|
||||
href: '/en/copilot/tutorials/article',
|
||||
hrefWithoutLanguage: '/copilot/tutorials/article',
|
||||
},
|
||||
],
|
||||
renderProp: vi.fn().mockResolvedValue('rendered'),
|
||||
renderTitle: vi.fn().mockResolvedValue('Test Article'),
|
||||
render: vi.fn().mockResolvedValue('rendered content'),
|
||||
buildRedirects: vi.fn().mockReturnValue({}),
|
||||
}
|
||||
|
||||
mockFindPage.mockReturnValue(testPage as any)
|
||||
|
||||
const req = createMockRequest({ rawRecommended: ['/copilot/tutorials/article'] })
|
||||
|
||||
await resolveRecommended(req, mockRes, mockNext)
|
||||
|
||||
expect(mockFindPage).toHaveBeenCalledWith(
|
||||
'/en/copilot/tutorials/article',
|
||||
req.context!.pages,
|
||||
req.context!.redirects,
|
||||
)
|
||||
expect((req.context!.page as any).recommended).toEqual([
|
||||
{
|
||||
title: 'Test Article',
|
||||
intro: '<p>Test intro</p>',
|
||||
href: '/en/copilot/tutorials/article',
|
||||
category: ['copilot', 'tutorials'],
|
||||
},
|
||||
])
|
||||
expect(mockNext).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
test('should handle articles not found', async () => {
|
||||
mockFindPage.mockReturnValue(undefined)
|
||||
|
||||
const req = createMockRequest({ rawRecommended: ['/nonexistent-article'] })
|
||||
|
||||
await resolveRecommended(req, mockRes, mockNext)
|
||||
|
||||
expect(mockFindPage).toHaveBeenCalledWith(
|
||||
'/en/nonexistent-article',
|
||||
req.context!.pages,
|
||||
req.context!.redirects,
|
||||
)
|
||||
expect((req.context!.page as any).recommended).toEqual([])
|
||||
expect(mockNext).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
test('should handle errors gracefully', async () => {
|
||||
mockFindPage.mockImplementation(() => {
|
||||
throw new Error('Test error')
|
||||
})
|
||||
|
||||
const req = createMockRequest({ rawRecommended: ['/error-article'] })
|
||||
|
||||
await resolveRecommended(req, mockRes, mockNext)
|
||||
|
||||
expect((req.context!.page as any).recommended).toEqual([])
|
||||
expect(mockNext).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
test('should handle mixed valid and invalid articles', async () => {
|
||||
const testPage: Partial<import('@/types').Page> = {
|
||||
mtime: Date.now(),
|
||||
title: 'Valid Article',
|
||||
rawTitle: 'Valid Article',
|
||||
intro: 'Valid intro',
|
||||
rawIntro: 'Valid intro',
|
||||
relativePath: 'test/valid.md',
|
||||
fullPath: '/full/path/test/valid.md',
|
||||
languageCode: 'en',
|
||||
documentType: 'article',
|
||||
markdown: 'Valid content',
|
||||
versions: {},
|
||||
applicableVersions: ['free-pro-team@latest'],
|
||||
permalinks: [
|
||||
{
|
||||
languageCode: 'en',
|
||||
pageVersion: 'free-pro-team@latest',
|
||||
title: 'Valid Article',
|
||||
href: '/en/test/valid',
|
||||
hrefWithoutLanguage: '/test/valid',
|
||||
},
|
||||
],
|
||||
renderProp: vi.fn().mockResolvedValue('rendered'),
|
||||
renderTitle: vi.fn().mockResolvedValue('Valid Article'),
|
||||
render: vi.fn().mockResolvedValue('rendered content'),
|
||||
buildRedirects: vi.fn().mockReturnValue({}),
|
||||
}
|
||||
|
||||
mockFindPage.mockReturnValueOnce(testPage as any).mockReturnValueOnce(undefined)
|
||||
|
||||
const req = createMockRequest({ rawRecommended: ['/valid-article', '/invalid-article'] })
|
||||
|
||||
await resolveRecommended(req, mockRes, mockNext)
|
||||
|
||||
expect((req.context!.page as any).recommended).toEqual([
|
||||
{
|
||||
title: 'Valid Article',
|
||||
intro: '<p>Valid intro</p>',
|
||||
href: '/en/test/valid',
|
||||
category: ['test'],
|
||||
},
|
||||
])
|
||||
expect(mockNext).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
test('should try page-relative path when content-relative fails', async () => {
|
||||
const testPage: Partial<import('@/types').Page> = {
|
||||
mtime: Date.now(),
|
||||
title: 'Relative Article',
|
||||
rawTitle: 'Relative Article',
|
||||
intro: 'Relative intro',
|
||||
rawIntro: 'Relative intro',
|
||||
relativePath: 'copilot/relative-article.md',
|
||||
fullPath: '/full/path/copilot/relative-article.md',
|
||||
languageCode: 'en',
|
||||
documentType: 'article',
|
||||
markdown: 'Relative content',
|
||||
versions: {},
|
||||
applicableVersions: ['free-pro-team@latest'],
|
||||
permalinks: [
|
||||
{
|
||||
languageCode: 'en',
|
||||
pageVersion: 'free-pro-team@latest',
|
||||
title: 'Relative Article',
|
||||
href: '/en/copilot/relative-article',
|
||||
hrefWithoutLanguage: '/copilot/relative-article',
|
||||
},
|
||||
],
|
||||
renderProp: vi.fn().mockResolvedValue('rendered'),
|
||||
renderTitle: vi.fn().mockResolvedValue('Relative Article'),
|
||||
render: vi.fn().mockResolvedValue('rendered content'),
|
||||
buildRedirects: vi.fn().mockReturnValue({}),
|
||||
}
|
||||
|
||||
// Mock findPage to fail on first call (content-relative) and succeed on second (page-relative)
|
||||
mockFindPage.mockReturnValueOnce(undefined).mockReturnValueOnce(testPage as any)
|
||||
|
||||
const req = createMockRequest({
|
||||
rawRecommended: ['relative-article'],
|
||||
relativePath: 'copilot/index.md',
|
||||
})
|
||||
|
||||
await resolveRecommended(req, mockRes, mockNext)
|
||||
|
||||
expect(mockFindPage).toHaveBeenCalledTimes(2)
|
||||
expect(mockFindPage).toHaveBeenCalledWith(
|
||||
'/en/relative-article',
|
||||
req.context!.pages,
|
||||
req.context!.redirects,
|
||||
)
|
||||
expect(mockFindPage).toHaveBeenCalledWith(
|
||||
'/en/copilot/relative-article',
|
||||
req.context!.pages,
|
||||
req.context!.redirects,
|
||||
)
|
||||
expect((req.context!.page as any).recommended).toEqual([
|
||||
{
|
||||
title: 'Relative Article',
|
||||
intro: '<p>Relative intro</p>',
|
||||
href: '/en/copilot/relative-article',
|
||||
category: ['copilot'],
|
||||
},
|
||||
])
|
||||
expect(mockNext).toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
@@ -21,7 +21,7 @@ export const DiscoveryLanding = () => {
|
||||
<div>
|
||||
<LandingHero title={title} intro={intro} heroImage={heroImage} introLinks={introLinks} />
|
||||
<div className="container-xl px-3 px-md-6 mt-6 mb-4">
|
||||
<LandingCarousel flatArticles={flatArticles} recommended={recommended} />
|
||||
<LandingCarousel recommended={recommended} />
|
||||
<ArticleGrid flatArticles={flatArticles} />
|
||||
</div>
|
||||
</div>
|
||||
|
||||
@@ -2,22 +2,13 @@ import { useState, useEffect, useRef } from 'react'
|
||||
import { ChevronLeftIcon, ChevronRightIcon } from '@primer/octicons-react'
|
||||
import { Token } from '@primer/react'
|
||||
import cx from 'classnames'
|
||||
import type { TocItem } from '@/landings/types'
|
||||
import type { ResolvedArticle } from '@/types'
|
||||
import { useTranslation } from '@/languages/components/useTranslation'
|
||||
import styles from './LandingCarousel.module.scss'
|
||||
|
||||
type ProcessedArticleItem = {
|
||||
article: string
|
||||
title: string
|
||||
description: string
|
||||
url: string
|
||||
category: string[]
|
||||
}
|
||||
|
||||
type LandingCarouselProps = {
|
||||
heading?: string
|
||||
recommended?: string[] // Array of article paths
|
||||
flatArticles: TocItem[]
|
||||
recommended?: ResolvedArticle[]
|
||||
}
|
||||
|
||||
// Hook to get current items per view based on screen size
|
||||
@@ -47,11 +38,7 @@ const useResponsiveItemsPerView = () => {
|
||||
return itemsPerView
|
||||
}
|
||||
|
||||
export const LandingCarousel = ({
|
||||
heading = '',
|
||||
flatArticles,
|
||||
recommended,
|
||||
}: LandingCarouselProps) => {
|
||||
export const LandingCarousel = ({ heading = '', recommended }: LandingCarouselProps) => {
|
||||
const [currentPage, setCurrentPage] = useState(0)
|
||||
const [isAnimating, setIsAnimating] = useState(false)
|
||||
const itemsPerView = useResponsiveItemsPerView()
|
||||
@@ -65,6 +52,8 @@ export const LandingCarousel = ({
|
||||
setCurrentPage(0)
|
||||
}, [itemsPerView])
|
||||
|
||||
const processedItems: ResolvedArticle[] = recommended || []
|
||||
|
||||
// Cleanup timeout on unmount
|
||||
useEffect(() => {
|
||||
return () => {
|
||||
@@ -74,34 +63,6 @@ export const LandingCarousel = ({
|
||||
}
|
||||
}, [])
|
||||
|
||||
// Helper function to find article data from tocItems
|
||||
const findArticleData = (articlePath: string) => {
|
||||
if (typeof articlePath !== 'string') {
|
||||
console.warn('Invalid articlePath:', articlePath)
|
||||
return null
|
||||
}
|
||||
const cleanPath = articlePath.startsWith('/') ? articlePath.slice(1) : articlePath
|
||||
return flatArticles.find(
|
||||
(item) =>
|
||||
item.fullPath?.endsWith(cleanPath) ||
|
||||
item.fullPath?.includes(cleanPath.split('/').pop() || ''),
|
||||
)
|
||||
}
|
||||
|
||||
// Process recommended articles to get article data
|
||||
const processedItems = (recommended || [])
|
||||
.filter((item) => typeof item === 'string' && item.length > 0)
|
||||
.map((recommendedArticlePath) => {
|
||||
const articleData = findArticleData(recommendedArticlePath)
|
||||
return {
|
||||
article: recommendedArticlePath,
|
||||
title: articleData?.title || 'Unknown Article',
|
||||
description: articleData?.intro || '',
|
||||
url: articleData?.fullPath || recommendedArticlePath,
|
||||
category: articleData?.category || [],
|
||||
}
|
||||
})
|
||||
|
||||
const totalItems = processedItems.length
|
||||
const totalPages = Math.ceil(totalItems / itemsPerView)
|
||||
|
||||
@@ -182,22 +143,27 @@ export const LandingCarousel = ({
|
||||
className={cx(styles.itemsGrid, { [styles.animating]: isAnimating })}
|
||||
data-testid="carousel-items"
|
||||
>
|
||||
{visibleItems.map((article: ProcessedArticleItem, index) => (
|
||||
{visibleItems.map((article: ResolvedArticle, index) => (
|
||||
<div
|
||||
key={startIndex + index}
|
||||
className={cx(styles.articleCard, 'border', 'border-default', 'rounded-2')}
|
||||
>
|
||||
<div className="mb-2">
|
||||
{article.category.map((cat) => (
|
||||
{article.category.map((cat: string) => (
|
||||
<Token key={cat} text={cat} className="mr-1 mb-2" />
|
||||
))}
|
||||
</div>
|
||||
<h3 className={styles.articleTitle}>
|
||||
<a href={article.url} className={styles.articleLink}>
|
||||
<a href={article.href} className={styles.articleLink}>
|
||||
{article.title}
|
||||
</a>
|
||||
</h3>
|
||||
<p className={styles.articleDescription}>{article.description}</p>
|
||||
<div
|
||||
className={styles.articleDescription}
|
||||
dangerouslySetInnerHTML={{
|
||||
__html: article.intro as TrustedHTML,
|
||||
}}
|
||||
/>
|
||||
</div>
|
||||
))}
|
||||
</div>
|
||||
|
||||
@@ -1,8 +1,9 @@
|
||||
import { createContext, useContext } from 'react'
|
||||
import { FeaturedLink, getFeaturedLinksFromReq } from '@/landings/components/ProductLandingContext'
|
||||
import { getFeaturedLinksFromReq } from '@/landings/components/ProductLandingContext'
|
||||
import { mapRawTocItemToTocItem } from '@/landings/types'
|
||||
import type { TocItem } from '@/landings/types'
|
||||
import type { LearningTrack } from '@/types'
|
||||
import type { FeaturedLink } from '@/landings/components/ProductLandingContext'
|
||||
|
||||
export type LandingType = 'bespoke' | 'discovery' | 'journey'
|
||||
|
||||
@@ -20,8 +21,7 @@ export type LandingContextT = {
|
||||
currentLayout: string
|
||||
heroImage?: string
|
||||
// For discovery landing pages
|
||||
recommended?: string[] // Array of article paths
|
||||
// For discovery landing pages
|
||||
recommended?: Array<{ title: string; intro: string; href: string; category: string[] }> // Resolved article data
|
||||
introLinks?: Record<string, string>
|
||||
}
|
||||
|
||||
@@ -37,21 +37,21 @@ export const useLandingContext = (): LandingContextT => {
|
||||
return context
|
||||
}
|
||||
|
||||
/**
|
||||
* Server-side function to create LandingContext data from a request.
|
||||
*/
|
||||
export const getLandingContextFromRequest = async (
|
||||
req: any,
|
||||
landingType: LandingType,
|
||||
): Promise<LandingContextT> => {
|
||||
const page = req.context.page
|
||||
|
||||
let recommended: string[] = []
|
||||
let recommended: Array<{ title: string; intro: string; href: string; category: string[] }> = []
|
||||
|
||||
if (landingType === 'discovery') {
|
||||
// Support legacy `spotlight` property as `recommended` for pages like Copilot Cookbook
|
||||
// However, `spotlight` will have lower priority than the `recommended` property
|
||||
if (page.recommended && page.recommended.length > 0) {
|
||||
// Use resolved recommended articles from the page after middleware processing
|
||||
if (page.recommended && Array.isArray(page.recommended) && page.recommended.length > 0) {
|
||||
recommended = page.recommended
|
||||
} else if (page.spotlight && page.spotlight.length > 0) {
|
||||
// Remove the `image` property from spotlight items, since we don't use those for the carousel
|
||||
recommended = page.spotlight.map((item: any) => item.article)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -4,6 +4,14 @@ import type { Failbot } from '@github/failbot'
|
||||
import type enterpriseServerReleases from '@/versions/lib/enterprise-server-releases.d'
|
||||
import type { ValidOcticon } from '@/landings/types'
|
||||
|
||||
// Shared type for resolved article information used across landing pages and carousels
|
||||
export interface ResolvedArticle {
|
||||
title: string
|
||||
intro: string
|
||||
href: string
|
||||
category: string[]
|
||||
}
|
||||
|
||||
// Throughout our codebase we "extend" the Request object by attaching
|
||||
// things to it. For example `req.context = { currentCategory: 'foo' }`.
|
||||
// This type aims to match all the custom things we do to requests
|
||||
|
||||
Reference in New Issue
Block a user