Merge pull request #17863 from demeritcowboy/be-greener

report#43 - Icon after saving a civireport instance is misleading

CRM/Report/Form/Instance.php

@@ -365,7 +365,7 @@ class CRM_Report_Form_Instance {
     else {
       $statusMsg = ts('"%1" report has been successfully created. You are currently viewing the new report instance.', [1 => $instance->title]);
     }
-    CRM_Core_Session::setStatus($statusMsg);
+    CRM_Core_Session::setStatus($statusMsg, '', 'success');
 
     if ($redirect) {
       $urlParams = ['reset' => 1];

tests/phpunit/CRM/Report/Form/SampleForm.php

+<?php
+
+/**
+ * Helper class to simulate a report form but allow us access to some protected
+ * fields for tests.
+ * There's an argument that you shouldn't be testing against internal fields
+ * and that the getters in here should either be part of the real class or
+ * there should be some other public output to test against, but for the
+ * purposes of refactoring something like a big if/else block this is helpful
+ * to ensure the same before and after, and it's easier to remove this test
+ * class later if needed than stuff in the real class.
+ */
+class CRM_Report_Form_SampleForm extends CRM_Report_Form_Contribute_Summary {
+
+  public function getOutputMode():string {
+    return $this->_outputMode;
+  }
+
+  public function getAddPaging():bool {
+    return $this->addPaging;
+  }
+
+}

tests/phpunit/CRM/Report/FormTest.php

@@ -83,4 +83,110 @@ class CRM_Report_FormTest extends CiviUnitTestCase {
     }
   }
 
+  /**
+   * Test the processReportMode function.
+   *
+   * @dataProvider reportModeProvider
+   *
+   * @param array $input
+   * @param array $expected
+   */
+  public function testProcessReportMode($input, $expected) {
+    // This is a helper in the tests tree, not a real class in the main tree.
+    $form = new CRM_Report_Form_SampleForm();
+
+    $_REQUEST['output'] = $input['format'];
+    $_REQUEST['sendmail'] = $input['sendmail'];
+
+    $form->processReportMode();
+
+    unset($_REQUEST['output']);
+    unset($_REQUEST['sendmail']);
+
+    $this->assertEquals($expected, [
+      $form->getOutputMode(),
+      $form->getAddPaging(),
+      $form->printOnly,
+      $form->_absoluteUrl,
+    ]);
+  }
+
+  /**
+   * dataprovider for testProcessReportMode
+   *
+   * @return array
+   */
+  public function reportModeProvider() {
+    return [
+      'print no mail' => [
+        [
+          'format' => 'report_instance.print',
+          'sendmail' => NULL,
+        ],
+        [
+          // _outputMode
+          'print',
+          // addPaging
+          FALSE,
+          // printOnly
+          TRUE,
+          // _absoluteUrl
+          FALSE,
+        ],
+      ],
+      'print and mail' => [
+        [
+          'format' => 'report_instance.print',
+          'sendmail' => '1',
+        ],
+        ['print', FALSE, TRUE, TRUE],
+      ],
+      'csv no mail' => [
+        [
+          'format' => 'report_instance.csv',
+          'sendmail' => NULL,
+        ],
+        ['csv', FALSE, TRUE, TRUE],
+      ],
+      'csv and mail' => [
+        [
+          'format' => 'report_instance.csv',
+          'sendmail' => '1',
+        ],
+        ['csv', FALSE, TRUE, TRUE],
+      ],
+      'pdf no mail' => [
+        [
+          'format' => 'report_instance.pdf',
+          'sendmail' => NULL,
+        ],
+        ['pdf', FALSE, TRUE, TRUE],
+      ],
+      'pdf and mail' => [
+        [
+          'format' => 'report_instance.pdf',
+          'sendmail' => '1',
+        ],
+        ['pdf', FALSE, TRUE, TRUE],
+      ],
+      'unknown format no mail' => [
+        [
+          'format' => NULL,
+          'sendmail' => NULL,
+        ],
+        [NULL, TRUE, FALSE, FALSE],
+      ],
+      'unknown format and mail' => [
+        [
+          'format' => NULL,
+          'sendmail' => '1',
+        ],
+        // This is a bit inconsistent with the mail_report job which defaults
+        // to pdf when you don't specify a format. But for now this is what
+        // processReportMode does.
+        ['print', FALSE, TRUE, TRUE],
+      ],
+    ];
+  }
+
 }