Problem/Motivation

The config schema is incomplete and the existing schema throws errors:

    "revision_manager.settings:enabled_entities.node": {
        "key": "revision_manager.settings:enabled_entities.node",
        "status": "missing schema",
        "validatability": null,
        "data": null
    },

Proposed resolution

Let's provide a complete schema which passes validation.

Issue fork revision_manager-3559895

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

pfrenssen created an issue. See original summary.

pfrenssen opened merge request !4

pfrenssen’s picture

pfrenssen
Location Sofia
commented

I updated the schema but it looks like the module is currently not storing the configuration values with correct types. Instead we are passing through whatever we get from the form state. For example the entity type enabled status should be a boolean but is stored as an integer 0 or 1. Also the values for plugin settings (age and amount) which are declared as integers are stored as strings.

We should make sure the types are correctly cast before storing them, and we probably need to add an update hook to correct the values in existing installations.

pfrenssen’s picture

pfrenssen
Location Sofia
commented

Our dynamic use of third party settings doesn't seem to be possible to define in a config schema. The current implementation tries to do it like this:

'*.type.*.third_party.revision_manager':
  type: mapping
  label: 'Per-bundle revision manager plugin settings'

But this is not correctly parsed by the config schema resolver. But using something like node.type.*.third_party.revision_manager works fine.

This ticket seems to have been discussion the problem years ago, even before third party settings existed: #2645046: How do you add schema for every config bundle entity type, but it was closed due to lack of movement.

For now, maybe we should just provide the schema for the core entity types that we support?

j-barnes’s picture

j-barnes commented

@pfrenssen - Thanks for digging into this! The original idea was to keep it dynamic so adding new entity type plugins wouldn't require schema changes, but you're right - it looks like the schema system doesn't support it. Explicit definitions are the way to go for this.

j-barnes’s picture

j-barnes commented
Status: Active » Needs review

@pfrenssen - Thanks again for the working on this! I've rebased against 1.0.x and added a few fixes:

  • Removed unused PluginFormInterface import
  • Fixed taxonomy schema key (taxonomy.vocabulary.* not taxonomy_term.type.*)
  • Added system.menu.* schema for menu_link_content
  • Improved post_update: enabled_entities casting, isset() for status, and bundle third-party settings migration

All tests pass locally. Could you give it a look?

sunny-lee’s picture

sunny-lee commented
Status: Needs review » Reviewed & tested by the community

This looks good! Tested drush update db and it imported correctly without errors. Checked the functionality of the rest of revision manager - checked age, checked min, checked age + min, checked translations saving multiple, checked forward revisions. Tried on several media types. All looks good here.

j-barnes’s picture

j-barnes commented
Status: Reviewed & tested by the community » Fixed

Thanks for the work on this!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.