Make timestamps consistent at model layer

app/models/legacy-base.model.js

@@ -65,6 +65,159 @@ class LegacyBaseModel extends BaseModel {
   static get schema () {
     throw new Error('defaultSchema() not implemented in child class')
   }
+
+  /**
+   * Translations for non-standard column names to our standardised ones
+   *
+   * The primary example of non-standard column names in the legacy data is timestamps. For example, we have found the
+   * column which has the timestamp for when a record is created has been named `date_created`, 'created', 'modified',
+   * and 'created_at`.
+   *
+   * Within our code we want to just be using `createdAt` (which would be `created_at` in the DB), as this is what the
+   * field would have been called if Knex had been used to generate the migrations and build the tables.
+   *
+   * All models that extend `LegacyBaseModel` are expected to implement this getter. It should always return an array.
+   * If there are no translations an empty array is valid.
+   *
+   * @example
+   * ```javascript
+   * static get translations () {
+   *  return [
+   *    { database: 'created', model: 'createdAt' },
+   *    { database: 'modified', model: 'updatedAt' }
+   *  ]
+   * }
+   * ```
+   *
+   */
+  static get translations () {
+    throw new Error('translations() not implemented in child class')
+  }
+
+  /**
+   * Called when a Model is converted to database format.
+   *
+   * This is an Objection.js instance method which we are overriding. We use it to convert our standardised property
+   * names into the legacy ones, for example `createdAt` to `dateCreated`. Timestamps is a primary example
+   * where the legacy tables do not use a consistent naming convention. Various tables use different names for the same
+   * thing. But we want few surprises and lots of consistency in our code. So, this allows us to translate the names of
+   * properties before Objection.js finalises the database version.
+   *
+   * Each Model that extends `LegacyBaseModel` is expected to implement `static get translations` which defines the
+   * database to model translations needed. Any fields listed in `translations` not found will be ignored.
+   *
+   * For more details see
+   * {@link http://vincit.github.io/objection.js/api/model/instance-methods.html#parsedatabasejson $parseDatabaseJson()}
+   * and the
+   * {@link https://vincit.github.io/objection.js/api/model/overview.html#model-data-lifecycle Model data lifecycle}
+   *
+   * -------------------------------------------------------------------------------------------------------------------
+   *
+   * **WARNING!** This translation method _only_ works for instances. We do not yet have a solution that will also
+   * work with static queries. Running the following will cause an exception because `createdAt` (automatically
+   * converted to `created_at` in the final query) does not exist in the `water.events` table.
+   *
+   * ```javascript
+   * EventModel.query().where('createdAt', '<', new Date().toISOString())
+   * ```
+   *
+   * Overriding
+   * {@link https://vincit.github.io/objection.js/api/model/static-properties.html#static-columnnamemappers columnNameMappers}
+   * had the same result as what we're doing. We also looked into
+   * {@link https://vincit.github.io/objection.js/api/objection/#knexidentifiermapping knexIdentifierMapping}. This
+   * would do the mapping at a lower level, in the same way we've used
+   * {@link https://vincit.github.io/objection.js/api/objection/#knexsnakecasemappers knexSnakeCaseMappers} to
+   * make changes before Objection.js takes over.
+   *
+   * The first problem we hit was that you can only use one of these; you can't apply both to the config object based to
+   * knex (see `knexfile.application.js` and `db/db.js` for how we currently do it). This is because both methods
+   * return an object which defines `wrapIdentifier()` and `postProcessResponse()`. So, whichever is added last
+   * overwrites what the previous one set.
+   *
+   * You can resolve this by importing `objection/lib/utils/identifierMapping.js` directly and doing something like this
+   *
+   * ```javascript
+   * // https://gitter.im/Vincit/objection.js?at=5e25f065a259cb0f060754ee
+   * const og = require('objection/lib/utils/identifierMapping');
+   * const SNAKE_CASE_OVERRIDES = { col12: 'col_1_2' };
+   *
+   * function snakeCase(str, { upperCase = false, underscoreBeforeDigits = false } = {}) {
+   *   // if this column has a manual override, return the snake-cased column name from the override
+   *   if (str.length && str in SNAKE_CASE_OVERRIDES) {
+   *     return SNAKE_CASE_OVERRIDES[str];
+   *   }
+   *
+   *   // otherwise simply call the original snakeCase() function
+   *   return og.snakeCase(str, { upperCase: upperCase, underscoreBeforeDigits: underscoreBeforeDigits});
+   * }
+   *
+   * function knexSnakeCaseMappers(opt = {}) {
+   *   return og.knexIdentifierMappers({
+   *     parse: str => og.camelCase(str, opt),
+   *     format: str => snakeCase(str, opt), // call overridden snakeCase() function
+   *   });
+   * }
+   * ```
+   *
+   * But that leads us to the second problem; at this level translations are global. So, any reference to `date_created`
+   * would get translated to `created_at`, irrespective of the table referenced. If we only had to worry about database
+   * to model it would be okay. The problem is to make queries work we need to be able to translate back from our names
+   * to the database version. That is impossible where `created_at` might need to become `date_created`, `created`,
+   * or `modified`. With no access to the table name for context we cannot make that decision.
+   *
+   * This is why we have accepted we can only support translations when dealing with instances of a model for now.
+   *
+   * @param {Object} json The JSON POJO in internal format
+   * @returns {Object} The JSON POJO in database format
+   */
+  $formatDatabaseJson (json) {
+    json = super.$formatDatabaseJson(json)
+
+    for (const translation of this.constructor.translations) {
+      if (translation.model in json) {
+        json[translation.database] = json[translation.model]
+
+        delete json[translation.model]
+      }
+    }
+
+    return json
+  }
+
+  /**
+   * Called when a Model instance is created from a database JSON object. This method converts the JSON object from the
+   * database format to the internal format.
+   *
+   * This is an Objection.js instance method which we are overriding. We use it to convert unconventional database
+   * column names into standardised ones, for example `dateCreated` to `createdAt`. Timestamps is a primary example
+   * where the legacy tables do not use a consistent naming convention. Various tables use different names for the same
+   * thing. But we want few surprises and lots of consistency in our code. So, this allows us to translate the names of
+   * columns before Objection.js finalises the model instance.
+   *
+   * Each Model that extends `LegacyBaseModel` is expected to implement `static get translations` which defines the
+   * database to model translations needed. Any fields listed in `translations` not found will be ignored.
+   *
+   * For more details see
+   * {@link http://vincit.github.io/objection.js/api/model/instance-methods.html#parsedatabasejson $parseDatabaseJson()}
+   * and the
+   * {@link https://vincit.github.io/objection.js/api/model/overview.html#model-data-lifecycle Model data lifecycle}
+   *
+   * @param {Object} json The JSON POJO in database format
+   * @returns {Object} The JSON POJO in internal format
+   */
+  $parseDatabaseJson (json) {
+    json = super.$parseDatabaseJson(json)
+
+    for (const translation of this.constructor.translations) {
+      if (translation.database in json) {
+        json[translation.model] = json[translation.database]
+
+        delete json[translation.database]
+      }
+    }
+
+    return json
+  }
 }
 
 class SchemaQueryBuilder extends QueryBuilder {

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

@@ -18,6 +18,13 @@ class BillingBatchModel extends WaterBaseModel {
     return 'billingBatchId'
   }
 
+  static get translations () {
+    return [
+      { database: 'dateCreated', model: 'createdAt' },
+      { database: 'dateUpdated', model: 'updatedAt' }
+    ]
+  }
+
   static get relationMappings () {
     return {
       region: {

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

@@ -18,6 +18,13 @@ class BillingChargeCategoryModel extends WaterBaseModel {
     return 'billingChargeCategoryId'
   }
 
+  static get translations () {
+    return [
+      { database: 'dateCreated', model: 'createdAt' },
+      { database: 'dateUpdated', model: 'updatedAt' }
+    ]
+  }
+
   static get relationMappings () {
     return {
       chargeElements: {

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

@@ -18,6 +18,13 @@ class ChargeElementModel extends WaterBaseModel {
     return 'chargeElementId'
   }
 
+  static get translations () {
+    return [
+      { database: 'dateCreated', model: 'createdAt' },
+      { database: 'dateUpdated', model: 'updatedAt' }
+    ]
+  }
+
   static get relationMappings () {
     return {
       chargeVersion: {

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

@@ -18,6 +18,13 @@ class ChargePurposeModel extends WaterBaseModel {
     return 'chargePurposeId'
   }
 
+  static get translations () {
+    return [
+      { database: 'dateCreated', model: 'createdAt' },
+      { database: 'dateUpdated', model: 'updatedAt' }
+    ]
+  }
+
   static get relationMappings () {
     return {
       chargeElement: {

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

@@ -18,6 +18,13 @@ class ChargeVersionModel extends WaterBaseModel {
     return 'chargeVersionId'
   }
 
+  static get translations () {
+    return [
+      { database: 'dateCreated', model: 'createdAt' },
+      { database: 'dateUpdated', model: 'updatedAt' }
+    ]
+  }
+
   static get relationMappings () {
     return {
       licence: {

app/models/water/event.model.js

@@ -15,6 +15,13 @@ class EventModel extends WaterBaseModel {
   static get idColumn () {
     return 'eventId'
   }
+
+  static get translations () {
+    return [
+      { database: 'created', model: 'createdAt' },
+      { database: 'modified', model: 'updatedAt' }
+    ]
+  }
 }
 
 module.exports = EventModel

app/models/water/licence.model.js

@@ -18,6 +18,13 @@ class LicenceModel extends WaterBaseModel {
     return 'licenceId'
   }
 
+  static get translations () {
+    return [
+      { database: 'dateCreated', model: 'createdAt' },
+      { database: 'dateUpdated', model: 'updatedAt' }
+    ]
+  }
+
   static get relationMappings () {
     return {
       chargeVersions: {

app/models/water/region.model.js

@@ -18,6 +18,13 @@ class RegionModel extends WaterBaseModel {
     return 'regionId'
   }
 
+  static get translations () {
+    return [
+      { database: 'dateCreated', model: 'createdAt' },
+      { database: 'dateUpdated', model: 'updatedAt' }
+    ]
+  }
+
   static get relationMappings () {
     return {
       licences: {

app/presenters/supplementary-billing/create-billing-batch-event.presenter.js

@@ -11,9 +11,7 @@ function go (billingBatch) {
     billingBatchId,
     creditNoteCount,
     creditNoteValue,
-    dateCreated,
     createdAt,
-    dateUpdated,
     updatedAt,
     fromFinancialYearEnding,
     invoiceCount,
@@ -44,12 +42,8 @@ function go (billingBatch) {
       isSummer,
       netTotal,
       startYear: { yearEnding: fromFinancialYearEnding },
-      // NOTE: In the 'real' schema timestamp fields are dateCreated & dateUpdated. If you follow the standard
-      // convention of using a trigger as seen in db/migrations/[*]_create_update_timestamp_trigger.js you get
-      // createdAt & updatedAt. In our testing schema we use the later. To ensure unit tests pass we need to account
-      // for both.
-      dateCreated: dateCreated ?? createdAt,
-      dateUpdated: dateUpdated ?? updatedAt,
+      dateCreated: createdAt,
+      dateUpdated: updatedAt,
       invoiceCount,
       invoiceValue,
       creditNoteCount,