Trigger : Need Help for showing a nicer error

Question

I have created a trigger for the following behavior in my Salesforce org: When the user changes the Account Owner, it automatically changes the Contract Owner on the related contracts before update. However if some mandatory fields are not filled in the contract page the user received a red system error and that's what Iwould like to enhance.

I have written that trigger, don't hesitate to correct it because I am biginning in sAlesforce Apex code and I need to learn! Thanks a lot

trigger TriggerUpdateContractOwnerFromAcount on Account (Before Update) {
    for(Account Acc :Trigger.new){

    List<Contract> C = [select id, ownerid, AccountId,Account_Number__c,Contact__c,Name,status,StartDate,EndDate,OwnerExpirationNotice,
    Contract_Type__c,Annual_Value__c,Annual_Volume__c,UOM__c,Estimate_of_annual_PBIT__c,Capital_Required__c,Budgeted__c,
    Other_Costs__c from Contract where Account.id = :Acc.Id ];

    for (Contract Ctr : C)
    if(Ctr.Ownerid != Acc.Ownerid & Ctr.EndDate !=null||Ctr.Account_Number__c!= null||Ctr.Contact__c!= null||Ctr.Name!= null||Ctr.status!=null||Ctr.StartDate!= null||Ctr.EndDate!=null||Ctr.OwnerExpirationNotice!=null||
    Ctr.Contract_Type__c!= null||Ctr.Annual_Value__c!=null||Ctr.Annual_Volume__c!=null||Ctr.UOM__c!= null||Ctr.Estimate_of_annual_PBIT__c!=null||Ctr.Capital_Required__c!= null||Ctr.Budgeted__c!=null||
    Ctr.Other_Costs__c!=null){
    //Acc.owner.Adderror('Please complete mandatory fields on Contract before Changing the Account Owner');
    Ctr.Ownerid= Acc.Ownerid;
    }
    update C;

    for (Contract Ctr : C)
    if(Ctr.Ownerid != Acc.Ownerid & Ctr.EndDate ==null||Ctr.Account_Number__c== null||Ctr.Contact__c== null||Ctr.Name==null||Ctr.status==null||Ctr.StartDate== null||Ctr.EndDate==null||Ctr.OwnerExpirationNotice==null||
    Ctr.Contract_Type__c==null||Ctr.Annual_Value__c==null||Ctr.Annual_Volume__c==null||Ctr.UOM__c== null||Ctr.Estimate_of_annual_PBIT__c==null||Ctr.Capital_Required__c== null||Ctr.Budgeted__c==null||
    Ctr.Other_Costs__c==null){

    //add error message if mandatory fields are empty in related contract
    Acc.owner.Adderror('Please complete mandatory fields on Contract before Changing the Account Owner');

    }
    }
}

And this is the error I get on the page "Select New Owner" for Transfer (=Link Change owner on account page) :

Review all error messages below to correct your data.

Error: Apex trigger TriggerUpdateContractOwnerFromAcount caused an unexpected exception, contact your administrator: TriggerUpdateContractOwnerFromAcount: execution of BeforeUpdate

caused by: System.DmlException: Update failed. First exception on row 0 with id 800O00000002Z01IAE; first error: FIELD_FILTER_VALIDATION_EXCEPTION, Value does not exist or does not match filter criteria.: [AccountId]: Trigger.TriggerUpdateContractOwnerFromAcount: line 15, column 1

Hope this make sense, many thanks.

Answer 1

If you are looking for a custom, more user friendly error, then you are doing the correct thing using the addError(...) method(s) of sObject.

However, you are not properly handling the update code, or adding the error. haagen's approach is close in that case. You need to attempt the update using the Database.update(...) method and store a reference to the results. I would suggest maintaining a collection of the error messages, concatenating them into one string, then using that string as the error message in the addError() call.

Additionally, you are likely to hit governor limits with your current implementation. You are querying the Contracts of each Account, which could lead to 200 queries made. The limit is 100, so we also need to redo a little of the logic for querying and updating the Contracts.

trigger TriggerUpdateContractOwnerFromAcount on Account (Before Update) {
    // only maintain a list of the Contracts after we have updated them
    List<Contract> contractsToUpdate = new List<Contract>();
    // bulkify your select by placing it in the collection field of the for loop, this way SFDC takes care of bulk querying
    for (Contract Ctr : [SELECT <YOUR FIELDS> FROM Contract WHERE AccountId IN :Trigger.new]) {
        // since you are in an update trigger, you have the newMap field on Trigger that lets you pick out the Account related to the Contract
        Account Acc = Trigger.newMap.get(Ctr.AccountId);
        if(<YOUR LOGIC>){
            Ctr.Ownerid= Acc.Ownerid;
            contractsToUpdate.add(Ctr);
        }
    }

    // using true because I am assuming you do not actually want a partially successful save.  use false if you do want some to succeed and others to fail.
    Database.SaveResult[] results = Database.update(contractsToUpdate, true);
    List<String> errorMessages = new List<String>();
    for (Database.SaveResult sr : results) {
        if (!sr.isSuccess()) {
            for (Database.Error e : sr.getErrors()) {
                errorMessages.add(e.getMessage());
            }
        }
    }

    if (errorMessages.size() > 0) {
        // only need to add the error to the first object for the error to end everything
        Trigger.new[0].addError(String.join(errorMessages, '\n'));
    }
}

Answer 2

You can use the database function to better control and get a result set back instead.

List<Database.SaveResult> results  = Database.update(C, false);

for (Database.SaveResult result : results) 
{
    if (!result.isSuccess()) {
        for (Database.Error err : result.getErrors())
        {
            System.Debug('Update Errors ' + err.getStatusCode() + ' ' + err.getMessage() + ' ' + err.getFields());
        }
    } else {
        System.Debug('Update Success: ' + result.getId());
    }               
}   

The second parameter controls if the lines that where successfully updated should be committed to the database or rollback on error. You can find the details in the APEX reference manual

Answer 3

I think haagen's approach is a good one if the idea is to let the error happen in the trigger and then modify the error message.

Personally, I would just live with users getting the direct red error message...

The error you are getting means that the owner that you are assigning to the Contract is not allowed according to the lookup filter defined on the Contract Owner field. So your trigger would also need to replicate that logic in some way too.

The structure of your trigger appears to be this:

trigger TriggerUpdateContractOwnerFromAcount on Account (Before Update) {
    for(Account Acc :Trigger.new){
        List<Contract> C = [select ... from Contract where Account.id = :Acc.Id];
        for (Contract Ctr : C)
            if(...){
               Ctr.Ownerid= Acc.Ownerid;
            }
        update C;
        for (Contract Ctr : C)
            if(...){
                //add error message if mandatory fields are empty in related contract
                Acc.owner.Adderror('Please complete mandatory fields...');
            }
    }
}

which I'm not sure makes too much logical sense. Also doing queries and updates inside loops like this is an anti-pattern that will cause governor limits to be exceeded when bulk changes are ever done to Account in the org. Code organised like this would avoid that:

trigger TriggerUpdateContractOwnerFromAcount on Account (Before Update) {
    Contract[] updates = new Contract[] {};
    for (Contract c : [
            select ..., Account.Id
            from Contract
            where Account.Id in :Trigger.newMap.keySet()
            ]) {
        Account a = Trigger.newMap.get(c.Account.Id);
        if (c.OwnerId != a.OwnerId) {
            if (...) {
                a.addError('Please ...');
            } else {
                updates.add(new Contract(Id = c.Id, OwnerId = a.OwnerId));
            }
        }
    }
    update updates;
}

A few other points:

• logical ands "&&" are usually what you need not bitwise ands "&"
• where you mix ands and ors brackets are usually a good idea to be clear about precedence
• using plural names for collections and singular names for individual objects makes code clearer
• code layout and indenting become really important when you have a lot of factors in expressions as you do (and it can be worth moving such logic out into separate classes)