This method is currently living in our View storage class, it doesn't really belong there. The storage class doesn't need to care about plugin managers or anything like that. This is only used in Views UI, specifically in the list controller.

Let's move it there. I have currently just commented out the (random) test assertion in ViewStorageTest for this. Not sure we have a test specifically for the listing controller?

Files: 
CommentFileSizeAuthor
#20 deper.png37.71 KBdawehner
#20 vdc-2012882-20.patch10.17 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 57,233 pass(es).
[ View ]
#20 interdiff.txt1.64 KBdawehner
#16 2012882-16.patch10.22 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: PHP Fatal error encountered during run_tests.sh. See review log for details..
[ View ]
#16 interdiff-2012882-16.txt1.03 KBdamiankloip
#13 drupal-2012882-11.patch10.12 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 56,502 pass(es), 37 fail(s), and 0 exception(s).
[ View ]
#13 interdiff.txt644 bytesdawehner
#11 2012882-11.patch10.12 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 54,985 pass(es), 38 fail(s), and 0 exception(s).
[ View ]
#11 interdiff-2012882-11.txt838 bytesdamiankloip
#9 drupal-2012882-9.patch10.08 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed during invocation of run-tests.sh.
[ View ]
#8 2012882-8.patch6.45 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 56,364 pass(es).
[ View ]
#8 interdiff-2012882-8.txt916 bytesdamiankloip
#6 2012882-6.patch6.31 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 55,212 pass(es).
[ View ]
#6 interdiff-2012882-6.txt1.16 KBdamiankloip
#3 2012882-3.patch6.31 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 56,103 pass(es), 0 fail(s), and 21 exception(s).
[ View ]
#3 interdiff-2012882-3.txt3.72 KBdamiankloip
vdc.move-getDisplaysList.patch5.14 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 55,319 pass(es).
[ View ]

Comments

Status:Active» Needs review

+++ b/core/modules/views_ui/lib/Drupal/views_ui/ViewListController.phpundefined
@@ -9,6 +9,7 @@
+use Drupal\views\Views;
@@ -156,4 +157,24 @@ public function render() {
+  public function getDisplaysList($view) {

This can be protected now

+++ b/core/modules/views_ui/lib/Drupal/views_ui/ViewListController.phpundefined
@@ -9,6 +9,7 @@
+use Drupal\views\Views;
@@ -156,4 +157,24 @@ public function render() {
+    $manager = Views::pluginManager('display');

We can inject this here!

+++ b/core/modules/views_ui/views_ui.theme.incundefined
@@ -103,7 +101,7 @@ function template_preprocess_views_ui_view_info(&$variables) {
-  $output .= '<h3 class="views-ui-view-title">' . $variables['title'] . "</h3>\n";
+  $output .= '<h3 class="views-ui-view-title">' . $variables['label'] . "</h3>\n";

Oh come on :) Scope creep!

StatusFileSize
new3.72 KB
new6.31 KB
FAILED: [[SimpleTest]]: [MySQL] 56,103 pass(es), 0 fail(s), and 21 exception(s).
[ View ]

Ha :) How about this then? I'm not sure about injecting the storage controller? I guess we have to just get it from the container like that? As createInstance doesn't have knowledge that the actual EntityManager has called it.

+++ b/core/modules/views_ui/lib/Drupal/views_ui/ViewListController.phpundefined
@@ -7,13 +7,55 @@
+    $this->entityType = $entity_type;
+    $this->storage = $storage;
+    $this->entityInfo = $entity_info;

Any reason to not just call parent::__construct() I don't care much either way

+++ b/core/modules/views_ui/lib/Drupal/views_ui/ViewListController.phpundefined
@@ -7,13 +7,55 @@
+    $this->displayManager = $display_manager;
@@ -156,4 +198,24 @@ public function render() {
+    $manager = Views::pluginManager('display');
...
+      $definition = $manager->getDefinition($display['display_plugin']);

Seems you forgot to actually use it?

Status:Needs review» Needs work

The last submitted patch, 2012882-3.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new1.16 KB
new6.31 KB
PASSED: [[SimpleTest]]: [MySQL] 55,212 pass(es).
[ View ]

Whoops, missed out a use. Also made changes from feedback in #4, thanks.

EDIT: I did not call the parent, as that calls entity_get_info() in it's constructor. We can inject that now.

+++ b/core/modules/views_ui/lib/Drupal/views_ui/ViewListController.phpundefined
@@ -156,4 +198,23 @@ public function render() {
+   * Gets a list of displays included in the view.
+   *
+   * @return array
+   *   An array of display types that this view includes.

Missing @param documentation

+++ b/core/modules/views_ui/lib/Drupal/views_ui/ViewListController.phpundefined
@@ -156,4 +198,23 @@ public function render() {
+  protected function getDisplaysList($view) {

That's what I don't really get is how to test such functionality. This is clearly a method which could be proper unit tested, but it's marked as protected

StatusFileSize
new916 bytes
new6.45 KB
PASSED: [[SimpleTest]]: [MySQL] 56,364 pass(es).
[ View ]

Hmm, not sure. I guess we could only test it in the UI :/ If only we had some way to test protected methods or something.... ;) (Do you remember that issue?)

StatusFileSize
new10.08 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed during invocation of run-tests.sh.
[ View ]

Here is a unit test for this piece. This also fixes a small issue in the actual code, as it uses render arrays now.

Status:Needs review» Needs work

The last submitted patch, drupal-2012882-9.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new838 bytes
new10.12 KB
FAILED: [[SimpleTest]]: [MySQL] 54,985 pass(es), 38 fail(s), and 0 exception(s).
[ View ]

I guess we might have to do this for now?

Status:Needs review» Needs work

The last submitted patch, 2012882-11.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new644 bytes
new10.12 KB
FAILED: [[SimpleTest]]: [MySQL] 56,502 pass(es), 37 fail(s), and 0 exception(s).
[ View ]

This should fix it.

Status:Needs review» Needs work

The last submitted patch, drupal-2012882-11.patch, failed testing.

I guess we both had the same change for that.

Status:Needs work» Needs review
StatusFileSize
new1.03 KB
new10.22 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: PHP Fatal error encountered during run_tests.sh. See review log for details..
[ View ]

hmm, SO something has changed recently that breaks table cells being rendered with drupal_render() ?!! I'm sure this worked once upon a time.

Status:Needs review» Needs work
Issue tags:-VDC

The last submitted patch, 2012882-16.patch, failed testing.

Status:Needs work» Needs review

#16: 2012882-16.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+VDC

The last submitted patch, 2012882-16.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new1.64 KB
new10.17 KB
PASSED: [[SimpleTest]]: [MySQL] 57,233 pass(es).
[ View ]
new37.71 KB

We have to go deeper!

Note: This is an interdiff against #13

Of course!!

Status:Needs review» Reviewed & tested by the community

Nice fix @dawehner!

Status:Reviewed & tested by the community» Fixed

Committed a544e13 and pushed to 8.x. Thanks!

Automatically closed -- issue fixed for 2 weeks with no activity.