Skip to content

Commit 6cb0e0c

Browse files
bmacariniaheber
authored andcommitted
refactor(validation): Address review feedback
- Added unit tests for validation logic - Implemented String.isBlank() for robustness - Enforced braces and applied Prettier formatting
1 parent 7afc576 commit 6cb0e0c

File tree

4 files changed

+100
-12
lines changed

4 files changed

+100
-12
lines changed

dlrs/main/classes/RollupService.cls

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1059,8 +1059,7 @@ global with sharing class RollupService {
10591059
'[\r\n]+'
10601060
)
10611061
) {
1062-
String trimmedCriteriaField = criteriaField.trim();
1063-
lookupFields.add(trimmedCriteriaField);
1062+
lookupFields.add(criteriaField.trim());
10641063
}
10651064
}
10661065
// only include order by fields when query based rollup (concat, first, last, etc.) since changes to them

dlrs/main/classes/RollupServiceTest6.cls

Lines changed: 85 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -790,4 +790,88 @@ private class RollupServiceTest6 {
790790
.NumberOfEmployees
791791
);
792792
}
793-
}
793+
794+
@IsTest
795+
static void testTrimWhitespaceFromRelationshipCriteriaFieldsInput() {
796+
// Test supported?
797+
if (!TestContext.isSupported())
798+
return;
799+
800+
// Configure rollup
801+
LookupRollupSummary__c rollupSummary = new LookupRollupSummary__c();
802+
rollupSummary.Name = 'Test Whitespace Trim';
803+
rollupSummary.ParentObject__c = 'Account';
804+
rollupSummary.ChildObject__c = 'Opportunity';
805+
rollupSummary.RelationShipField__c = 'AccountId';
806+
rollupSummary.RelationShipCriteria__c = 'StageName != \'Prospecting\'';
807+
rollupSummary.RelationshipCriteriaFields__c = ' StageName';
808+
rollupSummary.FieldToAggregate__c = 'Amount';
809+
rollupSummary.AggregateOperation__c = 'Sum';
810+
rollupSummary.AggregateResultField__c = 'AnnualRevenue';
811+
rollupSummary.AggregateAllRows__c = true;
812+
rollupSummary.Active__c = true;
813+
rollupSummary.CalculationMode__c = 'Realtime';
814+
insert new List<LookupRollupSummary__c>{ rollupSummary };
815+
816+
// Test data
817+
Account account = new Account();
818+
account.Name = 'Test Account';
819+
insert account;
820+
Opportunity opp = new Opportunity();
821+
opp.Name = 'Test Opportunity';
822+
opp.StageName = 'Closed Won';
823+
opp.CloseDate = System.today();
824+
opp.AccountId = account.Id;
825+
opp.Amount = 100;
826+
827+
Test.startTest();
828+
insert opp;
829+
Test.stopTest();
830+
831+
// Assert rollup
832+
Account reloadedAcct = [
833+
SELECT AnnualRevenue
834+
FROM Account
835+
WHERE Id = :account.Id
836+
];
837+
Assert.areEqual(
838+
100,
839+
reloadedAcct.AnnualRevenue,
840+
'Rollup should have been calculated even with whitespace in criteria fields'
841+
);
842+
}
843+
844+
@IsTest
845+
static void testInvalidRelationshipCriteriaFieldWithWhitespace() {
846+
// Test supported?
847+
if (!TestContext.isSupported())
848+
return;
849+
850+
// Configure rollup
851+
LookupRollupSummary__c rollupSummary = new LookupRollupSummary__c();
852+
rollupSummary.Name = 'Test Whitespace Trim';
853+
rollupSummary.ParentObject__c = 'Account';
854+
rollupSummary.ChildObject__c = 'Opportunity';
855+
rollupSummary.RelationShipField__c = 'AccountId';
856+
rollupSummary.RelationShipCriteria__c = 'StageName != \'Prospecting\'';
857+
rollupSummary.RelationshipCriteriaFields__c = ' InvalidField';
858+
rollupSummary.FieldToAggregate__c = 'Amount';
859+
rollupSummary.AggregateOperation__c = 'Sum';
860+
rollupSummary.AggregateResultField__c = 'AnnualRevenue';
861+
rollupSummary.AggregateAllRows__c = true;
862+
rollupSummary.Active__c = true;
863+
rollupSummary.CalculationMode__c = 'Realtime';
864+
865+
try {
866+
insert new List<LookupRollupSummary__c>{ rollupSummary };
867+
Assert.fail(
868+
'Expected DmlException for invalid relationship criteria fields'
869+
);
870+
} catch (DmlException e) {
871+
Assert.isTrue(
872+
e.getMessage().contains('does not exist on the child object'),
873+
'Expected error message for invalid relationship criteria fields'
874+
);
875+
}
876+
}
877+
}

dlrs/main/classes/RollupSummaries.cls

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -176,9 +176,10 @@ public class RollupSummaries extends fflib_SObjectDomain {
176176
// Aggregate Result field
177177
aggregateResultField = parentObject.getFieldsMap()
178178
.get(lookupRollupSummary.AggregateResultField);
179-
if (aggregateResultField != null)
179+
if (aggregateResultField != null) {
180180
lookupRollupSummary.AggregateResultField = aggregateResultField.getDescribe()
181181
.getName();
182+
}
182183
}
183184
// Check the list of fields expressed in the relationship critiera fields
184185
if (
@@ -191,7 +192,9 @@ public class RollupSummaries extends fflib_SObjectDomain {
191192
);
192193
for (String field : fieldList) {
193194
String trimmedCriteriaField = field.trim();
194-
if (trimmedCriteriaField.length() == 0) continue;
195+
if (String.isBlank(trimmedCriteriaField)) {
196+
continue;
197+
}
195198
SObjectField relationshipCriteriaField = childObject.getFieldsMap()
196199
.get(trimmedCriteriaField);
197200
relationshipCriteriaFields.add(
@@ -355,18 +358,19 @@ public class RollupSummaries extends fflib_SObjectDomain {
355358
childObject != null &&
356359
lookupRollupSummary.RelationshipCriteriaFields != null
357360
) {
358-
359361
String[] fieldList = lookupRollupSummary.RelationshipCriteriaFields.split(
360362
'[\r\n]+'
361363
);
362364
String[] fieldsInError = new List<String>();
363365
for (String field : fieldList) {
364366
String trimmedCriteriaField = field.trim();
365-
if (trimmedCriteriaField.length() > 0)
366-
if (childObject.getFieldsMap().get(trimmedCriteriaField) == null)
367+
if (!String.isBlank(trimmedCriteriaField)) {
368+
if (childObject.getFieldsMap().get(trimmedCriteriaField) == null) {
367369
fieldsInError.add(trimmedCriteriaField);
370+
}
371+
}
368372
}
369-
if (fieldsInError.size() == 1)
373+
if (fieldsInError.size() == 1) {
370374
lookupRollupSummary.Fields.RelationshipCriteriaFields.addError(
371375
error(
372376
'Field ' +
@@ -376,7 +380,7 @@ public class RollupSummaries extends fflib_SObjectDomain {
376380
LookupRollupSummary__c.RelationshipCriteriaFields__c
377381
)
378382
);
379-
else if (fieldsInError.size() > 1)
383+
} else if (fieldsInError.size() > 1) {
380384
lookupRollupSummary.Fields.RelationshipCriteriaFields.addError(
381385
error(
382386
'Fields ' +
@@ -386,6 +390,7 @@ public class RollupSummaries extends fflib_SObjectDomain {
386390
LookupRollupSummary__c.RelationshipCriteriaFields__c
387391
)
388392
);
393+
}
389394
}
390395
// Row limit is only supported for certain operations
391396
LREngine.RollupOperation operation = OPERATION_PICKLIST_TO_ENUMS.get(
@@ -659,4 +664,4 @@ public class RollupSummaries extends fflib_SObjectDomain {
659664

660665
return parsedOrderByClause;
661666
}
662-
}
667+
}

dlrs/main/classes/RollupSummary.cls

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ public class RollupSummary {
219219
return (String) Record.get('RelationshipCriteriaFields__c');
220220
}
221221
set {
222-
if (value != null) {
222+
if (String.isBlank(value)) {
223223
Record.put('RelationshipCriteriaFields__c', value.trim());
224224
} else {
225225
Record.put('RelationshipCriteriaFields__c', null);

0 commit comments

Comments
 (0)