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

Add new /billing-accounts endpoint #382

Merged
merged 2 commits into from
Aug 25, 2023

Conversation

Cruikshanks
Copy link
Member

@Cruikshanks Cruikshanks commented Aug 25, 2023

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

We have an issue with the legacy system around billing accounts. When a user creates a new one the Charging Module API needs to know about it. This is because it manages the 'customer changes' file that gets passed to SSCL and the SOP system. The data imported from the file is used when generating invoices from the billing runs we generate.

This process is working fine. The issue is when a billing account gets updated. This is triggered by finding the existing account and opting to 'Change the Address'. During the journey you can also change the company and contact linked to the 'billing account' but the problem is with the address.

Though the system updates the address fine, it is sending the old address through to the Charging Module. We had to do some digging through the old code to figure out what was going on. We believe we know what the issue is but the fact we are uncertain speaks to just how complex the previous team have made this.

For example, the data is collected by the water-abstraction-ui which at the end of the journey submits it to the water-abstraction-service. That service works out whether it is dealing with an update or a new account (both are sent to the same endpoint). It then orchestrates the update to the database by making calls to endpoints in the water-abstraction-tactical-crm. The bit we loved was the tactical-crm will make HTTP calls to itself to get some of those updates applied! This is because it uses hapi-pg-rest-api, a custom package that generates a REST endpoint for a PostgreSQL table.

That covers updating the record in our DB. The update to the Charging Module API is done differently. Instead of sending the request and dealing with the response directly, a job is created and added to the BullMQ message queue.

When the job is picked up and processed it grabs the latest address and uses that when generating the request to the Charging Module API. On the face of it, this should all work. However, we have been able to repeatedly (though not consistently) demonstrate that the query it uses to return the 'current' address linked to the billing account is returning the old one. The process that updates the database applies an end_date to the invoice_account_address record. This is how the system marks addresses as no longer 'current'. But when the BullMQ job is being processed it's finding that the update hasn't happened so is picking the old address.

We believe it's a timing issue between the 2 processes.

So, why are we in this repo? We had to wade through the code base of 3 separate apps plus a custom package to understand what was going on. Added to that the 2 parts of this process; updating the DB and letting the Charging Module API know, are handled completely differently. Plus, you need knowledge of the BullMQ framework to understand how that second part is done. All of this is being done just to

  • Add a record in the DB
  • Update another one
  • Send an HTTP POST request to an external API

By the way, we're omitting the various custom controller generators, object manipulators, shared HTTP request modules, etc., which we assume are built to make stuff generic but which means there is a whole other level of complexity to each step. Add to that the haphazard and inconsistent way they have been applied.

We could keep digging and testing, and 'fix' the existing legacy code. But examples like this are why we have this repo and are migrating as much as we can as fast as we can.

So, we have decided to re-implement this function within water-abstraction-system. The previous PRs were some initial prep work. Adding the endpoint is the first step in changing a billing account address here.

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

We have an issue with the legacy system around billing accounts. When a user creates a new one the [Charging Module API](https://github.com/DEFRA/sroc-charging-module-api) needs to know about it. This is because it manages the 'customer changes' file that gets passed to SSCL and the SOP system. The data imported from the file is used when generating invoices from the billing runs we generate.

This process is working fine. The issue is when a billing account gets updated. This is triggered by finding the existing account and opting to 'Change the Address'. During the journey you can also change the company and contact linked to the 'billing account' but the problem is with the address.

Though the system updates the address fine, it is sending the old address through to the Charging Module. We had to do some digging through the old code to figure out what is going on. We believe we know what the issue is but the fact we are uncertain speaks to just how complex the previous team have made this.

For example, the data is collected by the [water-abstraction-ui](https://github.com/DEFRA/water-abstraction-ui) which at the end of the journey submits it to the [water-abstraction-service](https://github.com/DEFRA/water-abstraction-service). That service works out whether it is dealing with an update or a new account (both are sent to the same endpoint). It then orchestrates the update to the database by making calls to endpoints in the [water-abstraction-tactical-crm](https://github.com/DEFRA/water-abstraction-tactical-crm). The bit we loved was the **tactical-crm** will make http calls _to itself_ to get some of those updates applied! This is because it uses [hapi-pg-rest-api](https://github.com/DEFRA/hapi-pg-rest-api), a custom package that generates a REST endpoint for a PostgreSQL table.

That covers updating the record in our DB. The update to the Charging Module APi is done differently. Instead of sending the request and dealing with the response directly, a job is created and added to the [BullMQ message queue](https://docs.bullmq.io/).

When the job is picked up and processed it grabs the latest address and uses that when generating the request to the Charging Module API. On the face of it this should all work. But we have been able to repeatedly (though not consistently) demonstrate that the query it uses to return the 'current' address linked to the billing account is returning the old one. The process that updates the database applies an `end_date` to the `invoice_account_address` record. This is how the system marks addresses as no longer 'current'. But when the BullMQ job is being processed it's finding that update hasn't happened so is picking the old address.

We believe it's a timing issue between the 2 processes.

So, why are we in this repo? We had to wade through the code base of 3 separate apps plus a custom package to understand what was going on. Added to that the 2 parts to this process; updating the DB and letting the Charging Module API know, are handled completely differently. Plus, you need knowledge of the BullMQ framework to understand how that second part is done. _All of this_ is being done just to

- add a record in the DB
- update another one
- send a HTTP `POST` request to an external API

By the way, we're omitting the various custom controller generators, object manipulators, shared HTTP request modules, etc, we assume built to make stuff generic but which mean there is a whole other level of complexity to each step. Add to that the haphazard and inconsistent way they have been applied.

We could keep digging and testing, and 'fix' the existing legacy code. But examples like this are why we have this repo and are migrating as much as we can as fast as we can.

So, we have decided to re-implement this function within **water-abstraction-system**. The previous PR's were some initial prep work. Adding the endpoint is the first step in moving changing a billing account address here.
@Cruikshanks Cruikshanks added the enhancement New feature or request label Aug 25, 2023
@Cruikshanks Cruikshanks self-assigned this Aug 25, 2023
@Cruikshanks Cruikshanks marked this pull request as ready for review August 25, 2023 12:24
@Cruikshanks Cruikshanks requested review from Jozzey and StuAA78 August 25, 2023 12:25
@Cruikshanks Cruikshanks merged commit f990a3e into main Aug 25, 2023
@Cruikshanks Cruikshanks deleted the add-new-billing-accounts-change-address-endpoint branch August 25, 2023 13:17
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