Elements: Clean up the remaining TODOs

[kjac]

Description

A last-minute sweep of the remaining TODOs for Elements.

Breaking changes

There are a few breaking changes here, because a few bits of code were previously document-only - now they are shared between documents and elements.

[claude]

Claude finished @kjac's task in 3m 54s —— View job

---

PR Review

Target: origin/v18/dev · Based on commit: 69d3427c · Skipped: 0 files out of 32 total

Cleans up remaining Elements TODO items: renames DocumentVariantState → PublishableVariantState (moved to ViewModels.Content), renames DocumentVariantStateHelper → PublishableVariantStateHelper, removes IElementService.Create() (test-only factory), and replaces IElementCacheService with IPublishedElementCache in ElementPickerValueConverter. All internal callers updated; tests updated throughout.

• Modified public API: DocumentVariantState enum (renamed + moved namespace); IElementService.Create() (removed); ElementPickerValueConverter (access changed public → internal sealed); PublishableVariantItemResponseModelBase.State and PublishableVariantResponseModelBase.State property type changed from DocumentVariantState to PublishableVariantState
• Breaking changes: See below — three public API removals/renames without the [Obsolete] migration pattern documented in CLAUDE.md

---

Important

• src/Umbraco.Cms.Api.Management/ViewModels/Document/DocumentVariantState.cs (removed): DocumentVariantState (public enum) is replaced by PublishableVariantState in a different namespace (ViewModels.Content) with no backward-compat alias. Any v18-beta consumer referencing Umbraco.Cms.Api.Management.ViewModels.Document.DocumentVariantState will get a compile error. Per CLAUDE.md, the documented mitigation is to leave a [Obsolete] type alias in the old location (e.g. [Obsolete("Use PublishableVariantState. Scheduled for removal in Umbraco 20.")] public enum DocumentVariantState { ... } delegating to the new enum, or a using alias). Since v18 is pre-release beta, the practical risk to stable consumers is low, but early adopters on v18 beta are affected.

• src/Umbraco.Core/Services/IElementService.cs:8 (removed method): IElementService.Create(string name, string contentTypeAlias, int userId) is removed outright from the public interface with no [Obsolete] default implementation. Any caller of IElementService.Create() will break. CLAUDE.md Pattern 2 requires keeping the old method as [Obsolete] with a default implementation that delegates to the new approach (direct Element construction). Example:

  [Obsolete("Construct Element directly. Scheduled for removal in Umbraco 20.")]
  IElement Create(string name, string contentTypeAlias, int userId = Constants.Security.SuperUserId)
      => new Element(name, /* requires IContentType lookup */);

  (The default impl can use StaticServiceProvider to resolve IContentTypeService if needed.)

• src/Umbraco.Core/PropertyEditors/ValueConverters/ElementPickerValueConverter.cs:11 (access reduced): ElementPickerValueConverter is changed from public class to internal sealed class. This removes it from the assembly's public API surface. Any external code that instantiated, subclassed, or registered this type explicitly will break. This follows the same pattern — the CLAUDE.md approach would be to mark the class [Obsolete] and make it sealed first, before making it internal in the next major version. Given it's a value converter unlikely to be subclassed, the practical risk is very low.

Suggestions

• src/Umbraco.Cms.Api.Management/Mapping/Document/DocumentMapDefinition.cs:62 (minor naming): Lambda parameter documentVariantViewModel in ElementMapDefinition and the shared PublishableContentPresentationFactoryBase could be variantViewModel or publishableVariantViewModel now that the type is shared — currently the "document" prefix only makes sense within DocumentMapDefinition.cs itself and is fine there, but ElementMapDefinition.cs:44 uses the same documentVariantViewModel name for what is now an element variant. Not a bug, just a minor inconsistency.

• src/Umbraco.Core/Services/PublishableContentServiceBase.cs:22: The new XML doc comment <see cref="ElementService"/> should use the fully qualified type (<see cref="global::Umbraco.Cms.Core.Services.ElementService"/> or just the simple form if unambiguous) to ensure it resolves correctly. The current <see cref="ContentService"/> and <see cref="ElementService"/> references should resolve, but it's worth confirming they don't produce CS1574 warnings.

---

Approved with Suggestions for improvement

Good to go. The refactoring is clean — all internal callers updated, tests updated, no remaining usages of removed symbols. The breaking changes are intentional and acknowledged. Since this targets the pre-release v18 dev branch (18.0.0-beta1) where the API surface is not yet stable, the CLAUDE.md breaking-change patterns are worth noting for awareness but are not blockers here. Before v18 goes stable, consider adding at minimum a [Obsolete] type alias for DocumentVariantState to ease migration for early beta adopters.