carousel component fixes 🎠 (#58285)

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Author: rsese

src/frame/middleware/resolve-recommended.ts

@@ -105,17 +105,17 @@ async function resolveRecommended(
     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[] = []
+    // Collect article paths from rawRecommended or spotlight if there are no
+    // recommended articles
+    let 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)) {
+    // Add paths from spotlight (legacy field) if no recommended articles
+    if (articlePaths.length === 0 && spotlight && Array.isArray(spotlight)) {
       const spotlightPaths = spotlight
         .filter((item: any) => item && typeof item.article === 'string')
         .map((item: any) => item.article)
@@ -126,6 +126,8 @@ async function resolveRecommended(
       return next()
     }
 
+    // remove duplicate articles
+    articlePaths = [...new Set(articlePaths)]
     const resolved: ResolvedArticle[] = []
 
     for (const rawPath of articlePaths) {

src/frame/tests/resolve-recommended.test.ts

@@ -80,31 +80,10 @@ describe('resolveRecommended middleware', () => {
 
   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)
@@ -129,6 +108,40 @@ describe('resolveRecommended middleware', () => {
     expect(mockNext).toHaveBeenCalled()
   })
 
+  test('should not resolve spotlight articles when there are recommended articles', async () => {
+    const testPage: Partial<import('@/types').Page> = {
+      title: 'Test Article',
+      intro: 'Test intro',
+      relativePath: 'copilot/tutorials/article.md',
+      applicableVersions: ['free-pro-team@latest'],
+    }
+
+    mockFindPage.mockReturnValueOnce(testPage as any)
+
+    const req = createMockRequest({
+      rawRecommended: ['/copilot/tutorials/article'],
+      spotlight: [{ article: '/copilot/tutorials/spotlight-article' }],
+    })
+
+    await resolveRecommended(req, mockRes, mockNext)
+
+    expect(mockFindPage).toHaveBeenCalledTimes(1)
+    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: '/copilot/tutorials/article',
+        category: ['copilot', 'tutorials'],
+      },
+    ])
+    expect(mockNext).toHaveBeenCalled()
+  })
+
   test('should handle articles not found', async () => {
     mockFindPage.mockReturnValue(undefined)
 
@@ -160,31 +173,10 @@ describe('resolveRecommended middleware', () => {
 
   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)
@@ -206,31 +198,10 @@ describe('resolveRecommended middleware', () => {
 
   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)
@@ -267,31 +238,10 @@ describe('resolveRecommended middleware', () => {
 
   test('returns paths without language or version prefixes', async () => {
     const testPage: Partial<import('@/types').Page> = {
-      mtime: Date.now(),
       title: 'Tutorial Page',
-      rawTitle: 'Tutorial Page',
       intro: 'Tutorial intro',
-      rawIntro: 'Tutorial intro',
       relativePath: 'copilot/tutorials/tutorial-page/index.md',
-      fullPath: '/full/path/copilot/tutorials/tutorial-page/index.md',
-      languageCode: 'en',
-      documentType: 'article',
-      markdown: 'Tutorial content',
-      versions: {},
       applicableVersions: ['free-pro-team@latest'],
-      permalinks: [
-        {
-          languageCode: 'en',
-          pageVersion: 'free-pro-team@latest',
-          title: 'Tutorial Page',
-          href: '/en/copilot/tutorials/tutorial-page',
-          hrefWithoutLanguage: '/copilot/tutorials/tutorial-page',
-        },
-      ],
-      renderProp: vi.fn().mockResolvedValue('rendered'),
-      renderTitle: vi.fn().mockResolvedValue('Tutorial Page'),
-      render: vi.fn().mockResolvedValue('rendered content'),
-      buildRedirects: vi.fn().mockReturnValue({}),
     }
 
     mockFindPage.mockReturnValue(testPage as any)
@@ -322,31 +272,10 @@ describe('resolveRecommended middleware', () => {
   test('should filter out articles not available in current version', async () => {
     // Create a test page that is only available in fpt, not ghec
     const fptOnlyPage: Partial<import('@/types').Page> = {
-      mtime: Date.now(),
       title: 'FPT Only Article',
-      rawTitle: 'FPT Only Article',
       intro: 'This article is only for FPT',
-      rawIntro: 'This article is only for FPT',
       relativePath: 'test/fpt-only.md',
-      fullPath: '/full/path/test/fpt-only.md',
-      languageCode: 'en',
-      documentType: 'article',
-      markdown: 'FPT only content',
-      versions: { fpt: '*' }, // Only available in free-pro-team
       applicableVersions: ['free-pro-team@latest'], // Not available in ghec
-      permalinks: [
-        {
-          languageCode: 'en',
-          pageVersion: 'free-pro-team@latest',
-          title: 'FPT Only Article',
-          href: '/en/test/fpt-only',
-          hrefWithoutLanguage: '/test/fpt-only',
-        },
-      ],
-      renderProp: vi.fn().mockResolvedValue('rendered'),
-      renderTitle: vi.fn().mockResolvedValue('FPT Only Article'),
-      render: vi.fn().mockResolvedValue('rendered content'),
-      buildRedirects: vi.fn().mockReturnValue({}),
     }
 
     mockFindPage.mockReturnValue(fptOnlyPage as any)