How do I resolve the following error "You have uncommitted work pending. Please commit or rollback before calling out

Question

This is my code at the moment, the API is only set to receive the ID's one at a time (if this is a major thing I can ask for this to be changed) so that I can send them in a batch instead. Also any comments to improve my code would be appreciative I've only just started so I'm sure there will be improvements to be made.

public class sendToApi {
public static string token = 'test';
public static string sfid = '';
public static HttpResponse httpCallout() {
// Format SF ID to send to API 
string data = '{' + '"token": "' + token + '", ' +  '"sfid":' +  '"' + sfid + '"' + '}'; 
Http http = new Http(); 
HttpRequest request = new HttpRequest(); 
request.setEndpoint('https://api.com'); 
request.setMethod('POST'); 
request.setHeader('Content-Type', 'application/json;charset=UTF-8'); 
request.setBody(data);
HttpResponse response = Http.send(request);
System.debug('Http Response: ' + response);
System.debug('Salesforce ID: ' + sfid);
return response;

}
public static void updateRecord(DVLA_Lookup_Opportunity__c dv) {
    // dvlaOppToUpdate.Id = dv.Id;
    system.debug('sfid:' + dv.id);
    dv.SentToAPI__c = 'Sent';
    update dv;
}
@future (callout=true)
public static void dvlaUpdates() {
// Create a list of account records from a SOQL query
List<DVLA_Lookup_Opportunity__c> dvla = [SELECT Id, SentToAPI__c, Account_Id__c FROM DVLA_Lookup_Opportunity__c WHERE Stage__c = 'Closed Won' AND SentToAPI__c = 'Pending']; 

// Loop through the list and update the sfid field
for(DVLA_Lookup_Opportunity__c dv : dvla) {
    sfid = dv.id;
    HttpResponse res = null; 
    res = httpCallout();
    if (res.getStatusCode() == 200) {
    updateRecord(dv);
    }
    else {
        System.debug(res.getStatusCode());
    }
}

}

}

Answer 1

The linked question goes over the error itself, but to summarize, you cannot perform a callout in a given transaction after performing DML.

Your code currently does: callout -> dml -> callout for next record

Aside from that, you're performing DML inside of a loop (which is bad practice on the Salesforce platform, and likely to cause you to run into one of the governor limits).

If this API you're calling out to can (or can change to) accept multiple salesforce Ids, that'd be the way to go. Failing that, taking care of your "DML in loop" issue should also resolve the "uncommitted work pending" error.

On the Salesforce platform, it's much preferred to work on collections of data rather than on single instances. Instead of calling updateRecord() on every record one at a time, you would gather the records to be operated on in a List<DVLA_Lookup_Opportunity__c>, and send that List to your updateRecord() method (which also implies that you need to change the signature of that method).

public static void updateRecord(List<DVLA_Lookup_Opportunity__c> dvList) {
    // Do your manipulation first, then perform DML after you're done processing
    //   everything
    for(DVLA_Lookup_Opportunity__c dvItem :dvList){
        dv.SentToAPI__c = 'Sent';
    }
update dvList;

}
@future (callout=true)
public static void dvlaUpdates() {
    List<DVLA_Lookup_Opportunity__c> dvla = [SELECT Id, SentToAPI__c, Account_Id__c FROM DVLA_Lookup_Opportunity__c WHERE Stage__c = 'Closed Won' AND SentToAPI__c = 'Pending'];
// Again, perform the work you want to do, and gather the results in a list.
// THEN you can perform your DML
List&lt;DVLA_Lookup_Opportunity__c&gt; dvlaSuccess = new List&lt;DVLA_Lookup_Opportunity__c&gt;();

for(DVLA_Lookup_Opportunity__c dv : dvla) {
    sfid = dv.id;
    HttpResponse res = null; 
    res = httpCallout();
    if (res.getStatusCode() == 200) {
        dvlaSuccess.add(dv);
    }
    else {
        System.debug(res.getStatusCode());
    }
}

// DML (and anything that performs DML) should be done outside of any loop
updateRecord(dvlaSuccess);


}