Skip to content

Fix: move property out of group on import (#1009) - #1010

Merged
KevinJump merged 2 commits into
v17/mainfrom
v17/fix-1009-move-property-out-of-group
Jul 27, 2026
Merged

Fix: move property out of group on import (#1009)#1010
KevinJump merged 2 commits into
v17/mainfrom
v17/fix-1009-move-property-out-of-group

Conversation

@KevinJump

Copy link
Copy Markdown
Owner

Fixes #1009

What changed

DeserializePropertiesAsync in ContentTypeBaseSerializer now handles a property being moved out of all groups during import.

Why

When a content type's .config is edited so a property has an empty <Tab></Tab> and is removed from <Tabs>, importing did nothing — the property stayed in its original group. Bulk imports appeared to no-op; single-item imports appeared to "revert".

The deserialize loop only handled properties moving into a tab. For an existing property whose tab alias resolved to an empty string, the else branch fell through without taking any action, so the property was never removed from its old group.

How

  • propertiesToMove is now Dictionary<string, string?>, where a null target means "no group".
  • Added an else branch: when an existing property has no tab in the config but is still assigned to a group, it is queued to move to no-group.
  • MoveProperties passes the value to item.MovePropertyType(alias, null).

MovePropertyType(alias, null) was verified against Umbraco.Core 17.5.3 — a null group alias removes the property from its current group and sets PropertyGroupId = null, landing it in NoGroupPropertyTypes, which is the intended result. The change is reported with (No group) as the destination.

Notes for reviewer

  • Builds clean (uSync.Core), no new warnings. The IContentTypeBase interface declares the group-alias param non-nullable while the implementation accepts null, so the call uses the null-forgiving operator.
  • The issue also mentions a report-UI "revert" symptom on single-item import. That is a separate backoffice report/import flow concern, not part of this deserialize path; this change corrects the underlying import behaviour.
  • No automated test added: uSync.Tests has no content-type serializer harness (that requires a full Umbraco service context).

🤖 Generated with Claude Code

KevinJump and others added 2 commits July 27, 2026 10:38
Importing a content type where a property has been moved out of all
groups (empty <Tab> and no matching <Tabs> entry) did nothing - the
property stayed in its original group.

DeserializePropertiesAsync only handled properties moving *into* a tab.
When the tab alias resolved to empty for an existing property, the else
branch fell through without action.

Now, when an existing property has no tab in the config but is still in a
group, it is queued to move to "no group" via MovePropertyType(alias,
null), which Umbraco treats as removing it from its current group.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…1009)

Moving a property out of all groups still needed two imports. Root cause:
Umbraco's MovePropertyType(alias, null) removes the property from its
group but does NOT add it to the 'no group' collection - it orphans the
property, so the first import's save doesn't persist the move.

MoveProperties now re-adds the property with AddPropertyType after the
move to null, which lands it in NoGroupPropertyTypes. Added unit tests
documenting the orphaning behaviour and verifying the workaround.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@KevinJump

Copy link
Copy Markdown
Owner Author

Follow-up: the first commit made the property move, but it still needed two imports to stick. Traced the root cause:

Umbraco's MovePropertyType(alias, null) removes the property from its group but does not add it to the "no group" collection — it orphans the property. Confirmed with an in-memory test: after the call the property is gone from both the group and PropertyTypes/NoGroupPropertyTypes entirely. So the first import's save had nothing to persist, and the change only appeared to work on a later pass.

Fix in MoveProperties: after MovePropertyType(alias, null), re-home the property with AddPropertyType(property), which lands it in NoGroupPropertyTypes. Now it persists in a single import.

Added MovePropertyTypeTests — one test documents the orphaning behaviour, the other verifies the workaround.

@KevinJump
KevinJump merged commit 35ee35a into v17/main Jul 27, 2026
4 checks passed
@KevinJump
KevinJump deleted the v17/fix-1009-move-property-out-of-group branch July 27, 2026 10:22
KevinJump added a commit that referenced this pull request Aug 17, 2026
* Fix: move property out of group on import (#1009)

Importing a content type where a property has been moved out of all
groups (empty <Tab> and no matching <Tabs> entry) did nothing - the
property stayed in its original group.

DeserializePropertiesAsync only handled properties moving *into* a tab.
When the tab alias resolved to empty for an existing property, the else
branch fell through without action.

Now, when an existing property has no tab in the config but is still in a
group, it is queued to move to "no group" via MovePropertyType(alias,
null), which Umbraco treats as removing it from its current group.



* Fix: re-home property into no-group so move persists in one import (#1009)

Moving a property out of all groups still needed two imports. Root cause:
Umbraco's MovePropertyType(alias, null) removes the property from its
group but does NOT add it to the 'no group' collection - it orphans the
property, so the first import's save doesn't persist the move.

MoveProperties now re-adds the property with AddPropertyType after the
move to null, which lands it in NoGroupPropertyTypes. Added unit tests
documenting the orphaning behaviour and verifying the workaround.



---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Removing a Group fails

1 participant