[SalesForce] Apex Superbadge Automation Test Failure – System.DmlException: INSERT FAILED and UPDATE FAILED

Working on the Apex superbadge challenges and got stuck on Challenge 4 – "Test automation logic". I am getting the following error:
enter image description here

Although the test coverage is 100% and is working without any errors otherwise. Adding my code below:

Trigger

trigger MaintenanceRequest on Case (after update) {
// ToDo: Call MaintenanceRequestHelper.updateWorkOrders
List<Case> newrequests = new List<Case>();
for(Case c: Trigger.new){
    if((c.Type == 'Routine Maintenance' || c.Type == 'Repair') && c.IsClosed == true ){
        // Calling the helper function once the case is closed
        Case case_maintenance_days = [SELECT Equipment__r.Maintenance_cycle__c FROM Case WHERE Id = :c.Id];
        Integer maintenance_days = (Integer)case_maintenance_days.Equipment__r.Maintenance_cycle__c;
        newrequests.add(new Case(Subject = 'New Maintenance Request', Origin = 'Phone', Status = 'New', Equipment__c = c.Equipment__c, Vehicle__c = c.Vehicle__c, Date_Due__c = Date.today().addDays(maintenance_days) , Date_Reported__c = Date.today()));
    }
    MaintenanceRequestHelper.updateWorkOrders(newrequests);
}

}

Helper function

public with sharing class MaintenanceRequestHelper {

public static void updateWorkOrders(List<Case> newcases) {
    // TODO: Complete the method to update workorders
    insert newcases;
}        

}

The test class resulting in 100% code coverage

@isTest
private class MaintenanceRequestTest {
@isTest 
static void testRequestTrigger(){
    //Setting up new cas for the test
    Vehicle__c  v = new Vehicle__c(Name = 'Test Method Vehicle');
    Product2 p = new Product2(Name = 'Test Method Product', Maintenance_Cycle__c = 90, Lifespan_Months__c = 36);
    insert v;
    insert p;
    Case c = new Case(Subject = 'TEST Method Maintenance Request', 
             Origin = 'Phone', Status = 'New', 
             Equipment__c = p.Id, 
             Vehicle__c = v.Id, 
             Date_Due__c = Date.today().addDays((Integer)p.Maintenance_Cycle__c) , 
             Date_Reported__c = Date.today());
    insert c ;
    c.Status = 'Closed';
    Test.startTest();
    Database.SaveResult res = Database.update(c);
    Test.stopTest();
    System.assert(res.isSuccess());
    Id vid = v.Id;
    Integer casecount = Database.countQuery('SELECT count() FROM Case WHERE (Vehicle__c = :vId AND Status = \'New\')');
    System.assertEquals(1, casecount);
}

}

Any help or pointers are appreciated. Thanks.

EDIT: As per the best answer, updated the code and successfully passed the test. Here is the updated code:

trigger MaintenanceRequest on Case (after update) {
List<Case> newrequests = new List<Case>();
List<Product2> equipments = [SELECT Id, Maintenance_cycle__c FROM Product2];
Map<Id, Integer> equip_maintenance = new Map<Id, Integer>();
for(Product2 p: equipments){
    equip_maintenance.put(p.Id, (Integer) p.Maintenance_cycle__c);
}
for(Case c: Trigger.new){
    if((c.Type == 'Routine Maintenance' || c.Type == 'Repair') && c.IsClosed == true ){
        newrequests.add(new Case(Subject = 'New Maintenance Request', Origin = 'Phone', Status = 'New', Equipment__c = c.Equipment__c, Vehicle__c = c.Vehicle__c, Date_Due__c = Date.today().addDays(equip_maintenance.get(c.Equipment__c)) , Date_Reported__c = Date.today()));
    }
}
MaintenanceRequestHelper.updateWorkOrders(newrequests);

}

Best Answer

Try with MaintenanceRequestHelper.updateWorkOrders(newrequests); as outside for loop in the trigger. Your code will work fine for a single record, but for multiple records it will fail.

Example- Trigger.New has 2 cases. let's say C1 , C2.

1st iteration : When If criteria satisfies for C1, newrequests will have C1 and outside If statement, it calls method which inserts C1.

2nd iteration : newrequests still has C1 , when C2 also satisfies if condition, newrequests will have C1,C2 and it will call method which will try to insert C1 and C2. But wait, C1 is already inserted and it has record id associated with it.

That's why it's giving exception, you are trying to insert a record (C1) which is already inserted.

One fix is when you call MaintenanceRequestHelper.updateWorkOrders(newrequests); after this statement immediately clear the newrequest.clear() , so that in the 2nd iteration it has only C2

But wait, it's not bulkified , if trigger.new has > = 100 records, you will run into 101 SOQL limit , also for each single record, you are trying to call insert dml statement , that will again not work if trigger.new has > = 150 records.

Hence , put MaintenanceRequestHelper.updateWorkOrders(newrequests) outside for loop, you will get rid of DML limits and it should be fine to validate the trailhead. Ideally, you should move SOQL query as well outside for loop. Check this- https://developer.salesforce.com/page/Best_Practice%3A_Bulkify_Your_Code