Code Enhancements (tech debt and improvements)

[maxceem]

In this ticket I we can list small improvements that we might consider to make to keep the code clean, consistent and less prone to accidental issues.

• Rename commands create-index and delete-index to es:create and es:delete. Create one more command, es:reset that would delete index if exist and create it again without throwing error, but only logging if index was previously exists or no.
  - these commands should support creating/deleting individual indexes, or all indexes all together
• Update command init-db:
  • it should prompt for confirmation as all other commands which might remove some data. and support flag --force to skip confirmation.
  • it should always try to delete the DB first, before creating it, and log info if DB previously existed or no
  • move file from src directory to scripts/db folder
  • rename it to db:reset
• Refactor src/common/helper.js which became too big:
  • put esIndexPropertyMapping into a separate file as it is a config, maybe to config/esMapping.js?
  • put methods related to command/scirpts into a separate file like src/common/commandUtils.js
  • put method related to ES indexing into a separate file like src/common/esUtils.js
  • put method related to data import/export into a separate file like src/common/dataUtils.js
  • consider extracting some other methods if makes sense
• commands index:* can index in bulks and documents one by one. During bulk indexing we are setting refresh: true (https://github.com/topcoder-platform/taas-apis/blob/dev/src/common/helper.js#L245). And during indexing separate documents we don't set refresh at all https://github.com/topcoder-platform/taas-apis/blob/dev/src/common/helper.js#L276-L280.
  • I suggest keeping it consistent and set the same value in both cases bulk and single document indexing (does it makes sense?)
  • also, this value should be configured using env variable similar way as we config MAX_BULK_REQUEST_SIZE_MB, MAX_BULK_NUM_DOCUMENTS
  • see how refresh works https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-refresh.html
• all the dangerous scripts have confirmation prompt. At the moment we can just press Enter without typing Y - this is not what we want. But default if we press Enter it should mean N and we have explicitly confirm operation by entering Y.
• For consistency and correct naming instead of writing Bearer <token> inside req.authUser.jwtToken we have to only write <token> inside req.authUser.jwtToken without word Bearer see https://github.com/topcoder-platform/taas-apis/blob/dev/app-routes.js#L51. Because it could be confusing that jwtToken also contains word Bearer inside.
  We also have to update such places like this:

  let token
  if (currentUser.hasManagePermission || currentUser.isMachine) {
    const m2mToken = await getM2MToken()
    token = `Bearer ${m2mToken}`
  } else {
    token = currentUser.jwtToken
  }
  

  variable token here misleads, it should be something like authorizationHeader. Or we should assign a real token to the token variable, and later write .set('Authorization', \Bearer ${token}`). Such inconsistency is also caused by the fact that we don't store a pure token inside req.authUser.jwtToken`.
• we are following the rule: all the endpoints should pass data to ES from the DB. And all the endpoint should return data from the DB, not the data that we pass to the endpoint like create or update. But at the moment we are returning data to ES and from endpoints in a different way. We pass data to ES like this created.toJSON() but return like this created.dataValues. I guess we should make this consistent and always use toJSON() when return data from endpoints. See for example:

   await helper.postEvent(config.TAAS_RESOURCE_BOOKING_CREATE_TOPIC, created.toJSON())
   return created.dataValues
   
   // or
   
   await helper.postEvent(config.TAAS_WORK_PERIOD_UPDATE_TOPIC, updated.toJSON(), { oldValue: oldValue })
   return updated.dataValues