Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set-ItemTemplate not copying all languages and all versions, if field ID is different #1259

Closed
kningyi opened this issue Mar 11, 2022 · 17 comments
Assignees
Labels
🤩-release-highlight Exciting change that should be highlighted in the release notes and celebrated by SPE fans. area-commands Involves functions and cmdlets. impact-behaviour-change Nothing to be worried about. Now even better than before!
Milestone

Comments

@kningyi
Copy link

kningyi commented Mar 11, 2022

Expected Behavior

Set-ItemTemplate should perform the same as the c# method item.ChangeTemplate(templateItem).

Actual Behavior

When I use Set-ItemTemplate, only field values of the same field ID are preserved.

When I use Set-ItemTemplate with -FieldsToCopy argument, field values are always copied over, even if they are using standard values. Suggest adding a flag to omit copying if the field value is a standard value.

E.g. Template A has field named BannerImage (Item ID: 11111111...) and Template B has field named BannerImage (Item ID: 22222222...)

  • $item.ChangeTemplate($templateItemB) copies over such values by default, for all versions and languages.
  • $item | Set-ItemTemplate -TemplateItem $templateItemB does not copy such values by default.
    BannerImage value is left blank after changing the template.
  • $item | Set-ItemTemplate -TemplateItem $templateItemB -FieldsToCopy @{ "BannerImage" = "BannerImage"; } only copies such values only for the current $item.Version and $item.Language.
    Other versions and languages do not get the values.
  • $item | Set-ItemTemplate -TemplateItem $templateItemB -FieldsToCopy @{ "TextOne" = "TextTwo"; } copies TextOne value explicitly even when it's a standard value, causing the resulting TextTwo field to no longer be using the standard values from Template B.

Steps to Reproduce the Problem

  • Sitecore PowerShell Extensions 6.2.0.34182
  • Sitecore 10.0.1
  • Code:
    $item | Set-ItemTemplate -TemplateItem $templateItem -FieldsToCopy @{ "BannerImage" = "BannerImage"; "TextOne" = "TextTwo" }
    Take note BannerImage from source template has a different field ID as compared to BannerImage from target template
@michaellwest
Copy link
Member

WIP : You can now do this and it will Skip any fields that are detected as StandardValue.

$item = Get-Item -Path "master:{0633B0E2-BFF7-4E91-8558-41C9ABE8C46E}"
$newTemplateId = "{5A2C318B-2CD7-4601-A264-FB74CAC6F3F8}"
$fields = @{ "Field1" = "Field1"; "Field2" = "Field2" }
$item | Set-ItemTemplate -Template $newTemplateId -FieldsToCopy $fields -FieldCopyBehavior SkipStandardValue

@michaellwest
Copy link
Member

michaellwest commented Mar 12, 2022

Inspecting the Sitecore library, Set-ItemTemplate appears to operate in a very similar manner to item.ChangeTemplate(). Can you help provide details as to how Sitecore is copying all the fields?

The only difference I see in the Sitecore code is there is some special handling for cloned items.

@kningyi
Copy link
Author

kningyi commented Mar 12, 2022

From what I can tell after decompiling, item.ChangeTemplate() calls TemplateManager.ChangeTemplate(item, oldTemplate.GetTemplateChangeList(newTemplate)) instead of TemplateManager.ChangeTemplate(item, new TemplateChangeList(oldTemplate, newTemplate)).

Seems like that method performs an additional loop through the item's field list to check for field name if field ID doesn't exist in the new template:

        public TemplateChangeList GetTemplateChangeList(Template target)
        {
            Assert.ArgumentNotNull(target, "target");
            TemplateChangeList templateChangeList = new TemplateChangeList(this, target)
            {
                PreferredLanguage = Context.Language
            };
            TemplateField[] fields = this.GetFields();
            HashSet<string> strs = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
            TemplateField[] templateFieldArray = fields;
            for (int i = 0; i < (int)templateFieldArray.Length; i++)
            {
                TemplateField templateField = templateFieldArray[i];
                TemplateField field = target.GetField(templateField.ID);
                if (field == null)
                {
                    if (!strs.Contains(templateField.Name))
                    {
                        field = target.GetField(templateField.Name);
                        strs.Add(templateField.Name);
                    }
                    if (field == null)
                    {
                        templateChangeList.FieldRemoved(templateField);
                    }
                    else
                    {
                        templateChangeList.FieldChanged(templateField, field);
                    }
                }
            }
            return templateChangeList;
        }

Thanks for the new FieldCopyBehavior flag, but can you make it possible for -FieldsToCopy to copy field values for all languages and versions? It's kind of a let-down if I'm only able to copy over the values for the active item version (v3 en) only, and not the others like v2 en or v1 en or v3 de.

If it helps, my current workaround was to do a loop through in $item.Versions.GetVersions($true) to store the relevant field values for each version/language before changing the template.

@michaellwest
Copy link
Member

I'll check to see how the languages/versions are handled.

I did notice that the "Change Template" UI is doing something different.

Current SPE:

TemplateManager.ChangeTemplate(item, new TemplateChangeList(oldTemplate, newTemplate));

Sitecore UI:

var changeList = oldTemplate.GetTemplateChangeList(newTemplate);
TemplateManager.ChangeTemplate(item, changeList);

michaellwest added a commit that referenced this issue Mar 12, 2022
This is consistent with the Change Template UI in the Content Editor.
@michaellwest michaellwest added impact-behaviour-change Nothing to be worried about. Now even better than before! area-commands Involves functions and cmdlets. 🤩-release-highlight Exciting change that should be highlighted in the release notes and celebrated by SPE fans. labels Mar 12, 2022
@michaellwest
Copy link
Member

michaellwest commented Mar 12, 2022

This will cause the fields to be copied like the Change Template UI.

$item = Get-Item -Path "master:{0633B0E2-BFF7-4E91-8558-41C9ABE8C46E}"
$newTemplateId = "{5A2C318B-2CD7-4601-A264-FB74CAC6F3F8}"
$item | Set-ItemTemplate -Template $newTemplateId

Don't copy any fields

$item = Get-Item -Path "master:{0633B0E2-BFF7-4E91-8558-41C9ABE8C46E}"
$newTemplateId = "{5A2C318B-2CD7-4601-A264-FB74CAC6F3F8}"
$item | Set-ItemTemplate -Template $newTemplateId -FieldsToCopy @{ }

@michaellwest
Copy link
Member

Thinking about it further, you should be able to explicitly get the versions and languages you desire:

Get-Item -Path "master:\content\home" -Version * -Language *

@kningyi
Copy link
Author

kningyi commented Mar 12, 2022

Hm, you're not updating the parameter for TemplateManager.ChangeTemplate in your latest change for SetItemTemplateCommand.cs in line 108?

Thinking about it further, you should be able to explicitly get the versions and languages you desire

True, I sometimes use c# code in my scripts since I remember them easier lol. Just that, after changing the item template, all the other values in the other language versions may be no longer accessible if you don't remember to store them.

@michaellwest
Copy link
Member

Hm, you're not updating the parameter for TemplateManager.ChangeTemplate in your latest change for SetItemTemplateCommand.cs in line 108?

The way I look at it, if you are going to specify the fields to copy then you don't need the change list to include more fields. Right?

@kningyi
Copy link
Author

kningyi commented Mar 12, 2022

Haha true, then you should include in the documentation that fields copied over when the -FieldsToCopy flag is used will only be by ID, since Sitecore UI does it by ID and name.

I wonder though, this code templateChangeList.FieldChanged(sourceTemplateField, targetTemplateField) may actually do all the logic between lines 89-106 and 110-125 for us? Including all language versions?

@michaellwest
Copy link
Member

Haha true, then you should include in the documentation that fields copied over when the -FieldsToCopy flag is used will only be by ID, since Sitecore UI does it by ID and name.

It seems to use the ID when getting the field from the source template, and then Name on the target template. You could essentially do this manually with the Hashtable.

I wonder though, this code templateChangeList.FieldChanged(sourceTemplateField, targetTemplateField) may actually do all the logic between lines 89-106 and 110-125 for us? Including all language versions?

As long as we are allowing the developer to provide a list of fields we'll have to loop through. The change list would include more information than that provided by in the Hashtable. I don't see in the code where Sitecore is grabbing all languages. One place it does set the "Preferred Language" as the Context Language.

@michaellwest
Copy link
Member

Added a way to halt the progress if the field doesn't exist:

$item = Get-Item -Path "master:{0633B0E2-BFF7-4E91-8558-41C9ABE8C46E}"
$newTemplateId = "{5A2C318B-2CD7-4601-A264-FB74CAC6F3F8}"
$item | Set-ItemTemplate -Template $newTemplateId -FieldsToCopy @{ "Field1" = "Fieldx" } -FieldCopyBehavior StopOnFieldError

@kningyi
Copy link
Author

kningyi commented Mar 12, 2022

I don't see in the code where Sitecore is grabbing all languages.

Yeah, it's quite convulted, I think it's the method Sitecore.Data.DataProviders.Sql.SqlDataProvider.ChangeTemplate(ItemDefinition itemDefinition, TemplateChangeList changeList, CallContext context) that eventually runs a SQL query to update all the item field values' old field ids to the new field ids, which probably covers all language versions.

Ok, so I tried out the templateChangeList.FieldChanged(sourceTemplateField, targetTemplateField) and it actually works out great, covering all versions and languages. Probably best used with new TemplateChangeList(oldTemplate, newTemplate) as there seems to be no way to modify existing objects in the TemplateChangeList.Changes array.

Here's my workaround script that worked out pretty well for my own use:

$copyFields = @{
    "Banner Heading"="Title";
    "Banner Sub-Heading"="Subtitle";
    "Text"="Content Text";
}

$templateChangeList = [Sitecore.Data.Templates.TemplateChangeList]::new($oldTemplate, $newTemplate) 

$copiedFields = [System.Collections.ArrayList]@()
$copyFields.GetEnumerator() | ForEach-Object {
    $sourceField = $oldTemplate.GetField($_.key)
    $targetField = $newTemplate.GetField($_.value)
    if ($sourceField -and $targetField) {
        $templateChangeList.FieldChanged($sourceField, $targetField)
        $copiedFields.Add($sourceField.Id) > $null
    }
}

# get default TemplateChangeList to clone over
$templateChangeListDefault = $oldTemplate.GetTemplateChangeList($newTemplate)
$templateChangeListDefault.Changes | ForEach-Object {
    if (!$copiedFields.Contains($_.SourceField.Id)) {

        # to update old field ID to new field id, if old field ID is not in new template, but new template has the field name
        if ($_.Action -eq [Sitecore.Data.Templates.TemplateChangeAction]::ChangeFieldID) {
            $templateChangeList.FieldChanged($_.SourceField, $_.TargetField)
        }

        # to delete unsupported fields from the item after changing the template
        elseif ($_.Action -eq [Sitecore.Data.Templates.TemplateChangeAction]::DeleteField) {
            $templateChangeList.FieldRemoved($_.SourceField)
        }
    }
}

$templateChangeList.Changes | ForEach-Object { Write-Host $_.Action "`t"  $_.SourceField.Name "`t" $_.TargetField.Name }

[Sitecore.Data.Managers.TemplateManager]::ChangeTemplate($item, $templateChangeList)

@michaellwest
Copy link
Member

Would you say that the current implementation causes data loss unexpectedly?

@michaellwest
Copy link
Member

@mikaelnet do you have any insights you can share about this issue? Any recommendations?

@michaellwest
Copy link
Member

How about add another option to copy the fields? Then the Hashtable can serve as a way to map fields that would not convert properly (not match by name).

$item = Get-Item -Path "master:{0633B0E2-BFF7-4E91-8558-41C9ABE8C46E}"
$newTemplateId = "{5A2C318B-2CD7-4601-A264-FB74CAC6F3F8}"
$fieldMapping =  @{ "Field2" = "Field3" }
$item | Set-ItemTemplate -Template $newTemplateId -FieldCopyBehavior CopyAllFields -FieldsToCopy $fieldMapping

@kningyi
Copy link
Author

kningyi commented Mar 13, 2022

Would you say that the current implementation causes data loss unexpectedly?

IMO, using new TemplateChangeList(oldTemplate, newTemplate) results in no data loss, as there's no field values marked for deletion.
However, this results in a mismatch of expectations between Set-ItemTemplate and the Sitecore UI. Also, the Content Editor uses field ID instead of field name to display the field value, so it'll appear to the content author that data has been lost.

Additionally, since Set-ItemTemplate does not remove invalid fields (that are no longer in the new template), the db still stores those unneeded data. It also introduces potential bugs if the developer is using field name to display a field - What if the content author later on sets a different value via the UI? The item now has two different field values of the same field name - which field should the server render?

How about add another option to copy the fields?

Technically CopyAllFields can cause data loss here, as fields not in the new template are ear-marked for deletion... unless the dev adds those fields to the FieldsToCopy (then the language/version bug definitely needs to be fixed).

michaellwest added a commit that referenced this issue Apr 11, 2022
This is consistent with the Change Template UI in the Content Editor.
@mikaelnet
Copy link

@mikaelnet do you have any insights you can share about this issue? Any recommendations?

Sorry I'm late to this discussion. Not sure if this is still relevant, but I'll share a few thoughts that comes to my mind. Historically, I've been struggling with this as well and I usually end up building my own solution-custom methods for this.

I've found it important to validate if a template change is possible in the first place. For example, if an item is a clone or is cloned, the result may be quite unexpected. In a clone scenario one would obviously have some kind of broken/undefined state while changing the source item and all its clones. In a SPE scenario, it could be worth testing if the item is cloned or is a clone and use a -Force flag when violating this. Potentially have a flag for changing all the items involved at once, but that might be a bit tricky as well. Documented example code could perhaps be a better option in combination with a -Force flag.

When it comes to migrating fields, I think it's essential that content is migrated on all languages/versions, since a template change applies to all. At the same time, it's important to only migrate fields containing actual values. Any fields with field.HasValue == false should be left blank, so that inheritance to default/standard values, language fallback, clone inheritance etc. is kept unchanged. Just taking the field value without checking HasValue can cause tons of unwanted side effects.

Another thing to consider is the MasterID field in the Items table (Created from in the UI). If one changes the template of an item, it may be worth updating/removing this reference, and maybe also the Originator field. When items are migrated to another template, the reason could be that the old template is deprecated and is about to be removed. I'm not sure exactly what Sitecore versions have issues with this and can't remember the exact scenarios, but I've experienced a few times that the Content Editor breaks when these links are broken - in particular when pointing to branches that no longer exists. One idea could be to have Cmdlet options to either clear those fields or set new values to them when changing template. On the other hand, it may bloat it too much. Maybe a separate Cmdlet for addressing such issues would make sense.

@michaellwest michaellwest added this to the 6.4 milestone Sep 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤩-release-highlight Exciting change that should be highlighted in the release notes and celebrated by SPE fans. area-commands Involves functions and cmdlets. impact-behaviour-change Nothing to be worried about. Now even better than before!
Projects
None yet
Development

No branches or pull requests

3 participants