Do not depend on Core Search for the search index and search display view mode

Taking this now - found the problem and testing the solution now.

The solution is to add the configs to optional and only if you do not have the article and story type it will "silently" not install those configs. Core has support for this behavior and it is not a new pattern.

Adding a hook_requirements so that it would fail to install if the article and bundle are not available. Next up, adding the requirements for the specific fields that we need.

This should be good for review.

Adding newline and changing double quotes to single quotes

Sorry, a little follow up patch is required. And it does confuse me, the "links" parameter is shown in the export as a dependency

links:
    weight: 2
    settings: {  }
    third_party_settings: {  }

However, it is not present when looping over the fields. I'm going to say it is not required as everything installs just fine without it. The other change was just a mistake that I don't know how that can pass the test suite. I also installed it with the standard profile but the page content type didn't have field_image, ever.

Please review and commit this follow-up

So, this was more difficult than I thought. This adds a test for installing and uninstalling through the UI and make sure that it also removed the server and index. The test also removes the article content type and checks if the module fails to install. Since the configs are just exports, they are not being removed on uninstall. The reason is that they don't have a single dependency on config entities that makes them obsolete, and hence they are not marked for deletion. This is kind of strange as I even tried to make them a hard dependency on the module.

I had to move the view mode displays to optional and the view itself as well. Since we do check for the content types as the only hook_requirement, there is no need to add them to the install. It also conflicts if those view modes were already set up and we do not remove them on uninstall so they are fully optional. Since the view depends on those view mode displays, I had to move it to optional because the way the dependency system works. Eg, if you move that to "install" it fails to install because the stuff it depends on are listed in optional. Regardless of whether it adds it to your site or not, it will fail to install. Only the server and index are added to the install folder and those are also removed when uninstalling the module.

This brings me to a conflict, should we "uninstall" the configured view modes when uninstalling this module? Should we figure out if there are any other systems that use those view mode displays and if so, do not remove them?

Thanks

Updated patch to be a bit more clear in what it does. I tried to calculate the dependent other configs but failed miserably.

Do we need to check if these exist? They can't be uninstalled seperatly, can they? Not that it hurts, just wanted to know.

They can, they are all available in the UI.

Rerolled it to incorporate your comments

Doh, wrong way of getting the patch

addressed those :)

drunken_monkey: Nick_vh: Huh. I actually would have said the server and index not being uninstalled with the module is a good thing.
[4:16pm] Nick_vh: then it should also move to optional
[4:16pm] drunken_monkey: That way, you can just uninstall the module again after installing it and it won't decrease performance completely unnecessarily.
[4:16pm] drunken_monkey: Also, I think it
[4:16pm] oraculi joined the chat room.
[4:16pm] drunken_monkey: 's more in line with the config philosophy of D8.
[4:16pm] drunken_monkey: Nick_vh: Why?
[4:17pm] Nick_vh: So installing a module adds stuff, uninstalling doesn’t remove anything?
[4:17pm] Nick_vh: doesn’t make a lot of sense
[4:18pm] Nick_vh: and then when you install it again, the server is in “install” and the module will fail to get enabled
[4:18pm] Nick_vh: so it has to be optional
[4:18pm] Nick_vh: imho
[4:19pm] drunken_monkey: Nick_vh: As said, I think it's in line with D8's config philosophy: the site owns the configuration, not the module.
[4:19pm] drunken_monkey: Maybe ask someone about it?
[4:20pm] drunken_monkey: If they wanted installed config to be removed along with the module, I think they'd coded it that way.
[4:20pm] Nick_vh: Well, if it depends on config entities, that’s how it works
[4:20pm] Nick_vh: but should the module fail to install after a second try then?
[4:20pm] drunken_monkey: And, as said, leaving the module enabled completely unnecessarily reduces performance.
[4:20pm] Nick_vh: I think that’s why it’s called optional
[4:20pm] Nick_vh: I’m fine in not removing it
[4:20pm] drunken_monkey: Hm, might be we should move it to optional, yes.
[4:21pm] drunken_monkey: Or add a check to hook_requirements() to give a more meaningful error message.
[4:21pm] drunken_monkey: (Unless the one you'd get currently is good enough already.)
[4:24pm] Nick_vh: well, that one is pretty clear currently
[4:24pm] Nick_vh: it checks all the required components
[4:25pm] Nick_vh: but it shouldn’t fail when re-enabling then
[4:25pm] Nick_vh: and I agree in not removing it, maybe we do need to add it to the description
[4:26pm] drunken_monkey: Like, "Can be uninstalled again right afterwards."? Sounds sensible to me, yes.
[4:26pm] dasjo left the chat room. (Remote host closed the connection)
[4:27pm] drunken_monkey: Failing on re-installing makes sense, too, I'd say, since that wouldn't do anything anyways. We should just have a good error message. "It looks like the default setup provided by this module already exists on your site. Installation failed." or something.
[4:27pm] dasjo joined the chat room.
[4:31pm] Nick_vh: the thing is that Core provides the error message if we have it in “install"
[4:31pm] Nick_vh: so sure, we can check if the resources are there in hook_requirements and then fail
[4:31pm] Nick_vh: but with a clear message
[4:31pm] Nick_vh: that would make sense to me
[4:31pm] drunken_monkey: OK, great!
[4:31pm] Nick_vh: I’ll c/p this conversation to the patch
[4:31pm] drunken_monkey: Then let's do it like that.

So I think this patch complies with the discussion we had. You are now able to install the module and get all the configs, even if you already enabled the view display modes. On uninstall it will not remove the configs but when you try to install them again it will fail if you still have the index, server or view.

All of the above is also tested in an integration test. I have one exception which seems very unrelated so trying to test that here also.

removal of a container rebuild line.

This patch has to be committed, just for the fact it is 13.37kb!