Remove ViewExecutable::getMenuLinks(), its just wrong

Something like this.

Talking to Daniel, moving this to a new interface if probably the best idea.

Added DisplayMenuInterface. No interdiff...because.

Fatal error: Call to undefined method Drupal\views\Plugin\views\display\Block::getMenuLinks() in /var/lib/drupaltestbot/sites/default/files/checkout/core/modules/views/src/Plugin/views/display/DisplayPluginBase.php on line 2421
FATAL Drupal\block\Tests\Views\DisplayBlockTest: test runner returned a non-zero error code (255)

Fatal error: Call to undefined method Drupal\views\Plugin\views\display\DefaultDisplay::getMenuLinks() in /var/lib/drupaltestbot/sites/default/files/checkout/core/modules/views/src/Plugin/views/display/DisplayPluginBase.php on line 2421

Fatal error: Call to undefined method Drupal\views\Plugin\views\display\DefaultDisplay::getMenuLinks() in /var/lib/drupaltestbot/sites/default/files/checkout/core/modules/views/src/Plugin/views/display/DisplayPluginBase.php on line 2421
<h1>Uncaught exception thrown in shutdown function.</h1><p><em class="placeholder">Drupal\Core\Database\DatabaseExceptionWrapper</em>: SQLSTATE[42S02]: Base table or view not found: 1146 Table &#039;drupaltestbotmysql.simpletest993696search_total&#039; doesn&#039;t exist: SELECT t.word AS realword, i.word FROM {search_total} t LEFT JOIN {search_index} i ON t.word = i.word WHERE i.word IS NULL; Array
(
)
 in <em class="placeholder">search_update_totals()</em> (line <em class="placeholder">224</em> of <em class="placeholder">/var/lib/drupaltestbot/sites/default/files/checkout/core/modules/search/search.module</em>).<pre>search_update_totals()
call_user_func_array('search_update_totals', Array)
_drupal_shutdown_function()

?

gah

Most welcome :)

2498785-13.patch no longer applies.

error: patch failed: core/modules/views/src/Plugin/Derivative/ViewsMenuLink.php:8
error: core/modules/views/src/Plugin/Derivative/ViewsMenuLink.php: patch does not apply

Rerolled.

We need to init display now that #2497017: Views::getApplicableViews() initializes displays during route rebuilding etc. is in.

Thanks, this looks like a sensible fix to me overall. Thanks also for the beta evaluation.

+++ b/core/modules/views/src/Plugin/views/display/DisplayPluginBase.php
@@ -2612,6 +2593,13 @@ public function mergeDefaults() {
+   * {@inheritdoc}
+   */
+  public function remove() {
+
+  }

So I think we at least need an inline comment explaining why the base implementation is empty. However, it also makes me wonder if this method should also be on a separate interface if all display plugins don't actually need to implement it?

Also, looking into this further, I noticed that
\Drupal\views\Plugin\views\display\Block::remove() is still calling parent::remove(), which is now a null-op.

I thought about whether this should include a change record for the BC break. Given the assessment in the beta evaluation, maybe not, but maybe we should add one since there is technically a BC break, it was a public method on ViewExecutable, and maybe someone somewhere has some views menu manipulating code we didn't think of? What do you think?

Also gah at the testbot failures; looks like this issue got stuck in the eddies of the RTBC queue for a month.

No, I think I agree this is not really interface material. We then need to always check for another interface and that seems worse DX and more error prone. As much as people have tried to shoehorn new interfaces into views plugins, they are still basically dependent on the base class for a plugin type, especially displays.

I would imagine close to zero people will be using that, but sure, we could add a notice anyway. Can't hurt.

Created a CR here: https://www.drupal.org/node/2540418

Committed 3c12e21 and pushed to 8.0.x. Thanks!

Thanks for adding the CR and the beta evaluation.