dev/core#2487 Update defaults for civicrm_contribution_recur table #19934
Conversation
|
(Standard links)
|
| $pendingID = CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_ContributionRecur', 'contribution_status_id', 'Pending'); | ||
| CRM_Core_DAO::executeQuery(" | ||
| ALTER TABLE `civicrm_contribution_recur` | ||
| MODIFY COLUMN `start_date` datetime NOT NULL DEFAULT CURRENT_TIMESTAMP COMMENT 'The date the first scheduled recurring contribution occurs.', |
There was a problem hiding this comment.
Should these date columns be datetime or timestamps?
There was a problem hiding this comment.
@seamuslee001 I tested & the default works with datetime - which is what they already are. I wanted to leave any type change out of scope
There was a problem hiding this comment.
I too prefer to have separate PR/issue for changing default value compared to changing the type.
There was a problem hiding this comment.
@JoeMurray I did have the PR for that change (linked above) - I closed it to incorporate into this as I felt it had been canvassed & upgrade scripts don't play well as separate PRs
| @@ -587,7 +591,7 @@ public static function &fields() { | |||
| 'import' => TRUE, | |||
| 'where' => 'civicrm_contribution_recur.contribution_status_id', | |||
| 'export' => TRUE, | |||
| 'default' => '1', | |||
| 'default' => '2', | |||
There was a problem hiding this comment.
Might be nice to have comment
// ie Pending
There was a problem hiding this comment.
Agree it would be nice, but we can't put comments in a generated DAO file.
|
I'm merging as Eileen addressed feedback without any additional pushback, and I think this is better in than out. |
|
Thanks @colemanw - one thing I've been mulling over is https://lab.civicrm.org/dev/core/-/issues/2474 & perhaps whether even just adding a try-catch might mitigate the risk of that |
Overview
dev/core#2487 Update defaults for civicrm_contribution_recur table
Per https://lab.civicrm.org/dev/core/-/issues/2487 this brings the api behaviour closer to the UI - providing the same defaults as the UI does
Before
v4 api requires fields for which reasonable defaults could be used. Both apis apply the 'wrong' default for contribution_status_id and as it is not required it is easy to mistakenly create recurring contributions that bypass the code logic to manage status
After
Defaults in line with the UI
Technical Details
This bubbled to the top because of pain around tests for apiv4
Comments
The change of status for contribution_status_id was previously discussed here #19512 - the key thing to note (which was obviously not clear up front) is that the Contribution BAO has code to update the recurring contribution status as contributions are created with contribution_recur_id set