2

EDIT: SOLVED by Nagendra Singh

I had messed up my trigger calling for an ifBefore > all triggers, then calling for all the triggers again after the ifBefore clause. I added an else clause to fix it. Thank you Nagendra!

New Trigger Code:

    if (Trigger.isBefore) {
        if (Trigger.isInsert) {
            handler.beforeInsert();
        }
        if (Trigger.isUpdate) {
            handler.beforeUpdate();
        }
        if (Trigger.isDelete) {
            handler.beforeDelete();
        }
    }
    else {
        if (Trigger.isInsert) {
            handler.afterInsert();
        }
        if (Trigger.isUpdate) {
            handler.afterUpdate();
        }
        if (Trigger.isDelete) {
            handler.afterDelete();
        }
    }




ORIGINAL QUESTION:

I'm really struggling to solve this problem. So please save me!

All objects here are custom

I have a data model set up where I have a Treasury sObject which can have a lookup to itself, so essentially a master treasury with branches. (I've fully tested my insertion and updating of those records and all the behaviour is A-OK.)

I'm currently trying to test the logic for an Incoming payments sObject. When I try to test the insert, my after Insert logic calculates double its value. I confirmed that this behaviour was occurring by setting a static flag to ensure it ran once which fixed my tests but it's not a proper solution, the underlying problem is still there.

So, I really need help figuring this out! My org currently has no processes or workflows. My only logic is in my Apex code. As you'll see from the code too, I only read information from the inserted object.

I'm pretty new to this, so I'd love a run down of why this is occurring - thanks!

Incoming sObject Trigger

trigger IncomingTrigger on Incoming__c (before insert, before update, before delete, after insert, after update) {

    IncomingTriggerHandler handler = new IncomingTriggerHandler(Trigger.new, Trigger.oldMap);

    if (Trigger.isBefore) {
        if (Trigger.isInsert) {
            handler.beforeInsert();
        }
        if (Trigger.isUpdate) {
            handler.beforeUpdate();
        }
        if (Trigger.isDelete) {
            handler.beforeDelete();
        }
    }
    if (Trigger.isInsert) {
        handler.afterInsert();
    }
    if (Trigger.isUpdate) {
        handler.afterUpdate();
    }
    if (Trigger.isDelete) {
        handler.afterDelete();
    }
}

Incoming sObject TriggerHandler (removed irrelevant methods for legibility)

public with sharing class IncomingTriggerHandler {
    
    private List<Incoming__c> newRecords;
    private Map<ID, Incoming__c> oldRecords;

    public IncomingTriggerHandler(List<Incoming__c> newRecords, Map<ID, Incoming__c> oldRecords) {
        this.newRecords = newRecords;
        this.oldRecords = oldRecords;
    }

    public void beforeInsert() {
        for (Incoming__c record : newRecords) {
            if (record.Amount__c < 0) {
                record.addError('Cannot set a negative amount. Please use an outgoing record.');
            }
        }
    }

    public void afterInsert() {
        Map<ID, List<Incoming__c>> relevant = new Map<ID, List<Incoming__c>>();

        for (Incoming__c record : newRecords) {
            // Treasury specified and amount to add
            if (record.Treasury__c != null && record.Amount__c > 0) {
                if (relevant.get(record.Treasury__c) == null) {
                    relevant.put(record.Treasury__c, new List<Incoming__c>{record});
                } else {
                    relevant.get(record.Treasury__c).add(record);
                }
            }
        }

        if (relevant.keySet().size() > 0) {
            List<Treasury__c> updated = new List<Treasury__c>();
            // Add income to treasuries
            for (Treasury__c treasury : [SELECT Id, Amount__c, Unaccounted__c, Unallocated__c, RecordTypeId,
            Treasury__c, Treasury__r.Amount__c, (SELECT Id, Amount__c, Allocation__c FROM Branches__r) 
            FROM Treasury__c WHERE Id IN :relevant.keySet()]) {
                Double totalIncoming = 0;
                // Get total incoming
                for (Incoming__c record : relevant.get(treasury.Id)) {
                    totalIncoming += record.Amount__c;
                }

                // Treasury is a master
                if (treasury.RecordTypeId == '0124L000000uF4YQAU') {
                    treasury.Amount__c += totalIncoming;
                    if (treasury.Unallocated__c > 0) {
                        treasury.Unaccounted__c += totalIncoming * (treasury.Unallocated__c / 100);
                    }
                    updated.add(treasury);

                    for (Treasury__c branch : treasury.Branches__r) {
                        if (branch.Allocation__c > 0) {
                            branch.Amount__c += totalIncoming * (branch.Allocation__c / 100);
                            updated.add(branch);
                        }
                    }
                }
                // Treasury is a branch
                else {
                    treasury.Amount__c += totalIncoming;
                    updated.add(treasury);

                    Double newTotal = treasury.Treasury__r.Amount__c += totalIncoming;
                    updated.add(new Treasury__c(Id = treasury.Treasury__c, Amount__c = newTotal));
                }
            }
            update updated;
        }
    }

Current test (if you're interested)

@isTest
private class IncomingTriggerHandlerTest {

    @testSetup
    private static void init(){
        Treasury__c master = new Treasury__c(RecordTypeId = '0124L000000uF4YQAU');
        insert master;
        insert new Treasury__c(Treasury__c = master.Id,
        RecordTypeId = '0124L000000uF4dQAE', Allocation__c = 50);
    }

    @isTest
    private static void testInsertIncomeToTreasury() {
        Treasury__c treasury = [SELECT Id FROM Treasury__c 
        WHERE RecordTypeId = '0124L000000uF4YQAU'][0];

        Test.startTest();
        insert new Incoming__c(Treasury__c = treasury.Id, Amount__c = 500);
        Test.stopTest();

        treasury = [SELECT Id, Amount__c, Unaccounted__c,
        (SELECT Id, Amount__c FROM Branches__r)
        FROM Treasury__c WHERE Id = :treasury.Id][0];

        System.assertEquals(500, treasury.Amount__c);
        System.assertEquals(250, treasury.Unaccounted__c);
        System.assertEquals(250, treasury.Branches__r[0].Amount__c);
    }

We can ignore the other asserts, the main issue is that it's running twice and so the first assert is actually 1000, not 500.

Thanks so much!

2
  • Welcome to SFSE, a well documented question. However, out of curiosity, why not use a flow for all of this? what do you do that requires an APEX trigger?
    – Saariko
    Aug 8 '21 at 9:35
  • Thank you for the welcome! I chose not to use a flow or any sort of process because I wanted to avoid the overhead-headache later worrying about how triggers might interact with existing declarative functionality . . But how this turned out, I didn't escape those troubles at all LOL! This is just a personal project and it's mainly to build something useful for myself whilst learning how to code and use salesforce better though so I'd prefer to do all the logic through apex if possible. Though if you think that's a bad idea I'd love to know your thoughts on why. Thank you :) Aug 8 '21 at 9:40
4

I see you have a check for isBefore and this logic.

if (Trigger.isBefore) {
    if (Trigger.isInsert) {
        handler.beforeInsert();
    }
    if (Trigger.isUpdate) {
        handler.beforeUpdate();
    }
    if (Trigger.isDelete) {
        handler.beforeDelete();
    }
}

But after this logic you call this again.

if (Trigger.isInsert) {
    handler.afterInsert();
}
if (Trigger.isUpdate) {
    handler.afterUpdate();
}
if (Trigger.isDelete) {
    handler.afterDelete();
}

So technically, in the beforeInsert context handler.beforeInsert(); and handler.afterInsert(); both will get called, as there is no check that the after**** methods should be called in which trigger context.

So your after***** methods should get called only in isAfter context like this.

if(Trigger.isAfter) {
    if (Trigger.isInsert) {
        handler.afterInsert();
    }
    if (Trigger.isUpdate) {
        handler.afterUpdate();
    }
    if (Trigger.isDelete) {
        handler.afterDelete();
    }
}
1
  • Sorry I don't have enough rep to upvote this yet but omg, that was actually it, I feel kinda dumb but thank you so much, that was giving me such a massive headache!! I'm really thankful for the help! Aug 8 '21 at 10:42

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.