Conversation
a8aad6a to
f25d7ab
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #429 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 108 108
Lines 14872 15061 +189
Branches 1302 1318 +16
==========================================
+ Hits 14872 15061 +189
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
f25d7ab to
64712a7
Compare
ide
left a comment
There was a problem hiding this comment.
Approved but make the docs really clear that this method usually should not be used. Otherwise it may pop in PRs and people won't know better.
| tableValue: tableTuple[index], | ||
| })), | ||
| [], | ||
| { limit: 1, orderBy: undefined, offset: undefined }, |
There was a problem hiding this comment.
I forget, do we require orderBy and offset to be undefined vs. omitted?
There was a problem hiding this comment.
For this internal interface, yep. For the public method types, they can be omitted.
| * Load one entity where fieldName equals fieldValue, or null if no entity exists. | ||
| * Not cached or coalesced, and not guaranteed to be deterministic if multiple entities match the condition. |
There was a problem hiding this comment.
In the documentation, I would make clear always to use loadByFieldEqualingAsync when the given field uniquely identifies an entity and there is guaranteed to be at most one match.
loadOneByFieldEqualingAsync is only appropriate when there is the possibility of many results that match the given fields, and we are fetching one solely for performance optimization like EntityEdgeDeletionAuthorizationInferenceBehavior.ONE_IMPLIES_ALL.
(Same docs for the non-enforcing loader.)
There was a problem hiding this comment.
Decided to just make it private and call via loader['loadOneByFieldEqualingAsync']. I agree that there isn't a strong-enough docblock that could make it never be used. In the future we could optionally expose a internal method of getting a data manager outside of a loader, though I think that's overkill for now and loader has the cleanest API for field translation and authorization.
64712a7 to
e63584e
Compare
4dacb13 to
be40bcf
Compare
e63584e to
3e7e550
Compare
3e7e550 to
30fb9dc
Compare

Why
As discussed on #407, this moves EntityPrivacyUtils back to the core package since it's a core concept.
How
Move the files, then...
It required adding a new loader method,
loadOneByFieldEqualingAsync, which loads one of a set of entities for use with theEntityEdgeDeletionAuthorizationInferenceBehavior.ONE_IMPLIES_ALLdeletion permission check optimization.It is unlikely that this method will be used too frequently in application code so I marked it as
@internal, but it is still in the types same as every other method.Test Plan
Full coverage and tests for the EntityPrivacyUtils/canViewerDelete.