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
Garrett MillerGarrett Miller 

Update Field Between Account and Opportunity

Hi All, 

I am trying to get used to writing triggers doing a fairly simple trigger but I am stuck. 

I am trying to write a trigger where te scenario is:

  1.  Two objects Account and Opportunity
  2. Account field Renewal Term (Renewal_Term__c)that is picklist of years annual through sexennial
  3. Opportunity Field Contract Length ( Contract_Length__c ) that is a picklist of values 1-6
  4. I believe that there is a master-detail relationship between Account and Opportunity
  5. When an Opportunity is Closed Won, Renewal Term should update to the corresponding contract length from the Opportunity, provided that Renewal Term isn't already populated. 

Embarrassingly, all I have so far is:

 

    

trigger updatelength on Account (after update) {
    Opportunity__c[] opptoupdate = new Opporunity__c[]{};
        for(account c: trigger.new){
            for(Opportunity__c cp: [select id,name,Contract_Length__c from Opportunity__c where Account = :c.id]{

            }
        }
}


Would anyone mind helping me out?

Thanks!

Best Answer chosen by Garrett Miller
HarmpieHarmpie

You could probably solve this without a trigger, using the Process Builder. But regardless of that, keep the following in mind (some were mentioned before):

  • Account and Opportunity are standard objects, no need for __c
  • Trigger should be bulkified. Currently all sample code in this thread has a SOQL query in the inner loop, which will cause trouble
  • Try to create more generic triggers, not a trigger per use case (e.g. updatelength). Ideally 1 trigger per object
  • Separate logic from the trigger. I.e. create a separate class for the business logic and call it from the trigger. You don't want a trigger full of code.

You should end up with something similar to the simplified example below.

Trigger

trigger OpportunityTrigger on Opportunity (after update) {
    OpportunityHelper.setRenewalTerm(Trigger.newMap.keySet());
}
Class 
public class OpportunityHelper {
    public static void setRenewalTerm(Set<ID> oppIds) {
        List<Account> modifiedAccounts = new List<Account>();
        for(Opportunity o : [SELECT Id, Account.Renewal_Term__c, Contract_Length__c FROM Opportunity WHERE Id IN :oppIds AND IsClosed=true]) {
        	if(o.Account.Renewal_Term__c==null) {
        		o.Account.Renewal_Term__c = o.Contract_Length__c;
        		modifiedAccounts.add(o.Account);
        	}
        }
        update modifiedAccounts;
    }
}

 

All Answers

NRD 4 UNRD 4 U
Hi Garrett,

As per description it loooks like you want to udpate opportunity on the basis of opportunity update (clsed or not).
1. When opportunity is closed renewal term should update hence you need to write a trigger on Opportunity not on account. Before udpate should work here. 
trigger updatelength on Opportunity(before update)
2. Other thing you should keep in mind that you need to run this trigger only when Opportunity is closed.
3. Next you need to define what value should get udpated on the basis of renewal term like fi renewal term is A then length is 1. and then put conditions accordingly.


 
NRD 4 UNRD 4 U
trigger updatelength on opportunity(before update) {
    list<Opportunity> opptoupdate = new list<Opporunity>();
        for(opportunity c: trigger.new){
if(c.isClosed == True && (c.Contract_Length__c!=null || c.Contract_Length__c=='')){
            for(Account acc: [select id,name,renewal__c from Account where AccountId = :c.AccountId]){

//Condition how you want to update
c.Contract_Length__c=acc.renewal__c ;

            }
        }
}
}

note that opportunity and account are standard objects. Hence no need to use "__c"
HarmpieHarmpie

You could probably solve this without a trigger, using the Process Builder. But regardless of that, keep the following in mind (some were mentioned before):

  • Account and Opportunity are standard objects, no need for __c
  • Trigger should be bulkified. Currently all sample code in this thread has a SOQL query in the inner loop, which will cause trouble
  • Try to create more generic triggers, not a trigger per use case (e.g. updatelength). Ideally 1 trigger per object
  • Separate logic from the trigger. I.e. create a separate class for the business logic and call it from the trigger. You don't want a trigger full of code.

You should end up with something similar to the simplified example below.

Trigger

trigger OpportunityTrigger on Opportunity (after update) {
    OpportunityHelper.setRenewalTerm(Trigger.newMap.keySet());
}
Class 
public class OpportunityHelper {
    public static void setRenewalTerm(Set<ID> oppIds) {
        List<Account> modifiedAccounts = new List<Account>();
        for(Opportunity o : [SELECT Id, Account.Renewal_Term__c, Contract_Length__c FROM Opportunity WHERE Id IN :oppIds AND IsClosed=true]) {
        	if(o.Account.Renewal_Term__c==null) {
        		o.Account.Renewal_Term__c = o.Contract_Length__c;
        		modifiedAccounts.add(o.Account);
        	}
        }
        update modifiedAccounts;
    }
}

 
This was selected as the best answer
Garrett MillerGarrett Miller

Thanks @Harmpie! I was able to use your trigger and class successfully once I added in my Case conditions. To @NRD 4 U I appreciate the help as I can see how yours would work too. Do either of you have any suggestions on the Test Class?

What I have so far is:
 

@isTest
private class OpportunityHelperTestClass {
static testMethod void validateOpportunityHelper() 
{
    Account testAcct = new Account (Name = 'My Test Account');
    insert testAcct;
    
    Opportunity oppt = new Opportunity();
    oppt.Name = 'New Deal';
    oppt.AccountID = testAcct.ID;
    oppt.Contract_Length__c = '1';
    oppt.Amount = 3000;
    oppt.CloseDate = System.today();
    insert oppt;
    
    oppt.Contract_Length__c = '1';
    update oppt;
    
    oppt.Contract_Length__c = '2';
    update oppt;
    
    oppt.Contract_Length__c = '3';
    update oppt;
    
    oppt.Contract_Length__c = '4';
    update oppt;
    
    oppt.Contract_Length__c = '5';
    update oppt;
    
    oppt.Contract_Length__c = '6';
    update oppt;
    }
}

But I am unsure how to actually perform the check against the Account.Renewal_Term__c. Any suggestions would be appreciated!
HarmpieHarmpie

Didn't test compilation, so might get some syntax errors, but this should get you on the right track

 

@isTest
private class OpportunityHelperTestClass {

    @testSetup
    static void createTestData() {
        //
        // Create a few accounts to test with
        List<Account> allAccounts = new List<Account>();
        for(Integer x=0; x < 50; x++) {
            Account testAcct = new Account (Name = 'My Test Account '+x);
            allAccounts.add(testAcc);
        }
        insert allAccounts;
    }
    static testMethod void validateOpportunityHelper() {
        //
        // Set up the list of opportunities
        List<Account> testAccounts = [SELECT Id, renewal__c FROM Account WHERE Name like 'My Test Account%'];
        List<Opportunity> allOpps = new List<Opportunity>();
        for(Account a : testAccounts) {
            //
            // With 50 Accounts, this will total 300 Opportunities
            for(Integer y = 1; y < 7; y++) {
                Opportunity oppt = new Opportunity();
                oppt.Name = 'New Deal '+y;
                oppt.AccountID = a.Id;
                oppt.Contract_Length__c = String.valueOf(y);
                oppt.Amount = 3000;
                oppt.CloseDate = System.today();
                allOpps.add(oppt);

            }
        }

        test.startTest();
        //
        // Insert the opportunities after starttest() to test limits
        insert allOpps;
        
        test.stopTest();

        //
        // Verify results
        List<Opportunity> opps = [SELECT Id, Name, Contract_Length__c, Account.renewal__c FROM Opportunity WHERE Name LIKE like 'New Deal%'];
        Integer totalRecs = opps.size();
        System.assertEquals(300, totalRecs, 'Expected 300 Opportunities in database');

        for(Opportunity opp : opps) {
            System.assert(opp.Contract_Length__c == opp.Account.renewal__c, opp.Contract_Length__c+' is not equal to '+opp.Account.renewal__c);
        }
    }
}
Garrett MillerGarrett Miller
So I ran this test class and still have 0% code coverage. I am not sure what I could be doing wrong. 
Garrett MillerGarrett Miller

Here is my actual working class;

 

public class OpportunityHelper {
    public static void setRenewalTerm(Set<ID> oppIds ) {
        list<Account> modifiedAccounts = new List<Account>();
        for(Opportunity o: [SELECT Id, Account.Renewal_Term__c, Contract_Length__c FROM Opportunity WHERE Id IN: oppIds AND IsClosed=true]) {
            if(o.Account.Renewal_Term__c==null){
                if (o.Contract_Length__c == '1'){
                  o.Account.Renewal_Term__c = 'Annual';
                } else if (o.Contract_Length__c == '2') {
                  o.Account.Renewal_Term__c = 'Biennial';          
                } else if (o.Contract_Length__c == '3') {
                    o.Account.Renewal_Term__c = 'Triennial';  
                } else if (o.Contract_Length__c == '4') {
                    o.Account.Renewal_Term__c = 'Quadrennial';
                } else if (o.Contract_Length__c == '5') {
                    o.Account.Renewal_Term__c = 'Quinquennial';
                } else if (o.Contract_Length__c == '6') {
                    o.Account.Renewal_Term__c = 'Sexennial';
                } 
                modifiedAccounts.add(o.Account);
                
            }
        }
        update modifiedAccounts;
    }
}
NRD 4 UNRD 4 U
Garrett,

Your trigger seems to be called on update but the test class doesnt have any update calls.

In test class, close the opportuntities means change the status to closed and update the contract term. It should call your trigger.
HarmpieHarmpie
Small mistake in my code, like NRD said you need an update, my code assumes an insert
Garrett MillerGarrett Miller

Ah makes sense thanks @NRD 4 U 

I have an error: Update Failed. First Exception on row 0; first error: MISSING_ARGUMENT, Id not specified in an update call: []

I have changed the code so that it is:

List<Account> testAccounts = [ SELECT Id, Renewal_Term__c FROM Account WHERE Name like 'My Test Account%'];
    List<Opportunity> allOpps = new List<Opportunity>();
        for(Account a : testAccounts) {
            for (Integer y = 1; y < 7; y++) {
                Opportunity oppt = new Opportunity();                
                oppt.Name = 'New Deal '+y;
                oppt.AccountID = a.Id;
                oppt.Contract_Length__c = String.valueOf(y);
                oppt.Amount = 3000;
                oppt.StageName = 'Closed Won';
                oppt.Evaluating_Other_Solutions__c = 'Aprimo';
                oppt.CloseDate = System.today();
                allOpps.add(oppt);
                
            }
        }
    test.startTest();
        
    update allOpps;
    
    test.stopTest();
        
    List<Opportunity> opps = [SELECT Id, Name, Contract_Length__c, Account.Renewal_Term__c FROM Opportunity WHERE Name like 'New Deal%'];
	Integer totalRecs = opps.size();
	System.assertEquals(300, totalRecs, 'Expected 300 Opportunities in database');

        for(Opportunity opp : opps) {
            System.assert(opp.Contract_Length__c == opp.Account.Renewal_Term__c, opp.Contract_Length__c+' is not equal to '+opp.Account.Renewal_Term__c);
                }
        
          }        
    
    }

So that it updates allOpps instead of insert allOpps, but this error is throwing me for a loop as the Id appears to be specified. 
NRD 4 UNRD 4 U
List<Account> testAccounts = [ SELECT Id, Renewal_Term__c FROM Account WHERE Name like 'My Test Account%'];
    List<Opportunity> allOpps = new List<Opportunity>();
        for(Account a : testAccounts) {
            for (Integer y = 1; y < 7; y++) {
                Opportunity oppt = new Opportunity();                
                oppt.Name = 'New Deal '+y;
                oppt.AccountID = a.Id;
                oppt.Contract_Length__c = String.valueOf(y);
                oppt.Amount = 3000;
                oppt.StageName = 'Prospecting'; //Here you can give any initial stage name
                oppt.Evaluating_Other_Solutions__c = 'Aprimo';
                oppt.CloseDate = System.today();
                allOpps.add(oppt);
                
            }
        }
    test.startTest();
        
    Insert allOpps;

Opportunity UpdateOpp = [select Id, Name, CloseDate,StageName from opportunity where Name like '%1' limit 1];
UpdateOpp.stageName='Closed Won';
update updateOpp;
    
    test.stopTest();
        
    List<Opportunity> opps = [SELECT Id, Name, Contract_Length__c, Account.Renewal_Term__c FROM Opportunity WHERE Name like 'New Deal%'];
	Integer totalRecs = opps.size();
	System.assertEquals(300, totalRecs, 'Expected 300 Opportunities in database');

        for(Opportunity opp : opps) {
            System.assert(opp.Contract_Length__c == opp.Account.Renewal_Term__c, opp.Contract_Length__c+' is not equal to '+opp.Account.Renewal_Term__c);
                }
        
          }        
    
    }
Hi Garrett,

Try this.
You should first insert and then update.
 
Garrett MillerGarrett Miller
Wouldn't this just update one Opp? 
Garrett MillerGarrett Miller
@NRD 4 U
NRD 4 UNRD 4 U
Hi Garrett,

For unit testing you will have to insert the data first. Means you need to create your own data on which you will perform unit testing. hece you will have to first insert and then update. Both are requried for creating unit test.
Garrett MillerGarrett Miller
So I don't need to update all 300 opportunities? 
NRD 4 UNRD 4 U
Exactly, if you update only one record, that will work. One update call will cover your condition. you need to check for the condition /scenarios which you have given in your Apex class and satisfy those conditions through one record.
Garrett MillerGarrett Miller
OHHHHH, wow OK. Here I was thinking I had to test every Opportunity. This made writing test code make so much more sense. I was able to reach 100% coverage. Thanks! 
NRD 4 UNRD 4 U
Awesome, So you have become a developer/ Coder now :P

Enjoy.
Garrett MillerGarrett Miller
Yeah right, long way to go haha