Skip to content
This repository has been archived by the owner on Jun 27, 2020. It is now read-only.

User can submit access token #61

Merged
merged 23 commits into from
Dec 13, 2018
Merged

Conversation

RobAWilkinson
Copy link
Member

@RobAWilkinson RobAWilkinson commented Dec 5, 2018

Background

This allows a user to submit and save their personal access token

Issue Resolved

Issue #64

Definition of Done

  • User can submit and save their personal access token from the front end
  • User can submit and save their github username from the front end
  • User can submit and save their github org from the front end

Sorry about the lateness on this @hpjaj hopefully integrates well with your branch
pat

@RobAWilkinson RobAWilkinson requested a review from hpjaj December 5, 2018 02:47
@RobAWilkinson
Copy link
Member Author

Probably could also expand on this to accept a github org and a github username as well

app/controllers/graphql_controller.rb Outdated Show resolved Hide resolved
app/graphql/mutations/users/mutation_manifest.rb Outdated Show resolved Hide resolved
app/models/user.rb Outdated Show resolved Hide resolved
app/javascript/components/Profile.js Show resolved Hide resolved
@RobAWilkinson
Copy link
Member Author

need to update this to create a single mutation endpoint that can take all the params, or just a single one. per converstation with @hpjaj

Copy link
Contributor

@hpjaj hpjaj left a comment

Choose a reason for hiding this comment

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

Coming along nicely, @RobAWilkinson 👍

app/javascript/components/Profile.js Outdated Show resolved Hide resolved
app/javascript/components/Profile.js Outdated Show resolved Hide resolved
app/javascript/containers/Application.js Outdated Show resolved Hide resolved
app/javascript/components/Profile.js Outdated Show resolved Hide resolved
app/javascript/packs/application.js Outdated Show resolved Hide resolved
argument :githubUsername, String, required: false
argument :organizationId, Int, required: false
field :user, Types::UserType, null: true
field :errors, [String], null: true
Copy link
Member Author

@RobAWilkinson RobAWilkinson Dec 8, 2018

Choose a reason for hiding this comment

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

returns user: User, errors: [String]
Standard patterns across our graphql endpoints, wonder if it would be possible to abstract the formatting and error handling into some sort of global middleware?

app/controllers/graphql_controller.rb Show resolved Hide resolved
app/javascript/components/Profile.js Outdated Show resolved Hide resolved
class UpdateUserDetailsMutation < Mutations::BaseMutation
description 'Updates a User with the access_token'
null true
argument :personalAccessToken, String, required: false
Copy link
Contributor

Choose a reason for hiding this comment

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

On the ruby side, these will actually be the snake_case versions for these args (i.e. personal_access_token, etc.)

Copy link
Member Author

Choose a reason for hiding this comment

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

yup, graphql-ruby 🧙‍♂️ takes care of that

Copy link
Member Author

Choose a reason for hiding this comment

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

For reference, if you snake_case in ruby, the mutation automatically gets converted to camelCase. As makes sense (in retrospect), just snake_case everywhere and camelCase will happen everywhere in JS

Thanks graphql-ruby 👍 !

Copy link
Contributor

@hpjaj hpjaj left a comment

Choose a reason for hiding this comment

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

Looking great, man. Can you also pls run this on'her for good measure:

$ rubocop -a; rubocop

app/javascript/components/Profile.js Outdated Show resolved Hide resolved
Copy link
Contributor

@hpjaj hpjaj left a comment

Choose a reason for hiding this comment

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

Looks great!

@RobAWilkinson RobAWilkinson merged commit 3dd59a2 into master Dec 13, 2018
@hpjaj hpjaj deleted the user-can-submit-access-token branch December 13, 2018 23:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants