diff --git a/.github/workflows/sync-audit-logs.yml b/.github/workflows/sync-audit-logs.yml index 486265d962..9be9a4a4d5 100644 --- a/.github/workflows/sync-audit-logs.yml +++ b/.github/workflows/sync-audit-logs.yml @@ -51,34 +51,38 @@ jobs: # Needed for gh GITHUB_TOKEN: ${{ secrets.DOCS_BOT_PAT_BASE }} run: | - # If nothing to commit, exit now. It's fine. No orphans. - changes=$(git diff --name-only | wc -l) - untracked=$(git status --untracked-files --short | wc -l) - filesChanged=$(git diff --name-only) - # There will always be at least one file changed: - # src/audit-logs/lib/config.json - # If the config file is the only file changed, exit. - if [[ $changes -eq 1 ]] && [[ $untracked -eq 1 ]] && [[ $filesChanged == *lib/config.json ]]; then - echo "There are no changes to commit or untracked files. Exiting..." - exit 0 - fi - - git config --global user.name "docs-bot" - git config --global user.email "77750099+docs-bot@users.noreply.github.com" - + echo "Creating a new branch if needed..." branchname=audit-logs-schema-update-${{ steps.audit-log-allowlists.outputs.COMMIT_SHA }} - remotesha=$(git ls-remote --heads origin $branchname) if [ -n "$remotesha" ]; then # output is not empty, it means the remote branch exists echo "Branch $branchname already exists in 'github/docs-internal'. Exiting..." exit 0 fi - git checkout -b $branchname - git add . + echo "Created a new branch $branchname" + + echo "Preparing commit..." + git config --global user.name "docs-bot" + git config --global user.email "77750099+docs-bot@users.noreply.github.com" + git add -A . + echo "Prepared commit" + + echo "Check if there are changes..." + if git diff-index --cached --quiet HEAD -- . ':(exclude)src/audit-logs/lib/config.json' + then + echo "No real changes (only the SHA in config.json moved). Exiting…" + exit 0 + fi + echo "Changes detected, proceeding" + + echo "Creating commit..." git commit -m "Add updated audit log event data" + echo "Created commit" + + echo "Pushing commit..." git push origin $branchname + echo "Pushed commit" echo "Creating pull request..." gh pr create \ @@ -89,16 +93,21 @@ jobs: --repo github/docs-internal \ --label audit-log-pipeline \ --head=$branchname + echo "Created pull request" # can't approve your own PR, approve with Actions + echo "Approving pull request..." unset GITHUB_TOKEN gh auth login --with-token <<< "${{ secrets.GITHUB_TOKEN }}" gh pr review --approve + echo "Approved pull request" # Actions can't merge the PR so back to docs-bot to merge the PR + echo "Setting pull request to auto merge..." unset GITHUB_TOKEN gh auth login --with-token <<< "${{ secrets.DOCS_BOT_PAT_BASE }}" gh pr merge --auto + echo "Set pull request to auto merge" - uses: ./.github/actions/slack-alert if: ${{ failure() && github.event_name != 'workflow_dispatch' }} diff --git a/.github/workflows/sync-openapi.yml b/.github/workflows/sync-openapi.yml index ee347de378..480e57662a 100644 --- a/.github/workflows/sync-openapi.yml +++ b/.github/workflows/sync-openapi.yml @@ -49,6 +49,7 @@ jobs: repository: github/models-gateway path: models-gateway ref: main + token: ${{ secrets.DOCS_BOT_PAT_BASE }} - uses: ./.github/actions/node-npm-setup diff --git a/data/reusables/contributing/content-linter-rules.md b/data/reusables/contributing/content-linter-rules.md index bddff1394e..a37ffc994e 100644 --- a/data/reusables/contributing/content-linter-rules.md +++ b/data/reusables/contributing/content-linter-rules.md @@ -65,4 +65,4 @@ | GHD041 | third-party-action-pinning | Code examples that use third-party actions must always pin to a full length commit SHA | error | feature, actions | | GHD042 | liquid-tag-whitespace | Liquid tags should start and end with one whitespace. Liquid tag arguments should be separated by only one whitespace. | error | liquid, format | | GHD043 | link-quotation | Internal link titles must not be surrounded by quotations | error | links, url | -| GHD022 | liquid-ifversion-versions | Liquid `ifversion`, `elsif`, and `else` tags should be valid and not contain unsupported versions. | error | liquid, versioning | \ No newline at end of file +| GHD044 | octicon-aria-labels | Octicons should always have an aria-label attribute even if aria-hidden. | warning | accessibility, octicons | \ No newline at end of file diff --git a/src/content-linter/lib/linting-rules/index.js b/src/content-linter/lib/linting-rules/index.js index c98de5df65..2756723927 100644 --- a/src/content-linter/lib/linting-rules/index.js +++ b/src/content-linter/lib/linting-rules/index.js @@ -33,6 +33,7 @@ import { tableLiquidVersioning } from './table-liquid-versioning.js' import { thirdPartyActionPinning } from './third-party-action-pinning.js' import { liquidTagWhitespace } from './liquid-tag-whitespace.js' import { linkQuotation } from './link-quotation.js' +import { octiconAriaLabels } from './octicon-aria-labels.js' import { liquidIfversionVersions } from './liquid-ifversion-versions.js' const noDefaultAltText = markdownlintGitHub.find((elem) => @@ -82,6 +83,6 @@ export const gitHubDocsMarkdownlint = { thirdPartyActionPinning, liquidTagWhitespace, linkQuotation, - liquidIfversionVersions, + octiconAriaLabels, ], } diff --git a/src/content-linter/lib/linting-rules/octicon-aria-labels.js b/src/content-linter/lib/linting-rules/octicon-aria-labels.js new file mode 100644 index 0000000000..29eeed87c0 --- /dev/null +++ b/src/content-linter/lib/linting-rules/octicon-aria-labels.js @@ -0,0 +1,56 @@ +import { TokenKind } from 'liquidjs' +import { getLiquidTokens, getPositionData } from '../helpers/liquid-utils.js' +import { addFixErrorDetail } from '../helpers/utils.js' +/* +Octicons should always have an aria-label attribute even if aria hidden. For example: + + DO use aria-label + {% octicon "alert" aria-label="alert" %} + {% octicon "alert" aria-label="alert" aria-hidden="true" %} + {% octicon "alert" aria-label="alert" aria-hidden="true" class="foo" %} + + This is necessary for copilot to be able to recognize the svgs correctly when using our API. + +*/ + +export const octiconAriaLabels = { + names: ['GHD044', 'octicon-aria-labels'], + description: 'Octicons should always have an aria-label attribute even if aria-hidden.', + tags: ['accessibility', 'octicons'], + parser: 'markdownit', + function: (params, onError) => { + const content = params.lines.join('\n') + const tokens = getLiquidTokens(content) + .filter((token) => token.kind === TokenKind.Tag) + .filter((token) => token.name === 'octicon') + + for (const token of tokens) { + const { lineNumber, column, length } = getPositionData(token, params.lines) + + const hasAriaLabel = token.args.includes('aria-label=') + + if (!hasAriaLabel) { + const range = [column, length] + + const octiconNameMatch = token.args.match(/["']([^"']+)["']/) + const octiconName = octiconNameMatch ? octiconNameMatch[1] : 'icon' + const originalContent = token.content + const fixedContent = originalContent + ` aria-label="${octiconName}"` + + addFixErrorDetail( + onError, + lineNumber, + `octicon should have an aria-label even if aria hidden. Try using 'aria-label=${octiconName}'`, + token.content, + range, + { + lineNumber, + editColumn: column, + deleteCount: length, + insertText: `{% ${fixedContent} %}`, + }, + ) + } + } + }, +} diff --git a/src/content-linter/style/github-docs.js b/src/content-linter/style/github-docs.js index bc3672a1b7..a240cca239 100644 --- a/src/content-linter/style/github-docs.js +++ b/src/content-linter/style/github-docs.js @@ -168,6 +168,12 @@ const githubDocsConfig = { 'partial-markdown-files': true, 'yml-files': true, }, + 'octicon-aria-labels': { + // GHD044 + severity: 'warning', + 'partial-markdown-files': true, + 'yml-files': true, + }, } export const githubDocsFrontmatterConfig = { diff --git a/src/content-linter/tests/unit/octicon-aria-labels.js b/src/content-linter/tests/unit/octicon-aria-labels.js new file mode 100644 index 0000000000..c677c38ff0 --- /dev/null +++ b/src/content-linter/tests/unit/octicon-aria-labels.js @@ -0,0 +1,160 @@ +import { describe, expect, test } from 'vitest' +import { octiconAriaLabels } from '../../lib/linting-rules/octicon-aria-labels.js' + +describe('octicon-aria-labels', () => { + const rule = octiconAriaLabels + + test('detects octicon without aria-label', () => { + const errors = [] + const onError = (errorInfo) => { + errors.push(errorInfo) + } + + const content = ['This is a test with an octicon:', '{% octicon "alert" %}', 'Some more text.'] + + rule.function({ lines: content }, onError) + + expect(errors.length).toBe(1) + expect(errors[0].lineNumber).toBe(2) + expect(errors[0].detail).toContain('aria-label=alert') + expect(errors[0].fixInfo.insertText).toContain('aria-label="alert"') + }) + + test('ignores octicons with aria-label', () => { + const errors = [] + const onError = (errorInfo) => { + errors.push(errorInfo) + } + + const content = [ + 'This is a test with a proper octicon:', + '{% octicon "alert" aria-label="alert" %}', + 'Some more text.', + ] + + rule.function({ lines: content }, onError) + + expect(errors.length).toBe(0) + }) + + test('detects multiple octicons without aria-label', () => { + const errors = [] + const onError = (errorInfo) => { + errors.push(errorInfo) + } + + const content = [ + 'This is a test with multiple octicons:', + '{% octicon "alert" %}', + 'Some text in between.', + '{% octicon "check" %}', + 'More text.', + ] + + rule.function({ lines: content }, onError) + + expect(errors.length).toBe(2) + expect(errors[0].lineNumber).toBe(2) + expect(errors[0].detail).toContain('aria-label=alert') + expect(errors[1].lineNumber).toBe(4) + expect(errors[1].detail).toContain('aria-label=check') + }) + + test('ignores non-octicon liquid tags', () => { + const errors = [] + const onError = (errorInfo) => { + errors.push(errorInfo) + } + + const content = [ + 'This is a test with non-octicon tags:', + '{% data foo.bar %}', + '{% ifversion fpt %}', + 'Some text.', + '{% endif %}', + ] + + rule.function({ lines: content }, onError) + + expect(errors.length).toBe(0) + }) + + test('suggests correct fix for octicon with other attributes', () => { + const errors = [] + const onError = (errorInfo) => { + errors.push(errorInfo) + } + + const content = [ + 'This is a test with an octicon with other attributes:', + '{% octicon "plus" aria-hidden="true" class="foo" %}', + 'Some more text.', + ] + + rule.function({ lines: content }, onError) + + expect(errors.length).toBe(1) + expect(errors[0].lineNumber).toBe(2) + expect(errors[0].fixInfo.insertText).toContain('aria-label="plus"') + expect(errors[0].fixInfo.insertText).toContain('aria-hidden="true"') + expect(errors[0].fixInfo.insertText).toContain('class="foo"') + }) + + test('handles octicons with unusual spacing', () => { + const errors = [] + const onError = (errorInfo) => { + errors.push(errorInfo) + } + + const content = [ + 'This is a test with unusual spacing:', + '{% octicon "x" %}', + 'Some more text.', + ] + + rule.function({ lines: content }, onError) + + expect(errors.length).toBe(1) + expect(errors[0].lineNumber).toBe(2) + expect(errors[0].detail).toContain('aria-label=x') + }) + + test('handles octicons split across multiple lines', () => { + const errors = [] + const onError = (errorInfo) => { + errors.push(errorInfo) + } + + const content = [ + 'This is a test with a multi-line octicon:', + '{% octicon "chevron-down"', + ' class="dropdown-menu-icon"', + '%}', + 'Some more text.', + ] + + rule.function({ lines: content }, onError) + + expect(errors.length).toBe(1) + expect(errors[0].detail).toContain('aria-label=chevron-down') + }) + + test('falls back to "icon" when octicon name cannot be determined', () => { + const errors = [] + const onError = (errorInfo) => { + errors.push(errorInfo) + } + + const content = [ + 'This is a test with a malformed octicon:', + '{% octicon variable %}', + 'Some more text.', + ] + + rule.function({ lines: content }, onError) + + expect(errors.length).toBe(1) + expect(errors[0].detail).toContain('aria-label=icon') + expect(errors[0].fixInfo.insertText).toContain('aria-label="icon"') + }) +}) diff --git a/src/frame/lib/page.js b/src/frame/lib/page.js index 06ef08ef71..e890cef11d 100644 --- a/src/frame/lib/page.js +++ b/src/frame/lib/page.js @@ -8,11 +8,11 @@ import { getAlertTitles } from '#src/languages/lib/get-alert-titles.ts' import getTocItems from './get-toc-items.js' import Permalink from './permalink.js' import { renderContent } from '#src/content-render/index.js' -import processLearningTracks from '#src/learning-track/lib/process-learning-tracks.js' +import processLearningTracks from '#src/learning-track/lib/process-learning-tracks' import { productMap } from '#src/products/lib/all-products.ts' import slash from 'slash' import readFileContents from './read-file-contents.js' -import getLinkData from '#src/learning-track/lib/get-link-data.js' +import getLinkData from '#src/learning-track/lib/get-link-data' import getDocumentType from '#src/events/lib/get-document-type.ts' import { allTools } from '#src/tools/lib/all-tools.ts' import { renderContentWithFallback } from '#src/languages/lib/render-with-fallback.js' diff --git a/src/landings/middleware/featured-links.ts b/src/landings/middleware/featured-links.ts index 70edb636ba..eed8891868 100644 --- a/src/landings/middleware/featured-links.ts +++ b/src/landings/middleware/featured-links.ts @@ -1,8 +1,8 @@ import type { Response, NextFunction } from 'express' import type { ExtendedRequest, FeaturedLinkExpanded } from '@/types' -import getLinkData from '@/learning-track/lib/get-link-data.js' -import { renderContent } from '@/content-render/index.js' +import getLinkData from '@/learning-track/lib/get-link-data' +import { renderContent } from '@/content-render/index' /** * This is the max. number of featured links, by any category, that we @@ -73,12 +73,20 @@ export default async function featuredLinks( if (!(key in req.context.page.featuredLinks)) throw new Error('featureLinks key not found in Page') const pageFeaturedLink = req.context.page.featuredLinks[key] - req.context.featuredLinks[key] = (await getLinkData( - pageFeaturedLink, + // Handle different types of featuredLinks by converting to string array + const stringLinks = Array.isArray(pageFeaturedLink) + ? pageFeaturedLink.map((item) => (typeof item === 'string' ? item : item.href)) + : [] + + const linkData = await getLinkData( + stringLinks, req.context, { title: true, intro: true, fullTitle: true }, MAX_FEATURED_LINKS, - )) as FeaturedLinkExpanded[] // Remove ones `getLinkData` is TS + ) + // We need to use a type assertion here because the Page interfaces are incompatible + // between our local types and the global types, but the actual runtime objects are compatible + req.context.featuredLinks[key] = (linkData || []) as unknown as FeaturedLinkExpanded[] } } diff --git a/src/learning-track/lib/get-link-data.js b/src/learning-track/lib/get-link-data.ts similarity index 52% rename from src/learning-track/lib/get-link-data.js rename to src/learning-track/lib/get-link-data.ts index 5705f94b65..66856641bc 100644 --- a/src/learning-track/lib/get-link-data.js +++ b/src/learning-track/lib/get-link-data.ts @@ -1,32 +1,34 @@ import path from 'path' -import findPage from '#src/frame/lib/find-page.js' -import nonEnterpriseDefaultVersion from '#src/versions/lib/non-enterprise-default-version.js' -import removeFPTFromPath from '#src/versions/lib/remove-fpt-from-path.js' -import { renderContent } from '#src/content-render/index.js' -import { executeWithFallback } from '#src/languages/lib/render-with-fallback.js' +import findPage from '@/frame/lib/find-page' +import nonEnterpriseDefaultVersion from '@/versions/lib/non-enterprise-default-version' +import removeFPTFromPath from '@/versions/lib/remove-fpt-from-path' +import { renderContent } from '@/content-render/index' +import { executeWithFallback } from '@/languages/lib/render-with-fallback' +import { Context, LinkOptions, ProcessedLink } from './types' // rawLinks is an array of paths: [ '/foo' ] // we need to convert it to an array of localized objects: [ { href: '/en/foo', title: 'Foo', intro: 'Description here' } ] -export default async ( - rawLinks, - context, - option = { title: true, intro: true, fullTitle: false }, +export default async function getLinkData( + rawLinks: string[] | string | undefined, + context: Context, + options: LinkOptions = { title: true, intro: true, fullTitle: false }, maxLinks = Infinity, -) => { - if (!rawLinks) return +): Promise { + if (!rawLinks) return undefined if (typeof rawLinks === 'string') { - return await processLink(rawLinks, context, option) + const processedLink = await processLink(rawLinks, context, options) + return processedLink ? [processedLink] : undefined } - const links = [] + const links: ProcessedLink[] = [] // Using a for loop here because the async work is not network or // disk bound. It's CPU bound. // And if we use a for-loop we can potentially bail early if // the `maxLinks` is reached. That's instead of computing them all, // and then slicing the array. So it avoids wasted processing. for (const link of rawLinks) { - const processedLink = await processLink(link, context, option) + const processedLink = await processLink(link, context, options) if (processedLink) { links.push(processedLink) if (links.length >= maxLinks) { @@ -38,9 +40,13 @@ export default async ( return links } -async function processLink(link, context, option) { - const opts = { textOnly: true } - const linkHref = link.href || link +async function processLink( + link: string | { href: string }, + context: Context, + options: LinkOptions, +): Promise { + const opts: { textOnly: boolean; preferShort?: boolean } = { textOnly: true } + const linkHref = typeof link === 'string' ? link : link.href // Parse the link in case it includes Liquid conditionals const linkPath = linkHref.includes('{') ? await executeWithFallback( @@ -55,10 +61,13 @@ async function processLink(link, context, option) { if (!linkPath) return null const version = - context.currentVersion === 'homepage' ? nonEnterpriseDefaultVersion : context.currentVersion - const href = removeFPTFromPath(path.join('/', context.currentLanguage, version, linkPath)) + (context.currentVersion === 'homepage' + ? nonEnterpriseDefaultVersion + : context.currentVersion) || 'free-pro-team@latest' + const currentLanguage = context.currentLanguage || 'en' + const href = removeFPTFromPath(path.join('/', currentLanguage, version, linkPath)) - const linkedPage = findPage(href, context.pages, context.redirects) + const linkedPage = findPage(href, context.pages || {}, context.redirects || {}) if (!linkedPage) { // This can happen when the link depends on Liquid conditionals, // like... @@ -66,18 +75,18 @@ async function processLink(link, context, option) { return null } - const result = { href, page: linkedPage } + const result: ProcessedLink = { href, page: linkedPage } - if (option.title) { + if (options.title) { result.title = await linkedPage.renderTitle(context, opts) } - if (option.fullTitle) { + if (options.fullTitle) { opts.preferShort = false result.fullTitle = await linkedPage.renderTitle(context, opts) } - if (option.intro) { + if (options.intro) { result.intro = await linkedPage.renderProp('intro', context, opts) } return result diff --git a/src/learning-track/lib/process-learning-tracks.js b/src/learning-track/lib/process-learning-tracks.ts similarity index 78% rename from src/learning-track/lib/process-learning-tracks.js rename to src/learning-track/lib/process-learning-tracks.ts index fa38c529ee..97a5fee582 100644 --- a/src/learning-track/lib/process-learning-tracks.js +++ b/src/learning-track/lib/process-learning-tracks.ts @@ -1,15 +1,19 @@ -import getLinkData from './get-link-data.js' -import getApplicableVersions from '#src/versions/lib/get-applicable-versions.js' -import { getDataByLanguage } from '#src/data-directory/lib/get-data.js' -import { renderContent } from '#src/content-render/index.js' -import { executeWithFallback } from '#src/languages/lib/render-with-fallback.js' +import getLinkData from './get-link-data' +import getApplicableVersions from '@/versions/lib/get-applicable-versions' +import { getDataByLanguage } from '@/data-directory/lib/get-data' +import { renderContent } from '@/content-render/index' +import { executeWithFallback } from '@/languages/lib/render-with-fallback' +import { Context, TrackGuide, LearningTrack, ProcessedLearningTracks } from './types' const renderOpts = { textOnly: true } // This module returns an object that contains a single featured learning track // and an array of all the other learning tracks for the current version. -export default async function processLearningTracks(rawLearningTracks, context) { - const learningTracks = [] +export default async function processLearningTracks( + rawLearningTracks: string[], + context: Context, +): Promise { + const learningTracks: LearningTrack[] = [] if (!context.currentProduct) { throw new Error(`Missing context.currentProduct value.`) @@ -59,7 +63,7 @@ export default async function processLearningTracks(rawLearningTracks, context) // we need to have the English `title` and `description` to // fall back to. // - let enTrack + let enTrack: any if (context.currentLanguage !== 'en') { enTrack = getDataByLanguage( `learning-tracks.${context.currentProduct}.${renderedTrackName}`, @@ -86,26 +90,28 @@ export default async function processLearningTracks(rawLearningTracks, context) const title = await executeWithFallback( context, () => renderContent(track.title, context, renderOpts), - (enContext) => renderContent(enTrack.title, enContext, renderOpts), + (enContext: any) => renderContent(enTrack.title, enContext, renderOpts), ) const description = await executeWithFallback( context, () => renderContent(track.description, context, renderOpts), - (enContext) => renderContent(enTrack.description, enContext, renderOpts), + (enContext: any) => renderContent(enTrack.description, enContext, renderOpts), ) - const learningTrack = { + const guides = (await getLinkData(track.guides, context)) || [] + + const learningTrack: LearningTrack = { trackName: renderedTrackName, trackProduct: context.currentProduct || null, title, description, // getLinkData respects versioning and only returns guides available in the current version; // if no guides are available, the learningTrack.guides property will be an empty array. - guides: await getLinkData(track.guides, context), + guides: guides as TrackGuide[], } // Only add the track to the array of tracks if there are guides in this version and it's not the featured track. - if (learningTrack.guides.length) { + if (Array.isArray(learningTrack.guides) && learningTrack.guides.length > 0) { learningTracks.push(learningTrack) } } diff --git a/src/learning-track/lib/types.ts b/src/learning-track/lib/types.ts new file mode 100644 index 0000000000..2547d6db81 --- /dev/null +++ b/src/learning-track/lib/types.ts @@ -0,0 +1,122 @@ +/** + * Common types used across learning track components + */ + +/** + * Basic context interface for rendering operations + */ +export interface Context { + currentProduct?: string + currentLanguage?: string + currentVersion?: string + pages?: any + redirects?: any + // Additional properties that may be needed for rendering + [key: string]: any +} + +/** + * Options for retrieving link data + */ +export interface LinkOptions { + title?: boolean + intro?: boolean + fullTitle?: boolean +} + +/** + * Result of processing a link + */ +export interface ProcessedLink { + href: string + page: Page + title?: string + fullTitle?: string + intro?: string +} + +/** + * Definitions for featured links data + */ +export interface FeaturedLink { + title: string + href: string +} + +export interface PageFeaturedLinks { + [key: string]: string[] | FeaturedLink[] +} + +/** + * Page interface for basic page properties + */ +export interface Page { + renderTitle: (context: Context, opts: any) => Promise + renderProp: (prop: string, context: Context, opts: any) => Promise +} + +/** + * Guide in a learning track + */ +export interface TrackGuide { + href: string + page: Page + title: string + intro?: string +} + +/** + * A processed learning track + */ +export interface LearningTrack { + trackName: string + trackProduct: string | null + title: string + description: string + guides: TrackGuide[] +} + +/** + * Learning track metadata with guides + */ +export interface LearningTrackMetadata { + title: string + description: string + guides: string[] + versions?: any +} + +/** + * Collection of learning tracks by product and track name + */ +export interface LearningTracks { + [productId: string]: { + [trackName: string]: LearningTrackMetadata + } +} + +/** + * Return type for processLearningTracks function + */ +export interface ProcessedLearningTracks { + learningTracks: LearningTrack[] +} + +/** + * Learning track data for the current guide + */ +export interface CurrentLearningTrack { + trackName: string + trackProduct: string + trackTitle: string + numberOfGuides?: number + currentGuideIndex?: number + nextGuide?: { + href: string + title: string | undefined + } + prevGuide?: { + href: string + title: string | undefined + } +} diff --git a/src/learning-track/middleware/learning-track.ts b/src/learning-track/middleware/learning-track.ts index 35585f2189..6eb260427f 100644 --- a/src/learning-track/middleware/learning-track.ts +++ b/src/learning-track/middleware/learning-track.ts @@ -1,28 +1,22 @@ import type { Response, NextFunction } from 'express' -import type { - Context, - ExtendedRequest, - LearningTrack, - LearningTracks, - TrackGuide, - Page, -} from '@/types' +import type { ExtendedRequest, LearningTracks } from '@/types' +import type { Context, CurrentLearningTrack, TrackGuide } from '../lib/types' import { getPathWithoutLanguage, getPathWithoutVersion } from '@/frame/lib/path-utils.js' -import getLinkData from '../lib/get-link-data.js' +import getLinkData from '../lib/get-link-data' import { renderContent } from '@/content-render/index.js' import { executeWithFallback } from '@/languages/lib/render-with-fallback.js' import { getDeepDataByLanguage } from '@/data-directory/lib/get-data.js' export default async function learningTrack( - req: ExtendedRequest, + req: ExtendedRequest & { context: Context }, res: Response, next: NextFunction, ) { if (!req.context) throw new Error('request is not contextualized') const noTrack = () => { - req.context!.currentLearningTrack = null + req.context.currentLearningTrack = null return next() } @@ -94,12 +88,12 @@ export default async function learningTrack( () => '', // todo use english track.title ) - const currentLearningTrack: LearningTrack = { trackName, trackProduct, trackTitle } + const currentLearningTrack: CurrentLearningTrack = { trackName, trackProduct, trackTitle } const guidePath = getPathWithoutLanguage(getPathWithoutVersion(req.pagePath)) // The raw track.guides will return all guide paths, need to use getLinkData // so we only get guides available in the current version - const trackGuides = (await getLinkData(track.guides, req.context)) as TrackGuide[] + const trackGuides = ((await getLinkData(track.guides, req.context)) || []) as TrackGuide[] const trackGuidePaths = trackGuides.map((guide) => { return getPathWithoutLanguage(getPathWithoutVersion(guide.href)) @@ -137,8 +131,8 @@ export default async function learningTrack( intro: false, fullTitle: false, }) - if (!resultData) return noTrack() - const result = resultData as { href: string; page: Page; title: string } + if (!resultData || !resultData.length) return noTrack() + const result = resultData[0] const href = result.href const title = result.title @@ -152,8 +146,8 @@ export default async function learningTrack( intro: false, fullTitle: false, }) - if (!resultData) return noTrack() - const result = resultData as { href: string; page: Page; title: string } + if (!resultData || !resultData.length) return noTrack() + const result = resultData[0] const href = result.href const title = result.title diff --git a/src/search/components/input/AskAIResults.tsx b/src/search/components/input/AskAIResults.tsx index 7017cebfe5..86d1e9cd46 100644 --- a/src/search/components/input/AskAIResults.tsx +++ b/src/search/components/input/AskAIResults.tsx @@ -232,18 +232,75 @@ export function AskAIResults({ const decoder = new TextDecoder('utf-8') const reader = response.body.getReader() let done = false + let leftover = '' // <= carry‑over buffer setInitialLoading(false) + + const processLine = (parsedLine: any) => { + switch (parsedLine.chunkType) { + // A conversation ID will still be sent when a question cannot be answered + case 'CONVERSATION_ID': + conversationIdBuffer = parsedLine.conversation_id + setConversationId(parsedLine.conversation_id) + break + + case 'NO_CONTENT_SIGNAL': + // Serve canned response. A question that cannot be answered was asked + handleAICannotAnswer(conversationIdBuffer, 200) + break + + case 'SOURCES': + if (!isCancelled) { + sourcesBuffer = uniqBy( + sourcesBuffer.concat(parsedLine.sources as AIReference[]), + 'url', + ) + setReferences(sourcesBuffer) + } + break + + case 'MESSAGE_CHUNK': + if (!isCancelled) { + messageBuffer += parsedLine.text + setMessage(messageBuffer) + } + break + + case 'INPUT_CONTENT_FILTER': + // Serve canned response. A spam question was asked + handleAICannotAnswer( + conversationIdBuffer, + 200, + t('search.ai.responses.invalid_query'), + ) + break + } + + if (!isCancelled) setAnnouncement('Copilot Response Loading...') + } + while (!done && !isCancelled) { const { value, done: readerDone } = await reader.read() done = readerDone + + // The sources JSON chunk may be sent in multiple parts, so we need to decode it with a leftover buffer so that it can be parsed all at once + // So when we say "incomplete" or "leftover" we mean that the JSON is not complete yet, not that the message is incomplete if (value) { - const chunkStr = decoder.decode(value, { stream: true }) - const chunkLines = chunkStr.split('\n').filter((line) => line.trim() !== '') - for (const line of chunkLines) { - let parsedLine + // 1 append this chunk's text to whatever was left over + leftover += decoder.decode(value, { stream: true }) + + // 2 split on newline + const lines = leftover.split('\n') + + // 3 keep the *last* item (maybe incomplete) for next round + leftover = lines.pop() ?? '' + + // 4 parse all complete lines + for (const raw of lines) { + if (!raw.trim()) continue + + let parsedLine: any try { - parsedLine = JSON.parse(line) - // If midstream there is an error, like a connection reset / lost, our backend will send an error JSON + parsedLine = JSON.parse(raw) if (parsedLine?.errors) { sendAISearchResultEvent({ sources: [], @@ -255,50 +312,25 @@ export function AskAIResults({ setAISearchError() return } - } catch (e) { - console.warn( - 'Failed to parse JSON:', - e, - 'Line:', - line, - 'Typeof line: ', - typeof line, - ) + } catch (err) { + console.warn('Failed to parse JSON line:', raw, err) continue } - // A conversation ID will still be sent when a question cannot be answered - if (parsedLine.chunkType === 'CONVERSATION_ID') { - conversationIdBuffer = parsedLine.conversation_id - setConversationId(parsedLine.conversation_id) - } else if (parsedLine.chunkType === 'NO_CONTENT_SIGNAL') { - // Serve canned response. A question that cannot be answered was asked - handleAICannotAnswer(conversationIdBuffer, 200) - } else if (parsedLine.chunkType === 'SOURCES') { - if (!isCancelled) { - sourcesBuffer = sourcesBuffer.concat(parsedLine.sources) - sourcesBuffer = uniqBy(sourcesBuffer, 'url') - setReferences(sourcesBuffer) - } - } else if (parsedLine.chunkType === 'MESSAGE_CHUNK') { - if (!isCancelled) { - messageBuffer += parsedLine.text - setMessage(messageBuffer) - } - } else if (parsedLine.chunkType === 'INPUT_CONTENT_FILTER') { - // Serve canned response. A spam question was asked - handleAICannotAnswer( - conversationIdBuffer, - 200, - t('search.ai.responses.invalid_query'), - ) - } - if (!isCancelled) { - setAnnouncement('Copilot Response Loading...') - } + processLine(parsedLine) } } } + + // 5 flush whatever remains after the stream ends + if (!isCancelled && leftover.trim()) { + try { + const tail = JSON.parse(leftover) + processLine(tail) + } catch (err) { + console.warn('Failed to parse tail JSON:', leftover, err) + } + } } catch (error: any) { if (!isCancelled) { console.error('Failed to fetch search results:', error) diff --git a/src/search/components/input/SearchOverlay.tsx b/src/search/components/input/SearchOverlay.tsx index 958f6a002f..939776917b 100644 --- a/src/search/components/input/SearchOverlay.tsx +++ b/src/search/components/input/SearchOverlay.tsx @@ -482,6 +482,9 @@ export function SearchOverlay({ } } } else if (event.key === 'Enter') { + if (searchLoading) { + return + } event.preventDefault() let pressedGroupKey = SEARCH_OVERLAY_EVENT_GROUP let pressedGroupId = searchEventGroupId @@ -494,9 +497,7 @@ export function SearchOverlay({ pressedGroupId = askAIEventGroupId sendKeyboardEvent(event.key, pressedOnContext, pressedGroupId, pressedGroupKey) aiSearchOptionOnSelect({ term: urlSearchInputQuery } as AutocompleteSearchHit) - } - - if ( + } else if ( combinedOptions.length > 0 && selectedIndex >= 0 && selectedIndex < combinedOptions.length diff --git a/src/types.ts b/src/types.ts index 033371af77..2c7b79cb14 100644 --- a/src/types.ts +++ b/src/types.ts @@ -185,11 +185,11 @@ export type LearningTrack = { currentGuideIndex?: number nextGuide?: { href: string - title: string + title: string | undefined } prevGuide?: { href: string - title: string + title: string | undefined } }