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

Additional filters cannot take the name into account #104

Open
saverio-kantox opened this issue Jun 8, 2016 · 3 comments
Open

Additional filters cannot take the name into account #104

saverio-kantox opened this issue Jun 8, 2016 · 3 comments

Comments

@saverio-kantox
Copy link
Contributor

saverio-kantox commented Jun 8, 2016

The comment at https://github.com/cypriss/mutations/blob/master/lib/mutations/model_filter.rb#L5

# default is the attribute name.to_s.camelize.constantize.

indicates that the class of a model (if not given) is generated using the attribute name. But there is no way for the filter to know the name of the attribute, because it's never passed to the initializer:

# https://github.com/cypriss/mutations/blob/master/lib/mutations/hash_filter.rb#L8
@current_inputs[name.to_sym] = type_class.new(options, &block)

the name is never passed to the constructor, with two nasty consequences:

  1. There is no way to set the model class from the name of the attribute.
  2. The options in the filter initializer are always empty, so the class constraint always resolves to Object, making it totally useless.

update

the ModelFilter is treated very differently than the rest of filters, so this actually applies only to custom additional filters, not to the model filter itself.

@saverio-kantox saverio-kantox changed the title Model filter ignores the options Additional filters cannot take the name into account Jun 8, 2016
@saverio-kantox
Copy link
Contributor Author

saverio-kantox commented Jun 8, 2016

Ok the model name is managed specifically in the HashFilter with a specialized method model. So there is no way to generate additional filters that take into account the name.

This class resolves the issue:

module Mutations
  # a filter that is able to take its own name to operate
  class AdditionalNameAwareFilter < ::Mutations::InputFilter
    def initialize(name, opts = {})
      super(opts)
      @name = name
    end

    def self.inherited(base)
      type_name = base.name[/^Mutations::([a-zA-Z]*)Filter$/, 1].underscore

      Mutations::HashFilter.class_eval do
        define_method(type_name) do |name, options = {}, &block|
          @current_inputs[name.to_sym] = base.new(name, options, &block)
        end
      end

      Mutations::ArrayFilter.class_eval do
        define_method(type_name) do |name, options = {}, &block|
          @element_filter = base.new(name, options, &block)
        end
      end
    end
  end
end

@eliank
Copy link
Collaborator

eliank commented Jun 21, 2016

I'm not a big fan of the the AdditionalNameAwareFilter idea. But I don't like the special treatment the ModelFilter has either. Its a bit to much magic just to avoid having to add a parameter

@saverio-kantox
Copy link
Contributor Author

So why can't we just always pass the field name as additional option to the constructor?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants