refactor: replace $resource in angularjs form-api.client.factory.js with typescript FormService

[chowyiyin]

Problem

This PR replaces the $resource object in form-api.client.factory.js with API call services provided byFormService.ts. Due to coupling between the interceptors and form-fields.client.factory, interceptors are kept intact in form-api.client.factory.js. Previously, the accessMode query parameter was also added to all API calls. This migration removes this from any API calls in which it is irrelevant.

Closes #1835, #1768

Solution

getInterceptor is called in form-api.client.factory.js and the relevant interceptor is passed to functions in FormService.ts, that handle all API calls.

Tests

Unit Tests

• FormService.test.ts: add unit tests for functions in FormService.ts

Manual Tests

• Load forms dashboard page. Check that all available forms are present. (query)
• Click on any form. Should be redirected to edit page. Check that previous fields (if any) are present. (getAdmin)
• Click on open in new window in share tab without activating form. Should be redirected to error page. (getPublic)
• Click on open in new window in share tab after activating form. Should be redirected to public view of form. (getPublic)
• Duplicate a field in a form. Check that the field is duplicated correctly i.e. contents of the field are correct (update)
• Duplicate any form. Ensure duplicate form modal appears and duplicated form appears on the dashboard upon completion (save)
• Delete a form. Ensure that the form is no longer visible on the dashboard (delete)
• Create a form from scratch in storage mode and email mode. Should be redirected to edit page with no existing fields. Check that form appears on dashboard. (create)
• Select create new form and select any template. Modal with template form in public view should be displayed. (template)
• Select use template (following from previous check). Perform same checks done for create form.
• Select preview mode of any existing form. Should be redirected to preview page. (preview)
• Add a collaborator and change role to owner. Ownership should have changed. (transferOwner)
• Create myinfo fields. Check that myinfo fields are correctly stripped and injected.
• Duplicate myinfo fields. Check that myinfo fields are correctly stripped and injected.
• Duplicate a form with myinfo fields. Check that myinfo fields are correctly stripped and injected.
• Check that myinfo fields are correctly injected into public form view
• Check that forms can be createdfrom the examples page
• Check that forms can be created from the "Share Template" link under the "Share" tab

[liangyuanruo]

@mantariksh @chowyiyin Thanks for working on this - I'm thinking it could help with the React migration to also take a look at organising the different services better - FormApi, FormFields, FormService etc, probably in a separate PR.

[mantariksh]

This was a tough one to disentangle, great job!

When thinking about the implementation of the services, our first priority is to ensure that they are reusable in React. The point of the frontend services migration is to maximise the amount of code which is battle-tested in production in order to minimise the risk of the final cutover to React.

The FormService is currently doing quite a lot: injecting/stripping MyInfo data, converting POJOs into classes, handling error redirects etc. The issue with this is that it's entirely possible that we will handle all these things completely differently in React. For example, it's unlikely that we will port over the existing viewmodel classes (e.g. Form.class.js) into React.

I think a better approach might be to make the service dumb - it takes in the relevant data to call the correct endpoint, and returns the response. That's it. Any additional formatting of the request and response must be done by the caller. Then in the existing AngularJS FormAPI service, we perform all the necessary transformations on the request and response before and after calling the new TypeScript service. This way we can be sure that the new service is reusable - it's completely framework-agnostic as it only makes API calls.

Wdyt? Looping in @karrui and @tshuli too

[chowyiyin]

This was a tough one to disentangle, great job!

Thank you! :)

When thinking about the implementation of the services, our first priority is to ensure that they are reusable in React. The point of the frontend services migration is to maximise the amount of code which is battle-tested in production in order to minimise the risk of the final cutover to React.

The FormService is currently doing quite a lot: injecting/stripping MyInfo data, converting POJOs into classes, handling error redirects etc. The issue with this is that it's entirely possible that we will handle all these things completely differently in React. For example, it's unlikely that we will port over the existing viewmodel classes (e.g. Form.class.js) into React.

I think a better approach might be to make the service dumb - it takes in the relevant data to call the correct endpoint, and returns the response. That's it. Any additional formatting of the request and response must be done by the caller. Then in the existing AngularJS FormAPI service, we perform all the necessary transformations on the request and response before and after calling the new TypeScript service. This way we can be sure that the new service is reusable - it's completely framework-agnostic as it only makes API calls.

Do correct me if I am wrong, but I think this service is, in a way, framework agnostic since the things mentioned above (e.g. injecting and stripping MyInfo data) are actually handled in the interceptor? FormService simply uses the interceptor that is passed in. The way the interceptor transforms the response and request is up to the caller to decide (in this case, form-api.client.factory.js.

[mantariksh]

Do correct me if I am wrong, but I think this service is, in a way, framework agnostic since the things mentioned above (e.g. injecting and stripping MyInfo data) are actually handled in the interceptor?

This is true! But the reasons for needing interceptors are not framework-agnostic, eg the fact that a Form POJO has to be converted into a Form class. This reflects itself in the code too, e.g. in how the Interceptor type requires a responseError, which is needed purely because of how we handle redirects in AngularJS.

Furthermore, if the service just trusts the interceptors passed to it, then ultimately the caller has to implement the interceptor correctly, meaning there isn't much value in terms of code reuse.

[tshuli]

Agree with @mantariksh on scoping the service to calling the correct API and returning the response - I think this is also consistent with how we've written the other services!

[mantariksh]

first round of review, let's address these first before the next round!

[mantariksh]

Blocked by #2061

[mantariksh]

lgtm, really well done! let's test this super thoroughly on staging because it touches nearly every single important API. you can add the following checks to your manual tests:

• myinfo fields are correctly stripped and injected when creating myinfo fields
• myinfo fields are correctly stripped and injected when duplicating myinfo fields
• myinfo fields are correctly stripped and injected when duplicating a form with myinfo fields
• myinfo fields are correctly injected into public form view
• forms can be created from the examples page
• forms can be created from the "Share Template" link under the "Share" tab

[mantariksh]

I actually think the current implementation of extractFormId is wrong because we don't always have the form ID at the start of the path, but that can be fixed separately

[mantariksh]

thanks for the thorough testing! I know it's a lot of work but I hope the importance is clear, since this PR touches a lot of core functionality.