Skip to content

Commit

Permalink
Restructure 'seeding' to be Knex based (#1230)
Browse files Browse the repository at this point in the history
https://eaflood.atlassian.net/browse/WATER-4575

We have been working on replicating how the [water-abstraction-import](https://github.com/DEFRA/water-abstraction-import) service imports a licence. See [Import Licence versions](#1195) for example.

This is in readiness for moving to ReSP managing licences rather than NALD.

What it exposed was that our test data setup needed expanding. The import relies on 'real' reference data being present. So, it makes testing easier if this is the case in our test database when writing unit tests.

We started with [create wrapper for licence-version-purpose-condition table](#1181) which added the ability to seed licence-purpose condition types. This was then expanded by [Add new test reference data seeders](#1206) which added some more reference data, for example, purposes.

The issue we've had is that the existing changes are straddling two different mechanisms. We've included them as a seed file in our [Knex seeding capabilities](https://knexjs.org/guide/migrations.html#seed-files). This is reference data, so why not seed it?!

And we are seeding it whenever we perform a database clean. Our intent is to have real reference data when unit testing, and until we've removed calls to `DatabaseSupport.clean()` from all our tests, we need to do this each time. That has led to the logic and data itself living in the `test/` folder.

There is a conceptual problem with this: something we depend on 'for real' lives in our `test/` folder. Plus, the functionality is split into 2 places.

It also means our `test/support/seeders` have a mix of types. Original 'unit test seeders' that were intended to help move bulky test setup away from within the test itself, with 'knex seeders', things we'll also call when we run `npm run seed` against the non-test DB.

So, on reflection, we felt the need to carry out a little restructuring. _All_ the 'seeding' functionality is moved to `db/seeds` but with the ability to still call it from `clean()` until such time as we have updated all tests and can drop the command.

While we're doing this housekeeping, we also update our existing user seed to match the conventions that have been introduced while also re-implementing some existing ones, for example, preferring **Objection.js** to raw SQL queries.

The end result is that we can seed both the real and test databases with reference data, even when a database already has existing records, and this functionality will live in one place and be built on **Knex**.

But there's more!

Having made these changes to ensure both the real and test databases are correctly populated with reference data, it no longer made any sense to be adding new reference records within our tests.

So, we updated the relevant test helpers to provide a `select()` in place of the `add()` method. This means you can select the details of a reference record without having to first create or query for it.

We then went through the affected tests and 'test-seeders' and swapped calls to `Helper.add()` with `Helper.select()`. We also performed some housekeeping on some of the tests we touched.
  • Loading branch information
Cruikshanks authored Aug 7, 2024
1 parent 8a21e90 commit e8619c5
Show file tree
Hide file tree
Showing 112 changed files with 2,359 additions and 2,289 deletions.
1 change: 1 addition & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ jobs:
POSTGRES_DB_TEST: wabs_system_test
ENVIRONMENT: dev
COOKIE_SECRET: 1a1028df-a2af-468a-929b-e3a274be1208
DEFAULT_USER_PASSWORD: P@55word

# Service containers to run with `runner-job`
services:
Expand Down
14 changes: 0 additions & 14 deletions app/services/data/load/load.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,7 @@ const CompanyContactHelper = require('../../../../test/support/helpers/company-c
const CompanyHelper = require('../../../../test/support/helpers/company.helper.js')
const ContactHelper = require('../../../../test/support/helpers/contact.helper.js')
const EventHelper = require('../../../../test/support/helpers/event.helper.js')
const FinancialAgreementHelper = require('../../../../test/support/helpers/financial-agreement.helper.js')
const GaugingStationHelper = require('../../../../test/support/helpers/gauging-station.helper.js')
const GroupRoleHelper = require('../../../../test/support/helpers/group-role.helper.js')
const GroupHelper = require('../../../../test/support/helpers/group.helper.js')
const LicenceAgreementHelper = require('../../../../test/support/helpers/licence-agreement.helper.js')
const LicenceDocumentHeaderHelper = require('../../../../test/support/helpers/licence-document-header.helper.js')
const LicenceDocumentRoleHelper = require('../../../../test/support/helpers/licence-document-role.helper.js')
Expand All @@ -40,14 +37,11 @@ const LicenceEntityHelper = require('../../../../test/support/helpers/licence-en
const LicenceGaugingStationHelper = require('../../../../test/support/helpers/licence-gauging-station.helper.js')
const LicenceRoleHelper = require('../../../../test/support/helpers/licence-role.helper.js')
const LicenceSupplementaryYearHelper = require('../../../../test/support/helpers/licence-supplementary-year.helper.js')
const LicenceVersionPurposeConditionTypeHelper = require('../../../../test/support/helpers/licence-version-purpose-condition-type.helper.js')
const LicenceVersionPurposeConditionHelper = require('../../../../test/support/helpers/licence-version-purpose-condition.helper.js')
const LicenceVersionPurposeHelper = require('../../../../test/support/helpers/licence-version-purpose.helper.js')
const LicenceVersionHelper = require('../../../../test/support/helpers/licence-version.helper.js')
const LicenceHelper = require('../../../../test/support/helpers/licence.helper.js')
const PermitLicenceHelper = require('../../../../test/support/helpers/permit-licence.helper.js')
const PurposeHelper = require('../../../../test/support/helpers/purpose.helper.js')
const RegionHelper = require('../../../../test/support/helpers/region.helper.js')
const ReturnLogHelper = require('../../../../test/support/helpers/return-log.helper.js')
const ReturnRequirementPointHelper = require('../../../../test/support/helpers/return-requirement-point.helper.js')
const ReturnRequirementPurposeHelper = require('../../../../test/support/helpers/return-requirement-purpose.helper.js')
Expand All @@ -61,7 +55,6 @@ const ReviewChargeReferenceHelper = require('../../../../test/support/helpers/re
const ReviewChargeVersionHelper = require('../../../../test/support/helpers/review-charge-version.helper.js')
const ReviewLicenceHelper = require('../../../../test/support/helpers/review-licence.helper.js')
const ReviewReturnHelper = require('../../../../test/support/helpers/review-return.helper.js')
const RoleHelper = require('../../../../test/support/helpers/role.helper.js')
const ScheduledNotificationHelper = require('../../../../test/support/helpers/scheduled-notification.helper.js')
const SessionHelper = require('../../../../test/support/helpers/session.helper.js')
const TransactionHelper = require('../../../../test/support/helpers/transaction.helper.js')
Expand Down Expand Up @@ -92,10 +85,7 @@ const LOAD_HELPERS = {
companies: { helper: CompanyHelper, test: true, legacy: { schema: 'crm_v2', table: 'companies', id: 'company_id' } },
contacts: { helper: ContactHelper, test: true, legacy: { schema: 'crm_v2', table: 'contacts', id: 'contact_id' } },
events: { helper: EventHelper, test: false },
financialAgreements: { helper: FinancialAgreementHelper, test: true, legacy: { schema: 'water', table: 'financial_agreement_types', id: 'financial_agreement_type_id' } },
gaugingStations: { helper: GaugingStationHelper, test: true, legacy: { schema: 'water', table: 'gauging_stations', id: 'gauging_station_id' } },
groupRoles: { helper: GroupRoleHelper, test: false },
groups: { helper: GroupHelper, test: false },
licenceAgreements: { helper: LicenceAgreementHelper, test: true, legacy: { schema: 'water', table: 'licence_agreements', id: 'licence_agreement_id' } },
licenceDocumentHeaders: { helper: LicenceDocumentHeaderHelper, test: false },
licenceDocumentRoles: { helper: LicenceDocumentRoleHelper, test: true, legacy: { schema: 'crm_v2', table: 'document_roles', id: 'document_role_id' } },
Expand All @@ -105,14 +95,11 @@ const LOAD_HELPERS = {
licenceGaugingStations: { helper: LicenceGaugingStationHelper, test: true, legacy: { schema: 'water', table: 'licence_gauging_stations', id: 'licence_gauging_station_id' } },
licenceRoles: { helper: LicenceRoleHelper, test: false },
LicenceSupplementaryYears: { helper: LicenceSupplementaryYearHelper, test: false },
licenceVersionPurposeConditionTypes: { helper: LicenceVersionPurposeConditionTypeHelper, test: false },
licenceVersionPurposeConditions: { helper: LicenceVersionPurposeConditionHelper, test: false },
licenceVersionPurposes: { helper: LicenceVersionPurposeHelper, test: true, legacy: { schema: 'water', table: 'licence_version_purposes', id: 'licence_version_purpose_id' } },
licenceVersions: { helper: LicenceVersionHelper, test: true, legacy: { schema: 'water', table: 'licence_versions', id: 'licence_version_id' } },
licences: { helper: LicenceHelper, test: true, legacy: { schema: 'water', table: 'licences', id: 'licence_id' } },
permitLicences: { helper: PermitLicenceHelper, test: false },
purposes: { helper: PurposeHelper, test: true, legacy: { schema: 'water', table: 'purposes_uses', id: 'purpose_use_id' } },
regions: { helper: RegionHelper, test: true, legacy: { schema: 'water', table: 'regions', id: 'region_id' } },
returnLogs: { helper: ReturnLogHelper, test: true, legacy: { schema: 'returns', table: 'returns', id: 'return_id' } },
returnRequirementPoints: { helper: ReturnRequirementPointHelper, test: false },
returnRequirementPurposes: { helper: ReturnRequirementPurposeHelper, test: false },
Expand All @@ -126,7 +113,6 @@ const LOAD_HELPERS = {
reviewChargeVersions: { helper: ReviewChargeVersionHelper, test: false },
reviewLicences: { helper: ReviewLicenceHelper, test: false },
reviewReturns: { helper: ReviewReturnHelper, test: false },
roles: { helper: RoleHelper, test: false },
scheduledNotifications: { helper: ScheduledNotificationHelper },
sessions: { helper: SessionHelper, test: false },
transactions: { helper: TransactionHelper, test: false },
Expand Down
66 changes: 66 additions & 0 deletions db/seeds/01-regions.seed.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
'use strict'

const { db } = require('../db.js')

const { timestampForPostgres } = require('../../app/lib/general.lib.js')
const { data: regions } = require('./data/regions.js')
const RegionModel = require('../../app/models/region.model.js')

const ServerConfig = require('../../config/server.config.js')

async function seed () {
for (const region of regions) {
const exists = await _exists(region)

if (exists) {
await _update(region)
} else {
await _insert(region)
}
}
}

async function _applyTestFlag (region, id) {
if (region.name !== 'Test') {
return null
}

return db('regions').withSchema('water').update('isTest', true).where('regionId', id)
}

async function _exists (region) {
const { chargeRegionId, naldRegionId } = region

const result = await RegionModel.query()
.select('id')
.where('chargeRegionId', chargeRegionId)
.andWhere('naldRegionId', naldRegionId)
.limit(1)
.first()

return !!result
}

async function _insert (region) {
// The Test region is only intended to be seeded in our non-production environments
if (region.name === 'Test' && ServerConfig.environment === 'production') {
return
}

const result = await RegionModel.query().insert(region)

return _applyTestFlag(region, result.id)
}

async function _update (region) {
const { chargeRegionId, displayName, naldRegionId, name } = region

return RegionModel.query()
.patch({ displayName, name, updatedAt: timestampForPostgres() })
.where('chargeRegionId', chargeRegionId)
.andWhere('naldRegionId', naldRegionId)
}

module.exports = {
seed
}
159 changes: 0 additions & 159 deletions db/seeds/01-users.js

This file was deleted.

22 changes: 22 additions & 0 deletions db/seeds/02-purposes.seed.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
'use strict'

const { timestampForPostgres } = require('../../app/lib/general.lib.js')
const { data: purposes } = require('./data/purposes.js')
const PurposeModel = require('../../app/models/purpose.model.js')

async function seed () {
for (const purpose of purposes) {
await _upsert(purpose)
}
}

async function _upsert (purpose) {
return PurposeModel.query()
.insert({ ...purpose, updatedAt: timestampForPostgres() })
.onConflict('legacyId')
.merge(['description', 'lossFactor', 'twoPartTariff', 'updatedAt'])
}

module.exports = {
seed
}
22 changes: 22 additions & 0 deletions db/seeds/03-primary-purposes.seed.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
'use strict'

const { timestampForPostgres } = require('../../app/lib/general.lib.js')
const { data: primaryPurposes } = require('./data/primary-purposes.js')
const PrimaryPurposeModel = require('../../app/models/primary-purpose.model.js')

async function seed () {
for (const purpose of primaryPurposes) {
await _upsert(purpose)
}
}

async function _upsert (primaryPurpose) {
return PrimaryPurposeModel.query()
.insert({ ...primaryPurpose, updatedAt: timestampForPostgres() })
.onConflict('legacyId')
.merge(['description', 'updatedAt'])
}

module.exports = {
seed
}
22 changes: 22 additions & 0 deletions db/seeds/04-secondary-purposes.seed.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
'use strict'

const { timestampForPostgres } = require('../../app/lib/general.lib.js')
const { data: secondaryPurposes } = require('./data/secondary-purposes.js')
const SecondaryPurposeModel = require('../../app/models/secondary-purpose.model.js')

async function seed () {
for (const purpose of secondaryPurposes) {
await _upsert(purpose)
}
}

async function _upsert (secondaryPurpose) {
return SecondaryPurposeModel.query()
.insert({ ...secondaryPurpose, updatedAt: timestampForPostgres() })
.onConflict('legacyId')
.merge(['description', 'updatedAt'])
}

module.exports = {
seed
}
22 changes: 22 additions & 0 deletions db/seeds/05-licence-version-purpose-condition-types.seed.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
'use strict'

const { timestampForPostgres } = require('../../app/lib/general.lib.js')
const { data: licenceVersionPurposeConditionTypes } = require('./data/licence-version-purpose-condition-types.js')
const LicenceVersionPurposeConditionTypeModel = require('../../app/models/licence-version-purpose-condition-type.model.js')

async function seed () {
for (const licenceVersionPurposeConditionType of licenceVersionPurposeConditionTypes) {
await _upsert(licenceVersionPurposeConditionType)
}
}

async function _upsert (licenceVersionPurposeConditionType) {
return LicenceVersionPurposeConditionTypeModel.query()
.insert({ ...licenceVersionPurposeConditionType, updatedAt: timestampForPostgres() })
.onConflict(['code', 'subcode'])
.merge(['description', 'displayTitle', 'subcodeDescription', 'updatedAt'])
}

module.exports = {
seed
}
Loading

0 comments on commit e8619c5

Please sign in to comment.