Prevent hard-errors when VatPriceGenerator is invoked without a defau…

[firemind]

Guard against exceptions when calling Price#net_amount without a price.

Skip trying to calculate foreign prices in VatPriceGenerator when the default_price does not have an amount set.

Because the VatPriceGenerator is invoked in a before_validate hook when creating a product, this can lead to exceptions when an admin tries to create a product without specifying a price (and there are non-default countries for which a price needs to be calculated).

This PR allows the validations to run without raising exceptions so the admin can see the validation errors.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.64%. Comparing base (3ef8231) to head (94820f8).
⚠️ Report is 32 commits behind head on v3.4.

Additional details and impacted files

@@           Coverage Diff           @@
##             v3.4    #5324   +/-   ##
=======================================
  Coverage   86.64%   86.64%           
=======================================
  Files         580      580           
  Lines       14741    14743    +2     
=======================================
+ Hits        12772    12774    +2     
  Misses       1969     1969           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[waiting-for-dev]

I labeled the PR as a breaking change, as it's changing a behavior some people could be relying on. It'll be merged in the next major.

[adammathys]

I don't believe it's possible for amount to be nil as of v3.1. (See the "Other important changes" section in the CHANGELOG.md entry for v3.1, or the PR that removes nil as a valid value: #3987)

Which means this shouldn't be possible on recent versions of Solidus. (Assuming the task to remove any records with a nil amount has been run and there isn't any invalid data left in the database from before that change.)

[adammathys]

Oops, nevermind. I missed that we were trying to fix a before_validation callback on Spree::Variant.

In which case I think the preferred fix should be skipping that callback instead of modifying this public method. The callback already has a conditional associated to it, so I believe it would be a less invasive change to prevent it from running in the case when a price is invalid. (And we avoid any future confusion from having a nil check in a method where a nil value is considered invalid.)