Remove support for running validations during the client-side muting transition

[jkuester]

What feature do you want to improve?

Spinning this out of #9544 to give it proper attention and consideration (and particularly to get @dianabarsan's thoughts). Also, this is a per-requisite for #9544 and can be shipped ahead of that issue.

TLDR, as a part of #9544 we want to remove the key:value entries from the freetext search indexes to dramatically reduce the storage space used by these indexes on device. The goal of this issue is to remove validations support from webapp because the exists validation function relies on these key:value entries.

When the muting transition executes on Sentinel, it can run a set of configured validations on the muting/un-muting report before actually executing the action. The primary purpose here is to support muting via sms workflows (where you cannot really pre-validate the form data like you can when filling out an Enekto form in webapp).

Describe the improvement you'd like

When client-side muting was introduced into webapp, support was also included to continue running these validations when muting a client-side contact. The arguments for removing this functionality now are:

• For the most part the validations logic should instead be included in the Enekto form to prevent the user from ever submitting an invalid muting/un-muting report.
• The parts of the validations logic that are most difficult to re-create in an Enekto form (e.g. exists) are limited in scope when running offline (since they only have access to the data on the device) and so could produce different results than if the validation was run on the server. (e.g. the exists function does not find anything when running on the device, but could find something when running in Sentinel).

Because of this, and the perceived strong likelihood that no one is relying on validations for their webapp-based muting workflows the benefits of reducing disk usage on the device seem to outweigh the possible downsides of losing validations on client-side muting transitions.

Describe alternatives you've considered

Perhaps it would be possible to trigger the validations to run in Sentinel _for client-side muting/unmuting operations, once the muting reports are replicated to the server. Then maybe the server could roll-back the operation if the report fails the validations.

[mrjones-plip]

two questions:

1. I assume we don't think this is a breaking change per "perceived strong likelihood that no one is relying on validations for their webapp-based muting workflows" ? Conversely, I assume we won't ship this if we find that indeed folks are relying on these validations? (or we'll wait 'til 5.0)
2. Have we quantified the disk savings on handsets? given the scope of this change I wanted to be sure we knew the trade offs of saved space vs invasiveness of the change.

[jkuester]

we don't think this is a breaking change

Strictly speaking, this is definitely a breaking change in that it is removing functionality that previously existed. Pragmatically speaking, though, the question is if we really need to wait for a major release to remove functionality that is half-broken and probably unused.

Admittedly "probably unused" in that statement is doing a lot of heavy lifting. We don't have any telemetry in webapp around this functionality so we have no actual usage data. However, it should be possible to spot-check any configs we have access to and look at the muting config in the app-settings. Basically whenever muting.validations is populated we want all the entries in muting.mute_forms or muting.unmute_forms to have matching entries in forms. I think that would mean they are all SMS forms (and so there would not no muting/unmuting supported in webapp).

we won't ship this if we find that indeed folks are relying on these validations? (or we'll wait 'til 5.0)

If we find any cases where we have muting.validations configured for for app forms in webapp, then yeah, I think we should definitely not ship this in a minor release. We should take the opporutunity to better understand the workflows that partner is using and then we can decide if it still makes sense to remove this functionality in the next major release.

Have we quantified the disk savings on handsets?

AFICT this will be difficult to quantify with any super reliable precision since IndexedDB seems to like to do its own thing when it comes to garbage collection. However, I did a rough test with my user that has +6000 docs. Started by totally clearing all site data in my browser and doing a hard-reload of the app. Then logged in, synced all the docs, and clicked around enough to trigger the freetext indexes to be built. Then I measured the IndexedDB size and redid the whole thing again but without indexing the key:value pairs. The starting IndexedDB size with our current views was 183 MB. Without the key:value pairs the The IndexDB size went down to 140 MB (so -43 MB). So removing the key:value indexes resulted in a ~23% reduction in the total size of the CHT data on the device. Once again, this was a very rough test, but it does meet expectations based on the similar results we were seeing in Couch...

[mrjones-plip]

Thanks Josh! I don't think this meaningfully changes anything, but great to have the full info during the discussion

[dianabarsan]

I believe this to be a breaking change, no matter how we hard we try to squint.

and the perceived strong likelihood that no one is relying on validations for their webapp-based muting workflows

Given that we are branching out of medic-owned configurations and strongly encouraging partners and users to do their own thing, without any obligation to report back to us, we will have increasingly LESS visibility into what people are doing. So "perceptions" will not be enough to cut out features without qualifying them as breaking changes. We should assume that features are being used.

With that, I think validations for muting forms are nice, both server and client side. It sucks how we have sync and async validations that depend on data and we're forced to have them all together.

I think saving 43MB is not super significant for the client. It's the equivalent of ~30 seconds of video capture or one Android app.
Do we have proof that not having those 43MB increases app performance in a significant way?

[jkuester]

I believe this to be a breaking change.... We should assume that features are being used.

Fair. This is the kind of push-back I was expecting here. I do think it is valuable to have had the discussion, but ultimately, I 100% agree with your assessment of the situation. (Had to at least give the squinting a red-hot go before moving on!)

With that, I think validations for muting forms are nice, both server and client side.

I still don't think I fully appreciate the value you see in client-side validations for the muting transition (at least not at the data cost we are paying for it...). That being said, the key:value index entries could provide value for new functionality in the future. (e.g. one way that we could implement duplicate prevention for reports/contacts would track very close to the functionality of the exists validation.) If this was a case where we were proposing to add these index entries, I think I would still be opposed to it, but as things stand now where the index entries already exist, I will concede that I do not believe we have adequate justification to remove them at this time.

I think saving 43MB is not super significant for the client. It's the equivalent of ~30 seconds of video capture or one Android app.
Do we have proof that not having those 43MB increases app performance in a significant way?

My gut reaction as an engineer is to favor the ~23% reduction in storage usage regardless of what that translates to in actual MB. However, I agree that pragmatically this seems unlikely to result in much improvement to a users experience. I don't have hard numbers, but can say that anecdotally once the indexes have been built things like search time do not feel meaningfully different with or without the key:value indexes. And 43MB does not seem like a make-or-break number for filling up storage on modern Android devices.

Ultimately, the client bottleneck is in the syncing speeds (and not device storage). By the time the doc count starts to really affect the index sizes or storage space, we probably have long-since passed the amount of docs that we recommend syncing to an offline user.

Because of all this, I am going to go ahead and just close this issue. We do not want to make this change now and it is unclear if we would ever actually want to do this in the future.