Trigger to update Multi-Select Picklist

Question

I created a trigger to update a multi-select field on the Account object with the values from a multi-select field on child Opportunities. If values already exist in this field on the account then the values should be kept and only new values from the Opportunity need to be added.

The trigger for this which works as expected when a few values are selected on the Multi-select. But for some reason when many values are selected (e.g. 5+) the values are added to the Account field twice.

public static void updateAccountProducts(List<Opportunity> opps){ 
    Map<Id, String> mp = new Map<Id, String>();
    for(Opportunity o: opps){
        if(o.AccountId != null && o.Products__c != null){
            mp.put(o.AccountId, o.Products__c);
        }
    }

    List<Account> acctLst = [SELECT Id, Products_Purchased__c FROM Account WHERE ID IN :mp.KeySet()];

    for(Account a: acctLst){
        if(mp.ContainsKey(a.Id)){
           if(a.Products_Purchased__c != null){
               a.Products_Purchased__c = mp.get(a.Id).remove(a.Products_Purchased__c) +';'+ a.Products_Purchased__c;
           } else {
               a.Products_Purchased__c = mp.get(a.Id);
           }
        } 
    } 
    update acctLst;
} 

I am using string.remove() to remove the values that already exist before updating the account.

a.Products_Purchased__c = mp.get(a.Id).remove(a.Products_Purchased__c) +';'+ a.Products_Purchased__c;

Am I overlooking something that can be causing duplicate values to be added in certain situations? Maybe there is a better way to do this?

Answer 1

I think there are some improvements that I can suggest.

Caution: Multiple opportunities have the same account.

Since you are using a map of accountId and opportunity products, the values for only the last opportunity in the list will be stored.

Updated code with comments. It may not compile because it is typed on the notepad but you will get a general idea. The key here is to use a Set to store values, which will remove any duplicates automatically.

public static void updateAccountProducts(List<Opportunity> opps) {
    Map<Id, Set<String>> mp = new Map<Id, Set<String>>();
    for (Opportunity o : opps) {
        if (o.AccountId != null && String.isNotBlank(o.Products__c)) {
            //This will take care of multiple opportunities being associated with one account
            if (!mp.containsKey(o.AccountId)) {
                mp.put(o.AccountId, new Set<String>());
            }
            // Add all products here from all child opportunities
            mp.get(o.AccountId).addAll(o.Products__c.split(';'));
        }
    }

    List<Account> acctLst = [SELECT Id, Products_Purchased__c FROM Account WHERE ID IN :mp.KeySet()];

    for (Account a : acctLst) {
        if (mp.ContainsKey(a.Id)) {
            Set<String> products = mp.get(a.Id);
            if (String.isNotBlank(a.Products_Purchased__c)) {
                // Add existing account products so that they are not lost.
                products.addAll(a.Products_Purchased__c.split(';'));
            }
            //Now update the field.
            List<String> productsList = new List<String>();
            productsList.addAll(products);
            a.Products_Purchased__c = String.join(productsList, ';');
        }
    }
    update acctLst;
}