search

OrchardCMS / Orchard

Orchard is a free, open source, community-focused Content Management System built on the ASP.NET MVC platform.
https://orchardproject.net
BSD 3-Clause "New" or "Revised" License
2.38k stars 1.12k forks source link

ContentPartDriver updates ContentPart even when the model contains an error #5008

Open orchardbot opened 9 years ago

orchardbot commented 9 years ago

madmox created: https://orchard.codeplex.com/workitem/21179

Using orchard 1.8.1.

When you call updater.AddModelError() from a ContentPartDriver's Editor() method, it doesn't prevent orchard to update the attached ContentPart in the end of the process (more accurately, the changes are not rolled back). This is very annoying when you need to implement field validation for a ContentPart.

This behavior is observable in a stock Orchard installation, when you update your site settings' super user to a non-existant user, the SiteSettingsPartDriver adds a model error ("The user {0} was not found") but the part is updated anyway (and you end up with a superuser-less website, so test this with caution...).

orchardbot commented 9 years ago

@Piedone commented:

That all parts (or view models used in drivers) get updated on a model update is by design: you need to run the model binder to see whether there are any validation errors. If there are, then the transaction is cancelled and although the part objects got populated changes won't be persisted.

That a validation error causes faulty data to be saved for site settings then indeed this is an issue, but a specific one.

orchardbot commented 9 years ago

madmox commented:

OK, I think I misunderstood the issue. When you create or update a content type through the standard process (/Admin/Contents/Create/XXX/) the transaction is cancelled. The issue I observed was with a custom controller using the _contentManager.UpdateEditor() method. In this case you have to manually call _transactionManager.Cancel() if ModelState.IsValid is false. This is actually quite logic.

Still, there is an issue with SiteSettingsPart, and maybe a few more parts. The behavior is quite strange, if I debug the ContentManager.UpdateEditor() call in Orchard.Core.Settings.Controllers.AdminController.IndexPOST(), the part is never updated (and no error is displayed), but if I set the breakpoint after the call the part is updated and no error is added to the model state...