Add ViewExecutable::getStylePlugin and ViewExecutable::getRowPlugin as public methods

Working on this at the DrupalConNOLA Sprint

First try

@zaurav

Next time Upload the patch , Please update the status to review 'Needs review'.

@zaurav, thank for working on this!

+++ b/core/modules/views/src/ViewExecutable.php
@@ -214,15 +214,22 @@ class ViewExecutable implements \Serializable {
+  private $style_plugin;
...
+  private $rowPlugin;

These should be protected so that they can be overridden by child classes. Other than that, it looks great.

+++ b/core/modules/views/src/ViewExecutable.php
@@ -214,15 +214,22 @@ class ViewExecutable implements \Serializable {
-  public $style_plugin;
+  private $style_plugin;
...
-  public $rowPlugin;
+  private $rowPlugin;

Heh, completely spaced on this... These need to remain public for now, but marked as @deprecated (as you have done). Since we shipped 8.0 with these as public methods, it's an API change to change that.

@mikeker, thank you for helping me.

2nd try.

+++ b/core/modules/views/src/ViewExecutable.php
@@ -221,6 +224,10 @@ class ViewExecutable implements \Serializable {
+   * ¶

Looks like a trailing whitespace got in the last change...

+++ b/core/modules/views/src/ViewExecutable.php
@@ -214,6 +214,9 @@ class ViewExecutable implements \Serializable {
+   * @deprecated in Drupal 8.0.x and will be removed before 9.0.0. Use

@@ -221,6 +224,10 @@ class ViewExecutable implements \Serializable {
+   * @deprecated in Drupal 8.0.x and will be removed before 9.0.0. Use

Sorry, missed this the first time. Technically, we're deprecating this in 8.2.x.

Implemented changes from #9 #10 #11.

Unsure what to return in the case of $rowPlugin empty.

@zaurav, Don't forget to set the status to Needs Review so the testbot tests the new patch.

Tagging for NOLA sprints. I mentored @zaurav on this issue.

1. +++ b/core/modules/views/src/ViewExecutable.php
   @@ -214,6 +214,9 @@ class ViewExecutable implements \Serializable {
   +   * @deprecated in Drupal 8.0.x and will be removed before 8.2.x. Use
   
   @@ -221,6 +224,10 @@ class ViewExecutable implements \Serializable {
   +   * @deprecated in Drupal 8.0.x and will be removed before 8.2.x. Use
   
   @@ -846,6 +853,8 @@ public function newDisplay($plugin_id = 'page', $title = NULL, $id = NULL) {
   +   * @deprecated in Drupal 8.0.x and will be removed before 8.2.x.
   

   I'm pretty sure we can't deprecate for 8.2.x, and need to target 9.0.0, particularly for a getter method.

   Also I doubt we'd really be *removing* these properties. Maybe say we'll be removing access to them.

2. +++ b/core/modules/views/src/ViewExecutable.php
   @@ -221,6 +224,10 @@ class ViewExecutable implements \Serializable {
       * The current used row plugin, if the style plugin supports row plugins.
   
   @@ -856,6 +865,35 @@ public function getStyle() {
   +  /**
   +   * Gets the current row plugin.
   

   The accessor doc line should match the property doc line, with 'Gets...' at the beginning.

3. +++ b/core/modules/views/src/ViewExecutable.php
   @@ -856,6 +865,35 @@ public function getStyle() {
   +   * @return \Drupal\views\Plugin\views\row\RowPluginBase
   +   *   The current row plugin.
   +   */
   +  public function getRowPlugin() {
   +    if (!isset($this->rowPlugin)) {
   +      return;
   +    }
   

   Since we can return NULL, then @return should be \Drupal\views\Plugin\views\row\RowPluginBase|null and explain when the method will return NULL.

Hi Paul, I have worked on your suggestions and updated the patch.

Hi Daniel, I have tried to update the patch with your suggestions. Please correct me if I am wrong.

@Vinay15, thank you for your work on this issue!

1. +++ b/core/modules/views/src/ViewExecutable.php
   @@ -214,6 +214,9 @@ class ViewExecutable implements \Serializable {
   +   * @deprecated as of Drupal 8.0.x and access will be removed in Drupal 8.2.x.
   +   *   Use getStylePlugin() instead to get this property.
   
   @@ -221,6 +224,9 @@ class ViewExecutable implements \Serializable {
   +   * @deprecated as of Drupal 8.0.x and access will be removed in Drupal 8.2.x.
   +   *   Use getRowPlugin() instead to get this property.
   

   Went the wrong way on this one. :) When @dawehner said it should be 8.2.x, he meant it should read "@deprecated as of Drupal 8.2.x and access will be removed in Drupal 9.0.0."

   We can't actually change the access during the 8.x cycle as that would break backwards compatibility. So we mark as deprecated (as of 8.2.x) and change access to protected in the 9.x branch.

2. +++ b/core/modules/views/src/ViewExecutable.php
   @@ -846,8 +852,20 @@ public function newDisplay($plugin_id = 'page', $title = NULL, $id = NULL) {
   +   * @deprecated as of Drupal 8.0.x and access will be removed in Drupal 8.2.x.
   

   Similar to the above, but should read "@deprecated as of Drupal 8.2.x and will be removed in Drupal 9.0.0."

   In this case the old method will be removed -- in the previous cases the property will remain but will go from public to protected.

Sorry for the nitpicks, but getting the version numbers correct is important when deprecating things.

Hey Mike, thanks a lot for correcting me.. :) I have updated the patch.

Re-rolling against 8.4.x.

1. +++ b/core/modules/views/src/ViewExecutable.php
   @@ -213,6 +213,9 @@ class ViewExecutable implements \Serializable {
   +   * @deprecated as of Drupal 8.2.x and access will be removed in Drupal 9.0.0.
   

   The version numbers need updating again, sadly.

2. +++ b/core/modules/views/src/ViewExecutable.php
   @@ -845,8 +851,20 @@ public function newDisplay($plugin_id = 'page', $title = NULL, $id = NULL) {
   +   * @deprecated as of Drupal 8.2.x and will be removed in Drupal 9.0.0.
   

   Docs should say what to use instead. I know it's in the source 2 lines lower, but on api.d.org, you only see the source if you click the 'show source' button.

Working on this at DistributedSprintUK18

Latest patch rerolled with 8.7x added

#28.2 still needs fixing.

Latest patch looks good, but I don't think it'll make it into 8.7.x, so the comments will need changing to say 8.8.x.

If the patch deprecate the variable $rowPlugin why the new code use it?

 /**
   * Gets the current row plugin.
   *
   * @return \Drupal\views\Plugin\views\row\RowPluginBase
   *   The current row plugin.
   */
  public function getRowPlugin() {
    if (!isset($this->rowPlugin)) {
      $this->rowPlugin = $this->display_handler->getPlugin('row');
    }

    return $this->rowPlugin;
  }

Because it's going to remain as an internal property. The point of this issue is to add accessors for these properties so that in D9 they can be changed to protected. The class itself can and should still refer to it.

Update per #37 to update references from 8.7.x to 8.8.x

Created a new patch and resolved Custom Commands Failed for 9.3.x in #41

Formatting fixes and fix issue link in doc.

Added one missing file from last patch(#41)

Getting below phpunit issues after this patch
1x: The "PHPUnit\TextUI\ResultPrinter" class is considered internal This class is not covered by the backward compatibility promise for PHPUnit. It may change without further notice. You should not use it from "Drupal\Tests\Listeners\HtmlOutputPrinter".
1x in DrupalListener::endTest from Drupal\Tests\Listeners

1x: The "Drupal\Tests\Listeners\DrupalListener" class implements "PHPUnit\Framework\TestListener" that is deprecated Use the `TestHook` interfaces instead.
1x in DrupalListener::endTest from Drupal\Tests\Listeners

1x: The "Drupal\Tests\Listeners\DrupalListener" class uses "PHPUnit\Framework\TestListenerDefaultImplementation" that is deprecated The `TestListener` interface is deprecated.
1x in DrupalListener::endTest from Drupal\Tests\Listeners

Seems these are deprecation issues due to phpunit version. See https://www.drupal.org/project/drupal/issues/3125809

Interdiff of #52

This is not at a stage for novice level work. Also, note that the latest patch has out of scope changes.