fix(diagnostics): give the startup service a body for browser_info
Build / Build-and-ng-test (pull_request) Successful in 6m6s
Lighthouse Checks / lighthouse (pull_request) Successful in 21m55s
Build / Build-and-test-development (pull_request) Successful in 26m43s

Both startupservice call sites pass null, and the attach guard skipped a
null payload, so the service whose log a support ticket is read from
never received browser_info / browser_url_vars.

The e2e test named for the startup service only asserted on getdata, so
the gap was invisible. The name is corrected to what it asserts, its
sibling sweep now excludes the diagnostics services (it must, once the
startup service really does carry them), and the guard itself is covered
by a unit spec - red before the fix, green after.
This commit is contained in:
dc committed 2026-09-25 15:57:53 +00:00
1 parent 11b6c23000
commit 5538e0c575
3 files changed
+128 -4

No files matched your search

+28 -4
View File
@@ -19,6 +19,18 @@ export {}
// session survived.
const hostUrl = Cypress.env('hosturl')
/** The services the app sends diagnostics to (see DIAGNOSTICS_SERVICES in
* client/src/app/services/sas.service.ts). */
const DIAGNOSTICS_SERVICES = [
'public/startupservice',
'editors/getdata',
'editors/getdynamiccolvals',
'editors/stagedata',
'editors/loadfile',
'editors/restore',
'auditors/postdata'
]
/** Boots the app at the root, accepting the evaluation agreement if shown. */
const bootApp = () => {
cy.visit(hostUrl, { timeout: 60000 })
@@ -67,12 +79,19 @@ context('browser_info input table: ', function () {
cy.then(() => {
const captured = Object.keys(programs)
// The startup service is a diagnostics service by design - it runs on
// every boot, which is the point of it - so it is excluded from this
// sweep. On a real estate it would otherwise appear here carrying
// browser_info and fail the test.
const plain = captured.filter(
(program) => !DIAGNOSTICS_SERVICES.some((s) => program.includes(s))
)
expect(
captured.length,
`service calls captured (${captured.join(', ')})`
plain.length,
`non-diagnostics service calls captured (${captured.join(', ')})`
).to.be.greaterThan(0)
captured.forEach((program) => {
plain.forEach((program) => {
expect(
programs[program],
`${program} carries no browser_info`
@@ -81,7 +100,7 @@ context('browser_info input table: ', function () {
})
})
it('2 | startup service and hook-running services receive it', () => {
it('2 | hook-running services receive it', () => {
const programs = capturePrograms()
bootApp()
@@ -98,6 +117,11 @@ context('browser_info input table: ', function () {
`service calls captured (${captured.join(', ')})`
).to.be.greaterThan(0)
// The startup service is the other diagnostics recipient, but it is not
// reachable from a mock estate: `checkSasjsDeploy` finds makedata in
// services/admin and routes to the deploy screen instead, so the app
// never calls it on boot. Its null-payload path is covered by the unit
// spec (src/app/services/sas.service.spec.ts).
const getdata = captured.find((p) => p.includes('editors/getdata'))
expect(getdata, 'editors/getdata captured').to.not.be.undefined
expect(
@@ -0,0 +1,89 @@
import { SasService } from './sas.service'
/**
* SasService's constructor is a plain 8-argument constructor with an empty
* body, and `request()` touches only a handful of collaborators, so this
* stubs exactly that surface - no TestBed/DI needed, same precedent as
* app.service.spec.ts and va-filter.service.spec.ts.
*
* The SASjs adapter is not built by the constructor either (sasServiceInit
* does that), so it is assigned directly - and its `request` spy is where the
* outgoing body is observable, which is the whole point of these tests.
*/
const buildService = () => {
const sasjsAdapter: any = {
request: jasmine.createSpy('request').and.resolveTo({}),
getSasRequests: () => []
}
const loggerService: any = {
logRequestData: jasmine.createSpy('logRequestData')
}
const userService: any = { user: undefined }
const eventService: any = {
showAbortModal: jasmine.createSpy('showAbortModal'),
startupDataLoaded: jasmine.createSpy('startupDataLoaded')
}
const router: any = { navigateByUrl: jasmine.createSpy('navigateByUrl') }
const service = new SasService(
{} as any, // appStoreService
userService,
eventService,
{} as any, // sasjsService
{} as any, // sasViyaService
loggerService,
{} as any, // startupCheckService
router
)
;(service as any).sasjsAdapter = sasjsAdapter
return { service, sasjsAdapter }
}
/** The body the adapter was asked to send for the most recent request. */
const sentBody = (sasjsAdapter: any) =>
sasjsAdapter.request.calls.mostRecent().args[1]
describe('SasService diagnostics payload', () => {
it('gives the startup service a body, so browser_info can ride on it', async () => {
const { service, sasjsAdapter } = buildService()
// Both startup service call sites pass null - there is nothing to send -
// and the service is the one whose log a support ticket is read from.
await service.request('public/startupservice', null)
const body = sentBody(sasjsAdapter)
expect(body).toBeTruthy()
expect(body.browser_info.length).toBe(1)
expect(body.browser_info[0].user_agent).toBe(navigator.userAgent)
expect('timezone' in body.browser_info[0]).toBe(true)
expect('tz_offset' in body.browser_info[0]).toBe(true)
})
it('leaves a non-diagnostics service payload alone', async () => {
const { service, sasjsAdapter } = buildService()
await service.request('public/viewlibs', null)
expect(sentBody(sasjsAdapter)).toBeNull()
})
it('adds browser_info to a body that already carries content', async () => {
const { service, sasjsAdapter } = buildService()
await service.request('editors/getdata', { table: 'MPE_X_TEST' })
const body = sentBody(sasjsAdapter)
expect(body.table).toBe('MPE_X_TEST')
expect(body.browser_info.length).toBe(1)
})
it('does not overwrite a browser_info the caller supplied', async () => {
const { service, sasjsAdapter } = buildService()
const supplied = [{ url: 'supplied by the caller' }]
await service.request('editors/getdata', { browser_info: supplied })
expect(sentBody(sasjsAdapter).browser_info).toBe(supplied)
})
})
+11
View File
@@ -147,6 +147,17 @@ export class SasService {
// distinguished. The browser_url_vars table carries the URL parameters
// as name/value pairs, which is easier for a SAS developer to read than
// parsing the url string.
// The startup service is called with a null payload - there is nothing to
// send - so give it one to carry the diagnostics. It is the service whose
// log a support ticket is read from, and without this it never receives
// them.
if (
DIAGNOSTICS_SERVICES.includes(url) &&
(data === null || data === undefined)
) {
data = {}
}
if (data && typeof data === 'object' && !data.browser_info) {
if (DIAGNOSTICS_SERVICES.includes(url)) {
const tz = Intl.DateTimeFormat().resolvedOptions().timeZone