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.

Sign up for GitHub

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

Jump to bottom

Fix Content Drivers #15680

Merged
merged 1 commit into from
Apr 7, 2024
Merged

Fix Content Drivers #15680

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 5 additions & 7 deletions src/OrchardCore.Modules/OrchardCore.Contents/Drivers/ContentsDriver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,19 +34,17 @@ public override async Task<IDisplayResult> DisplayAsync(ContentItem contentItem,
// We add custom alternates. This could be done generically to all shapes coming from ContentDisplayDriver but right now it's
// only necessary on this shape. Otherwise c.f. ContentPartDisplayDriver

var contentItemViewModel = new ContentItemViewModel(contentItem);

var results = new List<IDisplayResult>()
{
Shape("ContentsTags_SummaryAdmin", contentItemViewModel).Location("SummaryAdmin", "Tags:10"),
Shape("ContentsMeta_SummaryAdmin", contentItemViewModel).Location("SummaryAdmin", "Meta:20"),
Shape("ContentsTags_SummaryAdmin", new ContentItemViewModel(contentItem)).Location("SummaryAdmin", "Tags:10"),
Shape("ContentsMeta_SummaryAdmin", new ContentItemViewModel(contentItem)).Location("SummaryAdmin", "Meta:20"),
Copy link
Member

Choose a reason for hiding this comment

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

Why you need to new up ContentItemViewModel in each call?!!

Copy link
Member Author

Choose a reason for hiding this comment

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

I am not positive on this response. But, I believe because the VM here is a shape but rendered using a different type. For it to work correctly, each shape should be a new instance as it'll have to be built and rendered separately.

The PR #15652 caused the issue which created a single VM instead of having multiple

Copy link
Member

Choose a reason for hiding this comment

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

Seems weird

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah. It's worth trying to pass a model instead of a shape to all these views. In other words, do not let ContentItemViewModel inherit from ShapeViewModel, create a single instance and pass it to all these views. This may work, but could break someone. Anyway, if it works it should reduce allocations. Feel free to try it. Otherwise, I may try it when I have time. Meanwhile this PR fixes the referenced problem while we can entertain other ways to improve.

};

var contentTypeDefinition = await _contentDefinitionManager.GetTypeDefinitionAsync(contentItem.ContentType);

if (contentTypeDefinition != null)
{
var contentsMetadataShape = Shape("ContentsMetadata", contentItemViewModel)
var contentsMetadataShape = Shape("ContentsMetadata", new ContentItemViewModel(contentItem))
.Location("Detail", "Content:before");

contentsMetadataShape.Displaying(ctx =>
Expand Down Expand Up @@ -74,8 +72,8 @@ public override async Task<IDisplayResult> DisplayAsync(ContentItem contentItem,
var user = _httpContextAccessor.HttpContext.User;

results.Add(contentsMetadataShape);
results.Add(Shape("ContentsButtonEdit_SummaryAdmin", contentItemViewModel).Location("SummaryAdmin", "Actions:10"));
results.Add(Shape("ContentsButtonActions_SummaryAdmin", contentItemViewModel).Location("SummaryAdmin", "ActionsMenu:10")
results.Add(Shape("ContentsButtonEdit_SummaryAdmin", new ContentItemViewModel(contentItem)).Location("SummaryAdmin", "Actions:10"));
results.Add(Shape("ContentsButtonActions_SummaryAdmin", new ContentItemViewModel(contentItem)).Location("SummaryAdmin", "ActionsMenu:10")
.RenderWhen(async () =>
{
var hasPublishPermission = await _authorizationService.AuthorizeAsync(user, CommonPermissions.PublishContent, contentItem);
Expand Down
Loading