-
Notifications
You must be signed in to change notification settings - Fork 93
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
Comments
Ok the model name is managed specifically in the HashFilter with a specialized method 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 |
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 |
So why can't we just always pass the field name as additional option to the constructor? |
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:
the name is never passed to the constructor, with two nasty consequences:
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.The text was updated successfully, but these errors were encountered: