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

Freeze custom data elements when viewing an entity #12867

Merged

Conversation

mattwire
Copy link
Contributor

Overview

When viewing an entity which has custom data (and loads it via AJAX) it should not be editable.

Before

Custom data is editable (but not saved) when viewing:
localhost_8000_civicrm_admin_reltype_action view id 1 reset 1 1

After

Custom data is not editable when viewing:
localhost_8000_civicrm_admin_reltype_action view id 1 reset 1

Technical Details

Custom data is loaded via AJAX, but the AJAX form does not know what action is being performed. We add the action via the URL on the template and then use that action to freeze the elements if appropriate.

Comments

In this case we are seeing the issue when using the "RelationshipType" form and have the nz.co.fuzion.relatedpermissions extension enabled (which adds custom data to the "RelationshipType" entity.
@eileenmcnaughton This relates to entityform code that we've recently added. It's not a regression I don't think as it's around new "functionality" to view/edit custom data for any entity but it isn't currently working as it should, and I think this fix sorts that.

@@ -1639,6 +1639,9 @@ public static function buildQuickForm(&$form, &$groupTree, $inactiveNeeded = FAL
$fieldId = $field['id'];
$elementName = $field['element_name'];
CRM_Core_BAO_CustomField::addQuickFormElement($form, $elementName, $fieldId, $required);
if ($form->getAction() == CRM_Core_Action::VIEW) {
$form->getElement($elementName)->freeze();
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here we freeze the elements

@@ -48,6 +48,7 @@ public function preProcess() {
$this->_entityId = CRM_Utils_Request::retrieve('entityID', 'Positive');
$this->_groupID = CRM_Utils_Request::retrieve('groupID', 'Positive');
$this->_onlySubtype = CRM_Utils_Request::retrieve('onlySubtype', 'Boolean');
$this->_action = CRM_Utils_Request::retrieve('action', 'String');
$this->assign('cdType', FALSE);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Retrieve the action from the AJAX URL

Copy link
Member

Choose a reason for hiding this comment

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

I think type should be "Alphanumeric"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, updated

if ($name == 'action') {
if (!is_numeric($value) && is_string($value)) {
$value = CRM_Core_Action::resolve($value);
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was not allowing an action to be passed in via a numeric ID, only by name (eg. view/edit etc). As the action is assigned to the template numerically and we are creating a URL via the template it would be a real pain to map back to string only to map back to ID again.

@@ -61,6 +61,9 @@
{if $qfKey}
dataUrl += '&qf=' + '{$qfKey}';
{/if}
{if $action}
dataUrl += '&action=' + '{$action}';
{/if}
{literal}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add the action to the AJAX URL if it exists in the template.

@mattwire mattwire force-pushed the entityformajaxcustomdatafreeze branch from eca7c8d to 25cdc89 Compare September 25, 2018 15:37
@alifrumin
Copy link
Contributor

alifrumin commented Sep 27, 2018

@mattwire I'm at the CiviGovernance Sprint in New Jersey and will test this

It looks like the failing tests are unrelated.

I installed https://github.com/eileenmcnaughton/nz.co.fuzion.relatedpermissions on a test site with and without this pr and was able to replicate the behavior described by @mattwire above.

This is ready to be merged from my perspective.

@eileenmcnaughton eileenmcnaughton merged commit 8a687d6 into civicrm:master Sep 27, 2018
@eileenmcnaughton
Copy link
Contributor

merging per @alifrumin review

@mattwire mattwire deleted the entityformajaxcustomdatafreeze branch March 1, 2019 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants