U4-8147 - When cancelling the ContentService.Saving Event (for a new item) angular should not redirect.

Created by Andrew Cookson 10 Mar 2016, 13:55:49 Updated by Warren Buckley 21 Aug 2017, 09:41:58

Subtask of: U4-9609

EDIT: Shannon noticed (see below) that the real issue is:

The problem here lies in how the angular editor deals with the result and it still tries to redirect to a non-existing content item. You'll see that the route has actually changed from "...&create=true" to "/edit/0" or similar. The route shouldn't be changed if the content wasn't created.

Original Issue summary:

We our trying to add some additional validation to a document type "NewsItemPage". The document type has two properties "hideFromHomePage" and "pinToHomePage" both of which are checkboxes. We are trying to display an error message it both these are checked. The code below works fine for when updating pre-existing "NewsItemPage"'s. But if I try to create a new "NewsItemPage" I get the correct error message, but then a second error message, see "Request error: The URL returned a 404 (not found): /umbraco/backoffice/UmbracoApi/Content/GetById" and then a blank right hand side. I would have expected to be returned to my unsaved content.

protected override void ApplicationStarted(UmbracoApplicationBase umbracoApplication, ApplicationContext applicationContext) { ContentService.Saving += ContentService_Saving; } void ContentService_Saving(Umbraco.Core.Services.IContentService sender, Umbraco.Core.Events.SaveEventArgs e) { foreach (var entity in e.SavedEntities) { if (entity.ContentType.Alias == "NewsItemPage") { if (entity.GetValue("hideFromHomePage") && entity.GetValue("pinToHomePage")) { e.CancelOperation(new EventMessage("Mwahahahaha!", "Sorry but no!", EventMessageType.Error)); } } } }

Comments

Sebastiaan Janssen 10 Mar 2016, 14:24:12

Yes, well this makes sense: you're cancelling the save of the new item. This means that there is no new item, but Umbraco still tries to load it. So I would check if entity.HasIdentity and maybe clear the checkboxes? You can add an error message instead of cancelling the event if HasIdentity is false:

e.Messages.Add(new EventMessage("Mwahahahaha!", "Sorry but no!", EventMessageType.Error));

You would probably also want to hook into the publishing event here in this case to actually cancel the publish when HasIdentity is false.

A better option for the editors would be if you made a property editor that would only allow one selection.


Murray Roke 09 Apr 2017, 21:38:20

I agree with the OP's expectations. "I would have expected to be returned to my unsaved content." This is exactly what happens if the item is not 'new' you see your UNSAVED content with the error message, so you can correct and continue. currently the system throws away all your work. I don't want it thrown away OR saved, I want it kept as work in progress until I can make it save-able.

Excuse any presumption on my part in re-opening this ticket, but I think this should be re-examined.

Cheers. Murray.

P.S. in my case the validation is that a certain property is unique within it's siblings.


Sebastiaan Janssen 10 Apr 2017, 07:12:54

@Murray.Roke I understand what you're saying but you also need to understand that you are cancelling the saving event, meaning YOU are throwing away all your work. You can't say: "hey! don't save this to the database" and expect it to still be saved to the database. It sounds like you want to prevent the publishing of the incorrect data, so make sure to handle the publishing event.


Murray Roke 13 Apr 2017, 09:37:14

What I need to be able to do... Regardless of the name... is this....

Given a new record, when someone presses save, then I want to over-ride that action so it is as if they did not press it. (And also show a message)

... I would call this cacelling save.


Shannon Deminick 10 May 2017, 10:54:44

If you cancel a Save when the item has no Identity (which means it's being created) ... then it will not be persisted at all.

The problem here lies in how the angular editor deals with the result and it still tries to redirect to a non-existing content item. You'll see that the route has actually changed from "...&create=true" to "/edit/0" or similar. The route shouldn't be changed if the content wasn't created.


Murray Roke 10 May 2017, 10:59:25

Not persisted is what I want. And I want to keep the form fields populated... but if there is a 'refresh' going on how would it know what to put in the form fields (since it hasn't been persisted... do we need to 'not refresh' or just pass the input back to the form without persisting it?)

or do you mean to say that if we don't change the route then the refresh won't occur? (sounds great)


Shannon Deminick 10 May 2017, 11:02:00

It would simply not redirect, there's no refreshing needed, etc.., I'm surprised it doesn't already handle this situation, perhaps a regression issue.


Murray Roke 10 May 2017, 23:08:57

Updated to reflect actual issue, and re-opened as suggested by Shannon.


Paulius Putna 25 Jun 2017, 23:09:14

How complicated would be to implement this, I could give my best attempt? I got a project I am currently working on with this issue.


Shannon Deminick 26 Jun 2017, 06:14:23

Would be quite easy. The logic should already exist so there's probably a regression bug somewhere. You can see in the code, there's logic and comments specifically about this:

https://github.com/umbraco/Umbraco-CMS/blob/dev-v7/src/Umbraco.Web/Editors/ContentController.cs#L396

It would be worth doing a history check on this file since it seems perhaps there are comments written to deal with this but it's not actually being dealt with since CreateValidationErrorResponse just returns the standard validation http error status, it's not actually any 'special' status code that the comments are referring to.

You can fork the project and pull the latest dev-v7 branch, open the sln and start it, then you can breakpoint to here to see if it's doing what it's supposed to. Something tells me that the special status code for the UI to handle may not have been implemented or was reverted, or something along those lines.

This is the JS that performs the save: https://github.com/umbraco/Umbraco-CMS/blob/dev-v7/src/Umbraco.Web.UI.Client/src/views/content/content.edit.controller.js#L76

In theory the contentEditingHelper.contentEditorPerformSave should deal with the custom status code, etc...


Priority: Normal

Type: Bug

State: Open

Assignee:

Difficulty: Normal

Category:

Backwards Compatible: True

Fix Submitted:

Affected versions: 7.4.3

Due in version:

Sprint:

Story Points:

Cycle: 4