6

Resolved code for updating OpportunityLineItemSchedule ScheduleDate to new CloseDate when close date is changed on Opportunity object. Also to update OpportunityLineItem ServiceDate on change of Opportunity CloseDate Trigger:

    trigger OpportunityReScheduling on Opportunity (after update, before update, after insert) 
    {
        for (Opportunity o: Trigger.new)
        {
            if (Trigger.isBefore)
            {
                Opportunity oldCloseDate = Trigger.oldMap.get(o.ID);
                if (o.CloseDate != oldCloseDate.CloseDate) 
                {
                    Integer Oldday = Integer.valueOf(oldCloseDate.CloseDate.Day()); 

                    Integer day = Integer.valueOf(o.CloseDate.Day());
                    Integer month = Integer.valueOf(o.CloseDate.Month());
                    Integer year = Integer.valueOf(o.CloseDate.Year());                 

                    Integer DayDiff = day - Oldday;

                    OpportunitySchedulingHandler.ScheduleDateUpdate(o.id, day, month, year, DayDiff);
                }
            }

            if (Trigger.isAfter || Trigger.isInsert)
            {
                OpportunitySchedulingHandler.ScheduleServiceDateUpdate(o.id);   
            }
        }
    } 

Class:

    public with sharing class OpportunitySchedulingHandler {

     //Update LineItemSchedule dates for all scheduling dates

    public static void ScheduleDateUpdate(String oppid, Integer day, Integer month, Integer year, Integer DayDiff) 
    {
       List<OpportunityLineItem> idval = [SELECT id FROM OpportunityLineItem WHERE OpportunityId=:oppid];
       List<OpportunityLineItemSchedule> datelist = new List<OpportunityLineItemSchedule>();
       for (Integer i = 0; i < idval.size(); i++)
       {
           datelist = [SELECT ScheduleDate FROM OpportunityLineItemSchedule WHERE OpportunityLineItemId =:idval[i].id];
           for (Integer k = 0; k < datelist.size(); k++)
           {
               if(DayDiff <> 0)
               {
                //automatic functionality actually adds 24 list (12 for quantity 12 for revenue)
                //editing schedule for quantity
                if(k<12)
                {
                    Date mydate = date.newInstance(year,month,day);
                    datelist[k].ScheduleDate = mydate.addmonths(k);
                }
                //editing schedule for revenue
                else
                {
                    Date mydate = date.newInstance(year,month,day);
                    datelist[k].ScheduleDate = mydate.addmonths(k-12);
                }
               }
            }
           if(!datelist.isEmpty())
           {
                update datelist;
           }
        }
    }    

      //Update ServiceDate with closeDate

    public static void ScheduleServiceDateUpdate(String oppid)
    {
        List<Opportunity> oliList = [SELECT Id, Name, CloseDate, (SELECT Id, ServiceDate, OpportunityId from OpportunityLineItems) from Opportunity where Id =:oppid];
        List<OpportunityLineItem> oliUpdateList = new List<OpportunityLineItem>();
        for(Opportunity x : oliList)
        {
            for(OpportunityLineItem oli : x.OpportunityLineItems)
            {
                oli.ServiceDate = x.CloseDate;
                oliUpdateList.add(oli);
            }
        }
        if(!oliUpdateList.isEmpty()) 
        {
            update oliUpdateList;
        }
    }
} 

Test Class:

@isTest
private class OpportunityReSchedulingTest {

    static testMethod void myUnitTest() {
        // TO DO: implement unit test
        Test.starttest();

            Account acc = new Account();
            acc.Name='NewAcc';
            insert acc;

            Opportunity opp = new Opportunity();
            opp.Name='NewOpp';
            opp.AccountId=acc.Id;

            opp.StageName='Prospecting';
            opp.CloseDate=Date.today().addDays(10);

            insert opp;

            Product2 Prod =  new Product2();
            Prod.Name='NewProd';
            Prod.IsActive=True;
            insert Prod;

            PricebookEntry pbe = new PricebookEntry();
            pbe.Product2Id=Prod.Id;
            pbe.IsActive=True;
            pbe.UnitPrice=70;
            pbe.Pricebook2Id = Test.getStandardPricebookId();
            pbe.UseStandardPrice=false;

            insert pbe;

            OpportunityLineItem opli = new OpportunityLineItem();
            opli.UnitPrice = 57;
            opli.Quantity = 12;
            opli.OpportunityId=opp.Id;
            opli.PricebookEntryId=pbe.id;
            insert opli;

            opp.CloseDate = opp.CloseDate.addDays(20);
            update opp;


        Test.stoptest();
    }
}

Note:

Scheduling might give an error if OpportunityLineItemSchedule will be created manually (using establish/Re-establish function) for more than 12 schedules.

Dave
  • 507
  • 6
  • 27
  • Where are you having a problem at? Also, why are your doing a BeforeInsert and an AfterUpdate? A Before trigger operates very differently than an After trigger. I don't see you taking advantage of operating on Trigger.new in any kind of a "before" section of code. Perhaps you need to write the code separately and then combine the code into one trigger if you truly want a Before Insert and an After Update. – crmprogdev May 26 '15 at 19:20
  • Please refer comment below – Dave Jun 04 '15 at 13:57
  • 1
    Click on edit at bottom left of your post above to update it. Additional info should not be posted as an answer to your question. Please visit the [help] center to learn how to use this forum. Suggest you add debug statements to your code and look at the logs. See How do I start to debug my own Apex code?. – crmprogdev Jun 04 '15 at 14:02
  • Please check updated code and comments. – Dave Jun 15 '15 at 13:29

2 Answers2

5

Thanks for this original code, it has been such a helpful base for us. I thought I would share some fixes we have applied, they might help someone out there.

This is when revenue scheduling isn't the norm, but needs to be an option for your organisation, so the default product schedule is 1 line item.

Here is our fix. This removes the issue of the duplication of the line item.

Trigger:

trigger OpportunityReScheduling on Opportunity (after update, before update, after insert) 
    {
        for (Opportunity o: Trigger.new)
        {
            if (Trigger.isBefore)
            {
                Opportunity prevOpportunity = Trigger.oldMap.get(o.ID);
                if (o.CloseDate != prevOpportunity.CloseDate) 
                {
                    Integer DayDiff = prevOpportunity.CloseDate.daysBetween(o.CloseDate);

                    OpportunitySchedulingHandler.ScheduleDateUpdate(o.id, DayDiff);
                }
            }

            if (Trigger.isAfter || Trigger.isInsert)
            {
                OpportunitySchedulingHandler.ScheduleServiceDateUpdate(o.id);   
            }
        }
    }

Class:

public with sharing class OpportunitySchedulingHandler {

     //Update LineItemSchedule dates for all scheduling dates

    public static void ScheduleDateUpdate(String oppid, Integer DayDiff) 
    {
       List<OpportunityLineItem> idval = [SELECT id FROM OpportunityLineItem WHERE OpportunityId=:oppid];
       List<OpportunityLineItemSchedule> datelist = new List<OpportunityLineItemSchedule>();
       for (Integer i = 0; i < idval.size(); i++)
       {
           datelist = [SELECT ScheduleDate FROM OpportunityLineItemSchedule WHERE OpportunityLineItemId =:idval[i].id];
           Date firstDate = datelist[0].ScheduleDate.addDays(DayDiff);
           datelist[0].ScheduleDate = firstDate;

           Integer day = firstDate.day();
           Integer month = firstDate.month();
           Integer year = firstDate.year();

           for (Integer k = 1; k < datelist.size(); k++)
           {
               Integer nYear = year;
               Integer nMonth = month + k;
               Integer nDay = day;

               if (nMonth > 12) {
                   nMonth = nMonth - 12;
                   nYear = nYear + 1;
               }

               Set<Integer> longMonths = new Set<Integer> {1,3,5,7,8,10,12};

               if (nDay == 31 && ! longMonths.contains(nMonth)) {
                   nDay = 30;
               }

               if (nDay > 28 && nMonth == 2) {
                   nDay = 28;
               }

               Date mydate = date.newInstance(nYear,nMonth,nDay);
               datelist[k].ScheduleDate = mydate;
           }
           if(!datelist.isEmpty())
           {
                update datelist;
           }
        }
    }    

      //Update ServiceDate with closeDate

    public static void ScheduleServiceDateUpdate(String oppid)
    {
        List<Opportunity> oliList = [SELECT Id, Name, CloseDate, (SELECT Id, ServiceDate, OpportunityId from OpportunityLineItems) from Opportunity where Id =:oppid];
        List<OpportunityLineItem> oliUpdateList = new List<OpportunityLineItem>();
        for(Opportunity x : oliList)
        {
            for(OpportunityLineItem oli : x.OpportunityLineItems)
            {
                oli.ServiceDate = x.CloseDate;
                oliUpdateList.add(oli);
            }
        }
        if(!oliUpdateList.isEmpty()) 
        {
            update oliUpdateList;
        }
    }
}

I hope this is helpful to someone out there!

James

James Whiting
  • 68
  • 1
  • 7
3

I think the problem you're having is in the way you're getting the DateDiff value.

The close date can change, but the day of the month it changes to, may not change at all. The original date may be on Oct, 30 2015, and the new close date may be Dec 30, 2015. The dateDiff should be 61 days. However, you've been using a method that calculates it by getting the day of the month for both the old and the new. Unless the day of the month changes, you'll show no difference.

What if the difference is say 95 days and your current day of the month is 29? If you were calculating it correctly, your method wouldn't accommodate that. You'd be creating actually creating a new instance of an invalid date rather than a date that's 3 months out.You'll see where I've fixed that using the daysBetween method as shown below:

 if (Trigger.isBefore)
            {
                Opportunity oldCloseDate = Trigger.oldMap.get(o.ID);
                if (o.CloseDate != oldCloseDate.CloseDate) 
                {

                    Integer DayDiff = o.CloseDate.daysBetween(Trigger.oldMap.get(o.ID));

                    Date newDate = o.CloseDate;

                    OpportunitySchedulingHandler.ScheduleDateUpdate(o.id, newDate, DayDiff);
                }
            }

In your class, you need to modify how it handles this. First, your method should be defined to accept lists or maps, not single strings, integers, dates, etc. Your code is already bulkified to handle lists. I recommend you consider changing your DateDiff list to a map so you can link those values to the opp.id and then pass them along with your lists to your method after the end of your for loop all at once.

I think that another part of what's happening is that you're calling the two methods separately and not linking them with the oppId. As such, they're not being updated concurrently. I strongly suspect that's why new entries are being created for dates that are not linked to your existing line items; something which could be related to the fact this is in a Before context. To fix that, I'd recommend putting all of this into an After Trigger. You're not gaining anything by having both before and after triggers that I can see since trigger.new values are not being passed back into Opportunity.

 public static void ScheduleDateUpdate(list<String> oppids, list<date> newDates, list<Integer> DayDiff) 
 {
   List<OpportunityLineItem> idval = [SELECT id FROM OpportunityLineItem WHERE OpportunityId=:oppid];
   List<OpportunityLineItemSchedule> datelist = new List<OpportunityLineItemSchedule>();
   for (Integer i = 0; i < idval.size(); i++)
   {
       datelist = [SELECT ScheduleDate FROM OpportunityLineItemSchedule WHERE OpportunityLineItemId =:idval[i].id];
       for (Integer k = 0; k < datelist.size(); k++)
       {
           //Date mydate = Date.newInstance(datelist[k].ScheduleDate.Year(),datelist[k].ScheduleDate.Month(),datelist[k].ScheduleDate.Day());
           if(DayDiff <> 0)
           {
            Date mydate = date.newInstance(year,month,day);
            datelist[k].ScheduleDate = mydate.addmonths(k);
           }
        }
       update datelist;
   }
 }    

In your method above, your adding months to your scheduledDate by creating a new instance of the scheduled date, then adding the value of the iterator k to the number of months for the new instance. I can't see where this has anything to do with the DayDiff you calculated earlier in your trigger.

This tells me either you don't need to calculate the DayDiff in your trigger, or you need to be using it in the above section of your class to calculate the new ScheduleDate. I'd think the latter would be the case. If so, I'd expect you to take the existing ScheduleDate and use the addDays method to calculate the new date. If you do that, your two queries can be combined into one query and you'll only need a single for loop.

public static void ScheduleDateUpdate(map<Id,Integer> dayDiffMap) 
     {

       List<OpportunityLineItem> updtlst = new list<OpportunityLineItem>();    
       List<OpportunityLineItem> datelist = [SELECT id, ScheduleDate, OpportunityId FROM OpportunityLineItem WHERE OpportunityId IN :dayDiffMap.keyset()];

       For(OpportunityLineItemSchedule oli:datelist)
       {
          d = new OpportunityLineItemSchedule(Id = oli.Id, OpportunityId=oli.Opportunity.Id);
          d.ScheduleDate = oli.ScheduleDate.addDays(dayDiffMap.get(oli.OpportunityID);

          updtlst.add(d);
       }

       update updtlst;
    }

Notice that when you do this, all you need to pass it is the dayDiffMap which contains the oppIds and the Differences in Days. You don't need the new close date or a separate list of Opp Id's.

Edit

Here's some very relevant info from the ObjectReference on OpportunityLineItemScheule:

OLISchedule Allowed Field Types Table

Additionally:

The Quantity and Revenue fields have the following restrictions when this object is updated:

  • For a schedule of Type Quantity, you can’t update a null Revenue value to non-null. Likewise for a schedule of Type Revenue, you can’t update a null Quantity value to non-null.

  • You can’t null out the Quantity field for a schedule of Type Quantity. Likewise you can’t null out the Revenue field for a schedule of Type Revenue.

  • You can’t null out either the Revenue or Quantity fields for a schedule of type Both.

See the ObjectReference for even more restrictions related to this object as well as OLI...

crmprogdev
  • 40,955
  • 9
  • 58
  • 115
  • Thanks for explaining logic behind two different updates, In your class code, In line#5 you used OpportunityLineItem to fetch ScheduleDate and OpportunityID, where OpportunityLineItem contains OpportunityID but not ScheduleDate, same for OpportunityLineItemSchedule which contains ScheduleDate but not ID, if we try to link them by foreign key, it will throw error of invalid foreign key. same for line #9 where relationship is not valid + need to associate d with List or Integer where it won't allow foreign key use. – Dave Jun 15 '15 at 17:44
  • About my code, I used dayDiff previously where I was adding days as per dayDiff value, in this new code, I'm taking close date integers as a reference and adding months comparing that. If we add days, it will give error for month ending days, ex. if we add 31 Jan then it'll give 3rd Mar, 31 Mar, 1 May, 31 May and so on. for getting that correct, we'll need to add different logic for every month or have to add by month, so I used month here. – Dave Jun 15 '15 at 17:49
  • My bad, I missed that on the query, Was a long reply to your post. You should still be able to do a subquery to capture that in a single query. I'd have to look in Workbook or some other tool to verify, but that's what I'd expect. – crmprogdev Jun 15 '15 at 18:11
  • As for the difference in days, I was simply trying to point out that if both the prev and new close dates were on the 29th of the month, your method wouldn't show a difference in the number of days. Your method for months in your class to create the new scheduleDate, made no sense to me at all. I couldn't see how it was related to the difference between old & new close dates, or the diff value you passed in from the trigger. If this answer leads you to resolving your issues, please help the rest of the community by marking it as having answered your question. – crmprogdev Jun 15 '15 at 18:16
  • Sure, I'm trying to use the logic you provided ( Used it before using list, didn't work, using maps now ). If this leads to the solution, that would be great for me and others too, so I'll update new code here. Meanwhile, I'm updating upgrades with less errors in code time by time and yes, you're right, if I change just month with same date on, it is not making any change in schedule so I'll have to work on that but if we use days, then it just add days in current date which will create problem in scheduling date for month having less number of days than other (scheduled, ex. feb) – Dave Jun 15 '15 at 19:21
  • The addDays method will move you into the following month (or year). It doesn't just add days within the current month when applied to a Date value (dd/mm/yyyy or whatever format your locale uses) if that's your concern. It takes the month and year into account (catches leap years), so I'm not quite following the concern you have here. – crmprogdev Jun 15 '15 at 21:43
  • I updated code adding by days and as per your suggestion but it is still showing same error where revenue now stays as previous date with 0 quantity but quantity changes perfectly as per date. – Dave Jun 16 '15 at 13:30
  • As I said previously, I believe you need to be changing everything related to OLI and OLISchedule at the same time and in the same context. I'd move all of the Before code into the After Code and update them in the same context. The before and after update sections are doing updates on the same lines of OLISchedule and OLI that you just operated on. I believe that's the cause of the duplicate entries. Put all the code in the trigger as AfterUpdate and as either a single method or else call the 2nd from the 1st (or vice versa), & I'd expect that would solve your issue. – crmprogdev Jun 16 '15 at 17:36
  • No, Still having the same error, I kept every logic in after update, after insert, and still having the same error – Dave Jun 16 '15 at 18:52
  • I think you may want to reverse the order of the updates. First update OLI, then update OLISchedule. That should keep the two of them in sync. Isn't one a child of the other? I think your problem surrounds an issue of that sort. Not certain if they're in a M-D relationship or not, but you may want to check. – crmprogdev Jun 16 '15 at 18:59
  • OpportunityLineItem and OpportunityLineItemSchedule are standard objects and as far as I understand their functionaliry, those are parent child having relationship based on ID that's why we're matching ID here to fetch/update record. But as we're not deleting any data here just updating one data which is starting date of schedule and adding month/ day based on that. Previous trigger I had was deleting and inserting all schedules so feed was tracking that and showing amount from x - 0 , 0 - x . – Dave Jun 16 '15 at 19:14
  • See my edited response above with info from the Obj Reference that you'll want to see and consider. I think this should explain the source of the problems you're still seeing. – crmprogdev Jun 16 '15 at 19:31
  • worked on the comments you added there, still not working. still giving same error as multiple insertion. – Dave Jun 25 '15 at 20:07
  • Please edit your post to add you latest versions of trigger and class code. – crmprogdev Jun 26 '15 at 12:50
  • I didn't get much time to work on that after that, I've worked a little now and trying to figure out what's wrong, why it's not inserting any values now. I know logic is not complete and missing something there but trying to figure that out. (Code Updated) – Dave Jul 16 '15 at 13:46
  • Are you doing BeforeUpdate or AfterUpdate? I see your trigger is set to do both. That could be part of your issue. You query the opp before it's been updated when updating on BeforeUpdate. You can pass Trigger.New to the class instead to have the correct close date to use. – crmprogdev Jul 16 '15 at 14:09
  • Before update is to reschedule product sales on OpportunityLineItemSchedule and after update is just to change date in OpportunityLineItem based on opportunity close date. Those two are two different functionality consolidated here – Dave Jul 16 '15 at 14:44
  • I suspect you need to change update schedule; to upsert schedule;. That way it will either create new ones or update existing ones. Right now, if they don't already exist, it doesn't create them. – crmprogdev Jul 16 '15 at 15:21
  • Update should work as schedule does exist, it should not create schedule if it's not there, it just needs to change date of existing schedule, so update is good, New trigger logic looks simple to me considering adding days as per new schedule, I even tried creating custom formula field to note change date number and add/subtract that from existing schedule but this updated trigger doesn't make any change on that. – Dave Jul 16 '15 at 15:32
  • I'd logic before which was removing whole scheduling, Identifying new date, creating schedule depending on that & inserting new table but it was showing all updates (like Amount changed from 5000 to 0, amount changed to 0 to 5000) in feed which was the reported error – Dave Jul 16 '15 at 15:41
  • Problem is, you're calling the same method for both the BeforeInsert and BeforeUpdate triggers. You need to call a different method for the BeforeInsert trigger if you want to create new schedules with that trigger BeforeInsert and not BeforeUpdate. I also see that your classes appear to be expecting lists, which could be at the root of your problem. Check debug logs. – crmprogdev Jul 16 '15 at 16:35
  • I have created different logic for before and after events which doesn't relate to each other, just to double check after your comment, I removed that code and that behaved same way so before and after event is not making any difference, so please ignore after event in trigger and related class function – Dave Jul 16 '15 at 17:54
  • I'm referring to BeforeInsert and BeforeUpdate. You want to insert new Schedule records on BeforeInsert and update those same Schedule records on BeforeUpdate of Opportunity do you not? Isn't that what you're telling me the problem is that you're having? If not, I'll let you spend time working further on it to sort it our yourself. That's what my comments have been about all along today. – crmprogdev Jul 16 '15 at 18:08
  • There isn't BeforeInsert event on this trigger, entering new Schedule is automated process from salesforce but I just want to update that on change of record change in Opportunity. After trigger is just to make changes in OpportunityLineItems. so Update process is happening only once on change of field value in Opportunity object which is not possible through afterUpdate process. – Dave Jul 17 '15 at 12:55
  • I'd have sworn when I was reading your code yesterday, the trigger was defined as beforeinsert too. Must have misread. – crmprogdev Jul 17 '15 at 13:32
  • I just talked with salesforce premier support (developer support) and we checked query, due to product auto schedule functionality, salesforce is creating 12 schedules for quantity and related to that, it is creating more 12 schedules for revenue so total 24 schedules for one OpportunityLineItem with repetition of date twice. Trigger code is understanding 24 schedules and assigning values regarding that so here it is creating 24 schedules, 12 for revenue 12 for quantity. Trigger code is correct but it is salesforce standard functionality error here. (working with admin support now) – Dave Jul 20 '15 at 20:03
  • Solved error upon yesterday's diagnosis, Please upvote if you think this will help other developers. Thanks for your help so far, I can't upvote your answer as I don't have enough reputation to do that. – Dave Jul 21 '15 at 14:14
  • Thanks for letting us know the problem has been resolved. If this answer led you to resolving your issue, please help the rest of the community by marking it as having answered your question. – crmprogdev Jul 21 '15 at 14:19