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

Manage Event: avoid E_NOTICE in smarty #19137

Merged
merged 1 commit into from
Dec 7, 2020

Conversation

mlutfy
Copy link
Member

@mlutfy mlutfy commented Dec 7, 2020

Overview

To reproduce:

  • Display e-notices
  • Create a new Event
  • Go to Manage Event > Online Registration

When using a dev environment that displays notices, or throws exceptions, it will fail to load the page.

Technical Details

The associated Smarty tpl has "if" statements in other places to check against empty values, but it throws a notice on this:

    var profileBottomCount = Number({/literal}{$profilePostMultiple|@count}{literal});
    var profileBottomCountAdd = Number({/literal}{$profilePostMultipleAdd|@count}{literal});

whereas in other places, the code does this:

      {if $profilePostMultipleAdd}
        {foreach from=$profilePostMultipleAdd item=profilePostIdA key=profilePostNumA name=profilePostIdAName}

I figured it was preferable to assign upstream, rather than add "ifs" in smarty.

@civibot
Copy link

civibot bot commented Dec 7, 2020

(Standard links)

@civibot civibot bot added the master label Dec 7, 2020
@demeritcowboy
Copy link
Contributor

r-run: Can reproduce and fix works.

This makes me realize that in the loudnotices extension it should at least let through logging of smarty issues. I ended up just skipping them because there's so many and almost every page throws an exception - on this page you don't even get this far because it would error out at pageTitle.

@seamuslee001 seamuslee001 merged commit 643b43e into civicrm:master Dec 7, 2020
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