You need to sign in to do that
Don't have an account?

Adapt Trigger for Mass Load of Data
This code works just fine for individual record updates, but I'm trying to get it to work for a mass upload of data. Any insights?
trigger TRG_SurveyFeedback_UpdateParentAccounts on Survey_Feedback__c (before insert, before update) { Set<ID> setAcctId = new Set<ID>(); //Holds the Account Id from the Survey Feedback List<Account> currentAccount = new List<Account>(); //Holds Related Account on Survey Feedback List<Account> parentAccount = new List<Account>(); //Holds list of Parent Accounts for Related Account String relAcctId = ''; //Related Account Id Integer topAccount = 0; //Holds number of top-most account in parentAccount list Boolean boolParentAssnd = false; //Is Parent Account Assigned yet? for (Survey_Feedback__c sfb: Trigger.new){ setAcctId.add(sfb.Account__c); } currentAccount = [SELECT Id, Name FROM Account WHERE Id in :setAcctId]; relAcctId = currentAccount[0].Id; //Get the list of Accounts in the hierarchy for relAcctId. parentAccount = getAccountHierarchy(relAcctId); /** getAccountHeirarchy(string) @description The getAccountHierarchy function, which requires an accountId as a paramenter, fetches the list of accounts appearing above the accountId in he hierarchy. It returns a list (accList) that contains the following information for the Account: Id, Name, ParentId, and Location_Type__c */ List<Account> getAccountHierarchy(String accountId){ List<Account> accList = new List<Account>(); //Holds list of accounts above accountId Boolean isTop = false; //Is this the top-most account? while (!isTop){ Account a = [SELECT Id, Name, ParentId, Location_Type__c FROM Account WHERE Id =: accountId limit 1]; if (a.ParentId != null){ accountId = a.ParentId; }else{ isTop = true; } accList.add(a); } return accList; } //Get the index of the top-most account in parentAccount topAccount = parentAccount.size() - 1; for(Survey_Feedback__c sfb: Trigger.new){ //Note: start with index 1, because index 0 is the Related Account. for(Integer i = 1; i < parentAccount.size(); i++){ if(parentAccount[i].Location_Type__c == 'Parent' && boolParentAssnd == false){ sfb.Parent_Account__c = parentAccount[i].Id; boolParentAssnd = true; } } if(parentAccount[topAccount].Location_Type__c == 'Ultimate Parent' && parentAccount[topAccount].Id != relAcctId){ sfb.Ultimate_Parent_Account__c = parentAccount[topAccount].Id; } } }
Hi Jeff,
Thats kinda what I thought, here is some code that should help you out.
Please note this was done in notepad++ so may not compile straight away (could need a few little tweeks) but should be pretty close.
Trigger Code:
Note: As you can see all complexity has been removed from the trigger it's self. This will enable you to easily extend the trigger.
Helper Class (SurveyFeedback) Code:
Note: All the complexity has been moved into a class where we can now create tests eaisily without even inserting records.
Helper Class (Account) Code:
Note: I have also moved the account method into an account helper, this will be good for reuse in the future. It will also make it easier for testing and easier to maintain.
Conclusion
This new version should work reasonably well, but you should be aware of query limits in the getUltimateParentAccount method because if you have a vast hierarchy or decide to dataload aload of SurveyFeedback records you could still hit limits here.
I would suggest updating this in the future to include a field on an account called ultimate_Parent__c and move this into the account trigger, because I am sure that you create (or update parent accounts) on an account record less than you insert and update the values on the SurveyFeedback records.
Please note, if you do dataload the records you can reduce the batch size to avoid the limits being hit.
Maybe if I had a better view of your system I could have provided a better solution but I just hope this helps ;)
All Answers
Hi Jeff,
This is not very good design for a trigger, before I provide corrections it may be worth you providing a little explination as to what you are trying to achieve here, because I have a feeling this could be done in a more efficient way.
In the meantime, some useful points on trigger design:
- where possible have 1 trigger per object (this makes it easier to manage exicution order)
- always keep logic out of the trigger in a helper class (this is more maintainable easier to debug and better for testing).
- aviod queries in loops (this is always important in any apex code, as you quickly reach limits like this and it is just not very efficient). The best way to deal with this in your example would be to collect all the accIds and then do a single query, returning the results in a map for easy access.
I hope you find this useful, and if you provide a description of what you are trying to achieve I will try and respond to you later today.
The purpose is simply this:
A custom object called Survey Feedback is attached to the Account object. It contains a Master-Detail field for Account called Related_Account__c and then two lookup fields for Parent_Account__c and Ultimate_Parent_Account__c. The trigger is simply meant to go and find the first parent account above Related_Account__c and append that Id into Parent_Account__c, then find the top-most account, determine if it is the Ultimate Parent and then append that value, if applicable, into Ultimate_Parent_Account__c.
I understand that it's not the most pretty looking trigger, because it's in its initial stage of being developed. I like to work through my whole design and then break things up. I'm also being rushed, so even through that shouldn't be an excuse, it's also causing additional angst in this process.
Still need a little better understanding of Maps, so any insights you can provide are helpful. Thanks!
Hi Jeff,
Thats kinda what I thought, here is some code that should help you out.
Please note this was done in notepad++ so may not compile straight away (could need a few little tweeks) but should be pretty close.
Trigger Code:
Note: As you can see all complexity has been removed from the trigger it's self. This will enable you to easily extend the trigger.
Helper Class (SurveyFeedback) Code:
Note: All the complexity has been moved into a class where we can now create tests eaisily without even inserting records.
Helper Class (Account) Code:
Note: I have also moved the account method into an account helper, this will be good for reuse in the future. It will also make it easier for testing and easier to maintain.
Conclusion
This new version should work reasonably well, but you should be aware of query limits in the getUltimateParentAccount method because if you have a vast hierarchy or decide to dataload aload of SurveyFeedback records you could still hit limits here.
I would suggest updating this in the future to include a field on an account called ultimate_Parent__c and move this into the account trigger, because I am sure that you create (or update parent accounts) on an account record less than you insert and update the values on the SurveyFeedback records.
Please note, if you do dataload the records you can reduce the batch size to avoid the limits being hit.
Maybe if I had a better view of your system I could have provided a better solution but I just hope this helps ;)
One error I'm getting is the following:
Illegal assignment from LIST<Account> to MAP<Id,Account>
This is for this section of code (line 25) in SurveyFeedbackHelper:
Unfortunately, MAPS confound me a little bit. I'm no idiot, either, it may just be that the documentation out there feels nebulous to me and I would still consider myself a beginner in much of this. My past experiences are some Java and a lot of VB script.
Sorry Jeff,
I expected an issue or two since I wrote in notepad, so it was never compiled.
it should be:
When you do a query, you have the option of returning it into a map, as shown above. This will give you the record Id as the key and the record it's self as the value.
Thanks, I completely understand. Meanwhile, this seems to be working at the moment. I had to make one tweak to the Account Helper, but it was more of a business rule type of thing. Here are the updated helper classes.
For SurveyFeedbackHelper, ALL Survey Feedback records received have to be evaluated, since they are coming into the system for the first time from ClickTools. There isn't going to be a change, since it's the first time the system is seeing the records. The reason I include 'before update' is for the off-chance that something does happen to the account or anything else on the record, so that everything can be updated. Also, if a field is added later, we want to be able to update all historical records where needed.
The only thing that I have left is to troubleshoot my test class. It's now telling me that I'm trying to dereference a NULL object.
Hi Jeff,
First thing, I would suggest keeping the check to see if the BU has changed on the SurveyFeedbackHelper. The reason I suggest this is because you call this on before update. So for example if you were to say add a workflow to the record with a field update or change the owner or even update some random field on there. Without the check there it will fire off the method and then eventualy call the get ultimate parent method which as you know calls queries in a while loop (which is likely to hit query limits). Also it is good practice in triggers to check for the change in values before carrying out actions because you do not want to endup in some infinate loop between a field update from a workflow and the trigger update.
Just a warning, hope this makes sence.
Now I have had my rant about best practice onto the test failure.
This is essentialy saying that the account you are trying to retrieve in the ultimate parent method does not exist in the database.
If you share your test class I could take a look and explain how to resolve.
Sure thing:
Hi Jeff,
Update the following:
The issue is that your test does not create a parent account, and the code doesn't handle that very well. If you make the change added above the test should run.
Not sure if there is a need to check for the acctLocType? I assume if the parentId is null it will be an ultimate parent?
Anyway hope that gets you up and running!
Ok, this was the perfect DUH! moment for me after seeing your change. However, I could have sworn that my code did create a parent account via WizardTestHelper.createSiteAccount(); That method has a method within it that accesses createParentAccount() and then createUltParentAccount(). Maybe I'm wrong, though.
Meanwhile, there are times where the top account is not always an Ultimate Parent, which is why I'm having it do a check. We're actively cleaning our TCA (Trading Community Architecture) to get this fixed, but unti we do I still have to check, just in case.
That's great Jeff,
Glad we could get it sorted.
If you we happy with the help I provided and the notes on trigger best practices please provide kudos by clicking on the little blue star to the left of the post. Providing kudos is a great way to help people trust advice provided by community members and highlight useful posts for other community members who are having similar trouble.
It appears that the error is still happening. It doesn't have to do with the AccountHelper class, it's happening on line 14 of the SurveyFeedbackHelper class:
Is this a "chicken or the egg" type of situation?
Sorry, this was me missing the obvious. You can only use oldMap on update and delete triggers, not on insert.
If you removed the check to see if the value had changed, you can just replace oldMap with newMap here.
If you decided to keep it you will need to pass an additional boolean into the method to say if you are in insert or update, and only do the check if is in update (in either case you are ok to replace the oldMap with newMap on this loop).
Hope this helps, sorry for missing that. It was a late night!
Yes, but I'm reading that newMap is only available in before update, after insert, and after update triggers, which means I will still be dereferencing a null object, and I can't have the trigger fire on an after insert, because the record is read-only at that point. I'm going to have to rethink this through.
I think I figured out what to do. I changed the trigger to this. Hoping it works.
Hi Jeff,
If you are not using the check on update, just replace the code with the following. (Note: If you do use the check you may need two methods).
Trigger Code:
Helper Class (SurveyFeedback) Code:
Sorry for the need to refactor there. This should do the trick!
Not quite, I'm troubleshooting the bottom portion of the SurveyFeedbackHelper. Had to change this:
back to this:
Now I just have to fix this:
Hi jeff,
Here is the correction:
Lets hope its right this time. I don't know how I missed the map bit on the query. It would be easier if I compiled the code before sharing, but tI can't :(
Sorry!
You beat me to posting it, but I had actually figured this part out on my own for once. Having worked more with lists, I realized that I had to replace the map piece and step through the list instead. I think this is going to work out now. I just have some final testing to do on my end with loading records.
If everything works out, I will post the final working code for all of this discussion. Thanks for all of the guidance and patience. I have actually learned a LOT about maps in this whole process, not to mention some better disciplines moving forward.
Thanks!
Here are my results after testing.
It works OK if you're updating an existing record, but if you're adding a set of new records, feedback.id is null, so it assigns the same parent account to the whole list of them, regardless.
This is the one thing I think I'm going to have to solve, myself, unfortunately.
Hi Jeff,
This has been an intresting one!
The following should resolve the issue, though it may add a few extra script statments:
Note: Doing this you can get rid of the map.
Good luck!