Hi Merchers,

In Merchello v2.1.0, modifying Attributes for an existing Product Option will cause an error when Product Variants are regenerated on Save.

Steps to reproduce

1. Create a Product with SKU "BOX".
2. Check the This variant has options (like size or color) option.
3. Add an Option, "Color", with Attributes "Green" and "Turquoise".
4. Click Save.
5. Verify that you have Product Variants "BOX-Green" and "BOX-Turquoise".
6. Go to the Product Options tab.
7. Remove the "Green" Attribute for the "Color" Option. Replace it with "Brown".
8. Click "Save". An Exception will occur.

When you reload the Product, you will discover that the Product Options and Product Variants are now mismatched:

You will not be able to save the Product successfully until the mismatch has been tidied up, which involves deleting the Product Option entirely.

What happened

When you clicked save, EnsureVariants regenerated the matrix of Product Variants:

var attributeLists = product.GetPossibleProductAttributeCombinations().ToArray();

Then removed all Variants whose Attribute Count didn't match the new number of Product Options on the Product:

    // delete any variants that don't have the correct number of attributes
    var attCount = attributeLists.Any() ? attributeLists.First().Count() : 0;

    var removers = product.ProductVariants.Where(x => x.Attributes.Count() != attCount);
    foreach (var remover in removers.ToArray())
    {
        product.ProductVariants.Remove(remover.Sku);
        _productVariantService.Delete(remover);
    }

Whoops. We've just LEFT our BOX-Green and BOX-Turquoise Variants alive because their number of Attributes is still 1. However, when this is persisted to the database, Product Attribute "Green" no longer exists in the database, so Saving fails with a NullReferenceException:

exceptionMessage=Object reference not set to an instance of an object.
stackTrace=   at Merchello.Core.Persistence.Repositories.MerchelloRepositoryBase`1.AddOrUpdate(TEntity entity)
   at Merchello.Core.Services.ProductVariantService.CreateProductVariantWithKey(IProduct product, String name, String sku, Decimal price, ProductAttributeCollection attributes, Boolean raiseEvents)
   at Merchello.Core.Services.ProductVariantService.CreateProductVariantWithKey(IProduct product, ProductAttributeCollection attributes, Boolean raiseEvents)
   at Merchello.Core.Services.ProductService.EnsureVariants(IProduct product)
   at Merchello.Core.Services.ProductService.Save(IProduct product, Boolean raiseEvents)
   at Merchello.Web.Editors.ProductApiController.PutProduct(ProductDisplay product)
   at lambda_method(Closure , Object , Object[] )
   at System.Web.Http.Controllers.ReflectedHttpActionDescriptor.ActionExecutor.<>c__DisplayClass10.<GetExecutor>b__9(Object instance, Object[] methodParameters)
   at System.Web.Http.Controllers.ReflectedHttpActionDescriptor.ExecuteAsync(HttpControllerContext controllerContext, IDictionary`2 arguments, CancellationToken cancellationToken)
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Controllers.ApiControllerActionInvoker.<InvokeActionAsyncCore>d__0.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Filters.ActionFilterAttribute.<CallOnActionExecutedAsync>d__5.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Web.Http.Filters.ActionFilterAttribute.<CallOnActionExecutedAsync>d__5.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Filters.ActionFilterAttribute.<ExecuteActionFilterAsyncCore>d__0.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Filters.ActionFilterAttribute.<CallOnActionExecutedAsync>d__5.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Web.Http.Filters.ActionFilterAttribute.<CallOnActionExecutedAsync>d__5.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Filters.ActionFilterAttribute.<ExecuteActionFilterAsyncCore>d__0.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Filters.ActionFilterAttribute.<CallOnActionExecutedAsync>d__5.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Web.Http.Filters.ActionFilterAttribute.<CallOnActionExecutedAsync>d__5.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Filters.ActionFilterAttribute.<ExecuteActionFilterAsyncCore>d__0.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Controllers.ActionFilterResult.<ExecuteAsync>d__2.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Filters.AuthorizationFilterAttribute.<ExecuteAuthorizationFilterAsyncCore>d__2.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Web.Http.Dispatcher.HttpControllerDispatcher.<SendAsync>d__1.MoveNext()

Workaround

A temporary workaround for this is to just blitz all Product Variants. Here's the change I made to Merchello.Core, which I've rebuilt and dropped into my site (replaces lines 1562-1567 in ProductService.cs):

    //For now, remove all Product Variants so they all get recreated.
    //  This works around an issue where changing the details but keeping the same quantity of Product Options causes an exception on save when the variant's options can't be found.
    foreach (var remover in product.ProductVariants.ToArray())      //formerly: removers.ToArray()
    {
        product.ProductVariants.Remove(remover.Sku);
        _productVariantService.Delete(remover);
    }

Youtrack

I can't report this via the Merchello Issues YouTrack, because "The license has expired". Any questions, please reply here.

Side Issue

When deleting an Attribute for a Product Option via the UI, clicking on the "X" for the Attribute sometimes deletes another Attribute instead. Haven't looked into this but I suspect sort order and index might be getting mixed up under the hood.

Thanks

-Geoff.