fix(diagnostics): give the startup service a body for browser_info
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:
1 parent
11b6c23000
commit
5538e0c575
3 files changed
+128
-4
No files matched your search
@@ -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)
|
||||
})
|
||||
})
|
||||
@@ -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
|
||||
|
||||
Reference in new issue
Block a user