[ready for core review] CRM-19153 Fixed future payment dates for future start pledge dates #8785
Conversation
|
Could one of the admins please verify this patch? |
|
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 |
| @@ -934,7 +934,7 @@ public static function processFormContribution( | |||
| $pledgeParams['create_date'] = $pledgeParams['start_date'] = $pledgeParams['scheduled_date'] = date("Ymd"); | |||
There was a problem hiding this comment.
@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 ?
There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
I agree @seamuslee001
|
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. |
|
Also, please fix test errors. Thx. |
Edzelopez
changed the title
|
jenkins, test this please |
Edzelopez
force-pushed
the
CRM-19153
branch
2 times, most recently
from
dd0568f to
e9f772d
Compare
|
Could one of the admins please verify this? |
| @@ -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'))); | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@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
---------------------------------------- * 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
---------------------------------------- * 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
---------------------------------------- * 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
---------------------------------------- * 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
|
Rebased PR. Could one of the admins please verify this patch? |
|
@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? |
|
@monishdeb could you please QA this? Thanks. |
|
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 |
|
@eileenmcnaughton I already assigned on https://docs.google.com/document/d/107kGgGCQnsjPlGQqXUqO2P7u1mfdtnJ8EQ8pq_L7mIo/edit# for QA but not started review, so can I proceed ? |
|
@monishdeb sure don't let me stop you! |
|
cool :D |
|
@eileenmcnaughton @monishdeb THANKS! |
|
Tested pledge payments using Authorize.net, and tried on every scenario like :
Working fine |
https://issues.civicrm.org/jira/browse/CRM-19153