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#1186 - ignore exception if no description field #15055

Merged
merged 1 commit into from
Aug 17, 2019

Conversation

demeritcowboy
Copy link
Contributor

Overview

https://lab.civicrm.org/dev/core/issues/1186

Before

Datatables error because of uncaught exception

After

Golden silence.

Technical Details

Comments

@civibot
Copy link

civibot bot commented Aug 17, 2019

(Standard links)

@seamuslee001
Copy link
Contributor

@demeritcowboy I think it would be helpful if we were able to generate a unit test here

@demeritcowboy
Copy link
Contributor Author

@seamuslee001 I agree but I need to push back a little. Just no promises.

@eileenmcnaughton
Copy link
Contributor

@seamuslee001 I agree but I also think we should be more lenient when people are fixing regressions - so patch desirable but not required here IMHO

@eileenmcnaughton
Copy link
Contributor

@mattwire are you able to test?

seamuslee001 added a commit to seamuslee001/civicrm-core that referenced this pull request Aug 17, 2019
@seamuslee001
Copy link
Contributor

added a unit test here #15058

@eileenmcnaughton
Copy link
Contributor

this looks safe to me & has a test now - merging

@eileenmcnaughton eileenmcnaughton merged commit e251108 into civicrm:5.17 Aug 17, 2019
@demeritcowboy
Copy link
Contributor Author

Thanks!

@demeritcowboy demeritcowboy deleted the file-no-description branch August 17, 2019 04:52
@mattwire
Copy link
Contributor

Thanks @demeritcowboy @eileenmcnaughton

@MegaphoneJon
Copy link
Contributor

With PR #14658 applied (currently unreleased) this bug popped up on me today when generating PDF letters. So I applied this patch and can also confirm it fixes the issue.

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.

5 participants