-1

My question is: given I have an Opportunity, and there are no open mandatory sales tasks associated to it, when I try and change the opportunity stage then this shall be allowed by the system.

Given I have an aviation Opportunity, and there are open mandatory.

I tried this one and i don't know weather it is right or wrong. so, please help me for this scenario.

trigger Opportunity_Task on Opportunity (before update) {
    List<Task> ts =new List<Task> ();
    List<Opportunity> op =new List<Opportunity>();
    Map<Id,Opportunity> oldMap = trigger.oldMap;
    Map<Id,Opportunity> newMap = trigger.newMap;
    List<id> optyid = new List<id>();
    for(id opid:oldMap.keySet()) {
        Opportunity op =new Opportunity();
        if(oldMap.get(opid).tasks == null && op.stageName=='Closed Won') {
            op.addError('You can create a new task for that Opportunity');
            task t = new task();
            t.WhatId=opid;
            t.Description='kjdkfhsdkf';
            t.Status='open';
            ts.add(t);
        }
        else{
            if(oldMap.get(opid).tasks!=null && op.StageName=='Closed Won')
            op.addError('You cannot modify the opportunity status');

        }
    }
  • 2
    Your question seems to be incomplete. More specifically, you never fully describe your second scenario. Please edit your question to make your description of your problem complete. About the only thing that I can tell you right now is that your trigger won't work as you intend it to because oldMap.get(opid).tasks will be either null or an empty list. Data on parent (or child) records are not available in trigger context variables and must be explicitly queried. – Derek F Jun 11 '18 at 14:45
  • 2
    Your Opportunity related lists aren't populated in trigger context. You'd perform a query to locate them. But your logic also doesn't make any sense - with addError(), you're actually just blocking all Opportunity updates from being committed. – David Reed Jun 11 '18 at 14:45
6

What you're saying and what your code is saying is pretty different:

What you say you want

If there are open tasks, do not allow editing of the opportunity

Trigger OpportunityTrigger (before update) {
    // Since triggers do not have access to child records you'll have to query them
    // I like doing it as a subequery as it saves you from needing to map them out
    // yourself. Make sure to only include the tasks that you DO NOT
    // want to see.
    List<Opportunity> oppsWithTasks = new List<Opportunity>([
        SELECT Id, (
            SELECT Id
            FROM Tasks
            WHERE Status != 'Complete'
        )
        FROM Opportunity
        WHERE Id IN :Trigger.New
    ]);

    // Loop the the opps
    for (Opportunity opp : oppsWithTasks) {
        // If the query returns any tasks, you know this is an opp that should
        // not be changed.
        if (!opp.tasks.IsEmpty()) {
            // Add the error to the trigger object, not the object you queried
            // If you use opp.addError, nothing will happen.
            Trigger.NewMap.get(opp.Id).addError('Opp has open tasks');
        }
    }
}

What your code says you want

When an opp is closed, automatically create a task

List<Task> tasksToInsert = new List<Task>();

for (Opportunity opp : trigger.New) {
    // Make sure that the status is correct
    // Make sure that the update only occurs when the status is changed
    // Otherwise, you'll end up with too many tasks
    if (
        opp.StageName == 'Closed Won' && 
        opp.StageName != trigger.oldmap.get(opp.Id).StageName
    ) {
        // I prefer to initialize sobjects like this as it uses fewer
        // characters in code (and there is a character limit in apex)
        tasksToInsert.add(new Task(
            WhatId = opp.Id,
            Description = '',
            Status = ''
        ));
    }
}

// Check to see if there are any tasks before calling insert
if (!tasksToInsert.isEmpty()) {
    insert tasksToInsert;
}

Put together:

Trigger OpportunityTrigger (before update) {
    List<Task> tasksToInsert = new List<Task>();
    List<Opportunity> oppsWithTasks = new List<Opportunity>([
        SELECT Id, (
            SELECT Id
            FROM Tasks
            WHERE Status != 'Complete'
        )
        FROM Opportunity
        WHERE Id IN :Trigger.New
    ]);

    for (Opportunity opp : oppsWithTasks) {
        if (!opp.tasks.IsEmpty()) {
            Trigger.NewMap.get(opp.Id).addError('Opp has open tasks');
        }
    }

    for (Opportunity opp : trigger.New) {
        if (
            opp.StageName == 'Closed Won' &&
            opp.StageName != trigger.oldmap.get(opp.Id).StageName
        ) {
            tasksToInsert.add(new Task(
                WhatId = opp.Id,
                Description = '',
                Status = ''
            ));
        }
    }

    if (!tasksToInsert.isEmpty()) {
        insert tasksToInsert;
    }
}

Extra Tips (I'm bored)

It's best practice to create a utility class to hold most of the functionality of the trigger.

public class OpportunityUtils {
    public static void validateMandatoryTasksClosed(Map<Id, Opportunity> updatedOpps) {
        List<Opportunity> oppsWithTasks = new List<Opportunity>([
            SELECT Id, (
                SELECT Id
                FROM Tasks
                WHERE Status != 'Complete'
            )
            FROM Opportunity
            WHERE Id IN :updatedOpps.keySet()
        ]);

        for (Opportunity opp : oppsWithTasks) {
            if (!opp.tasks.IsEmpty()) {
                updatedOpps.get(opp.Id).addError('Opp has open tasks');
            }
        }
    }

    public static void autoCreateFollowUpTask(List<Opportunity> newOpps, Map<Id, Opportunity> oldOpps) {
        List<Task> tasksToInsert = new List<Task>();

        for (Opportunity newOpp : newOpps) {
            // Expanded to support insert triggers
            // Using oldOpps == null tells you if it's insert or not
            // This means that if an opp is inserted as a closed won the
            // task still gets created
            if (
                newOpp.StageName == 'Closed Won' && (
                    oldOpps == null ||
                    newOpp.StageName != oldOpps.get(newOpp.Id).StageName
                )
            ) {
                tasksToInsert.add(new Task(
                    WhatId = opp.Id,
                    Description = '',
                    Status = ''
                ));
            }
        }

        if (!tasksToInsert.isEmpty()) {
            insert tasksToInsert;
        }
    }
}

Then your trigger looks like this:

// Added a support for after update/after insert
Trigger OpportunityTrigger (before update, after update, after insert) {
    if (Trigger.IsBefore && Trigger.isUpdate) {
        OpportunityUtils.validateMandatoryTasksClosed(Trigger.newMap());
    } else if (Trigger.isAfter) {
        // Handle creating tasks in after trigger, not before trigger
        OpportunityUtils.autoCreateFollowUpTask(Trigger.New, Trigger.OldMap);
    }
}

New Switch feature for APEX

Reference

Trigger OpportunityTrigger (before update, after update, after insert) {
    switch on triggerEvent {
        when BEFORE_UPDATE {
            OpportunityUtils.validateMandatoryTasksClosed(Trigger.newMap());
        }

        when AFTER_INSERT, AFTER_UPDATE {
            OpportunityUtils.autoCreateFollowUpTask(Trigger.New, Trigger.OldMap);
        }
    }
}
  • 2
    Ah, good ole' boredom tips. One thing to note is that with the release of Summer '18, the new switch statement + the new Trigger.operationType context variable can quite nicely replace the old if/else checks using things like Trigger.isBefore and Trigger.isUpdate. developer.salesforce.com/blogs/2018/05/… – Derek F Jun 11 '18 at 16:14
  • Updated the answer to include a switch example. I will note that I did not see just an "AFTER" enumeration. I also don't feel this is any more/less readable. I do however, like that triggerEvent allows for passing a single parameter to a helper method which could help curve tricks like my oldMap == null that I use specifically to avoid passing in the trigger.isX to my methods. The more I look at it the more I like the old way, more control (for now). – gNerb Jun 11 '18 at 16:22

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.