Address validation in plugins bound to fail due to a logic issue in code.

[zikeji]

https://github.com/reactioncommerce/reaction/blob/master/server/methods/accounts/accounts.js#L285

We call the plugin's address validation method (as acquired further up in the code). We check the resulting address to see if it is a truthy variable. If it isn't, we then set the validation to failed - but then we try setting the validatedAddress.failedValidation prop to false (which, why are we setting it to false?). Unfortunately the validatedAddress variable is not an object, as any object - even an empty one, would be truthy. So it is impossible for validatedAddress to ever get to that point as an object, which means it should always fail at: https://github.com/reactioncommerce/reaction/blob/master/server/methods/accounts/accounts.js#L289

The taxes-avalara default plugin also should fail because of this. I have been unable to test it, however examining the code and mocking it in my tests reveals that it should fail as well. https://github.com/reactioncommerce/reaction/blob/master/imports/plugins/included/taxes-avalara/server/methods/taxCalc.js#L271 We set the validatedAddress to an empty string. The reasoning is so that it is a falsy value. If you examine the code you'll find that if no addresses are returned, the validatedAddress variable will be returned falsy (which will fail as mention in the above paragraph).

I haven't examined the code fully, but I would propose a solution as setting validatedAddress to an empty object.

    } else {
      // No address, fail validation
      validated = false;
      validatedAddress.failedValidation = false;
    }

becomes

    } else {
      // No address, fail validation
      validated = false;
      validatedAddress = {
        failedValidation: false
      };
    }

[zikeji]

I wanted to run it by the team first. I'm willing to PR my proposed solution and a fix for this: https://github.com/reactioncommerce/reaction/issues/3473

[zikeji]

You can still cause it to fail by passing a truthy address object as long as your formErrors has a length.

Based on my examination of the code it should be fine going with the above approach as a workaround.

We call the validation method here:

https://github.com/reactioncommerce/reaction/blob/ed0d5cb5033161fa1532a13fc9c4d7e2fd6b0b0f/imports/plugins/core/accounts/client/templates/addressBook/add/add.js#L96

https://github.com/reactioncommerce/reaction/blob/ed0d5cb5033161fa1532a13fc9c4d7e2fd6b0b0f/imports/plugins/core/accounts/client/templates/addressBook/edit/edit.js#L71

If validated is falsy we don't do anything with the validatedAddress object aside from store it in the state.

[Akarshit]

@zikeji failedValidation is not getting used anywhere in the app right now. But yes the bug you found is legitimate and the way to fix it too.

We would welcome a PR for this. Thanks :)

[Akarshit]

Closed by #3504