feat(accounts) : importing accounts from csv file

[mbayopanda]

This PR add the feature of importing accounts to use into the application from a csv file.

Close #3076
Close #3034

[jniles]

How many places to we use this logic? It looks like we use it in src/modules/inventory/list/modals/import.modal.js as well as src/modules/patients/documents/modals/documents.modal.js.

Since we use it in three place, I propose you make an issue for a directive that would do this calculation and rendering for you. Something like:

<bh-progress-meter current="currentProgress" total="totalProgress">
</bh-progress-meter>

That way, we can remove all this custom logic and just put the directive in the HTML. We can also test it to make sure it works the way we hope it to work.

[jniles]

We should add a translated error here - it is the second parameter on the BadRequest() constructor.

So you would do something like

new BadRequest('Something broke', 'ERRORS.EVERYTHING_BAD');

and then translate the ERRORS.EVERYTHING_BAD in the i18n files.

[jniles]

Why do we need params.option === IMPORT_CUSTOM_OHADA_ACCOUNTS twice? Could you leave a comment in the code? It looks like a bug from just reading the code.

[mbayopanda]

Twice in this condition or in this line and the line 52 ?

We have three options for loading accounts into the system :

• 1. Load default ohada accounts : this option doesn't require to send a csv file, so the system load the file directly from the server side.
• 2. Load custom ohada accounts : in this option, the user must add ohada accounts (by uploading a csv file) to default ohada accounts which are already loaded into the system.
• 3. Load accounts : this option is more generic, if the user need to load any kind of accounts (not ohada) into the system. the user must upload a csv file of accounts.

In the line 52, we test the first or the second option, in this case we must load ohada default accounts anyway.

In the line 57, we test the second or the third option, in this case we must load accounts from uploaded csv file of accounts.

[jniles]

This is an ideal place for an async/await function! You could do this:

async function importAccountFromFile(filePath, enterpriseId, option) {
  const data = await formatCsvToJson(filePath);

  if (!hasValidDataFormat(data)) {
    throw new BadRequest('The given file has a bad data format for accounts', 'ERRORS.BAD_DATA_FORMAT');
  }

  const transaction = db.transaction();

  data.forEach(item => {
    query = 'CALL ImportAccount(?, ?, ?, ?, ?, ?);';
    queryParams = [
      enterpriseId,
      item.account_number,
      item.account_label,
      item.account_type,
      item.account_parent || null,
      option,
    ];
    transaction.addQuery(query, queryParams);
  });
  
  return transaction.execute();
}

[jniles]

Looks like this function is a copy and paste of controllers/inventory/import/index.js's formatCsvToJson() function. Instead of copying and pasting the code, could we make a library for this and re-use the same code? It would also allow us to unit test this function.

[mbayopanda]

it is not totally a copy pasta, the line 120 is different (file.path versus file).

You are right, we can put this function in a library for unit test it.

[jniles]

Why does this need to be an SQL function? It doesn't look like it uses the database at all...

Can we do this in JavaScript? SQL functions are a pain to upgrade in production.

[mbayopanda]

The simple reason is this function is just used when we are in the option 1 and the account number has four digit, and the most important we cannot send this value (the predicted account type id) as parameters because it is not needed at anytime we call the ImportAccount procedure

[mbayopanda]

@jniles could you review this PR again please

[jniles]

Looks fine to me!

bors r+

[bors]

Build failed

• continuous-integration/travis-ci/push

[mbayopanda]

bors r+

[bors]

Build succeeded

• continuous-integration/travis-ci/push