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: fix error during site import #5449

Merged
merged 1 commit into from
Jan 24, 2025

Conversation

mficzel
Copy link
Member

@mficzel mficzel commented Jan 22, 2025

This adjusts the annotation to work with Flow 8.3.13 ... before the type with spaces was interpreted as mixed and caused the following error during site:import.

During the import of the "Sites.xml" from the package "Neos.Demo" an exception occurred: Error: During import an exception occurred: "Could not convert target type "Neos\Media\Domain\Model\ImageVariant": Could not convert target type "Neos\Media\Domain\Model\Adjustment\CropImageAdjustment", at property path "aspectRatio": Could not find a suitable type converter for "mixed" because the class / interface "mixed" does not exist."., see log for further information.

Resolves: #5448

Review instructions

The new behavior was introduced to flow with pr: neos/flow-development-collection#3424

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

@mficzel
Copy link
Member Author

mficzel commented Jan 22, 2025

While this lets the import run trough it will probably still be better to also have a proper flow fix in place.

This adjusts the annotation to work with Flow 8.3.13  ... before the type with spaces was interpreted as `mixed` and caused the following error during site:import.

```
During the import of the "Sites.xml" from the package "Neos.Demo" an exception occurred: Error: During import an exception occurred: "Could not convert target type "Neos\Media\Domain\Model\ImageVariant": Could not convert target type "Neos\Media\Domain\Model\Adjustment\CropImageAdjustment", at property path "aspectRatio": Could not find a suitable type converter for "mixed" because the class / interface "mixed" does not exist."., see log for further information.
```

Resolves: neos#5448
@mficzel mficzel force-pushed the bugfix/fixErrorDuringSiteImport branch from 04628c6 to e35d4de Compare January 22, 2025 18:49
Copy link
Contributor

@dlubitz dlubitz left a comment

Choose a reason for hiding this comment

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

Thank you for digging into this.

@mficzel
Copy link
Member Author

mficzel commented Jan 22, 2025

We should discuss wether reverting the flow change is the better option.

Adjusting type hints in a minor is not the best solution I can think of.

@dlubitz
Copy link
Contributor

dlubitz commented Jan 22, 2025

Yes we can discuss this. But from my POV the change just fixes (accidentially) a bug that allowed to have wrong @param annotations.

Copy link
Member

@kdambekalns kdambekalns left a comment

Choose a reason for hiding this comment

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

Works like a charm!

@kdambekalns
Copy link
Member

For PHP both string|int and string | int are fine, so we should also accept that. But I see no reason in keeping this issue "unfixed" with this PR – it fixes the problem now, and gives us (more) time to fix the underlying problem in Flow.

@kdambekalns kdambekalns merged commit 30f4f8c into neos:8.3 Jan 24, 2025
10 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.

3 participants