-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Content and media type CRUD controllers and services #14665
Merged
nikolajlauridsen
merged 69 commits into
v14/dev
from
v14/feature/content_type_controller
Aug 17, 2023
Merged
Content and media type CRUD controllers and services #14665
nikolajlauridsen
merged 69 commits into
v14/dev
from
v14/feature/content_type_controller
Aug 17, 2023
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Still need to figure out some kinks
# Conflicts: # src/Umbraco.Core/Services/OperationStatus/ContentTypeOperationStatus.cs
Let's try and be consistent
Just to make sure nothing explodes on the round trip
…found while testing
…ler' into v14/feature/content_type_controller # Conflicts: # src/Umbraco.Core/Services/ContentTypeEditing/DocumentTypeEditingService.cs # src/Umbraco.Core/Services/ContentTypeEditing/IDocumentTypeEditingService.cs
# Conflicts: # src/Umbraco.Cms.Api.Management/Controllers/DocumentType/DocumentTypeControllerBase.cs
# Conflicts: # src/Umbraco.Cms.Api.Management/Controllers/DocumentType/DeleteDocumentTypeController.cs # src/Umbraco.Cms.Api.Management/Controllers/DocumentType/DocumentTypeControllerBase.cs # src/Umbraco.Cms.Api.Management/Controllers/MediaType/MediaTypeControllerBase.cs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good, only found a couple of things 😄
src/Umbraco.Cms.Api.Management/Controllers/DocumentType/DocumentTypeControllerBase.cs
Outdated
Show resolved
Hide resolved
src/Umbraco.Core/Services/ContentTypeEditing/ContentTypeEditingService.cs
Show resolved
Hide resolved
src/Umbraco.Core/Services/ContentTypeEditing/ContentTypeEditingServiceBase.cs
Outdated
Show resolved
Hide resolved
src/Umbraco.Core/Services/ContentTypeEditing/ContentTypeEditingServiceBase.cs
Outdated
Show resolved
Hide resolved
…ntTypeControllerBase.cs Co-authored-by: Mole <nikolajlauridsen@protonmail.ch>
Looks great to me, and from what I've tested it all seems to work great 👍 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Prerequisites
Description
This adds content and media type CRUD capability to the management API. This includes support for inheritance and compositions, which are not supported by the current (intermediate) content type CRUD controllers.
The PR introduces the
ContentTypeEditingService
andMediaTypeEditingService
, which are the content type and media type equivalents to theContentEditingService
andMediaEditingService
, which were introduced earlier for content and media editing.Among many things, the PR includes a refactor of
ContentTypeSort
, which has been long overdue. The refactor is necessary to complete this PR without piling on additional technical debt. A fair few changes in this PR relates exclusively to the refactor, thus strictly not directly to the CRUD features being introduced.Testing this PR
It's no small feat to test this PR, as there are so many pitfalls in the content type space.
The PR includes 120+ new unit tests to test a myriad of service level scenarios. In all likelihood, not all edge cases are covered. Likewise, it is highly unlikely that a systematic test of the API will uncover all potential API issues - the permutations are simply too many. This is not to say that the API shouldn't be tested 😄 but the true test of the API will be when the new backoffice UI starts consuming it. We have to accept that this PR is not flawless and will need amending down the road.
To start testing, reach out to @kjac for Postman collections and test scenarios. Here follows some payloads that can be used initially to test a few creation scenarios.
Create composition base.
Create content type composed by the composition base, having its own tab and group.
Create content type composed by the composition base, adding its own property to the composition tab and group. Also adds the content type above as allowed child type.
Create a media type.