👻 Add questionnare required field to assessment

[ibolton336]

This is needed to aid UI in computing status for an assessment given a questionnaire is marked as not required. If an assessment exists for an archived questionnaire on an app or archetype, the assessment status is no longer valid & the assessment will need to be filtered from status consideration.

https://issues.redhat.com/browse/MTA-1956

[mansam]

Good work! I've got some comments that might let you simplify your approach.

[ibolton336]

@mansam Wasn't sure if this was the best way here- When trying

Cannot convert 'nil' to type 'Questionnaire'

I was seeing this type error. Is using the ID here acceptable?

[mansam]

You're getting that error about converting nil because Assessment.Questionnaire is a struct instead of a struct pointer. Because it's a struct and not a struct pointer, when the relationship isn't preloaded the struct will be present but all the fields will be zero-valued. So checking if the Questionnaire struct's ID field is 0 like that is a reasonable way to check if it's been preloaded. (Probably that field should be changed to a struct pointer like the rest of the model relationships so that it's more obvious when it hasn't been loaded.)

There are two improvements we can make here. One is that instead of doing this check in the handler logic in all the various places where we need to do it, we can move the check and the assignment into the Assessment.With method.

The second is that the zero-value of a boolean is false, so we can take advantage of that to do away with the check. assessment.Required is going to default to false anyway if you don't assign to it because the questionnaire isn't loaded, and the unloaded questionnaire's Required will default to false, so the assignment only matters if the Questionnaire is loaded and and required. So these two expressions are equivalent:

if assessment.Questionnaire.ID != 0 {
    assessment.Required = assessment.Questionnaire.Required
}

assessment.Required = assessment.Questionnaire.Required

(If we later fix assessement.Questionnaire to be a struct pointer, then we'll need to add a check for nil, but for now this works well.)

[mansam]

When you add a field with a camelCased name, be sure to add both a json and a yaml field tag with the camelCased spelling. (see the RiskMessages field on the line above). This prevents a problem where the yaml encoder will emit it from the api as camelcased but the strict decoder will not recognize it because it expects camelCased.

nitpick: Lets just call this Required for simplicity and for symmetry with the other fields that are ultimately inherited from the questionnaire.

[mansam]

LGTM!