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

Initial image upload #79

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Initial image upload #79

wants to merge 4 commits into from

Conversation

jamescowie
Copy link
Contributor

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
Copy link
Contributor Author

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 ?

@bobbyshaw
Copy link
Collaborator

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?

@jamescowie
Copy link
Contributor Author

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

@kalenjordan
Copy link
Owner

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.

@jamescowie
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants