From d0f79d0d0f51e3d5e6fad520153181718dbc41d5 Mon Sep 17 00:00:00 2001 From: Paulius Date: Mon, 30 Sep 2024 15:48:35 +0100 Subject: [PATCH] VR-228: query response bugfix (#181) * VR-228: curly bracket removal and controller update (types). * VR-228: rebase with main. * VR-228: a few refinements in regards to partials along with total emissions. * VR-228: second company mock.' * VR-228: switch back company number to DC. * VR-228: do not deconstruct @Body since CI/CD complains. * VR-228: remove id attribute since it's no longer needed. * VR-228: avoiding allowing excess properties for partial queries. * VR-228: avoid arbitrary data in the request body * VR-228: tidy up and a comment . * VR-228: validation check and grouping of partial query inputs. * VR-228: quantity -> quantities (rename). * VR-228: partial validation tests. * VR-228: typo.: --- package-lock.json | 4 +- package.json | 2 +- .../queries/__tests__/index.test.ts | 111 +++++++++++++++++- src/controllers/queries/index.ts | 82 +++++++++---- .../__tests__/companyHouseEntity.test.ts | 1 + .../fixtures/companyHouseFixtures.ts | 13 ++ .../__tests__/helpers/mockCompanyHouse.ts | 10 ++ src/models/arrays.ts | 6 + .../__tests__/partial-query.test.ts.snap | 4 +- .../queryResponses/respondToScope3.tsx | 2 +- src/views/queries/responseCo2scope3.tsx | 11 +- 11 files changed, 208 insertions(+), 38 deletions(-) create mode 100644 src/models/arrays.ts diff --git a/package-lock.json b/package-lock.json index 2a520cef..29033a26 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "veritable-ui", - "version": "0.9.4", + "version": "0.9.5", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "veritable-ui", - "version": "0.9.4", + "version": "0.9.5", "license": "Apache-2.0", "dependencies": { "@digicatapult/tsoa-oauth-express": "^0.1.46", diff --git a/package.json b/package.json index 5f8973b5..6d4e0348 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "veritable-ui", - "version": "0.9.4", + "version": "0.9.5", "description": "UI for Veritable", "main": "src/index.ts", "type": "module", diff --git a/src/controllers/queries/__tests__/index.test.ts b/src/controllers/queries/__tests__/index.test.ts index d690441e..75447da6 100644 --- a/src/controllers/queries/__tests__/index.test.ts +++ b/src/controllers/queries/__tests__/index.test.ts @@ -139,7 +139,110 @@ describe('QueriesController', () => { expect(result).to.equal('scope3_error_scope3') }) }) + describe('query responses', () => { + describe('partial query responses', () => { + describe('if invalid partial input', () => { + it('throws if connectionsIds array is not in the req.body', async () => { + const { args } = withQueriesMocks() + const controller = new QueriesController(...args) + try { + await controller.scope3CarbonConsumptionResponseSubmit(req, 'query-partial-id-test', { + companyId: 'some-company-id', + action: 'success', + partialQuery: ['on'], + productIds: ['product-1', 'product-2'], + quantities: ['10', '20'], + }) + // the below expect should never happen since we expect test to throw + expect(false).to.be.equal(true) + } catch (err) { + expect(err.toString()).to.be.equal('Error: missing a property in the request body') + } + }) + + it('throws if productIds array is not in the req.body', async () => { + const { args } = withQueriesMocks() + const controller = new QueriesController(...args) + try { + await controller.scope3CarbonConsumptionResponseSubmit(req, 'query-partial-id-test', { + companyId: 'some-company-id', + action: 'success', + partialQuery: ['on'], + connectionIds: ['conn-id-1', 'conn-id-2'], + quantities: ['10', '20'], + }) + // the below expect should never happen since we expect test to throw + expect(false).to.be.equal(true) + } catch (err) { + expect(err.toString()).to.be.equal('Error: missing a property in the request body') + } + }) + + it('throws if quantities and productIds arrays are not req.body', async () => { + const { args } = withQueriesMocks() + const controller = new QueriesController(...args) + try { + await controller.scope3CarbonConsumptionResponseSubmit(req, 'query-partial-id-test', { + companyId: 'some-company-id', + action: 'success', + partialQuery: ['on'], + connectionIds: ['conn-id-1', 'conn-id-2'], + }) + // the below expect should never happen since we expect test to throw + expect(false).to.be.equal(true) + } catch (err) { + expect(err.toString()).to.be.equal('Error: missing a property in the request body') + } + }) + + it('throws if arrays are not the same size', async () => { + const { args } = withQueriesMocks() + const controller = new QueriesController(...args) + try { + await controller.scope3CarbonConsumptionResponseSubmit(req, 'query-partial-id-test', { + companyId: 'some-company-id', + action: 'success', + partialQuery: ['on'], + productIds: ['product-id-1'], + quantities: ['1', '2', '3'], + connectionIds: ['conn-id-1', 'conn-id-2'], + }) + // the below expect should never happen since we expect test to throw + expect(false).to.be.equal(true) + } catch (err) { + expect(err.toString()).to.be.equal('Error: partial query validation failed, invalid data') + } + }) + }) + + it('sets a partial query status to resolved and renders response template', async () => { + const { args, dbMock } = withQueriesMocks() + const controller = new QueriesController(...args) + const getSpy = dbMock.get + const updateSpy = dbMock.update + const result = await controller + .scope3CarbonConsumptionResponseSubmit(req, 'query-partial-id-test', { + companyId: 'some-company-id', + action: 'success', + partialQuery: ['on'], + connectionIds: ['conn-id-1', 'conn-id-2'], + productIds: ['product-1', 'product-2'], + quantities: ['10', '20'], + }) + .then(toHTMLString) + + expect(getSpy.firstCall.calledWith('connection', { id: 'some-company-id', status: 'verified_both' })).to.equal( + true + ) + expect(getSpy.secondCall.calledWith('query', { id: 'query-partial-id-test' })).to.equal(true) + expect( + updateSpy.firstCall.calledWith('query', { id: 'query-partial-id-test' }, { status: 'resolved' }) + ).to.equal(true) + expect(result).to.be.equal('queriesResponse_template') + }) + }) + it('should call db as expected', async () => { const { args, dbMock } = withQueriesMocks() const controller = new QueriesController(...args) @@ -164,7 +267,7 @@ describe('QueriesController', () => { .scope3CarbonConsumptionResponseSubmit(req, '5390af91-c551-4d74-b394-d8ae0805059e', { companyId: '2345789', action: 'success', - totalScope3CarbonEmissions: '25', + emissions: '25', }) .then(toHTMLString) @@ -181,7 +284,7 @@ describe('QueriesController', () => { .scope3CarbonConsumptionResponseSubmit(req, '5390af91-c551-4d74-b394-d8ae0805059e', { companyId: '2345789', action: 'success', - totalScope3CarbonEmissions: '25', + emissions: '25', }) .then(toHTMLString) @@ -197,7 +300,7 @@ describe('QueriesController', () => { .scope3CarbonConsumptionResponseSubmit(req, '5390af91-c551-4d74-b394-d8ae0805059e', { companyId: '2345789', action: 'success', - totalScope3CarbonEmissions: '25', + emissions: '25', }) .then(toHTMLString) @@ -217,7 +320,7 @@ describe('QueriesController', () => { .scope3CarbonConsumptionResponseSubmit(req, '5390af91-c551-4d74-b394-d8ae0805059e', { companyId: '2345789', action: 'success', - totalScope3CarbonEmissions: '25', + emissions: '25', }) .then(toHTMLString) diff --git a/src/controllers/queries/index.ts b/src/controllers/queries/index.ts index 0c11afea..125f0987 100644 --- a/src/controllers/queries/index.ts +++ b/src/controllers/queries/index.ts @@ -4,6 +4,7 @@ import { injectable } from 'tsyringe' import pino from 'pino' import { InvalidInputError, NotFoundError } from '../../errors.js' +import { PartialQueryPayload, type PartialQuery } from '../../models/arrays.js' import Database from '../../models/db/index.js' import { ConnectionRow, QueryRow, Where } from '../../models/db/types.js' import { type UUID } from '../../models/strings.js' @@ -246,7 +247,7 @@ export class QueriesController extends HTMLController { * @returns a table of connections for partial query */ @SuccessResponse(200) - @Get('/{queryId}/partial/{companyId}') + @Get('/{queryId}/partial') public async scope3CO2Partial( @Request() req: express.Request, @Path() queryId: UUID, @@ -286,7 +287,7 @@ export class QueriesController extends HTMLController { * @returns - a tabe row for partial query */ @SuccessResponse(200) - @Get('/partial-select/{connectionId}/') + @Get('/partial-select/{connectionId}') public async partialSelect( @Request() req: express.Request, @Path() connectionId: UUID, @@ -311,6 +312,8 @@ export class QueriesController extends HTMLController { /** * Submits the query response page + * @param connections - since table contains only 3 cells this data will need to be + * devided into chunks of size 3 */ @SuccessResponse(200) @Post('/scope-3-carbon-consumption/{queryId}/response') @@ -321,33 +324,63 @@ export class QueriesController extends HTMLController { body: { companyId: UUID action: 'success' - totalScope3CarbonEmissions: string + emissions?: string + partialQuery?: 'on'[] + partialSelect?: 'on'[] + connectionIds?: string[] + productIds?: string[] + quantities?: string[] } ): Promise { - req.log.info('query page requested %j', { queryId, body }) + const { action, companyId, emissions, partialQuery, partialSelect, ...partial } = body + req.log.info('query page requested %j', { body }) - const [connection] = await this.db.get('connection', { id: body.companyId, status: 'verified_both' }, [ + const [connection]: ConnectionRow[] = await this.db.get('connection', { id: companyId, status: 'verified_both' }, [ ['updated_at', 'desc'], ]) if (!connection) { - req.log.warn('invalid input error %j', body) - throw new InvalidInputError(`Invalid connection ${body.companyId}`) + req.log.warn('invalid input error %j', { companyId, action }) + throw new InvalidInputError(`Invalid connection ${companyId}`) } if (!connection.agent_connection_id || connection.status !== 'verified_both') { - req.log.warn('invalid input error %j', body) + req.log.warn('invalid input error %j', { companyId, action, emissions }) throw new InvalidInputError(`Cannot query unverified connection`) } - const [queryRow] = await this.db.get('query', { id: queryId }) + const [queryRow]: QueryRow[] = await this.db.get('query', { id: queryId }) if (!queryRow.response_id) { req.log.warn('missing DRPC response_id to respond to %j', queryRow) - throw new InvalidInputError(`Missing queryId to respond to.`) + throw new InvalidInputError(`Missing response_id to respond to.`) } - const query = { - emissions: body.totalScope3CarbonEmissions, + const partialConnections: PartialQuery[] = [] + if (partial && partialQuery) { + if (!partial.connectionIds || !partial.productIds || !partial.quantities) { + throw new InvalidInputError('missing a property in the request body') + } + req.log.info('processing partial query %j', partial) + const size: number = this.validatePartialQuery(partial) + req.log.debug('partial query has been validated %j', { partial, size }) + + for (let i = 0; i < size; i++) { + partialConnections.push({ + connectionId: partial.connectionIds[i], + productId: partial.productIds[i], + quantity: parseInt(partial.quantities[i]), + }) + req.log.info('partial connection has been formatted %j', partialConnections[i]) + } + + req.log.debug('formatted partial connections %j', partialConnections) + } + + const query: { emissions: string; queryIdForResponse: UUID } = { + emissions: + (emissions as string) || + partialConnections.reduce((out: number, next: PartialQuery) => (out += next.quantity), 0).toString(), queryIdForResponse: queryRow.response_id, } - req.log.debug('query for DRPC response %j', query) + + req.log.debug('query for DRPC response with aggregated emissions %j', query) //send a drpc message with response let rpcResponse: DrpcResponse try { @@ -361,12 +394,12 @@ export class QueriesController extends HTMLController { ) req.log.info('submitting DRPC request %j', maybeResponse) if (!maybeResponse) { - return await this.handleError(req.log, queryRow, connection) + return this.handleError(req.log, queryRow, connection) } rpcResponse = maybeResponse } catch (err) { req.log.warn('DRPC has failed %j', err) - return await this.handleError(req.log, queryRow, connection, undefined, err) + return this.handleError(req.log, queryRow, connection, undefined, err) } const { result, error, id: rpcId } = rpcResponse @@ -379,17 +412,11 @@ export class QueriesController extends HTMLController { result, error, }) - await this.db.update( - 'query', - { id: queryId }, - { - status: 'resolved', - } - ) + await this.db.update('query', { id: queryId }, { status: 'resolved' }) if (!result || error) { req.log.warn('error happened while persisting query_rpc %j', error) - return await this.handleError(req.log, queryRow, connection, rpcId) + return this.handleError(req.log, queryRow, connection, rpcId) } return this.html( @@ -434,6 +461,15 @@ export class QueriesController extends HTMLController { ) } + private validatePartialQuery({ connectionIds: a, productIds: b, quantities: c }: PartialQueryPayload): number { + if (!a || !b || !c) throw new InvalidInputError('empty arrays of data provided') + if (a.length !== b.length || a.length !== c.length || b.length !== c.length) { + throw new InvalidInputError('partial query validation failed, invalid data') + } + + return a.length + } + private async handleError( logger: pino.Logger, query: QueryRow, diff --git a/src/models/__tests__/companyHouseEntity.test.ts b/src/models/__tests__/companyHouseEntity.test.ts index 64fc81ef..7c206b51 100644 --- a/src/models/__tests__/companyHouseEntity.test.ts +++ b/src/models/__tests__/companyHouseEntity.test.ts @@ -48,6 +48,7 @@ describe('companyHouseEntity', () => { const environment = new Env() const companyHouseObject = new CompanyHouseEntity(environment) const response = await companyHouseObject.localCompanyHouseProfile() + expect(response).deep.equal(successResponse) }) }) diff --git a/src/models/__tests__/fixtures/companyHouseFixtures.ts b/src/models/__tests__/fixtures/companyHouseFixtures.ts index 3193982b..82a25490 100644 --- a/src/models/__tests__/fixtures/companyHouseFixtures.ts +++ b/src/models/__tests__/fixtures/companyHouseFixtures.ts @@ -1,6 +1,7 @@ import { CompanyProfile } from '../../companyHouseEntity.js' export const validCompanyNumber = '07964699' +export const secondaryCompanyNumber = '11111111' export const invalidCompanyNumber = '079646992' export const noCompanyNumber = '' @@ -15,3 +16,15 @@ export const successResponse: CompanyProfile = { company_name: 'DIGITAL CATAPULT', company_number: '07964699', } + +export const successResponse2: CompanyProfile = { + registered_office_address: { + address_line_1: 'Flat 3 Nelmes Court, Nelmes Way, Nelmes Way', + postal_code: 'RM11 2QL', + locality: 'Hornchurch', + }, + company_status: 'active', + registered_office_is_in_dispute: false, + company_name: 'CARE ONUS LTD', + company_number: '11111111', +} diff --git a/src/models/__tests__/helpers/mockCompanyHouse.ts b/src/models/__tests__/helpers/mockCompanyHouse.ts index bd9731cc..8ef40d97 100644 --- a/src/models/__tests__/helpers/mockCompanyHouse.ts +++ b/src/models/__tests__/helpers/mockCompanyHouse.ts @@ -4,7 +4,9 @@ import { Env } from '../../../env.js' import { invalidCompanyNumber, noCompanyNumber, + secondaryCompanyNumber, successResponse, + successResponse2, validCompanyNumber, } from '../fixtures/companyHouseFixtures.js' @@ -27,6 +29,14 @@ export function withCompanyHouseMock() { .reply(200, successResponse) .persist() + client + .intercept({ + path: `/company/${secondaryCompanyNumber}`, + method: 'GET', + }) + .reply(200, successResponse2) + .persist() + client .intercept({ path: `/company/${noCompanyNumber}`, diff --git a/src/models/arrays.ts b/src/models/arrays.ts new file mode 100644 index 00000000..615810a8 --- /dev/null +++ b/src/models/arrays.ts @@ -0,0 +1,6 @@ +export type PartialQuery = { connectionId: string; productId: string; quantity: number } +export type PartialQueryPayload = { + connectionIds?: string[] + productIds?: string[] + quantities?: string[] +} diff --git a/src/views/queries/__tests__/partial-query.test.ts.snap b/src/views/queries/__tests__/partial-query.test.ts.snap index 821ed4fb..4b0eeb6a 100644 --- a/src/views/queries/__tests__/partial-query.test.ts.snap +++ b/src/views/queries/__tests__/partial-query.test.ts.snap @@ -1,5 +1,5 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`Partial Query if partial is set to false does not render connections list and leaves quantity input enabled 1`] = `"Veritable - Select Company

Select Company To Send Your Query To

Scope 3 Carbon Consumption

Provide the total scope 3 carbon consumption for the specified products / component.

If you do not have all the required information, please forward this query to your suppliers, to aggregate their responses before submitting the final total.

What are the total Scope 3 carbon emissions for the product/component below?

*\\" hx-target=\\"main\\" hx-swap=\\"innerHTML\\">

Product ID: product-id-test
Quantity: 2

*If partial response checkbox is ticked, you must share this query with another supplier in your network, where your responses will be aggregated.


"`; +exports[`Partial Query if partial is set to false does not render connections list and leaves quantity input enabled 1`] = `"Veritable - Select Company

Select Company To Send Your Query To

Scope 3 Carbon Consumption

Provide the total scope 3 carbon consumption for the specified products / component.

If you do not have all the required information, please forward this query to your suppliers, to aggregate their responses before submitting the final total.

What are the total Scope 3 carbon emissions for the product/component below?

*\\" hx-include=\\"[name='quantity'], [name='companyId'], [name='productId']\\" hx-target=\\"main\\" hx-swap=\\"innerHTML\\">

Product ID: product-id-test
Quantity: 2

*If partial response checkbox is ticked, you must share this query with another supplier in your network, where your responses will be aggregated.


"`; -exports[`Partial Query renders partial query table with connections that are verified 1`] = `"Veritable - Select Company

Select Company To Send Your Query To

Scope 3 Carbon Consumption

Provide the total scope 3 carbon consumption for the specified products / component.

If you do not have all the required information, please forward this query to your suppliers, to aggregate their responses before submitting the final total.

What are the total Scope 3 carbon emissions for the product/component below?

*\\" hx-target=\\"main\\" hx-swap=\\"innerHTML\\">

Product ID: product-id-test
Quantity: 2

*If partial response checkbox is ticked, you must share this query with another supplier in your network, where your responses will be aggregated.

SelectCompany NameProduct IDQuantity
I own you
I own you
I own you
I own you

"`; +exports[`Partial Query renders partial query table with connections that are verified 1`] = `"Veritable - Select Company

Select Company To Send Your Query To

Scope 3 Carbon Consumption

Provide the total scope 3 carbon consumption for the specified products / component.

If you do not have all the required information, please forward this query to your suppliers, to aggregate their responses before submitting the final total.

What are the total Scope 3 carbon emissions for the product/component below?

*\\" hx-include=\\"[name='quantity'], [name='companyId'], [name='productId']\\" hx-target=\\"main\\" hx-swap=\\"innerHTML\\">

Product ID: product-id-test
Quantity: 2

*If partial response checkbox is ticked, you must share this query with another supplier in your network, where your responses will be aggregated.

SelectCompany NameProduct IDQuantity
I own you
I own you
I own you
I own you

"`; diff --git a/src/views/queries/queryResponses/respondToScope3.tsx b/src/views/queries/queryResponses/respondToScope3.tsx index f1be6887..f860fe91 100644 --- a/src/views/queries/queryResponses/respondToScope3.tsx +++ b/src/views/queries/queryResponses/respondToScope3.tsx @@ -87,7 +87,7 @@ export default class Scope3CarbonConsumptionResponseTemplates { @@ -123,7 +124,7 @@ export default class Scope3CarbonConsumptionResponseTemplates { { return ( - + {Html.escapeHtml(props.company_name)}