U4-1339 - SurfaceController returning PartialView - cannot use ModelState ?

Created by Jonas Eriksson 19 Dec 2012, 17:50:04 Updated by Sebastiaan Janssen 05 Sep 2013, 11:12:11

I want to be able to return a model and modelstate after handling a post to a partial view.

I.e. after a form post on a partial I like to be able to see the validation messages and keep the model members in the form.

I did set the model using TempData["MyPartialViewModel"] = model to keep the model (it works, but I'm not sure if that's the recommended way?) However I can't figure how to keep the ModelState (I had an idea to set it manually too, but it's a read only dictionary).

Can RedirectToCurrentUmbracoPage perhaps keep current ModelState ?

Comments

Jonas Eriksson 19 Dec 2012, 18:45:50

This might be because the ViewData is emptied, see http://issues.umbraco.org/issue/U4-1319. http://stackoverflow.com/questions/9348897/mvc-3-validation-messages-not-shown-due-modelstate-lost-after-partialview


Phil Harvey 13 Jan 2013, 20:53:43

I had the same problem because I was using RedirectToCurrentUmbracoPage which performs a browser redirect, at which point all request state ModelState is lost.

I use CurrentUmbracoPage instead, at which point MVC validation started working correctly.

For example...

    [HttpPost]
    public ActionResult Submit(MyModel model)
    {
        if (!ModelState.IsValid)
            return CurrentUmbracoPage();

        // Model was valid, do some work...
    }


Phil Harvey 13 Jan 2013, 20:55:30

I should have added, I'm not sure this is a bug - CurrentUmbracoPage() is the correct way to return to an Umbraco page with validation errors from a SurfaceController, not RedirectToCurrentUmbracoPage ()


Sebastiaan Janssen 28 Jan 2013, 13:08:10

Think this is intended behavior and CurrentUmbracoPage() indeed is the way to go with this, before you do that you can add to TempData or ViewBag. If you still think there's something wrong then please re-open.


Sebastiaan Janssen 29 Jan 2013, 10:55:59

Okay, tests say: When using RedirectToCurrentUmbracoPage() adding a message to TempData works just fine, adding to ViewData results in nothing. When using CurrentUmbracoPage() neither of them work. Shannon, can you shed some light on this please?


Shannon Deminick 29 Jan 2013, 14:12:36

In the wide world of web the correct/standard way to return something from a POST (though Webforms does not follow this standard by default) is:

  • When a form submission is successful, you redirect ... with Umbraco, use RedirectToCurrentUmbracoPage()... or RedirectToUmbracoPage(id)
  • When a form submission is not successful you do not redirect .... with Umbraco, use CurrentUmbracoPage()

In MVC, ViewData is only available when you do not redirect. TempData is only available when you do redirect.

Perhaps this documentation and tutorial will help? http://our.umbraco.org/documentation/Reference/Mvc/forms


Matt Brailsford 29 Jan 2013, 14:20:23

Really? According to this (http://rachelappel.com/when-to-use-viewbag-viewdata-or-tempdata-in-asp.net-mvc-3-applications) TempData should be available for the current and subsequent request? Which would suggest that it should work on the CurrentUmbracoPage() too (I'm pretty sure this worked in v5 too, as it's a pretty standard way to show validation error messages no?)


Shannon Deminick 29 Jan 2013, 14:25:26

I'll double check but TempData should be used only for redirecting, that's the only reason it exists. It also uses SessionState by default so if you are in a load balanced environment you'll need to assign a different TempData provider or an SQL SessionState provider.

If you want to display validation/error messages than you won't be redirecting and should be using ModelState.AddModelError(...); or ViewData if you want to handle validation in a custom way.


Matt Brailsford 29 Jan 2013, 14:30:09

Another link (http://blogs.teamb.com/craigstuntz/2009/01/23/37947/) suggests that TempData was meant to be redirect data in MVC v1, but since v2, TempData now persists untill first access.


Shannon Deminick 29 Jan 2013, 14:32:27

Ok I'll have a look if you can actually use it without being in a redirect in Umbraco but I still stand by my comment... you shouldn't use this unless you are redirecting.


Shannon Deminick 29 Aug 2013, 04:51:38

Ok, finally got around to resolving all of this. First, there's nothing wrong with ViewData:

  • I can confirm that you can access ViewData/ViewBag data when returning CurrentUmbracoPage
  • NOTE: if you are trying to access ViewData/ViewBag from a view that is rendered from a child action, then you need to access it from the @ViewContext.ParentActionViewContext.ViewData because in your SurfaceController, you've added data to the master ViewContext

Second, @Matt is correct you can use TempData without a redirect, I've verified this on a regular MVC site using normal views and child actions. The problem with what I was trying to do was just merging the TempData into the umbraco controller context... however internally when a controller is Executed, MVC will make a call to 'Load' TempData that has been previously saved, if nothing is found it is cleared. So even though we've merged it it, it gets cleared on execute... because we have not explicitly 'Saved' the temp data on the intermediate controller.

See rev: 244dcff8da15c0384404e93b5c9ac4e24cdc95a0


Sebastiaan Janssen 29 Aug 2013, 10:35:55

This works great, except...

Controller:

TempData.Add("Information", "To show later"); return CurrentUmbracoPage();

View: var information = TempData["FormSuccess"]; ...

@information

{code}

Post the form, works fine and I get my TempData value in the view. Hit post again. Boom! "An item with the same key has already been added.". Even though I've accessed it in my view, shouldn't it have been destroyed then?

Using ViewData now for return CurrentUmbracoPage();


Shannon Deminick 30 Aug 2013, 00:59:56

I'll check what the functionality is in a normal MVC site, the reason this might be happening is because another request has not been made yet, which is why you shouldn't really be using TempData unless redirecting. I'll check. In the meantime you won't get errors if you do: TempData["Information"] = "To show later"; instead of Add


Shannon Deminick 30 Aug 2013, 01:34:20

Ok, that is now fixed in 697aa03758d62e1194887639efa41b4a4ef13ed8


Priority: Normal

Type: Bug

State: Fixed

Assignee: Shannon Deminick

Difficulty: Normal

Category:

Backwards Compatible: True

Fix Submitted:

Affected versions: 6.0.0, 6.1.0, 4.11.1, 4.11.2, 4.11.3, 4.11.4, 6.0.1, 4.11.5, 6.0.2, 4.11.6, 6.0.3, 6.0.4, 4.11.7, 6.1.1, 4.11.9, 4.11.8, 4.10.1, 4.11.10, 6.1.3, 6.1.4

Due in version: 6.1.5

Sprint:

Story Points:

Cycle: