Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
@@ -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).
74 changes: 73 additions & 1 deletion src/content-linter/tests/unit/frontmatter-landing-recommended.js
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -10,13 +10,28 @@ 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]

// Configure the test fixture to not split frontmatter and content
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],
Expand Down Expand Up @@ -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 changes: 8 additions & 0 deletions src/fixtures/fixtures/article-one.md
Original file line number Diff line number Diff line change
@@ -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 changes: 8 additions & 0 deletions src/fixtures/fixtures/content/article-one.md
Original file line number Diff line number Diff line change
@@ -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 changes: 7 additions & 0 deletions src/fixtures/fixtures/content/article-two.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
title: Article Two
---

# Article Two

This is the second test article.
7 changes: 7 additions & 0 deletions src/fixtures/fixtures/content/subdir/article-three.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
title: Article Three
---

# Article Three

This is the third test article in a subdirectory.
11 changes: 11 additions & 0 deletions src/fixtures/fixtures/test-discovery-landing.md
Original file line number Diff line number Diff line change
@@ -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.
3 changes: 3 additions & 0 deletions src/frame/lib/page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions src/frame/middleware/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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) {
Expand Down
Loading
Loading