SFDO-Community / declarative-lookup-rollup-summaries

Declarative Lookup Rollup Summaries (DLRS) is a community built and maintained Salesforce application that allows you to create cross object roll-ups declaratively - no code! For install instructions and documentation, visit our website https://sfdo-community-sprints.github.io/DLRS-Documentation/
https://sfdo-community-sprints.github.io/DLRS-Documentation/
BSD 3-Clause "New" or "Revised" License
696 stars 238 forks source link

Parent's target field being modified when no child record (or none with matching criteria) exist #161

Open AlexIsMaking opened 9 years ago

AlexIsMaking commented 9 years ago

I've been struggling to limit my rollups to only run on records where a record from the child object exists. I'm using version 1.19 of the app.

For example, if I have a Sum rollup from a custom object to an opportunity then even if there aren't any child records related to that opportunity, the rollup will still insert a 0 in the target field.

forecast rollup

Or if I specify a record type for the child object and no child records with that record type are related to the parent but a child record with a non matching record type is edited, the rollup still tries to update the parent.

activity rollup

This has caused expected edits when using the calculate function (forecast rollup) and when the rollup has fired as a result of changes being made to child records which don't meet the relationship criteria (activity rollup).

This may just be my criteria but it's causing real issues at the moment. As a result, I've had to uninstall the app from Production so I'd really appreciate any guidance that you can offer which will enable me get it up and running again.

afawcett commented 9 years ago

I think what your saying is you would rather the tool evaluate the criteria on each child edit to decide if it should recalculate the rollup or not? This is a reasonable optimisation i agree.

Right now it only checks to see if values on the child record in respect to the Field to Aggregate or fields in Relationship Criteria Fields have changed before deciding to recalculate the rollup. If the rollup recalculate results in no records (either because there are none or none match the criteria) it puts a 0 in the parent aggregate field.

I did notice in your Net FSR in Year rollup does not include IsClosed and RecordTypeId in the Relationship Criteria FIelds list btw, meaning if these are changed it will not cause the rollup to be recalculated.

Because the criteria is SOQL criteria, its not actually that possible to apply it without running a query, unless i implemented an Apex criteria parser. So while i never say never, its not an easy optimisation tbh. What i maybe might be able to do is pre-query the parent record and avoid overwriting it with the same value perhaps, not sure if this would help though...

Can you tell me more about why these parent updates are causing an issue? What are the validation rules or user permission issues? I'd like to try and reproduce one or two of them if possible.

AlexIsMaking commented 9 years ago

I think what your saying is you would rather the tool evaluate the criteria on each child edit to decide if it should recalculate the rollup or not?

Yes please.

Avoiding overwriting the same value would also be an improvement (for me at least) but wouldn't completely solve the issue.

Basically if the tool didn't put a 0 in the parent's target field, it wouldn't need to edit the record and that would solve my issue, as you say though, that's obviously easier said than done!

Updating the parent causes the following issues:

Calculate tool (using the calculate button):

Field change:

Both:

Just to clarify my point about the validation rules - this isn't an issue that's caused by the output of the rollup. The problem we have is that validation rules have been added to our org over time and we haven't updated records which don't meet the validation rule's criteria retrospectively. So if I add a validation rule to ensure that the record name contains an A, I've not updated all records which were created before that validation rule was created, to ensure that their name's contain an A. Which means that when the 0 is added to the parent record and it's changed, the validation rule blocks the rollup. In our case when the validation rule blocks the rollup, automated email notifications are sent, as well as a rollup summary log being created unnecessarily. That can be managed but it's another example of an unintended consequence when a record which doesn't need to be changed, is modified by the app. If I can specify a record type for the child object so that the parent's not updated - if it's not linked to the right child record - then I don't have to update every other parent record which was created before the validation rule was created, in order to ensure that the validation rule doesn't block the rollup.

My org's used by several divisions in our business, each with processes that are being managed in very different ways in Salesforce. We silo our records based on record type etc. and essentially I need to be able to avoid editing records with non-matching record types as much as possible.

I hope that information helps..

PS. Thanks for the pointers for the Net FSR In Year rollup.

afawcett commented 9 years ago

Wow love the depth in your response, i'll be re-reading this again for sure, i agree there is an enhancement to be here, just still zeroing in on it. So the zero behaviour was a fix a while back now, when the last child record got deleted, the last rollup value would remain, so 0 was added, but this change has now ment that the tool is doing more parent updates than needed. I'm wondering how to fix your issue and retain the original fix requirement?

AlexIsMaking commented 9 years ago

I'm glad that was useful and also slightly relieved you didn't fall asleep reading such a long reply!

So I've had a look for the issue which you were trying to solve by adding the 0 and I unfortunately I couldn't find it. But I wouldn't have expected a 0 to be added when all child records are deleted. If there are no child records then shouldn't a sum query to return null (even if the target field had previously contained a value), since there are no values to aggregate? If there are child records and all of the aggregated values are 0, I'd expect it to return 0. Either way though, I think I'm getting off topic there because whether a 0 or null is being added to the target field, the parent record's still being modified and that's partly what's causing the issue (hitting validation rules, for example).

I'm guessing that you don't want to change any of the code for the app?

If you are happy to, then could the code to update the target field be changed so that it only updates the target field (with either a 0 or to set it to null) on deletion? That would solve the issues when using the Calculate function. I'm also assuming that if the child record didn't meet the relationship criteria, then even when a child record is deleted, no change would be made to the parent record at all? In which case, I'm fairly confident that, that would solve the issue..

If that suggestion isn't any help, could you please post a link to the issue that you solved by adding the 0? It's difficult to offer solutions for my issue without knowing whether they'll undo the resolution to the other issue.

afawcett commented 9 years ago

Sorry for the late reply, i've been on holiday.

I think we are getting in sync on what the core issue is here, and yes i'm more than happy to change the code once we can arrive at a safe change / enhancement.

I think we have two enhancements here...

  1. Only update the parent record if changes to child record/s being updated meet the criteria
  2. Make a distinction between 0 and null, 0 means child records exist (and meet criteria) and happen to total to 0, null means no child records exist (that meet the criteria).

BTW, this is the issue that originally drove the current 0 behaviour is here.

To implement these two changes above, i'd need to think about the architecture of the tool, as the criteria is applied via the platforms database engine only after 'any' child record change. As an Apex programmer i don't have the ability to evaluate an already read record against SQL criteria. What might be an option is to have the user place this criteria in a formula field on the child object and the tool reference that before attempting to recalculate the rollup, hmmmm....

I'll mark this as an enhancement in the meantime, i'm pretty clear on what you need here, just need to think about the code change a bit more....

AlexIsMaking commented 9 years ago

Hi Andy,

Thanks for getting back to me and no worries about the wait.

It sounds like you know exactly what I was hoping for here and it looks like the changes that you're proposing won't cause issues for the user who requested the earlier change.

As far as the architecture's concerned - I'm afraid I can't offer a solution here, your knowledge of how the platforms works is far more advanced than mine. But the formula solution that you're proposing sounds good to me.

I look forward to seeing what solution you decide to apply. If I can be of any assistance please just let me know.

Thanks again.

AlexIsMaking commented 9 years ago

Following on from your point about the challenge of figuring out a solution to this issue, I just thought I'd mention that solving issue #162 would solve the most significant problem, that's caused by this issue - child records (which may or may not match the rollup criteria) being edited by users without the permission set and the rollup being blocked as a result.

So if you're trying to prioritize one issue over the other and #162 would take less time to fix then, as far as I'm concerned, it would make sense to fix #162 first.

afawcett commented 9 years ago

Noted thanks!

jondavis9898 commented 9 years ago

Hello @aplssf - I'm not sure this applies to your situation but in reading through the history of this issue, I believe the fix that was implemented for #226 might help (or at least I hope it will :)).

AlexIsMaking commented 9 years ago

Hi @jondavis9898 thanks for the heads up. If

When one (or more) records are detected as having a field that is involved in a rollup changed, all master record ids for all records in the trigger batch are evaluated and updated.

& zeros are being added to the parent's target field for each of those records, even when there is no value to aggregate, then it sounds like this would help.

So you're saying that at least some of the records in the batch wouldn't be updated? (I've not worked with batches at all so my knowledge on this topic is limited).

jondavis9898 commented 9 years ago

Hi @aplssf - Glad you think this might help your situation a bit. In short, what was occurring previously was every rollup target (parent) field was being updated on every record in the batch if at least one of those records "required" an update. What the fix does is ensure that only targets that require an update are updated. To require an update, something must have changed on the child records related to the rollup parent. In general, it wouldn't be accurate to say "at least some won't be updated" because they might all have changed. However, in your case, those that haven't changed should no longer be updated and therefore you'd avoid at least some scenarios where you were seeing the unnecessary zero (0).

Hope this helps, let me know if you have any more questions or would like more info.

AlexIsMaking commented 9 years ago

@jondavis9898 thanks for explaining, it does sound like this will avoid unnecessary updates to parent records (when no child records exist) which should speed up the batch calculate jobs significantly in large orgs so thank you!

Just to be clear, there's still a couple of outstanding issues

jondavis9898 commented 9 years ago

* Note * After thinking back through your questions, I realized my first reply a few minutes ago was incorrect. The text below is an edit of that first reply correcting my mistake. Apologize for any confusion.

@aplssf - I definitely think this fix will at least reduce the situations where your parent records are getting updated - just not sure it will eliminate all situations.

re: your questions: 1) Correct. When a new child is created any associated parents will be updated unfortunately. The reason for this is that the "criteria" would need to be evaluated manually (as opposed to using SOQL) and that would not be an easy undertaking unfortunately.
2) It depends :) On an update, the parent will only be updated if it contains at least one child who's field that is involved in the rollups (FieldToAggregate, any relationship criteria field, order by field, etc.) has changed. Prior to #226, if any child record in the batch had one of these fields changed, all parents for all records in the batch were updated. This is where you should expect the largest decrease in parent records being updated.

Hope this helps, feel free to let me know if you have any other questions or would like additional info.

AlexIsMaking commented 9 years ago

@jondavis9898 thanks for taking the time to explain this in so much detail, it's really good to have that clarity.

From myself & on behalf of everyone who this update is going to help, thank you for taking the time to make these improvements!

afawcett commented 9 years ago

Yes @jondavis9898 has done some fine work here, its now released in v1.23.

AlexIsMaking commented 9 years ago

@afawcett there's still a couple of outstanding issues here - 0's being inserted in the parent's target field when a child record's created but the field to aggregate is null & not being able to define criteria to specify the child records whose values should be rolled up (i.e. based on record type).

Would you like me to log these separately so that they can be addressed & closed one by one?

afawcett commented 9 years ago

Yes @aplssf ah yes, sorry my bad, yes separating them out would be perfect, thanks! :+1:

rtfoleystl commented 5 years ago

I know this is an very old thread, but I'm commenting here because the end of this thread suggests that the issue I'm struggling with might be addressed elsewhere but if that's true, I can't find it. Specifically, I am trying to resolve the problem where there are no child records (or a null value) but the parent still gets updated with a value of zero instead of null.
Looking for either a solution, or workaround. Big fan of the DLRS tool! Thank you!

shawnholt commented 3 years ago

Yes @afawcett @aplssf @rtfoleystl - apparently this issue has not been addressed. Parent target field is null, child field is null, criteria is to aggregate only if child field !=null. Aggregation occurs anyway and puts a "0" in the target field. Unfortunately, 0 and null have an entirely different meaning here (null means not input, 0 means value of 0) and the two are conflated....

afawcett commented 3 years ago

Re-opening - lets take another pass at this - it might have to be some flag we put on the rollup definition tbh - as i don't see a perfect way to not impact existing installs.

Scarrien commented 2 months ago

I just want to say that I'm having this problem now as well. I have multiple rollups from the same child record to the parent with different filters (test grade records to quiz 1 score/quiz 2 score/etc. fields), and we're having rollup fields filled with 0 when there should be null.

Is there a plan for when this will be resolved?

side note, I feel like the easiest way to handle this is to put a "No Children Result" field on the metadata record. It'll give people control over if it should be null in these cases or if they want a result of 0 (such as for a sum).

aheber commented 2 months ago

@Scarrien thanks for the input. I'm pushing this through the team for review and maybe we can write up a plan for community feedback.

One of the big gotcha's will be what should old rollups do. The "No Children Result" field would probably best support that and cover a lot of these use cases. That will still have some questions on if it should be some kind of free text, picklist, or combination.

Hopefully we can finally push this forward enough to get design decisions made. I'll confess this wasn't top of my priority list but maybe I can get someone else to drive the design and then the engineering shouldn't be too bad.