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/event#15) Fix regression in handling of post profiles in email #15380

Merged
merged 4 commits into from
Oct 3, 2019

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Oct 3, 2019

Overview

Fixes a regression from 5.17.0 when sending a confirmation email. If event uses any "post" profile forms, then the confirmation email includes subsections to summarize each post-profile. The subsection titles were miscomputed. This manifested as either a page crash or an inaccurate title.

See also: https://lab.civicrm.org/dev/event/issues/15

Before

Screen Shot 2019-10-03 at 7 44 02 AM

After

Screen Shot 2019-10-03 at 9 33 29 AM

Technical Details

The previous code took an array ($postProfileID) and casted it to an int. This produced an arbitrary int (0 or 1). The revised code handles the array as an array.

@civibot
Copy link

civibot bot commented Oct 3, 2019

(Standard links)

@civibot civibot bot added the 5.18 label Oct 3, 2019
The message templates use this loop:

```
{foreach from=$customPost item=customPos key=j}
   <tr> <th {$headerStyle}>{$customPost_grouptitle.$j}</th></tr>
```

Note that the keys in `$customPost` and `$customPost_grouptitle` need to
match.
@totten
Copy link
Member

totten commented Oct 3, 2019

I confirmed (r-run) that the first two commits fix the crash. The resulting content was still wonky, though - because it failed to display the headings:

Screen Shot 2019-10-03 at 8 21 06 AM

The third commit gets the headers going again:

Screen Shot 2019-10-03 at 9 33 29 AM

@totten totten changed the title Fix regression in handling of post profiles in email (dev/event#15) Fix regression in handling of post profiles in email Oct 3, 2019
@totten
Copy link
Member

totten commented Oct 3, 2019

Filled-in description

The unit-test suite includes cases in which the value is actually NULL.
@totten
Copy link
Member

totten commented Oct 3, 2019

Updated to pass unit-test. Manually r-run again in cases with (a) no post profiles and (b) two post profiles. In both cases, the resulting email had reasonable headers.

@totten
Copy link
Member

totten commented Oct 3, 2019

MOP was previously given over MM from @seamuslee001 / @eileenmcnaughton .

The last failure appears unrelated, so I'd call it passing.

@totten totten merged commit 71d4887 into civicrm:5.18 Oct 3, 2019
@totten totten deleted the cust_uf branch October 3, 2019 14:09
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