Add spec for Action.Submit associated inputs

[dclaux]

Related issue

[NodeJS][UX] Unable to have two SubmitAction Buttons and validation #5078

Description

This PR adds a small spec to allow actions to bypass input validation.

Microsoft Reviewers: Open in CodeFlow

[ghost]

Hi @dclaux. This non-spec pull request has had no recent activity for the past 5 days . Please take the necessary actions (review, address feedback or commit if reviewed already) to move this along.

[RebeccaAnne]

My only concern with this is whether it's consistent with what we may someday want to do for associated inputs. The thinking on that was that we'd have some sort of associated inputs property that could be "All" "None" or an array of ids (default to All for Submit actions). If we had that, it would be redundant with this. We could name this in such a way that it more naturally expands to that. For example a property called validationInputs (or something) that today could be "All" or "None" (default to All, only available on Submit/Execute actions today) but in the future could expand to include an array of selected inputs and/or more input types if we want.

[dclaux]

Totally fair, and I have no problem using a design that would work well with that future evolution. Two options IMO:

1. I don't think ignoreInputValidation would interfere/be redundant. We could later introduce an associatedInputs property which would list the ids of the inputs the action is tied to in an array. If unspecified, the assumption is "all the inputs" e.g. our current behavior. If specified, it has to be the exhaustive list of associated inputs. 'ignoreInputValidation` would remain the way to say "don't validate any input"

2. Instead of ignoreInputValidation we go for something like associatedInputs whose value can be:

   • unspecified or "all": validate all the inputs
   • "none": don't validate any input
   • Array of input Ids: only validate these inputs

The JS SDK would be able to accommodate option 2 without introducing any breaking change.

I am not sure I have a strong preference, but obviously I'd go with option 1 since JS already implements ignoreInputValidation.

[RebeccaAnne]

I think I prefer 2, it feels a little cleaner to me, but let's talk about it at the Wednesday meeting this week and run it by the team.

[golddove]

I like option 2.

Specifically, the first iteration could be something like:

associatedInputs = auto (default) | none

Where auto is equivalent to the current behavior, and none disables validation.

We can later introduce all and [id's...] as possible options when we are ready to ship associated inputs.

But one question - does an "unassociated" input also not get submitted in the data of that submit? i.e. Is enabling validation coupled with submitting data?

[dclaux]

The Action.Http model in Outlook has always relied on explicitly referenced inputs (although using a different mechanism) and only the values of the referenced inputs are passed to the service. It seems to me that we should follow the same model.

@golddove do you see a difference between "auto" and "all"? Do you think we will really need both? So far, we've only ever done "all" (although we slightly changed the behavior when we introduced validation) so I'm not sure "auto" is necessary.

[golddove]

In that case, going back to this bug, ignoring input validation would also have the side effect of disabling all data submission. Are we okay with that limitation? One option is to partition the data (our API would provide a list of associatedInput data, and otherInput data on submit).

And, yes, I'm referring to that slightly changed behavior we introduced (which doesn't submit all inputs). If we don't want to introduce a breaking change again on all renderers for this, we have to retain that as the default.

As for whether we really need the true all option that submits all inputs, I think it would be nice, but don't have a strong opinion on it. That's something we don't have to decide now.

[dclaux]

Not if we keep ignoreInputValidation as a separate flag. With that model it's clear that:

• You enable or disable input validation using ignoreInputValidation
• You specify which input values need to be transmitted to the backend service using associatedInputs - and if ignoreInputValidation is false, those inputs are also validated

[dclaux]

And by the way I don't believe we need to distinguish between "all" and the "almost all" model we now have. "all" is enough IMO.

[RebeccaAnne]

I don't think we should ever be sending un-validated data back to the backend. That seems likely to cause problems as code may assume that all inputs with validation have been validated client side. I think that validating and submission should be tied, so if it's not validated that's the same as not sent. Does anyone have a scenario where someone would want to send back an input that has validation without enforcing that validation? I think a schema/behavior that lets you ignore validation but still submit the inputs is making it a little too easy for card authors to shoot themselves in the foot. To me that means at the very least we should rename ignoreInputValidation to make it clear it affects submission as well. Really though I think we should just go with option 2.

On the auto/all topic, I agree with Risheek that we should call the value "auto" because explicitly calling it "all" and then not sending all the inputs is weird. Our behavior is a bit non-intuitive here already, and I don't think we'll improve the situation by "lying" about it :)

For what it's worth I don't feel strongly about whether we'll ever need to introduce an actual "all." It might come in handy, but if we have associated inputs the card author could do it manually if they really want. In any case that's a discussion for another day and outside the scope of this issue.

[ghost]

Hi @RebeccaAnne; Thanks for reviewing this previously stale pull request. Resetting staleness. @dclaux FYI.

[RebeccaAnne]

Per discussion we have agreed to add an associatedInputs property to Action.Submit. Current possible values are:
associatedInputs = auto (default) | none

If none is set, the values are neither validated nor returned to the host as part of the submit event.

[dclaux]

Spec is updated according to our decision.