Skip to content

Conversation

@ronaldbarendse
Copy link
Contributor

PR #16914 got merged and targeted for v17, but only contains a partial implementation of PR #14833 that targeted v13 (but was closed).

This fixes the error when creating a new member type folder using the Management API (thrown from EntityContainer), updates the no-op IMemberTypeContainerRepository implementation to inherit from the shared base container repository (so something actually gets persisted) and updates/adds the required member type container API endpoints.

All the /umbraco/management/api/v1/member-type/folder endpoints now work and the /umbraco/management/api/v1/tree/member-type one returns the folders as well. However, the backoffice UI still needs to be updated to support the folders (hence I'm currently creating this as a draft PR):

image

@AndyButland
Copy link
Contributor

AndyButland commented Nov 2, 2025

I've extended this now to add the client-side support, and also aligned further with media types such that we have features to move, duplicate, export and import. Seemed like it might be a bit dangerous to have a management API that supports features that the UI doesn't, in case some uses this - e.g. via the MCP Server - to create things they can't then use. And this way we can hopefully roll forward rather than roll back.

I've moved the target branch to main though as think this will need to come in 17.1 to give time for review and testing, and not add something late to the 17 release candidate.

@AndyButland AndyButland changed the base branch from release/17.0 to main November 2, 2025 18:27
# Conflicts:
#	src/Umbraco.Web.UI.Client/package-lock.json
#	src/Umbraco.Web.UI.Client/package.json
#	version.json
@AndyButland AndyButland marked this pull request as ready for review November 2, 2025 19:54
Copilot AI review requested due to automatic review settings November 2, 2025 19:54
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds folder support to the member type tree in Umbraco CMS, bringing it in line with functionality already available for document types and media types. The changes enable hierarchical organization of member types through folders.

Key changes include:

  • Added folder entity type (UMB_MEMBER_TYPE_FOLDER_ENTITY_TYPE) and container support for member types
  • Implemented folder creation, update, delete, move, duplicate, import, and export operations
  • Added tree endpoints for ancestors, children, and siblings with folder filtering
  • Updated the member type tree to support both folders and member types as tree items

Reviewed Changes

Copilot reviewed 106 out of 106 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
src/Umbraco.Web.UI.Client/src/packages/members/member-type/entity.ts Added new folder entity type constant
src/Umbraco.Web.UI.Client/src/packages/members/member-type/tree/types.ts Updated tree item model to support folder entity types
src/Umbraco.Web.UI.Client/src/packages/members/member-type/tree/server-data-source/* Created new server data source implementation with full tree support
src/Umbraco.Web.UI.Client/src/packages/members/member-type/tree/folder/* Added complete folder workspace, repository, and manifest implementation
src/Umbraco.Web.UI.Client/src/packages/members/member-type/entity-actions/* Added move, duplicate, import/export actions for member types
src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MemberTypeContainerRepository.cs Replaced no-op implementation with actual entity container repository
src/Umbraco.Core/Services/ContentTypeEditing/MemberTypeEditingService.cs Updated to support container (parent folder) during creation
src/Umbraco.Cms.Api.Management/Controllers/MemberType/* Added API controllers for move, import, export, and tree operations

@AndyButland AndyButland changed the title Implement member type containers Member types: Implement containers Nov 3, 2025
@ronaldbarendse
Copy link
Contributor Author

I've moved the target branch to main though as think this will need to come in 17.1 to give time for review and testing, and not add something late to the 17 release candidate.

Totally understandable, although I'd then suggest merging ac20b43 and b9a5fd5, so at least the service works and Deploy can add support for it by targeting CMS 17.0.0. Maybe some base classes of the controllers can already be updated as well, so those don't end up as breaking changes. As long as the tree API endpoints don't return the containers/folders yet, the UI will simply not show them yet and nothing in the CMS will seem 'broken'.

@AndyButland
Copy link
Contributor

Added in #20715 @ronaldbarendse. I found I couldn't make the controller base class changes and be compatible with the current front-end.

@ronaldbarendse
Copy link
Contributor Author

Thanks Andy! Including those changes in the next 17 RC should allow us to implement deploying member type containers/folders in Deploy and introduce the remaining UI work in a future minor 💪🏻

I had a quick look at changes to the existing base classes and the following changes shouldn't be breaking:

  • MemberTypeTreeControllerBase: changing NamedEntityTreeControllerBase<MemberTypeTreeItemResponseModel> to FolderTreeControllerBase<MemberTypeTreeItemResponseModel> while also implementing the newly introduced abstract FolderObjectType property isn't breaking, as it only introduces a new type into the hierarchy (see 'Introducing a new base class')..
  • CreateMemberTypeRequestModel: changing CreateContentTypeRequestModelBase<CreateMemberTypePropertyTypeRequestModel, CreateMemberTypePropertyTypeContainerRequestModel> to CreateContentTypeWithParentRequestModelBase<CreateMemberTypePropertyTypeRequestModel, CreateMemberTypePropertyTypeContainerRequestModel> also only introduces a new type into the hierarchy.
  • MemberTypeTreeItemResponseModel: changing NamedEntityTreeItemResponseModel to FolderTreeItemResponseModel does the same.

The remaining changes should be made backwards compatible using default implementations or adding additional overloads. We might want to already add the optional foldersOnly request parameter to the existing API controllers though, as adding new parameters still is a breaking change, regardless of adding a default value:

  • RootMemberTypeTreeController.Root(..., bool foldersOnly = false)
  • SiblingMemberTypeTreeController.Siblings(..., bool foldersOnly = false)

@AndyButland
Copy link
Contributor

Thanks @ronaldbarendse - I've added those last two to #20715

Copy link
Contributor Author

@ronaldbarendse ronaldbarendse left a comment

Choose a reason for hiding this comment

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

I've also reviewed the code and couldn't find any obvious issues, but did experience the following when testing in the backoffice:

  • Moving an item from a folder back to the root works, but besides the green 'Moved' notification, also shows a red 'Menu loading failed. Showing the first items again.' error and doesn't update the tree.
  • Performing a manual 'Reload children' on the tree root will show '...' at the bottom, which only shows the actual items after clicking/expanding it.
  • Something similar happens when creating a new folder (but at the top, since folders are shown first).

Other than this, all actions (create, rename, move, delete, duplicate, export and import) do work as expected.

@AndyButland
Copy link
Contributor

Thanks for the testing @ronaldbarendse. I saw issues similar to these, but I think they are outside of this PR:

@madsrasmussen
Copy link
Contributor

The frontend changes look good. We need to be aware that there is a fair bit of code duplication, especially around the import/export of content types. It would be good to make that reusable at some point. But let's get this merged first, as there's already more than enough happening. 😃

@nikolajlauridsen, feel free to merge after you've had a look.

# Conflicts:
#	src/Umbraco.Cms.Api.Management/Controllers/MemberType/Tree/RootMemberTypeTreeController.cs
#	src/Umbraco.Cms.Api.Management/Controllers/MemberType/Tree/SiblingMemberTypeTreeController.cs
Copy link
Contributor

@nikolajlauridsen nikolajlauridsen left a comment

Choose a reason for hiding this comment

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

Backend code looks good 👍 But I have a concern regarding an API breaking change

Copy link
Contributor

@nikolajlauridsen nikolajlauridsen left a comment

Choose a reason for hiding this comment

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

Backend changes look good; however, in testing, I found some weird UI behaviour when moving members around.

  • When moving out of a folder, I got an error and had to reload the menu. When I did it didn't load the member, and I had to click the ellipsis to load it
  • When moving into a folder,r the UI doesn't update, I have to "close" and "reopen" the folder to see the member
  • I'm allowed by the UI to move a member to the folder it's already in, but you're now allowed to and get an error.

I don't know if this is specific to this PR or a more general issue, but from a backend perspective, this PR is good to go 👍

bug.mp4

@AndyButland
Copy link
Contributor

AndyButland commented Nov 18, 2025

Investigating the issues raised:

I'm allowed by the UI to move a member to the folder it's already in, but you're now allowed to and get an error.

This issue exists for document types but not for data types. With data types the response is that the item is "Moved" even though of course it hasn't. So to resolve this I've aligned the server-side behaviour (so if you move to the folder it's already in, it just early returns with "success").

@madsrasmussen - for the other two, there are related issues still open on the tracker:

#20515
#20676

However, I actually don't see the problems @nikolajlauridsen has noted for document type containers, so seems there must be something missing for the member types. I can't spot it though, either through debugging or through just comparing files. Are you able to see where this is coming from and/or what I've missed for the front-end implementation for moving member types please?

@nikolajlauridsen
Copy link
Contributor

I'm allowed by the UI to move a member to the folder it's already in, but you're now allowed to and get an error.

This issue exists for document types but not for data types. With data types the response is that the item is "Moved" even though of course it hasn't. So to resolve this I've aligned the server-side behaviour (so if you move to the folder it's already in, it just early returns with "success".

Looks good to me, makes sense that it always succeeds since it's basically a no-op

@AndyButland
Copy link
Contributor

From more testing it seems a little inconsistent. Sometimes this error Menu loading failed. Showing the first items again pops up after a move to the root of the tree of member types, but not always. I'm minded to merge this in given you have both reviewed we have related issues on the tracker already, but I'll leave for a day or two in case you spot anything.

@nikolajlauridsen
Copy link
Contributor

I was thinking the same thing, if we already have issues for the other two points I mentioned, I think we should merge this since they're separate issues, I'm happy with how it is now 😄🚀

@AndyButland AndyButland merged commit 6a7360a into main Nov 20, 2025
27 of 28 checks passed
@AndyButland AndyButland deleted the v17/feature/member-type-containers branch November 20, 2025 10:28
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.

5 participants