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

Add cloud code resolver example #771

Merged
merged 5 commits into from
Oct 12, 2020

Conversation

185driver
Copy link
Contributor

This PR is a response to a suggestion made in the Parse Community Forum to add a custom Cloud Code resolver example to Parse's GraphQL docs.

I think the example fits the need for a real-life scenario showing the usefulness of custom resolvers and includes examples of both decoding and encoding a Relay Global Object Identification.

The included example does not include a section on how to set up custom schema for use with a Cloud Code resolver on a Parse Server, as that is covered in the README.md file. However, my personal view, having encountered difficulty in finding the setup instructions after much searching of the docs earlier, is that custom schema setup instructions for Cloud Code resolver use should be added to the docs as well. This would make it easier for newer users to discover and use this excellent feature of Parse Server.

Additionally, this PR fixes a minor punctuation error in the definition of a Relay Global Object Identification.

@Moumouls
Copy link
Member

Moumouls commented Oct 1, 2020

I'll take a look tomorrow !

Copy link
Member

@davimacedo davimacedo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding this. It will help a lot. I just believe we should include this under the customization section.

@185driver
Copy link
Contributor Author

Sounds good. Shall I place it on the second level then (as a sibling with Configuration)? Should I do it now or would you like me to wait for additional feedback to be given?

@davimacedo
Copy link
Member

Yes. I believe it makes sense. You can wait for @Moumouls feedback.

Copy link
Member

@Moumouls Moumouls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Example how to pass schema (or ref to another part of the docs)
Little fixes

Comment on lines 293 to 295
const cartItemJson = savedCartItem.toJSON();
Object.assign(cartItemJson, { id: cartItemId });
return cartItemJson;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What you think about ES6 shortcuts

Suggested change
const cartItemJson = savedCartItem.toJSON();
Object.assign(cartItemJson, { id: cartItemId });
return cartItemJson;
// Convert to a JSON object to handle adding the base64 id property.
return {...savedCartItem.toJSON(), id: cartItemId }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice. Thank you.

Comment on lines 250 to 256
```sh
# schema.graphql
extend type Mutation {
addToCart(id: ID!): CartItem! @resolve
}
```

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to show how to pass the schema into the ParseGraphQL Server, here developers can think that a simple schema.graphql at the root of the folder can do the job

Copy link
Contributor Author

@185driver 185driver Oct 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. I could use the example found in this PR. It's the code I used in my own project.

I'm hesitant to include the custom schema setup code in this section, as it seems to only be a related topic, and not quite this same one. Would it be okay for me to include the custom schema setup in the Getting Started section instead? If so, it could be inserted in one of a couple places:

  1. In the Using Express.Js sub-section (mixed in with the other ParseServer setup code), or
  2. Given its own sub-section named something like, Adding Custom Schema (I think I would prefer this method)

Comment on lines 330 to 331
**Note:** The `id` is a [Relay Global Object Identification](https://facebook.github.io/relay/graphql/objectidentification.htm); it's **not** a Parse `objectId`. Most of the time the `Relay Node Id` is a `Base64` of the `ParseClass` and the `objectId`. Cloud code does not recognize this type of `id`, so it must be converted to a Parse `objectId` for use in Cloud Code function queries.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid some code like this

  const decoded = Buffer.from(id, "base64").toString();
  const itemObjectId = decoded.split(":")[1];

We need to inform developers that they can use graphql-relay lib.

May be add some reference about the graphql lib graphql-relay and the const { type, id } = fromGlobalId(globalId); method

Link: https://github.com/graphql/graphql-relay-js#object-identification
Lib: https://www.npmjs.com/package/graphql-relay

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I use my Buffer based code in the example, but add a text explanation and reference to the graphql lib as an optional solution, or should I dump the Buffer based code, use the graphql lib for the code example, and then reference the lib in the text?

With that lib available (thanks for mentioning it!) I personally would use it in my own projects, but there's something to be said for using code examples that are a bit more vanilla JS in the documentation too. Your preference?

@Moumouls
Copy link
Member

Moumouls commented Oct 9, 2020

@185driver also may be move your code to customisation.md section :)

@185driver
Copy link
Contributor Author

I thought it might be easier to make the changes suggested by @Moumouls and submit them here for review. Hopefully things are moving in the right direction. Further suggestions are welcome.

Copy link
Member

@Moumouls Moumouls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems really good, example with fromGlobalId is better than the base64 custom code.
Juste add some link about the schema.graphql and i think we are good to go !

Comment on lines 299 to 305
```sh
# schema.graphql
extend type Mutation {
addToCart(id: ID!): CartItem! @resolve
}
```

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a note/link to the getting started "Adding Custom Schema" section for better developer experience

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do, thanks for the reminder.

@185driver
Copy link
Contributor Author

I've added the link back to Getting Started and to the GraphQL Relay lib as suggested.

After adding the custom schema section, the cloud code resolver section felt incomplete without a balancing query section, so I added that. Suggestions to change or remove it are welcome.

My feeling right now is that the two sections feel pretty complete and seem to fit together well. I believe the docs now provide someone like myself with enough official information to venture into Parse Server custom schema with a bit of confidence.

Copy link
Member

@davimacedo davimacedo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks pretty good to me. @Moumouls any additional change that you think we need here?

@Moumouls
Copy link
Member

Everything looks good to me :)

@davimacedo davimacedo merged commit dd2ea4d into parse-community:gh-pages Oct 12, 2020
@davimacedo
Copy link
Member

Good job guys!

@185driver 185driver deleted the cloud-resolver branch October 12, 2020 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants