Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding Condition for Content Type Alias #1550

Conversation

enkelmedia
Copy link
Contributor

@enkelmedia enkelmedia commented Apr 5, 2024

This PR implements a proposal for a solution to this issue: umbraco/Umbraco-CMS#16683, making it possible to add a condition for a entities content type so that we can conditionally show a workspaceView based on the current document/media/members document/media/member type.

This will for example allow to:

  • Show a given workspaceView only when a document of type "article" is being edited.
  • Only show a given workspaceView when a media item of type "image" is being edited.

Considerations

I did some research and found that the all the Workspace Contexts we need to work with here only handles the unique property of the entities Content Type (aka the GUID key). The contexts never access the or work with the alias, but they all expose the unique identifier via the contentTypeUnique observable.

Initially I wanted this feature to work with the alias (like document type alias, media type alias etc.) but this would require us to either add an additional resource-call to get the content type alias or that the Management API exposes the alias in the "details" endpoints (e.g document.documentType.alias). I'm not sure if any off these would be viable changes so I figured I'll start out with implementing this using the already existing contentTypeUnique property to showcase the idea.

After thinking about it, at the end of the day anyone that need this feature could just fetch the unique(key) based on the document/media/member type alias if needed. I can also imagine that many times when a configuration UI/picker is show for users, a package or feature could easily store the content type key (unique) and not just the alias. It might even be good since the alias is something that can be changed while keys stay the same.

A potential downside with alias is also that they are not necessary unique between members types, document types and media types - not unlikely that a document type called Image or Video could exist along side the same media type aliases. If we stick to unique(key) we would still be able to target generically by fetching the key based on the alias, but also more narrow since the key is unique.

Implementation

I basically added a new interface for Workspace Contexts, to implement UmbEntityWithContentTypeWorkspaceContext that has the already existing contentTypeUnique property as a contract. I updated the Document, Media and Member Workspace Contexts to implement this interface, the contentTypeUnique property was already there.

The main attraction is the new type UmbWorkspaceEntityContentTypeCondition that can be used to conditionally load extensions based on the entities content type.

For example:

const workspace: ManifestWorkspaceView = {
  type: 'workspaceView',
  alias: 'Example.WorkspaceView.EntityContentTypeCondition',
  name: "Example Workspace View With Entity Content Type Condition",
  element : () => import('./workspace-view.element.js'),
  meta: {
    icon : 'icon-bus',
    label : 'Conditional',
    pathname : 'conditional'
  },
  conditions : [
    {
      alias : 'Umb.Condition.EntityContentType',
      oneOf : ['29643452-cff9-47f2-98cd-7de4b6807681', 'e153ec65-71fa-4da9-bb90-f6555ea95362']
    }
  ]
};

Questions / Discussion

Feedback on naming is more than welcome. I was thinking that "entity content type" distinguishes this from just a "content type" since that is it's own thing. It also follows the UmbEntityWorkspaceContext-naming style for the context.

Like stated above, my initial thought was to use alias but I did reconsider this due to the fact that very little in the code base deal with the alias, I guess the we need to decide if we're fine with using the content type unique or if we should refactor.

If we should refactor we need to decided on with approach:

  1. Client side solution - allow the condition to take in alias and then fetch the content type unique to compare against the entity.
  2. Server side - Would require us to add document.documentType.alias, media.mediaType.alias and member.memberType.alias in the Management API and use these for the condition check.

Types of changes

  • Added new interface for WorkspaceContexts to optionally implement (implemented in document, media and member). Exposes the "contentTypeUnique" property.
  • Added condition for Workspace Entity Content Type that matches against the contentTypeUnique value.
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Chore (minor updates related to the tooling or maintenance of the repository, does not impact compiled assets)

Motivation and context

This feature makes the extension API for workspaceViews more flexible so that extensions can show up only in the context where they make sense. E.g something that works with a "Image" would not make sense on a "Video" media type.

How to test?

There is sample in the /examples/entity-content-type-condition/ folder.

Screenshots

umb-pr-content-type-unique

Checklist

  • If my change requires a change to the documentation, I have updated the documentation in this pull request.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.

@nielslyngsoe
Copy link
Member

nielslyngsoe commented Apr 19, 2024

Hi @enkelmedia

Very interesting, I did not read all the material(sorry). But I just wanted to mention we could make a Condition that would be Content Type Alias based.

Similar to your approach, we would make a new Token type and Interface for the Workspace Context. ( Like UmbContentTypeAliasWorkspaceContent)

But the condition would have to be specific towards a Workspace, so no one assumes they can use it on EntityActions, cause for EntityActions we do not know about the ContentType behind. So it would have to be named something like Umb.Condition.WorkspaceContentTypeAlias

In the Content(Document, Media, Member and Document-Blueprint) Workspaces, we have the Structure Manager, available on the property structure which holds the 'owner' Content Type. Which has both its Id and Alias.

But I would like to ask, did you consider compositions as a factor in this. Cause with this in mind, you may want the condition to be true if a Content is inheriting from the defined ContentType Alias. With this in mind I would say the Workspace Context method should be like this: contentTypeAliases or contentTypeUniques. Returning an Array of Aliases or Uniques.

The data about all the ContentTypes that makes up the specific ContentType can also be obtained via the Structure Manager :-)

Btw. I do also think the Content Type Alias would give a nicer configuration :-)

@enkelmedia
Copy link
Contributor Author

Hi!

Thanks for pointing out the ability to use "Content Type Alias", I did not notice the Structure Manager at the time the PR was created so I need to look into that again.

This is targeting a very specific use case, that is show/hide a workspaceView based on the current content type, I did not consider things like EntityActions so I will have to look closer at that. At the same time this got me thinging, maybe it would be useful to be able to put conditions on the entity actions as well - I would only like to show this action for content type A och B - the concepts are quite similar.

I did not consider the fact that a content type can consist of multiple compositions so that might need to be addressed as well.

I get your point about the fact that Content Type Alias would give a nicer configuration and this was my initial though as well, however I did reconsider that because a content type alias is not always unique between different entity types, there could be a document type with alias "Image" and a media type with the same alias but the key is always uniuqe - I guess both approaches would work but just need to keep this fact in mind if we choose to use the alias.

Do you think that it would be viable to add a method for contentTypeAliases or contentTypeUniques that would return the needed info for the content types?

I think I need to know the following to update the PR:

  1. Content Type Alias vs. Content Type Unique - Which should we go with?
  2. Can we add the contentTypeAliases or contentTypeUniques to Workspace Context? I guess that we can use the approach that I've started with the UmbEntityWithContentTypeWorkspaceContext that would indicate that this workspace context supports content types (aliases or uniques)

@enkelmedia
Copy link
Contributor Author

Hey, @nielslyngsoe, do you have any more feedback here? I did talk with @iOvergaard over at Discord. Would be nice to get this fixed before the branch gets to far behind.

@nielslyngsoe
Copy link
Member

Hi @enkelmedia

Sorry for my rather short reply here.

Lets stick to ContentTypeAliases for now, GUIDs seem too fragile and hard to 'read' for a Core feature. Where an Alias is very easy to discover when debugging a mistake.

To help you on the way I made the methods to expose aliases for you, so part of a UMB_PROPERTY_STRUCTURE_WORKSPACE you can now get workspace.structure._____

Branch name is: origin/feature/structure-manager-expose-content-type-aliases

I hope thats all needed for now :-) otherwise let me know :-)

Btw I would not aim for this to be part of 14.0, so lets try to get it in 14.1 :-)

@enkelmedia enkelmedia force-pushed the 1549-workspace-entity-content-type-condition branch from 8cdf800 to c8239d2 Compare June 4, 2024 21:04
@enkelmedia
Copy link
Contributor Author

@nielslyngsoe PR has now been updated.

Changes made:

  • Renamed to UmbWorkspaceContentTypeAliasCondition (Umb.Condition.WorkspaceContentTypeAlias)
  • Updated to use content type alias using the structure manager
  • Remove unneeded custom interface and token.

There are some examples included in the examples-folder, basically it can be used like this to only show a workspace view on content with type alias blogPost or mediaType1. Also support match : 'myAlias'.

const workspace: ManifestWorkspaceView = {
  type: 'workspaceView',
  alias: 'Example.WorkspaceView.EntityContentTypeCondition',
  name: "Example Workspace View With Entity Content Type Condition",
  element : () => import('./workspace-view.element.js'),
  meta: {
    icon : 'icon-bus',
    label : 'Conditional',
    pathname : 'conditional'
  },
  conditions : [
    {
      alias : 'Umb.Condition.WorkspaceContentTypeAlias',
      oneOf : ['blogPost','mediaType1']
    }
  ]
};

@enkelmedia enkelmedia force-pushed the 1549-workspace-entity-content-type-condition branch from 4db84b5 to 8ae0c76 Compare June 4, 2024 21:45
@enkelmedia enkelmedia changed the title Adding Condition for Entity Content Type Adding Condition for Content Type Alias Jun 4, 2024
# Conflicts:
#	src/packages/core/extension-registry/conditions/types.ts
@nielslyngsoe nielslyngsoe self-requested a review August 19, 2024 13:51
Copy link
Member

@nielslyngsoe nielslyngsoe left a comment

Choose a reason for hiding this comment

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

Yes, Very nice!

Copy link

sonarcloud bot commented Aug 19, 2024

@nielslyngsoe nielslyngsoe merged commit 890ede4 into umbraco:main Aug 19, 2024
8 checks passed
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.

2 participants