Skip to content

[WORLDSERVICE-464] Remove sw.js for Public Services #12566

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
elvinasv merged 13 commits into from
Apr 1, 2025
Merged

[WORLDSERVICE-464] Remove sw.js for Public Services #12566

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
023e582
refactor: remove sw.js for Public Service
elvinasv Mar 25, 2025
c823c44
test: update service worker tests to reflect non-WS
elvinasv Mar 25, 2025
9b2989b
Merge branch 'latest' of github.com:bbc/simorgh into 464-remove-sw-js…
elvinasv Mar 25, 2025
cc31758
refactor: remove Public Service related regex for getArticleSwRegex; …
elvinasv Mar 26, 2025
c210d37
Merge branch 'latest' of github.com:bbc/simorgh into 464-remove-sw-js…
elvinasv Mar 26, 2025
5bf38c4
Merge branch 'latest' of github.com:bbc/simorgh into 464-remove-sw-js…
elvinasv Mar 28, 2025
16f79f4
refactor: remove getArticleSwRegex, getArticleManifestRegex and relat…
elvinasv Mar 31, 2025
643a20d
Merge branch 'latest' of github.com:bbc/simorgh into 464-remove-sw-js…
elvinasv Mar 31, 2025
c3479eb
test: re-enable e2e test; remove outdated tests
elvinasv Mar 31, 2025
4f4d5bb
Merge pull request #12583 from bbc/594-ensure-app-cypress-tests-are-r…
elvinasv Mar 31, 2025
7f46c5e
Merge branch 'latest' of github.com:bbc/simorgh into 464-remove-sw-js…
elvinasv Mar 31, 2025
2b57114
refactor: keep sw test; remove array wrapper
elvinasv Mar 31, 2025
0434dbc
Merge branch 'latest' of github.com:bbc/simorgh into 464-remove-sw-js…
elvinasv Apr 1, 2025
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
1 change: 0 additions & 1 deletion src/app/lib/config/services/archive.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ export const service: DefaultServiceConfig = {
publishingPrinciples: null,
isTrustProjectParticipant: false,
script: latin,
swPath: '/articles/sw.js',
homePageTitle: 'Home',
showAdPlaceholder: false,
showRelatedTopics: true,
Expand Down
1 change: 0 additions & 1 deletion src/app/lib/config/services/scotland.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ export const service: DefaultServiceConfig = {
publishingPrinciples: null,
isTrustProjectParticipant: false,
script: latin,
swPath: '/articles/sw.js',
homePageTitle: 'Home',
passportHomes: ['BBCScotland'],
showAdPlaceholder: false,
Expand Down
11 changes: 5 additions & 6 deletions src/app/routes/utils/regex/index.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,18 +125,16 @@ describe('homePageDataPath', () => {
});

describe('articleSwPath', () => {
const validRoutes = [
'/news/articles/sw.js',
'/persian/articles/sw.js',
'/cymrufyw/erthyglau/sw.js',
];
const validRoutes = ['/gahuza/articles/sw.js', '/persian/articles/sw.js'];
shouldMatchValidRoutes(validRoutes, articleSwPath);

const invalidRoutes = [
'/news/sw.js',
'/news/articles/sw.js',
Copy link
Contributor

@karinathomasbbc karinathomasbbc Mar 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am wondering whether to add examples of all sw paths which are now invalid, for all Public Services? Especially if we have excluded them explictly from the regex.

So this would be:

  • /sport/articles/sw.js
  • /newsround/articles/sw.js
  • /archive/articles/sw.js
  • /scotland/articles/sw.js
  • /naidheachdan/sgeulachdan/sw.js

Copy link
Member Author

@elvinasv elvinasv Mar 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, updated

'/persian/articles/sw',
'/news/trad/sw.js',
'/cymrufyw/sw.js',
'/cymrufyw/erthyglau/sw.js',
];
shouldNotMatchInvalidRoutes(invalidRoutes, articleSwPath);
});
Expand Down Expand Up @@ -166,10 +164,11 @@ describe('articleManifestPath', () => {
});

describe('homePageSwPath', () => {
const validRoutes = ['/news/sw.js', '/persian/sw.js'];
const validRoutes = ['/gahuza/sw.js', '/persian/sw.js'];
shouldMatchValidRoutes(validRoutes, homePageSwPath);

const invalidRoutes = [
'/news/sw.js',
'/news/articles/sw.js',
'/persian/sw',
'/persian/simp/sw.js',
Expand Down
8 changes: 4 additions & 4 deletions src/app/routes/utils/regex/utils/__snapshots__/index.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@

exports[`regex utils snapshots should create expected regex from getAfricaEyeTVPageRegex 1`] = `"/worldservice/tv/africa_eye/:episodeId([a-z0-9]+)?:lite(.lite)?"`;

exports[`regex utils snapshots should create expected regex from getArticleManifestRegex 1`] = `"/:service(afaanoromoo|afrique|amharic|arabic|azeri|bengali|burmese|gahuza|gujarati|hausa|hindi|igbo|indonesia|japanese|korean|kyrgyz|marathi|mundo|nepali|pashto|persian|pidgin|portuguese|punjabi|russian|scotland|serbian|sinhala|somali|swahili|tamil|telugu|thai|tigrinya|turkce|ukchina|ukrainian|urdu|uzbek|vietnamese|ws|yoruba|zhongwen)/:local(articles|erthyglau|sgeulachdan)/manifest.json"`;
exports[`regex utils snapshots should create expected regex from getArticleManifestRegex 1`] = `"/:service(afaanoromoo|afrique|amharic|arabic|azeri|bengali|burmese|gahuza|gujarati|hausa|hindi|igbo|indonesia|japanese|korean|kyrgyz|marathi|mundo|nepali|pashto|persian|pidgin|portuguese|punjabi|russian|serbian|sinhala|somali|swahili|tamil|telugu|thai|tigrinya|turkce|ukchina|ukrainian|urdu|uzbek|vietnamese|ws|yoruba|zhongwen)/:local(articles|erthyglau|sgeulachdan)/manifest.json"`;

exports[`regex utils snapshots should create expected regex from getArticleRegex 1`] = `"/:service(afaanoromoo|afrique|amharic|arabic|archive|azeri|bengali|burmese|cymrufyw|gahuza|gujarati|hausa|hindi|igbo|indonesia|japanese|korean|kyrgyz|marathi|mundo|naidheachdan|nepali|news|newsround|pashto|persian|pidgin|portuguese|punjabi|russian|scotland|serbian|sinhala|somali|sport|swahili|tamil|telugu|thai|tigrinya|turkce|ukchina|ukrainian|urdu|uzbek|vietnamese|ws|yoruba|zhongwen)?:discipline(/[a-z0-9-_]{1,})?/:local(articles|erthyglau|sgeulachdan)/:id(c[a-zA-Z0-9]{10}o):variant(/simp|/trad|/cyr|/lat)?:nonCanonicalArticleRenderPlatform(.amp|.app|.lite)?"`;

exports[`regex utils snapshots should create expected regex from getArticleSwRegex 1`] = `"/:service(afaanoromoo|afrique|amharic|arabic|archive|azeri|bengali|burmese|cymrufyw|gahuza|gujarati|hausa|hindi|igbo|indonesia|japanese|korean|kyrgyz|marathi|mundo|naidheachdan|nepali|news|newsround|pashto|persian|pidgin|portuguese|punjabi|russian|scotland|serbian|sinhala|somali|sport|swahili|tamil|telugu|thai|tigrinya|turkce|ukchina|ukrainian|urdu|uzbek|vietnamese|ws|yoruba|zhongwen)/:local(articles|erthyglau|sgeulachdan)/sw.js"`;
exports[`regex utils snapshots should create expected regex from getArticleSwRegex 1`] = `"/:service(afaanoromoo|afrique|amharic|arabic|azeri|bengali|burmese|gahuza|gujarati|hausa|hindi|igbo|indonesia|japanese|korean|kyrgyz|marathi|mundo|nepali|pashto|persian|pidgin|portuguese|punjabi|russian|serbian|sinhala|somali|swahili|tamil|telugu|thai|tigrinya|turkce|ukchina|ukrainian|urdu|uzbek|vietnamese|ws|yoruba|zhongwen)/:local(articles|erthyglau|sgeulachdan)/sw.js"`;

exports[`regex utils snapshots should create expected regex from getCpsAssetRegex 1`] = `"/:service(afaanoromoo|afrique|amharic|arabic|archive|azeri|bengali|burmese|cymrufyw|gahuza|gujarati|hausa|hindi|igbo|indonesia|japanese|korean|kyrgyz|marathi|mundo|naidheachdan|nepali|news|newsround|pashto|persian|pidgin|portuguese|punjabi|russian|scotland|serbian|sinhala|somali|sport|swahili|tamil|telugu|thai|tigrinya|turkce|ukchina|ukrainian|urdu|uzbek|vietnamese|ws|yoruba|zhongwen):variant(/simp|/trad|/cyr|/lat)?/:assetUri([a-z0-9-_+]{0,}[0-9]{8,}):amp(.amp)?:lite(.lite)?"`;

Expand All @@ -18,7 +18,7 @@ exports[`regex utils snapshots should create expected regex from getLegacyAssetR

exports[`regex utils snapshots should create expected regex from getLiveRadioRegex 1`] = `"/:service(afaanoromoo|afrique|amharic|arabic|archive|azeri|bengali|burmese|cymrufyw|gahuza|gujarati|hausa|hindi|igbo|indonesia|japanese|korean|kyrgyz|marathi|mundo|naidheachdan|nepali|news|newsround|pashto|persian|pidgin|portuguese|punjabi|russian|scotland|serbian|sinhala|somali|sport|swahili|tamil|telugu|thai|tigrinya|turkce|ukchina|ukrainian|urdu|uzbek|vietnamese|ws|yoruba|zhongwen)/:masterBrand(bbc_[a-z]+_radio)/:mediaId(liveRadio):lite(.lite)?"`;

exports[`regex utils snapshots should create expected regex from getManifestRegex 1`] = `"/:service(afaanoromoo|afrique|amharic|arabic|azeri|bengali|burmese|gahuza|gujarati|hausa|hindi|igbo|indonesia|japanese|korean|kyrgyz|marathi|mundo|nepali|pashto|persian|pidgin|portuguese|punjabi|russian|scotland|serbian|sinhala|somali|swahili|tamil|telugu|thai|tigrinya|turkce|ukchina|ukrainian|urdu|uzbek|vietnamese|ws|yoruba|zhongwen)/manifest.json"`;
exports[`regex utils snapshots should create expected regex from getManifestRegex 1`] = `"/:service(afaanoromoo|afrique|amharic|arabic|azeri|bengali|burmese|gahuza|gujarati|hausa|hindi|igbo|indonesia|japanese|korean|kyrgyz|marathi|mundo|nepali|pashto|persian|pidgin|portuguese|punjabi|russian|serbian|sinhala|somali|swahili|tamil|telugu|thai|tigrinya|turkce|ukchina|ukrainian|urdu|uzbek|vietnamese|ws|yoruba|zhongwen)/manifest.json"`;

exports[`regex utils snapshots should create expected regex from getMostReadDataRegex 1`] = `"/:service(afaanoromoo|afrique|amharic|arabic|archive|azeri|bengali|burmese|cymrufyw|gahuza|gujarati|hausa|hindi|igbo|indonesia|japanese|korean|kyrgyz|marathi|mundo|naidheachdan|nepali|news|newsround|pashto|persian|pidgin|portuguese|punjabi|russian|scotland|serbian|sinhala|somali|sport|swahili|tamil|telugu|thai|tigrinya|turkce|ukchina|ukrainian|urdu|uzbek|vietnamese|ws|yoruba|zhongwen)/mostread:variant(/simp|/trad|/cyr|/lat)?.json"`;

Expand All @@ -36,6 +36,6 @@ exports[`regex utils snapshots should create expected regex from getRecommendati

exports[`regex utils snapshots should create expected regex from getSecondaryColumnDataRegex 1`] = `"/:service(afaanoromoo|afrique|amharic|arabic|archive|azeri|bengali|burmese|cymrufyw|gahuza|gujarati|hausa|hindi|igbo|indonesia|japanese|korean|kyrgyz|marathi|mundo|naidheachdan|nepali|news|newsround|pashto|persian|pidgin|portuguese|punjabi|russian|scotland|serbian|sinhala|somali|sport|swahili|tamil|telugu|thai|tigrinya|turkce|ukchina|ukrainian|urdu|uzbek|vietnamese|ws|yoruba|zhongwen)/sty-secondary-column:variant(/simp|/trad|/cyr|/lat)?.json"`;

exports[`regex utils snapshots should create expected regex from getSwRegex 1`] = `"/:service(afaanoromoo|afrique|amharic|arabic|archive|azeri|bengali|burmese|cymrufyw|gahuza|gujarati|hausa|hindi|igbo|indonesia|japanese|korean|kyrgyz|marathi|mundo|naidheachdan|nepali|news|newsround|pashto|persian|pidgin|portuguese|punjabi|russian|scotland|serbian|sinhala|somali|sport|swahili|tamil|telugu|thai|tigrinya|turkce|ukchina|ukrainian|urdu|uzbek|vietnamese|ws|yoruba|zhongwen)/sw.js"`;
exports[`regex utils snapshots should create expected regex from getSwRegex 1`] = `"/:service(afaanoromoo|afrique|amharic|arabic|azeri|bengali|burmese|gahuza|gujarati|hausa|hindi|igbo|indonesia|japanese|korean|kyrgyz|marathi|mundo|nepali|pashto|persian|pidgin|portuguese|punjabi|russian|serbian|sinhala|somali|swahili|tamil|telugu|thai|tigrinya|turkce|ukchina|ukrainian|urdu|uzbek|vietnamese|ws|yoruba|zhongwen)/sw.js"`;

exports[`regex utils snapshots should create expected regex from getTopicPageRegex 1`] = `"/:service(afaanoromoo|afrique|amharic|arabic|archive|azeri|bengali|burmese|cymrufyw|gahuza|gujarati|hausa|hindi|igbo|indonesia|japanese|korean|kyrgyz|marathi|mundo|naidheachdan|nepali|news|newsround|pashto|persian|pidgin|portuguese|punjabi|russian|scotland|serbian|sinhala|somali|sport|swahili|tamil|telugu|thai|tigrinya|turkce|ukchina|ukrainian|urdu|uzbek|vietnamese|ws|yoruba|zhongwen)/topics/:id([a-z0-9]+)?:variant(/simp|/trad|/cyr|/lat)?:lite(.lite)?"`;
13 changes: 7 additions & 6 deletions src/app/routes/utils/regex/utils/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,6 @@ export const getArticleRegex = services => {
return `/:service(${serviceRegex})?:discipline(${sportDisciplineRegex})?/:local(${articleLocalRegex})/:id(${idRegex}):variant(${variantRegex})?:nonCanonicalArticleRenderPlatform(${nonCanonicalArticleRenderPlatform})?`;
};

export const getArticleSwRegex = services => {
const serviceRegex = getServiceRegex(services);
return `/:service(${serviceRegex})/:local(${articleLocalRegex})/sw.js`;
};

const getWorldServices = services => {
const publicServices = [
'news',
Expand All @@ -35,11 +30,17 @@ const getWorldServices = services => {
'cymrufyw',
'naidheachdan',
'archive',
'scotland',
];

return services.filter(service => !publicServices.includes(service));
};

export const getArticleSwRegex = services => {
const serviceRegex = getServiceRegex(getWorldServices(services));
return `/:service(${serviceRegex})/:local(${articleLocalRegex})/sw.js`;
Copy link
Contributor

@karinathomasbbc karinathomasbbc Mar 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that the articleLocalRegex was for the public services, does it now make sense to remove this, and hardcode articles instead...?

Suggested change
return `/:service(${serviceRegex})/:local(${articleLocalRegex})/sw.js`;
return `/:service(${serviceRegex})/articles/sw.js`;

Copy link
Member Author

@elvinasv elvinasv Mar 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. Updated

};

export const getArticleManifestRegex = services => {
const serviceRegex = getServiceRegex(getWorldServices(services));
return `/:service(${serviceRegex})/:local(${articleLocalRegex})/manifest.json`;
Copy link
Contributor

@karinathomasbbc karinathomasbbc Mar 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is potentially out of scope (or is it? - Let's get a second opinion!) Let's do this while we're here: per my comment above anything that uses getWorldServices no longer needs articleLocalRegex. If we did change it, we would also need to update some tests.

(Note that this we recently removed manifest paths from Public Services, and we just missed this.

Suggested change
export const getArticleManifestRegex = services => {
const serviceRegex = getServiceRegex(getWorldServices(services));
return `/:service(${serviceRegex})/:local(${articleLocalRegex})/manifest.json`;
export const getArticleManifestRegex = services => {
const serviceRegex = getServiceRegex(getWorldServices(services));
return `/:service(${serviceRegex})/articles/manifest.json`;

Copy link
Member Author

@elvinasv elvinasv Mar 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense. Updated

Expand All @@ -51,7 +52,7 @@ export const getHomePageRegex = services => {
};

export const getSwRegex = services => {
const serviceRegex = getServiceRegex(services);
const serviceRegex = getServiceRegex(getWorldServices(services));
return `/:service(${serviceRegex})/sw.js`;
};

Expand Down
2 changes: 1 addition & 1 deletion src/server/index.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -953,7 +953,7 @@ describe('Server', () => {

describe('Service workers', () => {
it('should serve a file for existing service workers', async () => {
Copy link
Contributor

@karinathomasbbc karinathomasbbc Mar 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make sure there's still a test for existence of /world-service/sw.js? (choose one of the world services as an example)

Copy link
Member Author

@elvinasv elvinasv Mar 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, we should def keep it. Updated

await makeRequest('/news/articles/sw.js');
await makeRequest('/gahuza/articles/sw.js');
expect(sendFileSpy.mock.calls[0][0]).toEqual(
path.join(__dirname, '/public/sw.js'),
);
Expand Down
Loading