Schema missing when testing on collect_demo which depends on several modules

+++ b/src/Plugin/search_api/datasource/CollectDatasource.php
@@ -161,4 +161,12 @@ class CollectDatasource extends DatasourcePluginBase {
+  /**
+   * {@inheritdoc}
+   */
+  public function calculateDependencies() {
+    return parent::calculateDependencies();
+  }

This doesn't do anything yet. It only works because we added the enforced dependency on search_api_index.collect_demo, but that shouldn't be necessary.

So you need to figure the enabled models and add them as dependency. See the implementation in ContentEntity data source as an example. When you've made sure taht works then you can remove the enforced part again.

+++ b/demo/config/optional/field.field.node.article.comment.yml
@@ -0,0 +1,33 @@
+    - field.storage.node.comment

+++ b/demo/config/optional/field.field.node.article.field_image.yml
@@ -0,0 +1,32 @@
+    - field.storage.node.field_image

+++ b/demo/config/optional/field.field.node.article.field_tags.yml
@@ -0,0 +1,26 @@
+    - field.storage.node.field_tags

I think we should have these config files in demo/config/install dir.

--- /dev/null
+++ b/demo/config/optional/editor.editor.full_html.yml

Wondering if we can replace this with filter.format.full_html as we don't have dependencies on ckeditor and editor.

Looks fine to me.

Still a bit confused by this issue title and summary. There's not a single config schema change in this patch just dependencies and configuration.

Do we really need all those configs and fields for the demo? (comment field, image field and so on)

I was even unable to understand the problem and the proposal.

Sending back to work, possibly the issue needs to be discussed and a proper goal (issue, title, summary) needs to be defined.

Do we really need all those configs and fields for the demo? (comment field, image field and so on)

In collect.model.article we have all these fields, so if we want to recreate an entity I guess we need them.

Did you forget to provide the patch?

No, the patch is still the same.

In collect.model.article we have all these fields, so if we want to recreate an entity I guess we need them.

Right.. In most other demo projects, we moved away from re-using the article node type and instead use our own. I'd recommend to do that here as well. The main advantage is that you can make it required configuration, so you will get an error if something is messing. Just rename article in all file names and in them to "collect_demo" or so, node type name could be "Collectable content" or something like that.

Yes, something like that. But let's move them to config/install now, so that there is an error if they can't be installed.

Also, see https://www.drupal.org/documentation/git/configure on how to set up git so that renaming files show up as rename which is a lot easier to review.

+++ b/config/optional/field.storage.node.comment.yml
+++ b/config/optional/field.storage.node.field_image.yml
+++ b/config/optional/field.storage.node.field_tags.yml

Me still confused: Why are those not in demo?

What do you mean "not in demo"? If you mean why those files are not added in config/optional, then I guess we forgot to add them because we didn't have any demo tests. :)

I would expect that the fields above should be in the demo module shipped optional config. Not in the collect shipped optional config.

Yeah, that's true. They should be in demo/config/optional.

Took a look at this... @Berdir pointed out that

• field.storage.node.comment
• field.storage.node.field_image
• field.storage.node.field_tags

should be in demo/config/optional (because there could be the same storage files provided by core).

Consequently:

• field.field.node.collect_demo.comment
• field.field.node.collect_demo.field_image
• field.field.node.collect_demo.field_tags
• core.entity_form_display.node.collect_demo.default
• core.entity_view_display.node.collect_demo.default
• core.entity_view_display.node.collect_demo.rss
• core.entity_view_display.node.collect_demo.teaser

should be also in demo/config/optional because they depend on something optional.

The other stuff can be in demo/config/install (including views.view.collect_demo) which required schema (search API query) fix.

Edit: Updated CollectDemoTest in #29 #2519966: Demo article is not displayed on the front page

Here is the patch without CollectDemoTest included.

+++ b/demo/config/install/collect.model.collect_demo.yml
@@ -8,9 +8,9 @@ dependencies:
+id: collect_demo
+label: collect_demo

label collect_demo => Collect Demo
Fixed on commit.

Note that the current module doesn't allow to reinstall collect_demo at all after an uninstall:

exception 'Drupal\Core\Config\PreExistingConfigException' with message 'Configuration objects                            [error]
(collect.model.collect_demo, collect.model.user, collect.model.web_resource, field.field.node.collect_demo.body,
node.type.collect_demo, search_api.index.collect_demo, search_api.server.collect_demo, views.view.collect_demo) provided
by collect_demo already exist in active configuration'

The conflicting config on reinstall listed above should go to config optional.
Let's handle this as an update of this issue.

The problem with optional config is that it is a lot harder to debug when dependencies are missing.

The question, is what do you expect when you uninstall the demo module? that those indexes, node types and so on are kept or not? If not, then the answer is a forced dependency, not optional config.