Select list petition attribute fix for empty and required settings

[muisit]

The use case is: enrollment flows that have 'Affiliation' as attribute (explicitely) without a default generate a Select element without an empty option, causing the first option to be a default anyway.

This PR implements a fix in the following way:

• the 'empty' option for Select elements was not set for 'required' attributes (supposedly to prevent people from selecting the empty value even if it was not valid). This was removed: an empty value is now always generated
• the validation step for petition attributes was fixed. Previously, if no valid affiliation was selected, the CoPersonRole validation would fail, but no exception was thrown, so the whole process would continue anyway. This affects other validations as well.
• finally, even though Affiliation is a required element (even if set to 'optional'), no 'required' asterix was printed. This was fixed in the petition attributes view.

[boshrin]

I'm not sure I understand why EnrolleeCoPerson is given default values. I haven't tested it, but I expect this would break certain types of flows, such as Additional Role Enrollment, which rely on EnrolleeCoPerson being empty. I also generally don't like the idea of mucking with the requestData which should be just that -- the data from the request.

Also, affiliation (part of CoPersonRole) isn't an MVPA, so the use of the MVPA check in inList doesn't feel right.

[muisit]

The alternative to mucking with requestData would be to copy it and use the copied values in the whole routine. That would have caused more changes, but I can understand your argumentation. As a counter argument, fixing shortcoming or fixable mistakes in the requestData prevents using incorrect data later on. The alternative would introduce a separate copy-object, with the risk of later code using the wrong (original requestData) instead of the adjusted and validated requestDataCopy.

I see that the code sets a co_person_id value of '0' if the CoPerson is empty. If I recall correctly, this is done so the CoPersonRole validation code will not fail on the absence of a valid CoPerson (which may(!) need to be created still). In the current code, the whole validation of the CoPersonRole model is skipped, because it always fails. As the CoPersonRole is created based on requestData, I could put a line like this around line 1664:

$copyData=$requestData;
$copyData['EnrolleeCoPersonRole']['co_person_id'] = empty($coPersonId) ? 0 : $coPersonId;
$coRoleData = $this->validateModel('EnrolleeCoPersonRole', $copyData, $efAttrs);

and then keep the original after-the-validation attribute settings. My opinion was that the way I fixed it now required the least amount of changes and centralised the setting-of-additional-values, minimising the confusion in the control flow. But I have no strict preference, I can change it if you want.

I do not see how I 'use the MVPA check in inList'. The comment may point to that, but the check only indicates whether a 'required asterix' needs to be printed or not. I extended that check to include a situation outside of MVPA. Would you prefer an alternative where the same asterix is printed for non-MVPA situations, potentially leading to a case where 2 asterixes are printed in the future? Or use of an if-elseif construct, both printing the same output? Both seem rather tedious.