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

Issues with OR in Where clause #494

Closed
Avinava opened this issue Sep 13, 2023 · 11 comments · Fixed by #503 or #520
Closed

Issues with OR in Where clause #494

Avinava opened this issue Sep 13, 2023 · 11 comments · Fixed by #503 or #520

Comments

@Avinava
Copy link

Avinava commented Sep 13, 2023

We have been using apex-rollup for setting up rolls and it has been working great overall. However, we have encountered an issue where the filter criteria and rollup values are incorrect when OR statements are added in the WHERE clauses.

Example:

(
  (
      Status = 'Escalated'
      AND Type IN ('System', 'Issue')
  )
  OR
  (
      Type IN ('Maintenance', 'Break Fix')
      AND Status != 'New'
  )
  OR
  (
      Status = 'In-Progress'
      AND Type = 'Upgrade'
  )
)

I am currently debugging this issue to identify the root cause, but I wanted to bring it to your attention as a heads up in case you were already aware of it

Screenshot 2023-09-13 at 6 28 56 PM

version : v1.5.84

@jamessimone
Copy link
Owner

I would need to see a log generated by the rollup running without successfully updating things. I will say that you have a workaround at present, which is creating a formula field to contain the logic in this where clause and then using that formula field in the where clause instead.

@Avinava
Copy link
Author

Avinava commented Sep 13, 2023

@jamessimone yeah formula fields seems to be only work around, tried adding extra brackets around the clause doesn't seem to help.

Saw you added some fixes around OR clause, I am trying to replicate this using test class.

Will update you once I have something

@Avinava
Copy link
Author

Avinava commented Sep 13, 2023

  @IsTest
  static void shouldWorkMultipleOrInWhereClause() {
    Account acc = [SELECT Id, Name FROM Account];

    insert new List<Opportunity>{
      new Opportunity(StageName = 'one', CloseDate = System.today(), AccountId = acc.Id, Name = 'one', Amount = 2),
      new Opportunity(StageName = 'two', CloseDate = System.today(), AccountId = acc.Id, Name = 'two', Amount = 2),
      new Opportunity(StageName = 'three', CloseDate = System.today(), AccountId = acc.Id, Name = 'three', Amount = 2),
      new Opportunity(StageName = 'four', CloseDate = System.today().addDays(1), AccountId = acc.Id, Name = 'four', Amount = 2)
    };

    Test.startTest();
    Rollup__mdt mdt = new Rollup__mdt(
      CalcItem__c = 'Opportunity',
      LookupObject__c = 'Account',
      RollupFieldOnCalcItem__c = 'StageName',
      LookupFieldOnCalcItem__c = 'AccountId',
      LookupFieldOnLookupObject__c = 'Id',
      RollupFieldOnLookupObject__c = 'SicDesc',
      RollupOperation__c = 'LAST',
      CalcItemWhereClause__c = '(StageName = \'one\' AND CloseDate = Today) OR (StageName = \'four\' AND CloseDate = Today)'
    );
    Rollup.performFullRecalculation(
      mdt 
    );
    Test.stopTest();

    List<Opportunity> opps = Database.query('SELECT Id, StageName FROM Opportunity WHERE ' + mdt.CalcItemWhereClause__c);
    System.assertEquals('one', opps[opps.size() - 1].StageName);
    acc = [SELECT SicDesc FROM Account];
    System.assertEquals('one', acc.SicDesc);
  }

I believe this should be able to replicate the issue

@jamessimone
Copy link
Owner

@Avinava thanks for providing this repro - I must have missed the notification about your last comment. I'll have a look and let you know.

@jamessimone
Copy link
Owner

@Avinava I was able to reproduce this issue and have fixed it. It will be go out in the next version of Apex Rollup to be released, v1.5.86. Please note that you'll need to update your where clause - it should be (StageName = \'one\' AND CloseDate = TODAY) OR (StageName = \'four\' AND CloseDate = TODAY) - in other words, the date literals for TODAY should be capitalized.

With your current version, this will already work, but at the moment it might trigger a full recalc every time due to a related issue with query parsing. That's the fix that will be going out in the next version.

jamessimone added a commit that referenced this issue Sep 18, 2023
…ex from running unexpectedly for nested OR clauses
@Avinava
Copy link
Author

Avinava commented Sep 18, 2023

@jamessimone thanks for the quick fix !
I will try to get the latest commit in my dev box and test it !

@Avinava
Copy link
Author

Avinava commented Sep 18, 2023

@jamessimone regarding the Date literal, I am just curious does capitalisation really matter with soql and dynamic query ?
or it needs to be capitalised for apex-rollups internal classes ?

@jamessimone
Copy link
Owner

@Avinava SOQL is indeed case insensitive but apex rollup's internal classes are only partially SOQL-compliant with regards to sensitivity. I haven't expanded out the date literal support to be case insensitive.

jamessimone added a commit that referenced this issue Sep 25, 2023
* Fixes #494 by updating where clause query parsing to prevent batch apex from running unexpectedly for nested OR clauses
* Making some notes on WEEK_IN_YEAR date literal to come back to later
* Optimizations, added a test on full recalc path to ensure parent reset processor only runs when it's supposed to
* Fixes #500 by properly handling the stateful maintenance of previously reset parents when children span across multiple batches
@Avinava
Copy link
Author

Avinava commented Oct 4, 2023

@jamessimone
I tested lil bit more but seems like there is still an issue.

Below is a test condition that should work

@IsTest
  static void shouldWorkWithINandOR() {
    Account acc = [SELECT Id, Name FROM Account];

    insert new List<Opportunity>{
      new Opportunity(StageName = 'one', CloseDate = System.today(), AccountId = acc.Id, Name = 'one', Amount = 2, LeadSource = 'Web'),
      new Opportunity(StageName = 'two', CloseDate = System.today(), AccountId = acc.Id, Name = 'two', Amount = 2, LeadSource = 'Phone'),
      new Opportunity(StageName = 'three', CloseDate = System.today(), AccountId = acc.Id, Name = 'three', Amount = 2, LeadSource = 'Portal'),
      new Opportunity(StageName = 'four', CloseDate = System.today().addDays(1), AccountId = acc.Id, Name = 'four', Amount = 2, LeadSource = 'Walk In')
    };

    Test.startTest();
    Rollup__mdt mdt = new Rollup__mdt(
      CalcItem__c = 'Opportunity',
      LookupObject__c = 'Account',
      RollupFieldOnCalcItem__c = 'StageName',
      LookupFieldOnCalcItem__c = 'AccountId',
      LookupFieldOnLookupObject__c = 'Id',
      RollupFieldOnLookupObject__c = 'SicDesc',
      RollupOperation__c = 'LAST',
      CalcItemWhereClause__c = '(StageName = \'one\' AND LeadSource IN (\'Web\', \'Phone\') ) OR (LeadSource IN (\'Web\', \'Phone\') AND StageName != \'one\')'
    );
    Rollup.performFullRecalculation(mdt);
    Test.stopTest();

    List<Opportunity> opps = Database.query('SELECT Id, StageName FROM Opportunity WHERE ' + mdt.CalcItemWhereClause__c);
    System.assertEquals('two', opps[opps.size() - 1].StageName);
    acc = [SELECT SicDesc FROM Account];
    System.assertEquals('two', acc.SicDesc);
     // Validate that job ran as queueable - that the where clause was formatted correctly, in other words
    System.assertEquals('Completed', [SELECT Status FROM AsyncApexJob WHERE JobType = 'Queueable' LIMIT 1]?.Status);
  }
  

@jamessimone
Copy link
Owner

I will have a look! Thanks for bringing to my attention

@jamessimone jamessimone reopened this Oct 4, 2023
@jamessimone
Copy link
Owner

@Avinava I've identified where this issue is occurring and I'll be working on a fix for it. However, it's possible that in the meantime you can get this working yourself:

CalcItemWhereClause__c = ' LeadSource IN (\'Web\', \'Phone\') AND (StageName = \'one\' OR StageName != \'one\')'

works, for instance. I am assuming these are simply example values, though? In any case, part of the current issue is that the LeadSource part of the clause is duplicated, and refactoring that out helps massively. I am going to continue to try to fix the actual underlying issue - just pointing out that depending on what your actual values are, you may have a path forward even before a fix is introduced. Thanks!

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