2

I'm trying to figure out how to structure my trigger correctly. I have a trigger that fires properly when the detail is deleted, however I cannot figure out how to write a trigger handling the deletion of the master of that detail. The standard cascade delete doesn't cause the trigger on the detail object to fire, so I need something at the master level to delete records that are related to the master's children.

The master object is Sales Plan and detail is Sales Plan Activity. Tasks are tied to Sales Plan Activity records by a text field because we need Contacts and Accounts to be linked via the WhoId and WhatId.

Will you please help me structure my code properly?

public class SalesPlanDeleteTask {

public static void deleteTask(List<SalesPlan__c> plans) {

    for(SalesPlan__c planIds : plans) {

        //Get Ids of Sales Plan records that are being deleted
        List<SalesPlan__c> plan = [SELECT Id FROM SalesPlan__c WHERE Id =:planIds.Id];

        //List Sales Plan Activity records that will be deleted by deleting the parent Sales Plan
        SalesPlanActivity__c activityList = [SELECT Id, Sales_Plan__c, TaskId__c FROM SalesPlanActivity__c WHERE Sales_Plan__c=:plan];

        String act = activityList.Id;

        //Query for the Tasks that are tied by the custom text Id field (SalesPlanActivityId__c) that connects Task to Sales Plan Activity
        List<Task> taskList = [SELECT Id, SalesPlanActivityId__c, OkToDelete__c FROM Task WHERE SalesPlanActivityId__c=:act];
        if(!taskList.isEmpty()) {
            if(taskList[0].SalesPlanActivityId__c != null) {
                taskList[0].OkToDelete__c=TRUE;
                update taskList;
                delete taskList;
            }
        }

    }

}

}
  • 3
    You never populate planIds. – Adrian Larson Oct 5 '15 at 20:20
  • 2
    Also List<SalesPlanActivity__c> has no property called Id. You would need to construct that set of Ids as well and use the IN operator. – Adrian Larson Oct 5 '15 at 20:21
  • I updated the code. This code compiles and works for a scenario with 1 task, but isn't bulkified to handle multiple tasks. How could it be bulkified? – Tyler Oct 5 '15 at 20:38
  • 1
    Move your queries and dml out of your loops. Will answer later but someone will probably beat me to it. – Adrian Larson Oct 5 '15 at 20:49
3

This code is not correct. Following corrections are required:-

  1. There should not be any SOQL within For loop.
  2. Also, you can look into minimizing the queries (first query is non-required, as you should already have list of Sales plan)
  3. Perform DML statements (update and delete) outside of FOR loop.

Once you modify the code as per above suggestions, it'll be able to handle any level of cascade delete and will also work in bulk scenarios.

Furthermore, I'll recommend you to look into trigger design patterns and make use of it to avoid such issues. I personally like and recommend http://developer.force.com/cookbook/recipe/trigger-pattern-for-tidy-streamlined-bulkified-triggers

  • 1
    That pattern seems massively over engineered, but the rest of your answer is quality. – Adrian Larson Oct 5 '15 at 21:07
  • 1
    @AdrianLarson : You may feel that at start, but I can assure from my experience that this pattern (from that aspect any design pattern) is very instrumental in segregating various trigger scenarios. Moreover, for enterprise systems it also helps in making code more manageable. – Anshul Oct 6 '15 at 4:07
  • 1
    It is not the use of pattern I take issue with, but the selection thereof. I tend to use a much lighter and more specific handler pattern. – Adrian Larson Oct 6 '15 at 4:08
  • 1
    @AdrianLarson : :) got your point. Well, that i'll say as matter of preference. May be i've used the other one for way too long now :) – Anshul Oct 6 '15 at 4:10

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.