Database concurrency / entity tracking question (maybe bug)

Hello.
I'm not sure if this is really a bug but I thought I'd ask here.  I'm currently investigating the possibility of moving one of our websites onto the nopCommerce platform which will require some integration with our ERP system to maintain some of the product data and stock levels.  While looking at this I noticed this behaviour in nopCommerce (v3.4) regarding stock levels that I wasn't expecting:

  1. Open a product record for editing in the nop admin interface (one that is set to track inventory)
  2. Open another browser tab, browse to the same product in the store, add it to the cart and checkout.  This effectively reduces to product's stock level in the database by 1 since it's an inventory tracked product and someone just bought one.
  3. Go back to the admin interface tab and click on the save and continue button (without having made any changes to the product).
  4. At this point the product's stock level is reset to the number it was at when the product record was opened for editing and the decrease in stock that occurred while the record was being edited is lost.

I was expecting that there would be some sort of entity change tracking in place that would cause the product entity to only update properties that have actually changed but it seems like all the product's properties are being updated whether they have changed or not.  Is this the expected/desired behaviour? (or is it just me seeing this issue - I can't find any other forum posts that even mention it).

Thanks, Pete.

Hi Pete,

It's not the bug. It's by design. The system saves the value which you have entered on the product details page (a store owner enters "quantity", not "quantity adjustment" value). nopCommerce does not have any mechanism to prevent it.

So that means that any time a store admin wants to make changes to a product (e.g. update a description, change prices or categories) there's a possibility that when they save the changes the stock level will be overwritten with an incorrect value?  Maybe the mechanism for updating stock needs to be separate from the product editing page.

On a busy store, with lots of product administration going on you will run the risk of stock miscount.  

We addressed this with a javascript hack that checked the actual DB stock figure upon saving - and threw up an error if it was different.  This is not ideal though as the editor might lose their changes.

Important problem to address in the core I think ?

I agree the simple thing would be to move the amending of the stock quantities to another screen, but I suppose really the problem still would exist.  Preventing the changing of stock if an order is placed for that product in the mean time with some kind of change tracking would be best.

I've been having a dig through the source code and realised that for my purposes where I don't need the stock level to be manageable through the admin interface (because it'll be synced from our ERP) it's sufficient just to add the StockQuantity property to the list of ignored attributes in the ProductModel to Product Entity map in AutoMapperStartupTask.cs:

Mapper.CreateMap<ProductModel, Product>()
    .ForMember(dest => dest.ProductTags, mo => mo.Ignore())
    ...{code removed for brevity}...
    .ForMember(dest => dest.StockQuantity, mo => mo.Ignore());

Now when the POST Edit ActionResult of the ProductController is called, the StockQuantity property of the Product entity doesn't get overridden by the value posted back in the Product model and the stock levels can't get out of sync.  In addition to this I modified the Nop.Admin _createOrUpdate.Info.cshtml view to display the StockQuantity as plain text instead of as an editable attribute:

<td class="adminData">
  @Model.StockQuantity.ToString()
</td>

As far as I can tell this achieves what I need, though it does mean a change to the nopCommerce code which I was hoping to avoid.  What would be really handy for ERP integrations would be if there was some way in the admin interface to mark certain properties of entities as being controlled outside of nopCommerce which would then automatically add them to the list of ignored attributes in the appropriate model > entity map and make the admin editor display the attribute as plain text instead of an input.

I also have a suggestion for how this could be achieved while still allowing stock levels to be edited from the admin interface.  Make the change above to the AutoMapperStartupTask.cs to prevent the StockQuantity being updated from the Product model. Add a new property to the Nop.Admin.Models.Catalog.ProductModel called StockQuantityAdjustment:

[NopResourceDisplayName("Admin.Catalog.Products.Fields.StockQuantityAdjustment")]
    public int StockQuantityAdjustment { get; set; }

Then add this line to the POST Edit ActionResult of the ProductController.cs:

   //product
    product.StockQuantity += model.StockQuantityAdjustment;
    product = model.ToEntity(product);

And finally update the Nop.Admin _createOrUpdate.Info.cshtml view to this:

<td class="adminData">
  @Model.StockQuantity.ToString() &plusmn;
  @Html.EditorFor(model => model.StockQuantityAdjustment)
  @Html.ValidationMessageFor(model => model.StockQuantityAdjustment)
</td>

This displays the current stock level as plain text and provides an input next to it to submit adjustments to the stock like  +10 or -20.  Since these adjustments are getting applied to the StockQuantity that has just been retrieved from the database instead of the value shown on the page it should mean that stock levels remain in sync.  It might even make more sense to an admin managing stock in nop directly since booking new stock on to the system after a delivery would just be a case of inputing the adjustment quantities from the new delivery rather than having to calculate the new quantity by adding them to the existing quantity.

After working with NOP Commerce for a few weeks, I would not recommend this product to anyone.    The whole architecture is completely hacked together.

Turn on inventory tracking on your products.   It works great when you get 1 user an hour with unlimited quantities.   But now set a limited number of product, say 500.   Now let the customers come in, 1000 concurrent for instances.   The checkout channel, ProcessOrder, has no concurrency checks or locking when it deducts product totals.   SO one customer may buy 200, and the very next customer buys 20, as the orders get processed, say they are a second apart, the 200 gets subtracted, but because the process didnt finish, the 20 overrides it.   So now you  think you only sold 20 items, but in fact you have sold 220.   Scale this up from 2 users to 5000.    

So lets say you decide to go and fix this problem.   WOW, where are the separation of concerns?     Place order is 876 lines..  ahahahahahahh WTF

Many people use this product because they think it is more than it sells itself to be.   If you sell 1 product an hour and have 10 users a day, this is the product for you.   But do yourself a favor and use a hosted service.

Not to mention this doesnt work in a web farm.  Who uses session to manage anything these days.  

I could go on an on..

POS.   Save yourself many man hours and thousands of dollars.   Get something built by qualified engineers.

@petemitch and @TedF,

both are right.

Here in our store, we have around 800 orders/month and we started to suffer from this problem.

We dont agree with Tedf too much. Of course nop has its problems, but we should help to improve it and make it better, retributing all help we got from it.

The biggest problems I see are stock management and order process, because they can mess inventory and cause big trouble.

A nice improvement should be add some timestamp control around source, to prevent cross save when 2 or more people are working on same thing (products, orders, topics, etc).

What do you think? Lets help it and help us too!

Ivan.

Just one point.

Doing this:

.ForMember(dest => dest.StockQuantity, mo => mo.Ignore());

The stock quantity is not displayed anymore, so you get to do it manually where needed. (productlist method for example)

productModel.StockQuantity = x.StockQuantity;

the issue is resovled. please see commit here.