GetPreviewMarkup action fails when renaming doc type

[sniffdk]

If I select a doc type to use as a data blueprint and then later on change the doc type alias of that doc type, the GetPreviewMarkup action will fail and stop all properties from working. This is because the request includes a 'icContentTypeAlias' property containing the old doc type alias. SC then calls a helper in InnerContent to fetch the doc type based on the saved alias, which then fails because that alias no longer exists.

I think I would have used the id/guid of the doc type instead of its alias, is there any reason why you chose to pass the alias around? And have you discussed any solutions to work around this issue? Just curious 😃

[mattbrailsford]

We copied this approach from NC which we used to store guids but then was convinced to switch to aliases. I can't recall at this time why this was (maybe Lee can remember). We know it makes renaming hard, but there was a reason we changed.

That said, assuming doc types have udi's, maybe this could be a better approach now.

On 1 Nov 2017 9:48 p.m., "Mads Krohn" notifications@github.com wrote:

If I select a doc type to use as a data blueprint and then later on change the doc type alias of that doc type, the GetPreviewMarkup action will fail and stop all properties from working. This is because the request includes a 'icContentTypeAlias' property containing the old doc type alias. SC then calls a helper in InnerContent to fetch the doc type based on the saved alias, which then fails because that alias no longer exists.

I think I would have used the id/guid of the doc type instead of its alias, is there any reason why you chose to pass the alias around? And have you discussed any solutions to work around this issue? Just curious 😃

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/umco/umbraco-stacked-content/issues/21, or mute the thread https://github.com/notifications/unsubscribe-auth/AAgLyQz2SxG-DGHYFNABWDCSQtAN2EyJks5syOcygaJpZM4QO30O .

[sniffdk]

thanks for replying Matt, curious to hear if @leekelleher remembers 😄

[leekelleher]

Hmmmm... 😕

Firstly, thanks for raising @sniffdk!

I've been trying to recall why we did it this way. As far as I recall, we did originally use the Guid in NestedContent (and DocTypeGridEditor), but we had to write Courier resolvers to switch the Guid to the Alias, as that's how Courier worked, (it originally used the doc-type alias as the unique identifier). But then in Umbraco v7.3 the migration code changed all the Guids - in order to normalize them across all environments... and that broke all the NC/DTGE content. 😖

So we changed it to use the Alias instead, running the risk that only a doc-type alias rename would be a problem, and typically only happen during development - in which content could be recreated. (I know, it's an opinionated view).

Right now, I'd prefer us to use the Guid - it makes most sense. @mattbrailsford - I'm not sure of the ramifications of this would be, in terms of backwards compatibility? We could introduce a new property, e.g. "icContentTypeId" - use that, and if that doesn't exist, then fall back on the "icContentTypeAlias"?

[leekelleher]

Ah ha! There was a discussion about this on the NC issues, when support for multiple doctypes got added... https://github.com/umco/umbraco-nested-content/pull/4

[sniffdk]

I read/skimmed the discussion, and yes, it's indeed very opinionated 😆 Not sure I get the whole "Umbraco has this quirk where it can lose your data when renaming doc types, so it's ok for us to do it as well", but I digress 😆

I think what struck me the most, was not data loss but that the property just plain stopped working. Imho, seeing as this package is build against 7.4+ it's definitely a good idea to revert back to guids, don't think they are going anywhere 😄 Personally, I'm all for backwards compatibility, so running with both icContentTypeId and icContentTypeAlias sounds good to me, was thinking along the same lines. Guess that will require a change in Inner Content as well though?

If this is a solution you would be okay with, I'd be happy giving it a shot at a PR, just let me know. Otherwise, I'll just sit tight 😄

[mattbrailsford]

You're welcome to give it a shot. As I say I wonder if UDI's might be a good approach now, but feel free to take a look and see what you think.

Matt

On 6 Nov 2017 9:54 p.m., "Mads Krohn" notifications@github.com wrote:

I read/skimmed the discussion, and yes, it's indeed very opinionated 😆 Not sure I get the whole "Umbraco has this quirk where it can lose your data when renaming doc types, so it's ok for us to do it as well", but I digress 😆

I think what struck me the most, was not data loss but that the property just plain stopped working. Imho, seeing as this package is build against 7.4+ it's definitely a good idea to revert back to guids, don't think they are going anywhere 😄 Personally, I'm all for backwards compatibility, so running with both icContentTypeId and icContentTypeAlias sounds good to me, was thinking along the same lines. Guess that will require a change in Inner Content as well though?

If this is a solution you would be okay with, I'd be happy giving it a shot at a PR, just let me know. Otherwise, I'll just sit tight 😄

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/umco/umbraco-stacked-content/issues/21#issuecomment-342300349, or mute the thread https://github.com/notifications/unsubscribe-auth/AAgLyWUPu6sZlAgM6hfwX3mpe9xjkUDoks5sz4ATgaJpZM4QO30O .

[sniffdk]

Might just be the case, I'll take a look at it asap 😃

[leekelleher]

I was curious about how this could work, it looks like most of it could be handled over on the Inner Content side. I've made a start prototyping it in a separate branch - if anyone wants to take a look?

https://github.com/umco/umbraco-inner-content/tree/dev/contenttype-guid

So far I've only looked at the C# part: InnerContentHelper.cs#L114-L149

---

Interesting thing to note, usage of Guids in Umbraco APIs isn't totally wide spread yet, the only way to get a PublishedContentType object is via PublishedContentType.Get, which still only accepts a string alias.

[sniffdk]

Yeah, that was my take as well. I'll have to read up on the hack 😆 just a little swarmed atm 😞

[leekelleher]

@sniffdk You'll be happy to hear that we've just released v1.1.0 that resolves this issue! 🎉

[sniffdk]

That's bloody awesome! Tnak you for all your hard work 👍 😄