Skip to content

replace FileType with ImageType for images#65

Open
kochen wants to merge 2 commits into
Lakion:masterfrom
kochen:replace_FileType_with_ImageType
Open

replace FileType with ImageType for images#65
kochen wants to merge 2 commits into
Lakion:masterfrom
kochen:replace_FileType_with_ImageType

Conversation

@kochen

@kochen kochen commented Oct 17, 2017

Copy link
Copy Markdown

fixes #31
replaces #63

@michalmarcinkowski

Copy link
Copy Markdown
Member

Thanks @kochen! Could you have a look at the failing build?

@kochen

kochen commented Oct 17, 2017

Copy link
Copy Markdown
Author

Could not find a defined element with name "block". The defined ones are: cmf-block.

@michalmarcinkowski could it be that:

$this->getElement('block')

needs to actually be:

$this->getElement('cmf-block')

and it has nothing to do with my fix?

@kochen

kochen commented Oct 17, 2017

Copy link
Copy Markdown
Author

@michalmarcinkowski the fix I proposed, solves issue #31, allowing existing blocks which already contain image to be edited and image to be added/replaced.

But, when a new (i.e custom) block is created with a new image, there is an error:
The node '/cms/media' already has a child named 'image''.
which is eventually what causes the test to fail.

Any idea how to solve this one?

@michalmarcinkowski

Copy link
Copy Markdown
Member

How is it possible that the tests pass on master branch, but not on yours? Have you tried to debug which of your change cased the error?

@kochen

kochen commented Oct 18, 2017

Copy link
Copy Markdown
Author

@michalmarcinkowski because I replaced FileType with ImageType for images and it seem to behave differently.

@kochen

kochen commented Oct 30, 2017

Copy link
Copy Markdown
Author

@michalmarcinkowski simply because there are no test for "editing a block with image" :/

@michalmarcinkowski

michalmarcinkowski commented Oct 31, 2017

Copy link
Copy Markdown
Member

That's bad... @kochen are you able to fix the issue?

@kochen

kochen commented Oct 31, 2017

Copy link
Copy Markdown
Author

@michalmarcinkowski not from the little time I invested in it.
The problem seems to be much deeper than this plugin and CmfMediaBundle is abandoned anyway so you should consider some alternatives for the future anyway...

@michalmarcinkowski

Copy link
Copy Markdown
Member

Thanks for the feedback, if you will have any more information that could help, please share it with us 😉

@kochen

kochen commented Oct 31, 2017

Copy link
Copy Markdown
Author

@michalmarcinkowski first thing I'm trying to do is write some tests which will show the problem.
Any help you could provide here (specially on the factory/image)?

@kochen

kochen commented Oct 31, 2017

Copy link
Copy Markdown
Author

@michalmarcinkowski the problem is somewhere in the actual generation of the image:
image

instead of belonging to the actual content-block parent, it is independent and therefor a second block with an image will try to save another /cms/media/image which already exists...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The form's view data is expected File

2 participants