function readOnly(count){ }
Sign Up ›
Starting November 20, the site will be set to read-only. On December 4, 2023,
forum discussions will move to the Trailblazer Community.
Learn More

Browse by Topic
ShowAll Questionssorted byDate Posted
+ Start a Discussion
NPanterNPanter 

Absolute newbie - help with simple Apex trigger

Hello all,

 

While I have tonnes of experience with salesforce, my apex previously has been asking my developer.  Thought I would try a simple trigger, or what I thought would be a simple trigger.  Basically, this is what I want it to do:

 

On Cases there is a field called Case_Survey_Sent_Date__c 

On Contacts there is a field called Case_Survey_Sent_Date__c 

 

After the case is saved, I would like to check if the case.Case_Survey_Sent_Date__c is more recent then the contact.Case_Survey_Sent_Date__c and if so, update it.  This is what i have come up with, and lo and behold I am sucking it huge.  Here is what I have so far:

 

trigger updateAccountSurveyDate on Case (after update) {

     Case c;
     for (Contact p = [SELECT id, Case_Survey_Sent_Date__c FROM Contact])
     {
          if (p.Case_Survey_Sent_Date__c < c.Case_Survey_Sent_Date__c){
              p.Case_Survey_Sent_Date__c = c.Case_Survey_Sent_Date__c;
          } else {
               //
          }      
     }
}

 Now, take a few minutes to compose yourself after the fits of laughter of what I have done...would any of you gurus be able to point me in the error of my ways, or even give me a little to go on?

 

Thanks all,

 

Nik :smileyhappy:

Best Answer chosen by Admin (Salesforce Developers) 
David SchachDavid Schach

Here's how I would do it.  Note that you are evaluating Case and updating Contact - so you need to do two separate groups of actions.  First you compare dates and figure out which Contacts need to be updated.  Then you query those Contacts and do what you must to them.

 

trigger updateAccountSurveyDate on Case (after update) {

	map<id, Date> Contact_Date_map = new map<id, Date>();

	for(Case c : [SELECT id, Case_Survey_Sent_Date__c, ContactId, Contact.Case_Survey_Sent_Date__c FROM Case c WHERE Id IN :Trigger.New]){
		Date D1 = c.Case_Survey_Sent_Date__c;
		Date D2 = c.Contact.Case_Survey_Sent_Date__c;
		if(D1.daysBetween(D2) < 0 ){  // This is TRUE if case date is AFTER contact date
		// could also put if(D2.daysBetween(D1) > 0 )
			Contact_Date_map.put(c.ContactId, D1); // we have our ContactId and the new (more recent) date
		}
     }

	if(Contact_Date_map.keyset().size() > 0){
		List<Contact> contactsToUpdate = new List<Contact>();
		for(Contact c : [SELECT id, Case_Survey_Sent_Date__c FROM Contact WHERE Id IN :Contact_Date_map.keyset()]){
			c.Case_Survey_Sent_Date__c = Contact_Date_Map.get(c.id);
			contactsToUpdate.add(c);
		}
		try{
			update contactsToUpdate;
		} catch (DMLException e){
			system.debug('ERROR UPDATING CONTACTS: ' + e);
		}
	}
}

 I hope that makes sense and that I read your requirements properly.  Because you use "after update" you must be performing DML on another object.  If you were updating the Case record itself and just using the Contact record as a reference, you would use a "before update" trigger and the code will be twice as long.

 

And about bulkifying: There is NO reason that a trigger should not be bulkified.  Avoiding this just to save a few lines of code is sloppy.  Yes, I said it.  Sloppy.  

 

Next, you get to write your test code.  Have fun!  Don't forget that you need to work with Account (insert), Contact (insert), Case (insert), Case (update) and do that twice to account for both possible date comparisions (Case after Contact and Contact after Case).  Good luck!

 

PS.  I didn't check this code in production, so it may have typos and such.  Apologies in advance!

All Answers

David SchachDavid Schach
Good start. My style isto use the Date.daysBetween(Date) method. So...

If (Date1.daysBetween(Date2) > 0 ) {
Date2 = Date1;
}

I may have had those backwards, but I hope you get the general intent.

Hope that helps!
David SchachDavid Schach
Oh, i missed a huge thing:
Iterate over Trigger.new

For ( Case c : select id, date1, contactId, contact.Date2 from Case where id in: Trigger.new){
if ...
}

Too much to type on an iPad. Will get back to you ASAP...
colemabcolemab

I think you want to loop thru each of your cases, which would look something like this:

 

for(Case c:Trigger.new) {
  // do something here - var for current case is 'c'
}

 

Inside of this case, you will want to query the DB to find the contact related to the case.  The query for this would look something like this:

 

contact CurrentCaseContact = [SELECT Id, Case_Survey_Sent_Date__c, FROM Contact WHERE Id = :c.ContactId LIMIT 1];

 
David SchachDavid Schach
Not a best practice to query inside a for loop.
colemabcolemab

Agreed but he didn't ask for the unicorn of perfect.  He asked for help to get it working and this is a simple way to get it working.     Also given that the query only ever returns one row (due to limit and select by id), this should avoid hitting the query limits for the trigger.   I wait to see your solution and see how many more lines of code it is and how many lists/collections/maps it uses ........

 

EDIT:

 

Sorry but this guy is trying to learn the code, then get it to work. Once it works (and you understand it) then you can worry about best practices.  Can't skip from step A directly to step Z.

 

In any event, here is a great write up of best practices.   #2 addresses the query in a loop hitting limit (when 100 or more records hit the trigger at a time).
David SchachDavid Schach

Here's how I would do it.  Note that you are evaluating Case and updating Contact - so you need to do two separate groups of actions.  First you compare dates and figure out which Contacts need to be updated.  Then you query those Contacts and do what you must to them.

 

trigger updateAccountSurveyDate on Case (after update) {

	map<id, Date> Contact_Date_map = new map<id, Date>();

	for(Case c : [SELECT id, Case_Survey_Sent_Date__c, ContactId, Contact.Case_Survey_Sent_Date__c FROM Case c WHERE Id IN :Trigger.New]){
		Date D1 = c.Case_Survey_Sent_Date__c;
		Date D2 = c.Contact.Case_Survey_Sent_Date__c;
		if(D1.daysBetween(D2) < 0 ){  // This is TRUE if case date is AFTER contact date
		// could also put if(D2.daysBetween(D1) > 0 )
			Contact_Date_map.put(c.ContactId, D1); // we have our ContactId and the new (more recent) date
		}
     }

	if(Contact_Date_map.keyset().size() > 0){
		List<Contact> contactsToUpdate = new List<Contact>();
		for(Contact c : [SELECT id, Case_Survey_Sent_Date__c FROM Contact WHERE Id IN :Contact_Date_map.keyset()]){
			c.Case_Survey_Sent_Date__c = Contact_Date_Map.get(c.id);
			contactsToUpdate.add(c);
		}
		try{
			update contactsToUpdate;
		} catch (DMLException e){
			system.debug('ERROR UPDATING CONTACTS: ' + e);
		}
	}
}

 I hope that makes sense and that I read your requirements properly.  Because you use "after update" you must be performing DML on another object.  If you were updating the Case record itself and just using the Contact record as a reference, you would use a "before update" trigger and the code will be twice as long.

 

And about bulkifying: There is NO reason that a trigger should not be bulkified.  Avoiding this just to save a few lines of code is sloppy.  Yes, I said it.  Sloppy.  

 

Next, you get to write your test code.  Have fun!  Don't forget that you need to work with Account (insert), Contact (insert), Case (insert), Case (update) and do that twice to account for both possible date comparisions (Case after Contact and Contact after Case).  Good luck!

 

PS.  I didn't check this code in production, so it may have typos and such.  Apologies in advance!

This was selected as the best answer
NPanterNPanter

Wow - you guys are great!

 

To x2od, thanks for pointing out the best way to do this.  While I sometimes rush my thinking, it is nice to see a best practice solution.  And to Colemab - your points were excellent as well for the quick and dirty way.  A tip of the hat to both of you.

 

And x2od, not one spelling mistake!

 

Now, if anyone is looking for me over the next few days, I'll be figuring out a test case for this!  

:smileyvery-happy:
colemabcolemab

To be clear, I wasn't saying that a quick and dirty solution was the end of the road.  It was just a step in understanding how to do it the right way.   Given the orignal code, it was clear you didn't have a full grasp of what needed to be done and I was trying to step you thru the process of learning the right way.  Often doing it the wrong way is part of learning the right way.   I didn't want to jump to the solution and skip the steps in between only to see similar posts asking for code later.  Teach a man to fish vs. giving him fish type thing.

 

Glad you got it resolved though.