diff --git a/src/content-linter/lib/linting-rules/frontmatter-landing-recommended.js b/src/content-linter/lib/linting-rules/frontmatter-landing-recommended.js index 06bdbb4774cf..cdbc1667b19a 100644 --- a/src/content-linter/lib/linting-rules/frontmatter-landing-recommended.js +++ b/src/content-linter/lib/linting-rules/frontmatter-landing-recommended.js @@ -5,23 +5,13 @@ import { addError } from 'markdownlint-rule-helpers' import { getFrontmatter } from '../helpers/utils' function isValidArticlePath(articlePath, currentFilePath) { - 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 + // Article paths in recommended are relative to the current page's directory + const relativePath = articlePath.startsWith('/') ? articlePath.substring(1) : articlePath const currentDir = path.dirname(currentFilePath) - const relativePath = path.join(currentDir, `${normalizedPath}.md`) + const fullPath = path.join(currentDir, `${relativePath}.md`) try { - return fs.existsSync(relativePath) && fs.statSync(relativePath).isFile() + return fs.existsSync(fullPath) && fs.statSync(fullPath).isFile() } catch { return false } diff --git a/src/content-linter/tests/fixtures/landing-recommended/test-absolute-only.md b/src/content-linter/tests/fixtures/landing-recommended/test-absolute-only.md deleted file mode 100644 index 964f85cf6a49..000000000000 --- a/src/content-linter/tests/fixtures/landing-recommended/test-absolute-only.md +++ /dev/null @@ -1,13 +0,0 @@ ---- -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. diff --git a/src/content-linter/tests/fixtures/landing-recommended/test-absolute-priority.md b/src/content-linter/tests/fixtures/landing-recommended/test-absolute-priority.md deleted file mode 100644 index bc529b8d8b3e..000000000000 --- a/src/content-linter/tests/fixtures/landing-recommended/test-absolute-priority.md +++ /dev/null @@ -1,13 +0,0 @@ ---- -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. diff --git a/src/content-linter/tests/fixtures/landing-recommended/test-path-priority.md b/src/content-linter/tests/fixtures/landing-recommended/test-path-priority.md deleted file mode 100644 index 226c7f956a91..000000000000 --- a/src/content-linter/tests/fixtures/landing-recommended/test-path-priority.md +++ /dev/null @@ -1,17 +0,0 @@ ---- -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. diff --git a/src/content-linter/tests/fixtures/landing-recommended/test-priority-validation.md b/src/content-linter/tests/fixtures/landing-recommended/test-priority-validation.md deleted file mode 100644 index 248f88d8433c..000000000000 --- a/src/content-linter/tests/fixtures/landing-recommended/test-priority-validation.md +++ /dev/null @@ -1,14 +0,0 @@ ---- -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). diff --git a/src/content-linter/tests/unit/frontmatter-landing-recommended.js b/src/content-linter/tests/unit/frontmatter-landing-recommended.js index 7dc86e0c197c..12969e8c0fea 100644 --- a/src/content-linter/tests/unit/frontmatter-landing-recommended.js +++ b/src/content-linter/tests/unit/frontmatter-landing-recommended.js @@ -1,4 +1,4 @@ -import { describe, expect, test, beforeAll, afterAll } from 'vitest' +import { describe, expect, test } from 'vitest' import { runRule } from '@/content-linter/lib/init-test' import { frontmatterLandingRecommended } from '@/content-linter/lib/linting-rules/frontmatter-landing-recommended' @@ -10,12 +10,6 @@ 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] @@ -23,15 +17,6 @@ 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], @@ -90,61 +75,4 @@ 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') - }) }) diff --git a/src/fixtures/fixtures/article-one.md b/src/fixtures/fixtures/article-one.md deleted file mode 100644 index feb27b1fbd64..000000000000 --- a/src/fixtures/fixtures/article-one.md +++ /dev/null @@ -1,8 +0,0 @@ ---- -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! diff --git a/src/fixtures/fixtures/content/article-one.md b/src/fixtures/fixtures/content/article-one.md deleted file mode 100644 index bd3091463779..000000000000 --- a/src/fixtures/fixtures/content/article-one.md +++ /dev/null @@ -1,8 +0,0 @@ ---- -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! diff --git a/src/fixtures/fixtures/content/article-two.md b/src/fixtures/fixtures/content/article-two.md deleted file mode 100644 index 0bb5dfaa2653..000000000000 --- a/src/fixtures/fixtures/content/article-two.md +++ /dev/null @@ -1,7 +0,0 @@ ---- -title: Article Two ---- - -# Article Two - -This is the second test article. diff --git a/src/fixtures/fixtures/content/subdir/article-three.md b/src/fixtures/fixtures/content/subdir/article-three.md deleted file mode 100644 index 4543250472e1..000000000000 --- a/src/fixtures/fixtures/content/subdir/article-three.md +++ /dev/null @@ -1,7 +0,0 @@ ---- -title: Article Three ---- - -# Article Three - -This is the third test article in a subdirectory. diff --git a/src/fixtures/fixtures/test-discovery-landing.md b/src/fixtures/fixtures/test-discovery-landing.md deleted file mode 100644 index 5eb46113b294..000000000000 --- a/src/fixtures/fixtures/test-discovery-landing.md +++ /dev/null @@ -1,11 +0,0 @@ ---- -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. diff --git a/src/frame/lib/page.ts b/src/frame/lib/page.ts index 3ce5de6edcc6..5da0d76918a6 100644 --- a/src/frame/lib/page.ts +++ b/src/frame/lib/page.ts @@ -95,8 +95,6 @@ class Page { public rawIncludeGuides?: string[] public introLinks?: Record public rawIntroLinks?: Record - public recommended?: string[] - public rawRecommended?: string[] // Derived properties public languageCode!: string @@ -213,7 +211,6 @@ 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) diff --git a/src/frame/middleware/index.ts b/src/frame/middleware/index.ts index 0e64c35a0db7..1b9de2e6a309 100644 --- a/src/frame/middleware/index.ts +++ b/src/frame/middleware/index.ts @@ -43,7 +43,6 @@ 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' @@ -268,7 +267,6 @@ 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) { diff --git a/src/frame/middleware/resolve-recommended.ts b/src/frame/middleware/resolve-recommended.ts deleted file mode 100644 index 842883bf6751..000000000000 --- a/src/frame/middleware/resolve-recommended.ts +++ /dev/null @@ -1,165 +0,0 @@ -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 { - 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 diff --git a/src/frame/tests/content.ts b/src/frame/tests/content.ts index 0e145e55e82e..624d3a81c642 100644 --- a/src/frame/tests/content.ts +++ b/src/frame/tests/content.ts @@ -33,22 +33,10 @@ describe('content files', () => { return file.endsWith('.md') && !file.includes('README') }, ) as string[] - - 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), - ) - + const orphanedFiles = contentFiles.filter((file: string) => !relativeFiles.includes(file)) expect( - unexpectedOrphanedFiles.length, - `${unexpectedOrphanedFiles} orphaned files found on disk but not in site tree`, + orphanedFiles.length, + `${orphanedFiles} orphaned files found on disk but not in site tree`, ).toBe(0) }, ) diff --git a/src/frame/tests/resolve-recommended.test.ts b/src/frame/tests/resolve-recommended.test.ts deleted file mode 100644 index 2ed9f26ab12f..000000000000 --- a/src/frame/tests/resolve-recommended.test.ts +++ /dev/null @@ -1,265 +0,0 @@ -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) => `

${content}

`), -})) - -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 = { - 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: '

Test intro

', - 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 = { - 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: '

Valid intro

', - href: '/en/test/valid', - category: ['test'], - }, - ]) - expect(mockNext).toHaveBeenCalled() - }) - - test('should try page-relative path when content-relative fails', async () => { - const testPage: Partial = { - 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: '

Relative intro

', - href: '/en/copilot/relative-article', - category: ['copilot'], - }, - ]) - expect(mockNext).toHaveBeenCalled() - }) -}) diff --git a/src/landings/components/discovery/DiscoveryLanding.tsx b/src/landings/components/discovery/DiscoveryLanding.tsx index 657d8e9f3d42..f1a13c14cd11 100644 --- a/src/landings/components/discovery/DiscoveryLanding.tsx +++ b/src/landings/components/discovery/DiscoveryLanding.tsx @@ -21,7 +21,7 @@ export const DiscoveryLanding = () => {
- +
diff --git a/src/landings/components/shared/LandingCarousel.tsx b/src/landings/components/shared/LandingCarousel.tsx index aa085d5f0cf2..0a651c77bc1e 100644 --- a/src/landings/components/shared/LandingCarousel.tsx +++ b/src/landings/components/shared/LandingCarousel.tsx @@ -2,13 +2,22 @@ import { useState, useEffect, useRef } from 'react' import { ChevronLeftIcon, ChevronRightIcon } from '@primer/octicons-react' import { Token } from '@primer/react' import cx from 'classnames' -import type { ResolvedArticle } from '@/types' +import type { TocItem } from '@/landings/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?: ResolvedArticle[] + recommended?: string[] // Array of article paths + flatArticles: TocItem[] } // Hook to get current items per view based on screen size @@ -38,7 +47,11 @@ const useResponsiveItemsPerView = () => { return itemsPerView } -export const LandingCarousel = ({ heading = '', recommended }: LandingCarouselProps) => { +export const LandingCarousel = ({ + heading = '', + flatArticles, + recommended, +}: LandingCarouselProps) => { const [currentPage, setCurrentPage] = useState(0) const [isAnimating, setIsAnimating] = useState(false) const itemsPerView = useResponsiveItemsPerView() @@ -52,8 +65,6 @@ export const LandingCarousel = ({ heading = '', recommended }: LandingCarouselPr setCurrentPage(0) }, [itemsPerView]) - const processedItems: ResolvedArticle[] = recommended || [] - // Cleanup timeout on unmount useEffect(() => { return () => { @@ -63,6 +74,34 @@ export const LandingCarousel = ({ heading = '', recommended }: LandingCarouselPr } }, []) + // 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) @@ -143,27 +182,22 @@ export const LandingCarousel = ({ heading = '', recommended }: LandingCarouselPr className={cx(styles.itemsGrid, { [styles.animating]: isAnimating })} data-testid="carousel-items" > - {visibleItems.map((article: ResolvedArticle, index) => ( + {visibleItems.map((article: ProcessedArticleItem, index) => (
- {article.category.map((cat: string) => ( + {article.category.map((cat) => ( ))}

- + {article.title}

-
+

{article.description}

))}
diff --git a/src/landings/context/LandingContext.tsx b/src/landings/context/LandingContext.tsx index 6e4030555c55..0739c142295e 100644 --- a/src/landings/context/LandingContext.tsx +++ b/src/landings/context/LandingContext.tsx @@ -1,9 +1,8 @@ import { createContext, useContext } from 'react' -import { getFeaturedLinksFromReq } from '@/landings/components/ProductLandingContext' +import { FeaturedLink, 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' @@ -21,7 +20,8 @@ export type LandingContextT = { currentLayout: string heroImage?: string // For discovery landing pages - recommended?: Array<{ title: string; intro: string; href: string; category: string[] }> // Resolved article data + recommended?: string[] // Array of article paths + // For discovery landing pages introLinks?: Record } @@ -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 => { const page = req.context.page - let recommended: Array<{ title: string; intro: string; href: string; category: string[] }> = [] - + let recommended: string[] = [] if (landingType === 'discovery') { - // Use resolved recommended articles from the page after middleware processing - if (page.recommended && Array.isArray(page.recommended) && page.recommended.length > 0) { + // 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) { 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) } } diff --git a/src/types.ts b/src/types.ts index 53d39f91b62a..d5108fc5aad3 100644 --- a/src/types.ts +++ b/src/types.ts @@ -4,14 +4,6 @@ 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