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.

Sign up for GitHub

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

dev/core#5566 Fix clear options not showing for non-admin user #31705

Closed
wants to merge 3 commits into from
Closed

dev/core#5566 Fix clear options not showing for non-admin user #31705

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
34084de
Rename in-template variable for clarity
eileenmcnaughton Jan 3, 2025
b38144c
Fix clear radio not showing for non-admin user
eileenmcnaughton Jan 3, 2025
a0c54b8
Do not re-render the html in the error for radio fields with option v…
eileenmcnaughton Jan 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CRM/Core/BAO/CustomField.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -775,6 +775,9 @@ public static function addQuickFormElement(
if ($search || empty($useRequired)) {
$fieldAttributes['allowClear'] = TRUE;
}
if ($field->options_per_line) {
$fieldAttributes['options_per_line'] = $field->options_per_line;
}
$qf->addRadio($elementName, $label, $options, $fieldAttributes, NULL, $useRequired);
}
break;
Expand Down
4 changes: 3 additions & 1 deletion CRM/Core/Form.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1498,7 +1498,9 @@ public function &addRadio($name, $title, $values, $attributes = [], $separator =
$options[] = $element;
}
$group = $this->addGroup($options, $name, $title, $separator);

if (!empty($attributes['options_per_line'])) {
$group->setAttribute('options_per_line', $attributes['options_per_line']);
}
$optionEditKey = 'data-option-edit-path';
if (!empty($attributes[$optionEditKey])) {
$group->setAttribute($optionEditKey, $attributes[$optionEditKey]);
Expand Down
19 changes: 17 additions & 2 deletions CRM/Core/Form/Renderer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,13 @@ public static function &singleton() {
public function _elementToArray(&$element, $required, $error) {
self::updateAttributes($element, $required, $error);

if ($element->getType() === 'group' && $element->getAttribute('options_per_line')) {
// Our standard template renders the erroneous html as part of it.
// This results in double renders in some cases - see https://lab.civicrm.org/dev/core/-/issues/5571
// I suspect not rendering the html is probably often better but
// this adds it for our known problem case.
$this->setErrorTemplate('CRM/Form/errorClean.tpl');
}
$el = parent::_elementToArray($element, $required, $error);
$el['textLabel'] = $element->_label ?? NULL;

Expand Down Expand Up @@ -126,7 +133,13 @@ public function _elementToArray(&$element, $required, $error) {
else {
$typesToShowEditLink = ['select', 'group'];
$hasEditPath = NULL !== $element->getAttribute('data-option-edit-path');

if ($element->getType() === 'group' && $element->getAttribute('options_per_line')) {
// Our standard template renders the erroneous html as part of it.
// This results in double renders in some cases - see https://lab.civicrm.org/dev/core/-/issues/5571
// I suspect not rendering the html is probably often better but
// this adds it for our known problem case.
$this->setErrorTemplate('CRM/Form/errorClean.tpl');
}
if (in_array($element->getType(), $typesToShowEditLink) && $hasEditPath) {
$this->addOptionsEditLink($el, $element);
}
Expand All @@ -135,7 +148,9 @@ public function _elementToArray(&$element, $required, $error) {
$this->appendUnselectButton($el, $element);
}
}

// We can't do a get to check but the only time we ever use a different template is within this
// function.
$this->setErrorTemplate('CRM/Form/error.tpl');
return $el;
}

Expand Down
14 changes: 14 additions & 0 deletions templates/CRM/Form/errorLabel.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{*
+--------------------------------------------------------------------+
| Copyright CiviCRM LLC. All rights reserved. |
| |
| This work is published under the GNU AGPLv3 license with some |
| permitted exceptions and without any warranty. For full license |
| and copyright information, see https://civicrm.org/licensing |
+--------------------------------------------------------------------+
*}

{if $error}
<span class="crm-error">{$error}</span>
{/if}

80 changes: 29 additions & 51 deletions templates/CRM/Profile/Form/Dynamic.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,10 @@
{continue}
{/if}
{assign var="profileID" value=$field.group_id}
{assign var=n value=$field.name}
{assign var="profileFieldName" value=$field.name}
{assign var="rowIdentifier" value=$field.name}
{assign var="formElement" value=$form.$profileFieldName}

{if $field.groupTitle != $fieldset}
{if $mode neq 8 && $mode neq 4}
<div {if $context neq 'dialog'}id="profilewrap{$field.group_id}"{/if}>
Expand All @@ -75,7 +78,7 @@
{/if}
{if $field.field_type eq "Formatting"}
{$field.help_pre}
{elseif $n}
{elseif $profileFieldName}
{if $field.groupTitle != $fieldset}
{if $fieldset != $zeroField}
{if $groupHelpPost}
Expand All @@ -89,93 +92,68 @@
{/if}
<div class="form-layout-compressed">
{/if}
{if $field.help_pre && $action neq 4 && $form.$n.html}
<div class="crm-section helprow-{$n}-section helprow-pre" id="helprow-{$n}">
{if $field.help_pre && $action neq 4 && $form.$profileFieldName.html}
<div class="crm-section helprow-{$profileFieldName}-section helprow-pre" id="helprow-{$profileFieldName}">
<div class="content description">{$field.help_pre}</div>
</div>
{/if}
{if array_key_exists('options_per_line', $field) && $field.options_per_line}
<div class="crm-section editrow_{$n}-section form-item" id="editrow-{$n}">
<div class="label">{$form.$n.label}</div>
<div class="content edit-value">
{assign var="count" value=1}
{strip}
<table class="form-layout-compressed">
<tr>
{* sort by fails for option per line. Added a variable to iterate through the element array*}
{foreach name=outer key=key item=item from=$form.$n}
{* There are both numeric and non-numeric keys mixed in here, where the non-numeric are metadata that aren't arrays with html members. *}
{if is_array($item) && array_key_exists('html', $item)}
<td class="labels font-light">{$form.$n.$key.html}</td>
{if $count == $field.options_per_line}
</tr>
<tr>
{assign var="count" value=1}
{else}
{assign var="count" value=$count+1}
{/if}
{/if}
{/foreach}
</tr>
</table>
{/strip}
</div>
<div class="clear"></div>
{include file="CRM/UF/Form/FieldRadioOptionsPerLine.tpl"}
</div>{* end of main edit section div*}
{else}
<div id="editrow-{$n}" class="crm-section editrow_{$n}-section form-item">
{else}
<div id="editrow-{$profileFieldName}" class="crm-section editrow_{$profileFieldName}-section form-item">
<div class="label">
{$form.$n.label}
{$form.$profileFieldName.label}
</div>
<div class="edit-value content">
{if $n|substr:0:3 eq 'im-'}
{assign var="provider" value="$n-provider_id"}
{if $profileFieldName|substr:0:3 eq 'im-'}
{assign var="provider" value="$profileFieldName-provider_id"}
{$form.$provider.html}&nbsp;
{/if}
{if $n eq 'email_greeting' or $n eq 'postal_greeting' or $n eq 'addressee'}
{if $profileFieldName eq 'email_greeting' or $profileFieldName eq 'postal_greeting' or $profileFieldName eq 'addressee'}
{include file="CRM/Profile/Form/GreetingType.tpl"}
{elseif ( $n eq 'group' && $form.group ) || ( $n eq 'tag' && $form.tag )}
{include file="CRM/Contact/Form/Edit/TagsAndGroups.tpl" type=$n context="profile" tableLayout=1}
{elseif ( $form.$n.name eq 'image_URL' )}
{$form.$n.html}
{elseif ( $profileFieldName eq 'group' && $form.group ) || ( $profileFieldName eq 'tag' && $form.tag )}
{include file="CRM/Contact/Form/Edit/TagsAndGroups.tpl" type=$profileFieldName context="profile" tableLayout=1}
{elseif ( $form.$profileFieldName.name eq 'image_URL' )}
{$form.$profileFieldName.html}
{if !empty($imageURL)}
<div class="crm-section contact_image-section">
<div class="content">
{include file="CRM/Contact/Page/ContactImage.tpl"}
</div>
</div>
{/if}
{elseif $n|substr:0:5 eq 'phone'}
{assign var="phone_ext_field" value=$n|replace:'phone':'phone_ext'}
{$form.$n.html}
{elseif $profileFieldName|substr:0:5 eq 'phone'}
{assign var="phone_ext_field" value=$profileFieldName|replace:'phone':'phone_ext'}
{$form.$profileFieldName.html}
{if $form.$phone_ext_field.html}
&nbsp;{$form.$phone_ext_field.html}
{/if}
{else}
{if $field.html_type neq 'File' || ($field.html_type eq 'File' && !$field.is_view)}
{$form.$n.html}
{$form.$profileFieldName.html}
{/if}
{if $field.html_type eq 'Autocomplete-Select'}
{if $field.data_type eq 'ContactReference'}
{include file="CRM/Custom/Form/ContactReference.tpl" element_name = $n}
{include file="CRM/Custom/Form/ContactReference.tpl" element_name = $profileFieldName}
{/if}
{/if}
{/if}
</div>
<div class="clear"></div>
</div>

{if $form.$n.type eq 'file'}
<div class="crm-section file_displayURL-section file_displayURL{$n}-section"><div class="content">{$customFiles.$n.displayURL}</div></div>
{if !$fields.$n.is_view}
<div class="crm-section file_deleteURL-section file_deleteURL{$n}-section"><div class="content">{$customFiles.$n.deleteURL}</div></div>
{if $form.$profileFieldName.type eq 'file'}
<div class="crm-section file_displayURL-section file_displayURL{$profileFieldName}-section"><div class="content">{$customFiles.$profileFieldName.displayURL}</div></div>
{if !$fields.$profileFieldName.is_view}
<div class="crm-section file_deleteURL-section file_deleteURL{$profileFieldName}-section"><div class="content">{$customFiles.$profileFieldName.deleteURL}</div></div>
{/if}
{/if}
{/if}

{* Show explanatory text for field if not in 'view' mode *}
{if $field.help_post && $action neq 4 && $form.$n.html}
<div class="crm-section helprow-{$n}-section helprow-post" id="helprow-{$n}">
{if $field.help_post && $action neq 4 && $form.$profileFieldName.html}
<div class="crm-section helprow-{$profileFieldName}-section helprow-post" id="helprow-{$profileFieldName}">
<div class="content description">{$field.help_post}</div>
</div>
{/if}
Expand Down
20 changes: 20 additions & 0 deletions templates/CRM/UF/Form/FieldRadioOptionsPerLine.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<div class="crm-section editrow_{$profileFieldName}-section form-item" id="editrow-{$rowIdentifier}">
<div class="label option-label">{$formElement.label}</div>
<div class="content">
<div class="crm-multiple-checkbox-radio-options crm-options-per-line" style="--crm-opts-per-line:{$field.options_per_line};">
{foreach name=outer key=key item=item from=$formElement}
{if is_array($item) && array_key_exists('html', $item)}
<div class="crm-option-label-pair" >{$formElement.$key.html}</div>
{/if}
{/foreach}
</div>
{* Include the edit options list for admins *}
{if $formElement.html|strstr:"crm-option-edit-link"}
{$formElement.html|regex_replace:"@^.*(<a href=.*? class=.crm-option-edit-link.*?</a>)$@":"$1"}
{else}
{$formElement.html}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like it might duplicate things? I don't see this in the fields.tpl nor in dynamic.tpl previously

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or is this just the wrench?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@seamuslee001 yeah so what was happening is that because the radio options were not being laid out as desired the tpl was iterating through & rendering the options html not the radio group html - and then using that to re-add some of the radio group html (but was dropping the clear when the edit was not in play)

Although this PR does mostly get us there I wound up concluding there is a better approach - ie update the renderer to add the classes - see #31706 (as of writing the PR name is not a good description of what it does)

{/if}
</div>
<div class="clear"></div>
</div>
<div class="clear"></div>
18 changes: 1 addition & 17 deletions templates/CRM/UF/Form/Fields.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,23 +24,7 @@
</div>
{/if}
{if array_key_exists('options_per_line', $field) && $field.options_per_line != 0}
<div class="crm-section editrow_{$profileFieldName}-section form-item" id="editrow-{$rowIdentifier}">
<div class="label option-label">{$formElement.label}</div>
<div class="content">
<div class="crm-multiple-checkbox-radio-options crm-options-per-line" style="--crm-opts-per-line:{$field.options_per_line};">
{foreach name=outer key=key item=item from=$formElement}
{if is_array($item) && array_key_exists('html', $item)}
<div class="crm-option-label-pair" >{$formElement.$key.html}</div>
{/if}
{/foreach}
</div>
{* Include the edit options list for admins *}
{if $formElement.html|strstr:"crm-option-edit-link"}
{$formElement.html|regex_replace:"@^.*(<a href=.*? class=.crm-option-edit-link.*?</a>)$@":"$1"}
{/if}
</div>
<div class="clear"></div>
</div>
{include file="CRM/UF/Form/FieldRadioOptionsPerLine.tpl"}
{else}
<div class="crm-section editrow_{$profileFieldName}-section form-item" id="editrow-{$rowIdentifier}">
<div class="label">
Expand Down