Convert Views UI to routes

That's great so far!

+++ b/core/modules/views/views_ui/lib/Drupal/views_ui/Routing/ViewsUIController.phpundefined
@@ -0,0 +1,354 @@
+class ViewsUIController {

So in general all this controllers should just provide glue code between the actual functionality and the route, so for example they should not contain logic like reportFields(). Instead use a proper object which get stuff injected and let the controller be containerAware, so the question is whether we really should couple all code now and decouple later.

+++ b/core/modules/views/views_ui/lib/Drupal/views_ui/Routing/ViewsUIController.phpundefined
@@ -0,0 +1,354 @@
+  public function listing() {
...
+  public function add() {

Missing @return on all the functions.

+++ b/core/modules/views/views_ui/lib/Drupal/views_ui/Routing/ViewsUIController.phpundefined
@@ -0,0 +1,354 @@
+   * @param string|\Drupal\views\ViewStorageInterface $view
+   *   The view being deleted.

Maybe we should explain what $views_ui could be, as it is announced as string here. Also the @param is wrong as it's $view_ui

+++ b/core/modules/views/views_ui/lib/Drupal/views_ui/Routing/ViewsUIController.phpundefined
@@ -0,0 +1,354 @@
+    $build['preview'] = entity_get_form($view, 'preview');

If we have new code already, should we use the entity manager on this class now to get the form?

+++ b/core/modules/views/views_ui/lib/Drupal/views_ui/ViewEditFormController.phpundefined
@@ -644,7 +644,7 @@ public function renderDisplayTop(ViewUI $view) {
+          'href' => "admin/structure/views/nojs/edit-details/{$view->id()}/$display_id",

+++ b/core/modules/views/views_ui/lib/Drupal/views_ui/ViewPreviewFormController.phpundefined
@@ -82,7 +82,7 @@ protected function actions(array $form, array &$form_state) {
+          'path' => 'admin/structure/views/view/' . $view->id() . '/preview/' . $view->displayID,

+++ b/core/modules/views/views_ui/lib/Drupal/views_ui/ViewUI.phpundefined
@@ -630,7 +630,7 @@ public function renderPreview($display_id, $args = array()) {
+      $this->override_path = 'admin/structure/views/view/' . $this->id() . '/preview/' . $display_id;

Why do we have to change the paths now, this seems to be a bit out of scope of this issue.

+++ b/core/modules/views/views_ui/lib/Drupal/views_ui/ViewsUiBundle.phpundefined
@@ -0,0 +1,29 @@
+    $container->register('views_ui.controller', 'Drupal\views_ui\Routing\ViewsUIController')

As written before, I don't think that the route controllers should be put into the controller all the time.

+++ b/core/modules/views/views_ui/views_ui.moduleundefined
@@ -713,10 +549,10 @@ function views_ui_ajax_callback(ViewExecutable $view, $op) {
-function views_ui_confirm_delete($form, &$form_state, ViewExecutable $view) {
+function views_ui_confirm_delete($form, &$form_state, ViewStorageInterface $view) {
   $form['view'] = array('#type' => 'value', '#value' => $view);
   return confirm_form($form,
-    t('Are you sure you want to delete the %name view?', array('%name' => $view->storage->getHumanName())),
+    t('Are you sure you want to delete the %name view?', array('%name' => $view->getHumanName())),
     'admin/structure/views',
     t('This action cannot be undone.'),
     t('Delete'),
@@ -728,20 +564,6 @@ function views_ui_confirm_delete($form, &$form_state, ViewExecutable $view) {

@@ -728,20 +564,6 @@ function views_ui_confirm_delete($form, &$form_state, ViewExecutable $view) {
  * Form submission handler for views_ui_confirm_delete().
  */
 function views_ui_confirm_delete_submit($form, &$form_state) {
-  $form_state['values']['view']->storage->delete();
+  $form_state['values']['view']->delete();
   $form_state['redirect'] = 'admin/structure/views';

Maybe out of scope as well?

+++ b/core/modules/views/views_ui/lib/Drupal/views_ui/Routing/ViewsUIController.phpundefined
@@ -0,0 +1,354 @@
+class ViewsUIController {

So in general all this controllers should just provide glue code between the actual functionality and the route, so for example they should not contain logic like reportFields(). Instead use a proper object which get stuff injected and let the controller be containeraware

A new class called ViewsUIReportFields or something like that, see #1801356: Entity reference autocomplete using routes as example.
You want your stuff to be not only decoupled from the container but the functionality itself. Having a gigantic class is not that much better then page callbacks.

Regarding breaking, I'm sure there is an issue with tests etc. but it's too late to find that one.

Well yeah there are always a lot of things to fix, feel free to do what you think is best.

I totally agree that the main part should not be container aware, but why to pass the parts along?

It is getting better and better.

+++ b/core/modules/views/views_ui/lib/Drupal/views_ui/Routing/ViewsUIController.phpundefined
@@ -0,0 +1,475 @@
+  protected function upcastView($view) {

Missing @throws

+++ b/core/modules/views/views_ui/lib/Drupal/views_ui/ViewPreviewFormController.phpundefined
@@ -98,7 +98,7 @@ protected function actions(array $form, array &$form_state) {
+    $form_state['build_info']['args'][0] = drupal_container()->get('views_ui.controller')->getViewUI($view->id());

We should for sure move that to another place as the controller should not be used in the actual code, so what about creating an todo?

Overall I think this is a good first pass. There's plenty to clean up yet, but as Tim notes we should probably clean up the stuff this code is using first and iterate here. I'd be OK with moving forward with something more or less like this with the understanding that we will be revisiting it as time goes on and the dependent code here gets tidied up.

A few non-fatal comments:

+++ b/core/modules/views/lib/Drupal/views/Tests/UI/TagTest.php
@@ -38,12 +45,16 @@ public function testViewsUiAutocompleteTag() {
     // Make sure just ten results are returns.
-    $result = views_ui_autocomplete_tag('autocomplete_tag_test');
+    $service_controller = $this->container->get('views_ui.controller');
+    $request = $this->container->get('request');
+    $request->query->set('q', 'autocomplete_tag_test');
+    $result = $service_controller->autocompleteTag();
     $matches = (array) json_decode($result->getContent());

Pedantic nitpick. $service_controller as a variable name is confusing. It took me 3 tries to figure out that you mean "the controller which was registered as a service." I thought at first you were referring to the service container, aka the DIC object.

Maybe just call it $controller, since once it's been retrieved that's all it really is? (As I said, pedantic nitpick.)

+++ b/core/modules/views/views_ui/lib/Drupal/views_ui/Routing/ViewsUIController.php
@@ -0,0 +1,475 @@
+  public function __construct(EntityManager $entity_manager, ViewsDataCache $views_data, Request $request, TempStoreFactory $temp_store_factory) {
+    $this->entityManager = $entity_manager;
+    $this->viewsData = $views_data;
+    $this->request = $request;
+    $this->tempStore = $temp_store_factory->get('views');
+  }

The request will be passed to the individual controller methods if you specify a parameter of Request $request. No need to put it in the constructor. Actually that might break things in some obscure places. :-)

+++ b/core/modules/views/views_ui/lib/Drupal/views_ui/Routing/ViewsUIController.php
@@ -0,0 +1,475 @@
+  public function reportFields() {
+    $views = $this->entityManager->getStorageController('view')->load();
+

Where to put stuff like this is an interesting question. :-) In Symfony, I believe a lot of it ends up as dedicated utility methods on the Entity Manager (roughly equivalent to our Storage Controller), which makes sense when you are editing code rather than configuring it.

I think for right now I'm fine with leaving this code here. Once we clean up other parts of the code, as Tim notes, it may become a lot more obvious what to do with it. That's generally true for page callback => controller conversion in general. We can do it iteratively.

+++ b/core/modules/views/views_ui/lib/Drupal/views_ui/Routing/ViewsUIController.php
@@ -0,0 +1,475 @@
+  public function ajaxOperation($view, $op) {
+    // @todo Remove this when http://drupal.org/node/1798214 is in.
+    $view = $this->upcastView($view);
+
+    // Perform the operation.
+    $view->$op();
+
+    // If the request is via AJAX, return the rendered list as JSON.
+    if ($this->request->request->get('js')) {
+      $list = $this->entityManager->getListController('view')->render();
+      $response = new AjaxResponse();
+      $response->addCommand(new ReplaceCommand('#views-entity-list', drupal_render($list)));
+      return $response;
+    }

Flagging this as one of the places to just pass in the request directly to the controller method.

+++ b/core/modules/views/views_ui/lib/Drupal/views_ui/Routing/ViewsUIController.php
@@ -0,0 +1,475 @@
+  public function ajaxForm($js, $views_ui, $key, $display_id = NULL, $type = NULL, $id = NULL) {
+    // Determine if this is an AJAX submission.

Default values are not needed in controllers. The default values need to be specified by the route definition anyway.

Works for me. We can iterate from here.

In general this is impressive work!!

+++ b/core/modules/views/lib/Drupal/views/Tests/UI/TagTest.phpundefined
@@ -38,12 +45,16 @@ public function testViewsUiAutocompleteTag() {
+    $controller = $this->container->get('views_ui.controller');
+    $request = $this->container->get('request');
+    $request->query->set('q', 'autocomplete_tag_test');
+    $result = $controller->autocompleteTag($request);
...
+    $request->query->set('q', 'autocomplete_tag_test_even');
+    $result = $controller->autocompleteTag($request);

@@ -51,7 +62,8 @@ public function testViewsUiAutocompleteTag() {
+    $request->query->set('q', $this->randomName());
+    $result = $controller->autocompleteTag($request);

As a follow up we might could use Request::create() so we check the actual path as well, but yeah this is something which should be done over time.

+++ b/core/modules/views/views_ui/views_ui.routing.ymlundefined
@@ -70,14 +79,20 @@ views_ui.autocomplete:
+  pattern: '/admin/structure/views/view/{view}/edit/{display_id}'

Why do we not provide a upcaster that converts it's directly to a ViewUI object, I guess this would be maybe a bit more complicated to do?

After a full reinstall this worked really fine. Tested quite different parts of the views edit UI, so I think this is finally ready to go
so we can iterate on top of it later.

.

Awesome! It's lovely to see such a big module converted to use the new routing system. :)

Committed and pushed to 8.x. Thanks!