Restructure models by type and scheme

app/models/base.model.js

@@ -5,7 +5,7 @@
  * @module BaseModel
  */
 
-const { Model, QueryBuilder } = require('objection')
+const { Model } = require('objection')
 
 const { db } = require('../../db/db.js')
 
@@ -16,42 +16,26 @@ Model.knex(db)
 
 class BaseModel extends Model {
   /**
-   * An objective property we override to tell it where to search for models for relationships
+   * Array of paths to search for models used in relationships
    *
-   * When setting a relationship in a model we have to provide a reference to the related model. As we need to set the
-   * relationship on both sides this leads to
+   * This is an objective property we override. When setting a relationship in a model we have to provide a reference
+   * to the related model. As we need to set the relationship on both sides this leads to
    * {@link https://vincit.github.io/objection.js/guide/relations.html#require-loops|require-loops}. We can avoid this
    * by having the model tell Objection where to search for models for relationships. In the relationship declaration we
    * can then just use a string value
    *
    * ```
-   *  // ...
-   *  relation: Model.ManyToManyRelation,
-        modelClass: 'charge_version.model',
-      // ...
-      ```
-
-      We don't want to do this in every model so set it in the `BaseModel` as Objection recommends.
+   * // ...
+   * relation: Model.ManyToManyRelation,
+   * modelClass: 'charge_version.model',
+   * // ...
+   * ```
+   *
+   * We don't want to do this in every model so set it in the `BaseModel` as Objection recommends.
    */
   static get modelPaths () {
     return [__dirname]
   }
-
-  static get defaultSchema () {
-    return 'water'
-  }
 }
 
-class DefaultSchemaQueryBuilder extends QueryBuilder {
-  constructor (modelClass) {
-    super(modelClass)
-    if (modelClass.defaultSchema) {
-      this.withSchema(modelClass.defaultSchema)
-    }
-  }
-}
-
-Model.QueryBuilder = DefaultSchemaQueryBuilder
-Model.RelatedQueryBuilder = DefaultSchemaQueryBuilder
-
 module.exports = BaseModel

app/models/legacy-base.model.js

@@ -0,0 +1,77 @@
+'use strict'
+
+/**
+ * Base class for all models based on tables in the legacy schemas
+ * @module LegacyBaseModel
+ */
+
+const { QueryBuilder } = require('objection')
+const path = require('path')
+
+const BaseModel = require('./base.model.js')
+
+class LegacyBaseModel extends BaseModel {
+  /**
+   * Array of paths to search for models used in relationships
+   *
+   * > See `modelPaths()` in `BaseModel` for more details on this getter
+   *
+   * We override the one provided in `BaseModel` to include our legacy folders.
+   *
+   * @returns {Array} array of paths to search for related models
+   */
+  static get modelPaths () {
+    const currentPath = __dirname
+    return [
+      currentPath,
+      path.join(currentPath, 'water')
+    ]
+  }
+
+  /**
+   * Returns a custom `QueryBuilder` which supports declaring the schema to use when connecting to the DB
+   *
+   * See `schema()` for further details.
+   *
+   * @returns {Object} a custom Objection `QueryBuilder`
+   */
+  static get QueryBuilder () {
+    return SchemaQueryBuilder
+  }
+
+  /**
+   * Name of the database schema a model belongs to
+   *
+   * All the legacy repos talk to the same PostgreSQL DB server but split their data using
+   * {@link https://www.postgresql.org/docs/current/ddl-schemas.html schemas}. Objection.js however assumes you are
+   * using the **public** schema which is the default in PostgreSQL if you don't specify one.
+   *
+   * So, when Objection is working with our legacy models it needs to know what schemas they belong to. There is no
+   * out-of-the-box support for specifying this. But the maintainer behind Objection.js suggested this solution in
+   * {@link https://github.com/Vincit/objection.js/issues/85#issuecomment-185183032 an issue} (which we've tweaked
+   * slightly).
+   *
+   * You extend Objection's QueryBuilder (see `SchemaQueryBuilder`) and call Knex's `withSchema()` within it (Knex
+   * _does_ understand schemas). It expects the Model generating the query to have a custom getter `schema` which
+   * returns the schema name. You then override the getter `QueryBuilder` in your model and return your
+   * {@link https://vincit.github.io/objection.js/recipes/custom-query-builder.html custom QueryBuilder}. This is then
+   * used to build any queries, for example, `MyModel.query.find()` ensuring the schema name is declared and our data
+   * layer knows where to find the data.
+   *
+   * > In this base model model we throw an exception to ensure any child classes override this getter.
+   *
+   * @returns {string} the schema name, for example, 'water'
+   */
+  static get schema () {
+    throw new Error('defaultSchema() not implemented in child class')
+  }
+}
+
+class SchemaQueryBuilder extends QueryBuilder {
+  constructor (modelClass) {
+    super(modelClass)
+    this.withSchema(modelClass.schema)
+  }
+}
+
+module.exports = LegacyBaseModel

app/models/billing-batch.model.js → app/models/water/billing-batch.model.js

@@ -7,9 +7,9 @@
 
 const { Model } = require('objection')
 
-const BaseModel = require('./base.model.js')
+const WaterBaseModel = require('./water-base.model.js')
 
-class BillingBatchModel extends BaseModel {
+class BillingBatchModel extends WaterBaseModel {
   static get tableName () {
     return 'billingBatches'
   }

app/models/billing-charge-category.model.js → app/models/water/billing-charge-category.model.js

@@ -7,9 +7,9 @@
 
 const { Model } = require('objection')
 
-const BaseModel = require('./base.model.js')
+const WaterBaseModel = require('./water-base.model.js')
 
-class BillingChargeCategoryModel extends BaseModel {
+class BillingChargeCategoryModel extends WaterBaseModel {
   static get tableName () {
     return 'billingChargeCategories'
   }

app/models/charge-element.model.js → app/models/water/charge-element.model.js

@@ -7,9 +7,9 @@
 
 const { Model } = require('objection')
 
-const BaseModel = require('./base.model.js')
+const WaterBaseModel = require('./water-base.model.js')
 
-class ChargeElementModel extends BaseModel {
+class ChargeElementModel extends WaterBaseModel {
   static get tableName () {
     return 'chargeElements'
   }

app/models/charge-purpose.model.js → app/models/water/charge-purpose.model.js

@@ -7,9 +7,9 @@
 
 const { Model } = require('objection')
 
-const BaseModel = require('./base.model.js')
+const WaterBaseModel = require('./water-base.model.js')
 
-class ChargePurposeModel extends BaseModel {
+class ChargePurposeModel extends WaterBaseModel {
   static get tableName () {
     return 'chargePurposes'
   }

app/models/charge-version.model.js → app/models/water/charge-version.model.js

@@ -7,9 +7,9 @@
 
 const { Model } = require('objection')
 
-const BaseModel = require('./base.model.js')
+const WaterBaseModel = require('./water-base.model.js')
 
-class ChargeVersionModel extends BaseModel {
+class ChargeVersionModel extends WaterBaseModel {
   static get tableName () {
     return 'chargeVersions'
   }

app/models/event.model.js → app/models/water/event.model.js

@@ -5,9 +5,9 @@
  * @module EventModel
  */
 
-const BaseModel = require('./base.model.js')
+const WaterBaseModel = require('./water-base.model.js')
 
-class EventModel extends BaseModel {
+class EventModel extends WaterBaseModel {
   static get tableName () {
     return 'events'
   }

app/models/licence.model.js → app/models/water/licence.model.js

@@ -7,9 +7,9 @@
 
 const { Model } = require('objection')
 
-const BaseModel = require('./base.model.js')
+const WaterBaseModel = require('./water-base.model.js')
 
-class LicenceModel extends BaseModel {
+class LicenceModel extends WaterBaseModel {
   static get tableName () {
     return 'licences'
   }

app/models/region.model.js → app/models/water/region.model.js

@@ -7,9 +7,9 @@
 
 const { Model } = require('objection')
 
-const BaseModel = require('./base.model.js')
+const WaterBaseModel = require('./water-base.model.js')
 
-class RegionModel extends BaseModel {
+class RegionModel extends WaterBaseModel {
   static get tableName () {
     return 'regions'
   }

app/models/water/water-base.model.js

@@ -0,0 +1,16 @@
+'use strict'
+
+/**
+ * Base class for all models based on the legacy 'water' schema
+ * @module WaterBaseModel
+ */
+
+const LegacyBaseModel = require('../legacy-base.model.js')
+
+class WaterBaseModel extends LegacyBaseModel {
+  static get schema () {
+    return 'water'
+  }
+}
+
+module.exports = WaterBaseModel

app/services/supplementary-billing/create-billing-batch-event.service.js

@@ -6,7 +6,7 @@
  */
 
 const CreateBillingBatchEventPresenter = require('../../presenters/supplementary-billing/create-billing-batch-event.presenter.js')
-const EventModel = require('../../models/event.model.js')
+const EventModel = require('../../models/water/event.model.js')
 
 /**
  * Create an event for when a new bill run is initialised

app/services/supplementary-billing/create-billing-batch.service.js

@@ -5,7 +5,7 @@
  * @module CreateBillingBatchService
  */
 
-const BillingBatchModel = require('../../models/billing-batch.model.js')
+const BillingBatchModel = require('../../models/water/billing-batch.model.js')
 
 /**
  * Create a new billing batch

app/services/supplementary-billing/fetch-charge-versions.service.js

@@ -5,7 +5,7 @@
  * @module FetchChargeVersionsService
  */
 
-const ChargeVersion = require('../../models/charge-version.model.js')
+const ChargeVersion = require('../../models/water/charge-version.model.js')
 
 /**
  * Fetch all SROC charge versions linked to licences flagged for supplementary billing that are in the period being

app/services/supplementary-billing/fetch-licences.service.js

@@ -5,7 +5,7 @@
  * @module FetchLicencesService
  */
 
-const LicenceModel = require('../../models/licence.model.js')
+const LicenceModel = require('../../models/water/licence.model.js')
 
 /**
  * Fetches licences flagged for supplementary billing that are linked to the selected region

app/services/supplementary-billing/fetch-region.service.js

@@ -5,7 +5,7 @@
  * @module FetchRegionService
  */
 
-const RegionModel = require('../../models/region.model.js')
+const RegionModel = require('../../models/water/region.model.js')
 
 /**
  * Fetches the region with the matching NALD Region ID

test/models/legacy-base.model.test.js

@@ -0,0 +1,35 @@
+'use strict'
+
+// Test framework dependencies
+const Lab = require('@hapi/lab')
+const Code = require('@hapi/code')
+
+const { describe, it } = exports.lab = Lab.script()
+const { expect } = Code
+
+// Thing under test
+const LegacyBaseModel = require('../../app/models/legacy-base.model.js')
+
+describe('Legacy Base model', () => {
+  describe('.schema()', () => {
+    describe('when the getter is not overridden', () => {
+      class BadModel extends LegacyBaseModel {}
+
+      it('throws an error when called', () => {
+        expect(() => BadModel.query()).to.throw()
+      })
+    })
+
+    describe('when the getter is overridden', () => {
+      class GoodModel extends LegacyBaseModel {
+        static get schema () {
+          return 'water'
+        }
+      }
+
+      it('does not throw an error when called', () => {
+        expect(() => GoodModel.query()).not.to.throw()
+      })
+    })
+  })
+})

test/models/billing-batch.model.test.js → test/models/water/billing-batch.model.test.js

@@ -8,13 +8,13 @@ const { describe, it, beforeEach } = exports.lab = Lab.script()
 const { expect } = Code
 
 // Test helpers
-const BillingBatchHelper = require('../support/helpers/billing-batch.helper.js')
-const DatabaseHelper = require('../support/helpers/database.helper.js')
-const RegionHelper = require('../support/helpers/region.helper.js')
-const RegionModel = require('../../app/models/region.model.js')
+const BillingBatchHelper = require('../../support/helpers/water/billing-batch.helper.js')
+const DatabaseHelper = require('../../support/helpers/database.helper.js')
+const RegionHelper = require('../../support/helpers/water/region.helper.js')
+const RegionModel = require('../../../app/models/water/region.model.js')
 
 // Thing under test
-const BillingBatchModel = require('../../app/models/billing-batch.model.js')
+const BillingBatchModel = require('../../../app/models/water/billing-batch.model.js')
 
 describe('Billing Batch model', () => {
   let testRecord

test/models/billing-charge-category.model.test.js → test/models/water/billing-charge-category.model.test.js

@@ -8,13 +8,13 @@ const { describe, it, beforeEach } = exports.lab = Lab.script()
 const { expect } = Code
 
 // Test helpers
-const BillingChargeCategoryHelper = require('../support/helpers/billing-charge-category.helper.js')
-const ChargeElementHelper = require('../support/helpers/charge-element.helper.js')
-const ChargeElementModel = require('../../app/models/charge-element.model.js')
-const DatabaseHelper = require('../support/helpers/database.helper.js')
+const BillingChargeCategoryHelper = require('../../support/helpers/water/billing-charge-category.helper.js')
+const ChargeElementHelper = require('../../support/helpers/water/charge-element.helper.js')
+const ChargeElementModel = require('../../../app/models/water/charge-element.model.js')
+const DatabaseHelper = require('../../support/helpers/database.helper.js')
 
 // Thing under test
-const BillingChargeCategoryModel = require('../../app/models/billing-charge-category.model.js')
+const BillingChargeCategoryModel = require('../../../app/models/water/billing-charge-category.model.js')
 
 describe('Billing Charge Category model', () => {
   let testRecord

test/models/charge-element.model.test.js → test/models/water/charge-element.model.test.js

@@ -8,17 +8,17 @@ const { describe, it, beforeEach } = exports.lab = Lab.script()
 const { expect } = Code
 
 // Test helpers
-const BillingChargeCategoryHelper = require('../support/helpers/billing-charge-category.helper.js')
-const BillingChargeCategoryModel = require('../../app/models/billing-charge-category.model.js')
-const ChargeElementHelper = require('../support/helpers/charge-element.helper.js')
-const ChargePurposeHelper = require('../support/helpers/charge-purpose.helper.js')
-const ChargePurposeModel = require('../../app/models/charge-purpose.model.js')
-const ChargeVersionHelper = require('../support/helpers/charge-version.helper.js')
-const ChargeVersionModel = require('../../app/models/charge-version.model.js')
-const DatabaseHelper = require('../support/helpers/database.helper.js')
+const BillingChargeCategoryHelper = require('../../support/helpers/water/billing-charge-category.helper.js')
+const BillingChargeCategoryModel = require('../../../app/models/water/billing-charge-category.model.js')
+const ChargeElementHelper = require('../../support/helpers/water/charge-element.helper.js')
+const ChargePurposeHelper = require('../../support/helpers/water/charge-purpose.helper.js')
+const ChargePurposeModel = require('../../../app/models/water/charge-purpose.model.js')
+const ChargeVersionHelper = require('../../support/helpers/water/charge-version.helper.js')
+const ChargeVersionModel = require('../../../app/models/water/charge-version.model.js')
+const DatabaseHelper = require('../../support/helpers/database.helper.js')
 
 // Thing under test
-const ChargeElementModel = require('../../app/models/charge-element.model.js')
+const ChargeElementModel = require('../../../app/models/water/charge-element.model.js')
 
 describe('Charge Element model', () => {
   let testRecord