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

Fix GridFSLoader #461

Merged
merged 2 commits into from
Sep 17, 2014
Merged

Fix GridFSLoader #461

merged 2 commits into from
Sep 17, 2014

Conversation

aldeck
Copy link
Contributor

@aldeck aldeck commented Jul 23, 2014

It seems that a previously suggested patch never made it into the repo (cf. issue #305)

aldeck and others added 2 commits June 6, 2014 01:05
It seems that a previously suggested patch never made it into the repo (cf. issue liip#305)
@peterrehm
Copy link
Contributor

Can you merge this one? It is the same as in #331 and is not working. It would be great if you could tag a new minor release as well.

@makasim
Copy link
Collaborator

makasim commented Sep 17, 2014

There were several commits that reverts previous work. Let's say one commit changes $image['file'] -> $image->getFile() where the next one changes it back. I am not sure why different people have such problems. Maybe because of the grid fs version they use.

I think both variants has to be supported.

@makasim makasim added Level: Bug 🐞 This item involves a legitimate regression (bug) to existing functionality. State: Rejected This item has been rejected as being invalid or otherwise not desired by maintainers. labels Sep 17, 2014
@peterrehm
Copy link
Contributor

What we could do is to check if $image['file'] exists and if not use the getter. I dont see why there should be access as array possible....

@makasim
Copy link
Collaborator

makasim commented Sep 17, 2014

According to the history it was fixed in 0.x. https://github.com/liip/LiipImagineBundle/commits/0.x/Imagine/Data/Loader/GridFSLoader.php

But 1.x somehow misses this change: https://github.com/liip/LiipImagineBundle/blob/master/Binary/Loader/GridFSLoader.php

I think we can merge this as is

makasim added a commit that referenced this pull request Sep 17, 2014
@makasim makasim merged commit 017bd45 into liip:master Sep 17, 2014
@makasim
Copy link
Collaborator

makasim commented Sep 17, 2014

@peterrehm
Copy link
Contributor

Awesome, thanks 👍

@aldeck
Copy link
Contributor Author

aldeck commented Sep 17, 2014

Thanks guys :-)
I guess the array access was working with a very early version of the doctrine/mongo odm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Level: Bug 🐞 This item involves a legitimate regression (bug) to existing functionality. State: Rejected This item has been rejected as being invalid or otherwise not desired by maintainers.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants