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

AdminUI: updates to Custom Group & Field #26329

Merged
merged 1 commit into from
May 25, 2023
Merged

Conversation

aydun
Copy link
Contributor

@aydun aydun commented May 24, 2023

Overview

This enhances the Administer Custom Group and Field pages in the AdminUI extension.

Before

It is not always obvious which custom group contains a specific custom field resulting in multiple rounds of opening up the field listing for a group, realising it is the wrong one, going back and trying again.

After

Custom field names are listed on the custom group page along with a filter making it very quick to find and edit a field.

Also has a few tweaks/standardisations in line with other AdminUI pages.

Technical Details

The field listing is formatted by replacing commas with '<br>'. If a field label contains a comma it will have wrong line breaks, but that is fairly unlikely and not a big deal if it happens. Ideally, the fields would be available as an array to allow formatting, but until then, this is good enough. This does not affect filtering.

Comments

@civibot
Copy link

civibot bot commented May 24, 2023

(Standard links)

@civibot civibot bot added the master label May 24, 2023
@@ -127,7 +130,7 @@
'alignment' => 'text-right',
],
[
'text' => E::ts('Actions'),
'text' => E::ts(''),
Copy link
Member

Choose a reason for hiding this comment

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

Bit of a waste of the ts() function here...

],
],
'type' => 'buttons',
'alignment' => 'text-right',
],
[
'text' => '',
'text' => E::ts(''),
Copy link
Member

Choose a reason for hiding this comment

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

Is this a bug in the Export API?

@aydun
Copy link
Contributor Author

aydun commented May 24, 2023

Thanks - updated.

@colemanw
Copy link
Member

@aydun I just went down a wee rabbit hole with this one. Because the raw data (which has the results of GROUP_CONCAT(fields) already as an array) is available in the rewrite function... yet just out of reach.
It can be assigned for use in the Smarty syntax, as it was in this earlier draft of the PR, however our security policy prevents anything useful being done with it as php-based modifiers like |implode are disallowed.
The list of modifiers we allow in strict mode is much smaller than necessary (there's nothing insecure about strtolower or uc_words or implode). But I'm slightly hesitant to expand that list because it increases the size of our code contract (and if we ever switched from Smarty2 to some other template system, we might regret all the weird & wonderful search display rewrites we'd have to backward-support).
So that leaves me with... this PR as-is.

group page.
A few tweaks/standardisations in line with other AdminUI pages.
@aydun
Copy link
Contributor Author

aydun commented May 25, 2023

Thanks for investigating that rabbit hole @colemanw ! Given that, lets go with it as it is.

@colemanw colemanw merged commit b63f146 into civicrm:master May 25, 2023
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