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

[Date picker][REF] Convert jcalendar date fields to date picker on me… #15177

Merged

Conversation

seamuslee001
Copy link
Contributor

@seamuslee001 seamuslee001 commented Aug 31, 2019

…mber search screen

Overview

Switches the member date fields from using jcalendar to datepicker and this tidies up the search template a bit as well

Before

Jcalender fields used

Screen Shot 2019-09-03 at 9 25 50 AM

After

Datepicker used

Screen Shot 2019-09-03 at 9 25 09 AM

ping @eileenmcnaughton

@civibot
Copy link

civibot bot commented Aug 31, 2019

(Standard links)

@civibot civibot bot added the master label Aug 31, 2019
@seamuslee001 seamuslee001 force-pushed the member_fields_datepicker_conversion branch 3 times, most recently from 394f315 to 0d75625 Compare August 31, 2019 22:35
@colemanw
Copy link
Member

Looks like test fails might be related.

@seamuslee001 seamuslee001 force-pushed the member_fields_datepicker_conversion branch from 0d75625 to fddf3d6 Compare August 31, 2019 23:57
@eileenmcnaughton
Copy link
Contributor

Also with this we need to ensure _relative gets to

$this->buildRelativeDateQuery($values);

& is processed there - NOT converted into high/low values which will not be loaded correctly from the db when loading saved groups

Old way

  • convert relative dates to high & low & save a separate 'relative_dates' array in formValues in saved search

New way

  • retain x_date_relative = 'this.month' when saving. 'relative_dates' array may still be saved (for now) but is irrelevant.

@seamuslee001 seamuslee001 force-pushed the member_fields_datepicker_conversion branch from fddf3d6 to 19fb921 Compare September 1, 2019 01:25
@seamuslee001
Copy link
Contributor Author

ok @eileenmcnaughton should hopefully have fixed that

@seamuslee001 seamuslee001 force-pushed the member_fields_datepicker_conversion branch from 19fb921 to 987238f Compare September 1, 2019 01:27
@eileenmcnaughton
Copy link
Contributor

@seamuslee001 the fail relates to the xml change - since this will be stale once we merge the pledge one it might be worth resolving the xml change for this as a separate PR & then rebase this one pledge & that are merged

@seamuslee001 seamuslee001 force-pushed the member_fields_datepicker_conversion branch 2 times, most recently from b414fb3 to 8a85f7d Compare September 2, 2019 08:04
@@ -165,29 +165,36 @@ public static function whereClauseSingle(&$values, &$query) {
switch ($name) {
case 'member_join_date_low':
case 'member_join_date_high':
case 'membership_join_date_low':
Copy link
Contributor

Choose a reason for hiding this comment

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

@seamuslee001 I'm not keen on this - it leaves the deprecated code 'looking useful' - I'd rather separate out the 2 & have deprecation notices in the obsolete code.

We could handle the 'good fields' like this #15191

@seamuslee001 seamuslee001 force-pushed the member_fields_datepicker_conversion branch from 8a85f7d to 1b36b65 Compare September 2, 2019 21:08
@seamuslee001
Copy link
Contributor Author

@eileenmcnaughton have updated now can you re-check pls

@eileenmcnaughton
Copy link
Contributor

OK I think the way the tpl changes close up the whitespace makes sense - will add a sig-markup label for themers

@eileenmcnaughton
Copy link
Contributor

My searches are working with this but when I went back to check on my previously created & upgraded group I'm seeing warnings - so the upgrade didn't quite work

Screen Shot 2019-09-03 at 9 31 51 AM

@seamuslee001
Copy link
Contributor Author

@eileenmcnaughton and the upgrade definitely happened?

@eileenmcnaughton
Copy link
Contributor

@seamuslee001 yes - but look here

public function renameField($oldName, $newName) {
- we aren't handling _high, _low, _relative in there like I assumed we would be....

@seamuslee001 seamuslee001 force-pushed the member_fields_datepicker_conversion branch from 1b36b65 to 9298f6f Compare September 2, 2019 21:37
@seamuslee001
Copy link
Contributor Author

@eileenmcnaughton i have just pushed an update to fix the upgrade

@eileenmcnaughton
Copy link
Contributor

@seamuslee001 OK - I was expecting us to make the function handle them but that is fine

@seamuslee001
Copy link
Contributor Author

@eileenmcnaughton we have already used this function before in previous upgrades so i'm more nervous about touching it rather than touching the upgrade code

@eileenmcnaughton
Copy link
Contributor

OK - let's merge this but I have a worry that the legacy group I created didn't save the relative dates - similar to the pledge one - and that feels suspicious to me - like there could have been a previous regression affecting creating new smart groups with relative dates. This 'fixes' but could it be the case that a smart group created in 5.15 with criteria 'End date' = 'this.month' is NOT working as a relative group - there might be some issue there to track

@seamuslee001
Copy link
Contributor Author

@eileenmcnaughton i have also just pushed up a commit to fix the batch form and some report filters

@eileenmcnaughton
Copy link
Contributor

Ah - looks like I messed up the regex here 7f17570#diff-54576153cb6a72d6ea7e22a5e8ca63f2R442

@seamuslee001 seamuslee001 force-pushed the member_fields_datepicker_conversion branch from 35fdaa5 to d49e404 Compare September 2, 2019 22:38
…mber search screen

Ensure that relative fields are handled as per newer handling

Update to eileen's metadata date_query functions and fix template

Fix upgrade by including each of the 3 separate field names
…d add in unit test of problem with mulitple relative dates in the one smart group

Fix test that was just added by moving assigning of fieldPossibilities lower
@seamuslee001 seamuslee001 force-pushed the member_fields_datepicker_conversion branch from 25237b3 to 1915f8d Compare September 3, 2019 00:27
@seamuslee001 seamuslee001 merged commit 5ef27eb into civicrm:master Sep 3, 2019
@seamuslee001 seamuslee001 deleted the member_fields_datepicker_conversion branch September 3, 2019 01:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants