3
votes

I'm writing a devise-jwt-based authentication system for my graphql-ruby using app. In the process, I've made a mutation for creating a new user account, which takes 7 parameters, which creates quite a lot of repetition in my code:

module Mutations
  class SignUpMutation < Mutations::BaseMutation
    argument :email, String, required: true
    argument :password, String, required: true
    argument :family_name, String, required: true
    argument :family_name_phonetic, String, required: true
    argument :given_name, String, required: true
    argument :given_name_phonetic, String, required: true
    argument :newsletter_optin, Boolean, required: false

    field :token, String, null: true
    field :user, Types::UserType, null: true

    def resolve(email:, password:,
                family_name:, family_name_phonetic:,
                given_name:, given_name_phonetic:,
                newsletter_optin:
               )
      result = {
        token: nil,
        user: nil
      }
      new_user = User.new(
        email: email,
        password: password,
        family_name: family_name,
        family_name_phonetic: family_name_phonetic,
        given_name: given_name,
        given_name_phonetic: given_name_phonetic,
        newsletter_optin: newsletter_optin
      )
      if new_user.save!
        result[:token] = new_user.token
        result[:user] = new_user
      end
      result
    end
  end
end

How could I DRY this up to avoid repeating the names of the mutation arguments all over the place?

Thank you in advance!

5
Typically when people refer to DRY it is about not repeating knowledge. The amount of times a variable name shows up is not necessarily a problem. Something not being DRY would be if you saw the same function declared in multiple different files or the same constant living being declared in multiple different places. That constant should have one concrete source. While you may find it annoying, I don't actually see a problem with this code. - unflores
@unflores That makes sense, it just seems odd to me that there would be no way around this verbosity as Ruby got me used to more concise code :) Thanks for your feedback! - gueorgui
Honestly, I've spent way to much time in the past trying to remove duplication with meta-programming of some sort only to realize that I can't search on it. There are plenty of cases for keeping things DRY, but sometimes more code is the right answer. /shrug - unflores

5 Answers

5
votes

Answering my own question. The correct way to not have to deal with so many parameters is to use Input Objects instead of separate parameters. From the graphql-ruby documentation:

Input object types are complex inputs for GraphQL operations. They’re great for fields that need a lot of structured input, like mutations or search fields.

So I've defined my Input Object as such:

module Types
  class UserAttributes < Types::BaseInputObject
    description 'Attributes for creating or updating a user'
    argument :email, String, required: true
    argument :password, String, required: true
    argument :family_name, String, required: true
    argument :family_name_phonetic, String, required: true
    argument :given_name, String, required: true
    argument :given_name_phonetic, String, required: true
    argument :newsletter_optin, Boolean, required: false
  end
end

and then refactored my mutation like this:

module Mutations
  class SignUpMutation < Mutations::BaseMutation
    argument :attributes, Types::UserAttributes, required: true

    field :token, String, null: true
    field :user, Types::UserType, null: true

    def resolve(attributes:)
      result = {
        token: nil,
        user: nil
      }
      new_user = User.new(attributes.to_hash)
      if new_user.save!
        result[:token] = new_user.token
        result[:user] = new_user
      end
      result
    end
  end
end

Finally, this code feels more ruby-like :)

2
votes

If you'd like, you could do something like this:

[
  :email,
  :password,
  :family_name,
  :family_name_phonetic,
  :given_name,
  :given_name_phonetic
].each do |arg|
  argument arg, String, required: true
end

You might think any more than this is overkill, but Ruby is very flexible. If you really wanted to, you could even do something like

def resolve(email:, password:,
            family_name:, family_name_phonetic:,
            given_name:, given_name_phonetic:,
            newsletter_optin:)
  result = {
    token: nil,
    user: nil
  }
  params = method(__method__).parameters.map(&:last)
  opts = params.map{|p| [p, eval(p.to_s)]}.to_h
  new_user = User.new(opts)
  if new_user.save!
    result[:token] = new_user.token
    result[:user] = new_user
  end
  result
end

You can see this answer for an explanation

If you wanted even more than this, you could use a more detailed field list, and define_method - you could get it all the way to the point where you only type e.g. :email once.

Would that be better? Maybe, if you've got hundreds of these to do. Or if you want to start defining things at runtime.

1
votes

You may try double splat (**) operator.

module Mutations
  class SignUpMutation < Mutations::BaseMutation
    argument :email, String, required: true
    argument :password, String, required: true
    argument :family_name, String, required: true
    argument :family_name_phonetic, String, required: true
    argument :given_name, String, required: true
    argument :given_name_phonetic, String, required: true
    argument :newsletter_optin, Boolean, required: false

    field :token, String, null: true
    field :user, Types::UserType, null: true

    def resolve(**arguments)
      result = {
        token: nil,
        user: nil
      }

      new_user = User.new(
        email: arguments[:email],
        password: arguments[:password],
        family_name: arguments[:family_name],
        family_name_phonetic: arguments[:family_name_phonetic],
        given_name: arguments[:given_name],
        given_name_phonetic: arguments[:given_name_phonetic],
        newsletter_optin: arguments[:newsletter_optin]
      )

      if new_user.save!
        result[:token] = new_user.token
        result[:user] = new_user
      end

      result
    end
  end
end

Of course, creating a new type like you have done would be neater. But there're cases you can combine them together, like

module Mutations
  class SignUpMutation < Mutations::BaseMutation
    argument :another_attribute, String, required: true
    argument :attributes, Types::UserAttributes, required: true

    field :token, String, null: true
    field :user, Types::UserType, null: true

    def resolve(**arguments)
      result = {
        token: nil,
        user: nil
      }

      # use arguments[:another_attribute] for something else.

      new_user = User.new(arguments[:attributes].to_hash)

      if new_user.save!
        result[:token] = new_user.token
        result[:user] = new_user
      end

      result
    end
  end
end
0
votes

In your case I would use input objects as well, but what would you do if you had an existing API with clients relying on the schema and you want to "DRY up" those duplicated arguments that are all the same across different mutations/fields?

If you just go ahead and implement a new input object you'll change the schema and the clients will very likely break. I suppose there is no way of keeping the schema identical when moving existing arguments into an input object, right?

0
votes

A better approach without disturbing the existing GraphQL schema would be to define a InputType with all the common arguments like:

module Types
    module Inputs
        class CommonInputType < Types::Root::BaseInputObject
            graphql_name("my_common_input_type")

            argument :email, String, required: true
            argument :newsletter_optin, Boolean, required: true
            ...
            argument :posts, [Types::Inputs::Post], required: true
        end
    end
end

& use it in some mutation with additional arguments like:

module Mutations
    class CreateUser < Mutations::BaseMutation

        argument :additional_arg_one, ID, required: true
        argument :additional_arg_two, String, required: false
        ...

        Types::Inputs::CommonInputType.arguments.each do |arg,properties|
          argument arg.to_sym, properties.graphql_definition.type
        end

    end
end