Conversation
AndyButland
left a comment
There was a problem hiding this comment.
Code looks good and aligned with what you did on 17 in the linked PR @kjac, but I've found an issue I believe with removing property types. These are coming out as RefreshOther, which isn't correct.
If I debug line 229 in ContentTypeServiceBase:
var hasAnyPropertyBeenRemoved = dirty.WasPropertyDirty("HasPropertyTypeBeenRemoved");hasAnyPropertyBeenRemoved is false if I remove a property from the document type.
src/Umbraco.Infrastructure/Search/IndexingNotificationHandler.ContentType.cs
Outdated
Show resolved
Hide resolved
|
My bad, @AndyButland 🙈 I forgot to push the backport of #21856, which was a prerequisite for #21910 in V17. It's pushed now. |
…ng the expected change type.
AndyButland
left a comment
There was a problem hiding this comment.
All looks good now, though I was still finding the the removal of a property wasn't leading to the expected RefreshMain change type. I think I found the fix though, and demonstrated it with a failing and now passing integration test - so have pushed that up. Please take a look to make sure it makes sense to you before merging.
Otherwise I've tested the various scenarios of property add/removal, document type name, alias changes, composition and variance changes - and all yield the expected.
Prerequisites
Description
This is a partial backport of #21910 to V13. Partial because the cache redo for V15 is logically not applicable here 😄
The PR still classifies changes as "structural" or "non-structural". See #21910 for details.
Test results
I've been testing this on a local database with ~10000 pages of the same content type.