Skip to content

Commit

Permalink
Merge pull request #223 from SalesforceFoundation/feature/JRS-improve…
Browse files Browse the repository at this point in the history
…ments

optimize ProcessListVRS to reuse existing Hours if possible.  Issue #154
  • Loading branch information
Nicolas Campbell authored Dec 5, 2016
2 parents d560f25 + c6d5a39 commit 8cc81ad
Show file tree
Hide file tree
Showing 6 changed files with 567 additions and 118 deletions.
17 changes: 12 additions & 5 deletions src/classes/VOL_BATCH_Recurrence_TEST.cls
Original file line number Diff line number Diff line change
Expand Up @@ -58,16 +58,23 @@ public class VOL_BATCH_Recurrence_TEST {
}

@isTest private static void testSingleBatch() {

// set future recurrence to low value for initial JRS insert
VOL_SharedCode.VolunteersSettings.Recurring_Job_Future_Months__c = 1;
createRecurringJob();
List<Volunteer_Shift__c> shifts = [SELECT Id FROM Volunteer_Shift__c];
integer cShifts = shifts.size();
system.assert(cShifts >= 0 && cShifts <= 5);

// now increase our future recurrence limit to a larger size and run the batch
VOL_SharedCode.VolunteersSettings.Recurring_Job_Future_Months__c = 2;
VOL_BATCH_Recurrence batchRecSched = new VOL_BATCH_Recurrence();

Test.startTest();
database.executeBatch(batchRecSched);
Test.stopTest();
Job_Recurrence_Schedule__c recurSched = [SELECT Id FROM Job_Recurrence_Schedule__c LIMIT 1];
List<Volunteer_Shift__c> shifts = [SELECT Id, Job_Recurrence_Schedule__c FROM Volunteer_Shift__c WHERE Job_Recurrence_Schedule__c = :recurSched.Id];

System.assertNotEquals(0, shifts.size(), 'Multiple shifts should have been created by the batch.');
shifts = [SELECT Id FROM Volunteer_Shift__c];
integer cShiftsNew = shifts.size() - cShifts;
system.assert(cShiftsNew >=3 && cShiftsNew <= 5);
}

}
11 changes: 7 additions & 4 deletions src/classes/VOL_JRS.cls
Original file line number Diff line number Diff line change
Expand Up @@ -132,25 +132,28 @@ public with sharing class VOL_JRS {
listVRS = [select Id, Name, Contact__c, Schedule_Start_Date_Time__c, Schedule_End_Date__c, Duration__c,
Weekly_Occurrence__c, Days_Of_Week__c, Volunteer_Job__c, Volunteer_Hours_Status__c, Number_of_Volunteers__c, Comments__c
from Volunteer_Recurrence_Schedule__c where Volunteer_Job__c in : setJobId];
VOL_VRS.ProcessListVRS(listVRS);
VOL_VRS.ProcessListVRS(listVRS, null, fReviewAllShifts);
} else {
ProcessListVRSFuture(setJobId);
ProcessListVRSFuture(setJobId, fReviewAllShifts);
}
}

//******************************************************************************************************
// Process all the VRS's for the set of Job's. This Future cover was created to avoid
// hitting apex system statement limits that we would hit if there were many VRS's for a given job.
// fReviewAllShifts parameter specifies whether called from the trigger on JRS's, in
// which case we should review all shifts under the Jobs, or from the scheduled batch,
// in which case we only need to be looking to add additional shifts in the future.
@future (callout=false)
private static void ProcessListVRSFuture(set<ID> setJobId) {
private static void ProcessListVRSFuture(set<ID> setJobId, boolean fReviewAllShifts) {

list<Volunteer_Recurrence_Schedule__c> listVRS = new list<Volunteer_Recurrence_Schedule__c>();
listVRS = [select Id, Name, Contact__c, Schedule_Start_Date_Time__c, Schedule_End_Date__c, Duration__c,
Weekly_Occurrence__c, Days_Of_Week__c, Volunteer_Job__c, Volunteer_Hours_Status__c, Number_of_Volunteers__c, Comments__c
from Volunteer_Recurrence_Schedule__c where Volunteer_Job__c in : setJobId];

// process the VRS's to create hours as needed.
VOL_VRS.ProcessListVRS(listVRS);
VOL_VRS.ProcessListVRS(listVRS, null, fReviewAllShifts);
}


Expand Down
95 changes: 95 additions & 0 deletions src/classes/VOL_JRS_TEST.cls
Original file line number Diff line number Diff line change
Expand Up @@ -153,4 +153,99 @@ public with sharing class VOL_JRS_TEST {

}

//******************************************************************************************************
// test that modifying a JRS reuses already existing Shifts
public static testmethod void testJRSShiftReuse() {

// create test data
Campaign cmp = new Campaign(recordtypeid=VOL_SharedCode.recordtypeIdVolunteersCampaign,
name='Job Calendar Test Campaign', IsActive=true);
insert cmp;
Volunteer_Job__c job = new Volunteer_Job__c(name='Job1', campaign__c=cmp.Id);
insert job;
Job_Recurrence_Schedule__c jrs = new Job_Recurrence_Schedule__c(Volunteer_Job__c = job.Id);
jrs.Days_of_Week__c = 'Monday';
jrs.Duration__c = 1;
jrs.Schedule_Start_Date_Time__c = system.now();
jrs.Weekly_Occurrence__c = 'Every';
jrs.Desired_Number_of_Volunteers__c = 5;
jrs.Description__c = 'initial description';
insert jrs;

list<Volunteer_Shift__c> listShift = [select Id from Volunteer_Shift__c];
set<ID> setShiftId = new set<ID>();
for (Volunteer_Shift__c shift : listShift)
setShiftId.add(shift.Id);
integer cShifts = setShiftId.size();
system.assert(cShifts > 0);

// now modify the JRS
jrs.Days_of_Week__c = 'Monday;Tuesday';
jrs.Description__c = 'new description';
Test.startTest();
update jrs;
Test.stopTest();

listShift = [select Id, CreatedDate, LastModifiedDate, Description__c from Volunteer_Shift__c];
integer cShiftsInitial = 0;
integer cShiftsNew = 0;
for (Volunteer_Shift__c shift : listShift) {
if (setShiftId.contains(shift.Id)) {
cShiftsInitial++;
} else {
cShiftsNew++;
}
system.assertEquals('new description', shift.Description__c);
}
system.assertEquals(cShifts, cShiftsInitial);
system.assert(cShifts == cShiftsNew || cShifts == cShiftsNew-1 || cShifts == cShiftsNew+1);
}

//******************************************************************************************************
// test that modifying a JRS deletes already existing Shifts that no longer match the JRS
public static testmethod void testJRSShiftDelete() {

// create test data
Campaign cmp = new Campaign(recordtypeid=VOL_SharedCode.recordtypeIdVolunteersCampaign,
name='Job Calendar Test Campaign', IsActive=true);
insert cmp;
Volunteer_Job__c job = new Volunteer_Job__c(name='Job1', campaign__c=cmp.Id);
insert job;
Job_Recurrence_Schedule__c jrs = new Job_Recurrence_Schedule__c(Volunteer_Job__c = job.Id);
jrs.Days_of_Week__c = 'Monday';
jrs.Duration__c = 1;
jrs.Schedule_Start_Date_Time__c = system.now();
jrs.Weekly_Occurrence__c = 'Every';
jrs.Desired_Number_of_Volunteers__c = 5;
jrs.Description__c = 'initial description';
insert jrs;

list<Volunteer_Shift__c> listShift = [select Id from Volunteer_Shift__c];
set<ID> setShiftId = new set<ID>();
for (Volunteer_Shift__c shift : listShift)
setShiftId.add(shift.Id);
integer cShifts = setShiftId.size();
system.assert(cShifts > 0);

// now modify the JRS
jrs.Days_of_Week__c = 'Tuesday';
jrs.Description__c = 'new description';
Test.startTest();
update jrs;
Test.stopTest();

listShift = [select Id, CreatedDate, LastModifiedDate, Description__c from Volunteer_Shift__c];
integer cShiftsInitial = 0;
integer cShiftsNew = 0;
for (Volunteer_Shift__c shift : listShift) {
if (setShiftId.contains(shift.Id)) {
cShiftsInitial++;
} else {
cShiftsNew++;
}
system.assertEquals('new description', shift.Description__c);
}
system.assertEquals(0, cShiftsInitial);
system.assert(cShifts == cShiftsNew || cShifts == cShiftsNew-1 || cShifts == cShiftsNew+1);
}
}
119 changes: 93 additions & 26 deletions src/classes/VOL_VRS.cls
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,15 @@ public with sharing class VOL_VRS {
// no longer match, and creates new hours into the future.
// called from both the VRS trigger (when the user modifies a specific VRS),
// as well as from the JRS processing, so new shifts get their hours assigned.
public static void ProcessListVRS(list<Volunteer_Recurrence_Schedule__c> listVRS) {
// @param listVRS the list of VRS's to process
// @param listVRSOld If called from the AfterUpdate trigger, the trigger.old list
// @param fReviewAllShifts Specifies whether called from the trigger on JRS's, in
// which case we should review all shifts under the Jobs, or from the scheduled batch,
// in which case we only need to be looking to add additional shifts in the future.
public static void ProcessListVRS(
list<Volunteer_Recurrence_Schedule__c> listVRS,
list<Volunteer_Recurrence_Schedule__c> listVRSOld,
boolean fReviewAllShifts) {

// get a set of the VRS ID's for querying
// also the Job ID's they are associated with
Expand All @@ -93,15 +101,27 @@ public with sharing class VOL_VRS {
setJobId.add(vrs.Volunteer_Job__c);
setContactId.add(vrs.Contact__c);
}

// if the VRS was updated, then deal with potent changes to the VRS's job or contact
if (listVRSOld != null) {
for (Volunteer_Recurrence_Schedule__c vrs : listVRSOld) {
setVRSId.add(vrs.Id);
setJobId.add(vrs.Volunteer_Job__c);
setContactId.add(vrs.Contact__c);
}
}

// get all shifts of the jobs these vrs's are associated with
list<Volunteer_Shift__c> listShift = new list<Volunteer_Shift__c>();
listShift = [select Id, Duration__c, Start_Date_Time__c, Desired_Number_of_Volunteers__c,
Volunteer_Job__c from Volunteer_Shift__c
where Volunteer_Job__c in :setJobId];

// delete all Hours that aren't completed or canceled.
DeleteListVRS(listVRS, true);
if (fReviewAllShifts) {
listShift = [select Id, Duration__c, Start_Date_Time__c, Desired_Number_of_Volunteers__c,
Volunteer_Job__c from Volunteer_Shift__c
where Volunteer_Job__c in :setJobId];
} else {
listShift = [select Id, Duration__c, Start_Date_Time__c, Desired_Number_of_Volunteers__c,
Volunteer_Job__c from Volunteer_Shift__c
where Volunteer_Job__c in :setJobId and Start_Date_Time__c >= TODAY];
}

// construct a map of Job to its associated list of VRS's
map<ID, list<Volunteer_Recurrence_Schedule__c>> mapJobIdListVRS = new map<ID, list<Volunteer_Recurrence_Schedule__c>>();
Expand All @@ -117,35 +137,66 @@ public with sharing class VOL_VRS {
// in order to avoid creating hours that already exist (completed or canceled),
// create a set of shiftId|contactId tuples, which allows us to quickly see if we
// already have an hours record for the given shift and contact.
list<Volunteer_Hours__c> listHoursExisting = [select Id, Volunteer_Job__c, Volunteer_Recurrence_Schedule__c,
Volunteer_Shift__c, Contact__c from
Volunteer_Hours__c where Volunteer_Job__c in :setJobId and Contact__c in :setContactId];
set<string> setShiftIdContactId = new set<string>();
list<Volunteer_Hours__c> listHoursExisting = new list<Volunteer_Hours__c>();
if (fReviewAllShifts) {
listHoursExisting = [select Id, Volunteer_Job__c, Volunteer_Recurrence_Schedule__c,
Volunteer_Shift__c, Status__c, Contact__c from
Volunteer_Hours__c where Volunteer_Job__c in :setJobId and Contact__c in :setContactId];
} else {
listHoursExisting = [select Id, Volunteer_Job__c, Volunteer_Recurrence_Schedule__c,
Volunteer_Shift__c, Status__c, Contact__c from
Volunteer_Hours__c where Volunteer_Job__c in :setJobId and Contact__c in :setContactId and Shift_Start_Date_Time__c >= TODAY];
}
map<string, Volunteer_Hours__c> mapShiftIdContactIdToHours = new map<string, Volunteer_Hours__c>();
for (Volunteer_Hours__c hr : listHoursExisting) {
setShiftIdContactId.add(hr.Volunteer_Shift__c + '|' + hr.Contact__c);
mapShiftIdContactIdToHours.put(hr.Volunteer_Shift__c + '|' + hr.Contact__c, hr);
}

// create hours for these vrs's for each shift
list<Volunteer_Hours__c> listHoursNew = new list<Volunteer_Hours__c>();
list<Volunteer_Hours__c> listHoursUpdate = new list<Volunteer_Hours__c>();
for (Volunteer_Shift__c shift : listShift) {
list<Volunteer_Recurrence_Schedule__c> listVRSforJob = mapJobIdListVRS.get(shift.Volunteer_Job__c);
AssignVRSHours(shift, listVRSforJob, setShiftIdContactId, listHoursNew);
AssignVRSHours(shift, listVRSforJob, mapShiftIdContactIdToHours, listHoursNew, listHoursUpdate);
}
if (listHoursNew.size() > 0)
insert listHoursNew;

if (listHoursNew.size() > 0) {
insert listHoursNew;
}

if (listHoursUpdate.size() > 0) {
update listHoursUpdate;
}

// mapShiftIdContactIdToHours now contains only the Hours that did not match the VRS's,
// so we can delete them if they aren't marked Completed or Canceled.
list<Volunteer_Hours__c> listHRDelete = new list<Volunteer_Hours__c>();
for (Volunteer_Hours__c hr : mapShiftIdContactIdToHours.values()) {
if (hr.Status__c != 'Completed' && hr.Status__c != 'Canceled') {
listHRDelete.add(hr);
}
}
if (listHRDelete.size() > 0) {
system.debug('****DJH: deleting listHRDelete: ' + listHRDelete);
delete listHRDelete;
}
}

//******************************************************************************************************
// for the specified shift, create VolunteerHours records for all volunteers
// @description for the specified shift, create VolunteerHours records for all volunteers
// who have a recurring schedule which should include this shift.
// note that it puts the hours on the provided list, and doesn't actually
// save them to the database.
public static void AssignVRSHours(
// @param shift The shift to assign Hours to
// @param listVRS The list of VRS's to consider for assigning hours to the shift
// @param mapShiftIdContactIdToHours A map of shiftId/ContactId keys to existing Hours. As Hours are
// matched, they are removed from this map. So upon return, this map contains those Hours that no longer
// match their VRS (so they can be considered for deletion).
// @param listHoursNew The returned set of Hours to create (leaving creation to the caller)
// @param listHoursUpdate The returned set of Hours to update (leaving update to the caller)
private static void AssignVRSHours(
Volunteer_Shift__c shift,
list<Volunteer_Recurrence_Schedule__c> listVRS,
set<string> setShiftIdContactId,
list<Volunteer_Hours__c> listHoursNew
map<string, Volunteer_Hours__c> mapShiftIdContactIdToHours,
list<Volunteer_Hours__c> listHoursNew,
list<Volunteer_Hours__c> listHoursUpdate
) {

if (listVRS == null) return;
Expand Down Expand Up @@ -176,19 +227,35 @@ public with sharing class VOL_VRS {

if (vrs.Weekly_Occurrence__c == null)
continue;

if ((listWhichWeeks[nweek] ||
vrs.Weekly_Occurrence__c.contains('Every') ||
(vrs.Weekly_Occurrence__c.contains('Alternate') && alternateWeekVRS(vrs, dtShift))) &&
listWhichDays[nday] &&
vrs.Schedule_Start_Date_Time__c != null &&
dtShift >= vrs.Schedule_Start_Date_Time__c.Date() &&
(vrs.Schedule_End_Date__c == null || vrs.Schedule_End_Date__c >= dtShift)) {
// we need to deal with the person already having hours on this shift
// that might be completed, or canceled, or web signup.
// avoid creating another confirmed record.
if (setShiftIdContactId.contains(shift.Id + '|' + vrs.Contact__c)) {
// note that if we match, we remove the key from the map, so we know it was matched.
Volunteer_Hours__c hrExisting = mapShiftIdContactIdToHours.get(shift.Id + '|' + vrs.Contact__c);
if (hrExisting != null) {
// if shift/hour in the future, we update a subset of fields
if (dtShift >= System.today()) {
hrExisting.Hours_Worked__c = vrs.Duration__c;
if (vrs.Number_of_Volunteers__c != null && vrs.Number_of_Volunteers__c >= 1)
hrExisting.Number_of_Volunteers__c = vrs.Number_of_Volunteers__c;
if (vrs.Volunteer_Hours_Status__c != null)
hrExisting.Status__c = vrs.Volunteer_Hours_Status__c;
hrExisting.Planned_Start_Date_Time__c = datetime.newInstance(shift.Start_Date_Time__c.date(), vrs.Schedule_Start_Date_Time__c.time());
if (vrs.Comments__c != null)
hrExisting.Comments__c = vrs.Comments__c;
listHoursUpdate.add(hrExisting);
}
// remove it from the map so we know it was matched
mapShiftIdContactIdToHours.remove(shift.Id + '|' + vrs.Contact__c);
continue;
}

Expand Down Expand Up @@ -216,7 +283,7 @@ public with sharing class VOL_VRS {
hr.Comments__c = vrs.Comments__c;
listHoursNew.add(hr);
//cDesiredVols--;
}
}
}

// we let the caller commit the new hours to the db.
Expand Down
Loading

0 comments on commit 8cc81ad

Please sign in to comment.