Fixed children content types not beeing updated

[jensotto]

Q	A
Bug fix?	yes
New feature?	no
New sample?	no
Related issues?	#2270

What's in this Pull Request?

Some properties for a content type supports pushing down to children. This was disabled by PR #2270. This PR changes it back so that children are updated if any of the properties that supports pushing down to children are changed and if any of the field references are set to update children content types.

Future versions of the provisioning schema should probably implement a content type level attribute for specifying if children are to be updated instead of relying on this to be specified on the field refs.

[jensotto]

@jansenbe why does this need schema changes? There should be no need for that.

[jansenbe]

given UpdateChildren in FieldRef defaults to true your code change now will trigger update of child content types whereas we today do not do that. This will possibly impact existing users without them understanding why. If we have a new property at content type level that defaults to false we can safely introduce this. Would recommend adding an issue in https://github.com/SharePoint/PnP-Provisioning-Schema/issues to ask for this change and refer to this PR. This PR can stay open until we've released a new schema

[jensotto]

@jansenbe The same can be said when implementation of #2270 was done. Updating of children was done before and suddenly stopped without any notification. I don't agree that this should wait for a schema change. It will be possible to opt out of updating children now. This was not possible before, and #2270 blocked it entirely.
So basically implementation in this PR is no different than what was the case prior to #2270. #2270 implemented this bug and should therefore not have been merged.

I've already contributed a PR in https://github.com/SharePoint/PnP-Provisioning-Schema to address this.

[jensotto]

@jansenbe I really don't understand the hesitation regarding this, and I don't understand why you allow verifiable broken functionality to have precedence over a unjustified fear of introducing mass content type child updates.
Child updates will not happen with this PR unless the provisioning engine changes at least one of the properties that support push down. This will never happen unless someone actually wants to change a content type property that supports push down. If this PR is not in place it will be impossible to use the provisioning engine to update any of the properties that supports push down on content types in lists or inherited content types.

[jansenbe]

When we read a template the fieldref property UpdateChildren is always set to true, whereas we currently block updating child content types. So your PR does introduce a change in behavior. I agree with you that we do need to support updating child content types, but I don't think we should change the default behavior. This is a change I want to first discuss with others, so stay tuned for an update.

[jensotto]

I understand that you want to discuss this PR with others @jansenbe. But to be fair content type updates was always pushed down to children before #2270, so I disagree that it's a change in behavior. If you are so concerned about changes in behavior you should never have approved #2270. That PR silently broke updating child content types.

[jensotto]

@jansenbe I would like to emphasis that this PR in fact does not increase the likelihood of updating child content types. It is also not true that the code currently block updating child content types.
It's only content type properties that are blocked for updating child content types. And this is a bug that was introduced in #2270.
It is true that the default value of UpdateChildren is true for any field reference, but this does not have any impact on when children are updated by this PR. If UpdateChildren is true an update of children will happen regardless of this PR when fields are modified/added/removed. The only difference with this PR is that it will enable content type properties that support push down to also be updated. This will only happen if there is a change in one of those properties.
So this PR should not have the impact that you predict. It will remove the bug introduced in #2270, but only if there are at least one field reference which has the UpdateChildren set to true. This is where the future changes of the provisioning schema is needed.

[vlad-ivanov-d]

@jensotto thanks for PR. I've spent hours until I found this PR in Google. I couldn't understand why a field from site content type is not adding to the list content type level. Your are absolutely right, it was broken by #2270 PR.
@jansenbe the default behavior was to update children. But that was broken by #2270 PR. I have to use the last working version 3.14.1910.1. It works there as expected.

[jensotto]

@jansenbe, @PaoloPia This issue needs to be fixed before abandoning this project.