dev/core#561 Convert grant search fields to use datepicker #13211
Conversation
|
(Standard links)
|
eileenmcnaughton
changed the title
eileenmcnaughton
force-pushed
the
search_grant
branch
from
ec8ec53 to
2e4e06b
Compare
|
@seamuslee001 ping |
|
Ok will look shortly @eileenmcnaughton |
eileenmcnaughton
force-pushed
the
search_grant
branch
from
2e4e06b to
0dfa9a5
Compare
|
@eileenmcnaughton can you rebase this now pls |
|
@eileenmcnaughton i note that in the change we have lost the date not set checkbox, is that a specific thing your changing here? |
|
@seamuslee001 that checkbox seems to achieve the same thing as the little x you get with datepicker |
eileenmcnaughton
force-pushed
the
search_grant
branch
from
0dfa9a5 to
35c6bbc
Compare
|
@eileenmcnaughton i think there is actually a functional difference because a clear date picker box means include dates and rows with no dates whereas looking at the implementation of a checkbox it looks like its about actually querying that the column is NULL https://github.com/civicrm/civicrm-core/pull/13211/files#diff-f5e872367f13e067d643ef634b73ba14R136 |
eileenmcnaughton
force-pushed
the
search_grant
branch
from
35c6bbc to
d1f0f10
Compare
|
@seamuslee001 OK - I've returned the ugliness |
|
@eileenmcnaughton do we need to worry about smart groups built off this form? |
|
@seamuslee001 argh - is that possible? |
|
@eileenmcnaughton not sure just assumed that it was the case with searches maybe not maybe only limited to contact based searches |
|
(CiviCRM Review Template WORD-1.2) |
|
@eileenmcnaughton It looks like we have broken the tab on Advanced Search tho for grants |
eileenmcnaughton
force-pushed
the
search_grant
branch
3 times, most recently
from
ac4397a to
fb178bc
Compare
|
@seamuslee001 I looked a bit further & the date range stuff uses jcalendar :-( - worse it's used in lots of places. My currently thinking is that it's best to mature an approach for switching across (& for handling smart group data) before replacing instances elsewhere - as a bit of an 'unloved corner' grant feels like a good place to develop this approach - I've tinkered a little here but it is not complete from a formatting pov, a post process pov or a conversion pov. |
eileenmcnaughton
force-pushed
the
search_grant
branch
3 times, most recently
from
b972b26 to
1765248
Compare
|
Tested on dmaster and functionally looks fine on Find Grants and on Advanced Search, including creating a Smart Group. @monishdeb could you put in some love on this PR to advance the formatting, post process and conversion areas that @eileenmcnaughton mentions? I hope looking at the code the meaning of her points becomes clearer. |
eileenmcnaughton
force-pushed
the
search_grant
branch
2 times, most recently
from
e1be97f to
e897889
Compare
|
@seamuslee001 I've fixed this up & I'm happy it's working as far as it goes. I've decided to remove the patch that does the 'in-form' conversion in favour of an upgrade conversion & am working on that as a separate patch |
eileenmcnaughton
force-pushed
the
search_grant
branch
from
e897889 to
e212360
Compare
|
I tested this along with #13395 and confirmed that an existing smart group creating from the date fields on grant forms are changed correctly and the new form works well. I think we should merge this and the Routine one and then look through some of the others @colemanw @monishdeb |
|
thanks @seamuslee001 I think it woud be good if once this is merged someone could attempt the campaign search form & try to apply similar methodology where possible |
|
Thanks for the testing @seamuslee001. I left a note for @eileenmcnaughton on the other PR and will merge both once that's resolved. |
Overview
Converts grant date search date files to use jcalendar
Before
After
Technical Details
#13210 is a pre-requisite
Takes some of the approach form #13013 (leaving out the default handling part & with some tweaks)
Comments
The grant fields are actually date fields not datetime - which would be more appropriate IMHO - so the change from date+time to date is appropriate here.