Add support for member type container

[bjarnef]

Prerequisites

• I have added steps to test this contribution in the description below

Description

I noticed we are able to create folders (also called containers) under document types and media types, but not for member types. The codebase did contain part of the logic to support containers for member types.

I think it would be great to make this a bit more consistent between content types.
It working with larger solutions with multiple member types, it may also make a bit more structure.

The MemberTypeAndGroupTreeControllerBase is used for both member types tree and member groups tree, so for now it only add this to member types.

I have streamlined the dialogs a bit more for document, media and member types. I also noticed the "create" action for media type allowed to select folder/container as child to an existing media type, which it not supported and document types doesn't allow this. Folder/container are only allowed at root or under other folders/containers.

I have generated a guid, but we can change it to something else if needed.

Notice this also requires Umbraco Deploy to implement changes to support member type container entity.

chrome_djiuFkK2eL.mp4

[github-actions]

Hi there @bjarnef, thank you for this contribution! 👍

While we wait for one of the Core Collaborators team to have a look at your work, we wanted to let you know about that we have a checklist for some of the things we will consider during review:

• It's clear what problem this is solving, there's a connected issue or a description of what the changes do and how to test them
• The automated tests all pass (see "Checks" tab on this PR)
• The level of security for this contribution is the same or improved
• The level of performance for this contribution is the same or improved
• Avoids creating breaking changes; note that behavioral changes might also be perceived as breaking
• If this is a new feature, Umbraco HQ provided guidance on the implementation beforehand
• 💡 The contribution looks original and the contributor is presumably allowed to share it

Don't worry if you got something wrong. We like to think of a pull request as the start of a conversation, we're happy to provide guidance on improving your contribution.

If you realize that you might want to make some changes then you can do that by adding new commits to the branch you created for this work and pushing new commits. They should then automatically show up as updates to this pull request.

Thanks, from your friendly Umbraco GitHub bot 🤖 🙂

[bjarnef]

I have fixed the tree rendering in 821e54e although it probably should update the GetNodesFromService method instead.

Not sure we should have MemberTypeAndGroupTreeControllerBase in future as member groups are different from member types.

I would say a base controller for document types, media types and member types makes sense, e.g. ContentTypeTreeControllerBase.

In another PR it would be great to allow moving folders/containers. Move the node is basically just a change in parent id, which would be very useful when working with many blocks structured in folder, but I think it also need to trigger an event, so e.g. Umbraco Deploy and uSync can handle schema change, when the folders including children are moved.

In a future release a lot of the Content Type specific logic could probably be refactored, since much of the logic like, Move, Copy etc. it the same between the document, media and member type controllers and they could probably just inherit from a ContentTypeTreeControllerBase instead with shared logic.

[bjarnef]

I think this is a breaking changing this since the method is public.
Not sure what the best way to handle this? Maybe as an optional parameter although it isn't consistent with document type and media type controllers.. and make a breaking change regarding this sometime later in a major version?

public MemberTypeDisplay? GetEmpty()

changed to:

public MemberTypeDisplay? GetEmpty(int parentId)

Furhtermore the getScaffold function has changes to this, but if developers used this directly it would fallback to default value or int which is 0.
We can handle fallback here it parentId is undefined or null, then set parentId to -1.

getScaffold: function (parentId) {

    parentId = parentId ?? -1;
    
    return umbRequestHelper.resourcePromise(
       $http.get(
           umbRequestHelper.getApiUrl(
               "memberTypeApiBaseUrl",
               "GetEmpty", { parentId: parentId })),
       'Failed to retrieve content type scaffold');
},

[bjarnef]

@AndyButland I guess support of member type containers in Umbraco Deploy eventually would be much similar to document type and media type containers?

[AndyButland]

@AndyButland I guess support of member type containers in Umbraco Deploy eventually would be much similar to document type and media type containers?

Yes, should be easy enough to add but we would have to make some updates to support deployment of these containers. So if and when this is merged in and targeted for a release, please someone ping one of us working on Deploy to make sure we include it in an update of that too.

[bjarnef]

I will also ping @KevinJump about this as I guess he would like to support it in uSync package as well if/when merged.

[emmagarland]

Hi @bjarnef

I've just noticed this update to create containers under member types - thanks for this!

As it might be a breaking change, I have sent it over to HQ to have a discussion about.

Thanks again,

Emma

[bjarnef]

Hi @emmagarland

Regarding possible breaking changes I think the main concern is GetEmpty() method.
Maybe also MemberTypeContainerRepository now inheriting EntityContainerRepository, so entity container methods implemented from IMemberTypeContainerRepository has been removed as they are handled in EntityContainerRepository, but I don't think is breaks anything 🤞

and then MemberTypeAndGroupTreeControllerBase obsolete a mehtod and implement a new overload method.

I also added [Authorize(Policy = AuthorizationPolicies.TreeAccessMemberTypes)] to PostSave and GetEmpty() to be consistent with content and media type. I think it should be okay to assume the user has access to member types to perform these actions.

[nul800sebastiaan]

Hi @bjarnef, as we talked about a few weeks ago, I'll close this one while we focus on getting Bellissima in good shape.

I'll add the generic comment for people who are reading along with this:

As you may or may not know, we're working on a rewrite of the Umbraco backoffice, codenamed Bellissima (https://umbraco.com/blog/bellissima-preview-releases-of-the-new-backoffice/) and we're trying to make sure that it has feature parity with the current AngularJS-based backoffice.

We're at a point in the development phase where we want to start wrapping up a beta version to ship some time "soon" and we're going to have to implement a feature freeze on the current backoffice so that we don't keep increasing the scope of Bellissima. Unfortunately that means we're going to have to stop accepting PRs like this one that create significant changes to the current backoffice.

As a general rule from now on, we're still happy for features in C# code but anything that gets introduced or refactored in AngularJS probably won't make it. Bugfixes are still great of course!

Thanks for the efforts here and we hope to see you back as a contributor soon! 👍

[bjarnef]

@nul800sebastiaan do we want this in Umbraco 14? Ideally I think Document, Media and Member types should interit from a base ContentTypeController which is reposible for some of the same features like compositions and entity containers (folders).

[nul800sebastiaan]

I'm mostly questioning the number of member types that would make this make sense. I can see that some people in rare cases need containers for media types, but I haven't seen cases where it makes sense for member types.

Anyway, new feature requests go in discussions, so let's see what comes out of that - https://github.com/umbraco/Umbraco-CMS/discussions

[bjarnef]

Yeah, not sure how much people will use it, but as document, media and member types are most streamlined compared to previous version of Umbraco, they are almost identical and the current codebase have a lot of redundant code regarding these.

I see the content types (document, media and member) as much of the same logic and should probably in future inherit same controller.

I don't see a big need for entity containers (folders) for media types as well, but its an option :)

[markadrake]

The argument for this feature is more about consistency in the design and behavior patterns of the backoffice, not so much about whether people will use it or not.

Do people expect it to be there? My answer is yes.