Problem with trigger for inserting product to custom object from Opportunity Line Item?

Question

I created 2 custom objects Order and Order Line Item, they are same with Opportunity and Opportunity Line Item. Also Order and Opportunity has lookup, so every Opportunity has a related Order object. What I want is when I insert a product to Opportunity Line Item, same product to custom Order Line Item of related Order automatically inserted.

I have this trigger, it gives no error but nothing happens when I run, what am I missing?:

trigger CreateOrderLineItem on OpportunityLineItem (after insert, before update) {
    List<Order_Line_Item__c> oliList = new List<Order_Line_Item__c>();
    for(OpportunityLineItem opli : [Select opportunityid, Id, pricebookentryid, unitprice, quantity From OpportunityLineItem Where id IN:Trigger.newMap.keyset()])
    {
        for(Opportunity oppList : [Select id, Orders__c From Opportunity Where id=:opli.OpportunityId])
        {        
            for(PricebookEntry pbe : [Select pricebook2id, product2id, unitprice, name From PricebookEntry Where Id=:opli.PricebookEntryId])
            {
                for(Pricebook2 pb2 : [Select Id From Pricebook2 Where Id=:pbe.Pricebook2Id])
                {
                    for(Product2 pro : [Select id, Name, ProductCode, Order_Line_Items__c From Product2 Where Id =: pbe.product2id])
                    {
                        for(Order__c orrList : [Select Id, Opportunities__c From Order__c Where Opportunities__c =:oppList.id])
                        {
                            Order_Line_Item__c oli = new Order_Line_Item__c(Name='Opp-Product', Product2__c=pro.Id, UnitPrice__c=opli.UnitPrice, Quantity__c = opli.Quantity, Product_Name__c=pro.Name, Order__c=oppList.Orders__c, id = pro.Order_Line_Items__c);
                            oliList.add(oli);
                        }
                    }
                }
            }
        }
        if(oliList.size()>0){
            insert oliList;
            update oliList;
        }
    }
}

Answer 1

You've got many unnecessary queries, and of course, all the looping will cause your code to fail in situations where you're trying to load or mass update data (and, one should never say it won't happen, because it does, frequently). The correct solution involves the use of maps to load data efficiently.

As a side note, I don't see how you could possibly want to store the relationsthip to the order line item at the product level, since you are likely going to have more than one order at a time, and products will surely be used by multiple orders at the same time. You should move this lookup to the OpportunityLineItem level.

Here's a design you could use:

trigger OpportunityLineTrigger on OpportunityLineItem(before insert, before update) {
    // Variables
    Map<Id, Opportunity> opps = new Map<Id, Opportunity>();
    Map<Id, PricebookEntry> prices = new Map<Id, PricebookEntry>();
    map<opportunitylineitem, order_line_item__c> lines = new map<opportunitylineitem, order_line_item__c>();
    // Load initial data
    for(OpportunityLineItem record: Trigger.new) {
        opps.put(record.OpportunityId, null);
        prices.put(record.PricebookEntryId, null);
    }
    // Query data records
    opps.putAll([SELECT Id, Orders__c FROM Opportunity WHERE Id IN :opps.keySet()]);
    prices.putAll([SELECT Id, Product2.Name, Product2Id FROM PricebookEntry WHERE Id IN :prices.keySet()]);
    // Prepare for insert/update (upsert)
    for(OpportunityLineItem record: Trigger.new) {
        lines.put(record,
            new Order_Line_Item__c(
                Name='Opp-Product', 
                Product2__c=prices.get(record.PricebookEntryId).Product2Id, 
                UnitPrice__c=record.UnitPrice, 
                Quantity__c=record.Quantity, 
                Product_Name__c=prices.get(record.PricebookEntryId).Product2.Name, 
                Order__c=opps.get(record.OpportunityId).Orders__c,
                Id=record.Order_Line_Item__c));
    }
    // Commit to database
    upsert lines.values();
    // Store the resulting order line item ID for future use.
    for(OpportunityLineItem record: Trigger.new) {
        record.Order_Line_Item__c = lines.get(record).Id;
    }
}

This version knocks the entire code down to two queries. Note the use of the "upsert" keyword; you can't use "insert/update" the way you were trying in your trigger, because it would cause errors if any existing order line item was updated.

NOTE: I've specifically omitted the error checking bits here for brevity and clarity.