Trigger ineffective - no errors received.. it just doesnt work!

Question

Im very new to apex or indeed any program language to try not to laugh!

Criteria given to me = if a case is of record type 'travel agent' and the case contact field is populated, update that contacts record custom field 'Luxury_Agent_ID__c' with the Contacts ID.

This is my trigger, no errors are being received but it is not effective. Can anyone advise where I am going wrong, I suspect it is in the latter part of the trigger but I have fiddled about to no avail...

trigger HeroIdfromCase on Case (after insert, after update) {
    Set<Id> LuxuryCaseIds = new Set<Id>(); 
        Id recordTypeId = Schema.SObjectType.Case.RecordTypeInfosByName.get('Travel Agent').RecordTypeId;

    for (Case c: Trigger.new) {
        if( c.RecordTypeId== recordTypeId) 
            // if case record type is 'Travel Agent'
        { 
            if( c.Contact != null)  {
                // AND if the case contact  field is populated 
                LuxuryCaseIds.add (c.id);
        }

        }
         List<Contact> ContactsForUpdating = [SELECT Luxury_Agent_ID__c
                                                       FROM Contact
                                                       WHERE Id  IN: LuxuryCaseIds];
         for (Contact LX: ContactsForUpdating) {
        LX.Luxury_Agent_ID__c = 'LX.Id';
    }
    update ContactsForUpdating;
  }
}

Add few debug statements to your code and check the debug logs. It will help you in analyzing the issue. — Rohit Patil, Oct 23 '15 at 10:41
thank you Rohit.. im not really sure what that is yet but will look it up and see what I can do so now it reads as per the next comment — Louisa Pidcock, Oct 23 '15 at 10:53
Process builder could accomplish this, no need for a trigger. — Eric, Oct 23 '15 at 13:49

score 1 · Accepted Answer · edited Apr 13 '17 at 12:54

One issue I can see is that you are storing Case Ids in LuxuryCaseIds with this line:

 LuxuryCaseIds.add (c.id);

You then try to use that list of Case Ids to find contacts which will never return any records.

List<Contact> ContactsForUpdating = [SELECT Luxury_Agent_ID__c
                                                   FROM Contact
                                                   WHERE Id  IN: LuxuryCaseIds];

If you want to build a list of Contact Ids then you most likely want to do something like this:

luxuryContactIds.add (c.ContactId);

Adding debugging to your code will help you understand what your code is doing and will help you with issues like this in the future. Have a look at How do I start to debug my own Apex code? to get started with debugging your code.

i didnt change the variable name but did add the 'Contact' part to the id ( luxuryCaseIds.add (c.ContactId);)- and now it works!! thank you ever so much!! — Louisa Pidcock, Oct 23 '15 at 11:31

score 0 · Answer 2 · answered Oct 23 '15 at 11:29

Looking at your code and brief, I think you're trying to do this:

trigger HeroIdfromCase on Case (after insert, after update) {
    // renamed this to contactIds, as that is what you are collecting
    Set<Id> contactIds = new Set<Id>();
    Id recordTypeId = Schema.SObjectType.Case.RecordTypeInfosByName.get('Travel Agent').RecordTypeId;

    for (Case c: Trigger.new) {
        if( c.RecordTypeId== recordTypeId) {  // if case record type is 'Travel Agent'
            if( c.ContactId != null)  {
                // add to list of all the Contacts that will need Updating
                contactIds.add(c.ContactId);
            }
        }
    }

    // using the list of Contact IDs, query the Contact object so you can retrieve the Luxury_Agent_ID__c and update it. Note, this has been moved out of your
    // main loop to 'bulkify' the code (i.e. it does not perform a query within a loop as before)
    List<Contact> contactsForUpdating = [SELECT Id, Luxury_Agent_ID__c FROM Contact WHERE Id IN :contactIds];

    // loop the list of contacts you've just retreived and update the Luxury_Agent_ID__c
    for (Contact contact: contactsForUpdating) {
        contact.Luxury_Agent_ID__c = contact.Id;
    }
    update contactsForUpdating;
}

I've provided some comments in the code to try and explain, but I would google salesforce trigger bulkification and also look into some of the online resources for Apex development, Apex is not a particularly hard language to learn but the many (many) salesforce-isms can trip you up.

thanks for this Phil - i really must get my head around bulkification.. ive been an admin for 6 years and trying my hand at code so im quite nervous — Louisa Pidcock, Oct 23 '15 at 11:34
have updated my terminology as you suggest.. I am just confusing myself otherwise — Louisa Pidcock, Oct 23 '15 at 11:36

score 0 · Answer 3 · answered Oct 23 '15 at 12:31

I see 4 things that should be fixed. Three will make your code not work, the fourth will make it more efficient.

The literal reference to 'LX.Id' will fail.
The null check on the Contact field would be better as c.ContactId
There is also a problem with how you use the LuxuryCaseIds set.

if( c.ContactId != null) { // AND if the case contact field is populated LuxuryCaseIds.add (c.id); }

Here's the thing. You actually need to populate this with the ContactId field.

So instead, do this:

if( c.ContactId != null)  {
  // AND if the case contact  field is populated 
  LuxuryCaseIds.add (c.ContactId);
}

There is one final thing...you catually can save yourself your trip to the DB (your SOQL query).

If you always want to copy this Contact ID value, every time, no matter what, you can do this:

List<Contact> contactsToUpdate = new List<Contact>();

for (Case c: Trigger.new){
  if( c.RecordTypeId== recordTypeId && c.ContactId != null)  {
    // AND if the case contact  field is populated 
    Contact aContact = newContact();
    aContact.Id = c.ContactId;
    aContact.Luxury_Agent_Id__c = c.ContactId;
    contactsToUpdate.add(aContact);
  }
update contactsToUpdate;

This works because Apex doesn't care whether the record for your sObject is actually in the DB, when you set it up to update, as long as it has an Id. And you don't need to query for the Luxury_Agent_Id__c value, because you know what it is going to be set to.

There is one slightly inefficient drawback to this strategy, you are always updating those contact records, regardless of whether the value was already there.

Trigger ineffective - no errors received.. it just doesnt work!

3 Answers3