Skip to content

Commit

Permalink
Refactor supplementary test code (#68)
Browse files Browse the repository at this point in the history
https://eaflood.atlassian.net/browse/WATER-3854

We are about to [Update "create bill run" endpoint to create a bill run](#56). Currently, everything is happening in the controller whereas our convention is to do everything in a service and keep our controllers ['skinny'](https://blog.makersacademy.com/forget-fat-models-its-time-for-skinny-controllers-and-skinny-models-a9b84ec481b7).

So, we need a service, something like `SupplementaryService`. But oh no! It already exists.

This change is about refactoring the code currently in `SupplementaryService` to a more explicit testing version so we can free up the namespace. Whilst we're at it we also rename the `/test` path to `check/` as everyone was getting confused by seeing `test/` folders in various places in the code 😁
  • Loading branch information
Cruikshanks authored Dec 21, 2022
1 parent a95dc35 commit dcf09a5
Show file tree
Hide file tree
Showing 9 changed files with 43 additions and 46 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@
* @module SupplementaryController
*/

const SupplementaryService = require('../../services/supplementary-billing/supplementary.service.js')
const SupplementaryDataService = require('../../services/check/supplementary-data.service.js')

async function index (request, h) {
const result = await SupplementaryService.go(request.query.region)
const result = await SupplementaryDataService.go(request.query.region)

return h.response(result).code(200)
}
Expand Down
4 changes: 2 additions & 2 deletions app/plugins/router.plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,18 @@

const AssetRoutes = require('../routes/assets.routes.js')
const BillRunRoutes = require('../routes/bill-runs.routes')
const CheckRoutes = require('../routes/check.routes.js')
const FilterRoutesService = require('../services/plugins/filter-routes.service.js')
const HealthRoutes = require('../routes/health.routes.js')
const RootRoutes = require('../routes/root.routes.js')
const TestRoutes = require('../routes/test.routes.js')

const AirbrakeConfig = require('../../config/airbrake.config.js')

const routes = [
...RootRoutes,
...AssetRoutes,
...HealthRoutes,
...TestRoutes,
...CheckRoutes,
...BillRunRoutes
]

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
'use strict'

/**
* Formats responses from the `SupplementaryService`
* @module SupplementaryPresenter
* Formats responses from the `SupplementaryDataService`
* @module SupplementaryDataPresenter
*/

function go (data) {
Expand Down
16 changes: 16 additions & 0 deletions app/routes/check.routes.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
'use strict'

const SupplementaryController = require('../controllers/check/supplementary.controller.js')

const routes = [
{
method: 'GET',
path: '/check/supplementary',
handler: SupplementaryController.index,
options: {
description: 'Used by the delivery team to check the SROC supplementary billing logic'
}
}
]

module.exports = routes
16 changes: 0 additions & 16 deletions app/routes/test.routes.js

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,18 +1,15 @@
'use strict'

/**
* Determines the billing periods, licences and charge versions used to generate an SROC supplementary bill run
*
* WIP: This is currently being used to generate testing data to confirm we are understanding SROC supplementary
* billing. We intend to refactor things so that the service starts representing what is actually required.
* @module SupplementaryService
* Confirms the billing periods, licences and charge versions used to generate an SROC supplementary bill run
* @module SupplementaryDataService
*/

const BillingPeriodService = require('./billing-period.service.js')
const FetchChargeVersionsService = require('./fetch-charge-versions.service.js')
const FetchLicencesService = require('./fetch-licences.service.js')
const FetchRegionService = require('./fetch-region.service.js')
const SupplementaryPresenter = require('../../presenters/supplementary.presenter.js')
const BillingPeriodService = require('../supplementary-billing/billing-period.service.js')
const FetchChargeVersionsService = require('../supplementary-billing/fetch-charge-versions.service.js')
const FetchLicencesService = require('../supplementary-billing/fetch-licences.service.js')
const FetchRegionService = require('../supplementary-billing/fetch-region.service.js')
const SupplementaryDataPresenter = require('../../presenters/check/supplementary-data.presenter.js')

async function go (naldRegionId) {
const region = await FetchRegionService.go(naldRegionId)
Expand All @@ -29,7 +26,7 @@ async function go (naldRegionId) {
}

function _response (data) {
return SupplementaryPresenter.go(data)
return SupplementaryDataPresenter.go(data)
}

module.exports = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const { describe, it, beforeEach, after } = exports.lab = Lab.script()
const { expect } = Code

// Things we need to stub
const SupplementaryService = require('../../../app/services/supplementary-billing/supplementary.service.js')
const SupplementaryDataService = require('../../../app/services/check/supplementary-data.service.js')

// For running our service
const { init } = require('../../../app/server.js')
Expand All @@ -25,16 +25,16 @@ describe('Supplementary controller', () => {
Sinon.restore()
})

describe('GET /test/supplementary', () => {
describe('GET /check/supplementary', () => {
const options = {
method: 'GET',
url: '/test/supplementary?region=9'
url: '/check/supplementary?region=9'
}

let response

beforeEach(async () => {
Sinon.stub(SupplementaryService, 'go').resolves({ billingPeriods: [], licences: [], chargeVersions: [] })
Sinon.stub(SupplementaryDataService, 'go').resolves({ billingPeriods: [], licences: [], chargeVersions: [] })

response = await server.inject(options)
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const { describe, it, beforeEach } = exports.lab = Lab.script()
const { expect } = Code

// Thing under test
const SupplementaryPresenter = require('../../app/presenters/supplementary.presenter.js')
const SupplementaryDataPresenter = require('../../../app/presenters/check/supplementary-data.presenter.js')

describe('Supplementary presenter', () => {
let data
Expand Down Expand Up @@ -44,7 +44,7 @@ describe('Supplementary presenter', () => {
})

it('correctly presents the data', () => {
const result = SupplementaryPresenter.go(data)
const result = SupplementaryDataPresenter.go(data)

expect(result.billingPeriods).to.have.length(1)
expect(result.billingPeriods[0]).to.equal(data.billingPeriods[0])
Expand All @@ -70,7 +70,7 @@ describe('Supplementary presenter', () => {
})

it('correctly presents the data', () => {
const result = SupplementaryPresenter.go(data)
const result = SupplementaryDataPresenter.go(data)

expect(result.billingPeriods).to.be.empty()
expect(result.licences).to.be.empty()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const FetchLicencesService = require('../../../app/services/supplementary-billin
const FetchRegionService = require('../../../app/services/supplementary-billing/fetch-region.service.js')

// Thing under test
const SupplementaryService = require('../../../app/services/supplementary-billing/supplementary.service.js')
const SupplementaryDataService = require('../../../app/services/check/supplementary-data.service.js')

describe('Supplementary service', () => {
const naldRegionId = 9
Expand All @@ -40,7 +40,7 @@ describe('Supplementary service', () => {
})

it('always includes the current billing period', async () => {
const result = await SupplementaryService.go(naldRegionId)
const result = await SupplementaryDataService.go(naldRegionId)

expect(result.billingPeriods.length).to.equal(1)
expect(result.billingPeriods[0]).to.equal(currentBillingPeriod)
Expand All @@ -63,7 +63,7 @@ describe('Supplementary service', () => {
})

it('returns the matching licences', async () => {
const result = await SupplementaryService.go(naldRegionId)
const result = await SupplementaryDataService.go(naldRegionId)

expect(result.licences.length).to.equal(1)
expect(result.licences[0].licenceId).to.equal(testRecords[0].licenceId)
Expand All @@ -76,7 +76,7 @@ describe('Supplementary service', () => {
})

it('returns no results', async () => {
const result = await SupplementaryService.go(naldRegionId)
const result = await SupplementaryDataService.go(naldRegionId)

expect(result.licences).to.be.empty()
})
Expand All @@ -102,7 +102,7 @@ describe('Supplementary service', () => {
})

it('returns the matching charge versions', async () => {
const result = await SupplementaryService.go(naldRegionId)
const result = await SupplementaryDataService.go(naldRegionId)

expect(result.chargeVersions.length).to.equal(1)
expect(result.chargeVersions[0].chargeVersionId).to.equal(testRecords[0].chargeVersionId)
Expand All @@ -115,7 +115,7 @@ describe('Supplementary service', () => {
})

it('returns no results', async () => {
const result = await SupplementaryService.go(naldRegionId)
const result = await SupplementaryDataService.go(naldRegionId)

expect(result.chargeVersions).to.be.empty()
})
Expand Down

0 comments on commit dcf09a5

Please sign in to comment.