Convert system_theme_disable() to a new style controller

+++ b/core/modules/system/lib/Drupal/system/Controller/ThemeController.php
@@ -0,0 +1,46 @@
+use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;
+/**
+ * Controller for theme handling.

Missing an empty line here.

+++ b/core/modules/system/lib/Drupal/system/Controller/ThemeController.php
@@ -0,0 +1,46 @@
+        if ($theme === config('system.theme')->get('default') || $theme === config('system.theme')->get('admin')) {

Config should be injected.

+++ b/core/modules/system/lib/Drupal/system/Controller/ThemeController.php
@@ -0,0 +1,46 @@
+      drupal_goto('admin/appearance');

This should use RedirectResponse.

I'm taking up this issue and joining it with issue #1987852: Convert system_theme_enable() to a new style controller

amateescu,

You mention in comment #3 that the config should be injected.
I'm trying to find an example for this, but can't seem to find anything useful.

I tried injecting a ConfigFactory, but then i end up with the next error:

Drupal\Core\Config\ConfigImporterException: config.installer is already importing in Drupal\Core\Config\ConfigImporter->import() (line 219 of /Users/Jeroen/development/drupal/core/lib/Drupal/Core/Config/ConfigImporter.php)

Do you have any pointers or examples of where to look ?

Thanks !

Sure, you can use this one as an example: StatisticsSettingsForm::create().

If that doesn't work out, could you post your patch anyway so more people can help you debug it? It's also possible that this might be a bug coming from this newly committed issue #1890784: Refactor configuration import and sync functions.

Here's my current patch, which to me seems rather complete, but ends up with the error mentioned in comment #5.
Maybe i'm missing something ?

Here's a patch that implements a proper constructor and implements the ControllerInterface. As a result, I'm injecting the config factory and using in the disable() function.

+++ b/core/modules/system/lib/Drupal/system/Controller/ThemeController.phpundefined
@@ -0,0 +1,74 @@
+   * @var \Drupal\Core\Config\ConfigFactory|ConfigFactory

Needs comment

+++ b/core/modules/system/lib/Drupal/system/Controller/ThemeController.phpundefined
@@ -0,0 +1,74 @@
+      $config = $this->configFactory;

Should $config = $this->configFactory->get('system.theme');

In #7, I"m not getting the right set of config. This patch gets system.theme config properly.

@cosmicdreams, this issue was assigned to @oenie and he is working on it..

@oenie, I applied the patch locally (after fixing the conflict in system.routing.yml) and theme enabling/disabling works just fine, no errrors at all :) Let's try to feed the testbot an updated patch.

PS. I also fixed some trailing whitespaces and code style errors. Interdiff is to #7.

+++ b/core/modules/system/lib/Drupal/system/Controller/ThemeController.phpundefined
@@ -0,0 +1,126 @@
+        if ($theme === $this->configFactory->get('system.theme')->get('default') || $theme === $this->configFactory->get('system.theme')->get('admin')) {

Let's just inject the system.theme config object and not the full factory

+++ b/core/modules/system/lib/Drupal/system/Controller/ThemeController.phpundefined
@@ -0,0 +1,126 @@
+      return new RedirectResponse(url("admin/appearance", array('absolute' => TRUE)));
...
+      return new RedirectResponse(url("admin/appearance", array('absolute' => TRUE)));

/me hides ... single quotes.

+++ b/core/modules/system/system.moduleundefined
@@ -704,17 +704,13 @@ function system_menu() {
   $items['admin/appearance/enable'] = array(
     'title' => 'Enable theme',
-    'page callback' => 'system_theme_enable',
-    'access arguments' => array('administer themes'),
+    'route_name' => 'system_theme_enable',
     'type' => MENU_CALLBACK,
-    'file' => 'system.admin.inc',
   );
   $items['admin/appearance/disable'] = array(
     'title' => 'Disable theme',
-    'page callback' => 'system_theme_disable',
-    'access arguments' => array('administer themes'),
+    'route_name' => 'system_theme_disable',
     'type' => MENU_CALLBACK,
-    'file' => 'system.admin.inc',

Let's get rid of it ... they are menu callbacks.

Another try, including dawehner's comments.
I've attached an interdiff to the patch of comment 11

Let's try that again as a patch.
Another try, including dawehner's comments.
I've attached an interdiff to the patch of comment #12 by amateescu

#15: 1987850-route-system-theme-disable-12.patch queued for re-testing.

Reroll of the patch against the current code.
Learning a bunch of new stuff :)

Looks great!

Committed f94711c and pushed to 8.x. Thanks!