Error in Apex Trigger adding trigger.isUpdate

Question

With the help of others I wrote an apex trigger that places the last completed task's subject into a field called Last_Activity_Subject__c for both opportunities and leads.

The trigger worked fine when before I added the trigger.IsUpdate in the first if statement, but it did not change the Last_Activity_Subject__c field if a future task was marked as completed.

The problem now is Salesforce does not allow leads to be converted to opportunities and gives this error Error:

System.DmlException: Update failed. First exception on row 0 with id 00TJ000000ROVfGMAX; first error: CANNOT_INSERT_UPDATE_ACTIVATE_ENTITY, updateRelatedLeadOrOpportunity: execution of AfterUpdate caused by: System.ListException: Duplicate id in list: 006J000000FriCoIAJ Trigger.updateRelatedLeadOrOpportunity: line 67, column 1: [] (System Code).

Is there a way to keep the functionality of the code that I have now while also letting me convert leads to opportunities?

Here is my trigger

Trigger updateRelatedLeadOrOpportunity on Task (after insert,after update) {
List<Id> OpportunityIds = new List<Id>();
List<Opportunity> OpportunityList = new List<Opportunity>();
List<Id> LeadIds = new List<Id>();
List<Lead> LeadList = new List<Lead>();
List<Task> taskToProcess = new List<Task>();

for(Task t :trigger.new)
    {
    //This will make sure that we only process the task who's subject is added/updated
    if(t.status == 'Completed'&& ((trigger.isInsert | trigger.isUpdate) || trigger.newMap.get(t.id).subject != trigger.oldMap.get(t.id).subject)){
        //You'll have WhoId populated when the Task related to a Lead/Contact(only) 
        //and whatId will be populated if the task is realted to Account/Opportunity/Custom Objects...
        if(t.whatId !=null)
        {
            Schema.SObjectType tType= t.whatid.getSObjectType();
            if(tType == Opportunity.Schema.SObjectType)
            {
                OpportunityIds.add(t.Whatid);
                taskToProcess.add(t);
            }

        }

        if(t.whoId != null){

            Schema.SObjectType tType= t.whoId.getSObjectType();
            if(tType == Lead.Schema.SObjectType)
            {
                LeadIds.add(t.whoId);
                taskToProcess.add(t);
            }
        }


    }   
}

//Querying the related Opportunity as well as Lead based on whatid on Task
Map<Id,Opportunity> OpportunityMap =  new Map<Id,Opportunity>([select id,Last_Activity_Subject__C from Opportunity where id in:OpportunityIds]);
Map<Id,Lead> LeadMap =  new Map<Id,Lead>([select id,Last_Activity_Subject__C from Lead where id in:LeadIds]);
//Now we have all the tasks that we need to process in this loop
for(Task t :taskToProcess){
    if(t.whatid!=null)
    {
        Schema.SObjectType tType= t.whatId.getSObjectType();
        if(tType == Opportunity.Schema.SObjectType){
            Opportunity opp = OpportunityMap.get(t.whatId);
            opp.Last_Activity_Subject__C = t.subject;
            OpportunityList.add(opp);
        }


    }

    if(t.whoId!=null){
        Schema.SObjectType tType= t.whoId.getSObjectType();
        if(tType == Lead.Schema.SObjectType){
            Lead lead = LeadMap.get(t.whoId);
            lead.Last_Activity_Subject__C = t.subject;
            LeadList.add(lead);
        }

    }
}
// updating the Opportunity and lead
if(!OpportunityList.isEmpty()) update OpportunityList;
if(!LeadList.isEmpty()) update LeadList;

}

And Test Class

@istest public class TestupdateRelatedLeadOrOpportunity{

private Static testmethod void TestLastActivitUpdateSubject(){

    //You'll need to populate data into all other required fields and with respect to any validation rules if you have any.
    Lead l = new Lead(Firstname = 'TestFName', Company = 'TestCompany', Website = 'TestWebsite.com', Origin_Date__C = Date.newInstance(2014,12,31), Origin__c = 'Other', Status = 'Created', Lastname = 'TestLName');
    insert l;

    //You'll need to populate data into all other required fields and with respect to any validation rules if you have any.
    Account a = new Account(Name = 'Test Account');
    insert a;
    Opportunity o = new Opportunity(Name = 'Test Oppotunity', Website__c = 'TestWebsite.com', StageName = 'Prospecting', CloseDate = Date.newInstance(2014,12,31), AccountId = a.id);
    insert o;

    //These tasks will be related to Lead & Opportunity hence it will cause the trigger to execute and upadte the task Subject to Last_Activity_Subject__C field
    List<task> t = new List<task>{ new task(WhoID = l.id, Subject='Call', Status='Completed', Priority='Normal'), new task(WhatID = o.id, Subject='Email', Status='Completed', Priority='Normal')};
    insert t;
    Lead lead = [select Last_Activity_Subject__C from Lead where id = :l.id];
    Opportunity opp = [select id,Last_Activity_Subject__C from Opportunity where id = :o.id];
    System.assertEquals('Call', lead.Last_Activity_Subject__C);
    System.assertEquals('Email', opp.Last_Activity_Subject__C);
}

}

Answer 1

The issue is that since your trigger runs in bulk, there may be cases where there is more than one Task in Trigger.new that is connected to the same Opportunity. When you are getting ready to update the Opportunities, you have to make sure that only one copy of each Opportunity gets into the final list for update. There are several ways to do that, but I would use a map to collect those updated Opportunity records in a deduplicated way:

map<Id, Opportunity> oppsToUpdateMap = new map<Id, Opportunity>();

...
    //OpportunityList.add(opp);
    oppsToUpdateMap.put(opp.Id, opp); //this will overwrite if this opp is already in the map

...
//if(!OpportunityList.isEmpty()) update OpportunityList;
update oppsToUpdateMap.values(); //BTW, there is no need to check if the list is empty

The only concern here is that your Opportunity field Last_Activity_Subject__c will only retain the value from the last Task in your taskToProcess list. If you wanted to use some different logic to determine which Task to use, that would make it a little more complicated.