+ Start a Discussion
ManderMander 

What's the issue with doing a DML in a trigger?

Hi!  I outsourced a request that was over my head, but there's an issue we didn't consider that our partner wants to charge a lot to fix.  I'd like to do it myself, but they're recommending against it.  Can someone help me understand if their reason makes sense?  
 
We use Cases as Product RMAs.  Each RMA Case has at least one custom object Return Line Item (RLI) related to it.  The RLI contains the product information (part number, quantity sold, qty returning, qty received, etc).  Our problem was that large customers would only return part of the product they requested an RMA for.  Because RMA cases had to be processed in full, credit would be delayed and inventory would be incorrect.  We asked our vendor to create a process that would create a second case for anything that had been returned and update the original case to show only what was pending return.

So for example:
Original Case
Line 1 has qty 3 expected and nothing has been received
Line 2 has qty 2 expected and 1 has been received
Line 3 has qty 4 expected and 4 have been received
 
Press Button.
 
Line 1 for qty 3 stays on the original case
Line 2 for qty 1 stays on the original case
 
Line 2 for qty 1 moves to the cloned case
Line 3 for qty 4 moves to the cloned case
  
Our issue is that from the example above Line 3 still exists on the original case and the qty expected is now 0.  It’s causing confusion in our process and I want to delete that RLI from the original case.  I created the following code to do so, but when I asked the opinion of the person who created the original process he said “There is the issue of doing DML in a trigger, and doing a query in a for loop is never a good idea, especially with how many transactions your trigger structure currently processes.”  What does he mean?  Is it really a concern since only one case would be updated at a time?
 
trigger DeleteZeroQtyReturn on Return_Line_Item__c (after update) {

List<Id> lstId = new List<Id>();
 
for(Return_Line_Item__c RLI: Trigger.old){
        List<Return_Line_Item__c> existRLIList = [Select Id from Return_Line_Item__c where QTY_Returned__c = 0 and Split_with_Button__c=true];
        delete existRLIList;
    }

}

 
mjohnson-TICmjohnson-TIC
Its not usually an issue unless you regularly update these records in batch sizes greater than 100.
ManderMander
Okay, good because we don't.  But let's say we did.  Why does that matter?
mjohnson-TICmjohnson-TIC
When running the update, you would get an error message saying you have reached a governor limit in DML or select statements.
Swati GSwati G
Why you need query in for loop?

You can query Return_Line_Item__c which are updated and have quantity as 0.

List<Return_Line_Item__c> existRLIList = [Select Id from Return_Line_Item__c where QTY_Returned__c = 0 and Split_with_Button__c=true and id in: Trigger.new ];
elete existRLIList;