Skip to content

Take custom mutations when mounting gem in routes#5

Merged
mcelicalderon merged 3 commits intomasterfrom
take-options-route-mount
Aug 30, 2019
Merged

Take custom mutations when mounting gem in routes#5
mcelicalderon merged 3 commits intomasterfrom
take-options-route-mount

Conversation

@mcelicalderon
Copy link
Copy Markdown
Member

No description provided.

@mcelicalderon mcelicalderon marked this pull request as ready for review August 30, 2019 03:43
@mcelicalderon mcelicalderon requested a review from 00dav00 August 30, 2019 03:43
@mcelicalderon mcelicalderon force-pushed the take-options-route-mount branch from 2d96400 to 8716767 Compare August 30, 2019 04:09
@mcelicalderon mcelicalderon force-pushed the take-options-route-mount branch from 8716767 to 521fc1d Compare August 30, 2019 04:48
argument :email, String, required: true
argument :password, String, required: true

field :authenticable, GraphqlDevise::Types::AuthenticableType, null: true
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why is this removed?


yield current_resource if block_given?

{ success: true, errors: [], authenticable: current_resource }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should authenticable be among the response fields?

Comment thread Gemfile

gemspec

gem 'listen'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why is this removed?

skip: [:sessions, :registrations, :passwords, :confirmations, :omniauth_callbacks, :unlocks]
)

authenticable_type = opts[:authenticable_type] ||
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What about this kind indentation, I find it clearer:

authenticable_type = 
  opts[:authenticable_type] || 
  GraphqlDevise::Util::ClassGetter.call("Types::#{resource}Type") ||
  GraphqlDevise::Types::AuthenticableType

}.freeze

default_mutations.each do |action, mutation|
used_mutation = if mutation_options[action].present?
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What about this style for indentation?

used_mutation = 
   if mutation_options[action].present?
     ...
   else
     ...
   end

Comment thread lib/graphql_devise/util/class_getter.rb Outdated
@@ -0,0 +1,11 @@
module GraphqlDevise
module Util
module ClassGetter
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this class is very similar to safe_constantize. WDYT about using this instead?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤦‍♂

@mcelicalderon mcelicalderon merged commit c4ce4a1 into master Aug 30, 2019
@mcelicalderon mcelicalderon deleted the take-options-route-mount branch August 30, 2019 15:29
@mcelicalderon mcelicalderon added the enhancement New feature or request label Oct 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants