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

dev/core#561 Convert Case date fields on search forms from jcalendar … #15614

Merged

Conversation

seamuslee001
Copy link
Contributor

@seamuslee001 seamuslee001 commented Oct 26, 2019

…to datepicker

Overview

This converts the case start and case end date search fields from using the jcalendar to using datepicker

Before

Jcalendar is used for these fields

After

datepicker is used

https://lab.civicrm.org/dev/core/issues/561

ping @eileenmcnaughton @monishdeb @colemanw

@civibot
Copy link

civibot bot commented Oct 26, 2019

(Standard links)

@eileenmcnaughton
Copy link
Contributor

@seamuslee001 OK - I'll test this but then we had better try to get https://github.com/civicrm/civicrm-core/pull/15370/files merged

@eileenmcnaughton
Copy link
Contributor

Actually @seamuslee001 I'll merge the other first since it has schema changes & then we can tweak this - is that OK? There is a minor bug with the other but we can fix that

@seamuslee001
Copy link
Contributor Author

sure

@eileenmcnaughton
Copy link
Contributor

@seamuslee001 done - note we need to fix up the qill on subject now - as part of this or after.

Can you rebase?

@eileenmcnaughton
Copy link
Contributor

@seamuslee001 also note this on the mailing one - https://github.com/civicrm/civicrm-core/pull/15369/files (although it has some changes in it that need teasing out IMHO)

@seamuslee001
Copy link
Contributor Author

seamuslee001 commented Oct 26, 2019

@eileenmcnaughton i am also getting an e-notice

Warning: array_key_exists() expects parameter 2 to be array, null given in CRM_Case_Form_Search->postProcess() (line 223 of /home/seamus/buildkit/build/47-test/sites/all/modules/civicrm/CRM/Case/Form/Search.php).

now on a base search

@eileenmcnaughton
Copy link
Contributor

@seamuslee001 yeah - we need to fix that but it made more sense to bring it in & fix than stale that one & then fix IMHO

@seamuslee001
Copy link
Contributor Author

@eileenmcnaughton the case_subject issue looks to be because https://github.com/civicrm/civicrm-core/blob/master/CRM/Core/Form/Search.php#L149 is converting the value to be an array ['LIKE' => passed in value]

@eileenmcnaughton
Copy link
Contributor

@seamuslee001 yeah - the issue is the handling in buildQill - It needs to be something like

        list($op, $value) = CRM_Contact_BAO_Query::buildQillForFieldValue('CRM_Contribute_DAO_Contribution', $name, $value, $op, $pseudoExtraParam);
        if (!($name == 'id' && $value == 0)) {
          $query->_qill[$grouping][] = ts('%1 %2 %3', [1 => $fields[$qillName]['title'], 2 => $op, 3 => $value]);
        }

but I want it to be more generic / not need special casing

@seamuslee001
Copy link
Contributor Author

@eileenmcnaughton i got it fixed in both parts now hopefully

@@ -1828,6 +1830,12 @@ public function whereClauseSingle(&$values, $isForcePrimaryOnly = FALSE) {
$this->buildRelativeDateQuery($values);
return;
}

// Handle situations where we have already gone through function convertTextStringsToUseLikeOperator.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@eileenmcnaughton this moves the handling that was added for the sortName field but tries to more generically handle it such that if we are in an array then set the $op to be LIKE and set the $value part of the key back to be the string rather than an arrray

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually this is still not working for me - let's pull it out & do it as a separate PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok have pulled this change out

/**
* @return null
*/
public function getFormValues() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@eileenmcnaughton this was what was causing the e-notice i was seeing

@eileenmcnaughton
Copy link
Contributor

@seamuslee001 I'm OK to merge this but think more follow up is needed. It fixes the date fields but I think

  1. we need to remove some more code &
  2. I'm still seeing the notice that was unfixed in @monishdeb's patch

Screen Shot 2019-10-26 at 3 56 17 PM

However, this did correctly convert a smart group I created with a relative date.

@@ -3498,7 +3506,7 @@ public function sortName(&$values) {
return;
}

$input = $value = is_array($value) ? trim($value['LIKE']) : trim($value);
$input = $value = trim($value);
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

Copy link
Contributor Author

Choose a reason for hiding this comment

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

pulled the change out

@eileenmcnaughton
Copy link
Contributor

@seamuslee001 here is the cleanup I think we can do after

eileenmcnaughton@b27fe37

…to datepicker

Swap to using buildDateRangeQuery function

Fix notice error and generically handle cases where we have specified to use the LIKE operator for strings
CRM/Case/BAO/Query.php Show resolved Hide resolved
@eileenmcnaughton
Copy link
Contributor

OK I'm good to merge with those changes bu we should do the follow up cleanup

@eileenmcnaughton eileenmcnaughton merged commit d16bd3c into civicrm:master Oct 26, 2019
@eileenmcnaughton eileenmcnaughton deleted the convert_case_date_jcalendar branch October 26, 2019 10:04
@eileenmcnaughton
Copy link
Contributor

@demeritcowboy FYI

@demeritcowboy
Copy link
Contributor

@demeritcowboy FYI

Looks good except was the force=1 expected to work with dates? For example if I do /civicrm/case/search?reset=1&force=1&case_start_date_relative=0&case_start_date_low=20190701&case_start_date_high=20191101, it fills in the search fields properly but doesn't filter the results. Whereas that same search works properly if I then click the search button.

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