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

dev/core#2076 Fix for standalone event registrations confirmation email #18697

Closed
wants to merge 2 commits into from

Conversation

kewljuice
Copy link
Contributor

Overview

Fix for standalone event registrations confirmation email (event_offline_receipt) so that empty fields are not included in the mail.

https://lab.civicrm.org/dev/core/-/issues/2076

Fix for standalone event registrations confirmation email (event_offline_receipt) so that empty fields are not included in the mail.
@civibot
Copy link

civibot bot commented Oct 7, 2020

(Standard links)

@civibot civibot bot added the master label Oct 7, 2020
@jaapjansma
Copy link
Contributor

@kewljuice the automatic build is failing because of a code style warning. Could you fix that?

The automatic build was failing because of a code style warning.
@kewljuice
Copy link
Contributor Author

@jaapjansma code style warning fixed.

@jaapjansma
Copy link
Contributor

@kewljuice I have tested your patch on the test environment of this PR and I still get the custom field which is not used in the registration profile in the confirmation mail. So it looks like this patch does not solve the issue.

What I have done is the following:

  1. Created a custom group for participant with two custom fields
  2. Created an event with online registration and a confirmation mail. Added only one custom field to the registration profile
  3. Registered a person for an event through civicrms backend

Both fields are included in the confirmation mail. And I would only expect the field which I have selected in the profile.

Can you have another look?

@mattwire mattwire changed the title dev/core#2076 dev/core#2076 Fix for standalone event registrations confirmation email Oct 14, 2020
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Nov 29, 2020
After digging into civicrm#18697 I came to the
conclusion that currently both the online & offline event receipts have
sections for custom profiles and custom data (marked as customGroup).

The profiles are only assigned from the online flow and the data only from the offline
flow so in each receipt template one is unused.

I agree with the discussion on https://lab.civicrm.org/dev/core/-/issues/2076
that suggests that since every event has (potentially) assigned profiles
we could use those in both scenarios and never use the generic 'customGroup'.
The situation is less clear for contributions (where there is no profile
if there is no contribution page). Thus, I am inclined
to remove customGroup from the offline template too, after fixing to
include the profiles. However, there are additional
steps to get to the point where that is doable - at this stage I think
we CAN remove it from the online template because I cannot find anywhere
in the code where customGroup is assigned in that flow, nor logically
think of any reason other than copy and paste for it
@eileenmcnaughton
Copy link
Contributor

I'm going to close this out because I think it has stagnated because it's not clear that we should be making this change. In particular it's not clear that all sites would prefer to suppress empty fields. I think the right place to do this is in the message template - my preference is that in fact we suppress these fields by default for back office (since people often get in trouble with information they don't intend going out).

However, this change in the message template would remove the blank rows

 {/if}
 {foreach from=$val item=v key=f}
-{$f}: {$v}
+  {if $v}
+    {$f}: {$v}
+  {/if}
 {/foreach}
 {/if}
 {/foreach}

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.

3 participants