refactor app-simplefin.ts to make it easily testable #440
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,15 +1,21 @@ | ||
| import express from 'express'; | ||
| import { inspect } from 'util'; | ||
| import { SecretName, secretsService } from '../services/secrets-service.js'; | ||
| import { handleError } from '../app-gocardless/util/handle-error.js'; | ||
| import { requestLoggerMiddleware } from '../util/middlewares.js'; | ||
| import { SimpleFinService } from './services/simplefin-service.ts'; | ||
| import { SimplefinApi } from './services/simplefin-api.ts'; | ||
| import { HttpsClient } from './httpClient.ts'; | ||
|
|
||
| const app = express(); | ||
| export { app as handlers }; | ||
| app.use(express.json()); | ||
| app.use(requestLoggerMiddleware); | ||
|
|
||
| const simplefinService = new SimpleFinService( | ||
| new SimplefinApi(new HttpsClient()), | ||
| ); | ||
|
Comment on lines
+15
to
+17
There was a problem hiding this comment. dependency injection, makes it easier to test There was a problem hiding this comment. I tried to mock There was a problem hiding this comment. I was able to test that, if the |
||
|
|
||
| app.post( | ||
| '/status', | ||
| handleError(async (req, res) => { | ||
|
|
@@ -36,13 +42,14 @@ | |
| if (token == null || token === 'Forbidden') { | ||
| throw new Error('No token'); | ||
| } else { | ||
| accessKey = await simplefinService.getAccessKey(token); | ||
| secretsService.set(SecretName.simplefin_accessKey, accessKey); | ||
| if (accessKey == null || accessKey === 'Forbidden') { | ||
| throw new Error('No access key'); | ||
| } | ||
| } | ||
| } | ||
| //eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
| } catch (error) { | ||
| invalidToken(res); | ||
| return; | ||
|
|
@@ -53,7 +60,11 @@ | |
| const endDate = new Date(now.getFullYear(), now.getMonth() + 1, 1); | ||
|
|
||
| try { | ||
| const accounts = await simplefinService.getAccounts( | ||
| accessKey, | ||
| startDate, | ||
| endDate, | ||
| ); | ||
|
|
||
| res.send({ | ||
| status: 'ok', | ||
|
There was a problem hiding this comment. I've gone over the code in the frontend that uses the accounts from the server, and made sure that the variables used there had the same name as the ones in |
||
|
|
@@ -82,15 +93,17 @@ | |
|
|
||
| let results; | ||
| try { | ||
| results = await simplefinService.getTransactions( | ||
| accessKey, | ||
| new Date(startDate), | ||
| ); | ||
| } catch (e) { | ||
| serverDown(e, res); | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| const account = results.accounts.find((a) => a.id === accountId); | ||
|
There was a problem hiding this comment. If no error is caught on line 100, |
||
| if (!account) { | ||
| console.log( | ||
| `The account "${accountId}" was not found. Here were the accounts returned:`, | ||
|
|
@@ -131,8 +144,7 @@ | |
| const response = {}; | ||
|
|
||
| const balance = parseInt(account.balance.replace('.', '')); | ||
| const date = account.balanceDate.toISOString() | ||
| .split('T')[0]; | ||
|
|
||
| response.balances = [ | ||
|
|
@@ -164,20 +176,20 @@ | |
|
|
||
| let dateToUse = 0; | ||
|
|
||
| if (trans.isPending()) { | ||
|
There was a problem hiding this comment.
|
||
| newTrans.booked = false; | ||
| dateToUse = trans.transacted_at.getTime() / 1000; | ||
|
There was a problem hiding this comment. I didn't want to change the logic further down in this function, so I just reverted the |
||
| } else { | ||
| newTrans.booked = true; | ||
| dateToUse = trans.posted.getTime() / 1000; | ||
| } | ||
|
|
||
| newTrans.bookingDate = new Date(dateToUse * 1000) | ||
| .toISOString() | ||
| .split('T')[0]; | ||
|
|
||
| newTrans.date = new Date(dateToUse * 1000).toISOString().split('T')[0]; | ||
| // newTrans.payeeName = trans.payee; TODO: Is this used? | ||
|
There was a problem hiding this comment. The There was a problem hiding this comment. Hmm, seems this was changed not too long ago, so I guess the Simplefin docs are not quite up to date :/ |
||
| newTrans.remittanceInformationUnstructured = trans.description; | ||
| newTrans.transactionAmount = { amount: trans.amount, currency: 'USD' }; | ||
| newTrans.transactionId = trans.id; | ||
|
|
@@ -240,106 +252,3 @@ | |
| }, | ||
| }); | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| export class GenericSimplefinError extends Error { | ||
| details: object; | ||
| constructor(data = {}) { | ||
| super('GoCardless returned error'); | ||
| this.details = data; | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| export interface CustomRequestOptions { | ||
| headers?: { [key: string]: string | number }; | ||
| port: number; | ||
| method: string; | ||
| } | ||
|
|
||
| export interface HttpClient { | ||
| request(url: string, options: CustomRequestOptions): Promise<string>; | ||
| } | ||
|
|
||
| import https from 'https'; | ||
|
|
||
| export class HttpsClient implements HttpClient { | ||
| request(url: string, options: CustomRequestOptions): Promise<string> { | ||
| return new Promise((resolve, reject) => { | ||
| const req = https.request(new URL(url), options, (response) => { | ||
| // reject on bad status | ||
| if (response.statusCode < 200 || response.statusCode >= 300) { | ||
| return reject( | ||
| new Error(`${response.statusCode} ${response.statusMessage}`), | ||
| ); | ||
| } | ||
| let data = ''; | ||
| response.on('data', (d: Buffer) => { | ||
| data += d.toString(); | ||
| }); | ||
| response.on('end', () => { | ||
| resolve(data); | ||
| }); | ||
| }); | ||
| req.on('error', (e: Error) => { | ||
| reject(e); | ||
| }); | ||
| req.end(); | ||
| }); | ||
| } | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 built all the docker images and ran them, making sure the server ran and was accessible with
curl.