Apex trigger addError() always shows error-msg

adderrorapextrigger

Good day, everyone.
I have started learning Apex. And need help with triggers. The problem is:
I have custom object Gift Card, which is not related to any other objects. On Opportunity I have custom text field Gift Card. While inserting name of gift card I need to check if this card is active, if not I should show some error. If card is active I need to substract amount of card from amount of Opportunity. But using this code I always see only error message, it doesn't matter if card is active or not.

trigger cardFromOpportunity on Opportunity (before insert, before update) {
    for(Opportunity opp : Trigger.new){
        for(Gift_Card__c card:[SELECT Name,Amount__c,Active__c FROM Gift_Card__c]){
            if(opp.Gift_Card__c == card.Name && card.Active__c == true){
                opp.Amount -= card.Amount__c;
                card.Active__c = false;
                update card;
            }else if(opp.Gift_Card__c == card.Name && card.Active__c == false){
                opp.Gift_Card__c.addError('Error');
            }
        }
    }
}

Best Answer

There are several malpractices within your code, this may not be the answer to the issue why you're seeing the add error every time, but still should be some improvement to avoid running into Dml/Soql limits (which you can find here).

trigger cardFromOpportunity on Opportunity (before insert, before update) {
    Set<String> gcNames = new Set<String>();
    
    for(Opportunity opp : Trigger.new){
        gcNames.add(opp.Gift_Card__c)
    }
    Map<String,Gift_Card__c> giftCards = new Map<String,Gift_Card__c> ();
    for (Gift_Card__c gift: [SELECT Name,Amount__c FROM Gift_Card__c WHERE Name in: gcNames]){
        giftCards.put(gift.Name,gift);
    }
    for(Opportunity opp : Trigger.new){     
        if(giftCards.containsKey(opp.Gift_Card__c)){
            Gift_Card__c card = giftCards.get(opp.Gift_Card__c);
            if(card.Active__c){
                opp.Amount -= giftCards.get(opp.Gift_Card__c).Amount__c;
                giftCards.get(opp.Gift_Card__c).Active__c = false;
            }
            else{
                opp.Gift_Card__c.addError('Error');
            }
        }  
    }
    update giftCards.values();
}

Just to clarify , this code isn't meant to be a guide to solve the functional requirements of yours , but just a quick help on how to think in terms of bulkification of code and making queries selective.

Also probably you shouldn't be storing the Name as a String on the obect GiftCard but rather create a Lookup relationship towards Opportunity Object, or a Master-Detail relationship if it fits requirements.

Related Solutions

[SalesForce] Trigger working like a validation rule

Both CustomActivity__c and StudyTeam__c are children of Study__c, so you first need to get all the Study__c Records that are related to your list of CustomActivity__c records. You could do this with a set or map, I generally prefer maps, but it's not totally necessary here as your not really doing anything with the Study__c object, but I use a map anyway

map<Id,Study__c> studyMap = new map<Id,Study__c>();
for(CustomActivity__c cAct : val){
    studyMap.put(cAct.Study__c, null);
}
studyMap.remove(null);
studyMap.putAll([Select Id, Name From Study Where Id In : studyMap.keyset()]);

So now you have a map of the Study__c objects that are parents to your CustomActivity records. The next thing you need to do is query the StudyTeam__c records that have any of your Study__c records as a parent.

list<StudyTeam__c> sTeamList = [Select Id, Name, Study__c, User__c From StudyTeam__c Where Study__c in : studyMap.keyset()];

So now you have a list of all the StudyTeam__c objects you need to check the User__c field against your new CustomActivity records.

In order to compare them, you need an elegant way to access a Study__c record, and all the associated StudyTeam__c User__c values, so I would use a map for this

map<Id,set<Id>> study2IdsMap = new map<Id,set<Id>>();
for(StudyTeam__c sTeam : sTeamList){
     if(!study2IdsMap.containsKey(sTeam.Study__c)){
          study2IdsMap.put(sTeam.Study__c, new set<Id>();
     }
     study2IdsMap.get(sTeam.Study__c).add(sTeam.User__c); 
}

Now you have a map that takes a Study__c object and returns a full set of all user IDs that are associated with a StudyTeam__c object. So now you just need to make sure the User__c field on your CustomActivity__c field is in that set, or else throw an error.

for(CustomActivity__c cAct : val){
     if(!study2IdsMap.get(cAct.Study__c).contains(cAct.User__c)){
          cAct.addError('YOU CANNOT DO THAT!!');
     }
}

So then you just tie all the pieces together

public class CustomActivityUserValidation {

    public static void validation(List<CustomActivity__c> val) {

         map<Id,Study__c> studyMap = new map<Id,Study__c>();
         for(CustomActivity__c cAct : val){
            studyMap.put(cAct.Study__c, null);
         }
         studyMap.remove(null);
         studyMap.putAll([Select Id, Name From Study Where Id In : studyMap.keyset()]);

         list<StudyTeam__c> sTeamList = [Select Id, Name, Study__c, User__c From StudyTeam__c Where Study__c in : studyMap.keyset()];

         map<Id,set<Id>> study2IdsMap = new map<Id,set<Id>>();
         for(StudyTeam__c sTeam : sTeamList){
            if(!study2IdsMap.containsKey(sTeam.Study__c)){
               study2IdsMap.put(sTeam.Study__c, new set<Id>();
            }
            study2IdsMap.get(sTeam.Study__c).add(sTeam.User__c); 
         }

         for(CustomActivity__c cAct : val){
            if(!study2IdsMap.get(cAct.Study__c).contains(cAct.User__c)){
                cAct.addError('YOU CANNOT DO THAT!!');
            }
         }

    }// End public static void

}// End class

DML error when migrating before insert trigger logic to apex class

In BeforeTrigger you can change the value of the Lead directly and do not perform DML, as it will be changed. So, in your method, just remove the updateList and the DML statement, and it will work.

Other issue is that the tmap will be empty so you won’t have the right values. You can do your map static (as is) and populate directly in your method instead of in constructor.

Best Answer

Related Solutions

[SalesForce] Trigger working like a validation rule

DML error when migrating before insert trigger logic to apex class

Related Topic