Fix TaxRate.match under ruby 2.3

[jhawthorn]

The behaviour of delete_if has changed in ruby 2.3. Previously, in ruby 2.2, it would delete the items from the Array immediately. Now, in ruby 2.3, the items are removed after iterating over all the items.

This replaces the #delete_if loop from TaxRate.match with more explicit behaviour with a more explicit #each loop. This works in all versions of ruby we support and makes the behaviour of that method more clear.

See https://bugs.ruby-lang.org/issues/10714

This might be made obsolete by @mamhoff's work improving the tax system, but I think it's important to see what the existing behaviour was. I would also like to backport this change to our 1.0.x and 1.1.x branches.

This fixes spec runs under ruby 2.3

[jhawthorn]

Proof this passes on ruby 2.3 https://circleci.com/gh/jhawthorn/solidus/449

[tvdeyen]

Note: The example in the comment is wrong and against existing european tax laws! We have to change this in further PRs.

Correct is: When you ship from Spain to France, your customer will have to pay Spanish VAT. The entire idea of an online store giving "tax refunds" is ... wrong.

See https://en.wikipedia.org/wiki/European_Union_value_added_tax#EU_VAT_area

Disclaimer: No offense to @jhawthorn :)

[mamhoff]

Note also: That is the bit of code that I spent most time trying to understand and that I'm most sure about has to go away. It essentially takes in an AR collection of tax rates, and removes some if they have the same tax category. Which rate is removed depends on the order of tax rates in the collection (hence the intermittent failures in #650 I had to xit) .

[jhawthorn]

I agree. The logic performs is bizarre, seems almost accidental. I think we should probably backport this for 1.0.x and 1.1.x, but ideally get improved logic for our upcoming 1.2 release.

[jordan-brough]

👍

[cbrunsdon]

Good by me too, thanks for updating that hawth. And yes, will be great to get this thing gone.