ref: migrate fetch form submission counts flow to Typescript

[karrui]

Problem

This PR add full flow for GET /{formId}/adminform/submissions/count endpoint. This initial implementation is free of middlewares, and all checks are done in the handler. Authentication middleware is being used for the route now that #568 is in.

Note that the routes are not migrated to typescript yet, and will be done in a separate PR.

Solution

Features:

• Add SubmissionService#getFormSubmissionsCount neverthrown function
• Add adminFormUtils#assertHasReadPermissions function to check if user has read permissions.
• Update app.route('/:formId([a-fA-F0-9]{24})/adminform/submissions/count') handler flow
  • Remove all middleware usage, use new AdminFormController#handleCountFormSubmissions function
  • Add joi validation for valid query.startDate and query.endDate params.

Improvements:

• move submission module tests into modules/submissions/__tests__ directory
• add JSDoc + stricter types for dateUtils#createQueryWithDateParam
• move UserController#getUserIdFromSession to authUtils#getUserIdFromSession

Bug Fixes:

• Correct typings of SubmissionModel to better follow schema

Tests

• Add handleCountFormSubmissions tests in admin-form.controller.spec.ts
• Add getFormSubmissionsCount tests in submission.service.spec.ts

[mantariksh]

if we use isErr like this, isn't it basically as good as try-catch? do you think it might be cleaner to actually chain the results and use a single map and mapErr at the end?

[karrui]

It's different from try-catch though. To reach the next step, you have to actively unwrap the Result unlike in try catch where devs may forget to throw.

Doing it this way allows for better error logs, though it also makes sense to chain them and map/mapErr since the service themselves already logs errors

Thoughts, @liangyuanruo?

[mantariksh]

yeah imo it's fine for the logs to get more generic at higher and higher layers, as long as each layer is responsible for logging the error. I think it's better to have cleaner code here by taking advantage of chaining, since there's no significant benefit to having duplicative log messages between the controller and service.

[karrui]

We just go back and forth, the initial implementation was chaining, then it became distinct steps, then etc etc HAHAHA. I'll go with this for now, can always open a PR to chain it again. Too many rebases now :P

[liangyuanruo]

yup this is ok with me too, as long as the Result type is narrowed by testing with isErr(). I wouldn't want to be too opinionated here.

[mantariksh]

lgtm, the question on chaining is still outstanding but not a dealbreaker