Currently, somewhere along the way, we have broken the way a style plugin decides whether it can use fields.

Steps to reproduce
- Create a new view
- Create a new display for that view (E.g. page)
- Change row plugin type to fields (E.g. from 'Content', 'User', etc.. - entity)
- Fields plugin will be selected ok, but where you would expect to see fields (and link to add them) you will see 'The selected style or row format does not utilize fields.' text.

This should have a UI test to reproduce.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

damiankloip’s picture

Waaa, sorry. Let's add the usesRowPlugin option to the test style plugin too - so the test is fairer.

Status: Needs review » Needs work

The last submitted patch, vdc-style-fields.patch, failed testing.

damiankloip’s picture

Forgot about the style plugin test. If we just move around the assertions and setUsesRowPlugin to FALSE instead of TRUE, we should be good for both tests.

Les Lim’s picture

Status: Needs work » Needs review

Changing status for tests.

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

The last submitted patch, 2015999.patch, failed testing.

damiankloip’s picture

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

#3: 2015999.patch queued for re-testing.

dawehner’s picture

+++ b/core/modules/views/lib/Drupal/views/Tests/Plugin/StyleTest.phpundefined
@@ -50,45 +50,46 @@ protected function setUp() {
+    $view->style_plugin->setUsesRowPlugin(FALSE);
+    $this->assertTrue($view->style_plugin instanceof StyleTestPlugin, 'Make sure the right style plugin class is loaded.');

Should we also check that $view->rowPlugin is now not an instance of RowTest?

Beside from this, this looks perfect.

damiankloip’s picture

Yeah. let's do it! I thought we might as well check that it is using the Fields row plugin instead.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Thank you!

catch’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/modules/views/lib/Drupal/views/Tests/Plugin/StyleTest.phpundefined
@@ -50,45 +51,47 @@ protected function setUp() {
-    $this->assertTrue(strpos($output, $random_text) !== FALSE, 'Take sure that the rendering of the style plugin appears in the output of the view.');

I know it's already there, but two cases of 'Take sure' in lines that are changed.

damiankloip’s picture

Status: Needs work » Needs review
FileSize
1.3 KB
6.98 KB

Fair point, changed those lines.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

I'm sorry for all the "take" sure all over the place.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/modules/views/tests/modules/views_test_data/lib/Drupal/views_test_data/Plugin/views/style/StyleTest.phpundefined
@@ -34,6 +34,13 @@ class StyleTest extends StylePluginBase {
+   * Does the style plugin allows to use style plugins.

This should be something like... Can the style plugin use row plugins.

damiankloip’s picture

Status: Needs work » Needs review
FileSize
721 bytes
6.98 KB

Yep, sure. Fixed.

damiankloip’s picture

FileSize
711 bytes
6.97 KB

Errhhhmm, Let's try that again.

damiankloip’s picture

Status: Needs review » Reviewed & tested by the community

And back again.

dawehner’s picture

+1

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 62ed405 and pushed to 8.x. Thanks!

effulgentsia’s picture

Status: Fixed » Needs work
diff --git a/core/git b/core/git
new file mode 100644

What's this file for?

damiankloip’s picture

Good question!

effulgentsia’s picture

Status: Needs work » Needs review
FileSize
81 bytes

In case it's not there for a reason, here's the patch to remove it.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Would be cool to understand what added the file in the first place.

damiankloip’s picture

Its a mistake, beyond doubt.

neclimdul’s picture

Looks like it got added in #8. assume it was just an accident on the command line. things happen and its hard to see in the patch. RTBC for sure. Maybe the first 0 line patch? ;)

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Whoops should have caught that on review... my bad.

Committed f36f480 and pushed to 8.x. Thanks!

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

Anonymous’s picture

Issue summary: View changes

Updated issue summary.