diff --git a/frontend/src/composables/pageTitle.js b/frontend/src/composables/pageTitle.js index b466eb6f..0b5777b0 100644 --- a/frontend/src/composables/pageTitle.js +++ b/frontend/src/composables/pageTitle.js @@ -1,7 +1,7 @@ // The browser tab title, including WHICH record is open. // // The router can only name the KIND of page, because it runs before the view -// has fetched anything: every application read "West Jefferson ShopDB - +// has fetched anything: every application read " ShopDB - // Application", so two tabs on two different machines were indistinguishable, // and a shared link previewed as the section rather than the thing. // @@ -51,7 +51,7 @@ export async function applyGenericTitle(key, page) { if (key !== activeKey) return // navigated on while we awaited // A view that named the record before the facility name arrived composed its // title against the placeholder prefix, so rewrite it rather than leaving - // 'ShopDB - CSF16' on a site called West Jefferson. + // 'ShopDB - CSF16' without the facility name in front of it. document.title = compose(recordTitle !== null ? recordTitle : page) } diff --git a/frontend/src/utils/siteSettings.js b/frontend/src/utils/siteSettings.js index c7ec67ef..a333a950 100644 --- a/frontend/src/utils/siteSettings.js +++ b/frontend/src/utils/siteSettings.js @@ -98,22 +98,20 @@ function darkenHexColor(hex, fraction) { // ServiceNow ticket-link config. Returns the enabled flag, incident/change URL // templates, and the global-search URL template (all with a {ticket} -// placeholder). Defaults mirror the previously-hardcoded GE ServiceNow URLs. +// placeholder). +// +// The defaults are BLANK and the integration ships OFF, matching the server. +// They used to be one organisation's QA ServiceNow tenant, so any site that had +// not configured ServiceNow still rendered ticket links - pointing its +// technicians at a tenant they cannot reach and, for an adopting site, at +// somebody else's. A blank URL renders tickets as plain text, which is the +// correct behaviour for "not configured". export async function getServicenowUrls() { - const enabledRaw = await getSetting('servicenow_enabled', 'true') + const enabledRaw = await getSetting('servicenow_enabled', 'false') const enabled = enabledRaw !== 'false' && enabledRaw !== '0' && enabledRaw !== false - const incidentUrl = await getSetting( - 'servicenow_incident_url', - 'https://geaerospaceqa.service-now.com/now/nav/ui/search/0f8b85d0c7922010099a308dc7c2606a/params/search-term/{ticket}/global-search-data-config-id/c861cea2c7022010099a308dc7c26041/' - ) - const changeUrl = await getSetting( - 'servicenow_change_url', - 'https://geaerospaceqa.service-now.com/now/nav/ui/search/0f8b85d0c7922010099a308dc7c2606a/params/search-term/{ticket}/global-search-data-config-id/c861cea2c7022010099a308dc7c26041/' - ) - const searchUrl = await getSetting( - 'servicenow_search_url', - 'https://geaerospaceqa.service-now.com/now/nav/ui/search/0f8b85d0c7922010099a308dc7c2606a/params/search-term/{ticket}/global-search-data-config-id/c861cea2c7022010099a308dc7c26041/' - ) + const incidentUrl = await getSetting('servicenow_incident_url', '') + const changeUrl = await getSetting('servicenow_change_url', '') + const searchUrl = await getSetting('servicenow_search_url', '') return { enabled, incidentUrl, changeUrl, searchUrl } } diff --git a/frontend/src/views/settings/ServiceNowSettings.vue b/frontend/src/views/settings/ServiceNowSettings.vue index bfdb8c3f..ffcd7a8b 100644 --- a/frontend/src/views/settings/ServiceNowSettings.vue +++ b/frontend/src/views/settings/ServiceNowSettings.vue @@ -34,7 +34,7 @@ diff --git a/scripts/check-naming-and-style.sh b/scripts/check-naming-and-style.sh index 84e20097..8730db14 100755 --- a/scripts/check-naming-and-style.sh +++ b/scripts/check-naming-and-style.sh @@ -233,17 +233,29 @@ echo "==> Checking for site-specific literals in product code (ADR-015)..." # publication scrub greps for the very strings below - written literally, the # rule's own definition trips the gate that enforces it. Same reason # tests/test_docs_publishable.py splits its terms. -SITE_HOST='tsg'\''wp00525' -SITE_DOMAIN='\.geaero'\''space\.net' -SITE_NAME='West Jeff'\''erson' +# Adjacent quoted strings CONCATENATE: 'tsg''wp00525' is tsgwp00525. The +# apostrophe-escape form ('tsg'\''wp00525') looks similar and is not - it +# embeds a literal ' in the middle, and every one of these patterns then matches +# nothing. This check silently passed everything from the day it was written +# until 2026-08-26, which is how a QA ServiceNow tenant sat in the shipped +# defaults for months with the gate green. +SITE_HOST='tsg''wp00525' +SITE_DOMAIN='\.geaero''space\.net' +SITE_NAME='West Jeff''erson' SITE_NETS='10\.134\.48\.|10\.48\.249\.' -SITE_PATTERNS="${SITE_HOST}|${SITE_DOMAIN}|\bwjs\b|${SITE_NAME}|${SITE_NETS}" +# A named SaaS TENANT is as site-specific as a hostname, and this one shipped as +# the default ServiceNow URL for months: every adopting site would have linked +# its technicians into another organisation's QA instance. The vendor domain +# itself is fine to mention; the tenant is not. +SITE_TENANTS='geaero''spaceqa' +SITE_PATTERNS="${SITE_HOST}|${SITE_DOMAIN}|\bwjs\b|${SITE_NAME}|${SITE_NETS}|${SITE_TENANTS}" SITE_HITS=$(grep -rPni "$SITE_PATTERNS" "${EXCLUDES[@]}" \ --include='*.py' --include='*.vue' --include='*.js' \ --include='*.ps1' --include='*.psm1' --include='*.sh' --include='*.iss' \ --include='*.json' --include='*.html' \ plugins/ shopdb/ scripts/ deploy/ frontend/src/ tools/ 2>/dev/null \ | grep -v '/tests\?/' \ + | grep -v '\.spec\.js:' \ | grep -v 'site_imports/' \ | grep -v 'installer/bundle/' \ | grep -v '\.plugins-staged/' \ diff --git a/shopdb/core/api/search.py b/shopdb/core/api/search.py index 70da6bc1..33fafd10 100644 --- a/shopdb/core/api/search.py +++ b/shopdb/core/api/search.py @@ -72,13 +72,12 @@ def _require_enabled(name): if pm and not pm.registry.is_enabled(name): raise ImportError(f'{name} plugin disabled') -# Shipped GE defaults. Settings override these per-site; identical fallbacks -# live here so this consumer works even if the settings seed has not run. -SERVICENOW_URL_DEFAULT = ( - 'https://geaerospaceqa.service-now.com/now/nav/ui/search/' - '0f8b85d0c7922010099a308dc7c2606a/params/search-term/{ticket}/' - 'global-search-data-config-id/c861cea2c7022010099a308dc7c26041/' -) +# Fallbacks for when the settings seed has not run. The ServiceNow URL is BLANK +# on purpose and must stay that way: a hardcoded tenant here would link one +# site's technicians into another site's ServiceNow, and the code below already +# treats a blank URL as "ServiceNow not configured" and renders tickets as plain +# text. There is no sensible cross-site default for a tenant URL. +SERVICENOW_URL_DEFAULT = '' SERVICENOW_PREFIXES_DEFAULT = 'GEINC,GECHG,GERIT,GESCT' EMPLOYEEID_PATTERN_DEFAULT = r'^\d{9}$' @@ -108,9 +107,12 @@ def _get_search_integrations(): prefixes_raw = SERVICENOW_PREFIXES_DEFAULT prefixes = [p.strip() for p in str(prefixes_raw).split(',') if p.strip()] + # Missing means unconfigured, and unconfigured means OFF - matching the + # shipped default. Defaulting to True here made a site with no ServiceNow + # settings at all behave as though the integration were on. servicenow_enabled = settings.get('servicenow_enabled') if servicenow_enabled is None: - servicenow_enabled = True + servicenow_enabled = False servicenow_url = settings.get('servicenow_search_url') if servicenow_url is None: diff --git a/shopdb/core/api/settings.py b/shopdb/core/api/settings.py index c38d7bdb..66b4ad01 100644 --- a/shopdb/core/api/settings.py +++ b/shopdb/core/api/settings.py @@ -727,20 +727,22 @@ def build_default_settings(): defaults = sitedefaults + brandingdefaults + printingdefaults + identifierdefaults + searchdefaults + mapdefaults + [ # ServiceNow ticket links. Each instance points these at its own # ServiceNow tenant; blank/disabled renders tickets as plain text. + # + # Shipped OFF and BLANK, like the Zabbix block below. These defaults + # used to be one tenant's QA instance, which is wrong twice over: a + # sister site adopting this product would silently link its technicians + # at somebody else's ServiceNow, and even at the originating site a QA + # tenant is not where a real ticket lives. { 'key': 'servicenow_enabled', - 'value': 'true', + 'value': 'false', 'valuetype': 'boolean', 'category': 'integrations', 'description': 'Enable ServiceNow ticket recognition and links in search and dashboards' }, { 'key': 'servicenow_search_url', - 'value': ( - 'https://geaerospaceqa.service-now.com/now/nav/ui/search/' - '0f8b85d0c7922010099a308dc7c2606a/params/search-term/{ticket}/' - 'global-search-data-config-id/c861cea2c7022010099a308dc7c26041/' - ), + 'value': '', 'valuetype': 'string', 'category': 'integrations', 'description': 'ServiceNow global-search URL template. {ticket} is the ticket number.' @@ -754,14 +756,14 @@ def build_default_settings(): }, { 'key': 'servicenow_incident_url', - 'value': 'https://geaerospaceqa.service-now.com/now/nav/ui/search/0f8b85d0c7922010099a308dc7c2606a/params/search-term/{ticket}/global-search-data-config-id/c861cea2c7022010099a308dc7c26041/', + 'value': '', 'valuetype': 'string', 'category': 'integrations', 'description': 'ServiceNow incident URL template. {ticket} is the incident number.' }, { 'key': 'servicenow_change_url', - 'value': 'https://geaerospaceqa.service-now.com/now/nav/ui/search/0f8b85d0c7922010099a308dc7c2606a/params/search-term/{ticket}/global-search-data-config-id/c861cea2c7022010099a308dc7c26041/', + 'value': '', 'valuetype': 'string', 'category': 'integrations', 'description': 'ServiceNow change-request URL template. {ticket} is the change number.' diff --git a/tests/test_core/test_search_integrations.py b/tests/test_core/test_search_integrations.py index 4da87cc2..a6c4d483 100644 --- a/tests/test_core/test_search_integrations.py +++ b/tests/test_core/test_search_integrations.py @@ -14,14 +14,41 @@ from shopdb.core.api.settings import invalidate_settings_cache from shopdb.core.api.search import _get_search_integrations, _classify_query +def _configure_servicenow(): + """Turn ServiceNow on with a tenant URL. + + Required now, and that is the point: the integration ships OFF with a BLANK + URL, because the defaults used to be one organisation's QA tenant and any + adopting site inherited links into somebody else's ServiceNow. + """ + Setting.set('servicenow_enabled', True, valuetype='boolean', + category='integrations') + Setting.set('servicenow_search_url', + 'https://example.service-now.com/search/{ticket}', + valuetype='string', category='integrations') + + def _redirect(client, auth_headers, term): resp = client.get(f'/api/search?q={term}', headers=auth_headers) assert resp.status_code == 200, resp.get_json() return resp.get_json()['data'].get('redirect') +def test_unconfigured_servicenow_yields_no_redirect(client, db, auth_headers): + """Shipped OFF and BLANK. Unconfigured must mean inactive. + + The defaults were once a QA ServiceNow tenant with enabled=true, so a site + that had never configured ServiceNow still sent its technicians to a tenant + they could not reach - and an adopting site to another organisation's. + """ + invalidate_settings_cache() + redirect = _redirect(client, auth_headers, 'GEINC123') + assert redirect is None or redirect.get('type') != 'servicenow' + + def test_default_prefix_classifies_servicenow(client, db, auth_headers): - """With no override, a GE-prefixed ticket redirects to ServiceNow.""" + """Once configured, a default-prefixed ticket redirects to ServiceNow.""" + _configure_servicenow() invalidate_settings_cache() redirect = _redirect(client, auth_headers, 'GEINC123') assert redirect is not None @@ -30,6 +57,7 @@ def test_default_prefix_classifies_servicenow(client, db, auth_headers): def test_custom_prefix_classifies_servicenow(client, db, auth_headers): """A site-configured prefix classifies its tickets as ServiceNow.""" + _configure_servicenow() Setting.set('servicenow_ticket_prefixes', 'ACMEINC', valuetype='string', category='integrations') invalidate_settings_cache() diff --git a/tests/test_core/test_settings_branding.py b/tests/test_core/test_settings_branding.py index 274f1f1f..37c62a56 100644 --- a/tests/test_core/test_settings_branding.py +++ b/tests/test_core/test_settings_branding.py @@ -143,12 +143,19 @@ def test_defaults_contain_new_branding_keys(): def test_defaults_contain_new_integration_keys(): """build_default_settings seeds the ServiceNow integration keys.""" by_key = _defaults_by_key() - assert by_key['servicenow_enabled']['value'] == 'true' + # Ships OFF with blank URLs: the defaults were once a QA tenant, which an + # adopting site would have inherited as links into another organisation. + assert by_key['servicenow_enabled']['value'] == 'false' assert by_key['servicenow_enabled']['category'] == 'integrations' assert by_key['servicenow_ticket_prefixes']['value'] == 'GEINC,GECHG,GERIT,GESCT' - assert '{ticket}' in by_key['servicenow_search_url']['value'] - assert '{ticket}' in by_key['servicenow_incident_url']['value'] - assert '{ticket}' in by_key['servicenow_change_url']['value'] + # Blank, not a tenant URL. There is no cross-site default for "which + # ServiceNow instance is yours", and a blank URL is what the search code + # reads as "not configured" - tickets then render as plain text. + for key in ('servicenow_search_url', 'servicenow_incident_url', + 'servicenow_change_url'): + assert by_key[key]['value'] == '', key + assert '{ticket}' in by_key[key]['description'], \ + 'the placeholder must still be documented for whoever fills it in' def test_defaults_contain_new_site_keys():