-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Adds Hooks API #500
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
Adds Hooks API #500
Conversation
@gfosco maybe we should move Parse.Hooks.js to parse-cloud-express |
@flovilmart updated the pull request. |
4 similar comments
@flovilmart updated the pull request. |
@flovilmart updated the pull request. |
@flovilmart updated the pull request. |
@flovilmart updated the pull request. |
This is great... One thought, how well do you think our current tests cover cloud code usage? |
The tests cover purely hooks creation and calls. The wrapper comes from the multi server cloud code and was fully functioning. That would be pretty easy to setup a basic http server on local host and run tests from there. Do you want me to add that to this test suite? |
} else { | ||
throw new Parse.Error(143, "invalid hook declaration"); | ||
} | ||
//console.log("SAVE!", hook, query); |
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.
Cleanup
@flovilmart updated the pull request. |
function enforceMasterKeyAccess(req) { | ||
return new Promise((fulfill, reject) => { | ||
if (!req.auth.isMaster) { | ||
console.log("REQ AUTH IS NOT MASTER!"); |
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.
Lets not log stuff via console.log. We probably don't need to log anything here, if we do we should log it via the LoggerAdapter.
Awesome PR :) hooks are sweet. Can you mark it as experimental until we can verify that it's compatible with the dashboard? Also we discussed with our backend experts and when we described the strategy of running Cloud Code in a separate process they called it "insane" and "an operational nightmare". It's cool that this supports multiple apps, but I think that the only way we will be able to merge a fully supported way to run multiple apps is if each app has it's own process. If that means that each app has to run on a separate port, then so be it. |
@drew-gross yeah that's the point, I'm decoupling every element to be able to run multiple app in separate PR's that will make it easier for everyone. |
@drew-gross added process.env.PARSE_EXPERIMENTAL_HOOKS_ENABLED for now. |
@flovilmart updated the pull request. |
1 similar comment
@flovilmart updated the pull request. |
rebased and improved architecture with HooksController, HooksRouter to match what's going on now. |
@flovilmart updated the pull request. |
3 similar comments
@flovilmart updated the pull request. |
@flovilmart updated the pull request. |
@flovilmart updated the pull request. |
Adds Parse.Hooks.js in src/cloud-code/Parse.Hooks.js Moves Cloud code related functions in src/cloud-code
@flovilmart updated the pull request. |
@@ -88,6 +89,11 @@ function ParseServer({ | |||
serverURL = requiredParameter('You must provide a serverURL!'), | |||
maxUploadSize = '20mb' | |||
}) { | |||
|
|||
// Initialize the node client SDK automatically | |||
Parse.initialize(appId, javascriptKey || '', masterKey); |
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.
Nit: The || ''
on these two are unnecessary because of the = || ''
and requiredParameter('You must provide a serverURL!')
in the argument destructuring.
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.
sure
@flovilmart updated the pull request. |
@flovilmart updated the pull request. |
👍 👍 👍 ! |
Adds Parse.Hooks.js in src/cloud-code/Parse.Hooks.js
Moves Cloud code related functions in src/cloud-code
Effort to decouple in small PR's #263