6

When I update my model I get an error on a child relation which I also try to update.

My model, say Order has a releationship with OrderItem. In my view I have the details of the order together with an editortemplate for the orderitems. When I update the data the link to Order is null but the orderid is filled, so it should be able to link it, TryUpdateModel returns true, the save however fails with:

InvalidOperationException: The operation failed: The relationship could not be changed because one or more of the foreign-key properties is non-nullable. When a change is made to a relationship, the related foreign-key property is set to a null value. If the foreign-key does not support null values, a new relationship must be defined, the foreign-key property must be assigned another non-null value, or the unrelated object must be deleted.]

My update method:

    public ActionResult ChangeOrder(Order model)
    {
        var order = this.orderRepository.GetOrder(model.OrderId);

        if (ModelState.IsValid)
        {
            var success = this.TryUpdateModel(order);
        }

        this.orderRepository.Save();

        return this.View(order);
    }

I tried all solutions I saw on SO and other sources, none succeeded.

I use .Net MVC 3, EF 4.3.1 together with DBContext.

Bas Danen
  • 311
  • 1
  • 10
  • 1
    Generally don't use `TryUpdateModel` as it does **too much** within it's black box. I usually like setting values manually. Try doing things manually without that helper method. – Only Bolivian Here Jul 14 '12 at 19:57
  • You will likely need a solution similar to this: http://stackoverflow.com/a/5540956/270591 (updating detached parent with child collection). `TryUpdateModel` won't do all the necessary work to perform a correct update. – Slauma Jul 14 '12 at 21:06
  • can you provide code of `Order`? – Mohayemin Jul 18 '12 at 06:57

2 Answers2

4

There are a number of code smells here, which I'll try to be elegant with when correcting :)

I can only assume that "Order" is your EF entity? If so, I would highly recommend keeping it separate from the view by creating a view model for your form and copying the data in to it. Your view model should really only contain properties that your form will be using or manipulating.

I also presume orderRepository.GetOrder() is a data layer call that retrieves an order from a data store?

You are also declaring potentially unused variables. "var order =" will be loaded even if your model is invalid, and "var success =" is never used.

TryUpdateModel and UpdateModel aren't very robust for real-world programming. I'm not entirely convinced they should be there at all, if I'm honest. I generally use a more abstracted approach, such as the service / factory pattern. It's more work, but gives you a lot more control.

In your case, I would recommend the following pattern. There's minimal abstraction, but it still gives you more control than using TryUpdateModel / UpdateModel:

    public ActionResult ChangeOrder(OrderViewModel model) {
        if(ModelState.IsValid) {
            // Retrieve original order
            var order = orderRepository.GetOrder(model.OrderId);

            // Update primitive properties
            order.Property1 = model.Property1;
            order.Property2 = model.Property2;
            order.Property3 = model.Property3;
            order.Property4 = model.Property4;

            // Update collections manually
            order.Collection1 = model.Collection1.Select(x => new Collection1Item {
                Prop1 = x.Prop1,
                Prop2 = x.Prop2
            });

            try {
                // Save to repository
                orderRepository.SaveOrder(order);
            } catch (Exception ex) {
                ModelState.AddModelError("", ex.Message);
                return View(model);
            }
            return RedirectToAction("SuccessAction");
        }
        return View(model);
    }

Not ideal, but it should serve you a bit better...

I refer you to this post, which is similar.

Community
  • 1
  • 1
Spikeh
  • 3,540
  • 4
  • 24
  • 49
3

I assume that the user can perform the following actions in your view:

  1. Modify order (header) data
  2. Delete an existing order item
  3. Modify order item data
  4. Add a new order item

To do a correct update of the changed object graph (order + list of order items) you need to deal with all four cases. TryUpdateModel won't be able to perform a correct update of the object graph in the database.

I write the following code directly using a context. You can abstract the use of the context away into your repository. Make sure that you use the same context instance in every repository that is involved in the following code.

public ActionResult ChangeOrder(Order model)
{
    if (ModelState.IsValid)
    {
        // load the order from DB INCLUDING the current order items in the DB
        var orderInDB = context.Orders.Include(o => o.OrderItems)
            .Single(o => o.OrderId == model.OrderId);

        // (1) Update modified order header properties
        context.Entry(orderInDB).CurrentValues.SetValues(model);

        // (2) Delete the order items from the DB
        // that have been removed in the view
        foreach (var item in orderInDB.OrderItems.ToList())
        {
            if (!model.OrderItems.Any(oi => oi.OrderItemId == item.OrderItemId))
                context.OrderItems.Remove(item);
                // Omitting this call "Remove from context/DB" causes
                // the exception you are having
        }

        foreach (var item in model.OrderItems)
        { 
            var orderItem = orderInDB.OrderItems
                .SingleOrDefault(oi => oi.OrderItemId == item.OrderItemId);

            if (orderItem != null)
            {
                // (3) Existing order item: Update modified item properties
                context.Entry(orderItem).CurrentValues.SetValues(item);
            }
            else
            {
                // (4) New order item: Add it
                orderInDB.OrderItems.Add(item);
            }
        }

        context.SaveChanges();

        return RedirectToAction("Index"); // or some other view
    }

    return View(model);
}
Slauma
  • 175,098
  • 59
  • 401
  • 420