trigger CloseDate on Opportunity (after insert, after update) {

    Map<Id, Opportunity> CloseDate = new Map<Id, Opportunity>();

    for (Opportunity O: Trigger.new)

    if(Trigger.isInsert || Trigger.isUpdate) {

    List<OpportunityLineItem> opportunityLineItemList=new list<OpportunityLineItem>(); // Collecting all OpportunityLineItems 

    opportunityLineItemList=[select id,Close_Date__c,Opportunityid from OpportunityLineItem where Opportunityid in :CloseDate.keyset()]; 
    // SQL query to extract OpportunityLineItems whose Id matches the Id in the CloseDate map and placing them into a list(Collection) 

    if(opportunityLineItemList.size()>0){  // IfCondition= if OLIL(Collection) size is more 0

    for(integer j=0;j<opportunityLineItemList.size();j++){ // Loop = if j value less than OLIL(collection) size 
  // OLIL  array index record close_date__c is set to Opportunity's CloseDate.


        update opportunityLineItemList;


Error Error: Compile Error: Illegal assignment from Opportunity to Date at line 17 column 30

  • there are various issues with this code. I don't see any value in your CloseDate map. it is blank. second your CloseDate map has Opporutnity as value and you are assigning that value to date field at line 17 – Himanshu Apr 27 '16 at 17:50
  • thank you Himanshu.... I made a mistake by placing opportunity as value in the map. – jay Apr 27 '16 at 17:56
  • @Himanshu....I have changed that to date value, the code compiles and saves but does not trigger and update the Opportunity closedate on opportunity line item close date. – jay Apr 27 '16 at 18:08
  • your code still not correct. replace closeDate.keyset() to trigger.newmap.keyset() – Himanshu Apr 27 '16 at 18:11
  • and as adrian mentioned you can achieve this functionality using process builder and update close date without writing single line of code – Himanshu Apr 27 '16 at 18:12

At the most basic level, your problem lies here:


This line can be broken down as follows:

Opportunity record = CloseDate.get(opportunityLineItemList[j].Opportunityid);
opportunityLineItemList[j].Close_Date__c = record;

However, you need a Date value to assign, not an Opportunity value. As mentioned in the comments, you never populate your CloseDate map, so the record will be null. It seems safe to assume you really just mean to use trigger.new. In that case, the above two lines should be changed to:

Date closeDate = trigger.newMap.get(opportunityLineItemList[j].Opportunityid).CloseDate;
opportunityLineItemList[j].Close_Date__c = closeDate;

Other issues with your code are numerous.

Never query inside a loop! No DML inside a loop!

You can query for child OpportunityLineItem records outside your loop structure. Additionally, you do not need two separate assignments to it.

Instead of:

for (/*opportunity loop*/)
    if (/*trigger operations*/)
        List<OpportunityLineItem> olis = new List<OpportunityLineItems>();
        olis = [/*query*/];
        update olis;

Just do:

if (/*trigger operations*/)
    List<OpportunityLineItem> olis = [/*query*/];
    for (/*opportunity loop*/) { /*logic*/ }
    update olis;

No need to check if collection is empty

There is no problem with looping over an empty list. The following is fine, for instance:

for (OpportunityLineItem oli : new List<OpportunityLineItem>()) { /*logic*/ }

You can accomplish this functionality in a declarative way.

Process Builder is perfectly capable of executing this functionality. Some of the benefits of using declarative functionality are that it does not require unit testing, and it is simpler to maintain/extend.

| improve this answer | |
  • Thank you Adrian... Will follow the best practices, as advised. – jay Apr 27 '16 at 18:28

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.