Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Required flag is being ignored for added fields #170

Open
asberenyi opened this issue Apr 11, 2020 · 2 comments
Open

Required flag is being ignored for added fields #170

asberenyi opened this issue Apr 11, 2020 · 2 comments

Comments

@asberenyi
Copy link

asberenyi commented Apr 11, 2020

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?

@jinjie
Copy link

jinjie commented Mar 10, 2021

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

@asberenyi
Copy link
Author

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');
        }
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants