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

Use absolute paths for relations in legacy models #562

Merged
merged 2 commits into from
Dec 2, 2023

Conversation

Cruikshanks
Copy link
Member

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

As part of the work we have been doing on two-part tariff we are going to be creating all our new tables in the default public schema.

We have also decided that when there is a legacy table that we are still going to need we will create a View of it in the public schema. This allows us to correct any issues with naming conventions, strip out unused fields, and join entities that are currently sat in different schemas. The first example of this approach was done in PR #531 .

Whilst we transition from the models based directly on the tables to ones on our new views we'll have 2 of everything; 2 BillRunModel, AddressModel, etc. We didn't think this would be a problem but our initial attempts to create the new models have exposed an issue.

Because we are not able to take advantage of ESM modules, when you specify a relationship between models you have to provide a path. Handily, Objection.js has a solution where you provide a base class and specify a modelPaths property.

  static get modelPaths () {
    return [__dirname]
  }

Because we put our legacy models in folders to help make it clear which schemas they came from, we needed to extend this further in the LegacyBaseModel.

  static get modelPaths () {
    const currentPath = __dirname
    return [
      currentPath,
      path.join(currentPath, 'returns'),
      path.join(currentPath, 'water'),
      path.join(currentPath, 'crm-v2'),
      path.join(currentPath, 'idm')
    ]
  }

The issue is when Objection is trying to work out the relationships in a legacy model, it is seeing both models. This is causing relationships to break and unit tests to fail. Fortunately, the fix is simple. We just need to stop including the path to the new models in the LegacyBaseModel. Then, when Objection comes to deal with a relationship the only models it is seeing are the legacy ones. The same goes for the new models, because they only extend BaseModel they only know of the ones in the root of src/models.

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

As part of the work we have been doing on two-part tariff we are going to be creating all our new tables in the default `public` schema.

We have also decided that when there is a legacy table that we are still going to need we will create a [View](https://www.postgresql.org/docs/current/sql-createview.html) of it in the `public` schema. This allows us to correct any issues with naming conventions, strip out unused fields, and join entities that are currently sat in different schemas. The first example of this approach was done in PR #531 .

Whilst we transition from the models based directly on the tables to ones on our new views we'll have 2 of everything; 2 `BillRunModel`, `AddressModel`, etc. We didn't think this would be a problem but our initial attempts to create the new models has exposed an issue.

Because we are not able to take advantage of ESM modules, when you specify a relationship between models you have to provide a path. Handily, [Objection.js](https://vincit.github.io/objection.js/) has a solution where you provide a base class and specify a `modelPaths` property.

```javascript
  static get modelPaths () {
    return [__dirname]
  }
```

Because we put our legacy models in folders to help make it clear which schemas they came from, we needed to extend this further in the `LegacyBaseModel`.

```javascript
  static get modelPaths () {
    const currentPath = __dirname
    return [
      currentPath,
      path.join(currentPath, 'returns'),
      path.join(currentPath, 'water'),
      path.join(currentPath, 'crm-v2'),
      path.join(currentPath, 'idm')
    ]
  }
```

The issue is when Objection is trying to work out the relationships in a legacy model, it is seeing both models. This is causing relationships to break and unit tests to fail. Fortunately, the fix is simple. We just need to stop including the path to the new models in the `LegacyBaseModel`. Then, when Objection comes to deal with a relationship the only models it is seeing are the legacy ones. The same goes for the new models, because they only extend `BaseModel` they only know of the ones in the root of `src/models`.
@Cruikshanks Cruikshanks added the bug Something isn't working label Dec 2, 2023
@Cruikshanks Cruikshanks self-assigned this Dec 2, 2023
@Cruikshanks Cruikshanks marked this pull request as ready for review December 2, 2023 21:57
@Cruikshanks Cruikshanks merged commit d30fa22 into main Dec 2, 2023
7 checks passed
@Cruikshanks Cruikshanks deleted the fix-multiple-model-issue branch December 2, 2023 21:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant