So I'm new to writing Apex Classes and Triggers - and I know I still have to "bulkify" my code but I can't understand why the logic isn't working here. Basically I'm trying to make an trigger on opportunity that loops through the contacts associated, and sets a contact custom field "In_Pipe__c" to "true" if the contact has any opportunities "New (Renewable)" and not closed. Not sure what I'm missing here but any help would be appreciated! Thanks!!

trigger Is_In_Pipe_v3 on Opportunity (after update, after insert) {
for (Opportunity currentOpp : Trigger.new) {
    List<OpportunityContactRole> oppContact = [SELECT ContactID FROM OpportunityContactRole WHERE OpportunityID = :currentOpp.Id];
        for (OpportunityContactRole currentContact : oppContact) {


            List<Opportunity> oppList =  [ SELECT Id, Name
                                From Opportunity
                                Where Id IN (SELECT OpportunityId
                                    FROM OpportunityContactRole
                                    WHERE ContactId = :currentContact.Id)
                                AND Type = 'New (Renewable)'
                                AND StageName != 'Closed Won'
                                AND StageName != 'Closed Lost' ];
             Contact thisContact = [SELECT Id From Contact Where Id IN (SELECT ContactId FROM OpportunityContactRole)];

    if (oppList.size() > 0) {
        thisContact.InPipe__c = true;
    }
    else {
        thisContact.InPipe__c = false;
    }
}

} }

Attribution to: amizzo

Possible Suggestion/Solution #1

You aren't calling update on thisContact :) You should add each contact you are updating to a list of contacts and then call update listOfContacts. Be sure to handle any exceptions with a try/catch block.

Attribution to: Kyle

Possible Suggestion/Solution #2

The challenge with triggers across multiple objects is that there are many cases to consider such as:

• the Opportunity changing to match the condition
• the Opportunity changing to not match the condition
• the Opportunity being deleted
• the OpportunityContactRole being inserted or updated
• the OpportunityContactRole being deleted

A trigger of the following form will (I think - tests need adding to be more sure) handle the first two. (If all 5 conditions are reasonable you will need to write further triggers for those.)

trigger Is_In_Pipe_v3 on Opportunity (after update, after insert) {
    // Find the Opportunites where the key fields have changed
    Set<Id> changedOppIds = new Set<Id>();
    for (Opportunity newOpp : Trigger.new) {
        Opportunity oldOpp = Trigger.oldMap.get(newOpp.Id);
        Boolean isNew = newOpp.Type == 'New (Renewable)'
                && newOpp.StageName != 'Closed Won'
                && newOpp.StageName != 'Closed Won';
        Boolean isOld = oldOpp.Type == 'New (Renewable)'
                && oldOpp.StageName != 'Closed Won'
                && oldOpp.StageName != 'Closed Won';
        if (isOld != isNew) {
            changedOppIds.add(newOpp.Id);
        }
    }
    if (changedOppIds.size() > 0) {
        List<Contact> contacts = new List<Contact>();
        for (AggregateResult ar : [
                select ContactId i, count(OpportunityId) c
                from OpportunityContactRole
                where OpportunityId in :changedOppIds
                and Opportunity.Type = 'New (Renewable)'
                and Opportunity.StageName not in ('Closed Won', 'Closed Lost')
                group by ContactId
                ]) {
           Id contactId = (Id) ar.get('i');
           Integer oppCount = (Integer) ar.get('c');
           contacts.add(new Contact(Id = contactId, InPipe__c = oppCount > 0));
        }
        update contacts;
    }
}

The "bulkification" is that the work is minimised by only looking at specific changes to Opportunity, there are no queries inside loops, and the minimum amount of looping is done in Apex (by making use of the databases aggregation capability). Performance will be pretty linear depending on how many Contacts are affected by the change, rather than rising exponentially as more Contacts are affected because of nested loops.

The above code is making use of Relationship Queries. These allow you to query across multiple objects in one go, reducing the need to connect together separate queries in code. If you are going to be writing many triggers, it is worth understanding these well.

The other important technique to get used to when writing triggers is the use of maps and sets; see e.g. Common Bulk Trigger Idioms.

Attribution to: Keith C
This content is remixed from stackoverflow or stackexchange. Please visit https://salesforce.stackexchange.com/questions/33604