Skip to content
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.

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

Fix regex misuse. #199

Merged
merged 2 commits into from
Jun 10, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 1 addition & 1 deletion spec/javascripts/environment.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ describe('Popup.environment', function () {
function createEnvironmentForUrl (location) {
var a = document.createElement('a')
a.href = location
return Popup.environment(a.href, a.host, a.origin).allEnvironments
return Popup.environment(a.href, a.hostname, a.origin).allEnvironments
}

it('returns the correct environment links when the user is on production', function () {
Expand Down
2 changes: 1 addition & 1 deletion src/fetch-page-data.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
chrome.runtime.sendMessage({
action: 'populatePopup',
currentLocation: window.location.href,
currentHost: window.location.host,
currentHost: window.location.hostname,
currentOrigin: window.location.origin,
currentPathname: window.location.pathname,
renderingApplication: getMetatag('govuk:rendering-application'),
Expand Down
7 changes: 4 additions & 3 deletions src/popup/content_links.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,13 @@ Popup.generateContentLinks = function (location, origin, pathname, currentEnviro

// If no path can be found (which means we're probably in a publishing app)
// Similarly if we're on GOVUK account, not many of the links are relevant
if (!path || origin.match(/www.account/)) {
if (!path || origin.includes('www.account')) {
return []
}

// This is 'https://www.gov.uk' or 'https://www-origin.integration.publishing.service.gov.uk/', etc.
if (origin === 'http://webarchive.nationalarchives.gov.uk' || origin.match(/draft-origin/) || origin.match(/content-data/) || origin.match(/support/)) {
// Origin looks like 'https://www.gov.uk' or 'https://www-origin.integration.publishing.service.gov.uk/' or similar.
if (origin === 'http://webarchive.nationalarchives.gov.uk' ||
/draft-origin|content-data|support/.test(origin)) {
origin = 'https://www.gov.uk'
}

Expand Down
11 changes: 4 additions & 7 deletions src/popup/environment.js
Original file line number Diff line number Diff line change
@@ -1,18 +1,16 @@
var Popup = Popup || {}

// Given a location, host and origin, generate URLs for all GOV.UK environments.
// Given a location, hostname and origin, generate URLs for all GOV.UK environments.
//
// Returns a hash with envs, including one with `class: "current"` to show
// the current environment.
Popup.environment = function (location, host, origin) {
function isPartOfGOVUK () {
return host.match(/www.gov.uk/) ||
host.match(/publishing.service.gov.uk/) ||
host.match(/dev.gov.uk/)
return /^(www|.*\.publishing\.service|(www\.)?dev)\.gov\.uk$/.test(host)
}

function isGOVUKAccount () {
return host.match(/www.account/) || host.match(/login.service.dev/)
return /^(www\.account.*\.service\.gov\.uk|login\.service\.dev)$/.test(host)
}

if (!isPartOfGOVUK()) {
Expand All @@ -28,7 +26,6 @@ Popup.environment = function (location, host, origin) {
host: 'https://www.gov.uk',
origin: origin
}

}
}

Expand Down Expand Up @@ -64,7 +61,7 @@ Popup.environment = function (location, host, origin) {
]

var application = isGOVUKAccount() ? host.split('.')[1] : host.split('.')[0]
var inFrontend = application.match(/www/) && !isGOVUKAccount()
var inFrontend = application.includes('www') && !isGOVUKAccount()
var environments = ENVIRONMENTS

var currentEnvironment
Expand Down
18 changes: 9 additions & 9 deletions src/popup/extract_path.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,23 @@
Popup.extractPath = function (location, pathname, renderingApplication) {
var extractedPath

if (location.match(/api\/content/)) {
if (location.includes('api/content')) {
extractedPath = pathname.replace('api/content/', '')
} else if (location.match(/anonymous_feedback/)) {
} else if (location.includes('anonymous_feedback')) {
extractedPath = extractQueryParameter(location, 'path')
} else if (location.match(/content-data/)) {
} else if (location.includes('content-data')) {
extractedPath = pathname.replace('metrics/', '')
} else if (location.match(/nationalarchives.gov.uk/)) {
} else if (location.includes('nationalarchives.gov.uk')) {
github-advanced-security[bot] marked this conversation as resolved.
Fixed
Show resolved Hide resolved
extractedPath = pathname.split('https://www.gov.uk')[1]
} else if (location.match(/api\/search.json/)) {
} else if (location.includes('api/search.json')) {
extractedPath = extractQueryParameter(location, 'filter_link')
} else if (location.match(/info/)) {
} else if (location.includes('info')) {
extractedPath = pathname.replace('info/', '')
} else if (location.match(/api.*\.json/)) {
} else if (/api.*\.json/.test(location)) {
extractedPath = pathname.replace('api/', '').replace('.json', '')
} else if (location.match(/visualise/)) {
} else if (location.includes('visualise')) {
extractedPath = pathname.replace('/y/visualise', '')
} else if (location.match(/www/) || location.match(/draft-origin/)) {
} else if (/www|draft-origin/.test(location)) {
extractedPath = pathname
}

Expand Down
12 changes: 6 additions & 6 deletions src/popup/popup.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,9 @@ var Popup = Popup || {};
})

// Render the popup.
function renderPopup (location, host, origin, pathname, renderingApplication, windowHeight, abTestBuckets) {
function renderPopup (location, hostname, origin, pathname, renderingApplication, windowHeight, abTestBuckets) {
// Creates a view object with the data and render a template with it.
var view = createView(location, host, origin, pathname, renderingApplication, abTestBuckets)
var view = createView(location, hostname, origin, pathname, renderingApplication, abTestBuckets)

var contentStore = view.contentLinks.find(function (el) { return el.name === 'Content item (JSON)' })

Expand Down Expand Up @@ -162,7 +162,7 @@ var Popup = Popup || {};
// TODO: we're not actually re-rendering the popup correctly here, because
// we don't have access to the DOM here. This is a temporary solution to
// make most functionality work after the user clicks a button in the popup.
renderPopup(location.href, location.host, location.origin, location.pathname, {})
renderPopup(location.href, location.hostname, location.origin, location.pathname, {})
})
})

Expand Down Expand Up @@ -212,11 +212,11 @@ var Popup = Popup || {};
})
}

// This is the view object. It takes a location, host, origin, the name of the
// This is the view object. It takes a location, hostname, origin, the name of the
// rendering app and a list of A/B test buckets and creates an object with all
// URLs and other view data to render the popup.
function createView (location, host, origin, pathname, renderingApplication, abTestBuckets) {
var environment = Popup.environment(location, host, origin)
function createView (location, hostname, origin, pathname, renderingApplication, abTestBuckets) {
var environment = Popup.environment(location, hostname, origin)
var contentLinks = Popup.generateContentLinks(location, origin, pathname, environment.currentEnvironment, renderingApplication)
// var abTests = Popup.findActiveAbTests(abTestBuckets)

Expand Down
Loading