-
-
Notifications
You must be signed in to change notification settings - Fork 824
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
Re Add CRM_Case_Form_Task::PreProcessCommon() #11928
Re Add CRM_Case_Form_Task::PreProcessCommon() #11928
Conversation
@seamuslee001 So I know this was related to the case export failing? Do we need the |
@mattwire certainly the preProcessCommon because it is called here https://github.com/civicrm/civicrm-core/blob/master/CRM/Export/Form/Select.php#L123 the caseIds i put back in in case there was some other legacy code hanging around in the case tasks that needed it. |
@mattwire the reason why i am thinking there maybe legacy handling is because Coleman had to make this change https://github.com/civicrm/civicrm-core/pull/11926/files#diff-d62c51b919ba7bb286f97c0b2eb837c3L89 as well |
@seamuslee001 So the original plan with #11411 was just to duplicate what was done elsewhere but then it was pointed out that there's a lot of duplicate code so I created the Core/Form/Task.php (similar to what I did for CRM/Core/Task) except that I only made the changes for Case/Form/Task.php. Looking at the export code I think we should either drop preProcessCommon or move it to Core/Form/Task.php as it's obviously meant to be called by all task classes. |
@mattwire there might be a quick fix & a follow up clean up here ... @seamuslee001 this would stop the fatal - do you think we should merge this & keep testing? |
(needs to be against 5.0) |
@eileenmcnaughton i would say so |
de1620f
to
a097dd4
Compare
@seamuslee001 nope :-( Error: Using $this when not in object context in CRM_Core_Form_Task::preProcess() (line 82 of /Users/emcnaughton/buildkit/build/dmaster/sites/all/modules/civicrm/CRM/Core/Form/Task.php). |
Simplify down and use parent PreProcess function
a097dd4
to
1bedada
Compare
Using this I AM able to get an export (yay). Im going for a quick merge as it may affect the possibly related issue & I want all to be working from the same base. @mattwire am hoping you will follow up with a measured consideration of this function against master |
ping @eileenmcnaughton @colemanw