Page MenuHomePhabricator
Create Task
Maniphest T357471

Consolidate rest of normalize into WMFMessage class
Closed, ResolvedPublic
Actions

Details

SubjectRepoBranchLines +/-
More follow on clean up - remove contact_id variablewikimedia/fundraising/crmmaster+9 -26
Move contribution_tracking_id lookup to Recurring Messagewikimedia/fundraising/crmmaster+287 -69
Update OptInQueueConsuer to use a Message class, apiv4wikimedia/fundraising/crmmaster+135 -95
Clean up handling of fiscal_number / legal_identifierwikimedia/fundraising/crmmaster+22 -25
Move validation of required fields, duplicates onto the messagewikimedia/fundraising/crmmaster+95 -43
Return handling for tags & groups to import classeswikimedia/fundraising/crmmaster+15 -25
Move function back to only callerwikimedia/fundraising/crmmaster+22 -22
Was it just a good idea at the time?wikimedia/fundraising/crmmaster+52 -127
Move getContributionRecurID to Message classwikimedia/fundraising/crmmaster+137 -66
Switch test to use api rather than overloaded functionwikimedia/fundraising/crmmaster+10 -8
Use getter for currency in exception messagewikimedia/fundraising/crmmaster+1 -1
Fold the amount wrangling back into the parentwikimedia/fundraising/crmmaster+14 -34
Simplify early return check on moneywikimedia/fundraising/crmmaster+6 -8
Simplify setting of amountswikimedia/fundraising/crmmaster+16 -8
Move converstion rate handling to getterswikimedia/fundraising/crmmaster+53 -34
Extract getConversionRatewikimedia/fundraising/crmmaster+27 -20
Move function called from one class to that classwikimedia/fundraising/crmmaster+18 -18
Move validation to RecurMessage classwikimedia/fundraising/crmmaster+19 -6
Move 2 line function back into callerwikimedia/fundraising/crmmaster+2 -20
Move get subscriber ID to child message classwikimedia/fundraising/crmmaster+54 -20
Extract getFinancialTypeID() in message classwikimedia/fundraising/crmmaster+28 -14
Update recurring Queue consumer to instantiate recurring message directlywikimedia/fundraising/crmmaster+6 -4
Show related patches Customize query in gerrit

Related Objects
Search...

Event Timeline

Eileenmcnaughton created this task.Feb 13 2024, 8:34 PM
Eileenmcnaughton moved this task from Triage to Sprint +1 on the Fundraising-Backlog board.
AKanji-WMF moved this task from Sprint +1 to Current Sprint on the Fundraising-Backlog board.Feb 13 2024, 8:38 PM
AKanji-WMF added a project: Fundraising Sprint: canOfWorms(🪱).Feb 13 2024, 9:23 PM
gerritbot added a comment.Feb 14 2024, 10:31 PM

Change 1003554 had a related patch set uploaded (by Eileen; author: Eileen):

[wikimedia/fundraising/crm@master] Update recurring Queue consumer to instantiate recurring message directly

https://gerrit.wikimedia.org/r/1003554

gerritbot added a project: Patch-For-Review.Feb 14 2024, 10:31 PM

Change 1003555 had a related patch set uploaded (by Eileen; author: Eileen):

[wikimedia/fundraising/crm@master] Extract getFinancialTypeID() in message class

https://gerrit.wikimedia.org/r/1003555

gerritbot added a comment.Feb 14 2024, 10:51 PM

Change 1003560 had a related patch set uploaded (by Eileen; author: Eileen):

[wikimedia/fundraising/crm@master] Move get subscriber ID to child message class

https://gerrit.wikimedia.org/r/1003560

gerritbot added a comment.Feb 16 2024, 8:28 PM

Change 1003554 merged by jenkins-bot:

[wikimedia/fundraising/crm@master] Update recurring Queue consumer to instantiate recurring message directly

https://gerrit.wikimedia.org/r/1003554

gerritbot added a comment.Feb 16 2024, 8:51 PM

Change 1003555 merged by jenkins-bot:

[wikimedia/fundraising/crm@master] Extract getFinancialTypeID() in message class

https://gerrit.wikimedia.org/r/1003555

gerritbot added a comment.Feb 19 2024, 9:14 PM

Change 1003560 merged by jenkins-bot:

[wikimedia/fundraising/crm@master] Move get subscriber ID to child message class

https://gerrit.wikimedia.org/r/1003560

Maintenance_bot removed a project: Patch-For-Review.Feb 19 2024, 9:30 PM
Eileenmcnaughton claimed this task.Feb 19 2024, 10:32 PM
Eileenmcnaughton moved this task from Backlog to Doing on the Fundraising Sprint: canOfWorms(🪱) board.
gerritbot added a comment.Feb 19 2024, 10:33 PM

Change 1004790 had a related patch set uploaded (by Eileen; author: Eileen):

[wikimedia/fundraising/crm@master] Move validation to RecurMessage class

https://gerrit.wikimedia.org/r/1004790

gerritbot added a project: Patch-For-Review.Feb 19 2024, 10:33 PM
gerritbot added a comment.Feb 19 2024, 10:41 PM

Change 1004791 had a related patch set uploaded (by Eileen; author: Eileen):

[wikimedia/fundraising/crm@master] Move getContributionRecurID to Message class

https://gerrit.wikimedia.org/r/1004791

gerritbot added a comment.Feb 20 2024, 12:12 AM

Change 1004795 had a related patch set uploaded (by Eileen; author: Eileen):

[wikimedia/fundraising/crm@master] Switch test to use api rather than overloaded function

https://gerrit.wikimedia.org/r/1004795

gerritbot added a comment.Feb 20 2024, 12:12 AM

Change 1004796 had a related patch set uploaded (by Eileen; author: Eileen):

[wikimedia/fundraising/crm@master] Move contribution_tracking_id lookup to Recurring Message

https://gerrit.wikimedia.org/r/1004796

gerritbot added a comment.Feb 20 2024, 12:20 AM

Change 1004797 had a related patch set uploaded (by Eileen; author: Eileen):

[wikimedia/fundraising/crm@master] Move 2 line function back into caller

https://gerrit.wikimedia.org/r/1004797

gerritbot added a comment.Feb 20 2024, 4:32 PM

Change 1004797 merged by jenkins-bot:

[wikimedia/fundraising/crm@master] Move 2 line function back into caller

https://gerrit.wikimedia.org/r/1004797

gerritbot added a comment.Feb 20 2024, 4:32 PM

Change 1004790 merged by jenkins-bot:

[wikimedia/fundraising/crm@master] Move validation to RecurMessage class

https://gerrit.wikimedia.org/r/1004790

gerritbot added a comment.Feb 20 2024, 10:22 PM

Change 1005181 had a related patch set uploaded (by Eileen; author: Eileen):

[wikimedia/fundraising/crm@master] Extract getConversionRate

https://gerrit.wikimedia.org/r/1005181

gerritbot added a comment.Feb 20 2024, 10:22 PM

Change 1005182 had a related patch set uploaded (by Eileen; author: Eileen):

[wikimedia/fundraising/crm@master] Move converstion rate handling to getters

https://gerrit.wikimedia.org/r/1005182

gerritbot added a comment.Feb 20 2024, 10:22 PM

Change 1005183 had a related patch set uploaded (by Eileen; author: Eileen):

[wikimedia/fundraising/crm@master] Simplify setting of amounts

https://gerrit.wikimedia.org/r/1005183

gerritbot added a comment.Feb 20 2024, 10:22 PM

Change 1005184 had a related patch set uploaded (by Eileen; author: Eileen):

[wikimedia/fundraising/crm@master] Simplify early return check on money

https://gerrit.wikimedia.org/r/1005184

gerritbot added a comment.Feb 20 2024, 10:22 PM

Change 1005185 had a related patch set uploaded (by Eileen; author: Eileen):

[wikimedia/fundraising/crm@master] Fold the amount wrangling back into the parent

https://gerrit.wikimedia.org/r/1005185

gerritbot added a comment.Feb 20 2024, 11:13 PM

Change 1005198 had a related patch set uploaded (by Eileen; author: Eileen):

[wikimedia/fundraising/crm@master] Move function called from one class to that class

https://gerrit.wikimedia.org/r/1005198

Eileenmcnaughton moved this task from Doing to Ready for Review on the Fundraising Sprint: canOfWorms(🪱) board.Feb 23 2024, 6:17 PM
gerritbot added a comment.Feb 27 2024, 5:26 PM

Change 1005198 merged by jenkins-bot:

[wikimedia/fundraising/crm@master] Move function called from one class to that class

https://gerrit.wikimedia.org/r/1005198

gerritbot added a comment.Feb 28 2024, 3:39 PM

Change 1005181 merged by jenkins-bot:

[wikimedia/fundraising/crm@master] Extract getConversionRate

https://gerrit.wikimedia.org/r/1005181

gerritbot added a comment.Feb 28 2024, 3:45 PM

Change 1005182 merged by jenkins-bot:

[wikimedia/fundraising/crm@master] Move converstion rate handling to getters

https://gerrit.wikimedia.org/r/1005182

gerritbot added a comment.Feb 28 2024, 3:47 PM

Change 1005183 merged by jenkins-bot:

[wikimedia/fundraising/crm@master] Simplify setting of amounts

https://gerrit.wikimedia.org/r/1005183

gerritbot added a comment.Feb 28 2024, 3:48 PM

Change 1007365 had a related patch set uploaded (by Ejegg; author: Ejegg):

[wikimedia/fundraising/crm@master] Use getter for currency in exception message

https://gerrit.wikimedia.org/r/1007365

gerritbot added a comment.Feb 28 2024, 3:48 PM

Change 1005184 merged by jenkins-bot:

[wikimedia/fundraising/crm@master] Simplify early return check on money

https://gerrit.wikimedia.org/r/1005184

gerritbot added a comment.Feb 28 2024, 4:04 PM

Change 1005185 merged by jenkins-bot:

[wikimedia/fundraising/crm@master] Fold the amount wrangling back into the parent

https://gerrit.wikimedia.org/r/1005185

gerritbot added a comment.Feb 28 2024, 11:55 PM

Change 1007365 merged by jenkins-bot:

[wikimedia/fundraising/crm@master] Use getter for currency in exception message

https://gerrit.wikimedia.org/r/1007365

AKanji-WMF edited projects, added Fundraising Sprint: didAnyoneTryThis(); removed Fundraising Sprint: canOfWorms(🪱).Feb 29 2024, 5:43 PM
gerritbot added a comment.Feb 29 2024, 10:30 PM

Change 1004795 merged by jenkins-bot:

[wikimedia/fundraising/crm@master] Switch test to use api rather than overloaded function

https://gerrit.wikimedia.org/r/1004795

Eileenmcnaughton moved this task from Backlog to Doing on the Fundraising Sprint: didAnyoneTryThis() board.Mar 4 2024, 8:44 PM
gerritbot added a comment.Mar 5 2024, 3:37 PM

Change 1004791 merged by jenkins-bot:

[wikimedia/fundraising/crm@master] Move getContributionRecurID to Message class

https://gerrit.wikimedia.org/r/1004791

gerritbot added a comment.Mar 5 2024, 8:32 PM

Change 1008946 had a related patch set uploaded (by Eileen; author: Eileen):

[wikimedia/fundraising/crm@master] Move function back to only caller

https://gerrit.wikimedia.org/r/1008946

gerritbot added a comment.Mar 7 2024, 8:02 PM

Change 1009596 had a related patch set uploaded (by Eileen; author: Eileen):

[wikimedia/fundraising/crm@master] Was it just a good idea at the time?

https://gerrit.wikimedia.org/r/1009596

gerritbot added a comment.Mar 7 2024, 8:26 PM

Change 1009596 merged by jenkins-bot:

[wikimedia/fundraising/crm@master] Was it just a good idea at the time?

https://gerrit.wikimedia.org/r/1009596

Eileenmcnaughton added a comment.Mar 7 2024, 11:12 PM

If anyone has headspace I'm looking at decommisioning wmf_civicrm_verify_message_and_stage (which would mean moving a cut down version back into each of the 2 callers)

It is called from 2 places

  • wmf_civicrm_contribution_message_import
  • UPIQueueConsumer::insertContributionRecur()

I think I can get the function down to 3 lines (which is small enough to move back)

$msg = $message->normalize();
$message->validate();
$msg = wmf_civicrm_add_contribution_tracking_if_missing($msg);

(one of the callers already calls $message->validate();)

The question really is about the $message->validate()

The current validate checks are

  • are gross, currency, gateway_txn_id & gateway present
  • is gross > 0 (ditto net if present)
  • is it a duplicate message

I could move all that to the DonationMessage::validate() & have RecurDonationMessage::validate() include parent::validate()

The question then is whether everywhere currently calling RecurDonationMessage::validate() is OK with those checks being done - which really means adding them into RecurringQueueConsumer::normalizeMessage since the other place that calls $message->validate(); already calls this normalize flow

If that IS OK it does beg the question as to why this Adam-esque logging happens

// just... don't
\Civi::log('wmf')->info('wmf_civicrm: Not freaking out about non-monetary message.');
gerritbot added a comment.Mar 8 2024, 2:32 PM

Change 1008946 merged by jenkins-bot:

[wikimedia/fundraising/crm@master] Move function back to only caller

https://gerrit.wikimedia.org/r/1008946

XenoRyet added a project: Fundraising Sprint: explain($LikeImFive).Mar 12 2024, 7:26 PM
Eileenmcnaughton moved this task from Backlog to Doing on the Fundraising Sprint: explain($LikeImFive) board.Mar 13 2024, 3:27 AM
gerritbot added a comment.Mar 14 2024, 8:26 PM

Change 1011181 had a related patch set uploaded (by Eileen; author: Eileen):

[wikimedia/fundraising/crm@master] Return handling for tags & groups to import classes

https://gerrit.wikimedia.org/r/1011181

Eileenmcnaughton moved this task from Doing to Ready for Review on the Fundraising Sprint: explain($LikeImFive) board.Mar 19 2024, 2:06 AM

I think we might have tugged on the thread for long enough from this end for now - I think once these ones are reviewed it might be better to focus on a different end

gerritbot added a comment.Mar 19 2024, 9:08 AM

Change 1012606 had a related patch set uploaded (by Eileen; author: Eileen):

[wikimedia/fundraising/crm@master] Move validation of required fields, duplicates onto the message

https://gerrit.wikimedia.org/r/1012606

AKanji-WMF edited projects, added Fundraising Sprint: fridayFridayGottaGetDownOnFriday($fun,$fun,$fun); removed Fundraising Sprint: explain($LikeImFive).Mar 26 2024, 8:51 PM
gerritbot added a comment.Mar 26 2024, 9:42 PM

Change #1011181 merged by jenkins-bot:

[wikimedia/fundraising/crm@master] Return handling for tags & groups to import classes

https://gerrit.wikimedia.org/r/1011181

gerritbot added a comment.Apr 3 2024, 2:47 AM

Change #1012606 merged by Eileen:

[wikimedia/fundraising/crm@master] Move validation of required fields, duplicates onto the message

https://gerrit.wikimedia.org/r/1012606

AKanji-WMF edited projects, added Fundraising Sprint: gone(); removed Fundraising Sprint: fridayFridayGottaGetDownOnFriday($fun,$fun,$fun).Apr 9 2024, 8:09 PM
AKanji-WMF edited projects, added Fundraising Sprint: hammertime($touch_this=false); removed Fundraising Sprint: gone().Apr 23 2024, 8:59 PM
Eileenmcnaughton moved this task from Backlog to Doing on the Fundraising Sprint: hammertime($touch_this=false) board.Apr 23 2024, 10:08 PM
gerritbot added a comment.Apr 24 2024, 2:05 AM

Change #1023564 had a related patch set uploaded (by Eileen; author: Eileen):

[wikimedia/fundraising/crm@master] Clean up handling of fiscal_number / legal_identifier

https://gerrit.wikimedia.org/r/1023564

gerritbot added a comment.Apr 29 2024, 9:41 PM

Change #1025472 had a related patch set uploaded (by Eileen; author: Eileen):

[wikimedia/fundraising/crm@master] Update OptInQueueConsuer to use a Message class, apiv4

https://gerrit.wikimedia.org/r/1025472

Eileenmcnaughton moved this task from Doing to Ready for Review on the Fundraising Sprint: hammertime($touch_this=false) board.Apr 30 2024, 12:32 AM

this one is the one currently needing review https://gerrit.wikimedia.org/r/c/wikimedia/fundraising/crm/+/1022309

AKanji-WMF edited projects, added Fundraising Sprint: ick(); removed Fundraising Sprint: hammertime($touch_this=false).Tue, May 7, 8:02 PM
Eileenmcnaughton closed subtask T363524: Remove unused import code from main codebase as Resolved.Tue, May 7, 8:07 PM
Eileenmcnaughton moved this task from Backlog to Ready for Review on the Fundraising Sprint: ick() board.Thu, May 9, 8:01 AM
gerritbot added a comment.Fri, May 10, 3:43 PM

Change #1023564 merged by jenkins-bot:

[wikimedia/fundraising/crm@master] Clean up handling of fiscal_number / legal_identifier

https://gerrit.wikimedia.org/r/1023564

gerritbot added a comment.Tue, May 14, 9:22 PM

Change #1025472 merged by Ejegg:

[wikimedia/fundraising/crm@master] Update OptInQueueConsuer to use a Message class, apiv4

https://gerrit.wikimedia.org/r/1025472

gerritbot added a comment.Fri, May 17, 4:38 AM

Change #1004796 abandoned by Eileen:

[wikimedia/fundraising/crm@master] Move contribution_tracking_id lookup to Recurring Message

Reason:

https://gerrit.wikimedia.org/r/1004796

Maintenance_bot removed a project: Patch-For-Review.Fri, May 17, 5:30 AM
gerritbot added a comment.Fri, May 17, 9:45 PM

Change #1032878 had a related patch set uploaded (by Eileen; author: Eileen):

[wikimedia/fundraising/crm@master] More follow on clean up - remove contact_id variable

https://gerrit.wikimedia.org/r/1032878

gerritbot added a project: Patch-For-Review.Fri, May 17, 9:45 PM
Eileenmcnaughton moved this task from Ready for Review to Done on the Fundraising Sprint: ick() board.Tue, May 21, 2:26 AM

This one is as long as a piece of string & there is def more that could be done but I think maybe under another phab

gerritbot added a comment.Tue, May 21, 2:58 PM

Change #1032878 merged by jenkins-bot:

[wikimedia/fundraising/crm@master] More follow on clean up - remove contact_id variable

https://gerrit.wikimedia.org/r/1032878

Maintenance_bot removed a project: Patch-For-Review.Tue, May 21, 3:31 PM
XenoRyet closed this task as Resolved.Tue, May 21, 7:03 PM
XenoRyet set Final Story Points to 8.
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL