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

#8565: Delete and Unpublish buttons in backoffice content edit #8566

Open
wants to merge 13 commits into
base: 1.10.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
26 changes: 26 additions & 0 deletions src/Orchard.Web/Core/Contents/Controllers/AdminController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,32 @@ public ActionResult EditAndPublishPOST(int id, string returnUrl) {
return EditPOST(id, returnUrl, contentItem => _contentManager.Publish(contentItem));
}

/// <summary>
/// This action is specific to the submit button of the edit form.
/// Unpublish logic is the same of the Unpublish action, which is called by the content list.
/// </summary>
/// <param name="id"></param>
/// <param name="returnUrl"></param>
/// <returns></returns>
[HttpPost, ActionName("Edit")]
Copy link
Member

Choose a reason for hiding this comment

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

Why is this different that the other unpublish/delete actions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added delete and unpublish actions referencing specific submits to the controller. There already were unpublish and delete actions (the ones called from the lists), so I called those inside the new actions, specific to the edit form.

[Mvc.FormValueRequired("submit.Unpublish")]
public ActionResult EditUnpublishPOST(int id, string returnUrl) {
return Unpublish(id, returnUrl);
}

/// <summary>
/// This action is specific to the submit button of the edit form.
/// Delete logic is the same of the Remove action, which is called by the content list.
/// </summary>
/// <param name="id"></param>
/// <param name="returnUrl"></param>
/// <returns></returns>
[HttpPost, ActionName("Edit")]
[Mvc.FormValueRequired("submit.Delete")]
public ActionResult EditDeletePOST(int id, string returnUrl) {
return Remove(id, returnUrl);
}

private ActionResult EditPOST(int id, string returnUrl, Action<ContentItem> conditionallyPublish) {
var contentItem = _contentManager.Get(id, VersionOptions.DraftRequired);

Expand Down
10 changes: 9 additions & 1 deletion src/Orchard.Web/Core/Contents/Drivers/ContentsDriver.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,16 @@ protected override DriverResult Display(ContentPart part, string displayType, dy
protected override DriverResult Editor(ContentPart part, dynamic shapeHelper) {
var results = new List<DriverResult> { ContentShape("Content_SaveButton", saveButton => saveButton) };

if (part.TypeDefinition.Settings.GetModel<ContentTypeSettings>().Draftable)
if (part.TypeDefinition.Settings.GetModel<ContentTypeSettings>().Draftable) {
results.Add(ContentShape("Content_PublishButton", publishButton => publishButton));
if (part.ContentItem.IsPublished()) {
BenedekFarkas marked this conversation as resolved.
Show resolved Hide resolved
results.Add(ContentShape("Content_UnpublishButton", unpublishButton => unpublishButton));
}
Comment on lines +26 to +28
Copy link
Member

@BenedekFarkas BenedekFarkas Apr 16, 2024

Choose a reason for hiding this comment

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

Re: the previous comment - shouldn't be just this then, since the template already checks IsPublished?

Suggested change
if (part.ContentItem.IsPublished()) {
results.Add(ContentShape("Content_UnpublishButton", unpublishButton => unpublishButton));
}
results.Add(ContentShape("Content_UnpublishButton", unpublishButton => unpublishButton));

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't remember right now. I'll check asap.

Copy link
Member

Choose a reason for hiding this comment

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

The Content.UnpublishButton template does check if the item is published.

}

if (part.Id > 0) {
results.Add(ContentShape("Content_DeleteButton", deleteButton => deleteButton));
}

return Combined(results.ToArray());
}
Expand Down
2 changes: 2 additions & 0 deletions src/Orchard.Web/Core/Contents/Placement.info
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@
Parts_Contents_Publish_SummaryAdmin
-->
<!-- edit "shape" -->
<Place Content_UnpublishButton="Sidebar:25"/>
<Place Content_PublishButton="Sidebar:24"/>
<Place Content_SaveButton="Sidebar:23"/>
<Place Content_DeleteButton="Sidebar:22"/>
Comment on lines +9 to +12
Copy link
Member

Choose a reason for hiding this comment

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

Reordered them according to placement. Also Content_DeleteButton shouldn't be the first one, because it will be become the default action (e.g., when you hit enter while editing the title) just by virtue of being the first in the rendered HTML.
SaveButton should come first, then PublishButton (because SaveButton is not rendered if the type is not Draftable).

Suggested change
<Place Content_UnpublishButton="Sidebar:25"/>
<Place Content_PublishButton="Sidebar:24"/>
<Place Content_SaveButton="Sidebar:23"/>
<Place Content_DeleteButton="Sidebar:22"/>
<Place Content_SaveButton="Sidebar:23"/>
<Place Content_PublishButton="Sidebar:24"/>
<Place Content_UnpublishButton="Sidebar:25"/>
<Place Content_DeleteButton="Sidebar:26"/>

<Match DisplayType="Detail">
<Place Parts_Contents_Publish="Content:5"/>
</Match>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
@using Orchard.ContentManagement;
@using Orchard.Core.Contents;
@using Orchard.Utility.Extensions
Copy link
Member

Choose a reason for hiding this comment

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

Unused using.


@if (Authorizer.Authorize(Permissions.DeleteContent, (IContent)Model.ContentItem)) {
<fieldset class="delete-button">
<button type="submit" name="submit.Delete" value="submit.Delete" itemprop="RemoveUrl">@T("Delete")</button>
</fieldset>
}
11 changes: 11 additions & 0 deletions src/Orchard.Web/Core/Contents/Views/Content.UnpublishButton.cshtml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
@using Orchard.ContentManagement;
@using Orchard.Core.Contents;
@using Orchard.Utility.Extensions;
Copy link
Member

Choose a reason for hiding this comment

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

Unused using.

@{
var contentItem = Model.ContentItem as IContent;
}
Comment on lines +4 to +6
Copy link
Member

Choose a reason for hiding this comment

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

Add empty lines around this block. I pointed this out earlier too and it would be easier if you just accepted my change suggestions in bulk, so that I don't have to go through all of them again to figure out what's fixed and what isn't.

Suggested change
@{
var contentItem = Model.ContentItem as IContent;
}
@{
var contentItem = Model.ContentItem as IContent;
}

@if (Authorizer.Authorize(Permissions.PublishContent, contentItem) && contentItem.IsPublished()) {
<fieldset class="unpublish-button">
<button type="submit" name="submit.Unpublish" value="submit.Unpublish">@T("Unpublish")</button>
</fieldset>
}
2 changes: 2 additions & 0 deletions src/Orchard.Web/Core/Orchard.Core.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -610,6 +610,8 @@
<Content Include="Navigation\Views\Admin\Edit.cshtml" />
</ItemGroup>
<ItemGroup>
<Content Include="Contents\Views\Content.UnpublishButton.cshtml" />
<Content Include="Contents\Views\Content.DeleteButton.cshtml" />
<None Include="packages.config" />
<Content Include="Title\Views\DefinitionTemplates\TitlePartSettings.cshtml" />
</ItemGroup>
Expand Down
Copy link
Member

Choose a reason for hiding this comment

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

Please use remove and sort usings and auto-formatting.

Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,12 @@ public ActionResult EditPOST(int blogId, int postId, string returnUrl) {
});
}

[HttpPost, ActionName("Edit")]
[Mvc.FormValueRequired("submit.Delete")]
public ActionResult EditDeletePOST(int blogId, int postId, string returnUrl) {
return Delete(blogId, postId);
}

[HttpPost, ActionName("Edit")]
[FormValueRequired("submit.Publish")]
public ActionResult EditAndPublishPOST(int blogId, int postId, string returnUrl) {
Expand All @@ -156,6 +162,12 @@ public ActionResult EditAndPublishPOST(int blogId, int postId, string returnUrl)
return EditPOST(blogId, postId, returnUrl, contentItem => Services.ContentManager.Publish(contentItem));
}

[HttpPost, ActionName("Edit")]
[Mvc.FormValueRequired("submit.Unpublish")]
public ActionResult EditUnpublishPOST(int blogId, int postId, string returnUrl) {
return Unpublish(blogId, postId);
}

public ActionResult EditPOST(int blogId, int postId, string returnUrl, Action<ContentItem> conditionallyPublish) {
var blog = _blogService.Get(blogId, VersionOptions.Latest);
if (blog == null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1101,17 +1101,17 @@ html.dyn #submit-pager, html.dyn .apply-bulk-actions-auto { display:none; }
padding:0;
}

fieldset.publish-button, fieldset.delete-button, fieldset.save-button {
fieldset.publish-button, fieldset.delete-button, fieldset.save-button, fieldset.unpublish-button {
clear:none;
float:left;
}
fieldset.save-button {
clear:left;
}
fieldset.publish-button {
fieldset.publish-button, fieldset.unpublish-button {
margin: 0 12px 0 0;
padding: 0 12px;
border-right:1px solid #ccc;
border-right: 1px solid #ccc;
}
fieldset.delete-button {
margin: 0 0 0 12px;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,6 @@ protected override DriverResult Editor(LayerPart layerPart, dynamic shapeHelper)
() => shapeHelper.EditorTemplate(TemplateName: "Parts.Widgets.LayerPart", Model: layerPart, Prefix: Prefix))
};

if (layerPart.Id > 0)
results.Add(ContentShape("Widget_DeleteButton",
deleteButton => deleteButton));

return Combined(results.ToArray());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System.Collections.Generic;
using Orchard.ContentManagement;
using Orchard.ContentManagement.Drivers;
using Orchard.Core.Contents.Settings;
using Orchard.Localization;
using Orchard.Utility.Extensions;
using Orchard.Widgets.Models;
Expand Down Expand Up @@ -35,26 +36,27 @@ protected override DriverResult Editor(WidgetPart widgetPart, dynamic shapeHelpe
() => shapeHelper.EditorTemplate(TemplateName: "Parts.Widgets.WidgetPart", Model: widgetPart, Prefix: Prefix))
};

if (widgetPart.Id > 0)
results.Add(ContentShape("Widget_DeleteButton",
deleteButton => deleteButton));
if (widgetPart.Id > 0 && widgetPart.TypeDefinition.Settings.GetModel<ContentTypeSettings>().Draftable) {
BenedekFarkas marked this conversation as resolved.
Show resolved Hide resolved
results.Add(ContentShape("Content_UnpublishButton",
unpublishButton => unpublishButton));
}
Comment on lines +39 to +42
Copy link
Member

Choose a reason for hiding this comment

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

This is already rendered by ContentsDriver.

Suggested change
if (widgetPart.Id > 0 && widgetPart.TypeDefinition.Settings.GetModel<ContentTypeSettings>().Draftable) {
results.Add(ContentShape("Content_UnpublishButton",
unpublishButton => unpublishButton));
}


return Combined(results.ToArray());
}

protected override DriverResult Editor(WidgetPart widgetPart, IUpdateModel updater, dynamic shapeHelper) {
updater.TryUpdateModel(widgetPart, Prefix, null, null);

if(string.IsNullOrWhiteSpace(widgetPart.Title)) {
if (string.IsNullOrWhiteSpace(widgetPart.Title)) {
updater.AddModelError("Title", T("Title can't be empty."));
}

// if there is a name, ensure it's unique
if(!string.IsNullOrWhiteSpace(widgetPart.Name)) {
if (!string.IsNullOrWhiteSpace(widgetPart.Name)) {
widgetPart.Name = widgetPart.Name.ToHtmlName();

var widgets = _contentManager.Query<WidgetPart, WidgetPartRecord>().Where(x => x.Name == widgetPart.Name && x.Id != widgetPart.Id).Count();
if(widgets > 0) {
if (widgets > 0) {
updater.AddModelError("Name", T("A Widget with the same Name already exists."));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,9 +207,6 @@
<Content Include="Views\EditorTemplates\Parts.Widgets.LayerPart.cshtml" />
<Content Include="Views\EditorTemplates\Parts.Widgets.WidgetPart.cshtml" />
</ItemGroup>
<ItemGroup>
<Content Include="Views\Widget.DeleteButton.cshtml" />
</ItemGroup>
<ItemGroup>
<Content Include="Web.config" />
</ItemGroup>
Expand Down
1 change: 0 additions & 1 deletion src/Orchard.Web/Modules/Orchard.Widgets/Placement.info
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
<Placement>
<Place Widget_DeleteButton="Sidebar:25"/>
<Place Parts_Widgets_LayerPart="Content:1"/>
<Place Parts_Widgets_WidgetPart="Content:1"/>
<Place Parts_Widgets_WidegetBagPart="Content:5"/>
Expand Down

This file was deleted.

8 changes: 4 additions & 4 deletions src/Orchard.Web/Themes/TheAdmin/Styles/site.css
Original file line number Diff line number Diff line change
Expand Up @@ -1130,14 +1130,14 @@ html.dyn #submit-pager, html.dyn .apply-bulk-actions-auto { display:none; }
padding:0;
}

fieldset.publish-button, fieldset.delete-button, fieldset.save-button {
fieldset.publish-button, fieldset.delete-button, fieldset.save-button, fieldset.unpublish-button {
clear:none;
float:left;
}
fieldset.save-button {
clear:left;
}
fieldset.publish-button {
fieldset.publish-button, fieldset.unpublish-button {
margin: 0 12px 0 0;
padding: 0 12px;
border-right:1px solid #ccc;
Copy link
Member

Choose a reason for hiding this comment

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

This styling was overcomplicated to begin with, but we don't need to complicate it further. You can apply this to the whole section between lines 1128 and 1147, so we don't need to care about how many items are rendered and in what order:

.edit-item-sidebar fieldset {
    margin: 0;
    padding: 0;
    clear: none;
    float: left;
}
.edit-item-sidebar fieldset:not(:first-child) {
    margin-left: 12px;
}
.edit-item-sidebar fieldset:first-child {
    clear: left;
}
fieldset.publish-button, fieldset.unpublish-button {
    padding-right: 12px;
    border-right: 1px solid #ccc;
}
fieldset.delete-button {
    float: right;
}

Expand Down Expand Up @@ -1495,13 +1495,13 @@ html.dir-rtl {
margin-left:inherit;
margin-right:10px;
}
.dir-rtl fieldset.publish-button, fieldset.delete-button, .dir-rtl fieldset.save-button {
Copy link
Member

Choose a reason for hiding this comment

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

Same as above, RTL version for lines 1498-1509:

.dir-rtl .edit-item-sidebar fieldset {
    float: right;
}
.dir-rtl .edit-item-sidebar fieldset:first-child {
    clear: right;
}
.dir-rtl .edit-item-sidebar fieldset:not(:first-child) {
    margin-left: 0;
    margin-right: 12px;
}
.dir-rtl fieldset.publish-button, fieldset.unpublish-button {
    padding-right: 0;
    border-right: none;
    padding-left: 12px;
    border-left: 1px solid #ccc;
}
.dir-rtl fieldset.delete-button {
    float: left;
}

.dir-rtl fieldset.publish-button, fieldset.delete-button, .dir-rtl fieldset.save-button, .dir-rtl fieldset.unpublish-button {
float:right;
}
.dir-rtl fieldset.save-button {
clear:right;
}
.dir-rtl fieldset.publish-button {
.dir-rtl fieldset.publish-button, .dir-rtl fieldset.unpublish-button {
margin: 0 0 0 12px ;
}
.dir-rtl fieldset.delete-button {
Expand Down