+ Start a Discussion
Renuka SharmaRenuka Sharma 

Not sure if Trigger Context variables is required or not for the below trigger

Hello all

i am very new to the apex development, i am written a trigger where it gives extra sharing rule to the user when the CRM user__c field is populated, i have written a test class where it gives 91% coverage, according to the my manager this is not a best pratices because there are not trigger context variables, can anyone please point out what needs to be updated
 
trigger OppShareWithCRMOwner on Opportunity (after insert,after update){
    List<OpportunityShare> csShareList = new List<OpportunityShare>();
    List<OpportunityShare> removeShareList = new List<OpportunityShare>();
    Set<Id> OPPids = new Set<Id>();
    for( Opportunity cs : trigger.new ) {
        if( cs.CRM_User__c != NULL ) {
            // Create a new LeadShare object for each Lead where CRM_User__c field is not NULL.
            OpportunityShare csShare = new OpportunityShare();
            // Give Read write access to that user for this particular Lead record.
            csShare.OpportunityAccessLevel = 'Read';
            // Assign Lead Id of Lead record.
            csShare.OpportunityId = cs.id;
            // Assign user id to grant read write access to this particular Lead record.
            csShare.UserOrGroupId = cs.CRM_User__c;
            csShareList.add( csShare );
        }
        if( cs.CRM_User__c == NULL ) {
            OPPids.add( cs.id);
        }
    }
    
    if(OPPids.size()>0){
        removeShareList = [Select id,OpportunityId,UserOrGroupId from OpportunityShare where OpportunityId=:OPPids AND RowCause = 'Manual'];
        if(removeShareList.size()>0){
            delete removeShareList;
        }
    }
    if( csShareList != null && csShareList.size() != 0 ) {
        try {
            insert csShareList;
            update csShareList;
            
            System.debug('size of my list is: ' +csShareList.size());
 
        }catch( Exception e ) {
            trigger.new[0].CRM_User__c.addError('Error::::::'+e.getMessage());
        }
    }
}

Test class
 
@isTest
private class OppShareWithCRMOwnerTestClass {

    // test that newly inserted records marked as pubic=true have corresponding shares created
    static testMethod void testAddShares() {

    Set<ID> ids = new Set<ID>();
    Account testAcct = new Account (Name = 'My Test Account');
     insert testAcct;
    List<Opportunity> Opps = new List<Opportunity>();
        User u = [Select id,name from user where isactive=true Limit 1];
    for (Integer i=0;i<10;i++)
      Opps.add(new Opportunity(Name='First',AccountId = testAcct.ID,
      StageName = 'Demo',CloseDate = System.today(),LeadSource = 'Customer',Country__c = 'India',CRM_User__c=u.Id));

    insert Opps;

    // get a set of all new created ids
    for (Opportunity c : Opps)
      ids.add(c.id);

    // assert that 50 shares were created
    List<OpportunityShare> shares = [select id from OpportunityShare where 
      OpportunityId IN :ids and RowCause = 'Manual'];
    System.assertEquals(shares.size(),10);

    }

    // insert records and switch them from public = true to public = false
    static testMethod void testUpdateContacts() {

        Set<ID> ids = new Set<ID>();
        
        Account testAcct = new Account (Name = 'My Test Account');
        insert testAcct;
        List<Opportunity> Opps = new List<Opportunity>();
        User u = [Select id,name from user where isactive=true Limit 1];
        
        for (Integer i=0;i<10;i++)
            Opps.add(new Opportunity(Name='First',AccountId = testAcct.ID,
                                     StageName = 'Demo',CloseDate = System.today(),LeadSource = 'Customer',Country__c = 'India',CRM_User__c=u.Id));

    insert Opps;

    for (Opportunity c : Opps)
      ids.add(c.id);

    update Opps;

    // assert that 0 shares exist
    List<OpportunityShare> shares = [select id from OpportunityShare where 
      OpportunityId IN :ids and RowCause = 'Manual'];
    System.assertNotEquals(shares.size(),0);

    for (Opportunity c : Opps)
      c.CRM_User__c=u.Id;

    update Opps;

    // assert that 50 shares were created
    shares = [select id from OpportunityShare where OpportunityId IN :ids and RowCause = 'Manual'];
    System.assertEquals(shares.size(),10);

    for (Opportunity c : Opps)
      c.CRM_User__c=u.Id;

    update Opps;

    // assert that 0 shares exist
    shares = [select id from OpportunityShare where OpportunityId IN :ids and RowCause = 'Manual'];
    System.assertNotEquals(shares.size(),0);

    }
}

 
PRAKASH JADA 13PRAKASH JADA 13

Hi,

https://developer.salesforce.com/forums/?id=906F0000000DBl8IAG
This link will help you to understand more about the triggers. 


Apart from that in your code, there are non-writable fields and 2 DMLS on each record. Instead of using the insert and update you can use upsert.