Skip to content

Conversation

@saifsultanc
Copy link
Contributor

@saifsultanc saifsultanc commented Jan 27, 2025

Context

⛑️ Ticket(s): https://secure.helpscout.net/conversation/2826241649/76973

Summary

Adding an :all_fields modifier that can be applied to Multi Select fields' merge tags (e.g. {Multi Select A:1:all_fields})

Demo of the update:

https://www.loom.com/share/8d065e7769fd445b92c6c5c341c73ebf

@saifsultanc saifsultanc added the enhancement New feature or request label Jan 27, 2025
Copy link
Contributor

@claygriffiths claygriffiths left a comment

Choose a reason for hiding this comment

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

Conditional approval, either explore https://github.com/gravitywiz/gp-populate-anything/blob/a924ffcc780cc4bb01c117a194c1fa674fda6193/class-gp-populate-anything.php#L2608-L2644 or confirm that the customer is using this modifier in a place where only admins can see the output.

If the customer is using the merge tag for admins only, go ahead and add a security disclaimer to this modifier based on my feedback.

$form = GFAPI::get_form( $entry['form_id'] );

// Safety check: Ensure the entry belongs to the same form as the field.
if ( $field->{'gppa-choices-primary-property'} != $entry['form_id'] ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

For added security, I'm curious if you could somehow recreate this logic or extract the logic in GPPA into a method that could be re-used: https://github.com/gravitywiz/gp-populate-anything/blob/a924ffcc780cc4bb01c117a194c1fa674fda6193/class-gp-populate-anything.php#L2608-L2644

Alternatively, if it's only used in the context of admins, you could add a security disclaimer to this modifier saying that it should not be used in places where non-admins can see the output of this merge tag modifier.

@claygriffiths claygriffiths changed the title gw-advanced-merge-tags.php: Added support for {all_fields} modifier to show data for all entries. gw-advanced-merge-tags.php: Added support for :all_fields modifier to show data for dynamically populated entries. Feb 4, 2025
return rgar( $value_array, $index );
}
break;
case 'all_fields':
Copy link
Contributor

Choose a reason for hiding this comment

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

I am open to different names for this like all_dynamic_entries or something? Curious what @spivurno thinks.

@github-actions
Copy link

github-actions bot commented Feb 4, 2025

Warnings
⚠️ When ready, don't forget to request reviews on this pull request from your fellow wizards.

Generated by 🚫 dangerJS against 9ea8e1e

@saifsultanc
Copy link
Contributor Author

saifsultanc commented Feb 6, 2025

Closing this as discussed here: https://gravitywiz.slack.com/archives/C04RQJ232PQ/p1738690427047309

@saifsultanc saifsultanc closed this Feb 6, 2025
@saifsultanc saifsultanc deleted the saif/add/76973-add-support-for-all-fields-list-of-entries branch February 6, 2025 06:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Development

Successfully merging this pull request may close these issues.

3 participants