-
Notifications
You must be signed in to change notification settings - Fork 22
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
Initial image upload #79
base: master
Are you sure you want to change the base?
Conversation
Ready for review. Allows uploading of images to Imgur and they are displayed on the site: Moved config settings into config.json and included a better lib that supports the API. Future enhancements are assigning to album in Imgur so all images are uploaded to a single repo. What are your thoughts on this ? |
Thanks for another submission as always! What are the edge cases here? There's doesn't seem to be any image validation on our side so I assume Imgur does that? What errors can come back from imgur as the current code assumes success. I've been thinking about adding flash message for a little while, they might be useful here also. Do we need to do anything to limit the size at which they're shown on the frontend to ensure that display of the site isn't broken or performance isn't negatively effected? |
Evening Buddy, Validation is handled via Imgur so it takes some of the hassle away from us. I think that flash messages would be a bonus as we currently have a die() and exceptions are handled via index.php that did cause me some issues debugging. If we can implement a different solution for exceptions and messages that would be great. I tend to use http://symfony.com/doc/current/components/http_foundation/sessions.html#flash-messages that we can use as a component in the project. What options were you looking at here ? When displaying on the front end we can look at several different solutions. Imgur does not resize so we get returned the full image each time that could be large or small. Currently the CSS applies a height and width that is not great for performance. One option is to parse the image being returned using GD or alternative to crop but then we might be better pre processing the images prior to upload if we look at this method. Thinking about admin as well, I propose that we add a flag against the user table for "is admin" then on a post page if this is TRUE we offer a link to delete that has validation in the controller action. Simple solution for now until a more advanced admin interface is required. Im also thinking about introducing a form component from Symfony so that the form validation is handled in a more efficient way. Having a HTTP request object and form builder removes some of the overhead we would have to develop to fulfil this requirement. http://silex.sensiolabs.org/doc/providers/form.html |
Good stuff @jamescowie ! I like the idea of using imgur. It looks like we're never actually uploading the file to the server here? If so, I'd want to review it closely, make sure the file extension is validated. And actually probably just wait until we move this to it's own hosting account just to be on the safe side. |
Evening. I think its sensible testing this more when its moves. This gives us more time to work on what we want to achieve from content sharing. Ill move the ideas thread over to mageunity so we can get more input to what others want to get from this. Using Imgur the files are initially uploaded to the server in the tmp directory as it is still a php form. I will try and find a list of supported image types that the system supports so we can guard against those or just add in the common gif jpg etc. |
Still a work in progress but I wanted to see how easy it is to upload an image to imgur. This is in reference to #78
Plan is to use Guzzle and not curl directly in the controller. Also to move the client-id into the site configuration.