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

Replace uploaded file with stored one on save entity. #102

Merged
merged 1 commit into from
May 19, 2013

Conversation

makasim
Copy link
Contributor

@makasim makasim commented Jan 18, 2013

It fixes #34

Without this change it is not possible to use exercise/elastica-bundle with mapper-attachment plugin. The uploaded file is removed on flush but the elastica bundle tries to index the file content at same time. Is there a change to get this merged?

@ASKozienko
Copy link

+1

@Baachi
Copy link
Contributor

Baachi commented Jan 18, 2013

Nice PR.
But i'm not sure about the implementation, because we planned to create a option to disable the doctrine lifecycle callbacks.

Maybe its better to move your code inside the Storage class.

@makasim
Copy link
Contributor Author

makasim commented Jan 18, 2013

But i'm not sure about the implementation, because we planned to create a option to disable the doctrine lifecycle callbacks.

I am curios what is the purpose of it?

Maybe its better to move your code inside the Storage class.

-1. As I got Storage and Injector were designed to be separate services. Putting injector to storage would be kind a mess.

@Baachi
Copy link
Contributor

Baachi commented Jan 18, 2013

For explanation see #8 (comment)

Thats right, maybe not the best implementation, other ideas?

@makasim
Copy link
Contributor Author

makasim commented Jan 18, 2013

Things described in #8 is not in the scope of this PR. It should definitely be another PR. I would stay with this implementation for now.

@makasim
Copy link
Contributor Author

makasim commented Mar 8, 2013

@Baachi Are you gonna merge it? What else should be done here?

@Baachi
Copy link
Contributor

Baachi commented Mar 8, 2013

@makasim No, i haven't the required privileges.

@ftassi ftassi merged commit d1e495f into dustin10:master May 19, 2013
@makasim
Copy link
Contributor Author

makasim commented May 27, 2013

@ftassi closed? is this bug is fixed in master?

@ftassi
Copy link
Collaborator

ftassi commented May 27, 2013

@makasim Should be fixed in master, try using it in your applications and let me know if you have any issues.

@makasim
Copy link
Contributor Author

makasim commented May 27, 2013

thanks

@makasim makasim deleted the update-uploaded-file-on-save branch August 15, 2013 12:36
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.

After flush file is moved but entity still contains UploadedFile
4 participants