jamessimone / apex-rollup

Fast, configurable, elastically scaling custom rollup solution. Apex Invocable action, one-liner Apex trigger/CMDT-driven logic, and scheduled Apex-ready.
MIT License
210 stars 30 forks source link

Rollup not calculating SUM correctly of all child records #491

Closed baobao917 closed 11 months ago

baobao917 commented 12 months ago

I am using ApexRollup to SUM the accounts payable (vendor invoices) balance based on their respective agings but it is not calculating correctly.

I have recorded a video to hopefully more clearly explain and have tried to summarize below.

Video - https://www.loom.com/share/cebe4f3edc08495f90cdfb02fabf9fa3?sid=9f9a0162-31b1-413a-a8f3-ffd31d8f6474

A vendor invoice in our org is called a "Payable". It can be represented as a positive number (invoice) or negative number (credit memo) but are the same child object record "Payable". Both types have balances that are aged. A credit memo can be applied to a invoice which reduces the balances of both respective records. When applying the credit memo to an invoice, it is not calculating the aging bucket correctly and actually INCREASES the balance amount.

I think that perhaps the rollup calculation is only SUMMING the movement of one child record (the credit memo) balance from -100 to 0 (i.e. movement of +100) and not SUMMING the movement of the second child record (the invoice) balance from 2100 to 2000 (i.e. movement of -100). (In other words, not calculating on both child records)

Believing that was the case, I used the "Is Full Record Set" set to TRUE so that it converted to a full calculation rollup on all the child records. This is based on this documentation: image

However, that did not change the incorrect results. I also attempted to do a full manual recalculate using the "Recalculate Rollups" button and that did not change the incorrect results.

I am also attaching a debug here as well as well as screenshots of the rollup CMDT and the rollup control CMDT apex-07L3C00000M41wlUAB.log

Rollup CMDT image

Rollup Control CMDT image

What the Aging 0-60 should reflect PRIOR to credit memo application (Correct-Compare to SF report calculation) image image

What the Aging 0-60 should reflect AFTER credit memo application (Incorrect-Compare to SF report calculation) image image

jamessimone commented 12 months ago

@baobao917 I would update your where clause to AcctSeed_Age__c IN ('0-Current', '1-30 Days', '31-60 Days')

baobao917 commented 12 months ago

@baobao917 I would update your where clause to AcctSeed_Age__c IN ('0-Current', '1-30 Days', '31-60 Days')

Hi @jamessimone - Thanks for the input. It however didn't resolve the issue in this ticket. I recorded a video demonstrating.

https://www.loom.com/share/a2527bbb65dd4f0bb40329202536f0a0

2nd quick follow-up video b/c I forgot one extra detail https://www.loom.com/share/c34c9d281c9e45c5bb61a1bd00375e2d

jamessimone commented 12 months ago

@baobao917 firstly - let me just thank you for recording those videos. I was remiss in not thanking you previously. I really appreciate you taking the time out of your day to do so.

The first debug log that you attached in your initial message was just the synchronous part of the rollup, which is not where the actual logic is performed when your Should Run As value on your rollup control is set to Queueable or Batchable. If you were to temporarily update that value to Synchronous Rollup, you should get all the log info in one log, which might help to track down how this issue is occurring. I'm primarily looking to see how both payables end up getting updated - if they're updated one at a time, and this is some kind of race condition, or if there's an actual logic error with them both being updated at the same time.

baobao917 commented 12 months ago

@jamessimone No problem! Thank you for being so responsive! I'm really hoping to be able to eventually use this as a solution to replace DLRS for all my clients.

So I tried running as synchronous and I got a recursion error. Here is the error and a video showing you.

Video - https://www.loom.com/share/1f8f78d8aa9142f2851331b640541ce1?sid=1f4579fd-30d3-438c-892a-971447051810

Error text - ApexRollupPayableTrigger: execution of AfterUpdate caused by: System.NullPointerException: Attempt to de-reference a null object Class.RollupRecursionItem.equals: line 45, column 1 External entry point Class.RollupEvaluator.RecursiveUpdateEvaluator.matches: line 903, column 1 Class.RollupCalculator.winnowItems: line 304, column 1 Class.RollupCalculator.performRollup: line 172, column 1 Class.RollupAsyncProcessor.getCalculator: line 1347, column 1 Class.RollupAsyncProcessor.getUpdatedLookupItemsByRollup: line 1250, column 1 Class.RollupAsyncProcessor.process: line 694, column 1 Class.RollupAsyncProcessor.runCalc: line 299, column 1 Class.Rollup.runFromTrigger: line 1640, column 1 Trigger.ApexRollupPayableTrigger: line 3, column 1

Error screenshot image

Debug log apex-07L3C00000M43IyUAJ.log

jamessimone commented 12 months ago

@baobao917 if you don't run the process synchronously, do you get that same error in the Queueable version of the log?

baobao917 commented 12 months ago

@baobao917 if you don't run the process synchronously, do you get that same error in the Queueable version of the log?

@jamessimone No I don't. Please let me know what you would like for me to do as next step. I can rerun it as queueable....just let me know how I can identify the correct log to send to you since what I did last time didn't give you what you need. Thanks!

jamessimone commented 12 months ago

When you update a record to start the rollup process, there should be three logs generated - one with an Aura origin from the synchronous part of the update, and two queueable handler origins from the async rollup running and then the rollup finalizer running. All three would be helpful.

baobao917 commented 12 months ago

@jamessimone Thanks...here is the files: image

apex-07L3C00000M4RdGUAV.log

apex-07L3C00000M4RdLUAV.log

apex-07L3C00000M4Rb2UAF.log

apex-07L3C00000M4RdQUAV.log

jamessimone commented 11 months ago

@baobao917 apologies for the slow response, with Dreamforce this week it's been hard to find the time. I'll hopefully be able to review these logs today and I'll follow up if I have any other questions.

jamessimone commented 11 months ago

@baobao917 it looks like the large queueable log (which, for whatever reason, shows up as <empty> under the Log Origin, is getting cut off due to your profiling for Apex being set to Finest. If you cut that back to Fine, the log shouldn't get truncated.

baobao917 commented 11 months ago

@jamessimone No worries. I figured DF2023 had everyone's attention. Here are the logs with SFDC_DevConsole set to FINE. image

apex-07L3C00000M76uRUAR.log apex-07L3C00000M76r0UAB.log apex-07L3C00000M76uSUAR.log apex-07L3C00000M76PQUAZ.log

Also I mentioned how when clicking the "Recalculate Rollups" button, it actually did calculate everything correctly. I included logs from that as well below in case you needed for comparison.

image apex-07L3C00000M76y4UAB.log apex-07L3C00000M76y9UAB.log apex-07L3C00000M76yAUAR.log apex-07L3C00000M76yEUAR.log apex-07L3C00000M76yJUAR.log

jamessimone commented 11 months ago

@baobao917 I believe I fixed this issue in #497 - would you be able to confirm?

baobao917 commented 11 months ago

@baobao917 I believe I fixed this issue in #497 - would you be able to confirm?

@jamessimone That looks like it did the trick! Thanks!

jamessimone commented 11 months ago

Great! Glad to hear it

baobao917 commented 11 months ago

@jamessimone Looks like this same recalculation bug is occurring when doing a full recalculation from the Recalculate Rollup tab. Should I reopen this issue or create a brand new issue?

image