From e639208eb1b7c18862439c8642f6a8fdf7af5d5d Mon Sep 17 00:00:00 2001 From: Joe Fusco Date: Wed, 18 Sep 2024 12:00:03 -0400 Subject: [PATCH 1/4] Use sitemap index path for all sitemaps --- .../src/server/sitemaps/createSitemaps.ts | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/packages/faustwp-core/src/server/sitemaps/createSitemaps.ts b/packages/faustwp-core/src/server/sitemaps/createSitemaps.ts index 2fd92187b..8553fb57a 100644 --- a/packages/faustwp-core/src/server/sitemaps/createSitemaps.ts +++ b/packages/faustwp-core/src/server/sitemaps/createSitemaps.ts @@ -62,8 +62,7 @@ export async function createRootSitemapIndex( throw new Error('Request object must have URL'); } - // get sitemapIndexPath config param - // fetch sitemap from WP + // Get the trimmed sitemap index path const trimmedWpUrl = trim(getWpUrl(), '/'); const trimmedFrontendUrl = trim(frontendUrl, '/'); const trimmedSitemapIndexPath = trim( @@ -75,10 +74,10 @@ export async function createRootSitemapIndex( let sitemaps: SitemapSchemaSitemapElement[] = []; if (!isUndefined(pages) && isArray(pages) && pages.length) { - const trimmedFaustPagesPart = `${trim( - SITEMAP_INDEX_PATH, + const trimmedFaustPagesPart = `${trimmedSitemapIndexPath}?sitemap=${trim( + FAUST_PAGES_PATHNAME, '/', - )}?sitemap=${trim(FAUST_PAGES_PATHNAME, '/')}`; + )}`; const sitemapFaustPagesUrl = `${trimmedFrontendUrl}/${trimmedFaustPagesPart}`; sitemaps = [ @@ -170,11 +169,11 @@ export async function createRootSitemapIndex( * * @example * Replaces http://headless.local/wp-sitemap-posts-page-1.xml with - * http://localhost:3000/wp-sitemap-posts-page-1.xml + * http://localhost:3000/sitemap_index.xml?sitemap=wp-sitemap-posts-page-1.xml */ wpSitemaps.forEach((sitemap) => { const url = new URL(sitemap.loc); - const sitemapUrl = `${trim(frontendUrl, '/')}/sitemap.xml?sitemap=${trim( + const sitemapUrl = `${trimmedFrontendUrl}/${trimmedSitemapIndexPath}?sitemap=${trim( url.pathname, '/', )}`; @@ -293,11 +292,11 @@ export async function handleSitemapPath( let urls: SitemapSchemaUrlElement[] = []; /** - * Replace the existing WordPress URL in each "loc" with the headless URL + * Replace the existing WordPress URL in each "loc" with the frontend URL * * @example - * Replaces http://headless.local/wp-sitemap-posts-page-1.xml with - * http://localhost:3000/wp-sitemap-posts-page-1.xml + * Replaces http://headless.local/page-1/ with + * http://localhost:3000/page-1/ */ wpSitemapUrls.forEach((url) => { urls = [ From f5cd07d1fc3e10953caa11141a8601c11bfabae4 Mon Sep 17 00:00:00 2001 From: Joe Fusco Date: Wed, 18 Sep 2024 14:34:09 -0400 Subject: [PATCH 2/4] Add test coverage for sitemap config --- .../server/sitemaps/createSitemaps.test.ts | 187 ++++++++++++++---- .../server/sitemaps/getSitemapProps.test.ts | 31 +++ 2 files changed, 179 insertions(+), 39 deletions(-) diff --git a/packages/faustwp-core/tests/server/sitemaps/createSitemaps.test.ts b/packages/faustwp-core/tests/server/sitemaps/createSitemaps.test.ts index 1827cff8e..d2fec7115 100644 --- a/packages/faustwp-core/tests/server/sitemaps/createSitemaps.test.ts +++ b/packages/faustwp-core/tests/server/sitemaps/createSitemaps.test.ts @@ -15,18 +15,17 @@ describe('createRootSitemapIndex', () => { process.env = envBackup; }); - const invalidSitemapIndexXML = ` + const invalidSitemapIndexXML = ` `; - const validSitemapIndex1RecordXML = ` + const validSitemapIndex1RecordXML = ` http://headless.local/post-sitemap.xml - - `; + `; - const validSitemapIndexXML = ` + const validSitemapIndexXML = ` http://headless.local/post-sitemap.xml @@ -41,8 +40,7 @@ describe('createRootSitemapIndex', () => { http://headless.local/author-sitemap.xml 2021-12-17T16:56:55+00:00 - - `; + `; it('returns undefined when the response to the WP sitemap is not ok', async () => { jest.spyOn(global, 'fetch').mockImplementationOnce(() => { @@ -86,10 +84,7 @@ describe('createRootSitemapIndex', () => { }); it('ignores sitemaps that match the sitemapPathsToIgnore property', async () => { - const createSitemapIndexSpy = jest.spyOn( - sitemapUtils, - 'createSitemapIndex', - ); + const createSitemapIndexSpy = jest.spyOn(sitemapUtils, 'createSitemapIndex'); jest.spyOn(global, 'fetch').mockImplementationOnce(() => { return Promise.resolve({ ok: true, @@ -123,10 +118,7 @@ describe('createRootSitemapIndex', () => { }); it('ignores sitemaps that match a sitemapPathsToIgnore wildcard', async () => { - const createSitemapIndexSpy = jest.spyOn( - sitemapUtils, - 'createSitemapIndex', - ); + const createSitemapIndexSpy = jest.spyOn(sitemapUtils, 'createSitemapIndex'); jest.spyOn(global, 'fetch').mockImplementationOnce(() => { return Promise.resolve({ ok: true, @@ -160,10 +152,7 @@ describe('createRootSitemapIndex', () => { }); it('returns the proper sitemap urls with replacing URLs', async () => { - const createSitemapIndexSpy = jest.spyOn( - sitemapUtils, - 'createSitemapIndex', - ); + const createSitemapIndexSpy = jest.spyOn(sitemapUtils, 'createSitemapIndex'); jest.spyOn(global, 'fetch').mockImplementationOnce(() => { return Promise.resolve({ ok: true, @@ -211,10 +200,7 @@ describe('createRootSitemapIndex', () => { * configured properly. Ensure it returns an array of URLs */ it('returns the proper sitemap urls with only 1 record', async () => { - const createSitemapIndexSpy = jest.spyOn( - sitemapUtils, - 'createSitemapIndex', - ); + const createSitemapIndexSpy = jest.spyOn(sitemapUtils, 'createSitemapIndex'); jest.spyOn(global, 'fetch').mockImplementationOnce(() => { return Promise.resolve({ ok: true, @@ -242,6 +228,132 @@ describe('createRootSitemapIndex', () => { expect(createSitemapIndexSpy).toHaveBeenCalledWith(expectedSitemaps); }); + + // New tests for sitemapIndexPath feature + + it('uses the default sitemapIndexPath when none is provided', async () => { + const createSitemapIndexSpy = jest.spyOn(sitemapUtils, 'createSitemapIndex'); + jest.spyOn(global, 'fetch').mockImplementationOnce((url) => { + expect(url).toBe('http://headless.local/sitemap.xml'); + return Promise.resolve({ + ok: true, + status: 200, + text: () => Promise.resolve(validSitemapIndexXML), + }) as Promise; + }); + + const req = { + url: 'http://localhost:3000/sitemap.xml', + } as NextRequest; + + const config: GetSitemapPropsConfig = { + frontendUrl: 'http://localhost:3000', + }; + + await createSitemaps.createRootSitemapIndex(req, config); + + const expectedSitemaps = [ + { + loc: 'http://localhost:3000/sitemap.xml?sitemap=post-sitemap.xml', + }, + { + loc: 'http://localhost:3000/sitemap.xml?sitemap=page-sitemap.xml', + }, + { + loc: 'http://localhost:3000/sitemap.xml?sitemap=category-sitemap.xml', + }, + { + loc: 'http://localhost:3000/sitemap.xml?sitemap=author-sitemap.xml', + lastmod: '2021-12-17T16:56:55+00:00', + }, + ]; + + expect(createSitemapIndexSpy).toHaveBeenCalledWith(expectedSitemaps); + }); + + it('uses the custom sitemapIndexPath when provided', async () => { + jest.spyOn(global, 'fetch').mockImplementationOnce((url) => { + expect(url).toBe('http://headless.local/custom-sitemap-index.xml'); + return Promise.resolve({ + ok: true, + status: 200, + text: () => Promise.resolve(validSitemapIndexXML), + }) as Promise; + }); + + const req = { + url: 'http://localhost:3000/custom-sitemap-index.xml', + } as NextRequest; + + const config: GetSitemapPropsConfig = { + frontendUrl: 'http://localhost:3000', + sitemapIndexPath: '/custom-sitemap-index.xml', + }; + + const createSitemapIndexSpy = jest.spyOn(sitemapUtils, 'createSitemapIndex'); + + await createSitemaps.createRootSitemapIndex(req, config); + + const expectedSitemaps = [ + { + loc: 'http://localhost:3000/custom-sitemap-index.xml?sitemap=post-sitemap.xml', + }, + { + loc: 'http://localhost:3000/custom-sitemap-index.xml?sitemap=page-sitemap.xml', + }, + { + loc: 'http://localhost:3000/custom-sitemap-index.xml?sitemap=category-sitemap.xml', + }, + { + loc: 'http://localhost:3000/custom-sitemap-index.xml?sitemap=author-sitemap.xml', + lastmod: '2021-12-17T16:56:55+00:00', + }, + ]; + + expect(createSitemapIndexSpy).toHaveBeenCalledWith(expectedSitemaps); + }); + + it('constructs correct child sitemap URLs with custom sitemapIndexPath', async () => { + jest.spyOn(global, 'fetch').mockImplementationOnce((url) => { + expect(url).toBe('http://headless.local/sitemap_index.xml'); + return Promise.resolve({ + ok: true, + status: 200, + text: () => Promise.resolve(validSitemapIndexXML), + }) as Promise; + }); + + const req = { + url: 'http://localhost:3000/sitemap_index.xml', + } as NextRequest; + + const config: GetSitemapPropsConfig = { + frontendUrl: 'http://localhost:3000', + sitemapIndexPath: '/sitemap_index.xml', + }; + + const createSitemapIndexSpy = jest.spyOn(sitemapUtils, 'createSitemapIndex'); + + await createSitemaps.createRootSitemapIndex(req, config); + + const expectedSitemaps = [ + { + loc: 'http://localhost:3000/sitemap_index.xml?sitemap=post-sitemap.xml', + }, + { + loc: 'http://localhost:3000/sitemap_index.xml?sitemap=page-sitemap.xml', + }, + { + loc: 'http://localhost:3000/sitemap_index.xml?sitemap=category-sitemap.xml', + }, + { + loc: 'http://localhost:3000/sitemap_index.xml?sitemap=author-sitemap.xml', + lastmod: '2021-12-17T16:56:55+00:00', + }, + ]; + + expect(createSitemapIndexSpy).toHaveBeenCalledWith(expectedSitemaps); + }); }); describe('createPagesSitemap()', () => { @@ -259,7 +371,7 @@ describe('createPagesSitemap()', () => { frontendUrl: 'http://localhost:3000', }; - let req = { + const req = { url: 'http://localhost:3000/sitemap-faust-pages.xml', } as NextRequest; @@ -310,7 +422,7 @@ describe('createPagesSitemap()', () => { const createSitemapSpy = jest.spyOn(sitemapUtils, 'createSitemap'); - let req = { + const req = { url: 'http://localhost:3000/sitemap-faust-pages.xml', } as NextRequest; @@ -330,8 +442,8 @@ describe('handleSitemapPath()', () => { afterAll(() => { process.env = envBackup; }); - const validSitemapXML = ` - + const validSitemapXML = ` + http://headless.local/ @@ -347,19 +459,16 @@ describe('handleSitemapPath()', () => { weekly 0.5 + `; - - `; - - const validSitemap1RecordXML = ` - + const validSitemap1RecordXML = ` + http://headless.local/about - - `; + `; - const invalidSitemapXML = ` + const invalidSitemapXML = ` `; it('returns undefined when the response to the WP sitemap is not ok', async () => { @@ -371,7 +480,7 @@ describe('handleSitemapPath()', () => { }); const req = { - url: 'http://localhost:3000/sitemap-posts.xml', + url: 'http://localhost:3000/sitemap.xml?sitemap=post-sitemap.xml', } as NextRequest; const config: GetSitemapPropsConfig = { @@ -392,7 +501,7 @@ describe('handleSitemapPath()', () => { }); const req = { - url: 'http://localhost:3000/sitemap-posts.xml', + url: 'http://localhost:3000/sitemap.xml?sitemap=post-sitemap.xml', } as NextRequest; const config: GetSitemapPropsConfig = { @@ -415,7 +524,7 @@ describe('handleSitemapPath()', () => { }); const req = { - url: 'http://localhost:5000/sitemap-pages.xml', + url: 'http://localhost:5000/sitemap.xml?sitemap=page-sitemap.xml', } as NextRequest; const config: GetSitemapPropsConfig = { @@ -461,7 +570,7 @@ describe('handleSitemapPath()', () => { }); const req = { - url: 'http://localhost:3000/sitemap-pages.xml', + url: 'http://localhost:3000/sitemap.xml?sitemap=page-sitemap.xml', } as NextRequest; const config: GetSitemapPropsConfig = { diff --git a/packages/faustwp-core/tests/server/sitemaps/getSitemapProps.test.ts b/packages/faustwp-core/tests/server/sitemaps/getSitemapProps.test.ts index 0f148e13e..7ee6a1d29 100644 --- a/packages/faustwp-core/tests/server/sitemaps/getSitemapProps.test.ts +++ b/packages/faustwp-core/tests/server/sitemaps/getSitemapProps.test.ts @@ -105,4 +105,35 @@ describe('validateConfig', () => { 'The pages path property must be a string', ); }); + + it('throws an error if sitemapIndexPath is not a string', () => { + const config: any = { + frontendUrl: 'http://localhost:3000', + sitemapIndexPath: 123, + }; + + expect(() => getSitemapProps.validateConfig(config)).toThrow( + 'sitemapIndexPath must be a string', + ); + }); + + it('throws an error if sitemapIndexPath does not start with a forward slash', () => { + const config: any = { + frontendUrl: 'http://localhost:3000', + sitemapIndexPath: 'sitemap_index.xml', + }; + + expect(() => getSitemapProps.validateConfig(config)).toThrow( + 'sitemapIndexPath must start with a forward slash', + ); + }); + + it('passes validation when sitemapIndexPath is a valid string', () => { + const config: any = { + frontendUrl: 'http://localhost:3000', + sitemapIndexPath: '/sitemap_index.xml', + }; + + expect(() => getSitemapProps.validateConfig(config)).not.toThrow(); + }); }); From 82d5a724ae7139711fcf3542875714235a655c38 Mon Sep 17 00:00:00 2001 From: Joe Fusco Date: Wed, 18 Sep 2024 14:40:31 -0400 Subject: [PATCH 3/4] Add changeset --- .changeset/tricky-windows-destroy.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/tricky-windows-destroy.md diff --git a/.changeset/tricky-windows-destroy.md b/.changeset/tricky-windows-destroy.md new file mode 100644 index 000000000..a0870ecbb --- /dev/null +++ b/.changeset/tricky-windows-destroy.md @@ -0,0 +1,5 @@ +--- +'@faustwp/core': patch +--- + +Fix incorrect sitemap URLs when using sitemapIndexPath From 59d7f237637726d670c0cf7001d32df8b8355827 Mon Sep 17 00:00:00 2001 From: Joe Fusco Date: Wed, 18 Sep 2024 14:58:36 -0400 Subject: [PATCH 4/4] Clean up comments --- .../faustwp-core/tests/server/sitemaps/createSitemaps.test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/faustwp-core/tests/server/sitemaps/createSitemaps.test.ts b/packages/faustwp-core/tests/server/sitemaps/createSitemaps.test.ts index d2fec7115..70206f2bd 100644 --- a/packages/faustwp-core/tests/server/sitemaps/createSitemaps.test.ts +++ b/packages/faustwp-core/tests/server/sitemaps/createSitemaps.test.ts @@ -229,8 +229,6 @@ describe('createRootSitemapIndex', () => { expect(createSitemapIndexSpy).toHaveBeenCalledWith(expectedSitemaps); }); - // New tests for sitemapIndexPath feature - it('uses the default sitemapIndexPath when none is provided', async () => { const createSitemapIndexSpy = jest.spyOn(sitemapUtils, 'createSitemapIndex'); jest.spyOn(global, 'fetch').mockImplementationOnce((url) => {