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

Allow attributes to be set to nil #16

Merged

Conversation

brianstien
Copy link
Collaborator

This adds logic to check a repeated string field called 'nullify'
on incoming proto messages.

For direct attributes, if there is an attribute with the same name
as an attribute specified in the nullify field, this attribute will be
set to nil.

For transformed attributes there is now an option when the transformer
is declared that will specify a name in the nullify field to inspect.
If the our key is in the nullify array of an incoming proto, the
transformer will set the transformed attribute to nil.

This adds logic to check a repeated string field called 'nullify'
on incoming proto messages.  If there is an attribute with the same name
as an attribute specified in the nullify field, this attribute will be
set to nil.

For transformed attributes there is now an option when the transformer
is declared that will specify a name in the nullify field to inspect.
If the our key is in the nullify array of an incoming proto, the
transformer will set the transformed attribute to nil.
#
def attribute_from_proto(attribute, *args, &block)
options = args.extract_options!
transformation = args.first || block
Copy link
Owner

Choose a reason for hiding this comment

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

This is following the previous pattern, but I think it might cleaner to simply call this callable here and eliminate the else clause below. The code will be tighter and it's one less variable to initialize.

Copy link
Owner

Choose a reason for hiding this comment

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

Or we can clean it up even more with something like this:

callable = args.first || block

if callable.is_a?(Symbol)
  callable = lambda { |value| self.__send__(callable, value) }
else
  raise AttributeTransformerError unless callable.respond_to?(:call)
end

It will skip the callable check when we know that we've just created something that is callable.

@liveh2o
Copy link
Owner

liveh2o commented Jul 16, 2015

I like this pattern. It solves a very real problem with using Protocol Buffers and AR, and opens the door for future extensions and options.

Other than my inline comments, would you mind adding a section to the README explaining the problem and how this solves it?

liveh2o added a commit that referenced this pull request Jul 20, 2015
@liveh2o liveh2o merged commit 639b28d into liveh2o:master Jul 20, 2015
@liveh2o
Copy link
Owner

liveh2o commented Jul 20, 2015

👍

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

Successfully merging this pull request may close these issues.

2 participants