Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Determine licence start date 4 returns reqs setup #655

Merged
merged 3 commits into from
Jan 11, 2024

Conversation

Cruikshanks
Copy link
Member

@Cruikshanks Cruikshanks commented Jan 11, 2024

https://eaflood.atlassian.net/browse/WATER-4261

This change supports the return requirements setup work

Tickets WATER-4263 and WATER-4266 cover adding controls and validation to the start date page in the set-up journeys (returns required and not required).

Users will be offered the choice to choose an existing start date or enter one manually. The existing start date needs to come from the current licence version record.

So, this change adds functionality to the InitiateReturnRequirementSessionService to add the start date to the session for use later in the page.


Note about using returning(*)

In our new test we compared startDate in the result with new Date('2022-05-01'). It kept failing because startDate was coming back as a string; '2022-05-01T00:00:00.000Z'.

Well, this one had us puzzled!

The type for water.licence_versions.start_date is DATE in the DB. We still expect this to come back from the DB when queried as a JavaScript Date. But we were getting a string; '2022-05-01T00:00:00.000Z'.

We went down a rabbit hole of first assuming pg was returning the value as a string and therefore we needed to add some custom parsing to db/db.js

pg.types.setTypeParser(pg.types.builtins.DATE, (value) => {
  return value === null ? null : new Date(value)
})

But that didn't work. Then we wondered whether Objection.js was doing something when parsing the values from Knex.js. So we plugged into the model hooks and confirmed all was well.

  $parseDatabaseJson (json) {
    json = super.$parseDatabaseJson(json)

    if (json.startDate !== undefined) {
      console.log('🚀 ~ LicenceVersionModel ~ $parseDatabaseJson ~ startDate:', json.startDate)
    }

    return json
  }

  $formatJson(json) {
    json = super.$formatJson(json)

    if (json.startDate !== undefined) {
      console.log('🚀 ~ LicenceVersionModel ~ $formatJson ~ startDate:', json.startDate)
    }

    return json
  }

In all cases, the value was presented as a Date and not a string. But when we output session immediately after creating it we go this.

🚀 ~ _createSession ~ session: SessionModel {
  data: {
    journey: 'returns-required',
    licence: {
      id: '8674134c-98b0-45ed-ac90-443dc739434b',
      startDate: '2022-05-01T00:00:00.000Z',
      licenceRef: '01/426',
      licenceHolder: 'Licence Holder Ltd'
    }
  },
  id: '49e601ac-9342-420c-94a0-0adb3dbb104f',
  createdAt: 2024-01-11T13:16:23.348Z,
  updatedAt: 2024-01-11T13:16:23.348Z
}

You can see startDate: is a string! 🤬😩. More out of desperation we amended the returning() statement to just specify the id (as that is all we wanted back). Hazzah! This worked.

🚀 ~ _createSession ~ session: SessionModel {
  data: {
    licence: {
      id: '9032aa77-47b0-450a-81db-43cd7e5056f3',
      licenceRef: '01/615',
      licenceHolder: 'Licence Holder Ltd',
      startDate: 2022-05-01T00:00:00.000Z
    },
    journey: 'returns-required'
  },
  id: '985c5a14-07ca-426c-bc1d-2712377c9dcc'
}

So, we have to conclude something 'extra' is going on when returning('*') is used. We need to try and be aware of this in future. But now we have a solution we're not going digging to find out what and whether its Knex.js or Objection.js at the root.

https://eaflood.atlassian.net/browse/WATER-4261

> This change supports the return requirements setup work

Tickets WATER-4263 and WATER-4266 cover adding controls and validation to the start date page in the set-up journeys (returns required and not required).

Users will be offered the choice to choose an existing start date or enter one manually. The existing start date needs to come from the current licence version record.

So, this change adds functionality to the `InitiateReturnRequirementSessionService` to add the start date to the session for use later in the page.
@Cruikshanks Cruikshanks added the enhancement New feature or request label Jan 11, 2024
@Cruikshanks Cruikshanks self-assigned this Jan 11, 2024
With this change we should return only the 'current' version for a licence which will give us the start date to use in the `/start-date` page.

> This will fail the unit test. More in the next commit!
Well this one had us puzzled!

The type for `water.licence_versions.start_date` is `DATE` in the DB. Ideally you want all dates to be held as a timestamp else PG will default to the local systems time zone when returning a value. A timestamp ensures you get to set what the time zone should be (UTC in our case).

But we are where we are. We still expect this to come back from the DB when queried as a JavaScript `Date`. But we were getting a string; `'2022-05-01T00:00:00.000Z'`.

We went down a rabbit hole of first assuming pg was returning the value as a string and therefore we needed to add some custom parsing to `db/db.js`

```javascript
pg.types.setTypeParser(pg.types.builtins.DATE, (value) => {
  return value === null ? null : new Date(value)
})
```

But that didn't work. Then we wondered whether Objection.js was doing something when parsing the values from Knex.js. So we plugged into the model hooks and confirmed all was well.

```javascript
  $parseDatabaseJson (json) {
    // Remember to call the super implementation.
    json = super.$parseDatabaseJson(json)

    // Remember to always check if the json object has the particular
    // field. It may not exist if the user has used `select('id')`
    // or any other select that excludes the field.
    if (json.startDate !== undefined) {
      console.log('🚀 ~ LicenceVersionModel ~ $parseDatabaseJson ~ startDate:', json.startDate)
    }

    return json
  }

  $formatJson(json) {
    // Remember to call the super class's implementation.
    json = super.$formatJson(json)
    // Do your conversion here.

    // Remember to always check if the json object has the particular
    // field. It may not exist if the user has used `select('id')`
    // or any other select that excludes the field.
    if (json.startDate !== undefined) {
      console.log('🚀 ~ LicenceVersionModel ~ $formatJson ~ startDate:', json.startDate)
    }

    return json
  }
```

In all cases the value was presenting as a `Date` and not a string. But when we output `session` immediately after creating it we go this.

```javascript
🚀 ~ _createSession ~ session: SessionModel {
  data: {
    journey: 'returns-required',
    licence: {
      id: '8674134c-98b0-45ed-ac90-443dc739434b',
      startDate: '2022-05-01T00:00:00.000Z',
      licenceRef: '01/426',
      licenceHolder: 'Licence Holder Ltd'
    }
  },
  id: '49e601ac-9342-420c-94a0-0adb3dbb104f',
  createdAt: 2024-01-11T13:16:23.348Z,
  updatedAt: 2024-01-11T13:16:23.348Z
}
```

You can see `startDate:` is a string! 🤬😩. More out of desperation we amended the `returning()` statement to just specify the `id` (as that is all we really wanted back). Hazzah! This worked.

```javascript
🚀 ~ _createSession ~ session: SessionModel {
  data: {
    licence: {
      id: '9032aa77-47b0-450a-81db-43cd7e5056f3',
      licenceRef: '01/615',
      licenceHolder: 'Licence Holder Ltd',
      startDate: 2022-05-01T00:00:00.000Z
    },
    journey: 'returns-required'
  },
  id: '985c5a14-07ca-426c-bc1d-2712377c9dcc'
}
```

So, we have to conclude something 'extra' is going on when `returning('*')` is used. We need to try and be aware of this in future. But now we have a solution we've not going digging to find out what and whether its **Knex.js** or **Objection.js** at the root.
@Cruikshanks Cruikshanks marked this pull request as ready for review January 11, 2024 13:25
@Cruikshanks Cruikshanks merged commit 829a344 into main Jan 11, 2024
6 checks passed
@Cruikshanks Cruikshanks deleted the determine-version-start-date branch January 11, 2024 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants