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

Improving the documentation #54

Open
jonnydgreen opened this issue Dec 3, 2021 · 5 comments
Open

Improving the documentation #54

jonnydgreen opened this issue Dec 3, 2021 · 5 comments
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers

Comments

@jonnydgreen
Copy link
Collaborator

We should take a look at the Mercurius Auth documentation to make sure it is super clear for new and existing users. We should also make sure it provides everything a developer needs to use and understand Mercurius Auth (without having to look closer at the underlying code/tests for example). Before starting an implementation, it would be great to understand the following:

  • Does the docs structure make sense? What could be improved?
  • Is the README sufficient? Is it missing any key information?
  • Do we want a best practices/examples section?
  • Anything else that is missing or think should be included?

Keen to hear everyone's thoughts on this!

@jonnydgreen jonnydgreen added documentation Improvements or additions to documentation good first issue Good for newcomers labels Dec 3, 2021
@stolinski
Copy link

stolinski commented Feb 16, 2022

To share some real user thoughts I'm currently hitting while implementing. Where I can't figure out when applyPolicy is even called. I was thinking it would be called anytime a query with the @auth directive on it existed, but I can't seem to get it to fire at all.

Edit: Turns out codegen was stripping the directives from the schema, so I was doing things correctly. On that note of docs though, it might be helpful to give a ELI5 difference between the external and directive based options, as well as a plain english descriptions for things, ie. "applyPolicy is run every time..."

Some more thoughts here. In the examples we show the directive with a parameter
add(x: Int, y: Int): Int @auth(requires: USER)
but in the applyPolicy,

async applyPolicy (authDirectiveAST, parent, args, context, info) {
    return context.auth.identity === 'admin'
  },

This is hardcoded as === 'admin'. So just glancing, the question I wonder is how does the requires: USER connect to the applyPolicy statement.

In this case is authDirectiveAST.arguments[0].value.value the best way to access the directive value? Or am I missing something.

Sorry for the ramble, but as a new user these are the things I'm hitting.

@jonnydgreen
Copy link
Collaborator Author

Thanks for the feedback, that's super helpful! :)

To summarise your key points:

  • Better description on the difference between external and directive options
  • Make sure the examples better match the intended schema
  • Add some examples on the best way to access the directive value (note, this is currently the best way as we should have everything from the AST). We could maybe expose some helpers for this perhaps? In general, I think we could provide some more usage example here!

Would you be interested in helping refine the documentation?

@stolinski
Copy link

Would you be interested in helping refine the documentation?

I'd love to take a stab at it.

@jonnydgreen
Copy link
Collaborator Author

Awesome! Feel free to drop a draft PR in the repo and we can work on it together :)

@jonnydgreen
Copy link
Collaborator Author

@stolinski I was thinking of starting this work later this week, would you be still interested in helping out?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants