5

I'm getting up to speed with Apex - I have written a trigger that works, but is this the approach that an experienced coder would take?

I have a custom object (vist_detail__c) which has a lookup to Contracts. We create tasks on the custom object. I want an alert when the task date is beyond the Contract end date.

(The commenting shows that I began by adding the trigger.new IDs to a set but then I couldn't figure out how to access the Task date to compare with Contract end date so I dropped the set idea)

trigger MaCwTaskTriggerTest on Task (after insert, after update) {
    system.debug('Hey Dude');
//    Set<Id> cwIds = new Set<Id>();
    for (Task aTask: Trigger.new)
    {
//    cwIds.add(aTask.WhatId);
//      system.debug('Added to set: '+aTask.WhatId);
//  }
//    for (Id i : cwIds) {
//        system.debug('Id in list is '+i);

        vist_Detail__c vD = [SELECT contract__r.endDate FROM vist_Detail__c 
                                    WHERE Id = :aTask.WhatId];
          Date cEndDate = vD.contract__r.endDate;
          system.debug('Contract end date: '+cEndDate);
          if (aTask.ActivityDate > cEndDate) {
            system.debug('Task Date greater than Contract end date');                                  
            }
          else {
            system.debug('Task Date less than or equal to Contract end date');                                  
               }
    }
}

It works, but is it best practice?

Any suggestions on how best to alert a user? The task is created by workflow. I can't see a situation where these tasks would be created in bulk in which case would emailing user be sensible? I have the Apex for that.

Many Thanks, Mike.

  • It's unclear to me what you're trying to accomplish with the trigger. The trigger will only fire when there's been a change to Task. So if you're trying to determine if a Task is beyond its due date, a Trigger isn't going to notify you. Instead, you'll need to run a class to check all Tasks where the status is open on Tasks related to vist_Detail__c. – crmprogdev Jul 16 '15 at 13:16
  • 1
    Check out David Liu's tutorials for more info on the query inside a loop issue. – Alex S Jul 16 '15 at 13:17
  • 1
    Have you thought about using your trigger to populate a date field on the task, then create a workflow (triggered when the field is populated) to send a time based email alert if the task status is still open at the scheduled time? Time based workflows only send alerts / update fields etc. if their criteria are still met at the scheduled time so include Text(Status) = 'Open'. It should be easier to maintain than code. – Alex S Jul 16 '15 at 13:42
  • @crmprogdev - I want to notify a user when a Task is created with a Date that is beyond the end date of the customer contract so that engineers are not despatched to a site where the contract has ended and the customer refuses to pay. – MikeA Jul 16 '15 at 16:30
  • @alex - David Liu's tutorials are really good - I need to watch and read them again! Good idea about triggering a workflow - I will update an invisible checkbox and use that to trigger an email if a Task has been created which is after the customer contract has ended. – MikeA Jul 16 '15 at 16:33
4

Problem 1:

Query inside for loop

Problem 2:

Without checking the whatId of Task to be Id of Vist_Detail__c, the query can return 0 records and throw

List has no rows for assignment error

Look for documentation to rectify the above mentioned errors.

Hope it helps. Happy learning!

1

Good catch - Thanks @vigneshwaran.

Like this?

trigger MaCwTaskTriggerTest on Task (after insert, after update) {
    Set<Id> tIds = new Set<Id>(); // Task IDs
    Set<Id> cwIds = new Set<Id>(); // Task WhatIDs
    Map<Id, Task> TMap;
    Map<Id, vist_Detail__c> vDMap;


    for (Task aTask: Trigger.new)
    {
      cwIds.add(aTask.WhatId);
      system.debug('Task.WhatId added to set: '+aTask.WhatId);
      tIds.add(aTask.Id);
      system.debug('Task.Id added to set: '+aTask.Id);
    }

    TMap = new Map<Id, Task>([SELECT Id, ActivityDate FROM Task 
                                    WHERE Id in :tIds]);
    vDMap = new Map<Id, vist_Detail__c>([SELECT Id, contract__r.endDate FROM vist_Detail__c 
                                    WHERE Id in :cwIds]);
    system.debug('vDMap = '+vDMap);

        if (vDMap.size() > 0) {
            for (Task aTask: Trigger.new) {
              system.debug ('Task Activity Date: '+TMap.get(aTask.Id).ActivityDate);
              system.debug ('Contract End Date: '+vDMap.get(aTask.WhatId).contract__r.endDate);
                if (TMap.get(aTask.Id).ActivityDate > vDMap.get(aTask.WhatId).contract__r.endDate)
                system.debug('Task Date greater than Contract end date');                                  
            else
                system.debug('Task Date less than or equal to Contract end date');                                  
            }
        }
       else system.debug('No VDs selected');
    }
  • 3
    this should be posted as an update to the original post, not as an answer – cropredy Jul 16 '15 at 17:03
  • 2
    and TMap is unnecessary as you have access to Trigger.newMap – cropredy Jul 16 '15 at 17:04

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.