Cannot AddModelError during HttpPost in SurfaceController
When validating a model in a SurfaceController during a form post back I'm unable to use ModelState.AddModelError to add validation error messages for providing feedback to the user. Any ideas why?
I can use ModelState.AddModelError in [ChildActionOnly] rendering method without issue.
[ChildActionOnly]
public ActionResult VerifyEmail(VerifyEmailModel model)
{
// This DOES work
ModelState.AddModelError("SomeProperty", "Some error message to display.");
return View(model);
}
[HttpPost]
[ValidateAntiForgeryToken]
public ActionResult VerifyEmailSubmit(VerifyEmailModel model)
{
// This DOESN'T work
ModelState.AddModelError("SomeProperty", "Some error message to display.");
return CurrentUmbracoPage();
}
Any ideas how to get around this issue?
I guess I can try to code up custom a System.ComponentModel.DataAnnotations.ValidationAttribute but the validation I need to do requires looking up data based on other model properties so starts to get a bit complex.
I have wired this up, as it puzzled me why this wouldn't work for you, the only difference I can see, is I'm returning a PartialView(model) from the ChildActionSurface controller:
[ChildActionOnly]
public ActionResult VerifyEmail(VerifyEmailModel model)
{
// This DOES work
//ModelState.AddModelError("SomeProperty", "Some error message to display.");
return PartialView(model);
}
[HttpPost]
[ValidateAntiForgeryToken]
public ActionResult VerifyEmailSubmit(VerifyEmailModel model)
{
// This DOESN'T work
ModelState.AddModelError("SomeProperty", "Some error message to display.");
return CurrentUmbracoPage();
}
and I call the child action in my template like so:
@Html.Action("VerifyEmail", "TestSurface", new VerifyEmailModel() {})
and my form is just that generated by MVC, but with Html.BeginForm replaced with Html.BeginUmbracoForm..
Hi Marc, thanks for the suggestion. Unfortunately returning a PartialView didn't make any difference.
I'm wondering if it's due to the way I'm hooking up partials in macros? ...
I'm creating MVC partials for use in Umbraco supported by a SurfaceController to handle rendering and post-back.
I then wrap these partials in macros so they can be dropped into page content alongside other content, rather than creating a special Document Type for each special page required (there would be a lot). It also gives the content creators more flexibility with content for the special pages as these partials just get dropped in as macros alongside normal content.
I've moved the call to render the child action to a Macro:
@Html.Action("VerifyEmail", "TestSurface", new VerifyEmailModel() {})
and in both scenarios if I render the Macro in the template using:
@Umbraco.RenderMacro("VerifyEmail")
or allow the macro to be inserted into a rich text area:
The form is displayed, and the modelstate error added correctly when the form posts back to the VerifyEmailSubmit action.
Strange!! What else could be going on ?
Can you check the output of the Macro isn't cached in the back office macro settings ?
and secondly you talk about having to call ModelState.Clear(); in the stack overlflow article - you are not calling that somewhere in your action here ?
It's always difficult to get your head around exactly what people are building but I would probably create a specific VerifyEmail DocumentType, and use a technique called RouteHijacking to handle the request
(https://our.umbraco.org/documentation/reference/routing/custom-controllers)
or
I sometimes have a generic System Page document type, but then create a specific template, eg VerifyEmail template, that the System page can be published with.
These approaches allows you to allocate a different icon to the page in the backoffice tree, that then gives a clue to it's purpose to editors.
or the other option is to use an 'altTemplate', you can trigger any Umbraco page to load with a different template by adding pageUrl?altTemplate=templateName, (by convention this also works for pageUrl/templateName
But having a generic doc type, and allowing the dropping of a macro into it to define it's function I tend to steer clear of, as the number of different macros can confuse editors and it's a bit clunky to edit parameters etc - but that said it should work, (well it does work on my machine :-)) -
I tend to use this approach when it's for an editor to drop a specific piece of functionality, which could appear on any page that has a rich text editor - a specifically formatted piece of text or markup around an image / image gallery / call to action buttons etc and sometimes forms...
... maybe though that is what you are trying to do.
Thanks for all the suggestions - very much appreciated!
Regarding:
@Html.Action("VerifyEmail", "TestSurface", new VerifyEmailModel() {})
I've been using this instead:
@Html.Action("VerifyEmail", "TestSurface")
as I'm finding the new VerifyEmailModel() resets my model state each time so postbacks won't preserve state for user input, and also means my model is no longer pulled from url params. But I guess this doesn't have too much relevance to my problem.
The macro isn't cached either as you can see from the screenshot:
I hear what you're saying about cluttering macros for the end user, but I figured this would be more preferable than cluttering Document Types and Templates as the user will only use macros rarely, but the other two items will pop up on their radar a lot more often when adding vanilla content pages.
I've figured a work-around for now by coding up my own custom ValidationAttribute and using ModelState.Clear() when the model is empty (first time page is viewed before a postback).
Thanks for taking the time to describing the other possible approaches for me. I will look into these as alternatives when time allows. It sounds like I've taken an approach which is a bit outside the norm.
A bit worrying you can get my approach to work without issue though when I can't. I guess something must be slightly different.
the reason I ask is there was a bug introduced for Html.Action in 7.4.1 and I'm wondering if the fix for this has broken ModelState somehow; I don't think you should have to call ModelState.Clear() to prevent the data annotations firing when the form is first rendered.
On the architectural front, using Doc Types, means can decide which Doc Types are allowed to be created underneath which Document Type - so you can keep the choice when new nodes are created uncluttered, and you can't accidentally create your Verify Email page in the wrong place... (once the content has been created, if it's for a one off page, you can disallow that type from being created again, which again keeps the clutter down)
Hi Marc, I have v7.4.2 installed via NuGet. Will be curious to learn if it is indeed a bug.
Good point regarding Document Types, forgot that you restrict them based on parent page Document Type so not a big issue for end CMS users after all. I'll have a play.
Not sure if this helps at all but I've been having the same issue on v7.4.2 when calling :
return CurrentUmbracoPage();
the ModelState information is lost.
This problem occurs when rendering the Action in a Macro Partial, however if I render the Action in a template the ModelState is retained.
It's a shame because using a macro to add content to the grid would of worked very well.
There have been quite a few changes in the core to the way models are handled, to support the new ModelBuilder functionality in 7.4.x
I've downloaded 7.4.3
I'm finding now if my surface controller looks like this:
public class FormSurfaceController : SurfaceController
{
// GET: FormSurface
[ChildActionOnly]
public ActionResult RenderForm(string queryStringParameter)
{
//validate the querystring parameter (or model) in the context of the initial get request
//eg don't use the same model here for RenderForm as you do for handling the form submit
// the redirect back to a querystring is a different context to the posting of the form
// and will probably have different validation rules.
if (queryStringParameter != "fish")
{
return Content("Not a valid fish");
}
var viewModel = new FormModel() { Name = queryStringParameter };
return PartialView("~/views/partials/TestForm.cshtml",viewModel);
}
[HttpPost]
[ValidateAntiForgeryToken]
public ActionResult SubmitForm(FormModel postedForm )
{
if (!ModelState.IsValid)
{
return CurrentUmbracoPage();
}
else
{
return RedirectToCurrentUmbracoPage();
}
}
}
I've upgraded to Umbraco version 7.4.3 assembly: 1.0.5948.18141 via NuGet but still need to use ModelState.Clear() in my surface controller (whether used by macro or directly within a document type template).
If ModelState.Clear() isn't used then validation errors show immediately for the properties, this is before the form is even submitted and validation should be taking place.
I also find that any changes to model made in the submit method of the surface controller, public ActionResult SubmitForm(FormModel postedForm) in your example, does not make it back to the render method. Is there a certain way I should be preserving the model for sharing between the two methods? At present I'm using TempData["Model"] = model to work around this and pass the updates back to the render method.
In my example I'm not using the model in the initial child action RenderForm, because if you did then this will trigger ModelBinding in MVC, between the properties on the Querystring and the Model's validation rules - which is why you are seeing the validation errors on the form before it is posted.
Really you have two different models, maybe with similar properties, but two different contexts that require different validation rules for those context.
So when the RenderForm child action is used, you are validating the context of the initial load of the page (eg I'm presuming values passed via the QueryString)
If these values are invalid you probably want to display a different partial view explaining their invalidness.
But if all is ok, then you can construct your new model to use with the form and pass it to the partial view containing the form. Now when the form is posted back, the ModelBinding fires, and if you have any validation errors against the posted data, returning CurrentUmbracoPage() will keep the ModelState intact, and you will see any Validation errors, along with any text updated in the form fields:
Cannot AddModelError during HttpPost in SurfaceController
When validating a model in a
SurfaceController
during a form post back I'm unable to useModelState.AddModelError
to add validation error messages for providing feedback to the user. Any ideas why?I can use
ModelState.AddModelError
in[ChildActionOnly]
rendering method without issue.Any ideas how to get around this issue?
I guess I can try to code up custom a
System.ComponentModel.DataAnnotations.ValidationAttribute
but the validation I need to do requires looking up data based on other model properties so starts to get a bit complex.Hi Gavin
I have wired this up, as it puzzled me why this wouldn't work for you, the only difference I can see, is I'm returning a PartialView(model) from the ChildActionSurface controller:
and I call the child action in my template like so:
and my form is just that generated by MVC, but with Html.BeginForm replaced with Html.BeginUmbracoForm..
with this setup; then the manually added modelstate error is displayed after postback:
if that helps as a reference point ?
Hi Marc, thanks for the suggestion. Unfortunately returning a PartialView didn't make any difference.
I'm wondering if it's due to the way I'm hooking up partials in macros? ...
I'm creating MVC partials for use in Umbraco supported by a
SurfaceController
to handle rendering and post-back.I then wrap these partials in macros so they can be dropped into page content alongside other content, rather than creating a special Document Type for each special page required (there would be a lot). It also gives the content creators more flexibility with content for the special pages as these partials just get dropped in as macros alongside normal content.
Macro:
The model for this particular page is populated from url parameters in this case, via a link in a verification email sent to the user: http://localhost:12345/identity/verify-email?UserId=2c0152c6-74f3-4824-bb3a-2fc780431e4e&VerificationCode=749529288
Perhaps there's a flaw in the way I'm trying to do things with Umbraco using macros? I also experienced a similar issue with maintaining view / model state which I worked around. I'd posted my question on Stack Overflow, which explains in a bit more detail how I'm attempting to get things to work: http://stackoverflow.com/questions/36464957/umbraco-surfacecontroller-model-state-model-validation-issues
Please advise if my approach is flawed / outside of the Umbraco norm. This is my first Umbraco project so I'm going through a lot of learnings.
Hi Gavin
I've moved the call to render the child action to a Macro:
and in both scenarios if I render the Macro in the template using:
or allow the macro to be inserted into a rich text area:
The form is displayed, and the modelstate error added correctly when the form posts back to the VerifyEmailSubmit action.
Strange!! What else could be going on ?
Can you check the output of the Macro isn't cached in the back office macro settings ?
and secondly you talk about having to call ModelState.Clear(); in the stack overlflow article - you are not calling that somewhere in your action here ?
It's always difficult to get your head around exactly what people are building but I would probably create a specific VerifyEmail DocumentType, and use a technique called RouteHijacking to handle the request (https://our.umbraco.org/documentation/reference/routing/custom-controllers)
or
I sometimes have a generic System Page document type, but then create a specific template, eg VerifyEmail template, that the System page can be published with.
These approaches allows you to allocate a different icon to the page in the backoffice tree, that then gives a clue to it's purpose to editors.
or the other option is to use an 'altTemplate', you can trigger any Umbraco page to load with a different template by adding pageUrl?altTemplate=templateName, (by convention this also works for pageUrl/templateName
But having a generic doc type, and allowing the dropping of a macro into it to define it's function I tend to steer clear of, as the number of different macros can confuse editors and it's a bit clunky to edit parameters etc - but that said it should work, (well it does work on my machine :-)) -
I tend to use this approach when it's for an editor to drop a specific piece of functionality, which could appear on any page that has a rich text editor - a specifically formatted piece of text or markup around an image / image gallery / call to action buttons etc and sometimes forms... ... maybe though that is what you are trying to do.
Hi Marc,
Thanks for all the suggestions - very much appreciated!
Regarding:
I've been using this instead:
as I'm finding the
new VerifyEmailModel()
resets my model state each time so postbacks won't preserve state for user input, and also means my model is no longer pulled from url params. But I guess this doesn't have too much relevance to my problem.The macro isn't cached either as you can see from the screenshot:
I'm not calling
ModelState.Clear()
either, though I will need to as all data validation attributes are run on initial page render for some reason (across all my macros) so the[Required]
validation attribute is displaying an error immediately on viewing the page. Exactly the same as I've described here - http://stackoverflow.com/questions/36464957/umbraco-surfacecontroller-model-state-model-validation-issuesI hear what you're saying about cluttering macros for the end user, but I figured this would be more preferable than cluttering Document Types and Templates as the user will only use macros rarely, but the other two items will pop up on their radar a lot more often when adding vanilla content pages.
I've figured a work-around for now by coding up my own custom
ValidationAttribute
and usingModelState.Clear()
when the model is empty (first time page is viewed before a postback).Thanks for taking the time to describing the other possible approaches for me. I will look into these as alternatives when time allows. It sounds like I've taken an approach which is a bit outside the norm.
A bit worrying you can get my approach to work without issue though when I can't. I guess something must be slightly different.
just a thought which version of Umbraco is this ?
the reason I ask is there was a bug introduced for Html.Action in 7.4.1 and I'm wondering if the fix for this has broken ModelState somehow; I don't think you should have to call ModelState.Clear() to prevent the data annotations firing when the form is first rendered.
On the architectural front, using Doc Types, means can decide which Doc Types are allowed to be created underneath which Document Type - so you can keep the choice when new nodes are created uncluttered, and you can't accidentally create your Verify Email page in the wrong place... (once the content has been created, if it's for a one off page, you can disallow that type from being created again, which again keeps the clutter down)
regards
Marc
Hi Marc, I have v7.4.2 installed via NuGet. Will be curious to learn if it is indeed a bug.
Good point regarding Document Types, forgot that you restrict them based on parent page Document Type so not a big issue for end CMS users after all. I'll have a play.
Thanks again, Gavin
Not sure if this helps at all but I've been having the same issue on v7.4.2 when calling :
the ModelState information is lost.
This problem occurs when rendering the Action in a Macro Partial, however if I render the Action in a template the ModelState is retained. It's a shame because using a macro to add content to the grid would of worked very well.
Hi Chris / Gavin
There have been quite a few changes in the core to the way models are handled, to support the new ModelBuilder functionality in 7.4.x
I've downloaded 7.4.3
I'm finding now if my surface controller looks like this:
and macro looks like this:
and partial view containing the form looks like this:
and model looks like this:
and in one template I render the form like so:
and in another template I render the macro instead:
That in both circumstances (and if I insert the macro into a grid element)
The modelstate is preserved across form posts, and I get validation message regarding required fields when returning CurrentUmbracoPage !!
So fingers crossed it's fixed in 7.4.3 ? (this was the closest issue I could see: http://issues.umbraco.org/issue/U4-8216)
Thanks for the update Marc - much appreciated. And great to see such a quick fix - impressed! I look forward to having a play next version.
I've upgraded to Umbraco version 7.4.3 assembly: 1.0.5948.18141 via NuGet but still need to use
ModelState.Clear()
in my surface controller (whether used by macro or directly within a document type template).If
ModelState.Clear()
isn't used then validation errors show immediately for the properties, this is before the form is even submitted and validation should be taking place.I also find that any changes to model made in the submit method of the surface controller,
public ActionResult SubmitForm(FormModel postedForm)
in your example, does not make it back to the render method. Is there a certain way I should be preserving the model for sharing between the two methods? At present I'm usingTempData["Model"] = model
to work around this and pass the updates back to the render method.Hi Gavin
In my example I'm not using the model in the initial child action RenderForm, because if you did then this will trigger ModelBinding in MVC, between the properties on the Querystring and the Model's validation rules - which is why you are seeing the validation errors on the form before it is posted.
Really you have two different models, maybe with similar properties, but two different contexts that require different validation rules for those context.
So when the RenderForm child action is used, you are validating the context of the initial load of the page (eg I'm presuming values passed via the QueryString)
If these values are invalid you probably want to display a different partial view explaining their invalidness.
But if all is ok, then you can construct your new model to use with the form and pass it to the partial view containing the form. Now when the form is posted back, the ModelBinding fires, and if you have any validation errors against the posted data, returning CurrentUmbracoPage() will keep the ModelState intact, and you will see any Validation errors, along with any text updated in the form fields:
is working on a reply...