0

I'm trying to prevent the deletion of accounts if one of the contacts within the account has a "Do Not Delete" checkbox as true. Here's a trigger (My very first one!)

Please let me know if this is even possible. it isn't throwing any error when I delete an account with the contact DnD checked

trigger prevdeletion on Account (before delete)
     {for (Account a:trigger.old){
    ID accountId;
    List<Contact> contacts = new List<Contact>([SELECT Id, Name,Do_Not_Delete__c FROM Contact WHERE AccountId = :accountId]);
        for(Contact c:Contacts){
            if(c.Do_Not_Delete__c=Boolean.valueOf('true') ){
                a.adderror('Account cannot be deleted because child contact is protected.');
            }
         }
     }
}
  • You don't seem to have initialized accountId with the Account's Id. You need something as accountId = a.Id and then utilize that in the SOQL. – Jayant Das Jun 6 at 21:15
  • Looks like you've accidentally created duplicate accounts. Please use the Contact link at the bottom of every page to request that they be merged. – David Reed Jun 7 at 11:53
2

You seem to have not initialized the accountId which is used in your SOQL as below to fetch the Contacts and thus you are not getting any results.

ID accountId; // this is not assigned to the intended account's id
List<Contact> contacts = 
    new List<Contact>(
       [SELECT Id, Name,Do_Not_Delete__c 
        FROM Contact 
        WHERE AccountId = :accountId]); // issue here

Then, as David as highlighted you are using = which won't compare the value but assign the result of Boolean.valueOf('true') to the field's value. You will need to use == for comparison without the need of using Boolean.valueOf as long as the field is a Boolean field.

While you can fix the issue by assigning accountId = a.Id and addressing other issues of querying all Contacts and then iterating through those to find the field's value, a better approach here would be to utilize Aggregate Function.

Your overall code could look as:

AggregateResult[] contactCounts = 
    [SELECT AccountId, COUNT(Id) 
     FROM Contact 
     WHERE Do_Not_Delete__c = TRUE 
         AND AccountId IN :Trigger.oldMap.keySet() 
     GROUP BY AccountId];

// now put all account ids and their respective contact counts in a map
Map<Id, Integer> accountContactCount = new Map<Id, Integer>();
for (AggregateResult ar : contactCounts)  {
    accountContactCount.put(ar.get('AccountId'), ar.get('expr0'));
}

// now iterate through the trigger records and verify if contact exists, then add error
for (Account a : trigger.old) {
    if(accountContactCount.get(a.Id) > 0) {
        a.adderror('Account cannot be deleted because child contact is protected.');
    }
}
1

It's possible, but you have a couple of issues with your trigger.

Apex Issues

ID accountId;

This variable is uninitialized, so it defaults to null.

List<Contact> contacts = new List<Contact>([SELECT Id, Name,Do_Not_Delete__c FROM Contact WHERE AccountId = :accountId]);

Then, this query is locating Contacts whose Account is ... null. That's not what you want here. Instead, use :a.Id, but see below for more on bulkification.

if(c.Do_Not_Delete__c=Boolean.valueOf('true') ){
  1. if statements evaluate Booleans already, so you just need to evaluate the Boolean variable c.Do_Not_Delete__c:

    if(c.Do_Not_Delete__c){
    
  2. You can write literal Booleans in Apex: true and false. No need for Boolean.valueOf().

  3. You've accidentally written an assignment statement with a single equals sign, =. == is what you need for comparison.

Bulkification Issues

As written, your code runs one query per Account. That will throw a limit exception if 100 Accounts are deleted. I strongly recommend completing the Bulk Apex Triggers module on Trailhead to learn bulkification patterns.

Here, luckily, the fix is pretty easy. Just do a parent-child subquery:

 for (Account a : [SELECT Id, (SELECT Id, Name,Do_Not_Delete__c FROM Contacts) FROM Account WHERE Id IN :Trigger.oldMap.keySet()]) {
    for(Contact c: a.Contacts) {

Then your logic goes in the inner for loop, but you only run a single query for the entire trigger set.

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.