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

Don't crash when contribution page is selected as a column #539

Merged

Conversation

demeritcowboy
Copy link
Contributor

  1. Pick Contribution Page as one of the columns.
  2. Type Error crash if there are any contributions that come from a contribution page.

A couple things:

  • This function alters the display value for a column in the report. Returning array makes no sense. Should be string, possibly null but '' makes more sense than null.
  • The commit that caused this I assume was some phpstorm-suggested mass-replace based on docblocks.
    • The docblock in core is wrong. It's another of those functions that returns an array or string depending on the param. I'll make a separate PR in core for that.
    • That function is deprecated, but title is not a valid field for buildOptions for contribution pages (returns FALSE), so it's not really deprecated and I can't replace this call here with a call to buildOptions.

@seamuslee001
Copy link
Collaborator

/test

@KarinG
Copy link
Contributor

KarinG commented Jan 23, 2023

Confirming that this PR fixes the exact issue described in this PR.

@eileenmcnaughton eileenmcnaughton merged commit 46b0a5b into eileenmcnaughton:master Jan 23, 2023
@eileenmcnaughton
Copy link
Owner

Thanks @demeritcowboy @seamuslee001 @KarinG - I'm happy to tag a release if you wish - civi compatibility of the release is set to 5.55 & that can probably stay the same

@demeritcowboy
Copy link
Contributor Author

Thanks!
I don't know about a release. It's an easy patch but I'll let others comment.

@demeritcowboy demeritcowboy deleted the contributionpage branch January 23, 2023 23:54
@KarinG
Copy link
Contributor

KarinG commented Jan 24, 2023

A release would be great 👍

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.

4 participants