Replace log_date with created_date & modified_date in advanced search

[eileenmcnaughton]

Overview

This is almost the last step in getting rid of an awful lot of technical debt & hard to maintain code around jcalendar & datepicker.

It does change the UI a bit - the code to create the current UI is pretty tough going and from seeing users try to work with it I don't think all that code is making it more usable. OTOH we can remove a maintainability blocker by switching over

Before

After

Technical Details

@seamuslee001 I'm keen for you to take a look & see what you think

1. I've still got a problem to solve with display on the 'choose range' - I recall we had this before but I can't spot why

2. my read on what the code does without this change is it filters by contact.created_date or contact.modified_date AND it joins onto the modified contact (if entered) but the implication that it means 'this contact created it in this date range' is wrong and the current UI is misleading

Comments

Conversion not included as yet

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[seamuslee001]

@eileenmcnaughton i agree that at least as it pertains to the filtering on who did it the form is not clear at all, however looking at https://github.com/civicrm/civicrm-core/pull/15693/files#diff-e54381bfdf51e31cab376c71ca0d66ffR3978 i think we want to in our upgrade process default to switching to filtering on the created date field as that is the default practice currently

[seamuslee001]

@eileenmcnaughton i have just pushed a 2nd commit to reflect that in my PR #15702

[eileenmcnaughton]

@seamuslee001 cool - that makes sense re the upgrade script. What the form actually says is

'Modified in this date range AND modified by person x '
OR

'Created in this date range AND modified by person x'

I don't think person x is linked into the date range in any way by the query

That is both before and after this change - but I think that the UI implies different - more so before this change but also after

[eileenmcnaughton]

@seamuslee001 what about moving modifed+by to be between the other 2 fields - or to the right - that might be an easy way to be less misleading

[seamuslee001]

@eileenmcnaughton do you think we need to fix the issue where if you set one of the 2 new fields to choose date range both sets of choose date range fields show?

[eileenmcnaughton]

@seamuslee001 oh dang - I thought that was fixed now :-( I don't know what's causing it

[eileenmcnaughton]

is it broken markup?

[seamuslee001]

it feels like javascripty so maybe yeh that might be the case

[seamuslee001]

@eileenmcnaughton the issue is that this line in templates/CRM/Core/DatePickerRange.tpl
var n = cj(this).parent().parent(); resolves to the outer <tr> which is ok in most cases because we haven't had more than 1 datepickerrange field on the same row. However in this case they are on the same row. i think we might be able to get away with removing 1 of the parent()s but may need some testing

[eileenmcnaughton]

@seamuslee001 I also don't think they have to be on the same row TBH

[seamuslee001]

@eileenmcnaughton if we move them out of the same row then it will be fine tbh

[eileenmcnaughton]

@seamuslee001 wanna do that then?

[eileenmcnaughton]

@seamuslee001 I merged your change - are we there on this one now?

[seamuslee001]

@eileenmcnaughton yes i believe so from my POV

[eileenmcnaughton]

@seamuslee001 ok - let's MOP this & also your convert script - if not done

[eileenmcnaughton]

@seamuslee001 perhaps we should follow up by swapping the order of modified date & created date - possibly even putting 'modified date' on the same line as 'modified by'

[seamuslee001]

@eileenmcnaughton perhaps but will that invoke more confusion on the search form?

[eileenmcnaughton]

@seamuslee001 so the modified by is tied a bit to modified date but not at all to created date - so grouping it closer to modified date makes sense IMHO