Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUGFIX: Skip node data processing for invalid properties #4807

Merged
merged 3 commits into from
Dec 19, 2023

Conversation

markusguenther
Copy link
Member

@markusguenther markusguenther commented Dec 15, 2023

It can happen that node properties are not available in a new version of the package. But when the properties are still part of the neos_contentrepository_domain_model_nodedata table, the migration tries to process the data for a node and throws an exception as the node property is not available in the node type schema anymore.

To prevent that exceptions in the legacy data migration, we skip the procession for such properties and output a warning for the user.

Fixes: #4804

Review instructions

As it is described in the linked issue, the twitter related properties in the Neos-Seo package have been removed in the latest version for Neos 9. When you upgrade your project from Neos 8.3 to Neos 9.0 you will increase the package version as well, but the data is still part of your database. To be able to test it, you can install a Neos 8.3 instance and change a Twitter Card property in the backend and publish that. When you now upgrade the Neos to version 9.0 and execute ./flow cr:migrateLegacyData You should see something like that instead of an exception.
Screenshot 2023-12-15 at 18 04 02

Checklist

  • Code follows the PSR-2 coding style
  • Tests have been created, run and adjusted as needed
  • The PR is created against the lowest maintained branch
  • Reviewer - PR Title is brief but complete and starts with FEATURE|TASK|BUGFIX
  • Reviewer - The first section explains the change briefly for change-logs
  • Reviewer - Breaking Changes are marked with !!! and have upgrade-instructions

It can happen that node properties are not available in a new version of the package. But when the properties are still part of the neos_contentrepository_domain_model_nodedata table, the migration tries to process the data for a node and throws an exception as the node property is not available in the node type schema anymore.

To prevent that exceptions in the legacy data migration, we skip the procession for such properties and output a warning for the user.

Fixes: neos#4804
Copy link
Member

@bwaidelich bwaidelich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix makes sense to me in general, but I don't see how that should work.. A corresponding test would be nice

@bwaidelich
Copy link
Member

Looking good, I'll try to add some tests

Copy link
Member

@bwaidelich bwaidelich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking care, I just added some tests

@ahaeslich ahaeslich merged commit 0861612 into neos:9.0 Dec 19, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: Prevent migration of invalid node properties
3 participants