OrchardCMS / OrchardCore

Orchard Core is an open-source modular and multi-tenant application framework built with ASP.NET Core, and a content management system (CMS) built on top of that framework.
https://orchardcore.net
BSD 3-Clause "New" or "Revised" License
7.43k stars 2.4k forks source link

If you have a child theme to TheAdmin and disable TheAdmin, the admin is unusable #9836

Open rjpowers10 opened 3 years ago

rjpowers10 commented 3 years ago

Describe the bug

We made an admin theme that sets TheAdmin as the base theme in Manifest.cs. If you go to /Admin/Themes you can disable TheAdmin, but then the admin becomes unusable.

To Reproduce

Steps to reproduce the behavior:

  1. Create an admin theme that sets TheAdmin as the base theme. In my example I added very little to the theme (we only use it to override ContentPart.Edit.cshtml).
  2. Go to /Admin/Themes
  3. Disable TheAdmin

Expected behavior

I'm not exactly sure what the expectation here is. I see that the frontend has a safe theme but I don't think there is a safe theme for the admin. Maybe /Admin/Themes shouldn't allow you to disable base themes?

2021-06-30T15:59:22.7773645-04:00|Default|0HM9S05LBB95A:00000019|Serilog.AspNetCore.RequestLoggingMiddleware|ERR|HTTP GET /Admin/Themes responded 500 in 4449.6118 ms
System.Exception: Shape type 'Layout' not found
   at OrchardCore.DisplayManagement.Implementation.DefaultHtmlDisplay.ExecuteAsync(DisplayContext context)
   at OrchardCore.DisplayManagement.Implementation.DefaultHtmlDisplay.ExecuteAsync(DisplayContext context)
   at OrchardCore.DisplayManagement.Theming.ThemeLayout.ExecuteAsync()
   at Microsoft.AspNetCore.Mvc.Razor.RazorView.RenderPageCoreAsync(IRazorPage page, ViewContext context)
   at Microsoft.AspNetCore.Mvc.Razor.RazorView.RenderPageAsync(IRazorPage page, ViewContext context, Boolean invokeViewStarts)
   at Microsoft.AspNetCore.Mvc.Razor.RazorView.RenderLayoutAsync(ViewContext context, ViewBufferTextWriter bodyWriter)
   at Microsoft.AspNetCore.Mvc.Razor.RazorView.RenderAsync(ViewContext context)
   at Microsoft.AspNetCore.Mvc.ViewFeatures.ViewExecutor.ExecuteAsync(ViewContext viewContext, String contentType, Nullable`1 statusCode)
   at Microsoft.AspNetCore.Mvc.ViewFeatures.ViewExecutor.ExecuteAsync(ViewContext viewContext, String contentType, Nullable`1 statusCode)
   at Microsoft.AspNetCore.Mvc.ViewFeatures.ViewExecutor.ExecuteAsync(ActionContext actionContext, IView view, ViewDataDictionary viewData, ITempDataDictionary tempData, String contentType, Nullable`1 statusCode)
   at Microsoft.AspNetCore.Mvc.ViewFeatures.ViewResultExecutor.ExecuteAsync(ActionContext context, ViewResult result)
   at Microsoft.AspNetCore.Mvc.ViewResult.ExecuteResultAsync(ActionContext context)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeNextResultFilterAsync>g__Awaited|29_0[TFilter,TFilterAsync](ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.Rethrow(ResultExecutedContextSealed context)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.ResultNext[TFilter,TFilterAsync](State& next, Scope& scope, Object& state, Boolean& isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.InvokeResultFilters()
--- End of stack trace from previous location ---
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeNextResourceFilter>g__Awaited|24_0(ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.Rethrow(ResourceExecutedContextSealed context)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.InvokeFilterPipelineAsync()
--- End of stack trace from previous location ---
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeAsync>g__Awaited|17_0(ResourceInvoker invoker, Task task, IDisposable scope)
   at Microsoft.AspNetCore.Routing.EndpointMiddleware.<Invoke>g__AwaitRequestTask|6_0(Endpoint endpoint, Task requestTask, ILogger logger)
   at SixLabors.ImageSharp.Web.Middleware.ImageSharpMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Localization.RequestLocalizationMiddleware.Invoke(HttpContext context)
   at OrchardCore.Diagnostics.DiagnosticsStartupFilter.<>c__DisplayClass3_0.<<Configure>b__1>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.AspNetCore.Diagnostics.StatusCodePagesMiddleware.Invoke(HttpContext context)
   at OrchardCore.ContentPreview.PreviewStartupFilter.<>c.<<Configure>b__1_1>d.MoveNext()
--- End of stack trace from previous location ---
   at OrchardCore.Modules.ModularTenantRouterMiddleware.Invoke(HttpContext httpContext)
   at Serilog.AspNetCore.RequestLoggingMiddleware.Invoke(HttpContext httpContext)
deanmarcussen commented 3 years ago

Normally the expection is that you also provide the base theme as a dependency in the manifest

i.e. Dependencies = 'TheAdmin'

That should prevent it from being disabled.

rjpowers10 commented 3 years ago

Interesting, I would have expected the base theme to imply a dependency. Regardless, adding TheAdmin as a dependency didn't change anything. I was still able to disable TheAdmin and then the admin became unusable with the same error.

jtkech commented 3 years ago

@rjpowers10

I would have expected the base theme to imply a dependency.

It does, in fact a theme implicitly depends on all features that are not a theme, and on its base themes. It is used e.g. for the alternates views ordering, so that a theme has an higher precedence than a module feature and than its base themes.

In the Features page when disabling a feature (the dependency) there is an alert listing the dependent features (if any) that would also be disabled, here we don't check implicit dependencies, disabling a given feature should not disable all themes.

But yes, in the Themes page at least what is missing is to check if it is a base theme of other derived themes, and if so we could diplay the same kind of alert message saying that the derived themes (if any) would also be disabled.