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.

Sign up for GitHub

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

Jump to bottom

fflib_SObjectSelectorTest failure with Lookup relationship and Person Type Accounts #487

Open
CSigelmann opened this issue Apr 29, 2024 · 5 comments · May be fixed by #493
Open

fflib_SObjectSelectorTest failure with Lookup relationship and Person Type Accounts #487

CSigelmann opened this issue Apr 29, 2024 · 5 comments · May be fixed by #493
Assignees
@daveespo

Comments

@CSigelmann
Copy link

Describe the bug
The test toSOQL_When_SystemModeAndChildRelationship_Expect_WellFormedSOQL() relies on fflib_QueryFactory.getChildRelationship(SObjectType), which can get an unexpected relationship when Person Type Accounts are enabled. Opportunities is not guaranteed to be the first child relationship where childRow.getChildSObject() == Opportunity.sObjectType.

To Reproduce

https://github.com/CSigelmann/fflibSelectorTestBug

Steps to reproduce the behavior:

  1. Create default scratch org
  2. Deploy fflib
  3. Enable Person Type Accounts
  4. Create Lookup relationship to Contact on Opportunity with Field Name = "AFP" and Child Relationship Name = "OpportunitiesAFP"
  5. Run anonymous apex
Schema.SObjectType table = Account.sObjectType;
for (Schema.ChildRelationship childRow : table.getDescribe().getChildRelationships())
{
    if (childRow.getChildSObject() == Opportunity.sObjectType)
    {
        System.debug(childRow.getRelationshipName());
    }
}
  1. See that OpportunitiesAFP__pr comes before Opportunities
  2. Run fflib_SObjectSelectorTest.toSOQL_When_SystemModeAndChildRelationship_Expect_WellFormedSOQL()
  3. See that the test fails because OpportunitiesAFP__pr is being used instead of Opportunites

Note: I tried this with a different relationship name and did not experience the same results. OpportunitiesAgent__pr shows up below Opportunities every time. I am not sure how the order is determined, just that Opportunities coming first cannot be relied upon.

Expected behavior
The test should pass regardless of what custom relationships are in the org fflib is deployed to. It should either use fflib_QueryFactory.getChildRelationship(String) to get a specific relationship, or have a matcher that accepts other relationships in the assert.

Screenshots and text of error observed

fflib_SObjectSelectorTest.toSOQL_When_SystemModeAndChildRelationship_Expect_WellFormedSOQL()
System.AssertException: Assertion Failed: 
Expected: SELECT name, id, annualrevenue, accountnumber, (currencyisocode, )?\(SELECT name, id, amount, closedate(, currencyisocode)? FROM Opportunities ORDER BY Name ASC NULLS FIRST \)  FROM Account WITH SYSTEM_MODE ORDER BY Name ASC NULLS FIRST  
Actual:SELECT name, id, annualrevenue, accountnumber, (SELECT name, id, amount, closedate FROM OpportunitiesAFP__pr ORDER BY Name ASC NULLS FIRST )  FROM Account WITH SYSTEM_MODE ORDER BY Name ASC NULLS FIRST

Version
fflib-apex-common @ 41f92e9
fflib-apex-mocks @ b534ae8
@daveespo
Copy link
Contributor

That's an EXCELLENT bug report. Thank you!

While I understand that the test failure is what you're reporting, but are you finding any problems with using Person Accounts in general with SObjectSelector or SObjectUnitOfWork?

@CSigelmann
Copy link
Author

We are still in the very early stages of fflib adoption, and we just enabled Person Accounts a few days ago, so there hasn't been much opportunity to notice problems in general.

We noticed this because at the moment all of the fflib tests are being run when we deploy, so the first deployment after enabling Person Accounts failed with this test failure. We do have a manual workaround for now.

Ideally fflib tests wouldn't be run in our org, but I'm still researching if there's a good way to do that. Right now fflib is just in our source via git submodules.

@CSigelmann
Copy link
Author

To add to this, the version of fflib_QueryFactory.getChildRelationship that is causing this issue is only used by the deprecated versions of fflib_QueryFactory.subselectQuery, so we should not have problems with this outside of the test failure so long as we don't use deprecated functions.

daveespo added a commit that referenced this issue Aug 8, 2024
@daveespo
Fixes #487 - uses the explicit relationship name rather than the SObj…
8945e15 
…ect inference (deprecated) method for resolving the relationship name
@daveespo daveespo linked a pull request Aug 8, 2024 that will close this issue
Fixes #487 - uses the explicit relationship name rather than the SObject inference (deprecated) method for resolving the relationship name #493
Open
@daveespo
Copy link
Contributor

daveespo commented Aug 8, 2024

@CSigelmann -- thank you again for such a perfect repro case and clear diagnosis of the problem. You are indeed correct that that one test method was using the deprecated addQueryFactorySubselect method (which relies on inferring the name using the SObject type and hopes that there is only one relationship)

I have submitted PR#493 with the fix and it should be merged shortly

@daveespo
Copy link
Contributor

daveespo commented Aug 8, 2024

And for posterity, the list of relationship names that popped into existence when Person Accounts were enabled wasn't just the one that CSigelmann reported, there were two others which brought the list to:

08:44:15.1 (80775494)|USER_DEBUG|[6]|DEBUG|OpportunitiesAFP__pr
08:44:15.1 (80837740)|USER_DEBUG|[6]|DEBUG|Opportunities
08:44:15.1 (80894484)|USER_DEBUG|[6]|DEBUG|OpportunitiesAgent__pr
08:44:15.1 (80950490)|USER_DEBUG|[6]|DEBUG|PersonOpportunities

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@daveespo daveespo

Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@daveespo @ImJohnMDaniel @CSigelmann