From 1934b43d91a3905b8d10c78811e02a902441051b Mon Sep 17 00:00:00 2001 From: Nada Ismail <112021025+nadaismail-stripe@users.noreply.github.com> Date: Wed, 25 Jan 2023 11:37:26 -0800 Subject: [PATCH] Ensure updateCoupons trigger is deterministic (#988) --- .../classes/OrderCouponTriggerHandler.cls | 51 +++++++++++++- .../classes/test_updateOrderCouponTrigger.cls | 68 ++++++++++++++++--- .../default/triggers/updateCoupons.trigger | 5 +- 3 files changed, 110 insertions(+), 14 deletions(-) diff --git a/sfdx/force-app/main/default/classes/OrderCouponTriggerHandler.cls b/sfdx/force-app/main/default/classes/OrderCouponTriggerHandler.cls index 38aa0b5ac3..97b2ab4017 100644 --- a/sfdx/force-app/main/default/classes/OrderCouponTriggerHandler.cls +++ b/sfdx/force-app/main/default/classes/OrderCouponTriggerHandler.cls @@ -1,7 +1,7 @@ public with sharing class OrderCouponTriggerHandler { public class CouponException extends Sentry_Exception {} - private Map quoteIdsToOrderIds = null; + private Map quoteIdsToOrderIds = null; private Map quoteLines = null; // the order coupons that are bulk persisted at the end of the trigger execution private List orderCouponsToSave = null; @@ -21,6 +21,55 @@ public with sharing class OrderCouponTriggerHandler { quoteLines = new Map(); } + public void process() { + removePreviouslyProcessedOrders(); + processQuoteCoupons(); + processQuoteLineCoupons(); + } + + // in case the user has workflows that would cause the trigger to fire twice, let's ensure we don't create duplicate Order Stripe Coupons + public void removePreviouslyProcessedOrders() { + // create the reverse map of order id -> quote id + Map orderIdsToQuoteIds = new Map(); + for (Id quoteId : quoteIdsToOrderIds.keySet()) { + orderIdsToQuoteIds.put(quoteIdsToOrderIds.get(quoteId), quoteId); + } + + Map orderItemsById = new Map([SELECT Id, OrderId FROM OrderItem WHERE OrderId = :quoteIdsToOrderIds.values()]); + + // query for all Order Stripe Coupons that are tied to these orders or order items + List coupons = [ + SELECT + Order__c, + Order_Item__c + FROM Order_Stripe_Coupon__c + WHERE + Order__c = :quoteIdsToOrderIds.values() + OR Order_Item__c = :orderItemsById.keySet() + ]; + + for (Order_Stripe_Coupon__c coupon : coupons) { + Id orderId = null; + if (coupon.Order_Item__c != null) { + orderId = orderItemsById.get(coupon.Order_Item__c).OrderId; + } else if (coupon.Order__c != null) { + orderId = coupon.Order__c; + } + + // this should really never happen since an Order Stripe Coupon + // should always be created with either Order__c or Order_Item__c fields set + if (orderId == null) { + continue; + } + + // remove any quote we have already processed + Id quoteId = orderIdsToQuoteIds.get(orderId); + if (quoteIdsToOrderIds.containsKey(quoteId)) { + quoteIdsToOrderIds.remove(quoteId); + } + } + } + public void processQuoteCoupons() { // SECTION: fetch the quote->order coupon data List qscAssociations = getQuoteCouponAssociations(quoteIdsToOrderIds.keySet()); diff --git a/sfdx/force-app/main/default/classes/test_updateOrderCouponTrigger.cls b/sfdx/force-app/main/default/classes/test_updateOrderCouponTrigger.cls index ebf5cab373..3f044706c1 100644 --- a/sfdx/force-app/main/default/classes/test_updateOrderCouponTrigger.cls +++ b/sfdx/force-app/main/default/classes/test_updateOrderCouponTrigger.cls @@ -5,6 +5,7 @@ public with sharing class test_updateOrderCouponTrigger { private static final Integer COUPON_PERCENT_OFF = 55; private static final String COUPON_DURATION = 'once'; private static final String ORDER_STATUS_ACTIVATED = 'Activated'; + private static final String ORDER_STATUS_DRAFT = 'Draft'; @IsTest static public void testUpdateOrderCouponTriggerOnQuoteCoupons() { @@ -13,15 +14,14 @@ public with sharing class test_updateOrderCouponTrigger { return; } - // Setup test data + // setup test data SBQQ__Quote__c quote = createQuote(); Quote_Stripe_Coupon__c coupon = createStripeCoupon(); createStripeCouponQuoteAssociation(quote.Id, coupon.Id); + // fires trigger Test.startTest(); - Order order = activateOrderFromQuote(quote); - Test.stopTest(); // sanity check that the Quote Stripe Coupon Association was created @@ -38,7 +38,7 @@ public with sharing class test_updateOrderCouponTrigger { System.assertEquals(COUPON_DURATION, quoteCoupon.Duration__c); // trigger should have created an Order Stripe Coupon - List orderStripeCoupons = getOrderStripeCoupon(quoteCoupon.Id); + List orderStripeCoupons = getOrderCouponFromQuoteCouponId(quoteCoupon.Id); System.assertEquals(1, orderStripeCoupons.size()); Order_Stripe_Coupon__c orderStripeCoupon = orderStripeCoupons.get(0); @@ -52,16 +52,15 @@ public with sharing class test_updateOrderCouponTrigger { return; } - // Setup test data + // setup test data SBQQ__Quote__c quote = createQuote(); Quote_Stripe_Coupon__c coupon = createStripeCoupon(); SBQQ__QuoteLine__c quoteLine = [SELECT Id FROM SBQQ__QuoteLine__c WHERE SBQQ__Quote__c = :quote.Id]; createStripeCouponQuoteLineAssociation(quoteLine.Id, coupon.Id); + // fires trigger Test.startTest(); - Order order = activateOrderFromQuote(quote); - Test.stopTest(); List quoteLines = [SELECT Id FROM SBQQ__QuoteLine__c WHERE SBQQ__Quote__c = :quote.Id]; @@ -81,7 +80,7 @@ public with sharing class test_updateOrderCouponTrigger { System.assertEquals(COUPON_DURATION, quoteCoupon.Duration__c); // trigger should have created an Order Stripe Coupon - List orderStripeCoupons = getOrderStripeCoupon(quoteCoupon.Id); + List orderStripeCoupons = getOrderCouponFromQuoteCouponId(quoteCoupon.Id); System.assertEquals(1, orderStripeCoupons.size()); Order_Stripe_Coupon__c orderStripeCoupon = orderStripeCoupons.get(0); @@ -91,6 +90,48 @@ public with sharing class test_updateOrderCouponTrigger { System.assertEquals(1, orderItems.size()); } + @IsTest + static public void testDoubleFiringTrigger() { + Boolean isCpqInstalled = utilities.isCpqEnabled(); + if(!isCpqInstalled) { + return; + } + + // setup test data + SBQQ__Quote__c quote = createQuote(); + SBQQ__QuoteLine__c quoteLine = [SELECT Id FROM SBQQ__QuoteLine__c WHERE SBQQ__Quote__c = :quote.Id]; + + // add a coupon on the quote + Quote_Stripe_Coupon__c quoteCoupon = createStripeCoupon(); + createStripeCouponQuoteAssociation(quote.Id, quoteCoupon.Id); + + // add two coupons on the quote line + Quote_Stripe_Coupon__c quoteLineCoupon = createStripeCoupon(); + createStripeCouponQuoteLineAssociation(quoteLine.Id, quoteLineCoupon.Id); + createStripeCouponQuoteLineAssociation(quoteLine.Id, quoteLineCoupon.Id); + + // fire trigger by activating the quote + Order order = activateOrderFromQuote(quote); + + // initial trigger should have created 3 order coupons (1 on the order and 2 on the order line) + List orderStripeCoupons = getOrderCouponFromQuoteCouponId(quoteCoupon.Id); + System.assertEquals(1, orderStripeCoupons.size()); + + List orderLineStripeCoupons = getOrderCouponFromQuoteCouponId(quoteLineCoupon.Id); + System.assertEquals(2, orderLineStripeCoupons.size()); + + // now let's refire the updateCoupons trigger + Test.startTest(); + refireTrigger(order); + Test.stopTest(); + + // confirm that no new order coupons were created due to the trigger firing again + orderStripeCoupons = getOrderCouponFromQuoteCouponId(quoteCoupon.Id); + System.assertEquals(1, orderStripeCoupons.size()); + orderLineStripeCoupons = getOrderCouponFromQuoteCouponId(quoteLineCoupon.Id); + System.assertEquals(2, orderLineStripeCoupons.size()); + } + static private Order activateOrderFromQuote(SBQQ__Quote__c quote) { // update quote to be ordered quote.SBQQ__Ordered__c = true; @@ -244,10 +285,19 @@ public with sharing class test_updateOrderCouponTrigger { ]; } - static public List getOrderStripeCoupon(Id quoteCouponId) { + static public List getOrderCouponFromQuoteCouponId(Id quoteCouponId) { return [ SELECT Id, Name__c, Percent_Off__c, Duration__c, Quote_Stripe_Coupon_Id__c, Order__c FROM Order_Stripe_Coupon__c WHERE Quote_Stripe_Coupon_Id__c = :quoteCouponId]; } + + // This helper is used only in test to cause the updateCoupons trigger to refire + static private void refireTrigger(Order order) + { + order.Status = ORDER_STATUS_DRAFT; + Database.update(order, true); + order.Status = ORDER_STATUS_ACTIVATED; + Database.update(order, true); + } } diff --git a/sfdx/force-app/main/default/triggers/updateCoupons.trigger b/sfdx/force-app/main/default/triggers/updateCoupons.trigger index ed3bc03fd8..3318d90304 100644 --- a/sfdx/force-app/main/default/triggers/updateCoupons.trigger +++ b/sfdx/force-app/main/default/triggers/updateCoupons.trigger @@ -1,6 +1,4 @@ trigger updateCoupons on Order (after update) { - public class CouponException extends Exception {} - try { // for all new Orders, check if the corresponding Quote has coupons and duplicate/copy to the corresponding order Map quoteIdsToOrderIds = new Map(); @@ -16,8 +14,7 @@ trigger updateCoupons on Order (after update) { } OrderCouponTriggerHandler handler = new OrderCouponTriggerHandler(quoteIdsToOrderIds); - handler.processQuoteCoupons(); - handler.processQuoteLineCoupons(); + handler.process(); handler.persistChanges(); handler.refreshState(new Map());