Adding support for AfterFind #2968
Conversation
| } | ||
| }; | ||
| logTriggerSuccessBeforeHook(triggerType, className, 'AfterFind', JSON.stringify(results), auth); | ||
| var resultsAsParseObjects = results.map(result => { |
There was a problem hiding this comment.
Replace with response.results as we don't need the temporary variable to be set.
|
@jbpringuey updated the pull request - view changes |
| if (triggerPromise && typeof triggerPromise.then === "function") { | ||
| return triggerPromise.then(function(promiseResults){ | ||
| if(promiseResults) { | ||
| var modifiedJSONResults = promiseResults.map(result => { |
There was a problem hiding this comment.
not sure why but I cannot seem to be able to replace this var with a const or a let as it fails . maybe a JS bug ...
|
ok done with all the changes. Hopefully this one is the good one ! |
| if (triggerPromise && typeof triggerPromise.then === "function") { | ||
| return triggerPromise.then(function(promiseResults){ | ||
| if(promiseResults) { | ||
| var modifiedJSONResults = promiseResults.map(result => { |
| @@ -240,6 +241,51 @@ function logTriggerErrorBeforeHook(triggerType, className, input, auth, error) { | |||
| }); | |||
| } | |||
|
|
|||
| export function maybeRunAfterFindTrigger(triggerType, auth, className, results, config) { | |||
| return new Promise(function (resolve, reject) { | |||
There was a problem hiding this comment.
can you use arrow function instead of function for the promises please?
| } | ||
| }); | ||
| } else { | ||
| var modifiedJSONResults = response.results.map(result => { |
There was a problem hiding this comment.
@flovilmart done with all changes except this one line. I have no idea why if I replace that var with a let it gives an internal error (the other var was actually ok to replace)
There was a problem hiding this comment.
What'a the actual error? You running the tests on node 4.5+?
There was a problem hiding this comment.
node -v returns v6.2.2. I am trying to get a better error than internal error but even wrapping it around try/catch is not giving me the message
There was a problem hiding this comment.
@flovilmart this one is really strange ... I tried to rewrite the code in different ways but it would fail with nothing in the logs ... not sure why
There was a problem hiding this comment.
@flovilmart what do we need to do to get it merged ? I am using a custom version of parse-server but would like to use the standard one and I have no idea about why I cannot turn that into a let .
|
@jbpringuey updated the pull request - view changes |
| return Parse.Object.fromJSON(result); | ||
| }); | ||
| const triggerPromise = trigger(request, response); | ||
| logTriggerAfterHook(triggerType, className, JSON.stringify(modifiedJSONResults), auth); |
There was a problem hiding this comment.
modifiedJSONResults is not defined here...
This is probably why you can't use anything else but var https://github.com/ParsePlatform/parse-server/pull/2968/files#diff-86ec9a2dbbd2c72ada5a115e9f9c4e6aR281 as this variable leaks out of the scope.
This line should be put after the promise has resolved. here
| }); | ||
| return resolve(modifiedJSONResults); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Add the logLine here by replacing with:
}).then((results) => {
logTriggerAfterHook(triggerType, className, JSON.stringify(results), auth);
return results;
});
| const triggerPromise = trigger(request, response); | ||
| logTriggerAfterHook(triggerType, className, JSON.stringify(modifiedJSONResults), auth); | ||
| if (triggerPromise && typeof triggerPromise.then === "function") { | ||
| return triggerPromise.then(function(promiseResults){ |
There was a problem hiding this comment.
use arrow function, beware of the spacings.
| return resolve(); | ||
| } | ||
| const request = getRequestObject(triggerType, auth, null, null, config); | ||
| const response = { |
There was a problem hiding this comment.
The construction of that hook doesn't seem to match the rest of the hooks.
the found objects should be available on req.objects not on res.results
Then the response object should have:
res.success(); // nothing, use the req.results
res.success(modifiedResults); // ignore req.results, and return modified results to the user
req.error(error); // Something wrong happened, return the error to the user
as well as the promise constructs as you handled them.
We should wait till req.success(), req.error() are called OR a returned promise has resolved.
If nothing is return from the afterFind function, we should wait till those methods are called.
This is dangerous to introduce a new usage pattern as we expect to have something done in that trigger (unlike afterSave which doesn't have a req.success or req.error)
|
As proposed at parse-server-modules/parse-evolution@b90c0f0 |
|
@flovilmart thanks for the awesome feedback, this week was crazy , I will fix all those concerns over the week end |
|
@jbpringuey any update? |
|
@jonas-db almost done, I could not get too much time last week end but will be done this week end |
|
@jbpringuey updated the pull request - view changes |
|
@jonas-db @flovilmart hopefully that one is good. |
| @@ -194,7 +204,7 @@ export function getResponseObject(request, resolve, reject) { | |||
| if (request.triggerName === Types.beforeSave) { | |||
| response['object'] = request.object._getSaveJSON(); | |||
| } | |||
| return resolve(response); | |||
| return resolve(response); | |||
There was a problem hiding this comment.
remove the spaces there :)
|
@jbpringuey updated the pull request - view changes |
|
@flovilmart great catch, fixed ! |
|
Thanks @jbpringuey ! Merging now! |
|
thanks ! |
Resubmitting (#2962) and fix git history issue.
For security reasons, I need to hide certain fields (like email) and rows from the API for use cases which are not supported by the current ACL framework. This plugin will enable all sorts of manipulation. With the promise, one can even augment their parse data !