[ready for core review] CRM-19153 Fixed future payment dates for future start pledge dates #8785

Edzelopez · 2016-07-29T16:03:49Z

CRM-19153: Future pledge start date causes improper future pledge payment dates
https://issues.civicrm.org/jira/browse/CRM-19153

Edzelopez · 2016-07-29T17:52:42Z

Could one of the admins please verify this patch?

eileenmcnaughton · 2016-07-31T21:27:39Z

Can we get a unit test on this? probably in the api_v3_ContributionPageTest class - which is where all the tests are for submitting that form

JoeMurray · 2016-08-16T17:55:31Z

CRM/Contribute/Form/Contribution/Confirm.php

@@ -934,7 +934,7 @@ public static function processFormContribution(
        $pledgeParams['create_date'] = $pledgeParams['start_date'] = $pledgeParams['scheduled_date'] = date("Ymd");


@mlutfy @eileenmcnaughton I don't want to cause more work for JMA ;) but shouldn't date formats in reports be drawn from civicrm/admin/setting/date?action=reset=1 ?

@JoeMurray I think the question is what is the destination once we have formatted the date, In this case here we are in the processFormContribution so most likely we are going to be inserting a date into the DB so need for format it into a date format that the db knows and i think Ymd is common for that. If we are building a form or something public then yes we would need to consider localised date formats. At least that is my thinking but @mlutfy @eileenmcnaughton correct me if i am wrong.

I agree @seamuslee001

JoeMurray · 2016-08-16T18:06:43Z

Could you ensure that there are automated tests for every field we added to forms. If the API tests don't included separate tests for each field, please ensure that all new fields are tested for set and get actions.

JoeMurray · 2016-08-16T18:08:07Z

Also, please fix test errors. Thx.

Edzelopez · 2016-09-02T13:00:31Z

jenkins, test this please

Edzelopez · 2016-09-08T07:47:19Z

Could one of the admins please verify this?

JoeMurray · 2016-09-09T20:28:13Z

CRM/Contribute/Form/ContributionPage/Amount.php

@@ -518,7 +518,7 @@ public function postProcess() {
        'calendar_month' => 'pledge_calendar_month',
      );
      if ($params['pledge_default_toggle'] == 'contribution_date') {
-        $fieldValue = json_encode(array('contribution_date' => date('Ymd')));
+        $fieldValue = json_encode(array('contribution_date' => date('m/d/Y')));


This smells a bit to me since there are settings for how dates are formatted. Just because the default format is American m/d/Y doesn't mean this will work in Canada for Ymd or d/m/Y formats. I'm going to see about creating an instance to check this.

Actually, this change was made in order to match how CiviCRM saves dates when used from the datepicker. I will check with Pradeep to see if there are any CRM functions I should be using instead.

@JoeMurray
I've checked and it looks good. Like @seamuslee001 mentioned above, this is only to save the current date in the database.

---------------------------------------- * CRM-19153: Future pledge start date causes improper future pledge payment dates https://issues.civicrm.org/jira/browse/CRM-19153

…week ---------------------------------------- * CRM-19153: Future pledge start date causes improper future pledge payment dates https://issues.civicrm.org/jira/browse/CRM-19153

…edge not selected ---------------------------------------- * CRM-19153: Future pledge start date causes improper future pledge payment dates https://issues.civicrm.org/jira/browse/CRM-19153

---------------------------------------- * CRM-19153: Future pledge start date causes improper future pledge payment dates https://issues.civicrm.org/jira/browse/CRM-19153

Edzelopez · 2016-10-25T10:37:18Z

Rebased PR. Could one of the admins please verify this patch?

Edzelopez · 2016-11-08T09:07:48Z

@eileenmcnaughton We would like to get these fixes merged as they are critical. I have included test coverage for them in this PR. Is there anything more I should be looking at in terms of date formats and localisation?

Edzelopez · 2016-11-10T06:24:34Z

@monishdeb could you please QA this? Thanks.

eileenmcnaughton · 2016-11-10T08:52:28Z

Hey, I'm going to work through the RC list in line with the calendar & I'll make sure this is assigned https://calendar.google.com/calendar/embed?src=OTFxaWIwdjE3bnU0b29tOGN2OHZzczlqcDBAZ3JvdXAuY2FsZW5kYXIuZ29vZ2xlLmNvbQ

monishdeb · 2016-11-10T09:14:53Z

@eileenmcnaughton I already assigned on https://docs.google.com/document/d/107kGgGCQnsjPlGQqXUqO2P7u1mfdtnJ8EQ8pq_L7mIo/edit# for QA but not started review, so can I proceed ?

eileenmcnaughton · 2016-11-10T09:15:38Z

@monishdeb sure don't let me stop you!

monishdeb · 2016-11-10T09:16:06Z

cool :D

Edzelopez · 2016-11-10T10:21:21Z

@eileenmcnaughton @monishdeb THANKS!

monishdeb · 2016-11-14T13:03:05Z

Tested pledge payments using Authorize.net, and tried on every scenario like :

Changed Date Input format
Future/past recurring start date

Working fine

civicrm-builder added the master label Jul 29, 2016

Edzelopez force-pushed the CRM-19153 branch from 3ffb3cd to e536046 Compare August 15, 2016 07:42

JoeMurray reviewed Aug 16, 2016
View reviewed changes

Edzelopez force-pushed the CRM-19153 branch from e536046 to 25aff72 Compare August 23, 2016 04:47

Edzelopez force-pushed the CRM-19153 branch from 4b95423 to 7c8cd80 Compare September 1, 2016 10:03

Edzelopez changed the title ~~CRM-19153 Fixed future payment dates for future start pledge dates~~ [ready for core review] CRM-19153 Fixed future payment dates for future start pledge dates Sep 1, 2016

Edzelopez force-pushed the CRM-19153 branch from 5d24bd3 to d12affc Compare September 2, 2016 07:58

Edzelopez force-pushed the CRM-19153 branch 2 times, most recently from dd0568f to e9f772d Compare September 8, 2016 07:00

JoeMurray reviewed Sep 9, 2016
View reviewed changes

Edzelopez added 11 commits October 25, 2016 16:04

CRM-19153 Fixed future payment dates for future start pledge dates

33dd32c

---------------------------------------- * CRM-19153: Future pledge start date causes improper future pledge payment dates https://issues.civicrm.org/jira/browse/CRM-19153

CRM-19153 Bug fixes for start date display & star date calculations

12e5325

---------------------------------------- * CRM-19153: Future pledge start date causes improper future pledge payment dates https://issues.civicrm.org/jira/browse/CRM-19153

CRM-19153 BUg fixes for non editable fields

d9a9ea3

---------------------------------------- * CRM-19153: Future pledge start date causes improper future pledge payment dates https://issues.civicrm.org/jira/browse/CRM-19153

CRM-19153 Bug fix for pledge payment

27da207

---------------------------------------- * CRM-19153: Future pledge start date causes improper future pledge payment dates https://issues.civicrm.org/jira/browse/CRM-19153

CRM-19153 Added unit test for pledge payments

603577b

---------------------------------------- * CRM-19153: Future pledge start date causes improper future pledge payment dates https://issues.civicrm.org/jira/browse/CRM-19153

CRM-19153 Fixed freeze date display, recurring for weekly installments

31fdc70

---------------------------------------- * CRM-19153: Future pledge start date causes improper future pledge payment dates https://issues.civicrm.org/jira/browse/CRM-19153

CRM-19153 Reverted changes to pledge base start date calculation for …

3549232

…week ---------------------------------------- * CRM-19153: Future pledge start date causes improper future pledge payment dates https://issues.civicrm.org/jira/browse/CRM-19153

CRM-19153 Prevented code from creating recurring contribution when pl…

cdc7779

…edge not selected ---------------------------------------- * CRM-19153: Future pledge start date causes improper future pledge payment dates https://issues.civicrm.org/jira/browse/CRM-19153

CRM-19153 Fixed unit test

a7c78cb

---------------------------------------- * CRM-19153: Future pledge start date causes improper future pledge payment dates https://issues.civicrm.org/jira/browse/CRM-19153

CRM-19153 Fixed jenkins errors

7e3c24b

---------------------------------------- * CRM-19153: Future pledge start date causes improper future pledge payment dates https://issues.civicrm.org/jira/browse/CRM-19153

CRM-19153 Fixed unit test

3044f49

---------------------------------------- * CRM-19153: Future pledge start date causes improper future pledge payment dates https://issues.civicrm.org/jira/browse/CRM-19153

Edzelopez force-pushed the CRM-19153 branch from e9f772d to 3044f49 Compare October 25, 2016 10:36

monishdeb merged commit 439dbd4 into civicrm:master Nov 14, 2016

pradpnayak deleted the CRM-19153 branch February 22, 2017 10:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ready for core review] CRM-19153 Fixed future payment dates for future start pledge dates #8785

[ready for core review] CRM-19153 Fixed future payment dates for future start pledge dates #8785

Edzelopez commented Jul 29, 2016

Edzelopez commented Jul 29, 2016

eileenmcnaughton commented Jul 31, 2016

JoeMurray Aug 16, 2016

seamuslee001 Sep 9, 2016

Edzelopez Oct 25, 2016

JoeMurray commented Aug 16, 2016

JoeMurray commented Aug 16, 2016

Edzelopez commented Sep 2, 2016

Edzelopez commented Sep 8, 2016

JoeMurray Sep 9, 2016

Edzelopez Sep 12, 2016

Edzelopez Oct 25, 2016

Edzelopez commented Oct 25, 2016

Edzelopez commented Nov 8, 2016

Edzelopez commented Nov 10, 2016

eileenmcnaughton commented Nov 10, 2016

monishdeb commented Nov 10, 2016

eileenmcnaughton commented Nov 10, 2016

monishdeb commented Nov 10, 2016

Edzelopez commented Nov 10, 2016

monishdeb commented Nov 14, 2016

		@@ -934,7 +934,7 @@ public static function processFormContribution(
		$pledgeParams['create_date'] = $pledgeParams['start_date'] = $pledgeParams['scheduled_date'] = date("Ymd");

[ready for core review] CRM-19153 Fixed future payment dates for future start pledge dates #8785

[ready for core review] CRM-19153 Fixed future payment dates for future start pledge dates #8785

Conversation

Edzelopez commented Jul 29, 2016

Edzelopez commented Jul 29, 2016

eileenmcnaughton commented Jul 31, 2016

JoeMurray Aug 16, 2016

Choose a reason for hiding this comment

seamuslee001 Sep 9, 2016

Choose a reason for hiding this comment

Edzelopez Oct 25, 2016

Choose a reason for hiding this comment

JoeMurray commented Aug 16, 2016

JoeMurray commented Aug 16, 2016

Edzelopez commented Sep 2, 2016

Edzelopez commented Sep 8, 2016

JoeMurray Sep 9, 2016

Choose a reason for hiding this comment

Edzelopez Sep 12, 2016

Choose a reason for hiding this comment

Edzelopez Oct 25, 2016

Choose a reason for hiding this comment

Edzelopez commented Oct 25, 2016

Edzelopez commented Nov 8, 2016

Edzelopez commented Nov 10, 2016

eileenmcnaughton commented Nov 10, 2016

monishdeb commented Nov 10, 2016

eileenmcnaughton commented Nov 10, 2016

monishdeb commented Nov 10, 2016

Edzelopez commented Nov 10, 2016

monishdeb commented Nov 14, 2016