0

on the Account Object i need a validation rule which need to prevent Accounts from being set to 'Dead' if there are Orders associated which have a Status <> Invoiced.

Account field to be checked = Dead_Live__c

Order field to be checked = Status

Account and Order have a Lookup relationship.

I'm new to apex but i know the only way to do this validation is using apex.

I know the code I've written is wrong but if you could help me with some guidelines pls:

trigger PreventLocationsDead on Account (before insert, before update) {

    List <Order> ord = [select id, AccountId, Status from Order WHERE Status = 'Invoiced'];
    for(Account acc:trigger.new){
    if(acc.Enqix_Dead_Live__c == 'Dead' && ord.size() > 1){
       acc.adderror('Location cannot be set to Dead if there are Orders associated with the Location with Status <> Invoiced');
    }
 }
}
2

Lets break it down into Algorithm to do so.

  1. Try to find out if Account status has changed to "Dead". You dont wanna do a query first as it might be useless if the status hasn't changed
  2. If there exist the Accounts that are "Dead" querry the Orders with status invoiced.
  3. If there does exist an order with Status ="Invoiced" perform add error

    trigger PreventLocationsDead on Account (before update) {
    
    List<Account> listOfAccountsWithStatusChanged = new List<Account>();
    for(Account acc: Trigger.new){
        if(acc.Enqix_Dead_Live__c != Trigger.oldMap.get(acc.Id).Enqix_Dead_Live__c  && acc.Enqix_Dead_Live__c=='Dead'){
            listOfAccountsWithStatusChanged.add(acc);
        }
    }
    
    if(!listOfAccountsWithStatusChanged.isEmpty()){
        List<AggregateResult> arList = [SELECT AccountId,count(Id)  from Order
                    WHERE Status = 'Invoiced' AND AccountId in : listOfAccountsWithStatusChanged group by AccountId ];
    
        for(AggregateResult ar : arList){
            if(ar.get('expr0')!=0){
                Trigger.newMap.get(ar.get('AccountId')).adderror(Location cannot be set to Dead if there are Orders associated with the Location with Status <> Invoiced);
            }
        }       
    
    }
    
    
    }
    
  • +1 although if(ar.get('expr0')!=0){ isn't necessary; aggregateresult queries won't return a row for which there are no matching records. – sfdcfox Oct 22 '18 at 17:40
  • Hello Pranay, first of all thank you for the quick response, im getting error on line 5 saying that variable acc doesn't exist , and on line 16 saying that 'Method does not exist or incorrect signature: void get(Object) from the type Map<Id,Account>' – Nuno Carvalho Oct 22 '18 at 19:15
  • @Pranay Jaiswal – Nuno Carvalho Oct 22 '18 at 19:43
  • @NunoCarvalho i did a typo. fixed it – Pranay Jaiswal Oct 23 '18 at 12:43
2

You're actually really close, good job!

First, since no new accounts will have orders, I have removed before Insert. Then, I queried for all the accounts involved, and retrieved all invoiced orders they could possibly have.

Finally, inside the loop, I get the orders from each account (do a quick check to ensure that the account was retrieved in the query, just in case) and then use that list to evaluate. Also, keep in mind that you want to compare to 0, not 1, as if there is only 1 invoice then the validation won't work as you want.

trigger PreventLocationsDead on Account (before update) {

    Map<Id, Account> currentAcc = new Map<Id, Account>(
        [select Id, (select Id from Orders where Status = 'Invoiced') from Account where Id = :trigger.newMap().keySet];
    );


    for(Account acc:trigger.new){
        List<Order> ord = new List<Order>();

        if (currentAcc.containsKey(acc.id)) {
            List<Order> ord = currentAcc.get(acc.id).Orders;
        }

       if(acc.Enqix_Dead_Live__c == 'Dead' && ord.size() >= 0){
          acc.adderror('Location cannot be set to Dead if there are Orders associated with the Location with Status <> Invoiced');
       }
    }
}

Please note that this is a quick example. I would recommend you isolate all the logic in a helper class and out of the trigger itself.

== UPDATE ==

As sfdcfox points out, Aggregate queries are better for a number of reasons. Here is an example using Agg queries instead. Prefer this, if you choose my answer.

trigger PreventLocationsDead on Account (before update) {

    Map<Id, Decimal> ordersByAcc = new Map<Id, Decimal>();
    for (AggregateResult agg : [select AccountId, Count(Id) NrRecords from Order where Status = 'Invoiced' group by AccountId]) {
        currentAcc.put ((Id) agg.get('AccountId'), (Decimal) agg.get('NrRecords'));
    }


    for(Account acc:trigger.new){
        Decimal ord = 0;

        if (ordersByAcc.containsKey(acc.id)) {
            ord = ordersByAcc.get(acc.id);
        }

       if(acc.Enqix_Dead_Live__c == 'Dead' && ord >= 0){
          acc.adderror('Location cannot be set to Dead if there are Orders associated with the Location with Status <> Invoiced');
       }
    }
}
  • You should use an AggregateResult instead. – sfdcfox Oct 22 '18 at 17:03
  • Not a bad idea. Would perform and scale better though with only 200 records in the trigger context the advantage is reduced. I'll contrive a quick example in a few. – Sebastian Kessel Oct 22 '18 at 17:04
  • 2
    No, there's a legitimate concern--if there are more than 1000 rows returned, you would get a runtime error on size(). – sfdcfox Oct 22 '18 at 17:06
  • Better, but this still allows setting it to dead if exactly one record matches. I realize this is a logic bug in the original code, but you should point that out. As it is, if there is exactly one record that is not invoiced, the error will not trigger. – sfdcfox Oct 22 '18 at 17:42
  • I frankly wasn't paying attention to the logic that closely. Was more concerned about teaching the OP how to separate the query. Change made. – Sebastian Kessel Oct 22 '18 at 18:17

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.