Remove views optional handler handling

To be honest I can't really imagine that renaming plugin IDs is a good idea in any place, the same is true for changing default values and so on and so forth.

Another option is, allowing modules to provide it's own plugins for a view. For above case, we can have content_tranlsation.plugin.view.content.yml which provides the plugins (i.e. fields/filters/sort) to content view. So when content_tranlsation module get enabled, the plugins will be automatically added.

The file format is [module name].plugin.view.[view name].yml. So after getting the configuration from [module mame].view.[view name].yml, we can call all the plugins (may need to figure out the order) and update the configuration on both module install & uninstall.

Owning other module's component is always risk especially after live cases.

We discussed this issue with @catch, @alexpott, @effulgentsia, @tim.plunkett, @mtift, @moshe weitzman, and myself at DrupalCon Austin (for reference: issue discussion notes and action items). We added the optional handlers to support a very specific usecase (progressive enhancement of the content admin view when someone enables the content translation module). However, this feature adds a lot of complexity that we aren't really prepared to support. So, in the interest of resolving the issue in the simplest way given where we are at in the release cycle, we agreed to:

• Remove the optional handler functionality.
• Provide a separate, disabled default view with the content translation module, with the same path (admin/content) and support for translation.
• Decide how to respond when c_t is enabled. We probably don't want to magically replace the existing view as the user might have customized it (or we could do so only if it matches the originally installed view). A simpler solution would be for c_t to simply notify the user that the other view is available with a dsm(), an altered message on admin/content, or an admin/reports message.

The issue is still critical, because the consequences of not resolving it before release are that a site can be broken simply by enabling a module that does something we onstensibly support.

Oh and I think this can be active now as we're no longer blocked on anything.

#2301045: Standard profile has views which include elements dependent on uninstalled modules, not valid in config now landed, so there is no actual optional handler elements anymore in the standard profile at least that we know. The feature itself should be removed as well.

Let's try.

This looks good. I remember seeing a message for those. This patch does not remove them. Are those still needed for the broken handler display?

There we go.

+++ b/core/modules/views/src/Plugin/views/BrokenHandlerTrait.php
--- a/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_view_optional.yml
+++ b/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_view_optional.yml

What is the remaining point of this file. By its name, it sounds like it would be for testing optional views elements? Which are not to be tested anymore?

There is indeed none. This file was still loaded on the UI test, so removed that in a hunk as well.

Yay, that looks great to me.

Yay, thanks!

We do all realise that this doesn't really fix anything except an optional message in the UI, and supressing a debug() message when a plugin is missing/broken?

@damiankloip: well the pre-patch API said "hey, come on in, we support optional elements" while the new one says "error, unknown". Quite a difference, no?

I guess anything that helps. Are we ok with not showing broken handlers anywhere apart from if you are on the edit form for a particular view?

+++ b/core/modules/views/src/Entity/View.php
@@ -268,7 +268,7 @@ public function calculateDependencies() {
-            if (isset($handler['provider']) && empty($handler['optional'])) {

This is the main change from the patch tbh.

@damiankloip did the patch remove display of the broken handler info in any way? was it displayed elsewhere outside of the edit form before this patch?

Well, kind of. Before we had this:

+++ b/core/modules/views/src/Plugin/ViewsHandlerManager.php
@@ -119,12 +115,8 @@ public function getHandler($item, $override = NULL) {
-    if (!$optional) {
-      // debug(t("Missing handler: @table @field @type", array('@table' => $table, '@field' => $field, '@type' => $this->handlerType)));
-    }

Which got commented out (hurting tests with all the messages iirc) "temporarily", then wrapped for the optional functionality. Now removed here.

All I am saying is now you will definitely not be getting info that your view is broken unless you are looking at each handler on the view edit.

Well, it was already dead code without a todo to fix it in any way? I mean we could add the commented line back but it would not change the behaviour a single bit :D

#2183983: Find hidden configuration schema issues is experimenting with throwing exceptions for undefined types wholesale when a config key has schema otherwise. Not sure what our policy is / should be though regarding requiring those or not.