Feedback

[github-classroom]

👋! GitHub Classroom created this pull request as a place for your teacher to leave feedback on your work. It will update automatically. Don’t close or merge this pull request, unless you’re instructed to do so by your teacher.
In this pull request, your teacher can leave comments and feedback on your code. Click the Subscribe button to be notified if that happens.
Click the Files changed or Commits tab to see all of the changes pushed to the default branch since the assignment started. Your teacher can see this too.

Notes for teachers

Use this PR to leave feedback. Here are some tips:

• Click the Files changed tab to see all of the changes pushed to the default branch since the assignment started. To leave comments on specific lines of code, put your cursor over a line of code and click the blue + (plus sign). To learn more about comments, read “Commenting on a pull request”.
• Click the Commits tab to see the commits pushed to the default branch. Click a commit to see specific changes.
• If you turned on autograding, then click the Checks tab to see the results.
• This page is an overview. It shows commits, line comments, and general comments. You can leave a general comment below.
  For more information about this pull request, read “Leaving assignment feedback in GitHub”.

Subscribed: @Adam-Atoms

[walters954]

Hi Adam! 👋

I can see you've put in significant effort on this integrations module - great job on creating a trigger handler class and implementing the HTTP callout patterns! However, there are some critical issues that will prevent your code from working in production that need to be addressed.

What You Did Well ✅

• Excellent separation of concerns by creating the ContactTriggerHelper class
• Good error handling with try-catch blocks
• Proper null checking throughout your code
• Using Named Credentials - great security practice!
• Clean JSON serialization/deserialization logic

Critical Issues That Must Be Fixed 🚨

I've left detailed inline comments, but the main issue is that callouts from triggers require @future annotation. Without this, your code will fail with "Callout from triggers are currently not supported" errors when it runs.

Your Next Steps 🎯

1. Add @future(callout=true) to your callout methods
2. Fix the missing setBody() call in your POST request
3. Add the Content-Type header for your POST request
4. Consider bulkification - your current async wrappers will hit governor limits with multiple records

Once you address these issues, your integration will be solid! Feel free to reach out if you have questions about the @future annotation or bulkification patterns.

Keep coding! 💪

[walters954]

🚨 Critical Issue: This method needs the @future(callout=true) annotation!

Callouts from triggers are not allowed in Salesforce. When your trigger calls this method, it will fail with:

Callout from triggers are currently not supported

Fix:

@future(callout=true)
public static void getDummyJSONUserFromId(String dummyUserId) {
    // ... rest of your code
}

Note: When using @future, all parameters must be primitives (String, Integer, Id, etc.) - no complex objects like Lists of Contacts.

[walters954]

The endpoint URL is missing the user ID! The DummyJSON API expects the ID in the URL path.

It should be:

userRequest.setEndpoint('callout:DummyJsonUser/' + dummyUserId);

This way you're calling /users/{id} to get a specific user.

[walters954]

⚠️ Governor Limit Issue: This approach will fail with bulk operations!

Your getDummyJSONAsync method loops and calls getDummyJSONUserFromId for each ID. Since that method will need @future, you can't call it multiple times in a loop (max 50 @future calls per transaction).

Better approach:

public static void checkIds(List<Contact> contacts) {
    List<String> dummyJsonIds = new List<String>();
    for (Contact cont : contacts) {
        if (cont.DummyJSON_Id__c == null) {
            cont.DummyJSON_Id__c = String.valueOf(Integer.valueOf((math.random() * 100)));
        }
        if (Integer.valueOf(cont.DummyJSON_Id__c) <= 100) {
            dummyJsonIds.add(cont.DummyJSON_Id__c);
        }
    }
    if (!dummyJsonIds.isEmpty()) {
        // Pass the whole list to a SINGLE @future method
        getDummyJSONUsersAsync(JSON.serialize(dummyJsonIds));
    }
}

@future(callout=true)
public static void getDummyJSONUsersAsync(String jsonIds) {
    List<String> ids = (List<String>)JSON.deserialize(jsonIds, List<String>.class);
    // Make one callout for each ID here
}

This keeps you within governor limits!

[walters954]

🚨 Critical Issue: Same problem here - this method needs @future(callout=true)!

@future(callout=true)
public static void postCreateDummyJSONUser(String contactId) {
    // ... rest of your code
}

[walters954]

Missing required calls: Your POST request is incomplete! You're generating the JSON payload but never setting it as the request body, and you're missing the Content-Type header.

Add these lines:

postUserRequest.setHeader('Content-Type', 'application/json');
postUserRequest.setBody(contJsonBody);

Without setBody(), you're sending an empty POST request!

[walters954]

Nice null checking here! 👍

One small improvement - you could use isEmpty() for the email check too:

cont.Email = String.isBlank(cont.Email) ? 'unknown' : cont.Email

isBlank() checks for both null and empty strings, which is more defensive.