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

Fix regression on case export from recent export fix #12517

Merged
merged 1 commit into from
Jul 20, 2018

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Jul 19, 2018

Overview

Fix enotices & fatal error on case export

Before

screenshot 2018-07-20 10 53 46

screenshot 2018-07-20 11 06 41

After

screenshot 2018-07-20 11 30 51

Technical Details

I experienced a lot of inconsistency in my testing of this & switching to a component specific class that extended the main one seemed much more reliable - and as a approach it offers the possibility of much cleaner code

Comments

I don't think this affects 5.4 rc

@mattwire @lcdservices @kcristiano ping

@civibot
Copy link

civibot bot commented Jul 19, 2018

(Standard links)

civicrm@3b4adc9 changed the way componentmode is defined

It now seems inconsistent
@lcdservices
Copy link
Contributor

tested and confirmed. this fixes the undefined property warnings.

@eileenmcnaughton
Copy link
Contributor Author

@lcdservices adding merge on pass on your review - did you check 5.4? I thought it wasn't there but also thought my head was spinning after looking at this for a while

@lcdservices
Copy link
Contributor

I tested against the master branch and could reproduce. After applying the PR the error was removed.

@eileenmcnaughton eileenmcnaughton merged commit 08013f5 into civicrm:master Jul 20, 2018
@eileenmcnaughton eileenmcnaughton deleted the case_export_fail branch July 20, 2018 04:41
@mattwire
Copy link
Contributor

👍

@eileenmcnaughton
Copy link
Contributor Author

@mattwire do you think 5.4 is affected?

@mattwire
Copy link
Contributor

@eileenmcnaughton Yes looks like 5.4 is affected :-( #12316 and #12320 will need to be backported before this PR can be applied to 5.4 though.

@eileenmcnaughton
Copy link
Contributor Author

@mattwire can you do a backport PR for 5.4?

@mattwire
Copy link
Contributor

Why are these commits not in the 5.4 branch but are in the 5.3 branch?

6e3090f
39fecc8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants