Update Flag Schema for Flag Type and Link Type Settings

I still get fails with this patch.

Also, how do we get testbot to test this when the branch tests are failing?

Ask someone in #drupal-contribute to force a test run, needs adminstrative permissions there. (e.g. me :))

Yes, this is definitely not complete yet.

I think there are other problems as well as this that are causing the tests to fail: I tried setting protected $strictConfigSchema = FALSE; on all our test classes, as suggested in the CR, and the tests still fail:

Drupal\flag\Tests\FlagConfirmFormTest                         13 passes  15 fails   7 exceptions
Drupal\flag\Tests\FlagEnableDisableTest                       11 passes  20 fails   5 exceptions
Drupal\flag\Tests\FlagFieldEntryTest                          18 passes  20 fails  12 exceptions
Drupal\flag\Tests\FlagSimpleTest                              49 passes  72 fails  35 exceptions

Yes, I helped @socketwench with one pull request that was working on fixing the tests, and we had at least one test that green, I think that was not yet merged, but it didn't get inyet.

I am getting errors in all tests.

FlagConfirmFormTest shows this:

Error message
Drupal\Core\Config\Schema\SchemaIncompleteException: Schema errors for flag.flag.test_label_123 with the following errors: flag.flag.test_label_123:flagTypeConfig.show_in_links missing schema, flag.flag.test_label_123:flagTypeConfig.show_as_field missing schema, flag.flag.test_label_123:flagTypeConfig.show_on_form missing schema, flag.flag.test_label_123:flagTypeConfig.show_contextual_link missing schema, flag.flag.test_label_123:flagTypeConfig.i18n missing schema, flag.flag.test_label_123:flagTypeConfig.access_author missing schema, flag.flag.test_label_123:linkTypeConfig.flag_confirmation missing schema, flag.flag.test_label_123:linkTypeConfig.unflag_confirmation missing schema in Drupal\Core\Config\Testing\ConfigSchemaChecker->onConfigSave() (line 98 of core/lib/Drupal/Core/Config/Testing/ConfigSchemaChecker.php).

At first glance, it looks like the other errors in the other tests are variations on this. (Or at least, I think we can work on that assumption until we make this error go away!)

Do you have the URL for the github pull request? I've had a look through the list and nothing's jumping out.

It was https://github.com/socketwench/flag-drupal8/pull/120, together with some additional fixes from me in 124. That would be #2410005: [regression] add flag form first step shouldn't have the machine name and link type.

> flag.flag.test_label_123:flagTypeConfig.i18n missing schema, flag.flag.test_label_123:flagTypeConfig.access_author missing schema,

How do we handle these? They only apply to flags on nodes, so the NodeFlagType plugin.

The existing patch here starts with adding the dynamic pattern, now you need to provide the specific schema for each plugin type, e.g. flag.link.confirm (note that the current patch uses the same pattern for both plugins, that needs to change).

Yes, that is annoying for derivate-based plugins that are available for a dynamic list of entity types. I believe there is a alter hook for config schema now, so you could possibly use that?

> The existing patch here starts with adding the dynamic pattern, now you need to provide the specific schema for each plugin type, e.g. flag.link.confirm (note that the current patch uses the same pattern for both plugins, that needs to change).

Sorry, you've lost me there...
This schema is for the flag config entity, isn't it? Are you saying we need to expand flagTypeConfig and linkTypeConfig into sub-arrays here?

> Yes, that is annoying for derivate-based plugins that are available for a dynamic list of entity types. I believe there is a alter hook for config schema now, so you could possibly use that?

Yup, it's https://api.drupal.org/api/drupal/core!modules!system!system.api.php/fun...

It has lots of warnings on it not to use it :/

Yes, exactly, assuming that the type for link plugins is flag.link.[%parent.link_type], then you need something like this for the confirm form:

flag.link.confirm:
  type: mapping:
  mapping:
    flag_confirmation:
      type: string
    unflag_confirmation:
      type: string

And then repeat that for every plugin you have, if they have no configuration, you still need to define it as type mapping without any actual mappings.

Here's how far I've got, in case anyone wants to pick it up. I'll unassign the issue for now.

@socketwench: Config schema converts value automatically.

I would suggest to just disable schema checks and first fix anything that is *not* related to config schema and then get back to this. Probably in #2410005: [regression] add flag form first step shouldn't have the machine name and link type.

Will try to have a look at that issue, unless someone else plans to do that in the next few hours?

Turns out I was wrong on the generic thing. derivatives are supported, see #2317865: Config schema definitions for plugins aren't applied to their derivatives, so entity:* or so as plugin ID should work and apply to all derivatives.

Better patch at #2541936: Fix config schema.