Pricing updated incorrectly with multiple add-ons being first removed and then added

[ollipuu]

Hi and sorry for the confusing title. But I think the issue is also quite confusing and there's a slight chance the problem is again between the chair and the keyboard.

In our web shop we have a product "Product X" with two add-ons, "Add-on 1" and "Add-on 2". The customer can purchase only one of each per product, so the quantity of both add-ons can only be 0 or 1. When the customer selects the product, we update the pricing instance as follows:

pricing
  .plan('product_x', { quantity: 1 })
  .addon('addon_1', { quantity: 0 })
  .addon('addon_2', { quantity: 0 });

So far everything is good. After the customer has selected the product, he/she decides to add the Add-on 1 to cart. Like this:

pricing
  .plan('product_x', { quantity: 1 })
  .addon('addon_1', { quantity: 1 })
  .addon('addon_2', { quantity: 0 });

The cart is updated like it should and everything looks ok. However, customer decides not to purchase add-ons after all and deselects Add-on 1:

pricing
  .plan('product_x', { quantity: 1 })
  .addon('addon_1', { quantity: 0 })
  .addon('addon_2', { quantity: 0 });

Again, everything looks good. But behind the scenes happens something here: https://github.com/recurly/recurly-js/blob/master/lib/recurly/pricing/subscription/index.js#L155 . Add-on 1 is removed correctly, but because this.items.addons didn't have Add-on 2 at this moment, it is added to this.items.addons(with quantity 0).

The issue is visible to customer when Add-on 1 is once again added to cart.

pricing
  .plan('product_x', { quantity: 1 })
  .addon('addon_1', { quantity: 1 })
  .addon('addon_2', { quantity: 0 });

In code Add-on 1 is added correctly, but when we're handling Add-on 2 (note that at this point it's already in this.items.addons) here: https://github.com/recurly/recurly-js/blob/master/lib/recurly/pricing/subscription/index.js#L147 and then here: https://github.com/recurly/recurly-js/blob/master/lib/recurly/pricing/index.js#L118, pos value will be zero and then we call this.items[prop].splice(0); which will then clear the whole addons array.

The result is that when the pricing instance should now have 1 * Product X, 1 * Add-on 1 and 0 * Add-on 2, instead it contains 1 * Product X, 0 * Add-on 1 and 0 * Add-on 2.

But feel free to correct me if I'm not using the Pricing class correctly. I did a quick and dirty unit test for this by updating https://github.com/recurly/recurly-js/blob/master/test/server/fixtures/plans/basic.json with a new add-on:

{
    "add_on_type": "fixed",
    "name": "Blurf",
    "code": "blurf",
    "quantity": 1,
    "price": {
      "USD": {
        "unit_amount": 3.0
      }
    },
    "usage_percentage": null,
    "usage_type": null
  }

And then wrote a new test here: https://github.com/recurly/recurly-js/blob/master/test/unit/pricing/subscription/subscription.test.js

    describe('with multiple addons after both have been removed', () => {
      it('updates the quantity and price accordingly', function (done) {
        this.pricing
          .plan('basic', { quantity : 1 })
          .addon('snarf', { quantity: 1 })
          .addon('blurf', { quantity: 0 })
          .addon('snarf', { quantity: 0 })
          .addon('blurf', { quantity: 0 })
          .addon('snarf', { quantity: 1 })
          .addon('blurf', { quantity: 0 })
          .done(price => {
            assert.equal(this.pricing.items.addons.length, 1);
            assert.equal(this.pricing.items.addons[0].code, 'snarf');
            assert.equal(this.pricing.items.addons[0].quantity, 1);
            assert.equal(price.now.addons, '1.00');
            done();
          });
      });
    });

Test result:

FAILED TESTS:
  Recurly.Pricing.Subscription
    with addons
      with multiple addons after both have been removed
        ✖ updates the quantity and price accordingly
          Chrome 106.0.0.0 (Mac OS 10.15.7)
        Error: Uncaught AssertionError: 0 == 1 (build/test-unit.js:130613)

Thanks in advance!

[douglaslise]

@ollipuu, we fixed a similar error in #790, released under the version 4.22.9. I tested using the unit test scenario you provided and it worked here.
Can you try it again using the latest version or any other version greater than or equal to 4.22.9?