-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
feat: Allow short circuit of beforeFind #9770
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
base: alpha
Are you sure you want to change the base?
Changes from all commits
6f65ec5
41bf1b7
21c9289
ca74929
207c3c0
bc68816
d66fa48
34f7741
5825abf
6daa039
ec37789
09fcde2
8de0edb
dd75062
8d6aae6
c5dd665
55c3b3b
d8525e9
04ae88f
f083527
f679ba0
dfd4082
43a87bb
c417118
c6463c3
0c2ad57
92ef147
546e5b0
00b104e
2a5dce9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,36 +23,91 @@ function checkTriggers(className, config, types) { | |
function checkLiveQuery(className, config) { | ||
return config.liveQueryController && config.liveQueryController.hasLiveQuery(className); | ||
} | ||
async function runFindTriggers( | ||
config, | ||
auth, | ||
className, | ||
restWhere, | ||
restOptions, | ||
clientSDK, | ||
context, | ||
isGet | ||
) { | ||
const result = await triggers.maybeRunQueryTrigger( | ||
triggers.Types.beforeFind, | ||
className, | ||
restWhere, | ||
restOptions, | ||
config, | ||
auth, | ||
context, | ||
isGet | ||
); | ||
|
||
restWhere = result.restWhere || restWhere; | ||
restOptions = result.restOptions || restOptions; | ||
|
||
if (result?.objects) { | ||
const objectsFromBeforeFind = result.objects; | ||
|
||
const afterFindProcessedObjects = await triggers.maybeRunAfterFindTrigger( | ||
triggers.Types.afterFind, | ||
auth, | ||
className, | ||
Comment on lines
+53
to
+56
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue: i suggest to introduce a new context variable like "beforeFindReturnObjects" to allow developers to adjust afterFind or skip afterFind logic if beforeFind returned objects |
||
objectsFromBeforeFind, | ||
config, | ||
new Parse.Query(className).withJSON({ where: restWhere, ...restOptions }), | ||
context | ||
); | ||
Comment on lines
+53
to
+61
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question: the after find trigger doesn't have the knowledge of "isGet" ? |
||
|
||
return { | ||
results: afterFindProcessedObjects, | ||
}; | ||
} | ||
|
||
// Returns a promise for an object with optional keys 'results' and 'count'. | ||
const find = async (config, auth, className, restWhere, restOptions, clientSDK, context) => { | ||
const query = await RestQuery({ | ||
method: RestQuery.Method.find, | ||
method: isGet ? RestQuery.Method.get : RestQuery.Method.find, | ||
config, | ||
auth, | ||
className, | ||
restWhere, | ||
restOptions, | ||
clientSDK, | ||
context, | ||
runBeforeFind: false, | ||
}); | ||
|
||
return query.execute(); | ||
} | ||
|
||
// Returns a promise for an object with optional keys 'results' and 'count'. | ||
const find = async (config, auth, className, restWhere, restOptions, clientSDK, context) => { | ||
enforceRoleSecurity('find', className, auth); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question: how the enforce role security was implemented before, it's not clear |
||
return runFindTriggers( | ||
config, | ||
auth, | ||
className, | ||
restWhere, | ||
restOptions, | ||
clientSDK, | ||
context, | ||
false | ||
); | ||
}; | ||
|
||
// get is just like find but only queries an objectId. | ||
const get = async (config, auth, className, objectId, restOptions, clientSDK, context) => { | ||
var restWhere = { objectId }; | ||
const query = await RestQuery({ | ||
method: RestQuery.Method.get, | ||
enforceRoleSecurity('get', className, auth); | ||
return runFindTriggers( | ||
config, | ||
auth, | ||
className, | ||
restWhere, | ||
{ objectId }, | ||
restOptions, | ||
clientSDK, | ||
context, | ||
}); | ||
return query.execute(); | ||
true | ||
); | ||
}; | ||
|
||
// Returns a promise that doesn't resolve to any useful value. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
polish: i can suggest to switch to object interface, because at usage (ex: the isGet) with land with a not truly understanble "true" or "flase".
Having something "isGet: true" is easier to review and maintain, could potentially avoid bugs