Return to Answer

edited body

edited Jul 2, 2018 at 18:46

trigger updateCase on Case (before update) { Set <Id> QIds = new Set<Id>(); List<Id> caseid = new List <Id>(); List<RecordType> rc = [SELECT Id, Name FromFROM RecordType WhereWHERE sObjectType = 'case' and Id = :oldCase.RecordTypeId LIMIT 1]; List<Group> gc = [SELECT Id, Name FROM Group WHERE Id =:Trigger.new[0].ownerid and Type = 'Queue']; List<Group> g = [SELECT Id, Name FROM Group WHERE Type = 'Queue']; //Get the Ids of the different Queues Id MarketingRecordTypeId = Schema.SObjectType.case.getRecordTypeInfosByName().get('Marketing').getRecordTypeId(); Id GTGroupLeadsRecordTypeId = Schema.SObjectType.case.getRecordTypeInfosByName().get('G&T Group Leads').getRecordTypeId(); Id CCSCRecordTypeId = Schema.SObjectType.case.getRecordTypeInfosByName().get('CCSC').getRecordTypeId(); Id EcommerceRecordTypeId = Schema.SObjectType.case.getRecordTypeInfosByName().get('E-Commerce').getRecordTypeId(); Case oldCase = new Case(); for (Case c:trigger.new){ oldCase = trigger.oldMap.get(c.Id); } for( Group qu :g){ if(qu.Name == 'DS Service Queue' || qu.Name == 'Data Services RFP' || qu.Name == 'Data Services') QIds.add(qu.Id); } //Loop through all Cases and check owner change system.debug('oldrecord type==>+' + oldCase.RecordTypeId ); for (Case c:trigger.new){ oldCase = trigger.oldMap.get(c.Id); system.debug('oldCase'+ oldCase); system.debug('RecordTypename' + oldCase.RecordType.name); // Guest Assistance Project Requirments - Auto stamp Department and Sub-Department when a GA case is re-assigned to other Queues if (oldCase.OwnerId != c.OwnerId && QIds.contains(oldCase.OwnerId)){ c.Status = 'Open'; if (c.Category__c == null || c.Sub_Department__c == null){ c.addError('You must provide a value for Category and Sub Department before changing the Case Owner'); } } for(RecordType rt : rc) { system.debug('Record Type name ==>' + rt.Name); if(rt.Name == 'Guest Assistance') { System.debug( 'RecordTypeIds ==>' + 'Marketing==>' + MarketingRecordTypeId + ' ' + 'G&T Group Leads==>' + GTGroupLeadsRecordTypeId + ' ' + 'CCSC==>' + CCSCRecordTypeId + ' ' + 'E-Commerce ==>' + EcommerceRecordTypeId); for( Group qu :gc){ if (qu.name == 'Marketing Queue') { c.RecordTypeId = MarketingRecordTypeId; c.Department__c = 'Marketing'; c.Sub_Department__c = 'Marketing'; c.Category__c = 'Website Feature'; } if (qu.name == 'G&T Service Queue' || Test.isRunningTest() ) { c.RecordTypeId = GTGroupLeadsRecordTypeId; c.Department__c = 'Group & Tour'; c.Sub_Department__c = 'Group & Tour'; c.Category__c = 'Group Event'; } if (qu.name == 'CCSC Queue'||Test.isRunningTest()) { c.RecordTypeId = CCSCRecordTypeId; c.Department__c = 'Guest Assistance'; c.Sub_Department__c = 'Guest Assistance'; c.Category__c = null ; } if (qu.name == 'E-Commerce'||Test.isRunningTest()) { c.RecordTypeId = EcommerceRecordTypeId; c.Department__c = 'E-Commerce'; c.Sub_Department__c = 'E-Commerce'; c.Category__c = 'Other'; } } } else if ((rt.Name == 'Marketing') || (rt.Name == 'G&T Group Leads') || (rt.Name == 'CCSC') || (rt.Name == 'E-Commerce'|| Test.isRunningTest())){ Id GuestAsstRecordTypeId = Schema.SObjectType.case.getRecordTypeInfosByName().get('Guest Assistance').getRecordTypeId(); for( Group qu :gc){ if (qu.name == 'Guest Assistance Queue') { c.RecordTypeId = GuestAsstRecordTypeId; c.Department__c = 'Guest Assistance'; c.Sub_Department__c = 'Guest Assistance'; c.Category__c = null; } } } } } }

trigger updateCase on Case (before update) { Set <Id> QIds = new Set<Id>(); List<Id> caseid = new List <Id>(); List<RecordType> rc = [SELECT Id, Name From RecordType Where sObjectType = 'case' and Id = :oldCase.RecordTypeId LIMIT 1]; List<Group> gc = [SELECT Id, Name FROM Group WHERE Id =:Trigger.new[0].ownerid and Type = 'Queue']; List<Group> g = [SELECT Id, Name FROM Group WHERE Type = 'Queue']; //Get the Ids of the different Queues Id MarketingRecordTypeId = Schema.SObjectType.case.getRecordTypeInfosByName().get('Marketing').getRecordTypeId(); Id GTGroupLeadsRecordTypeId = Schema.SObjectType.case.getRecordTypeInfosByName().get('G&T Group Leads').getRecordTypeId(); Id CCSCRecordTypeId = Schema.SObjectType.case.getRecordTypeInfosByName().get('CCSC').getRecordTypeId(); Id EcommerceRecordTypeId = Schema.SObjectType.case.getRecordTypeInfosByName().get('E-Commerce').getRecordTypeId(); Case oldCase = new Case(); for (Case c:trigger.new){ oldCase = trigger.oldMap.get(c.Id); } for( Group qu :g){ if(qu.Name == 'DS Service Queue' || qu.Name == 'Data Services RFP' || qu.Name == 'Data Services') QIds.add(qu.Id); } //Loop through all Cases and check owner change system.debug('oldrecord type==>+' + oldCase.RecordTypeId ); for (Case c:trigger.new){ oldCase = trigger.oldMap.get(c.Id); system.debug('oldCase'+ oldCase); system.debug('RecordTypename' + oldCase.RecordType.name); // Guest Assistance Project Requirments - Auto stamp Department and Sub-Department when a GA case is re-assigned to other Queues if (oldCase.OwnerId != c.OwnerId && QIds.contains(oldCase.OwnerId)){ c.Status = 'Open'; if (c.Category__c == null || c.Sub_Department__c == null){ c.addError('You must provide a value for Category and Sub Department before changing the Case Owner'); } } for(RecordType rt : rc) { system.debug('Record Type name ==>' + rt.Name); if(rt.Name == 'Guest Assistance') { System.debug( 'RecordTypeIds ==>' + 'Marketing==>' + MarketingRecordTypeId + ' ' + 'G&T Group Leads==>' + GTGroupLeadsRecordTypeId + ' ' + 'CCSC==>' + CCSCRecordTypeId + ' ' + 'E-Commerce ==>' + EcommerceRecordTypeId); for( Group qu :gc){ if (qu.name == 'Marketing Queue') { c.RecordTypeId = MarketingRecordTypeId; c.Department__c = 'Marketing'; c.Sub_Department__c = 'Marketing'; c.Category__c = 'Website Feature'; } if (qu.name == 'G&T Service Queue' || Test.isRunningTest() ) { c.RecordTypeId = GTGroupLeadsRecordTypeId; c.Department__c = 'Group & Tour'; c.Sub_Department__c = 'Group & Tour'; c.Category__c = 'Group Event'; } if (qu.name == 'CCSC Queue'||Test.isRunningTest()) { c.RecordTypeId = CCSCRecordTypeId; c.Department__c = 'Guest Assistance'; c.Sub_Department__c = 'Guest Assistance'; c.Category__c = null ; } if (qu.name == 'E-Commerce'||Test.isRunningTest()) { c.RecordTypeId = EcommerceRecordTypeId; c.Department__c = 'E-Commerce'; c.Sub_Department__c = 'E-Commerce'; c.Category__c = 'Other'; } } } else if ((rt.Name == 'Marketing') || (rt.Name == 'G&T Group Leads') || (rt.Name == 'CCSC') || (rt.Name == 'E-Commerce'|| Test.isRunningTest())){ Id GuestAsstRecordTypeId = Schema.SObjectType.case.getRecordTypeInfosByName().get('Guest Assistance').getRecordTypeId(); for( Group qu :gc){ if (qu.name == 'Guest Assistance Queue') { c.RecordTypeId = GuestAsstRecordTypeId; c.Department__c = 'Guest Assistance'; c.Sub_Department__c = 'Guest Assistance'; c.Category__c = null; } } } } } }

trigger updateCase on Case (before update) { Set <Id> QIds = new Set<Id>(); List<Id> caseid = new List <Id>(); List<RecordType> rc = [SELECT Id, Name FROM RecordType WHERE sObjectType = 'case' and Id = :oldCase.RecordTypeId LIMIT 1]; List<Group> gc = [SELECT Id, Name FROM Group WHERE Id =:Trigger.new[0].ownerid and Type = 'Queue']; List<Group> g = [SELECT Id, Name FROM Group WHERE Type = 'Queue']; //Get the Ids of the different Queues Id MarketingRecordTypeId = Schema.SObjectType.case.getRecordTypeInfosByName().get('Marketing').getRecordTypeId(); Id GTGroupLeadsRecordTypeId = Schema.SObjectType.case.getRecordTypeInfosByName().get('G&T Group Leads').getRecordTypeId(); Id CCSCRecordTypeId = Schema.SObjectType.case.getRecordTypeInfosByName().get('CCSC').getRecordTypeId(); Id EcommerceRecordTypeId = Schema.SObjectType.case.getRecordTypeInfosByName().get('E-Commerce').getRecordTypeId(); Case oldCase = new Case(); for (Case c:trigger.new){ oldCase = trigger.oldMap.get(c.Id); } for( Group qu :g){ if(qu.Name == 'DS Service Queue' || qu.Name == 'Data Services RFP' || qu.Name == 'Data Services') QIds.add(qu.Id); } //Loop through all Cases and check owner change system.debug('oldrecord type==>+' + oldCase.RecordTypeId ); for (Case c:trigger.new){ oldCase = trigger.oldMap.get(c.Id); system.debug('oldCase'+ oldCase); system.debug('RecordTypename' + oldCase.RecordType.name); // Guest Assistance Project Requirments - Auto stamp Department and Sub-Department when a GA case is re-assigned to other Queues if (oldCase.OwnerId != c.OwnerId && QIds.contains(oldCase.OwnerId)){ c.Status = 'Open'; if (c.Category__c == null || c.Sub_Department__c == null){ c.addError('You must provide a value for Category and Sub Department before changing the Case Owner'); } } for(RecordType rt : rc) { system.debug('Record Type name ==>' + rt.Name); if(rt.Name == 'Guest Assistance') { System.debug( 'RecordTypeIds ==>' + 'Marketing==>' + MarketingRecordTypeId + ' ' + 'G&T Group Leads==>' + GTGroupLeadsRecordTypeId + ' ' + 'CCSC==>' + CCSCRecordTypeId + ' ' + 'E-Commerce ==>' + EcommerceRecordTypeId); for( Group qu :gc){ if (qu.name == 'Marketing Queue') { c.RecordTypeId = MarketingRecordTypeId; c.Department__c = 'Marketing'; c.Sub_Department__c = 'Marketing'; c.Category__c = 'Website Feature'; } if (qu.name == 'G&T Service Queue' || Test.isRunningTest() ) { c.RecordTypeId = GTGroupLeadsRecordTypeId; c.Department__c = 'Group & Tour'; c.Sub_Department__c = 'Group & Tour'; c.Category__c = 'Group Event'; } if (qu.name == 'CCSC Queue'||Test.isRunningTest()) { c.RecordTypeId = CCSCRecordTypeId; c.Department__c = 'Guest Assistance'; c.Sub_Department__c = 'Guest Assistance'; c.Category__c = null ; } if (qu.name == 'E-Commerce'||Test.isRunningTest()) { c.RecordTypeId = EcommerceRecordTypeId; c.Department__c = 'E-Commerce'; c.Sub_Department__c = 'E-Commerce'; c.Category__c = 'Other'; } } } else if ((rt.Name == 'Marketing') || (rt.Name == 'G&T Group Leads') || (rt.Name == 'CCSC') || (rt.Name == 'E-Commerce'|| Test.isRunningTest())){ Id GuestAsstRecordTypeId = Schema.SObjectType.case.getRecordTypeInfosByName().get('Guest Assistance').getRecordTypeId(); for( Group qu :gc){ if (qu.name == 'Guest Assistance Queue') { c.RecordTypeId = GuestAsstRecordTypeId; c.Department__c = 'Guest Assistance'; c.Sub_Department__c = 'Guest Assistance'; c.Category__c = null; } } } } } }

Source Link

answered Jul 2, 2018 at 18:40

JaredT

I'm no Apex expert, nor coding expert in general. I do deal with Apex triggers quite a bit, both my own and those written by others.

Ultimately, I don't see any practices which break any of the first 8 commandments except maybe Thou shalt keep logic outside of triggers. It's pretty easy to toss all this into an Apex class and call that class from the trigger itself.

You didn't post your test class, which is fine so long as you actually have meaningful tests running for this code.

Where I am concerned is #15 Thou shalt not introduce extra logic for tests. You've got a lot of if there is a test running, change these values going on, which may provide you a false sense of confidence. Maybe this is part of a more grand testing scheme that I don't see, but from what's in front of me you should reconsider this practice.

Low Hanging Fruit

Your code is formatted pretty inconsistently which makes it hard to read. Stick to opening your curly braces either on the same line or on the next line, don't go back and forth.
Keep an eye on your indenting, that's worse than the curly brace thing.
Are you really saving much time by typing MarkeRecoTyId instead of MarketingRecordTypeId? If I went down a hundred lines of code, would I, the new guy know that Marke refers to Marketing and not Market?
Be thoughtful about naming your variables. I, the new guy, have no idea what gc refers to here:

gc = [select id, Name from Group where id =:Trigger.new[0].ownerid and Type = 'Queue'];

This takes me an extra minute or two when I get to a line like this: for( Group qu :gc){ ... } where I say, "bwuhh?" because those hieroglyphics mean nothing to anyone but the person who wrote the code.

Apex is not case-sensitive but many code editors themes rely on it for syntax highlighting. I plugged your trigger into VSCode using the popular Apex extension and things like list and Select did not get picked up by the highlighter. Again, this comes back to readability.
You can use the SOQL result to instantiate the lists which I dare not infer the meaning of, i.e.: List<RecordType> rc = [SELECT Id, Name From RecordType Where sObjectType = 'case' and Id = :oldCase.RecordTypeId LIMIT 1];

Line length. You've got a 201 character wide line:

 System.debug('RecordTypeIds ==>'+ 'Marketing==>' + MarkeRecoTyId + ' ' + 'G&T Group Leads==>'+ GTGrLRecoTyId + ' ' + 'CCSC==>' + CCSCRecoTyId + ' ' + 'E-Commerce ==>' + EcomRecoTyId );

I know it's just a debug statement, but imagine if you had to keep scrolling left to right to figure out where some of those variables with arcane names came from or what they are doing. Isn't this much easier to read?

 System.debug( 'RecordTypeIds ==>' + 'Marketing==>' + MarketingRecordTypeId + ' ' + 'G&T Group Leads==>' + GTGroupLeadsRecordTypeId + ' ' + 'CCSC==>' + CCSCRecordTypeId + ' ' + 'E-Commerce ==>' + EcommerceRecordTypeId);

Putting all of these points into practice (except renaming your 1-2 character variables, I still have no clue what those are supposed to be except maybe Group for g), here is a much happier looking trigger.

trigger updateCase on Case (before update) { Set <Id> QIds = new Set<Id>(); List<Id> caseid = new List <Id>(); List<RecordType> rc = [SELECT Id, Name From RecordType Where sObjectType = 'case' and Id = :oldCase.RecordTypeId LIMIT 1]; List<Group> gc = [SELECT Id, Name FROM Group WHERE Id =:Trigger.new[0].ownerid and Type = 'Queue']; List<Group> g = [SELECT Id, Name FROM Group WHERE Type = 'Queue']; //Get the Ids of the different Queues Id MarketingRecordTypeId = Schema.SObjectType.case.getRecordTypeInfosByName().get('Marketing').getRecordTypeId(); Id GTGroupLeadsRecordTypeId = Schema.SObjectType.case.getRecordTypeInfosByName().get('G&T Group Leads').getRecordTypeId(); Id CCSCRecordTypeId = Schema.SObjectType.case.getRecordTypeInfosByName().get('CCSC').getRecordTypeId(); Id EcommerceRecordTypeId = Schema.SObjectType.case.getRecordTypeInfosByName().get('E-Commerce').getRecordTypeId(); Case oldCase = new Case(); for (Case c:trigger.new){ oldCase = trigger.oldMap.get(c.Id); } for( Group qu :g){ if(qu.Name == 'DS Service Queue' || qu.Name == 'Data Services RFP' || qu.Name == 'Data Services') QIds.add(qu.Id); } //Loop through all Cases and check owner change system.debug('oldrecord type==>+' + oldCase.RecordTypeId ); for (Case c:trigger.new){ oldCase = trigger.oldMap.get(c.Id); system.debug('oldCase'+ oldCase); system.debug('RecordTypename' + oldCase.RecordType.name); // Guest Assistance Project Requirments - Auto stamp Department and Sub-Department when a GA case is re-assigned to other Queues if (oldCase.OwnerId != c.OwnerId && QIds.contains(oldCase.OwnerId)){ c.Status = 'Open'; if (c.Category__c == null || c.Sub_Department__c == null){ c.addError('You must provide a value for Category and Sub Department before changing the Case Owner'); } } for(RecordType rt : rc) { system.debug('Record Type name ==>' + rt.Name); if(rt.Name == 'Guest Assistance') { System.debug( 'RecordTypeIds ==>' + 'Marketing==>' + MarketingRecordTypeId + ' ' + 'G&T Group Leads==>' + GTGroupLeadsRecordTypeId + ' ' + 'CCSC==>' + CCSCRecordTypeId + ' ' + 'E-Commerce ==>' + EcommerceRecordTypeId); for( Group qu :gc){ if (qu.name == 'Marketing Queue') { c.RecordTypeId = MarketingRecordTypeId; c.Department__c = 'Marketing'; c.Sub_Department__c = 'Marketing'; c.Category__c = 'Website Feature'; } if (qu.name == 'G&T Service Queue' || Test.isRunningTest() ) { c.RecordTypeId = GTGroupLeadsRecordTypeId; c.Department__c = 'Group & Tour'; c.Sub_Department__c = 'Group & Tour'; c.Category__c = 'Group Event'; } if (qu.name == 'CCSC Queue'||Test.isRunningTest()) { c.RecordTypeId = CCSCRecordTypeId; c.Department__c = 'Guest Assistance'; c.Sub_Department__c = 'Guest Assistance'; c.Category__c = null ; } if (qu.name == 'E-Commerce'||Test.isRunningTest()) { c.RecordTypeId = EcommerceRecordTypeId; c.Department__c = 'E-Commerce'; c.Sub_Department__c = 'E-Commerce'; c.Category__c = 'Other'; } } } else if ((rt.Name == 'Marketing') || (rt.Name == 'G&T Group Leads') || (rt.Name == 'CCSC') || (rt.Name == 'E-Commerce'|| Test.isRunningTest())){ Id GuestAsstRecordTypeId = Schema.SObjectType.case.getRecordTypeInfosByName().get('Guest Assistance').getRecordTypeId(); for( Group qu :gc){ if (qu.name == 'Guest Assistance Queue') { c.RecordTypeId = GuestAsstRecordTypeId; c.Department__c = 'Guest Assistance'; c.Sub_Department__c = 'Guest Assistance'; c.Category__c = null; } } } } } }