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 url support for receive_date_high & receive_date_low #14594

Merged
merged 1 commit into from Jun 23, 2019

Conversation

@eileenmcnaughton
Copy link
Contributor

commented Jun 21, 2019

Overview

Url support for some fields on contribution search - this is part of a general standardisation.

receive_date_high=20180101
receive_date_low=20180909120000
contribution_cancel_date_high
contribution_cancel_date_low
invoice_number=9

Documentation issue civicrm/civicrm-dev-docs#624

Before

No support

After

Standardised Fields prefill - see invoice_number & receive date (cancel_date is off screen)
Screen Shot 2019-06-21 at 10 23 12 AM

Technical Details

Ensuring we get these is a standardised way also avoids some ad-hoc insecure adds we have had in the past.

Note that in doing this I fixed the new & somewhat experimental default function to use field uniquenames rather than field names (as this is how they are added to the form & it was required for cancel_date.

Support for receive_date_relative=this.day etc would be trivial but not yet tackled

Also I see that we historically support 'start' and 'end' I think something like this makes sense here (as a follow up)

+++ b/CRM/Contribute/Form/Search.php
@@ -152,6 +152,16 @@ class CRM_Contribute_Form_Search extends CRM_Core_Form_Search {
    * @throws \Exception
    */
   public function setDefaultValues() {
+    $lowReceiveDate = CRM_Utils_Request::retrieve('start', 'Timestamp');
+    if (!empty($lowReceiveDate)) {
+      $this->_formValues['receive_date_low'] = date('Y-m-d H:i:s', strtotime($lowReceiveDate));
+      CRM_Core_Error::deprecatedFunctionWarning('pass receive_date_low not start');
+    }
+    $highReceiveDate = CRM_Utils_Request::retrieve('end', 'Timestamp');
+    if (!empty($highReceiveDate)) {
+      $this->_formValues['receive_date_high'] = date('Y-m-d H:i:s', strtotime($highReceiveDate));
+      CRM_Core_Error::deprecatedFunctionWarning('pass receive_date_high not end');
+    }
     $this->_defaults = parent::setDefaultValues();
     if (empty($this->_defaults['contribution_status'])) {
       $this->_defaults['contribution_status'][1] = 1;
@@ -425,25 +435,6 @@ class CRM_Contribute_Form_Search extends CRM_Core_Form_Search {
       }
     }
 
-    $lowDate = CRM_Utils_Request::retrieve('start', 'Timestamp');
-    if ($lowDate) {
-      $lowDate = CRM_Utils_Type::escape($lowDate, 'Timestamp');
-      $date = CRM_Utils_Date::setDateDefaults($lowDate);
-      $this->_formValues['contribution_date_low'] = $this->_defaults['contribution_date_low'] = $date[0];
-    }
-
-    $highDate = CRM_Utils_Request::retrieve('end', 'Timestamp');
-    if ($highDate) {
-      $highDate = CRM_Utils_Type::escape($highDate, 'Timestamp');
-      $date = CRM_Utils_Date::setDateDefaults($highDate);
-      $this->_formValues['contribution_date_high'] = $this->_defaults['contribution_date_high'] = $date[0];
-    }
-
-    if ($highDate || $lowDate) {
-      //set the Choose Date Range value
-      $this->_formValues['contribution_date_relative'] = 0;
-    }
-
     $this->_limit = CRM_Utils_Request::retrieve('limit', 'Positive',
       $this
     );

Comments

@seamuslee001 @monishdeb @yashodha

Fix url support for receive_date_high & receive_date_low
This ensures that the following url params are supported
receive_date_high=20180101
receive_date_low....
contribution_cancel_date_high
contribution_cancel_date_low
invoice_number=

Note that in doing this I fixed the new & somewhat experimental default function to use field uniquenames
rather than field names (as this is how they are added to the form & it was required for cancel_date.

Support for receive_date_relative=this.day etc would be trivial but not yet tackled

Also I see that we historically support 'start' and 'end' I think we can tackle this by doing something like
if start is set then ->set('contribution_date_low' - probably with a deprecation notice)
I will leave for a follow up though
@civibot

This comment has been minimized.

Copy link

commented Jun 21, 2019

(Standard links)

@civibot civibot bot added the master label Jun 21, 2019

@MegaphoneJon

This comment has been minimized.

Copy link
Contributor

commented Jun 21, 2019

To add to the confusion, CiviReport params in the URL use _min and _max rather than _low/_high or _start/_end.

@eileenmcnaughton

This comment has been minimized.

Copy link
Contributor Author

commented Jun 21, 2019

@MegaphoneJon right - the whole 'break my soul' risk is high here :-)

@seamuslee001

This comment has been minimized.

Copy link
Contributor

commented Jun 22, 2019

@eileenmcnaughton

This comment has been minimized.

Copy link
Contributor Author

commented Jun 22, 2019

@seamuslee001 I think the patch I pasted in 'technical details' is probably the way to remove it - it retains handling for those odd params (in a deprecated way)

@seamuslee001

This comment has been minimized.

Copy link
Contributor

commented Jun 22, 2019

@eileenmcnaughton just looking over your comment i'm wondering if we should only get past the ifs if there is no _high/_low version in the URL?

@eileenmcnaughton

This comment has been minimized.

Copy link
Contributor Author

commented Jun 22, 2019

@seamuslee001 so talking about the patch in the comments rather than the content of this PR? I feel like we can encourage people to stop passing those with deprecation notices etc

@seamuslee001

This comment has been minimized.

Copy link
Contributor

commented Jun 23, 2019

I think this is fine and works well i'm going to merge this

@seamuslee001 seamuslee001 merged commit 03f0863 into civicrm:master Jun 23, 2019

1 check passed

default Build finished.
Details

@seamuslee001 seamuslee001 deleted the eileenmcnaughton:url_support branch Jun 23, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.