magehero icon indicating copy to clipboard operation
magehero copied to clipboard

Initial image upload

Open jamescowie opened this issue 10 years ago • 5 comments

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.

jamescowie avatar Nov 18 '14 00:11 jamescowie

Ready for review. Allows uploading of images to Imgur and they are displayed on the site:

screen shot 2014-11-18 at 20 29 03

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. We can also add some meta data. Title description etc to the image. Not sure how this could look from the UI as there is the description content area.

What are your thoughts on this ?

jamescowie avatar Nov 18 '14 20:11 jamescowie

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?

bobbyshaw avatar Nov 18 '14 22:11 bobbyshaw

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

jamescowie avatar Nov 18 '14 22:11 jamescowie

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.

kalenjordan avatar Nov 19 '14 00:11 kalenjordan

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.

jamescowie avatar Nov 19 '14 21:11 jamescowie