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

docs(gatsby-plugin-guess-js): Document JWT #15969

Merged
merged 2 commits into from
Aug 15, 2019
Merged

docs(gatsby-plugin-guess-js): Document JWT #15969

merged 2 commits into from
Aug 15, 2019

Conversation

omonk
Copy link
Contributor

@omonk omonk commented Jul 22, 2019

Description

gatsby-plugin-guess-js is able to accept a JWT for authenticating the plugin on deployment services.

The documentation previously didn't have any mention about how to use this on deployment services as locally it requires human interaction to grant permissions.

Related Issues

guess-js/gatsby-guess#19 (comment)
#15582

@omonk omonk requested a review from a team as a code owner July 22, 2019 06:56
Copy link
Contributor

@wardpeet wardpeet left a comment

Choose a reason for hiding this comment

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

Thanks, this is valuable! Just added a question & a small change.

@@ -36,6 +36,8 @@ module.exports = {
options: {
// Find the view id in the GA admin in a section labeled "views"
GAViewID: `VIEW_ID`,
// Add JWT token to perform authentication on deployment builds
jwt: GA_JWT
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you explain what GA_JWT holds?

For gatsbyjs.org we're doing the following to make it work:

jwt: {
  client_email: process.env.ANALYTICS_SERVICE_ACCOUNT,
  private_key: process.env.ANALYTICS_SERVICE_ACCOUNT_KEY,
},

ENV vars look like:

ANALYTICS_SERVICE_ACCOUNT="<name>@<project>.iam.gserviceaccount.com"
ANALYTICS_SERVICE_ACCOUNT_KEY="-----BEGIN PRIVATE KEY-----\nPRIVATE KEY STRING\n-----END PRIVATE KEY-----"

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 good point, tried to follow the rabbit hole into Guess webpack and ga packages but haven't been able to work out where this goes yet. I can see that client_email, private_key combo in www/ but it appears to be commented out currently?

Copy link
Contributor

Choose a reason for hiding this comment

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

We have a bug with guess so that's why :)


## How to get a JWT token?

[Here](https://2ality.com/2015/10/google-analytics-api.html)
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like we should just copy and modify https://2ality.com/2015/10/google-analytics-api.html#unlocking-the-google-analytics-api in the readme and give credits to 2ality url.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Another good point, I would probably be welcome to more suggestions as to how to do this in a simpler way (if possible?).

Maybe attaching something to a gatsby preBuild api to get a fresh JWT. My knowledge on JWT is a little limited but I would assume that this wouldn't be the best way as each JWT is valid until you make it non valid correct?

@lannonbr lannonbr changed the title Update documentation for guess-js plugin docs(gatsby-plugin-guess-js): Update documentation for guess-js plugin Jul 22, 2019
@lannonbr lannonbr added the type: documentation An issue or pull request for improving or updating Gatsby's documentation label Jul 22, 2019
Copy link
Contributor

@wardpeet wardpeet left a comment

Choose a reason for hiding this comment

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

It seems like there is already a PR open to create some changes to the documentation which opened a week before you #15774. I'll be closing this one.

We absolutely want to have you as a contributor, so please take a look at our open issues for ideas, and please reach out to us on Twitter at @gatsbyjs with questions.

We offer pair programming sessions if you’d like to work with one of our maintainers to make another contribution.

Thanks again, and we look forward to seeing more PRs from you in the future! 💪💜

@@ -36,6 +36,8 @@ module.exports = {
options: {
// Find the view id in the GA admin in a section labeled "views"
GAViewID: `VIEW_ID`,
// Add JWT token to perform authentication on deployment builds
jwt: GA_JWT
Copy link
Contributor

Choose a reason for hiding this comment

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

We have a bug with guess so that's why :)

@wardpeet wardpeet closed this Jul 23, 2019
@wardpeet
Copy link
Contributor

My mood changes fast 😂. Let's re-open this one and wait for #15774 to get merged and get some of your updates in as well. So you can get credit for your work! 💪

@wardpeet wardpeet reopened this Jul 23, 2019
@wardpeet wardpeet added the status: blocked This issue/PR can't be solved at the moment and shouldn't be closed/merged label Jul 23, 2019
@omonk
Copy link
Contributor Author

omonk commented Jul 23, 2019

🙈 totally missed that! Very similar to my changes yep. I think it would be really beneficially to add a solution to get the JWT from google in there as well!

@wardpeet wardpeet self-assigned this Jul 23, 2019
@wardpeet wardpeet removed the status: blocked This issue/PR can't be solved at the moment and shouldn't be closed/merged label Jul 23, 2019
@wardpeet
Copy link
Contributor

I would copy the first step of https://2ality.com/2015/10/google-analytics-api.html. Add a small paragraph that the token will expire in x time. If you need to update your token automatically please read https://2ality.com/2015/10/google-analytics-api.html

That's how I would suggest it 🤷‍♂

@sidharthachatterjee
Copy link
Contributor

@omonk I've made the changes @wardpeet requested but don't have access to push to your fork. Can you please allow me write access to your fork? I'll push the changes and get this merged in ASAP

Copy link
Contributor

@sidharthachatterjee sidharthachatterjee left a comment

Choose a reason for hiding this comment

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

Can't push further changes (that @wardpeet recommended) to this branch but I don't want to take away credit for this contribution so I'm merging this in and pushing the changes as a follow up to this!

Thank you so much @omonk 🙌

@sidharthachatterjee sidharthachatterjee changed the title docs(gatsby-plugin-guess-js): Update documentation for guess-js plugin docs(gatsby-plugin-guess-js): Document JWT Aug 15, 2019
@sidharthachatterjee sidharthachatterjee merged commit 5dc0b5a into gatsbyjs:master Aug 15, 2019
@gatsbot
Copy link

gatsbot bot commented Aug 15, 2019

Holy buckets, @omonk — we just merged your PR to Gatsby! 💪💜

Gatsby is built by awesome people like you. Let us say “thanks” in two ways:

  1. We’d like to send you some Gatsby swag. As a token of our appreciation, you can go to the Gatsby Swag Store and log in with your GitHub account to get a coupon code good for one free piece of swag. We’ve got Gatsby t-shirts, stickers, hats, scrunchies, and much more. (You can also unlock even more free swag with 5 contributions — wink wink nudge nudge.) See gatsby.dev/swag for details.
  2. We just invited you to join the Gatsby organization on GitHub. This will add you to our team of maintainers. Accept the invite by visiting https://github.com/orgs/gatsbyjs/invitation. By joining the team, you’ll be able to label issues, review pull requests, and merge approved pull requests.

If there’s anything we can do to help, please don’t hesitate to reach out to us: tweet at @gatsbyjs and we’ll come a-runnin’.

Thanks again!

@omonk
Copy link
Contributor Author

omonk commented Aug 16, 2019

@sidharthachatterjee Sorry for my delayed response, have been on holiday!

Thanks very much for merging that!

@wardpeet
Copy link
Contributor

I hope you had an awesome vacation! 🌴 Thanks for your work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: documentation An issue or pull request for improving or updating Gatsby's documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants