Calling TypedConfigManager::create() does not use the definitions of TypedConfigManager (config schema), but of TypedDataManager (field types)

Created on 23 November 2023, 10 months ago
Updated 5 December 2023, 9 months ago

Problem/Motivation

Discovered in πŸ“Œ Configuration schema & required keys Fixed .

TypedConfigManager does not correctly override all of the methods of its base class, TypedDataManager.

The result is that sometimes it will get confused and NOT use the correct plugin definitions (*.schema.yml files' contents, surfaced by \Drupal\Core\Config\Schema\ConfigSchemaDiscovery), but instead those of the parent class (@DataType plugins).

Steps to reproduce

Proposed resolution

Add the test coverage that was overlooked in #1866610: Introduce Kwalify-inspired schema format for configuration β†’ .

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

πŸ› Bug report
Status

Needs work

Version

11.0 πŸ”₯

Component
ConfigurationΒ  β†’

Last updated 1 day ago

Created by

πŸ‡§πŸ‡ͺBelgium Wim Leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

Live updates comments and jobs are added and updated live.
  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @Wim Leers
  • Merge request !5521Add basic test coverage. β†’ (Open) created by Wim Leers
  • Status changed to Needs review 10 months ago
  • πŸ‡§πŸ‡ͺBelgium Wim Leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί
  • πŸ‡§πŸ‡ͺBelgium Wim Leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    I think we should add explicit test coverage for all methods on TypedConfigManager, because it's unclear which code paths work correctly and which ones fail in the way described in the issue summary and demonstrated in the basic test coverage already in the MR.

  • Status changed to Needs work 10 months ago
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    The test added seemed to work. Not sure if this issue is meant to include all test coverage or if that should be a follow up. Could that be noted in IS?

  • πŸ‡§πŸ‡ͺBelgium borisson_ Mechelen, πŸ‡§πŸ‡ͺ

    I think the plan is to include all test coverage in this issue, I think that would make the most sense?

  • πŸ‡§πŸ‡ͺBelgium Wim Leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    Bumping to given the data corruption/pollution nature of the problem.

Production build 0.71.5 2024