More validation rules

[kaitlyn-sharo]

• "If there are N scenario.scenes, then scenario.scenes[0] through N-1 must contain transitions"

• Potentially validate that the keys in scenario.scenes[n].action_mapping.parameters are consistent with the action list.

• "[Field1] value must be consistent with [field2]

Example: scenario.state.characters.demographics.mission_importance must be consistent with scenario.state.mission.critical_ids

That is, within scenario.state

                  for each characters.id FOO with characters.demographics.mission_importance BAR,

                        there must be an entry in mission.critical_ids with key FOO and value BAR.

[nextcen-dgemoets]

We don't appear to be validating that the value for an entry in the character_importance field is from the controlled vocabulary (none, normal, important, priority, vip). We do still get a message saying there's a mismatch with mission_importance, but do you think we should flag that they specified something that's not part of the vocab? I don't feel strongly either way, since an error is flagged either way. But wanted to see what you think.

BTW, if they both use the same incorrect term, it flags that mission_importance must be one of the following values: ['none', 'normal', 'important', 'priority', 'vip'], so you still get at least the one error.

[nextcen-dgemoets]

If character_importance is missing, I'm not getting the mismatch errors I expect (e.g., if I set somebody's mission_importance to important.

Also, if I include character_importance with no entries, I get a traceback. It should be equivalent to supplying character_importance with everybody specified as normal. (Although of course it accomplishes nothing to include an empty character_importance, but it shouldn't crash.)

[nextcen-dgemoets]

Uh-oh...I just thought of something else we could/should validate . It could be in another ticket. We should validate that the specified parameters in an action mapping make sense. That is, a SITREP action shouldn't have a "treatment" parameter (actually, it shouldn't have any parameter). And an APPLY_TREATMENT action shouldn't have a "foobar" parameter (or more likely, a "tretment" parameter, that is, a typo).

There can come a point where we're going overboard with validation rules. I'm open to the suggestion that we shouldn't bother with this, but it means we'll silently ignore a treatment parameter because they accidentally misspelled it. I'm open to forgetting about this, or creating a ticket but not addressing it / making it low-priority.

[kaitlyn-sharo]

If character_importance is missing, I'm not getting the mismatch errors I expect (e.g., if I set somebody's mission_importance to important.

Also, if I include character_importance with no entries, I get a traceback. It should be equivalent to supplying character_importance with everybody specified as normal. (Although of course it accomplishes nothing to include an empty character_importance, but it shouldn't crash.)

Sorry, this was just due to a misunderstanding on my part. At one point I thought you said they were allowed not to have it at all in which case we wouldn't need to check it, but you just meant they don't have to have it if everyone is normal. I'll fix that!

[kaitlyn-sharo]

@nextcen-dgemoets I think I have addressed all applicable comments:

• Check character_importance to make sure it comes from the MissionImportanceEnum
• If character_importance is empty, that is equivalent to saying everything is "normal". therefore, if a character has "important" and character_importance is empty, we need to error
• Make sure if character_importance is empty, we don't get a python error

[kaitlyn-sharo]

Uh-oh...I just thought of something else we could/should validate . It could be in another ticket. We should validate that the specified parameters in an action mapping make sense. That is, a SITREP action shouldn't have a "treatment" parameter (actually, it shouldn't have any parameter). And an APPLY_TREATMENT action shouldn't have a "foobar" parameter (or more likely, a "tretment" parameter, that is, a typo).

There can come a point where we're going overboard with validation rules. I'm open to the suggestion that we shouldn't bother with this, but it means we'll silently ignore a treatment parameter because they accidentally misspelled it. I'm open to forgetting about this, or creating a ticket but not addressing it / making it low-priority.

I think for this one, I could go either way. If it's just going to silently ignore it, that's fine because it won't break anything. However, if they decide to have a treatment parameter that would do something if it was sent to the server, but they misspell it, then it won't be implemented because it'll be ignored. It might be safe to have a check, in that case. However, I don't think it's necessary to say "SITREP shouldn't have a treatment parameter". I think we just need to do "parameters are only allowed to come from this set [x, y, z]" to combat typos.

[nextcen-dgemoets]

I think for this one, I could go either way. If it's just going to silently ignore it, that's fine because it won't break anything. However, if they decide to have a treatment parameter that would do something if it was sent to the server, but they misspell it, then it won't be implemented because it'll be ignored. It might be safe to have a check, in that case. However, I don't think it's necessary to say "SITREP shouldn't have a treatment parameter". I think we just need to do "parameters are only allowed to come from this set [x, y, z]" to combat typos.

I think that's a good compromise: validate that specified parameters are from the allowed set of parameters across all actions, but don't validate that the parameters they specify apply to a given action.

[kaitlyn-sharo]

validate that specified parameters are from the allowed set of parameters across all actions

Created new ticket 181