Skip to content

Code Tidy: Remove obsolete MoveEventInfo.NewParent#22728

Merged
AndyButland merged 5 commits into
v18/devfrom
v18/task/moveEventInfo.NewParentId-obsoletion
May 6, 2026
Merged

Code Tidy: Remove obsolete MoveEventInfo.NewParent#22728
AndyButland merged 5 commits into
v18/devfrom
v18/task/moveEventInfo.NewParentId-obsoletion

Conversation

@Migaroez

@Migaroez Migaroez commented May 6, 2026

Copy link
Copy Markdown
Contributor

Description

  • Removed the obsoleted property
  • Updated Equals and GetHashCode to only use the NewParentKey
    The GetHashCode is not used directly anywhere so the change should not cause any issues with already persisted data.
  • Added constructor that only uses the NewParentKey
  • Obsoleted irrelevant constructors
  • Updated code that now uses the obsoleted constructors

Migaroez added 2 commits May 6, 2026 11:48
Updated methods that were still using it
Obsoleted constructors that were still setting the value.
@claude

claude Bot commented May 6, 2026

Copy link
Copy Markdown

Claude finished @Migaroez's task in 5m 2s —— View job


PR Review

Target: origin/v18/dev · Based on commit: 3e600335

Removes MoveEventInfo<TEntity>.NewParentId (correctly scheduled for removal in v18), obsoletes the int-based constructors, adds a key-only constructor, and updates several call sites throughout Core and Infrastructure to use the new constructor.

  • Modified public API: MoveEventInfo<TEntity> — removed NewParentId property; two constructors newly obsoleted; one new constructor added ((TEntity, string, Guid?))
  • Breaking changes: Removal of NewParentId is correct (was [Obsolete("... Scheduled for removal in Umbraco 18.")]); the two newly-obsoleted constructors mitigate any new breaks correctly.
  • Other changes: Consumers that passed NewParentId to Equals/GetHashCode will now get different equality/hash results (only NewParentKey is compared). External consumers who subclassed or overrode these methods are unaffected since GetHashCode is not virtual and both methods now reflect the new canonical identity.

Important

  • src/Umbraco.Core/Events/MoveEventInfo.cs:16 and :29: Both [Obsolete] messages say "Scheduled for removal in v19" — format and version are both wrong. Per CLAUDE.md policy, the target is current_major + 2 = 18 + 2 = 20, and the required format is "Scheduled for removal in Umbraco 20" (not v19).

    // Should be:
    [Obsolete("Use the overload without the newParentId parameter instead. Scheduled for removal in Umbraco 20.")]
  • src/Umbraco.Core/Services/ContentService.cs:1229: The PR correctly updated line 1195, but line 1229 still calls the now-obsoleted (int newParentId, Guid? newParentKey) constructor. itemParentKey is already resolved on the line above — this is a straightforward missed update:

    // Current (calls obsoleted constructor):
    return new MoveEventInfo<IContent>(x.Item1, x.Item2, x.Item1.ParentId, itemParentKey);
    
    // Fix (uses new constructor):
    return new MoveEventInfo<IContent>(x.Item1, x.Item2, itemParentKey);
  • src/Umbraco.Core/Services/ContentTypeServiceBase{TRepository,TItem}.cs:1139: Calls the now-obsoleted (int) constructor with containerId.Value. The Guid? value is already available as containerKey (the method parameter at line 1115):

    // Current (calls obsoleted constructor):
    var moveEventInfo = new MoveEventInfo<TItem>(toMove, toMove.Path, containerId.Value);
    
    // Fix:
    var moveEventInfo = new MoveEventInfo<TItem>(toMove, toMove.Path, containerKey);

Suggestions

  • src/Umbraco.Core/Events/MoveEventInfo.cs:30–32: The (int newParentId) obsolete constructor calls this(entity, originalPath, newParentId, null), which is also an obsolete constructor. Per the documented pattern, an obsolete member should delegate to the new member, not to another obsolete one. Prefer:

    [Obsolete("Use the overload with the newParentKey parameter instead. Scheduled for removal in Umbraco 20.")]
    public MoveEventInfo(TEntity entity, string originalPath, int newParentId)
        : this(entity, originalPath, newParentKey: null)
    {
    }

    This avoids a CS0618 chain and keeps the forwarding to the canonical new constructor.

  • Pre-existing missed callers (have FIXME comments — acceptable to leave for follow-up, but worth tracking): ContentTypeRepositoryBase.cs:154, DataTypeRepository.cs:170, MediaService.cs:1114, and MediaService.cs:1132 all call obsoleted constructors. They require a parent-key lookup per descendant to fix fully, so a follow-up PR is reasonable — but they should be tracked.


Request Changes

The two Important issues above should be addressed: the obsolete messages carry the wrong removal version/format, and two internal callers that could be updated in this PR were missed. The constructor-chain suggestion is minor but aligns with the repo's documented pattern.

@claude claude Bot added the area/backend label May 6, 2026
Migaroez added 2 commits May 6, 2026 13:19
Removed obsolete (parentId) cases and updated constructors

@AndyButland AndyButland left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spotted a few additional usages of the obsolete constructors that we may be able to clean-up, and a minor DRY up of the constructor code.

Comment thread src/Umbraco.Core/Events/MoveEventInfo.cs
Comment thread src/Umbraco.Core/Events/MoveEventInfo.cs Outdated
@AndyButland AndyButland changed the title Obsoletion: Remove MoveEventInfo.NewParent Code Tidy: Remove obsolete MoveEventInfo.NewParent May 6, 2026
@AndyButland AndyButland enabled auto-merge (squash) May 6, 2026 12:48
@AndyButland AndyButland merged commit b2ba4ab into v18/dev May 6, 2026
32 checks passed
@AndyButland AndyButland deleted the v18/task/moveEventInfo.NewParentId-obsoletion branch May 6, 2026 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants