+ Start a Discussion
anup-prakashanup-prakash 

Trigger Issue

Hi I have writen a trigger and it is not achieving what I want it to achieve.

 

I have a master-detail relation between a Prod__c(Master) and Prod_Detail__c (Detail)

Prod__c  has the following Fields:

Name,

Manufacturer__c

Description__c

 

Prod_Detail__c has the following Fields:

Prod__c (reference)

Name (labeled Lot Name)

Quantity__c

______________________________

 

what I am wanting to do with the help of trigger is to check if there is a duplicate entry on Prod_Detail__c  if there is then

It should update the quantity sum .. in case of insert as well as update.. this is what I have written but its still adding a duplicate record record and updating the field as well. I am stuck..

trigger CheckDuplicateBatchNumber on Prod_Detail__c (before insert, before update) {
	
	map<String, Prod_Detail__c> pdMap = new map<String, Prod_Detail__c>();  // map of match number to product name
	
	//check Duplicate Entry
	for(Prod_Detail__c pd : Trigger.new){
		if(trigger.isInsert || trigger.oldmap.get(pd.id).Name != pd.Name){
			String pdName = pd.Name.toUpperCase();
			if(!(pdMap.containskey(pdName))){

				pdMap.put(pdName,pd);
			}
			else if(pdMap.containskey(pdName)){ //update Quantity
				Prod_Detail__c pdObject = new Prod_Detail__c();
				pdObject = pdMap.get(pdName);
				pdObject.Quantity__c = pd.Quantity__c + pdObject.Quantity__c ;
				pdMap.put(pdName,pdObject);
			}
		}
	}
	
	list<Prod_Detail__c> upsertList = new list<Prod_Detail__c>();
	
	if(pdMap.size()!= Null){
		for(String pName : pdMap.keyset()){ 
			for(Prod_Detail__c pd: [Select ID, Name, Quantity__c, MRP__c, Prod__c, Unit_Price__c from Prod_Detail__c where Name IN :pdMap.keySet()]){
				if((pd.Name.equalsIgnoreCase(pName))&&(pd.Prod__c == pdMap.get(pName).Prod__c)){
					pd.Quantity__c = pd.Quantity__c + pdMap.get(pName).Quantity__c ;
					pd.Name = pd.name.toUpperCase();
					upsertList.add(pd);
					continue;
					
				}
				else {
					String nam;
					Prod_Detail__c pOb = pdMap.get(pName);
					nam = pOb.Name.toUpperCase();
					pOb.Name = nam;
					upsertList.add(pOb);
				}
			}
		}
		upsert(upsertList);
	} 
}

 

MellowRenMellowRen

You’ve almost answered your own question. Without being too simplistic you simply haven't put anything in this trigger to prevent the new record(s) from being saved. No matter what happens they will be committed to the database. All you are doing is updating the current records in the database.

 

Check out this link’s use of the addError() method as one way to stop a record saving. In this case, it won't help you do what you need though. Can I suggest a slight rethink? Have your trigger increase the quantity of the "new" record with the value of the original ones and instead of building an upsertlist build a deletelist. Use it to remove the originals whist these inserted/updated records replace them (with increased quantity). It should achieve what you want with the added advantage that you avoid a potential infinite loop—you are currently calling an update inside an update trigger which although the top bit of code should prevent it looping it is not good practice if you can aviod it.

 

Regards

MellowRen

anup-prakashanup-prakash
Thanks For the Reply MellowRen.. So Even If I delete the record and create an updated list with the increased qty.. And you said not to use the update inside the trigger?? How can I achieve it???
MellowRenMellowRen

anup-prakash

 

In a before trigger there is no need to create an update list, just modify the trigger.new values and let the system update them once the trigger has finished. For example:

 

trigger CheckDuplicateBatchNumber on Prod_Detail__c (before insert, before update) {
    
   for (Prod_Detail__c pd : allRelatedPDs) {
       pd.Quantity__c = pd.Quantity__c + 1;
   }
}

This code would update the quantity of all inserted/updated Prod_Detail__c records by one. Note, no update list or, more importantly, upsert command neccissary.This is also why your code is updating the quantities but still creating the records. My suggestion was since the trigger.new records will be updated anyway, change their quantities if needed and then delete the duplicates (using a delete command).

 

That being said, it occurs to me that my suggestion won't work in a batch situation where multiple inserts for the same kind are involved (a problem you also identified and had a crack at in your code).

 

I'll get back to you.

 

Regards

MellowRen

 

 

MellowRenMellowRen

OK, I think the best way to get around the multiple insert problem is to do this in an after trigger. Unfortunately despite my try-and-avoid-updating-within-an-update-trigger rule, that is exactly what is needed to be done.

 

BTW, you can't modify trigger.new records in an after trigger like you can in a before trigger.

 

So here is how I would tackle your problem (as a start):

 

trigger CheckDuplicateBatchNumber on Prod_Detail__c (after insert, after update) {
    
   //Get IDs of related Prods then get all Details belonging to them.
   Set<Id> prodID = new Set<Id>();
   for (Prod_Detail__c pd : Trigger.new) {
        prodID.add(pd.prod__c);
   }
   List<Prod_Detail__c> allRelatedPDs = [SELECT ID, Name, Quantity__c, Prod__c, Prod__r.Id FROM Prod_Detail__c WHERE Prod__r.Id IN :prodID];

   //Create a master map (ie the PDs to be updated if there are duplicates) and a matching set (for easy comparison).
   Map<String, Prod_Detail__c> pdMasterMap = new Map<String, Prod_Detail__c>();  
   Set<Prod_Detail__c> pdMasterSet = new Set<Prod_Detail__c>();  
   for (Prod_Detail__c pd : allRelatedPDs) {
       String pdName = pd.Name.toUpperCase();
       pdMasterMap.put((String)pd.Prod__r.Id + pdName,pd);
    }
    pdMasterSet.addAll(pdMasterMap.values());
   
    //Create an update map and delete list.
    Map<Id,Prod_Detail__c> pdToUpdateMap = new Map<Id, Prod_Detail__c>();
    List<Prod_Detail__c> pdToDeleteList = new List<Prod_Detail__c>();
    
    //Populate
    for (Prod_Detail__c pd : allRelatedPDs) {
  
        String ref = (String)pd.Prod__r.Id + pd.Name.toUpperCase();
        //why won’t "pdMasterSet.contains(pd)" work?
        Boolean inMasterset = false;
        for (Prod_Detail__c mastPd : pdMasterMap.values()) {
            if (mastPd.Id == pd.Id) inMasterset = true;
        }
    
        if (! inMasterset) {
             // String ref = (String)pd.Prod__r.Id + pd.Name.toUpperCase();
             Id masterID = pdMasterMap.get(ref).Id;
             if (! pdToUpdateMap.containsKey(masterID)) {
                 pdToUpdateMap.put(masterID,pdMasterMap.get(ref));
             }
             pdToUpdateMap.get(masterID).Quantity__c = pdToUpdateMap.get(masterID).Quantity__c + pd.Quantity__c;
             pdToDeleteList.add(pd);
          }
    }

    //Commit to Database.
    if (pdToDeleteList != null) {
        delete(pdToDeleteList); // This must be done first to stop the update from causing an infinite loop.
        update(pdToUpdateMap.values());
    }
}

 Two things I can think of immediately.

 

  1. Although this code won't go into an infinite loop (because I separated the pdToUpdateMap from the allRelatedPDs), it will run through completely twice, which is a bit of a waste. Wrapping it with a recurring trigger test will save you some CPU time.
  2. This, as it is currently written, will create an "ugly" user experience.The way I am creating the master map is pretty random so sometimes the record the user is updating may be the one selected to be deleted—so although the result will be correct, the user will go "huh?". Also, even if not deleted there is no feedback to the user as to why the quantity changed. Assuming of course there is any user interaction with these objects.

I'll leave these to you to modify as you see fit. 

 

Good luck.

MellowRen