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

adding filter on sobjects to mitigate issue with VF limit #1227

Merged
merged 2 commits into from
Jul 29, 2022

Conversation

seanpat09
Copy link
Contributor

Critical Changes

Mitigates bug in #1223 by filtering out EntityDefinitions where the following are false:
IsCustomizable
IsApexTriggerable
IsCustomSetting

Changes

Issues Closed

#1223

@JimBTek JimBTek added this to the 2.17 milestone Jul 26, 2022
@Szandor72
Copy link
Contributor

Can we please doublecheck the early return doesn't make the additional filtering obsolete? It looks to me that way

Can we also check the use of global describe - if we filter the results at one place, how about the others where the DescribeResult is used?

I cannot estimate if the additional check would prevent specific (rare) standard objects from being used by DLRS. I trust you on that :)

@sfenton3 sfenton3 self-requested a review July 26, 2022 18:38
@seanpat09
Copy link
Contributor Author

The initial early return is to check if there are still too many sObjects even after doing the check in the EntityDefinition table. I was trying to change as a little as possible so I didn't want to remove the reference to sobjectDescription, which is caching Schema.getGlobalDescribe(). filtering in the for loops is done because we're still iterating over sObjectDescription. Also those for loops do some other stuff with the describe information that I wasn't sure was available from the EntityDefinition table.

As for the other uses of the DescribeResult, I believe we should be fine because we're not looping over all the results, but just using the a single value in the map retrieved by whatever the user had selected.

I am not sure if there are any standard objects that are blocked by these changes either, but it looks like like the regular standard objects are there.

@sfenton3
Copy link
Member

Testing: Some objects have inordinately large number of child records. User object has the same number of child records as parent records. Something to keep note of when we make more changes to the new wizard.

Security feedback:
I previously received feedback to put hardcoded SOQL into a selector. If not, we should have With Security Enforced because this is periodically reviewed for security concerns on the app exchange, and may raise a flag.

List<EntityDefinition> entityDefinitions = [ SELECT QualifiedApiName FROM EntityDefinition WHERE IsCustomSetting = FALSE AND IsCustomizable = TRUE AND isApexTriggerable = TRUE ];

sfenton3
sfenton3 previously approved these changes Jul 27, 2022
@sfenton3 sfenton3 merged commit 39671ab into main Jul 29, 2022
@sfenton3 sfenton3 deleted the feature/issue-1223-filter-non-rollupable-objects branch July 29, 2022 18:33
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 this pull request may close these issues.

4 participants