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

FormBuilder: don't make 'Primary' required #26272

Merged
merged 1 commit into from
May 26, 2023

Conversation

aydun
Copy link
Contributor

@aydun aydun commented May 19, 2023

Overview

In the FormBuilder blocks for Address, Email, IM and Phone, don't make 'Primary' required, otherwise repeated blocks fail.

Before

These FormBuilder blocks bring make 'Is Primary' radios to be required. When using the 'Multiple' option to add multiple Emails (or other), form validation fails saying Primary is required despite being selected on one entity.

After

'Primary' is not required.

Technical Details

Comments

@civibot
Copy link

civibot bot commented May 19, 2023

(Standard links)

@civibot civibot bot added the master label May 19, 2023
@colemanw
Copy link
Member

I think this could be addressed at the api-metadata level. Like around here:

public function modifySpec(RequestSpec $spec) {
$spec->getFieldByName('location_type_id')->setRequired(TRUE);
}

@aydun aydun force-pushed the fix_primary_in_blocks branch from a84176f to 06c6ff1 Compare May 19, 2023 21:07
@aydun
Copy link
Contributor Author

aydun commented May 19, 2023

I think this could be addressed at the api-metadata level.

Ok, changed to use that approach.

@colemanw
Copy link
Member

Did that have the same effect on Afforms @aydun ?

@aydun
Copy link
Contributor Author

aydun commented May 19, 2023

Yes, those blocks load with 'primary' not being a required field.

@colemanw colemanw merged commit ec8e300 into civicrm:master May 26, 2023
@colemanw
Copy link
Member

This is a good change thanks @aydun. I think these fields actually weren't originally required but were accidentally changed during the big boolean cleanup of '22.

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

Successfully merging this pull request may close these issues.

2 participants