1

i have the below scenario. One account is having many contacts. contact_status__c is a checkbox field on contact. i am trying to write a trigger which will check at least 1 contact should have this checkbox checked. if that account has one contact and if user makes the field uncheck, it should throw an error

Below code is not working.

'You are trying to remove last contact status checkbox which is not allowed'

trigger -

trigger checkaccount on contact(before insert, before update){

    List<Id> accID = new List<Id>();
    for(contact c : trigger.new){
        if(c.accountId != null){
            accID.add(c.accountId);
        }
    }

    List<account> acc = [select id, (select id, contact_status__c from contacts where contact_status__c = true) from Account WHERE Id In: accID];
    system.debug('@@@@acc ' + acc);
    system.debug('@@@@acc ' + acc[0].contacts);
    Map<id, boolean> bool = new map<id,boolean>();
    for(Contact c : trigger.new){
        if(acc.size() == 0 ) {
            c.addError('You are trying to remove last contact status checkbox which is not allowed');
        }
    }

}

3 Answers 3

3

You've got a lot of problems with this code, mostly due to a lack of bulkification (meaning, the code should work right with multiple contacts). You need to use after-insert/after-update instead of before-insert/before-update, otherwise your query won't return the correct values. Further, you need to determine which account matches the contact. Here's a more approximate version you're looking for:

trigger checkaccount on contact (after insert, after update) {
  Set<Id> accountIds = new Set<Id>();
  for(Contact record: Trigger.new) {
    accountIds.add(record.AccountId);
  }
  accountIds.retainAll(new Map<Id, AggregateResult>([
    SELECT AccountId Id
    FROM Contact
    WHERE AccountId = :accountIds AND contact_status__c = true
    GROUP BY AccountId
  ]).keySet());
  for(Contact record: Trigger.new) {
    if(record.AccountId != null && !accountIds.contains(record.AccountId)) {
      record.addError('You are trying to remove last contact status checkbox which is not allowed');
    }
  }
}
2
  • I've never seen retainAll nor Map<Id, AggregateResult>.keySet() used this way. Great use of both and I learned something new today :-) Feb 23, 2020 at 19:13
  • 1
    @BrianMiller Glad I could show you something new 😊 There's a lot of cool ways you can mix these methods if you get a bit imaginative.
    – sfdcfox
    Feb 24, 2020 at 4:31
4

Your logic simply doesn't do what you're aiming to do.

List<account> acc = [select id, (select id, contact_status__c from contacts where contact_status__c = true) from Account WHERE Id In: accID];

Here, you query the Accounts of all Contacts in the trigger set.

for(Contact c : trigger.new){
    if(acc.size() == 0 ) {
        c.addError('You are trying to remove last contact status checkbox which is not allowed');
    }

But here, you're just asking, with acc.size(), "Do any of the Contacts in my trigger set have an Account?"

That's not what you want. You seem to want to ask "Does the Account of this Contact have any remaining Contacts, other than this one, that have contact_status__c set to true?"

To do that effectively, you'll need to make some changes. First, you'll want to put your Accounts in a Map, so that you can access them via the Contact's AccountId field. Here you'll want to use the fact that a Map<Id, sObject> has a constructor to which you can directly pass a List<sObject>: Map<Id, Account> accountMap = new Map<Id, Account>([QUERY HERE]).

Then, for each Contact, you'll want to access its related Account's list of Contacts, with accountMap.get(c.AccountId).Contacts. But remember to write logic first to check if the Contact has an Account, otherwise you'll get a NullPointerException.

That gets you part of the way there, but there's some additional issues you'll want to consider. Most importantly, your trigger should only react to changes to the Contact_Status__c field by adding this error - not to unrelated changes to the Contact. So before you check anything for each Contact, you'll want to check if that field has in fact changed from true to false, by looking at Trigger.oldMap.

Then there's one more consideration. You're running this trigger before insert, which doesn't really make sense since you cannot change the field from one value to another during an insert event, and before update. In before update context, your query will return the current value of that field for records in the trigger set. You probably want to run this trigger after update so that your query results reflect the change being made, and you get back an accurate count of Contacts with the field set.

0

To do this using before update is not correct as result of soql will be before the updation and it will show error(You Error) even if you are making first contact field contact_status__c to true.

Clearly as mentioned above, you are getting account size with acc.size() which is wrong. Instead of this you can use acc.contacts.size() but again for that you have to use loop inside loop.

BTW I've made some modification in your code as below:

trigger checkaccount on contact(after insert, after update){
    Set<Id> accID = new Set<Id>();
    for(contact c : trigger.new){
        if(c.accountId != null){
            accID.add(c.accountId);
        }
    }

    List<Contact> con= [select id, contact_status__c from contact where contact_status__c = true AND AccountId In: accID];
    for(Contact c : trigger.new){
        if(con.size() == 0 ) {
            c.addError('You are trying to remove last contact status checkbox which is not allowed');
        }
    }
}

You can further optimize this code by adding trigger.old and trigger.new comparison to confirm that particular field is changed not some other fields

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .