Only obfuscate if the instance has the field in question.

[craigmcnamara]

No description provided.

[nettofarah]

Hello, @craigmcnamara.
Thank you for submitting your PR.

Have you run into a bug because of this?
Also, can you add a test for this?

[craigmcnamara]

The code there is playing fast and loose with Object#send. It should check that an attribute exists on an instance before applying the obfuscated value to the instance. My guess is that the || '' was someones attempt to make a test pass with some kind of default value.

I had a fairly involved explore chain, and with the exception of this bug Polo works great.

[craigmcnamara]

It seems that the bug is only on the proc form of obfuscate.

[nettofarah]

maybe you mean scramble here?

[nettofarah]

I think I'm starting to understand this problem a little better now.
Let me try and explain what I think is going on:

We always check that the fields actually exist before we try to obfuscate it.
You can see the part that performs the check here:
https://github.com/IFTTT/polo/blob/master/lib/polo/translator.rb#L38

As you can see, we never actually try to apply a transformation on fields that don't exist. We would apply the transformation if the field is nil though.

With this mind we have two options:

a) Check if the field we're trying to obfuscate is nil and then transform it.
b) Just blindly try to run the block and assume that the lambda function would perform the validation itself.

I tend to like b a little better to be honest. Mostly because checking for nil is just one kind of validation.
The whole idea with passing in a lambda as a custom obfuscation strategy is that Polo doesn't have to be aware of how your data looks like.

I don't want to prolong the discussion for too long though, so I'm ok with adding the if check.
But I would love to know if just adding a nil check to your lambda would fix your problem.

Thank you!

[craigmcnamara]

A little more detail, here https://github.com/IFTTT/polo/blob/master/lib/polo/translator.rb#L38 the intersection of fields to scrub is only checked if there are any fields to scrub. The inner loop operates on the full original set of fields passed in. I tried rewriting it to get the fields from the intersection method, but more things in unrelated specs failed.

If you change the field check back to the original and run the new spec it fails with

  1) Polo Advanced Options obfuscate: [fields] only scrambles instances with the obfuscate field defined
     Failure/Error: instance.send("#{field}=", new_field_value(field, strategy, value))

     NoMethodError:
       undefined method `title=' for #<AR::Chef:0x007fd733702f58>

[nettofarah]

oh, yeah. That makes total sense.
You're correct @craigmcnamara.

I'll go ahead and merge this in.
We can deal with the optimization (using only intersected fields) when we work on: #29.

Thanks for catching this.

[nettofarah]

@craigmcnamara: Would you mind taking some time and responding to this?
#19

[craigmcnamara]

I'd be happy to, thanks for merging the pull.