Skip to content

Commit

Permalink
PP-12100 Allow user to apply filters after all services search timeout
Browse files Browse the repository at this point in the history
- Add new error for a ledger timeout on all services transactions search
- Link from the error page to the all services transaction search page,  without the automatic search, to avoid a perpetual timeout
- Refactor to share the code that populates the model
- Update unit tests
- Update cypress tests
  • Loading branch information
james-peacock-gds committed Apr 11, 2024
1 parent 9ca24f5 commit 69cf850
Show file tree
Hide file tree
Showing 19 changed files with 486 additions and 106 deletions.
11 changes: 10 additions & 1 deletion .secrets.baseline
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,15 @@
"line_number": 7
}
],
"test/cypress/integration/all-service-transactions/all-service-transactions-no-autosearch.cy.js": [
{
"type": "Hex High Entropy String",
"filename": "test/cypress/integration/all-service-transactions/all-service-transactions-no-autosearch.cy.js",
"hashed_secret": "7bb363901b8a686a4d3e1949032e469901cddaa8",
"is_verified": false,
"line_number": 7
}
],
"test/cypress/integration/dashboard/dashboard-disabled-account-banner.cy.js": [
{
"type": "Hex High Entropy String",
Expand Down Expand Up @@ -903,5 +912,5 @@
}
]
},
"generated_at": "2024-04-08T21:15:15Z"
"generated_at": "2024-04-10T15:39:24Z"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
'use strict'

const { response } = require('../../utils/response')
const { populateModel } = require('./populateModel')
const { getFilters } = require('../../utils/filters')
const permissions = require('../../utils/permissions')

module.exports = async function getTransactionsForAllServicesNoSearch (req, res, next) {
const filters = getFilters(req)
const { statusFilter } = req.params
const filterLiveAccounts = statusFilter !== 'test'
const userPermittedAccountsSummary = await permissions.getGatewayAccountsFor(req.user, filterLiveAccounts, 'transactions:read')
const model = await populateModel(req, { results: [] }, filters, null, filterLiveAccounts, userPermittedAccountsSummary);
model.allServicesTimeout = true
try {
return response(req, res, 'transactions/index', model)
} catch (err) {
next(new Error('Unable to fetch transaction information'))
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
const sinon = require('sinon')
const proxyquire = require('proxyquire')
const User = require('../../models/User.class')
const userFixtures = require('../../../test/fixtures/user.fixtures')
const gatewayAccountFixture = require('../../../test/fixtures/gateway-account.fixtures')
const Service = require('../../models/Service.class')
const serviceFixtures = require('../../../test/fixtures/service.fixtures')

describe('All service transactions no autosearch - GET', () => {
let req, res, next
const user = new User(userFixtures.validUserResponse())
const service = new Service(serviceFixtures.validServiceResponse({}))
let userPermittedAccountsSummary = {
gatewayAccountIds: [31],
headers: { shouldGetStripeHeaders: true, shouldGetMotoHeaders: true },
hasLiveAccounts: false,
hasStripeAccount: true,
hasTestStripeAccount: false
}
const modelMock = sinon.spy(() => ({
isStripeAccount: true,
filterLiveAccounts: true
}))
const responseMock = sinon.spy(() => null)
const filterMock = sinon.spy(() => ['a-filter'])

beforeEach(() => {
req = {
account: gatewayAccountFixture.validGatewayAccount({ 'payment_provider': 'stripe' }),
flash: sinon.spy(),
service: service,
user: user,
params: {},
url: 'http://selfservice/all-servce-transactions',
session: {},
headers: {
'x-request-id': 'correlation-id'
}
}
res = {
render: sinon.spy()
}
next = sinon.spy()
})

describe('Stripe account', () => {
it('should return a response with a model with `allServicesTimeout: true`', async () => {
await getController()(req, res, next)

sinon.assert.calledWith(filterMock, req)
sinon.assert.calledWith(modelMock, req, { results: [] }, ['a-filter'], null, true, userPermittedAccountsSummary)

sinon.assert.calledWith(responseMock, req, res, 'transactions/index', {
isStripeAccount: true,
filterLiveAccounts: true,
allServicesTimeout: true
})
})
})

function getController () {
return proxyquire('./all-service-transactions-no-autosearch.controller', {
'../../utils/permissions': {
getGatewayAccountsFor: sinon.spy(() => Promise.resolve(userPermittedAccountsSummary))
},
'./populateModel': {
populateModel: modelMock
},
'../../utils/response': {
response: responseMock
},
'../../utils/filters.js': {
getFilters: filterMock
}
})
}
})
50 changes: 12 additions & 38 deletions app/controllers/all-service-transactions/get.controller.js
Original file line number Diff line number Diff line change
@@ -1,19 +1,16 @@
'use strict'

const _ = require('lodash')
const url = require('url')

const { response } = require('../../utils/response')
const { ConnectorClient } = require('../../services/clients/connector.client.js')
const transactionService = require('../../services/transaction.service')
const { buildPaymentList } = require('../../utils/transaction-view.js')
const permissions = require('../../utils/permissions')
const { getFilters, describeFilters } = require('../../utils/filters.js')
const { getFilters } = require('../../utils/filters.js')
const paths = require('../../paths')
const states = require('../../utils/states')
const client = new ConnectorClient(process.env.CONNECTOR_URL)

const logger = require('../../utils/logger')(__filename)
const { NoServicesWithPermissionError } = require('../../errors')
const { populateModel } = require('./populateModel.js')

module.exports = async function getTransactionsForAllServices (req, res, next) {
const filters = getFilters(req)
Expand All @@ -22,9 +19,9 @@ module.exports = async function getTransactionsForAllServices (req, res, next) {
// default behaviour should be live
const { statusFilter } = req.params
const filterLiveAccounts = statusFilter !== 'test'

req.session.filters = url.parse(req.url).query
req.session.allServicesTransactionsStatusFilter = statusFilter

try {
const userPermittedAccountsSummary = await permissions.getGatewayAccountsFor(req.user, filterLiveAccounts, 'transactions:read')

Expand All @@ -37,39 +34,16 @@ module.exports = async function getTransactionsForAllServices (req, res, next) {
if (!userPermittedAccountsSummary.gatewayAccountIds.length) {
return next(new NoServicesWithPermissionError('You do not have any associated services with rights to view these transactions.'))
}
const searchResultOutput = await transactionService.search(userPermittedAccountsSummary.gatewayAccountIds, filters.result)
const cardTypes = await client.getAllCardTypes()
const downloadRoute = filterLiveAccounts ? paths.allServiceTransactions.download : paths.formattedPathFor(paths.allServiceTransactions.downloadStatusFilter, 'test')
const model = buildPaymentList(searchResultOutput, cardTypes, null, filters.result, filters.dateRangeState, downloadRoute, req.session.backPath)
delete req.session.backPath
model.search_path = filterLiveAccounts ? paths.allServiceTransactions.index : paths.formattedPathFor(paths.allServiceTransactions.indexStatusFilter, 'test')
model.filtersDescription = describeFilters(filters.result)
model.eventStates = states.allDisplayStateSelectorObjects(userPermittedAccountsSummary.hasStripeAccount)
.map(state => {
return {
value: state.key,
text: state.name,
selected: filters.result.selectedStates && filters.result.selectedStates.includes(state.name)
}
})
model.eventStates.unshift({ value: '', text: 'Any', selected: false })

model.stateFiltersFriendly = model.eventStates
.filter(state => state.selected)
.map(state => state.text)
.join(', ')
if (_.has(filters.result, 'brand')) {
model.cardBrands.forEach(brand => {
brand.selected = filters.result.brand.includes(brand.value)
})
let searchResultOutput
try {
searchResultOutput = await transactionService.search(userPermittedAccountsSummary.gatewayAccountIds, filters.result, true)
} catch (error) {
return next(error)
}
model.clearRedirect = model.search_path
model.isStripeAccount = userPermittedAccountsSummary.headers.shouldGetStripeHeaders
model.allServiceTransactions = true
model.filterLiveAccounts = filterLiveAccounts
model.hasLiveAccounts = userPermittedAccountsSummary.hasLiveAccounts
model.recurringEnabled = userPermittedAccountsSummary.hasRecurringAccount
model.isExperimentalFeaturesEnabled = req.user.serviceRoles.some(serviceRole => serviceRole.service.experimentalFeaturesEnabled)

const downloadRoute = filterLiveAccounts ? paths.allServiceTransactions.download : paths.formattedPathFor(paths.allServiceTransactions.downloadStatusFilter, 'test')
const model = await populateModel(req, searchResultOutput, filters, downloadRoute, filterLiveAccounts, userPermittedAccountsSummary)

return response(req, res, 'transactions/index', model)
} catch (err) {
Expand Down
76 changes: 24 additions & 52 deletions app/controllers/all-service-transactions/get.controller.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ const ledgerTransactionFixture = require('../../../test/fixtures/ledger-transact

describe('All service transactions - GET', () => {
let req, res, next
let allDisplayStateSelectorObjectsMock
const user = new User(userFixtures.validUserResponse())
const service = new Service(serviceFixtures.validServiceResponse({}))
const transactionSearchResponse = ledgerTransactionFixture.validTransactionSearchResponse(
Expand All @@ -21,6 +20,12 @@ describe('All service transactions - GET', () => {
hasStripeAccount: true,
hasTestStripeAccount: false
}
const modelMock = sinon.spy(() => ({
isStripeAccount: true,
filterLiveAccounts: true
}))
const responseMock = sinon.spy(() => null)
const filterMock = sinon.spy(() => ['a-filter'])

beforeEach(() => {
req = {
Expand All @@ -39,59 +44,19 @@ describe('All service transactions - GET', () => {
render: sinon.spy()
}
next = sinon.spy()
allDisplayStateSelectorObjectsMock = sinon.spy(() => ([{}]))
})

describe('Stripe account', () => {
it('should get dispute states', async () => {
userPermittedAccountsSummary.hasStripeAccount = true

it('should return a response with correct model and filters', async () => {
await getController()(req, res, next)

sinon.assert.calledWith(allDisplayStateSelectorObjectsMock, true)
sinon.assert.called(res.render)
})
})
sinon.assert.calledWith(filterMock, req)
sinon.assert.calledWith(modelMock, req, transactionSearchResponse, ['a-filter'], 'download-path', true, userPermittedAccountsSummary)

describe('Error results when from date is later than to date', () => {
const request = {
account: gatewayAccountFixture.validGatewayAccount({ 'payment_provider': 'stripe' }),
service: service,
user: user,
params: {},
query: {
fromDate: '03/5/2018',
fromTime: '01:00:00',
toDate: '01/5/2018',
toTime: '01:00:00'
},
url: 'http://selfservice/all-servce-transactions',
session: {}
}
const response = {
render: sinon.spy()
}

it('should return the response with the date-range failing validation with empty transaction results indicator', async () => {
await getController()(request, response, next)

sinon.assert.calledWith(response.render, 'transactions/index', sinon.match({
'isInvalidDateRange': true,
'hasResults': false,
'fromDateParam': '03/5/2018',
'toDateParam': '01/5/2018'
}))
})
})

describe('Non stripe account', () => {
it('should NOT get dispute states', async () => {
userPermittedAccountsSummary.hasStripeAccount = false
req.params.statusFilter = 'test'
await getController()(req, res, next)

sinon.assert.calledWith(allDisplayStateSelectorObjectsMock, false)
sinon.assert.called(res.render)
sinon.assert.calledWith(responseMock, req, res, 'transactions/index', {
isStripeAccount: true,
filterLiveAccounts: true
})
})
})

Expand All @@ -103,11 +68,18 @@ describe('All service transactions - GET', () => {
'../../services/transaction.service': {
search: sinon.spy(() => Promise.resolve(transactionSearchResponse))
},
'../../services/clients/connector.client.js': {
ConnectorClient: class {async getAllCardTypes () { return {} }}
'./populateModel': {
populateModel: modelMock
},
'../../utils/response': {
response: responseMock
},
'../../paths': {
allServiceTransactions: { download: 'download-path' },
formattedPathFor: () => 'formatted-path'
},
'../../utils/states': {
allDisplayStateSelectorObjects: allDisplayStateSelectorObjectsMock
'../../utils/filters.js': {
getFilters: filterMock
}
})
}
Expand Down
4 changes: 3 additions & 1 deletion app/controllers/all-service-transactions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@

const getController = require('./get.controller')
const downloadTransactions = require('./download-transactions.controller')
const noAutosearchTransactions = require('./all-service-transactions-no-autosearch.controller')

module.exports = {
getController,
downloadTransactions
downloadTransactions,
noAutosearchTransactions
}
47 changes: 47 additions & 0 deletions app/controllers/all-service-transactions/populateModel.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
const { buildPaymentList } = require('../../utils/transaction-view')
const paths = require('../../paths')
const { describeFilters } = require('../../utils/filters')
const states = require('../../utils/states')
const _ = require('lodash')
const { ConnectorClient } = require('../../services/clients/connector.client')
const client = new ConnectorClient(process.env.CONNECTOR_URL)

async function populateModel (req, searchResultOutput, filters, downloadRoute, filterLiveAccounts, userPermittedAccountsSummary) {
const cardTypes = await client.getAllCardTypes()
const model = buildPaymentList(searchResultOutput, cardTypes, null, filters.result, filters.dateRangeState, downloadRoute, req.session.backPath)
delete req.session.backPath
model.search_path = filterLiveAccounts ? paths.allServiceTransactions.index : paths.formattedPathFor(paths.allServiceTransactions.indexStatusFilter, 'test')
model.filtersDescription = describeFilters(filters.result)
model.eventStates = states.allDisplayStateSelectorObjects(userPermittedAccountsSummary.hasStripeAccount)
.map(state => {
return {
value: state.key,
text: state.name,
selected: filters.result.selectedStates && filters.result.selectedStates.includes(state.name)
}
})
model.eventStates.unshift({ value: '', text: 'Any', selected: false })

model.stateFiltersFriendly = model.eventStates
.filter(state => state.selected)
.map(state => state.text)
.join(', ')

if (_.has(filters.result, 'brand')) {
model.cardBrands.forEach(brand => {
brand.selected = filters.result.brand.includes(brand.value)
})
}

model.clearRedirect = model.search_path
model.isStripeAccount = userPermittedAccountsSummary.headers.shouldGetStripeHeaders
model.allServiceTransactions = true
model.filterLiveAccounts = filterLiveAccounts
model.hasLiveAccounts = userPermittedAccountsSummary.hasLiveAccounts
model.recurringEnabled = userPermittedAccountsSummary.hasRecurringAccount
model.isExperimentalFeaturesEnabled = req.user.serviceRoles.some(serviceRole => serviceRole.service.experimentalFeaturesEnabled)

return model
}

module.exports = { populateModel }
Loading

0 comments on commit 69cf850

Please sign in to comment.