Update ImagineBlock to use CmfMedia image #72
Conversation
| } elseif ($this->image && $this->image->getFile()) { | ||
| // TODO: this is needed due to a bug in PHPCRODM (http://www.doctrine-project.org/jira/browse/PHPCR-98) | ||
| // TODO: this can be removed once the bug is fixed | ||
| $this->image->getFile()->setFileContent($image->getFile()->getFileContent()); |
There was a problem hiding this comment.
are you sure that bug is indeed fixed?
There was a problem hiding this comment.
not 100%. I checked Jira and read the ticket has fixed as status, so I trusted JIRA :-)
Don't know in what test-case it happened, I think we should check. At least I verified that it doesn't solve the current issue with persisting the stream of the Resource.
There was a problem hiding this comment.
the problem happened when you upload a new image to a place that had a
previous image. replacing the child image document failed.
There was a problem hiding this comment.
ok, also got that error so we need to re-check this once the other is solved. Logged it in the cmfmedia code with a TODO by the way.
There was a problem hiding this comment.
so do we re-add this workaround for now?
There was a problem hiding this comment.
I think we already fixed it somehow, I do not get errors when testing to replace an image.
|
what is the status here? |
|
The code is ready but needs to be rebase once the xml mappings are merged. Then the PR can be merged once we decide to merge all CmfMedia PR's. |
| } elseif ($this->image) { | ||
| // TODO: https://github.com/doctrine/phpcr-odm/pull/262 | ||
| $this->image->copyContentFromFile($image); | ||
| } elseif ($image instanceof Image) { |
There was a problem hiding this comment.
is this intentionally the class and not the interface? if so, please add a comment to the code explaining
There was a problem hiding this comment.
changed this to the interface
|
where are we here? |
|
The code and changelog is ready. A web test case for the image block is to be added. After that the merge can be done if we find the MediaBundle is ready. For the MediaBundle I would like to merge symfony-cmf/media-bundle#40 tomorrow. Upload and download tests are also covered then. After that we have to check the bundle standards of the MediaBundle. |
| $this->image = $image; | ||
| } else { | ||
| $this->image = new Image; |
There was a problem hiding this comment.
i think we have agreed on not omitting the ()
There was a problem hiding this comment.
ah ok, will do that in future
|
merged manually |
See symfony-cmf/cmf-sandbox#189 for related CmfMedia PR's.
TODO to merge
Doctrine\Phpcr\ImagineBlockImageInterfacefor checks instead of Image document?image.id