Fix incorrect usage of exchange rate API service #7

Open
opened 4 months ago by inga-lovinde · 0 comments
Owner

According to the README:

Please use a currency rates API (https://api.exchangerate.host/2021-01-01) for transactions in currency other than Euros.

In exchange-rate.dto.ts, the following is hardcoded:

export enum URL {
  exchangeRateConvertUrl = 'https://api.exchangerate.host/{date}',
}

and the requests are made in exchange-rate.service.ts by

    return this.httpService
      .get(URL.exchangeRateConvertUrl.replace('{date}', input.date))

Additionally, the return type is declared as ExchangeRateResponse, and is obtained by

      .pipe(
        map((axiosResponse: AxiosResponse) => {
          return axiosResponse.data?.rates;
        }),
      );

where ExchangeRateResponse is declared as

export type ExchangeRateResponse = {
  motd: {
    msg: string;
    url: string;
  };
  success: true;
  query: {
    from: string;
    to: string;
    amount: number;
  };
  info: {
    rate: number;
  };
  historical: boolean;
  date: string;
  result: number;
};

Additionally, it is then used in transaction.controller.ts as

      this.exchangeRateService.convertCurrency(exhangeRateInput).subscribe({
        next: async (exchangeRateResponse) => {
            ...
            base_amount: parseInt(transactionInput.amount) * exchangeRateResponse[transactionInput.currency],

and it is mocked in transaction.controller.spec.ts as

    const ExchangeRateServiceProviderForUsd = {
      provide: ExchangeRateService,
      useFactory: () => ({
        convertCurrency: jest.fn(({ amount }) => ({
          subscribe: ({ next }) => next({ USD: 1.2 }),
        })),
      }),
    };

There are multiple layers of problems here:

  1. api.exchangerate.host requires an API key which is not provided, so the request will fail with 403 or similar error;
  2. According to documentation https://exchangerate.host/documentation , api.exchangerate.host does not have endpoints of the form https://api.exchangerate.host/2021-01-01 ; what it does have is endpoints like https://api.exchangerate.host/historical?access_key=YOUR_ACCESS_KEY&date=2021-01-01
  3. According to the documentation, this endpoint returns response with scalar fields success, terms, privacy, historical, date, timestamp, and source; and with dictionary field quote, containing entries like "USDAED": 3.67266. Yet exchange-rate.service.ts is attempting to return a non-existent field axiosResponse.data?.rates.
  4. According to exchange-rate.dto.ts, the service returns a complex structure, yet transaction.controller.ts expects it to be a dictionary containing entries like "USD": 1.2. In strict TS mode this would be a compiler error (trying to index a specific complex shape by an arbitrary string; trying to use the resulting value as a number), but most strict features of TS are disabled or not enabled in tsconfig.json, so this slips unnoticed;
  5. Similarly, transaction.controller.spec.ts mocks exchange rate service to return what transaction.controller.ts expects, not what exchange-rate.service.ts is declared to return;
  6. Additionally, it is unclear what is the meaning of the mocked value. Is it how many USDs are there in one EUR, or how many EURs are there in one USD? The fact that the only test checking this scenario seems to assume that "USD": 1.2 seems to mean that one USD is worth 0.8(6) EUR, which is odd (and also, why is the base currency of exchange service EUR, if we don't pass any currencies to it?)

The entire part of the code around currency conversion is broken, the only way to fix it is to rewrite it.

According to the README: > Please use a currency rates API ([https://api.exchangerate.host/2021-01-01](https://api.exchangerate.host/2021-01-01)) for transactions in currency other than Euros. In `exchange-rate.dto.ts`, the following is hardcoded: ``` export enum URL { exchangeRateConvertUrl = 'https://api.exchangerate.host/{date}', } ``` and the requests are made in `exchange-rate.service.ts` by ``` return this.httpService .get(URL.exchangeRateConvertUrl.replace('{date}', input.date)) ``` Additionally, the return type is declared as `ExchangeRateResponse`, and is obtained by ``` .pipe( map((axiosResponse: AxiosResponse) => { return axiosResponse.data?.rates; }), ); ``` where `ExchangeRateResponse` is declared as ``` export type ExchangeRateResponse = { motd: { msg: string; url: string; }; success: true; query: { from: string; to: string; amount: number; }; info: { rate: number; }; historical: boolean; date: string; result: number; }; ``` Additionally, it is then used in `transaction.controller.ts` as ``` this.exchangeRateService.convertCurrency(exhangeRateInput).subscribe({ next: async (exchangeRateResponse) => { ... base_amount: parseInt(transactionInput.amount) * exchangeRateResponse[transactionInput.currency], ``` and it is mocked in `transaction.controller.spec.ts` as ``` const ExchangeRateServiceProviderForUsd = { provide: ExchangeRateService, useFactory: () => ({ convertCurrency: jest.fn(({ amount }) => ({ subscribe: ({ next }) => next({ USD: 1.2 }), })), }), }; ``` There are multiple layers of problems here: 1. `api.exchangerate.host` requires an API key which is not provided, so the request will fail with 403 or similar error; 2. According to documentation https://exchangerate.host/documentation , `api.exchangerate.host` does not have endpoints of the form https://api.exchangerate.host/2021-01-01 ; what it does have is endpoints like https://api.exchangerate.host/historical?access_key=YOUR_ACCESS_KEY&date=2021-01-01 3. According to the documentation, this endpoint returns response with scalar fields `success`, `terms`, `privacy`, `historical`, `date`, `timestamp`, and `source`; and with dictionary field `quote`, containing entries like `"USDAED": 3.67266`. Yet `exchange-rate.service.ts` is attempting to return a non-existent field `axiosResponse.data?.rates`. 4. According to `exchange-rate.dto.ts`, the service returns a complex structure, yet `transaction.controller.ts` expects it to be a dictionary containing entries like `"USD": 1.2`. In strict TS mode this would be a compiler error (trying to index a specific complex shape by an arbitrary string; trying to use the resulting value as a number), but most strict features of TS are disabled or not enabled in `tsconfig.json`, so this slips unnoticed; 5. Similarly, `transaction.controller.spec.ts` mocks exchange rate service to return what `transaction.controller.ts` expects, not what `exchange-rate.service.ts` is declared to return; 6. Additionally, it is unclear what is the meaning of the mocked value. Is it how many USDs are there in one EUR, or how many EURs are there in one USD? The fact that the only test checking this scenario seems to assume that `"USD": 1.2` seems to mean that one USD is worth 0.8(6) EUR, which is odd (and also, why is the base currency of exchange service EUR, if we don't pass any currencies to it?) The entire part of the code around currency conversion is broken, the only way to fix it is to rewrite it.
Sign in to join this conversation.
No Label
No Milestone
No project
No Assignees
1 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: inga-lovinde/test-assignment-payments#7
Loading…
There is no content yet.