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

Re-Fix Case form task (export/print not working) (replace quick fix with proper fix) #11936

Merged
merged 4 commits into from
May 7, 2018

Conversation

mattwire
Copy link
Contributor

@mattwire mattwire commented Apr 4, 2018

…itched to using CRM_Core_Form_Task

@eileenmcnaughton @seamuslee001 @colemanw I think this is the correct fix for #11928 and #11926

@eileenmcnaughton
Copy link
Contributor

@mattwire thanks - did you check how that relates to the outstanding issue (print case to pdf only prints first case) - wondering if it fixes it

@seamuslee001
Copy link
Contributor

seamuslee001 commented Apr 4, 2018

@mattwire thanks for this, it mostly works except for not when you include a case token in the letter.

this is code that for me makes the case tokens work. It is very much not elegant enough however we need to some how pass $this->caseIds through the reason being is that the PDF code assumes we may have a case id see https://github.com/civicrm/civicrm-core/blob/master/CRM/Contact/Form/Task/PDF.php#L61 https://github.com/civicrm/civicrm-core/blob/master/CRM/Contact/Form/Task/PDF.php#L122 and more importantly https://github.com/civicrm/civicrm-core/blob/master/CRM/Contact/Form/Task/PDFLetterCommon.php#L401

I suppose they could be swapped to entityIds i guess but we would still need code to know to handle the call to replaceCaseTokens etc

    */
   public $_contactIds;

  /**
   * The array that holds all the case ids for use in PDF letters to sort tokens
   *
   * @var array
   */
  public $_caseIds;

   // Must be set to entity table name (eg. civicrm_participant) by child class
   static $tableName = NULL;
   // Must be set to entity shortname (eg. event)
@@ -136,6 +143,10 @@ abstract class CRM_Core_Form_Task extends CRM_Core_Form {

     $form->_entityIds = $form->_componentIds = $ids;

    if ($form::$entityShortname == 'case') {
      $form->_caseIds = $form->_entityIds;
    }

     //set the context for redirection for any task actions
     $qfKey = CRM_Utils_Request::retrieve('qfKey', 'String', $form);
     $urlParams = 'force=1';

@mattwire
Copy link
Contributor Author

mattwire commented Apr 5, 2018

@seamuslee001 Thanks for testing. I've just pushed a slightly more generic fix based on your proposed fix - can you confirm if that works for you - it seems to work here!

@eileenmcnaughton It doesn't fix the "only prints one PDF" but this PR should do: #11946

@eileenmcnaughton
Copy link
Contributor

@mattwire @seamuslee001 what is the status here? I thought we merged fixes for these bugs? If this is a fix we need to clarify steps to replicate. If it is a follow up tidy up it might be better dropped for now as we have a tonne of similar on our plate & this code seem to have settled down....

@mattwire
Copy link
Contributor Author

mattwire commented May 1, 2018

@eileenmcnaughton @seamuslee001 It just needs testing and it should be good to go. It is a follow-up to the quick fix that was committed before (which temporarily undid a lot of the core task abstraction work that I've done), so I think it is important to get it reviewed and back into core. We are constantly fighting with messy old code and this helps clear out that old code, reduce duplication and allow for generic tests to be written in the future rather than entity specific ones. For example it will eventually help with things like this: #12048

@eileenmcnaughton
Copy link
Contributor

OK @seamuslee001 @colemanw I feel both of you got closer to the regressions on this - do either of you feel ok to review this?

@eileenmcnaughton eileenmcnaughton changed the title Fix Case form task regressions (export/print not working) since we sw… Re-Fix Case form task (export/print not working) (replace quick fix with proper fix) May 1, 2018
$this->_task = $values['task'];
$className = 'CRM_' . ucfirst($this::$entityShortname) . '_Task';
$form->_task = $values['task'];
$className = 'CRM_' . ucfirst($form::$entityShortname) . '_Task';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mattwire testing this on doing an export of Cases got an error message here

Error: Access to undeclared static property: CRM_Export_Form_Select::$entityShortname 

@seamuslee001
Copy link
Contributor

(CiviCRM Review Template WORD-1.1)

  • (r-explain) Pass
  • (r-test) Pass
  • (r-code) Pass
  • (r-doc) Pass
  • (r-maint) Pass
  • (r-run) Pass i tested on a demo civicrm instance exporting a PDF with case tokens and also a csv primary fields export. Given the code the export function touches on i also tested doing a contribute primary fields export and confirmed that worked as expected. I also tested seeing what fields were on offer for the select fields in the contribute export and confirmed it makes sense
  • (r-user) Pass
  • (r-tech) Pass

@seamuslee001
Copy link
Contributor

Merging

@seamuslee001 seamuslee001 merged commit 6fa7151 into civicrm:master May 7, 2018
@eileenmcnaughton
Copy link
Contributor

Thanks @seamuslee001 @mattwire

@agh1
Copy link
Contributor

agh1 commented Jul 2, 2018

Quick cross-reference: I believe this is for dev/core#8

kcristiano added a commit to tadpolecc/civicrm that referenced this pull request Jul 19, 2018
civicrm/civicrm-core#11936
Signed-off-by: Kevin Cristiano <kcristiano@tadpole.cc>
kcristiano added a commit to tadpolecc/civicrm that referenced this pull request Jul 19, 2018
civicrm/civicrm-core#11936
Signed-off-by: Kevin Cristiano <kcristiano@tadpole.cc>
kcristiano added a commit to tadpolecc/civicrm that referenced this pull request Jul 19, 2018
civicrm/civicrm-core#11936
Signed-off-by: Kevin Cristiano <kcristiano@tadpole.cc>
kcristiano added a commit to tadpolecc/civicrm that referenced this pull request Jul 19, 2018
civicrm/civicrm-core#11936
Signed-off-by: Kevin Cristiano <kcristiano@tadpole.cc>
@mattwire mattwire deleted the case_form_task_fixes branch September 25, 2018 11:03
andyburnsco pushed a commit to andyburnsco/civicrm that referenced this pull request Mar 16, 2021
civicrm/civicrm-core#11936
Signed-off-by: Kevin Cristiano <kcristiano@tadpole.cc>
andyburnsco pushed a commit to andyburnsco/civicrm that referenced this pull request Mar 16, 2021
civicrm/civicrm-core#11936
Signed-off-by: Kevin Cristiano <kcristiano@tadpole.cc>
andyburnsco pushed a commit to andyburnsco/civicrm that referenced this pull request Mar 16, 2021
civicrm/civicrm-core#11936
Signed-off-by: Kevin Cristiano <kcristiano@tadpole.cc>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants