In #3523841: Versioned Component config entities (SDC, JS: prop_field_definitions, block: default_setting, all: slots for fallback) + component instances refer to versions ⇒ less data to store per XB field row we added support for collapsed inputs where the input matched the default static prop expression.
We didn't prohibit the use of uncollapsed inputs in content exports, test data etc because that would have added a lot more changes to the merge request.
We've seen the loose support cause issues on sites with exported values.
Don't allow uncollapsed inputs when using the default expression/static prop source
Show commandsStart 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:
changes
Comments
Comment #2
larowlanComment #3
wim leersThanks for opening this issue! @mglaman and I discovered this while debugging why content exported using https://drupal.org/project/default_content but then updated manually over time didn't behave as expected. This turns out to have been the reason.
Comment #5
wim leersThe sibling of this issue is simpler: #3538503: Disallow component trees with `component_version: active` — already fixed 👍
That sibling issue does not require any changes because it would've taken
component_version: activeas the input and then "upcasted" it to the actual active component version. IOW: it didn't make it into any component tree stored in the DB (content component trees for XB fields) nor in config (config component trees in XB config entities).This one, however, does require existing component trees to be updated. Which means this will require the very first update path that spans both content and config entities 🤓
Given that that requires expertise with writing update paths, and @larowlan just worked on the very first config update path in #3533458: Change CodeComponentDataProvider::getRequiredXbDataLibraries() to base its logic on information provided by the front-end rather than on naive string/regex matching, assigning to him, and bumping to .
Comment #6
larowlanI'm curious as to why this requires a content update path.
\Drupal\experience_builder\Plugin\Field\FieldType\ComponentTreeItem::preSavecalls\Drupal\experience_builder\Plugin\Field\FieldType\ComponentTreeItem::optimizeInputswhich calls\Drupal\experience_builder\ComponentSource\ComponentSourceInterface::optimizeExplicitInputwhich calls\Drupal\experience_builder\Plugin\ExperienceBuilder\ComponentSource\GeneratedFieldExplicitInputUxComponentSourceBase::collapseThat code has existed since #3529622: Make auto-save manager only support passing and returning entity objects, remove support for arbitrary data, use deterministic hashing to ensure entries are actually updates which was before a beta release.
I don't think we need to provide an update path for any sites older than that because there's been other API breaking changes since then.
I agree we do need to support an update path for component trees stored in config (pattern, content template, default values).
I'm not sure there's any way we can provide an update path for default content. Perhaps we can detect that during an event in the default content import stages and just trigger a deprecation.
Comment #8
larowlanMade some progress on this today
Update path is done for config, see #6 for why I don't think we need this for content.
Remaining phpunit failures are tricky
1) The magic 'uuid is cast to a target id' on default content import with media references
2)
\Drupal\Tests\experience_builder\Kernel\Plugin\ExperienceBuilder\ComponentSource\ComponentInputsEvolutionTest::testStorablePropShapeChangeswhich is showing that parsing from client model for the non-active version isn't collapsing properly. Will need to dig into that.Comment #9
larowlanThis is critical because it unearthed a possible data-loss/corruption bug in ClientServerConversionTrait
Comment #10
larowlanThis is green now
Comment #11
larowlanExpanded the change record
Comment #12
penyaskitoLGTM, added comments to MR.
Comment #13
penyaskitoI guess we want to revert 9ee987bf and rebase with HEAD for phpstan.
Comment #14
wim leers#6++
#9++
This again touching the update path test DB fixtures increases the importance of #3538537: Update path tests should also run against non-SQLite DBs.
LGTM — just a few questions and minor remarks! 😊
Comment #15
larowlanAddressed the minor remarks/questions
Comment #17
wim leersThanks, and turns out my sole "real" question was moot, I misread! 🙈
Comment #18
wim leersThis unblocked #3538537: Update path tests should also run against non-SQLite DBs — MR rolled there, and landed! 🥳