0

I have a custom checkbox on Contact and I am implementing something like: if the contact is edited AND the checkbox (api name active __c) is not checked then it should be deleted.

I have written below trigger but its not working and most irritating part is its not giving any error also

 if(trigger.isUpdate){
        for(Contact con : trigger.new){

            lstAccId.add(con.AccountId);
        }

        for(Account acc : [select id, Count_of_Active_Contacts__c, Count_of_Inactive_Contacts__c from account where id in : lstAccId]){


            for(contact con : trigger.new){
                Contact contact  = new Contact(Id = con.Id);
                if(contact.AccountId == acc.id){
                    if(contact.active__c == FALSE){
                         lstcontact.add( contact );
                        system.debug('---------------contact------------->'+contact);
                    }
                }
            }
        }
delete lstcontact;
        }

please help

7
  • @Ashwani It certainly can work (I've personally done this). However, this trigger still looks odd, so I think there's something deeper going on here.
    – sfdcfox
    Aug 20, 2015 at 12:55
  • @ashwani - it will work because i am using this "Contact contact = new Contact(Id = con.Id);" if i would not have used this . then it must have given error Aug 20, 2015 at 12:56
  • @sfdcfox - could you suggest me how u have done this? Aug 20, 2015 at 12:59
  • 1
    Yes, if runs on after update not before update. It looks odd as the approach of query contacts and filtering contacts may cause governor limit issues in future. And I don't like the delete :) @sfdcfox
    – Ashwani
    Aug 20, 2015 at 13:02
  • could you suggest me what might be the issue? Aug 20, 2015 at 13:03

2 Answers 2

4

You're creating a new contact, then comparing that new contact's account ID (which is always null), to a value in the query, which is always not-null.

Here's a version that would work:

if(Trigger.isUpdate) {
    Contact[] deletes = new Contact[0];
    for(Contact record: Trigger.new) {
        if(!record.Active__c) {
             deletes.add(new Contact(Id=record.Id));
        }
    }
    delete deletes;
}

There's no need to query the accounts at all, unless you were doing something else with that data. In that case, you can just query the accounts like this:

Map<Id, Account> accounts = new Map<Id, Account>(
    [SELECT Id, Count_of_Active_Contacts__c, Count_of_Inactive_Contacts__c FROM Account WHERE Id IN (SELECT AccountId FROM Contact WHERE Id IN :Trigger.new)]);

You can get the account details directly from the map in this case:

for(Contact record: Trigger.new) {
    Account relatedAccount = accounts.get(record.AccountId);
}
3
  • Thank you !!its works like charm ..but could you let me know if i can make my code work or what was issue in my code..just for future reference Aug 20, 2015 at 13:08
  • 2
    @Ashwani That sort of criticism really shouldn't be in public commentary like this. We're supposed to be friendly people here.
    – sfdcfox
    Aug 20, 2015 at 13:21
  • 3
    @AnkitaSingh As I've already stated (as well as Doug), you're comparing a NULL value to a NON-NULL value, which will never be true, and thus will never work. You'd observe this effect if you ran a unit test across your trigger (there would be no code coverage from if(contact.active__c == false) on).
    – sfdcfox
    Aug 20, 2015 at 13:25
2

In your code you have this

Contact contact  = new Contact(Id = con.Id);
if(contact.AccountId == acc.id){

You are not setting a value for contact.accountId so it will never match and go into the if part

1
  • how can i set the id there? Aug 20, 2015 at 13:07

You must log in to answer this question.

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