3

I'm changing the Clients__c value in Mopping__c if the StartDate__c, Status__c or Opportunity__c have changed in the needed Rent__c.

Structure: if Mopping 1 is related to Rent 1 which is related to Opportunity 1, and EndDate__c of the Mopping 1 == StartDate__c of Rent 2 which is too related to Opportunity 1, Clients__c of Mopping 1 should be equals to Clients__c of Rent 2:

enter image description here

But! If StartDate__c of Rent 2 is changed and != to EndDate__c of the Mopping 1, Clients__c of Mopping 1 should be null again. The same thing with changing Status__c of Rent 2 to 'Canceled' and it's Opportunity__c.

We have to check the old Mopping-neighbors of moved Rent and also new Mopping-neighbors. That's why I'm checking Trigger.old and Trigger.new in the one List. If our Mopping__c haven't already needed Rent__c which meets the conditions mop.EndDate__c == rent.StartDate__c && mop.Rent__r.Opportunity__c == rent.Opportunity__c && rent.Status__c != 'Canceled', the mop.Clients__c field should be null. But if Rent__c that matches the conditions appears, the mop.Clients__c changes again. Schema:

enter image description here

The problem 1: there can be 2 or more Rents with mop.EndDate__c == rent.StartDate__c && mop.Rent__r.Opportunity__c == rent.Opportunity__c, but only one of them will be witn rent.Status__c != 'Canceled'. And they can be updated in the same time. So when I'm trying to added in my list 2 same Moppongs for Rents, that meet different conditions, I'm getting Duplicate error and can't update my List.

The problem 2: I have here the loop in the loop. So I'm also getting Too many SOQL queries: 101 when inserting or updating many Rents.

Trigger:

trigger ClientsTrigger on Rent__c (after insert, after update) {
    List<Rent__c> allRents = new List<Rent__c>();
    TriggerHelper helper = new TriggerHelper();

    if(Trigger.isAfter && Trigger.isInsert) {
        allRents.addAll(Trigger.new);
        helper.updatingHelper(allRents);
    }
    if(Trigger.isAfter && Trigger.isUpdate) {
        allRents.addAll(Trigger.new);
        allRents.addAll(Trigger.old);
        helper.updatingHelper(allRents);
    }
}

Helper:

public void updatingHelper(List<Rent__c> allRents){
    List<Mopping__c> moppingsToUpdate = new List<Mopping__c>();
    Set<Id> oppIds = new Set<Id>();

    for(Rent__c rent : allRents){
        oppIds.add(rent.Opportunity__c);
    }
    Map<Id, Opportunity> rentsMap = new Map<Id, Opportunity>(
        [SELECT Id, (SELECT Id, Clients__c, StartDate__c, Opportunity__c, Status__c FROM Rents__r WHERE EndDate__c >=: Date.TODAY())
         FROM Opportunity WHERE Id IN : oppIds AND Name != 'Test']
    );
    List<Mopping__c> actualMoppings = [SELECT Id, Clients__c, EndDate__c, Rent__r.Opportunity__c FROM Mopping__c
                                       WHERE Rent__r.Opportunity__c IN : oppIds AND EndDate__c >=: Date.TODAY()];
    for(Mopping__c actualMopping: actualMoppings){
        List<Rent__c> neededRents = rentsMap.get(actualMopping.Rent__r.Opportunity__c).Rents__r;
        List<Date> listDate = new List<Date>();

        //Loop in loop is bad because of "Too many SOQL queries: 101":
        for(Rent__c neededRent: neededRents){
            listDate.add(neededRent.StartDate__c);
            if(actualMopping.EndDate__c == neededRent.StartDate__c && actualMopping.Clients__c != String.valueOf(neededRent.Clients__c) && neededRent.Status__c != 'Canceled'){
                actualMopping.Clients__c = String.valueOf(neededRent.Clients__c);
                moppingsToUpdate.add(actualMopping);
            } else if(actualMopping.EndDate__c == neededRent.StartDate__c && neededRent.Status__c == 'Canceled' && !moppingsToUpdate.contains(actualMopping)){      
                actualMopping.Clients__c = '';
                //Sometimes one Rent__c can become 'Canceled' and another Rent__c with the same EndDate__c and Opportunity
                //can be not 'Canceled'. Then we get the same Mopping in our List:
                moppingsToUpdate.add(actualMopping);
            }
        }
        if(!listDate.contains(actualMopping.EndDate__c)){
            actualMopping.Clients__c = '';
            moppingsToUpdate.add(actualMopping);
        }
        listDate.clear();
    }
    if(!moppingsToUpdate.isEmpty()){
        //Error with Duplicates in List (with Set I have the same problem):
        update moppingsToUpdate;
    }
}
1

I think you should try to set up a singletone pattern for your helper class. You need to make sure that every instance of the helper class is independent. This will help you to reduce governor limits.

You are passing all Rents records that are changing. I think you should use here a logic to pass only those Rents that have changes on the fields that have an effect on you logic, StartDate__c and Status__c. This way you reduce the amount of records for further processing. Because you are passing all Opportunities. You can pass the oldMap to your helper class?

for(Rent__c rent : allRents){
    oppIds.add(rent.Opportunity__c);
}

You should not use for loops inside for loops:

for(Mopping__c actualMopping: actualMoppings){
    List<Rent__c> neededRents = rentsMap.get(actualMopping.Rent__r.Opportunity__c).Rents__r;
    List<Date> listDate = new List<Date>();

    //Loop in loop is bad because of "Too many SOQL queries: 101":
    for(Rent__c neededRent: neededRents){
        listDate.add(neededRent.StartDate__c);
        if(actualMopping.EndDate__c == neededRent.StartDate__c && actualMopping.Clients__c != String.valueOf(neededRent.Clients__c) && neededRent.Status__c != 'Canceled'){
            actualMopping.Clients__c = String.valueOf(neededRent.Clients__c);
            moppingsToUpdate.add(actualMopping);
        } else if(actualMopping.EndDate__c == neededRent.StartDate__c && neededRent.Status__c == 'Canceled' && !moppingsToUpdate.contains(actualMopping)){      
            actualMopping.Clients__c = '';
            //Sometimes one Rent__c can become 'Canceled' and another Rent__c with the same EndDate__c and Opportunity
            //can be not 'Canceled'. Then we get the same Mopping in our List:
            moppingsToUpdate.add(actualMopping);
        }
    }

Try to get the List<Rent__c> neededRents out of the for loop.

You can do something like this:

List<Rent__c> neededRents = new List<Rent__c>();
for(Mopping__c actualMopping: actualMoppings){
neededRents.add(rentsMap.get(actualMopping.Rent__r.Opportunity__c).Rents__r);
}

I think that you can also move the List<Date> listDate = new List<Date>(); out of the first loop.

UPDATE:

I see that you are only using the listDate for a final check.

if(!listDate.contains(actualMopping.EndDate__c)){
        actualMopping.Clients__c = '';
        moppingsToUpdate.add(actualMopping);
    }

The list contains the neededRent which it is the same record than the ones on the neededRents, correct?.

So you can get the listDate out the first loop.

List<Rent__c> neededRents = new List<Rent__c>();
List<Date> listDate = new List<Date>();
for(Mopping__c actualMopping: actualMoppings) {    
    neededRents.add(rentsMap.get(
    actualMopping.Rent__r.Opportunity__c).Rents__r);
    listDate.add(neededRent.StartDate__c);
  }

I think that you should separate your logic on the tigger and have one method for updating records and one for inserting. I think that you can explore using before insert and before update as well.

For the trigger (on updates) , and as I mentioned before, you can sent those Rent records that are changing the fields you need to check to avoid operating over to many Opportunities on your process.

trigger ClientsTrigger on Rent__c (after update, after insert) {
// Add a list only for update
List<Rent__c> allRentsUpdate = new List<Rent__c>();
List<Rent__c> allRentsInsert = new List<Rent__c>();

TriggerHelper helper = new TriggerHelper();

if(Trigger.isAfter && Trigger.isInsert) {
    // Here will come the logic only for insert...
    // you can include a method only for insert
    // helper.insertHelper(allRentsInsert);  
}

if(Trigger.isAfter && Trigger.isUpdate) {
    for(Rent__c rent : Trigger.new) {
        // You can add more logic here
        if(Trigger.oldMap.get(rent.Id).StartDate__c != rent.StartDate__c) {
        allRentsUpdate.add(rent);
        }
    }

    helper.updatingHelper(allRentsUpdate);
  }
}

This way you pass to your helper the Rent records you want to execute.

For the second for loop you can try something like this, I didn't test this because I don't have your objects but this is a way to iterate over child records:

for(Rent__c neededRent: neededRents){
  for(Mopping__c actualMopping : neededRent.Mopping__r) {
        if(actualMopping.EndDate__c == neededRent.StartDate__c && actualMopping.Clients__c != String.valueOf(neededRent.Clients__c) && neededRent.Status__c != 'Canceled'){
            actualMopping.Clients__c = String.valueOf(neededRent.Clients__c);
            moppingsToUpdate.add(actualMopping);
        } else if(actualMopping.EndDate__c == neededRent.StartDate__c && neededRent.Status__c == 'Canceled' && !moppingsToUpdate.contains(actualMopping)){      
            actualMopping.Clients__c = '';
            moppingsToUpdate.add(actualMopping);
            }
        }
    }

This type of for loop is fine because you are iterating from the parent to the child on a relationship.

Can you post the TriggerHander as well?.. This will help us to let you know how to build a singletone for this scenario. It is really smart to use this pattern when possible, it will make a great difference on the overall governor limits when sharing classes.

  • Hi. Thank you. There are important changes not only with Status__c and StartDate__c. As I said already, Rents can jump between Opportunities too, and that's make my task really crazy. And I still didn't get how can I move the List<Rent__c> neededRents out of the first loop. I still need to compare values from neededRents and actualMoppings. – Apelsinova Nov 9 '19 at 9:57
  • Check the updates on the first answer. – Carlos Naranjo Nov 9 '19 at 17:06

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.