apex-enterprise-patterns / fflib-apex-common

Common Apex Library supporting Apex Enterprise Patterns and much more!
BSD 3-Clause "New" or "Revised" License
911 stars 517 forks source link

Selector classes on custom object errored out on parent field "OwnerId" #388

Closed briancodey closed 1 year ago

briancodey commented 2 years ago

Describe the bug With custom object, the OwnerId is referenced to two parent objects: Group and User. When I tried to retrieve related fields on the User object, the Selector class defaulted to the Group object. As such it threw the following error:

Invalid field 'Department' for object 'Group'
Error is in expression '{!dispatch}' in component <apex:page> in page issuessendmail: Class.fflib_QueryFactory.getFieldPath: line 117, column 1
Class.fflib_QueryFactory.selectField: line 207, column 1
Class.fflib_SObjectSelector.configureQueryFactoryFields: line 384, column 1
Class.IssuesSelector.selectById: line 47, column 1
Class.IssuesController.dispatch: line 29, column 1

Base Code:

public Issue__c selectById(Id recordId)
    {
        assertIsAccessible();
                fflib_QueryFactory issueQueryFactory = newQueryFactory();
                new UsersSelector().configureQueryFactoryFields(issueQueryFactory, 'OwnerId');
                new AccountsSelector().configureQueryFactoryFields(issueQueryFactory, 'AccountId__c');
                new ContactSelector().configureQueryFactoryFields(issueQueryFactory, 'ContactId__c');        
                return (Issue__c)Database.query(issueQueryFactory.setCondition('id =: recordId').toSOQL());
    }

What can I do to default/set to User object in the Base Code?

image

capeterson commented 2 years ago

This was a limitation of the initial implementation I wrote way back, partly since I wasn't able to find a reasonable API for expressing what the relationship was expected to be that didn't feel cumbersome with all the polymorphic lookup quirks, and partly because I built this initially for internal FinancialForce usage where this wasn't a common enough problem to justify devoting that much more time to it.

The issue at its core stems from this line of code: https://github.com/apex-enterprise-patterns/fflib-apex-common/blob/master/sfdx-source/apex-common/main/classes/fflib_QueryFactory.cls#L111 always defaulting to the "first" relationship. At the time I remember that User always showed up first in my testing, hence the comment that it "doesn't really matter", since group fields weren't (aren't?) accessible via the cross object notation. Looks like I was unwise to make the assumption that would always be stable.

An ugly hack that would likely be good enough is to test if the relationship is to the common group/user pairing and then always pick user. Not sure if I'll be able to find time to submit a PR for this before the holidays, but if nobody else steps up I wouldn't mind an excused to get my hands dirty with this code again eventually.

@ImJohnMDaniel thanks for flagging this to me.

briancodey commented 2 years ago

Thanks @capeterson for the context into the core issue.

JAertgeerts commented 2 years ago

With increasing objects introducing polymorphic relations (such as Surveys), this is going to be trickier to address

daveespo commented 1 year ago

This may have been fixed over on #137 (and the #402 PR that went with it)

Please re-test and reopen this issue if you find differently