1

Getting the error in line " taskObj.OwnerId = ConMap.get(ben.contact__c).contact__r.OwnerId;" becasue the ownerid field is on contact.

Contact is the parent of benefit, Here I am getting all the benefits in start method. I want to add contactid only once if it has more than one child for that I used SET. I want to use maps as I need to get the contact OwnerId field from contact object which I am fetching in the query in start method. How do I Access contact.ownerId field using a map? below is the code.

global Database.QueryLocator start(Database.BatchableContext bc) {
    Query='select contact__r.ownerId, contact__c, Task_creation_date__c, Status__c, task_created__c, type__c from Benefit__c Where Task_creation_date__c= TODAY AND Status__c IN (\'Active\',\'Pending\') AND Task_created__c =FALSE AND Type__c!=\'Short Term Medical\' AND Contact__c!=NULL';
    return Database.getQueryLocator(query);            
}

global void execute(Database.BatchableContext bc, List<Benefit__c> scope){

    List<Task> TaskList = new List<Task>();
    Map<id,benefit__c> ConMap = new map<id,benefit__c>();
    for (Benefit__c b : scope) 
    {
            if(!ConMap.containsKey(b.contact__c))
            {
                ConMap.put(b.contact__c, b);
            }
            b.task_created__c = TRUE;
     }
    system.debug('contact and its benefit------'+ConMap);
     for(Benefit__c ben : ConMap.values())
       {
        Task taskObj = new Task();
                        taskObj.OwnerId = ben.Contact__r.ownerId;
                        taskObj.ActivityDate = System.Today();
                        taskObj.Status = 'Open';
                        taskObj.Priority = 'Normal';
                        taskObj.Subject = 'Courtesy Call Task';
                        taskObj.WhoId = ben.Contact__c;
                        taskObj.Description = 'Courtesy Call Task';
                        taskObj.Type = 'Other';
                        taskObj.Sub_Type__c = 'Other';

            taskList.add(taskObj);

        }

I want to populate contact ownerid as the task ownerid but how do I access it from the map and keep the unique contact id in the map?

initial Code :

    Query='select contact__c, Task_creation_date__c, Status__c, task_created__c, type__c from Benefit__c Where Task_creation_date__c= TODAY AND Status__c IN (\'Active\',\'Pending\') AND Task_created__c =FALSE AND Type__c!=\'Short Term Medical\'';
    return Database.getQueryLocator(query);

}

global void execute(Database.BatchableContext bc, List<Benefit__c> scope){
    // process each batch of records

    List<Contact> contacts = new List<Contact>();
    List<Task> TaskList = new List<Task>();
    set<id> conset = New set<id>();
    for (Benefit__c b : scope) {
            conset.add(b.contact__c);
            b.task_created__c = TRUE;
     }

     recordsProcessed = conset.size();
     List<Contact> tempList = new List<Contact>();
     tempList = [Select Id,OwnerId, firstname from Contact where Id IN:(conset)];
     if(tempList.size()>0)
      {
       for(Contact con : tempList)
       {
        Task taskObj = new Task();
                        taskObj.OwnerId = con.OwnerId;
                        taskObj.ActivityDate = System.Today();
                        taskObj.Status = 'Open';
                        taskObj.Priority = 'Normal';
                        taskObj.Subject = 'Courtesy Call Task';
                        taskObj.WhoId = con.id;
                        taskObj.Description = 'Courtesy Call Task';
                        taskObj.Type = 'Other';
                        taskObj.Sub_Type__c = 'Other';

            taskList.add(taskObj);

        } //close for statement
    TotalTask = taskList.size();  
   } //close if loop
0

Posting this solution as a separate answer because it is a radically different approach.

If you want to only create one Task for each Contact__c value, you will need to take a different approach, because two records with the same Contact__c may be processed in separate execute blocks. Batch over an AggregateResult query instead to avoid this issue.

public with sharing class MyBatch implements Database.Batchable<AggregateResult>
{
    public Iterable<AggregateResult> start(Database.BatchableContext context)
    {
        return [
            SELECT Contact__c, Contact__r.OwnerId owner FROM Benefit__c
            WHERE Contact__c != null AND ...
            GROUP BY Contact__c, Contact__r.OwnerId
        ];
    }
    public void execute(Database.BatchableContext context, List<AggregateResult> scope)
    {
        Set<Id> contactIds = new Set<Id>();
        List<Task> tasks = new List<Task>();
        for (AggregateResult aggregate : scope)
        {
            Id ownerId = (Id)aggregate.get('owner');
            Id contactId = (Id)aggregate.get('Contact__c');

            contactIds.add(contactId);
            tasks.add(new Task(OwnerId=ownerId, /*other fields*/));
        }
        insert tasks;

        List<Benefit__c> benefits = [
            SELECT Id FROM Benefit__c WHERE Contact__c IN :contactIds
        ];
        for (Benefit__c benefit : benefits)
            benefit.Task_Created__c = true;
        update benefits;
    }
    public void finish(Database.BatchableContext context) { /*additional logic*/ }
}
  • I really appreciate your help, I added my initial code in which to avoid the query in execute method I thought of using maps. Even that code will fail in a scenario when benefit for the same contact will be in different batches. correct? And is there a way can we avoid the query in execute method to update the benefit field? – apple123 Sep 30 '16 at 17:36
  • You have correctly understood why I recommend this approach instead. I don't think you can avoid using a query in the execute method, and I don't think you need to worry about it. Each gets its own set of static governor limits. – Adrian Larson Sep 30 '16 at 17:38
  • final query : ` return [ select contact__r.ownerId owner, contact__c, Task_creation_date__c, Status__c, task_created__c, type__c from Benefit__c Where Task_creation_date__c= TODAY AND Status__c IN ('Active','Pending') AND Task_created__c =FALSE AND Type__c!='Short Term Medical' AND Contact__c!=NULL GROUP BY Contact__c, Contact__r.OwnerId ];` giving me erro :Field must be grouped or aggregated: Task_Creation_Date__c – apple123 Sep 30 '16 at 17:46
  • Yes, the error message tells you exactly what's wrong. Once you start grouping by fields, everything in your SELECT clause must be wrapped in an aggregation function or included in your GROUP BY clause. Why do you need your query to return any of those fields? It does not look like you use them in your execute logic. – Adrian Larson Sep 30 '16 at 17:48
  • you are actually correct, I never realized that, Thank you so much for pointing that out. I have updated the code as per your suggestion and I will continue my unit test now to see if it giving expected result. – apple123 Sep 30 '16 at 17:51
2

The below advice pertains to the issues in your code, but the correct fix here is to add a filter to your QueryLocator:

WHERE Contact__c != null AND /*other clauses*/

Now you can just do:

Set<Id> contactIds = new Set<Id>();
List<Task> tasks = new List<Task>();
for (Benefit__c benefit : scope)
{
    if (contactIds.contains(benefit.Contact__c)) continue;
    // create task and add to collection
    contactIds.add(benefit.Contact__c);
}

If you want to stick to your current Map and QueryLocator, you don't need to get the Benefit__c out of the map again, since you are already looping through the values. Also, you can check if the Contact__c field is null before doing any further processing. And you don't need to check if the Map is empty before your for loop, since an empty loop does nothing.

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

// no isEmpty check necessary
for (Benefit__c benefit : conMap.values())
{
    if (benfit.Contact__c != null)
    {
        Task task = new Task(OwnerId = benefit.Contact__r.OwnerId);
        //other logic
        tasks.add(task);
    }
}

Also note that if you expect this logic to create one Task for each Benefit__c, you are mistaken, since two Benefit__c records can have the same Contact__c, and so you will overwrite the value mapped to that key. If you want one per, you would need a Map<Id, List<Benefit__c>>.

Map<Id, List<Benefit__c>> contactToBenefits = new Map<Id, List<Benefit__c>>();
for (Benefit__c benefit : scope)
{
    if (benefit.Contact__c == null) continue;
    if (!contactToBenefits.containsKey(benefit.Contact__c))
        contactToBenefits.put(benefit.Contact__c, new List<Benefit__c>());
    contactToBenefits.get(benefit.Contact__c).add(benefit);
}

List<Task> tasks = new List<Task>();
for (List<Benefit__c> benefits : contactToBenefits.values())
{
    for (Benefit__c benefit : benefits)
    {
        // create task and add to collection
    }
}
  • That makes sense, silly mistake. But I still need map correct? and what should I do as I want only unique contact ids. List<Contact> contacts = new List<Contact>(); List<Task> TaskList = new List<Task>(); set<id> conset = New set<id>(); Map<id,benefit__c> ConMap = new map<id,Benefit__c>(); for (Benefit__c b : scope) { conset.add(b.contact__c); if(!conmap.isempty()){ ConMap.put(b.contact__c, b); } } I will remove the Set<id> as I do not need it anymore. correct? – apple123 Sep 30 '16 at 17:06
  • No you don't need a map at all. Still adding detail to my answer. – Adrian Larson Sep 30 '16 at 17:08
  • Thank you so much Adrian. I updated my code in the question, I want to avoid for loop inside for loop. Can you check the updated code in the question is it optimized? I also added the contact__c!=null to the querylocator. – apple123 Sep 30 '16 at 17:23
  • You don't have any nested loops, and again I don't think you need a Map at all. You need one Task per Contact__c? – Adrian Larson Sep 30 '16 at 17:24
  • Yes, one task per contact even if the contact has 4 benefits under it. – apple123 Sep 30 '16 at 17:25

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.