Merge pull request #679 from /issues/24/1

Fixes #24: Allow images to be uploaded when creating a new bug report.

Author: karlcow

.gitignore

@@ -7,6 +7,7 @@ fabfile.py
 *.db
 docs/build/
 screenshots/
+uploads/
 
 # these are our 'dist' files
 # checking them into the repo is silly

CONTRIBUTING.md

@@ -206,7 +206,7 @@ cd webcompat.com
 ### Detailed setup
 #### Installing pip
 
-We use `pip` to install other Python packages. You may need to install `pip` if you haven't do so for another project or Python development. 
+We use `pip` to install other Python packages. You may need to install `pip` if you haven't do so for another project or Python development.
 
 To determine if you need to, type the following command into the terminal:
 
@@ -279,6 +279,14 @@ You can now edit `config.py` and
 
 ![Auth 404](https://i.cloudup.com/8FDA5bVc7l.png)
 
+**Note**: If you see the following error,
+
+```
+RuntimeError: no destination for set uploads
+```
+
+That means your `config.py` is out of sync with `config.py.example`. You can copy over the `Image uploading settings` from config.py.example, as well as the `LOCALHOST = not PRODUCTION and not DEVELOPMENT` line.
+
 ### Starting The Server
 
 ``` bash
@@ -374,19 +382,19 @@ Our Python unit tests are vanilla flavored [`unittest`](https://docs.python.org/
 
 Unit tests are preferred for features or functionality that are independent of the browser front-end, i.e., API responses, application routes, etc.
 
-Important documentation links: 
+Important documentation links:
 * [Writing nose tests](https://nose.readthedocs.org/en/latest/writing_tests.html)
 * [`unittest`](https://docs.python.org/2/library/unittest.html)
 * [Testing Flask](http://flask.pocoo.org/docs/0.10/testing/)
 
 ### JS Functional Tests
 
-Functional tests are written in JavaScript, using [Intern](http://theintern.io/). There's a nice [guide on the Intern wiki](https://github.com/theintern/intern/wiki/Writing-Tests-with-Intern#functional-testing) that should explain enough to get you started. 
+Functional tests are written in JavaScript, using [Intern](http://theintern.io/). There's a nice [guide on the Intern wiki](https://github.com/theintern/intern/wiki/Writing-Tests-with-Intern#functional-testing) that should explain enough to get you started.
 
 Important documentation links:
 * [Leadfoot](https://theintern.github.io/leadfoot/): the library that drives the browser (via Selenium).
-* [ChaiJS](http://chaijs.com/api/assert/): the library used for assertions. 
-* [Intern wiki](https://github.com/theintern/intern/wiki): contains useful examples. 
+* [ChaiJS](http://chaijs.com/api/assert/): the library used for assertions.
+* [Intern wiki](https://github.com/theintern/intern/wiki): contains useful examples.
 
 It's also recommended to look at the other test files in the `tests/functional` directory to see how things are commonly done.
 

config.py.example

@@ -22,6 +22,8 @@ SECRET_KEY = "meow"
 # the PRODUCTION and DEVELOPMENT env var is set in uwsgi.conf
 PRODUCTION = os.environ.get('PRODUCTION')
 DEVELOPMENT = os.environ.get('DEVELOPMENT')
+# Are we serving the app from localhost?
+LOCALHOST = not PRODUCTION and not DEVELOPMENT
 
 # Cache settings
 if PRODUCTION:
@@ -36,6 +38,18 @@ else:
 RATELIMIT_STORAGE_URL = 'memory://'
 RATELIMIT_STRATEGY = 'moving-window'
 
+# Image uploading settings
+# See http://pythonhosted.org/Flask-Uploads/#configuration
+if PRODUCTION:
+    UPLOADS_DEFAULT_DEST = ''
+    UPLOADS_DEFAULT_URL = ''
+elif DEVELOPMENT:
+    UPLOADS_DEFAULT_DEST = ''
+    UPLOADS_DEFAULT_URL = ''
+elif LOCALHOST:
+    UPLOADS_DEFAULT_DEST = BASE_DIR
+    UPLOADS_DEFAULT_URL = 'http://localhost:5000/'
+
 DEBUG = False
 
 if not PRODUCTION:

requirements.txt

@@ -3,6 +3,8 @@ Flask==0.10.1
 Flask-Cache==0.13.1
 Flask-Limiter==0.7.4
 Flask-SQLAlchemy==1.0
+Flask-Uploads==0.1.3
+Flask-WTF==0.12
 GitHub-Flask==0.3.4
 WTForms==2.0.2
 ua-parser==0.3.5

run.py

@@ -20,8 +20,8 @@
 
 try:
     from webcompat import app
-except ImportError:
-    raise ImportError(IMPORT_ERROR)
+except ImportError, e:
+    raise ImportError('{0}\n\n{1}'.format(e, IMPORT_ERROR))
 
 
 BOT_HELP = '''

tests/functional/reporting.js

@@ -68,7 +68,7 @@ define([
         .sleep(2000);
     },
 
-    'validation works': function() {
+    'URL validation': function() {
       return this.remote
         .setFindTimeout(intern.config.wc.pageLoadTimeout)
         .get(require.toUrl(url + '?open=1'))
@@ -78,8 +78,8 @@ define([
         .end()
         .findByCssSelector('#url').click()
         .end()
-        // wait a second
-        .sleep(1000)
+        // wait a bit
+        .sleep(100)
         .findByXpath('//*[@id="new-report"]/div/form/div[1]/div[2]/div[1]').getAttribute('class')
         .then(function (className) {
           assert.include(className, 'js-form-error');
@@ -88,20 +88,25 @@ define([
         .end()
         .findByCssSelector('#url').type('sup')
         .end()
-        // wait a second
-        .sleep(1000)
+        // wait a bit
+        .sleep(100)
         // xpath to the #url formGroup
         .findByXpath('//*[@id="new-report"]/div/form/div[1]/div[2]/div[1]').getAttribute('class')
         .then(function (className) {
           assert.include(className, 'js-no-error');
           assert.notInclude(className, 'js-form-error');
         })
-        .end()
-        // click in the textarea to trigger validation for radios
+        .end();
+    },
+
+    'Problem type validation': function() {
+      return this.remote
+        .setFindTimeout(intern.config.wc.pageLoadTimeout)
+        .get(require.toUrl(url + '?open=1'))
         .findByCssSelector('#description').click()
         .end()
-        // wait a second
-        .sleep(1000)
+        // wait a bit
+        .sleep(100)
         .findByXpath('//*[@id="new-report"]/div/form/div[1]/div[1]/fieldset').getAttribute('class')
         .then(function (className) {
           assert.include(className, 'js-form-error');
@@ -111,65 +116,73 @@ define([
         // pick a problem type
         .findByCssSelector('#problem_category-0').click()
         .end()
-        // wait a second
-        .sleep(1000)
+        // wait a bit
+        .sleep(100)
         // validation message should be removed now
         .findByXpath('//*[@id="new-report"]/div/form/div[1]/div[1]/fieldset').getAttribute('class')
         .then(function (className) {
           assert.include(className, 'js-no-error');
           assert.notInclude(className, 'js-form-error');
         })
+        .end();
+    },
+
+    'Image extension validation': function() {
+      return this.remote
+        .setFindTimeout(intern.config.wc.pageLoadTimeout)
+        .get(require.toUrl(url + '?open=1'))
+        .findByCssSelector('#image').type('/path/to/foo.hacks')
+        .end()
+        // wait a bit
+        .sleep(100)
+        .findByXpath('//*[@id="new-report"]/div/form/div[2]/div[2]').getAttribute('class')
+        .then(function (className) {
+          assert.include(className, 'js-form-error');
+          assert.notInclude(className, 'js-no-error');
+        })
+        .end()
+        // pick a valid file type
+        .findByCssSelector('#image').type('/path/to/foo.jpg')
+        .end()
+        // wait a bit
+        .sleep(100)
+        // validation message should be removed now
+        .findByXpath('//*[@id="new-report"]/div/form/div[2]/div[2]').getAttribute('class')
+        .then(function (className) {
+          assert.include(className, 'js-no-error');
+          assert.notInclude(className, 'js-form-error');
+        })
+        .end();
+    },
+
+    'Submits are enabled': function() {
+      return this.remote
+        .setFindTimeout(intern.config.wc.pageLoadTimeout)
+        .get(require.toUrl(url + '?open=1'))
+        // pick a valid file type
+        .findByCssSelector('#image').type('/path/to/foo.png')
         .end()
-        // now make sure the buttons aren't disabled anymore
+        .findByCssSelector('#url').type('http://coolguy.biz')
+        .end()
+        // pick a problem type
+        .findByCssSelector('#problem_category-0').click()
+        .end()
+        .findByCssSelector('#description').click()
+        .end()
+        // wait a bit
+        .sleep(100)
+         // now make sure the buttons aren't disabled anymore
         .findByCssSelector('#submitanon').getAttribute('class')
         .then(function (className) {
           assert.notInclude(className, 'is-disabled');
         })
         .end()
+         // now make sure the buttons aren't disabled anymore
         .findByCssSelector('#submitgithub').getAttribute('class')
         .then(function (className) {
           assert.notInclude(className, 'is-disabled');
-        });
-    },
-
-    // 'anonymous report': function () {
-    //   var issueNumber;
-
-    //   return this.remote
-    //     .setFindTimeout(intern.config.wc.pageLoadTimeout)
-    //     .get(require.toUrl(url + "?open=1"))
-    //     .findByCssSelector('#url').type('some broken URL')
-    //     .end()
-    //     .findByCssSelector('#summary').type('this site doesnt work ' + Math.random())
-    //     .end()
-    //     .findByCssSelector('#submitanon').click()
-    //     .end()
-    //     .findByCssSelector('.wc-content--body h2').getVisibleText()
-    //     .then(function (text) {
-    //       // Make sure we got to the /thanks/<number> route
-    //       assert.equal(text, 'Thank you.');
-    //     })
-    //     .end()
-    //     .findByCssSelector('.wc-content--body a').getVisibleText()
-    //     .then(function (text) {
-    //       // Grab the issue number from the end of the URL link
-    //       issueNumber = text.split('/').pop();
-    //     })
-    //     .end()
-    //     .findByCssSelector('.wc-content--body a').getVisibleText().click()
-    //     .end()
-    //     .findByCssSelector('.js-issue-title').getVisibleText()
-    //     .then(function (text) {
-    //       // Make sure GitHub has the correct title
-    //       assert.include(text, 'some broken URL - this site doesnt work');
-    //     })
-    //     .end()
-    //     .findByCssSelector('.gh-header-number').getVisibleText()
-    //     .then(function (text) {
-    //       // Make sure GitHub has the correct issue number
-    //       assert.equal(text, '#' + issueNumber);
-    //     })
-    // }
-
+        })
+        .end();
+    }
   });
 });

tests/test_uploads.py

@@ -0,0 +1,101 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+'''Tests for image upload API endpoint.'''
+
+import os.path
+import sys
+import unittest
+
+from flask import Request
+from flaskext.uploads import TestingFileStorage
+from StringIO import StringIO
+from werkzeug import FileStorage
+from werkzeug.datastructures import MultiDict
+
+# Add webcompat module to import path
+sys.path.append(os.path.realpath(os.pardir))
+
+from webcompat import app
+
+
+class TestUploads(unittest.TestCase):
+    # modified from http://prschmid.blogspot.com/2013/05/unit-testing-flask-file-uploads-without.html
+    def setUp(self):
+        app.config['TESTING'] = True
+        self.app = app
+        self.test_client = self.app.test_client()
+
+    def tearDown(self):
+        pass
+
+    def testGet(self):
+        '''Test that /upload/ doesn't let you GET.'''
+        rv = self.test_client.get('/upload/')
+        self.assertEqual(rv.status_code, 404)
+
+    def testBadUploads(self):
+        # Loop over some files and the status codes that we are expecting
+        for filename, status_code in (
+                ('foo.xxx', 415),
+                ('foo', 415),
+                ('foo.rb', 415)):
+
+            # The reason why we are defining it in here and not outside
+            # this method is that we are setting the filename of the
+            # TestingFileStorage to be the one in the for loop. This way
+            # we can ensure that the filename that we are "uploading"
+            # is the same as the one being used by the application
+            class TestingRequest(Request):
+                """A testing request to use that will return a
+                TestingFileStorage to test the uploading."""
+                @property
+                def files(self):
+                    d = MultiDict()
+                    d['image'] = TestingFileStorage(filename=filename)
+                    return d
+
+            self.app.request_class = TestingRequest
+            test_client = self.app.test_client()
+
+            rv = test_client.post(
+                '/upload/',
+                data=dict(
+                    file=(StringIO('Fake image data'), filename),
+                ))
+            self.assertEqual(rv.status_code, status_code)
+
+    def testGoodUploads(self):
+        # Loop over some files and the URLs that we are expecting back
+        for filename, status_code in (
+                ('foo.png', 201),
+                ('foo.jpg', 201),
+                ('foo.gif', 201),
+                ('foo.jpe', 201),
+                ('foo.jpeg', 201),
+                ('foo.bmp', 201)):
+
+            class TestingRequest(Request):
+                """A testing request to use that will return a
+                TestingFileStorage to test the uploading."""
+                @property
+                def files(self):
+                    d = MultiDict()
+                    d['image'] = TestingFileStorage(filename=filename)
+                    return d
+
+            self.app.request_class = TestingRequest
+            test_client = self.app.test_client()
+
+            rv = test_client.post(
+                '/upload/',
+                data=dict(
+                    file=(StringIO('Fake image data'), filename),
+                ))
+            self.assertEqual(rv.status_code, status_code)
+
+if __name__ == '__main__':
+    unittest.main()

webcompat/__init__.py

@@ -26,6 +26,9 @@
 # import views after we initialize our github object
 import webcompat.views
 
-# register API blueprint
+# register blueprints
 from api.endpoints import api
+from api.uploads import uploads
+
 app.register_blueprint(api)
+app.register_blueprint(uploads)