Pull Request eval - UI for change document type

[Sebastiaan Janssen]

In order to make the process of accepting larger pull requests a bit more transparent I'll be doing regular screencasts to show what they do and how they behave. 

These posts will stay open for 1 week for feedback that should help the submitter of the pull request to refine some more.
After a week I'll close the topic so that we don't end up bikeshedding.

Today's video focuses on a new dialog that Andy Butland has implemented to allow you to change the document type of a content item.

The screencast: http://www.screenr.com/2DdH

The pull request: https://github.com/umbraco/Umbraco-CMS/pull/18

Personally, I absolutely love this new feature and I think after a bit more polish it should make it into v6.2.0. 

Please have a look and also make sure to inspect the code for anything weird.

We look forward to your feedback!

[Robert Foster]

I’m loving it – the only thing I can think of is in the interests of content security perhaps we can have an option on the document type to turn this feature on/off – i.e. if a document type has been “locked” then the content associated with it cannot be transferred to a new document type. If that makes sense…

 

Robert Foster

--
You received this message because you are subscribed to the Google Groups "Umbraco development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to umbraco-dev...@googlegroups.com.
To post to this group, send email to umbra...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/umbraco-dev/69d20a88-d1e1-4bb8-ae87-acc69f4e3b98%40googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
 
 

[Stephen Gay]

On Thursday, June 27, 2013 5:53:05 PM UTC+2, Sebastiaan Janssen wrote:

Today's video focuses on a new dialog that Andy Butland has implemented to allow you to change the document type of a content item.

We look forward to your feedback!

Loved the screenr, had no time to look at the code right now.
Love the feature.

Does it handle inherited properties all right? Both on the source and target content type?
Would be good if it refreshed the tree (new icon etc).

Stephan

[Jeavon Leopold]

Love this feature, I've seen it requested many times over numerous years! Permission to the feature would need to be configurable so that some users could have access while others would not.

Jeavon

[Tim Geyssens]

Nice work but I would vote to make this into a package instead of adding it to the core

Mainly because I don't see this being a frequently used feature 

[Morten Bock Sørensen]

On Thursday, June 27, 2013 6:53:49 PM UTC+2, Tim Geyssens wrote:

Nice work but I would vote to make this into a package instead of adding it to the core

Mainly because I don't see this being a frequently used feature 

This is actually one of the main things I hear from customers. They realize they picked the wrong doctype, and now they have to copy/paste all the stuff, and maybe update references to the node because it will have a new id, etc. etc.

I don't even think it should be an admin restricted feature. It's not fair to editors, that a wrong choice at the very first step of content creation, locks them down.

/Bock 

[Morten Bock Sørensen]

Another thing is that I think this sort of functionality belongs in the core, so it will be updated along with the core in case something changes in the was content is stored. I would not trust at package to do at job like this correctly over time, across core updates.

/Bock

[Andy Butland]

Thanks for this too and the comments so far.

To address a few:

@Jeavon/@Bock - I've set this up like various existing features so the permission can be assigned (or not) by user type.  By default only the administrator will have it (and actually, even that only with a database update due to the administrator permission are stored).  But other types can be assigned in in the back-office.

@Stephan - no it didn't handle inherited properties - good point, but it does now.  And it also syncs the tree on save.

@Seb - the issue you've found in the screener I still need to look at.  It seems that the API call to ChangeContentType() actually leaves all properties intact, so in certain cases you can still see old properties that are assigned to the content but aren't on the type the content is based on.  I'm not sure whether that should be changed in the API, or just worked around in the UI feature... but will give it some thought.

Just to note to due to not starting this on a clean git branch I've had trouble adding to the pull request... so have closed the original one and moved it to another here: 

https://github.com/umbraco/Umbraco-CMS/pull/38  

So if anyone is looking at the code... that's the one.

Andy

[Michiel van Oosterhout]

+1 for accepting this pull request into the core. The code looks clean, methods not too long, comments where necessary, and code seems to be quite idiomatic.

I personally would prefer it if this option was not in the context menu, but instead a button was added to the Properties tab, right next to the name of the current document type for the node.

[Morten Christensen]

@Andy regarding ChangeContentType() I believe I added the option to not keep differences. But I'll double check - should be handled in the API IMO.

- Morten

--

You received this message because you are subscribed to the Google Groups "Umbraco development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to umbraco-dev...@googlegroups.com.
To post to this group, send email to umbra...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/umbraco-dev/def108a5-e385-48cf-8d8d-7f29e5972408%40googlegroups.com.

[Stephen Gay]

On Thursday, June 27, 2013 10:48:18 PM UTC+2, Andy Butland wrote:

@Stephan - no it didn't handle inherited properties - good point, but it does now.  And it also syncs the tree on save.

Cool.

One more question... what happens to the document in terms of events? Is the document re-saved? re-published? There's no "content type has changed" document event at the moment, but there should be a way for ppl to react to such a change.

Stephan

[Tim Geyssens]

Well shouldn't the doc types be setup to make it as easy as possible for editors (ideally a single type they can choose when creating content) instead of giving them a tool to fix mistakes

Op donderdag 27 juni 2013 21:58:42 UTC+2 schreef Morten Bock Sørensen het volgende:

[Andy Butland]

@Stephan - it calls the ContentService.Save() method and ContentService.Publish() if the piece of content was already published.  So those events would fire at normal.  

[Niels Hartvig]

Love this - fantastic job and a tool that editors have requested and definitely belongs in the core (in the ideal world everything is perfectly planned during a project and/or people don't make mistakes, but that's rarely the reality)!

1) I believe that it's correct to keep the function in the context menu in v6 as that's the current convention (in Belle we have items closer to where it belongs, but in vCurrent it's not the convention to have actions outside of the context menu or toolbar)
2) I love that it matches properties by type and don't give the option of choosing something else = removes a lot of issues with edge cases
3) Would be great if after save that it removed the form and just showed the green success and maybe followed up by a summary of what's been done.

#h5yr!

/n

On Thursday, June 27, 2013 5:53:05 PM UTC+2, Sebastiaan Janssen wrote:

[Andy Butland]

Couple of further updates added to the pull request.

@Morten - thanks, you're right... you did create an override for this method that clears the properties, so I've switched to use that.  That looks to resolve the issue you found in the screener Seb

@Neils - have amended the confirmation screen as per your suggestion.

Andy

[Funka!]

This looks great! Exciting to see this functionality finally coming about.

I think for me (as the developer) that I would find this way more useful than my clients (as the editors) during the development process of a site. Too often a section of a website is developed and during the review process with the client, new changes are requested that are best resolved by changing that section of the website to be made of different document types. If there are just a few sample nodes in there so far, we don't find it too difficult to delete them and recreate them as the new type. So this new feature would make that a little easier.

But what would really make our lives a TON easier is when we already have dozens or hundreds of nodes, possibly already on a live site, that all need converting at once. Are there any plans for "bulk conversion" of nodes, by chance?

As far as the scenario where editors are presented with multiple document types and may choose the wrong one, we don't really run into that issue very much. Usually there is only one logical choice for them anyway, or the fields are so drastically different that the conversion wouldn't work anyway. This also speaks to the question Tim had, wondering whether this would be a "frequently-used" feature or not. So in my case, no, probably not something editors would use frequently (it at all), but certainly of an immense help during the development process where the client likes to keep changing his or her mind after you've already put a ton of work into things, or where a ton of existing nodes in a live site already exist that need to morph into something newer.

Thank you!

[Morten Bock Sørensen]

On Friday, June 28, 2013 9:31:19 AM UTC+2, Tim Geyssens wrote:

Well shouldn't the doc types be setup to make it as easy as possible for editors (ideally a single type they can choose when creating content) instead of giving them a tool to fix mistakes

I guess that is question of taste. I prefer to create multiple "text page" doctypes, that have different types of layout, instead of having one doctype to rule them all, where the rendering of the page is not easy to figure out for the user, and validation becomes difficult, because a property might be mandatory for one template/rendering, and not for another. 

So for that sort of conversion from "TextPage1" to "TextPage2", I think this feature is very useful.

/Bock