-
Notifications
You must be signed in to change notification settings - Fork 0
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
Create endpoint to generate fake data #347
Conversation
https://eaflood.atlassian.net/browse/WATER-4070 https://eaflood.atlassian.net/browse/WATER-4073 Whilst working on [Create endpoint to generate fake data](#347) we added a number of formatters. These take data from the DB and re-format them to match how the UI displays the values. We believe we are going to need these formatters in future changes we'll be making, the first of which is a re-designed bill run page. So, to help prepare for that we are adding those formatters to our `BasePresenter` so they are available to any future presenters we add.
https://eaflood.atlassian.net/browse/WATER-4070 https://eaflood.atlassian.net/browse/WATER-4073 Whilst working on [Create endpoint to generate fake data](#347) we added a number of formatters. These take DB data and re-format it to match how the UI displays the values. We will need these formatters in future changes we'll be making, the first of which is a re-designed bill run page. So, to help prepare for that we are adding those formatters to our `BasePresenter` so they are available to any future presenters we add.
For prototype and test purposes we need to be able to generate realistic-looking fake data. We therefore creat an endpoint to do this.
This adds the initial version of the service that will generate our 'mock' bill run. We start with finding the billing match that matches the ID provided as well as handling what happens if there is no match.
The service is just picking random from arrays of fixed values. We feel there is very little we need to cover so the tests just ensure that the service is 'working'.
a3aba94
to
20c1896
Compare
billingInvoices.forEach((billingInvoice) => { | ||
const { address, name } = GenerateMockDataService.go() | ||
|
||
billingInvoice.accountAddress = address |
Check failure
Code scanning / CodeQL
Insecure randomness
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All looking good, just a few small comments (it won't let me Request Changes since it's my own PR)
@@ -44,6 +56,7 @@ async function tearDown (_request, h) { | |||
|
|||
module.exports = { | |||
exportDb, | |||
mockData: mock, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to rename our mock
function to be mockData
instead of aliasing it in the exports?
* @module MockBillRunPresenter | ||
*/ | ||
|
||
const { convertPenceToPounds, formatAbstractionPeriod, formatLongDate, formatNumberAsMoney } = require('../base.presenter.js') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is just a little too long and should probably be split up.
transactionFile, | ||
billRunNumber, | ||
financialYear: `${fromFinancialYearEnding} to ${toFinancialYearEnding}`, | ||
debit: formatNumberAsMoney(convertPenceToPounds(netTotal)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do formatNumberAsMoney(convertPenceToPounds())
in several places. Do we want to break it out into a function of its own, _formatPenceAsPounds()
or something?
abstractionPeriodStartDay: startDay, | ||
abstractionPeriodStartMonth: startMonth, | ||
abstractionPeriodEndDay: endDay, | ||
abstractionPeriodEndMonth: endMonth, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to alias these seeing as we're passing them to a function and not using them directly? Or do we want to keep that cos if we don't alias them, our call to the function would be too big for one line?
return _response(billingBatch) | ||
} | ||
|
||
async function _fetchBillingBatch (id) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not 100% certain this needs to be async
seeing as it returns a query, and it's the returned query that we await
; but not going to quibble over it!
const GenerateMockDataService = require('./generate-mock-data.service.js') | ||
const MockBillRunPresenter = require('../../../presenters/data/mock-bill-run.presenter.js') | ||
|
||
async function go (id) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need some brief docs here?
* @param {*} transactions | ||
* @returns |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'd want to see us either flesh out @param
with the type and @returns
with some details, or remove them completely.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Paired on this with @StuAA78 . Admittedly, the later changes were done just by me so ideally would be confirmed as all good before merging.
But he's had the audacity to take time off (!!) and it would be unfair to ask the rest of the team to review this massive change cold.
So, making the executive decision to approve and merge-away! This is not a feature that is intended for use in production
anyway 😬
https://eaflood.atlassian.net/browse/WATER-4152 We are currently working on the SROC two part tariff bill. The first step of the process in the UI is to review how the returns have been matched. Our superstar design team are working on an improved design for the returns review screen but to do that they need data. Enter our mock data service! We previously [Create endpoint to generate fake data](#347) which was focused on a bill run. The design team would love it if we could do the same for the data the current returns review page uses. The page relies on data in the `water.billing_volumes` table. So, first step in generating the mock data is to add that model.
https://eaflood.atlassian.net/browse/WATER-4152 We are currently working on the SROC two-part tariff bill. The first step of the process in the UI is to review how the returns have been matched. Our superstar design team are working on an improved design for the returns review screen but to do that they need data. Enter our mock data service! We previously [Create endpoint to generate fake data](#347) which was focused on a bill run. The design team would love it if we could do the same for the data the current returns review page uses. The page relies on data in the `water.billing_volumes` table. So, the first step in generating the mock data is to add that model.
https://eaflood.atlassian.net/browse/WATER-4351 The project comes with an inbuilt mechanism to configure routes to not be available in protected environments, for example, production. You can see this in `app/routes/data.routes.js`. Both `/data/seed` and `/data/tear-down` are configured to be `excludeFromProd`. How this works is during startup our `app/plugins/router.plugin.js` calls `app/services/plugins/filter-routes.service.js` which handles filtering out those routes that should not be added when running in a protected environment. To be honest, `excludeFromProd` is a bad name because it could be any environment we add to this ```javascript function _protectedEnvironment (environment) { return ['prd'].includes(environment) } ``` When this was first implemented `pre` was included so we could confirm routes were not available prior to shipping to production. However, in [Create endpoint to generate fake data](#347) we took it out so we could get this fake data generator to work. It needed to be in `pre` because it generated data based on real bill runs. Clearly we weren't on the ball because we didn't even bother to update the comments which still exist ```javascript * So, we use this service to ensure any endpoint that is still being worked on is not available when the API is * running in production. We also include pre-production in our protected environments so we can test and ensure * an endpoint does not get registered as part of our release testing and sign-off. ``` Since that time the fake data generator is no more and we need to be able to test again that routes (and therefore pages) are not accessible in production. Hence, we need to add `pre` back into our list of protected environments.
https://eaflood.atlassian.net/browse/WATER-4351 The project comes with an inbuilt mechanism to configure routes to not be available in protected environments, for example, production. You can see this in `app/routes/data.routes.js`. Both `/data/seed` and `/data/tear-down` are configured to be `excludeFromProd`. How this works is during startup our `app/plugins/router.plugin.js` calls `app/services/plugins/filter-routes.service.js` which handles filtering out those routes that should not be added when running in a protected environment. To be honest, `excludeFromProd` is a bad name because it could be any environment we add to this ```javascript function _protectedEnvironment (environment) { return ['prd'].includes(environment) } ``` When this was first implemented `pre` was included so we could confirm routes were not available before shipping to production. However, in [Create endpoint to generate fake data](#347) we took it out so we could get this fake data generator to work. It needed to be in `pre` because it generated data based on real bill runs. Clearly, we weren't on the ball because we didn't even bother to update the comments which still exist ```javascript * So, we use this service to ensure any endpoint that is still being worked on is not available when the API is * running in production. We also include pre-production in our protected environments so we can test and ensure * an endpoint does not get registered as part of our release testing and sign-off. ``` Since that time the fake data generator is no more and we need to be able to test again that routes (and therefore pages) are not accessible in production. Hence, we need to add `pre` back into our list of protected environments.
https://eaflood.atlassian.net/browse/WATER-4070
https://eaflood.atlassian.net/browse/WATER-4073
We are looking to re-design the bill runs page in the internal service due to repeated issues with the existing one timing out. It is simply trying to return too much data when displaying annual bill runs.
To properly test the new design our UAT team hit a blocker; how do we demonstrate a large bill run in the prototype? They were able to manually create a bill run or 2. But to properly test this they needed 100's of licences represented and thousands of bills. They also had to be 'realistic' because previous testing had highlighted users got distracted by inaccuracies in the test data.
So, we have created a new feature in the app which allows the delivery team to 'mock' existing records. Currently, it only supports mocking bill runs but this change is done in a way it should be easy to add additional entity types if needed.
To mock a bill run, a team member would first identify one in the environment they are connected to that they would like to mock (the endpoint is only available in non-prod environments). They then extract the ID from the URL and make a request in their browser to
https://[non-prod-environment-domain]/system/data/mock/bill-run/96187453-7243-4f45-a6a7-d0c5f530cbac
The system will then use the existing bill run as a template, mock things like invoice and licence holder details and obfuscate other data before returning the mocked bill run as JSON. The format returned is based on what the prototype is already using so it can be copied and pasted right in.