Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Concat_Distinct with Break Line Delimiter not working #595

Closed
Silchuki14 opened this issue Jun 10, 2024 · 4 comments · Fixed by #597
Closed

Concat_Distinct with Break Line Delimiter not working #595

Silchuki14 opened this issue Jun 10, 2024 · 4 comments · Fixed by #597

Comments

@Silchuki14
Copy link

Silchuki14 commented Jun 10, 2024

(First time using Github, hoping this is detailed enough)

Hello James, we are wondering why BR() or
does not work as a delimiter like DLRS. Furthermore, it seems to cause a calculation issue that we wanted to bring up to your attention. Below are our troubleshooting records that hopefully can help answer the question. Debug logs attached as well.

Please let me know if you have any questions, or would need additional details.

Thank you!


ApexRollup V1.6.23

Test Records:

1 Terminal_Upgrade__c Record (Parent)

2+ Device_to_Upgrade__c Records (Child(s))

Device to Upgrade Apex Trigger

trigger ApexRollup_DevicetoUpgradeTrigger on Device_to_Upgrade__c(after insert, after update, before delete, after undelete) {
  // after delete is required ONLY if your org does Account / Contact / Lead / Case merges
  Rollup.runFromTrigger();
  // etc. You can invoke the above from your handler if you have one
}

*DLRS Apex Trigger is disabled

DLRS Comparables
Concat_Distinct on DevUps_Models__c field
d1621e24-157e-4a2a-ba0a-93b8bd17789f

Concat_Distinct on DevUps_Serial_Numbers__c field.
979fce0f-7c24-4bdc-853e-c47ad2250334

Rollup Controls and Rollup Configs on this “simple” Concat Rollup.
cd7368cb-252e-41a0-8a60-c1995dd438ac
*Queuable and not Synchronous because the former is understood as the preferred method.

*Rollup Logging enable for the purpose of this exercise.
2b93a53e-77c8-4526-b640-83f1fe35254f
f6b0863b-1137-4f9a-81ee-925bccef3de3

Concat_Distinct Issue

With DLRS, we used BR() as a delimiter to break line each value from the Rollup. It worked as expected. With Rollup, the break line delimiter seems to become part of the value, rendering the value distinct. As a result, the value is full of BR() and becomes inaccurate by duplicating values.

*For the purpose of this exercise, the Serial Number Rollup has been updated to no delimiter (comma as default) for comparison purposes.

Before doing triggering any tests, a Full Recalc on both Rollups has been performed. We start with the below field value:

Models: ()DevUp123BR()DevUp098

Serial Numbers: 098765, 123456

Test #1 - Create a new Device to Upgrade from Scratch (not cloned) - Same Values (Fail)

Model Added: DevUp123 - Serial Number Added: 123456

Expectation: Models: ()DevUp123BR()DevUp098 - Serial Numbers : 098765, 123456

Result: Models: ()DevUp098BR()()DevUp123BR()DevUp123 - Serial Numbers: 098765, 123456

Comment: As we can see with the test, if we use BR() (same result with
) as a delimited, the rollup seems to glue it to a value, making it distinct. Without delimiter (default comma), it works fine. We have done this same test, but instead of BR() we used an hyphen ( - ) and it worked as expected. Perhaps the break line specifically is the problem.

We use BR() with DLRS and it works as expected. We need this rollup to break line each value for Email Template purposes (instead of doing it directly in the template with a Substitute formula).

Attached are the Debug Logs I got from the test.
Contact_Distinct - Test1CreateNewDeviceSameValue1.log
Contact_Distinct - Test1CreateNewDeviceSameValue2.log
Contact_Distinct - Test1CreateNewDeviceSameValue3.log

@jamessimone
Copy link
Owner

@Silchuki14 use \n as the break line character - that would be my guess. BR() must be some special thing in DLRS, as that's not a valid line break character outside of formulas

@Silchuki14
Copy link
Author

Silchuki14 commented Jun 10, 2024

Hey @jamessimone,
Did the same test but with /n and got the same result.

image
apex-07LAu00000Euwj1MAB.log
apex-07LAu00000Ev4OfMAJ.log
apex-07LAu00000EvAVdMAN.log

Is this just something Apex Rollup cannot support (ok if that is the case).

@jamessimone
Copy link
Owner

@Silchuki14 I've found the issue - which is specific to new line characters - so I should be able to patch today

jamessimone added a commit that referenced this issue Jun 11, 2024
jamessimone added a commit that referenced this issue Jun 12, 2024
* Optimizes an if statement to the outside of a for loop when it isn't necessary to filter children prior to rolling up
* Tech debt cleanup - removes blanket Deeply Nested If Statement PMD suppression
* Further cleaned up one of the conditionals in RollupCalcItemReplacer by taking advantage of null coalesce
* Fixes #595 - thanks to @jongpie for reminding me to update the DLRS migration scripts
@jamessimone
Copy link
Owner

jamessimone commented Jun 12, 2024

@Silchuki14 this is fixed in #597. The new package version should be available to install via the README

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants