symbiote/silverstripe-memberprofiles

Required flag is being ignored for added fields

Opened this issue · 2 comments

All new fields set as required are not rendered as required fields. The only field rendered as required is Email. Example rendered code for a required field, missing the required="required" aria-required="true":

<div id="Form_RegisterForm_SecondaryPhoneNumber_Holder" class="field form-group text">
    <label class="left" for="Form_RegisterForm_SecondaryPhoneNumber">Secondary Phone Number (Optional)</label>
    <input type="text" name="SecondaryPhoneNumber" class="text form-control" id="Form_RegisterForm_SecondaryPhoneNumber">
</div>

Is there a way to activate this?

To be specific, this only happens if the field is set as Readonly.

As seen in the code:

if ($field->ProfileVisibility !== 'Readonly') {
$this->addRequiredField($field->MemberField);
}

May I ask why is the check required for Readonly? I believe removing the check will make the form work again. There must be a reason the logic was implemented.

Temporary fix: Unset "Readonly" to the field in Visibility

The problem:
There are two contexts to this form: profile visibility and registration visibility, but the required logic does not distinguish between them.

Each field can have a distinct setting for readonly for each context. If a member custom field is set to be required but is readonly on the profilevisibility then it will not be shown as required on the either the profile form or registerform. It is true that a readonly field must not be set to be required, otherwise even if it is not being edited (and can’t be edited as a readonly field) the browser will demand that it be edited.

This function fails to distinguish between the two contexts, where we may want it as required on the registerform, but not on the profileform.

The fix:
The MemberProfilePageController has the RegisterForm function that creates the form. It calls the MemberProfileValidator class to determine whether to make the field readonly, passing it the fields object, starting at line 193. My change, on line 196, passes the additional parameter for context, which is ‘Register’.

public function RegisterForm()
    {
        $form = new Form(
            $this,
            'RegisterForm',
            $this->getProfileFields('Registration'),
            new FieldList(
                new FormAction('register', _t('MemberProfiles.REGISTER', 'Register'))
            ),
            // Alex modification of:
            //  new MemberProfileValidator($this->Fields())
            // Add context parameter ('Register'), see MemberProfileValidator line 47
            new MemberProfileValidator($this->Fields(),'Register')
        );

The MemberProfileValidator deals with the field settings for required, and is modified below to accept the $context parameter. If the $context is ‘Register, then a required field is shown as required in the registerform even if it is readonly on the profileform, where it will now not be required.

public function __construct($fields, $context, $member = null)
    {
        parent::__construct();

        $this->fields = $fields;
        $this->member = $member;

        foreach ($this->fields as $field) {
            if ($field->Required) {
                // Alex modification; check context (see mod in MemberProfilePageController  line 196)
                if ($context !== 'Register') {
                    if ($field->ProfileVisibility !== 'Readonly') { //Alex
                    // if ($field->ProfileVisibility !== 'Readonly' || $field->RegistrationVisibility == 'Edit') {
                        $this->addRequiredField($field->MemberField);
                    }
                } else {
                    // if ($field->ProfileVisibility !== 'Readonly') { //Alex
                    if ($field->ProfileVisibility !== 'Readonly' || $field->RegistrationVisibility == 'Edit') {
                        $this->addRequiredField($field->MemberField);
                    }
                }
            }
            if ($field->Unique) {
                $this->unique[] = $field->MemberField;
            }
        }

        if ($member && $member->ID && $member->Password) {
            $this->removeRequiredField('Password');
        }
    }