Add form load time to xforms and es forms mapping

[Charl1996]

Review by commit. The vast majority of the files are test updates (the last commit), so don't panic and simply review by commit.

This PR is a duplicate of this one, but with the commit history cleaned up.

Product Description

No visible changes.

Technical Summary

Mobile ticket
HQ Ticket

A new form meta field is added, formLoadTime, which tracks how long a form takes to load. The mobile work has been done by Ahmad, while this is the HQ part.

To generate the ES migration I ran the following command:

./manage.py make_elastic_migration --update-index forms:form

Feature Flag

No specific FF

Safety Assurance

Safety story

Tested locally. Staging testing to be done.

Automated test coverage

No automated testing yet

QA Plan

No QA planned. QA will ensue as part of this PR.

Migrations

• The migrations in this code can be safely applied first independently of the code

Rollback instructions

• This PR can be reverted after deploy with no further considerations

This PR contains an elastic search mapping update migration which cannot be reverted. In the event of a revert a simple comment in the mapping file should indicate that the field should be removed during the next reindex.

Labels & Review

• Risk label is set correctly
• The set of people pinged as reviewers is appropriate for the level of risk of the change

[mkangia]

Great job on getting those tests fixed so soon.

[mkangia]

@AmitPhulera
Just confirming that this is the right type for this?

This value is going to be in miliseconds.

[Charl1996]

@AmitPhulera suggested that we think about storing this as a date field. I'm quickly finding out what the timeStart refers to. If it refers to the timestamp a user clicked to open a form, we could store the formLoadTime as a date and not a difference. The benefit of doing this is that we could later do all sorts of date queries on this field if we wanted to.

[mkangia]

Hey @Charl1996
Whats the challenge with how it is currently?

I don't believe we can do this. I believe we need the time it took to load the form. I don't think we can get that using timeStart and any other timestamp.

[AmitPhulera]

My understanding is that we are storing epoch time in formLoadTime, Is that correct. If so that is a essentially a date field so it should be treated as date in the mappings unless there is a strong reason to keep it as a keyword.

[Charl1996]

@AmitPhulera

My understanding is that we are storing epoch time in formLoadTime

The value stored in this field is something like "230" or "190", which denotes the duration in milliseconds it took to load the form. It's not epoch time

[AmitPhulera]

Ahh! I see. Feel free to discard my suggestion about storing it as date.
Another question, why are we not storing it as integer then?

[Charl1996]

Mmm, I suppose integer would be better, yes.

[mkangia]

Great work @Charl1996

Just couple of things:

1. Lets have steps on the PR in case this needs to be reverted? I guess @AmitPhulera can guide us on it.
2. Is this something we can take through QA as well? I guess it would be good to confirm that form submissions from different places will work well. If you are confident that all should go well and have done your own testing on staging, that is fine then.

[Charl1996]

@mkangia

Is this something we can take through QA as well?

I was thinking to involve QA with this PR and only do testing on staging myself.

Lets have steps on the PR in case this needs to be reverted?
Good idea.

@AmitPhulera Would a revert require a new migration? I couldn't simply revert locally, so I'm wondering if I need to create a new migration to revert it?

[mkangia]

I was thinking to involve QA with this PR and only do testing on staging myself.

Hey @Charl1996 not sure I understand this. Is QA with this PR or the second one?

[Charl1996]

@mkangia

I was thinking to involve QA with this PR and only do testing on staging myself.

Hey @Charl1996 not sure I understand this. Is QA with this PR or the second one?

Sorry, I mistyped. I'll go through formal QA (with QA team) only on second ticket. For this one me and Ahmad is going to test ourselves.

[Charl1996]

But for now, the deploy to staging is failing to migrate. Ugh

[AmitPhulera]

@AmitPhulera Would a revert require a new migration? I couldn't simply revert locally, so I'm wondering if I need to create a new migration to revert it?

Hey @Charl1996, Its not possible to delete the fields form mappings in an index after they are created. Only thing that can be done is adding a comment in the mappings file to remove the attribute when the next reindex happens.

[Charl1996]

Update:
The plan is to change the field from keyword to integer. The way this is going to work is that I'm going to add a new field called loadTime and we'll treat the current field, formLoadTime, as deleted. This is because the process to reindex is a big lift.

This does not mean app changes and most of this PR will stay the same, but there will be some mapping to be done between ES and HQ, which I'm in the process of updating.