From d8c24159926ef5a6b1ddc66b65a14bcd4d844572 Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: Jun 07 2017 09:53:02 +0000
Subject: [PATCH 1/7] Add support for uploading multiple files to an issue at once


Fixes https://pagure.io/pagure/issue/2315

Signed-off-by: Pierre-Yves Chibon <pingou@pingoured.fr>

---

diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py
index c0a6313..c0ce5bb 100644
--- a/pagure/lib/__init__.py
+++ b/pagure/lib/__init__.py
@@ -2511,7 +2511,7 @@ def add_attachment(repo, issue, attachmentfolder, user, filename, filestream):
         hashlib.sha256(filestream.read()).hexdigest(),
         werkzeug.secure_filename(filename)
     )
-    filedir = os.path.join(attachmentfolder, repo.fullname)
+    filedir = os.path.join(attachmentfolder, repo.fullname, 'files')
     filepath = os.path.join(filedir, filename)
 
     if os.path.exists(filepath):
diff --git a/pagure/static/upload.js b/pagure/static/upload.js
index 387669b..6c9c251 100644
--- a/pagure/static/upload.js
+++ b/pagure/static/upload.js
@@ -63,16 +63,17 @@ function doUpload(csrf_token, files) {
         return;
       }
       else {
-        console.log(data);
         var _txt = $("#comment").val();
         if (_txt) {
           _txt += '\n';
         }
-        $("#comment").val(
-           _txt
-           + '[![' + data.filename + '](' + data.filelocation + ')]('
-            + data.filelocation + ')'
-        )
+        var _urls = '';
+        for (var i = 0, ie = data.filenames.length; i < ie; i++) {
+          _urls += '[![' + data.filenames[i] + ']('
+            + data.filelocations[i] + ')]('
+            + data.filelocations[i] + ')'
+        }
+        $("#comment").val(_txt + _urls)
       }
       setTimeout(
         function(){
diff --git a/pagure/ui/issues.py b/pagure/ui/issues.py
index 9c897b4..deb93c9 100644
--- a/pagure/ui/issues.py
+++ b/pagure/ui/issues.py
@@ -1249,26 +1249,33 @@ def upload_issue(repo, issueid, username=None, namespace=None):
     form = pagure.forms.UploadFileForm()
 
     if form.validate_on_submit():
-        filestream = flask.request.files['filestream']
-        new_filename = pagure.lib.add_attachment(
-            repo=repo,
-            issue=issue,
-            attachmentfolder=APP.config['ATTACHMENTS_FOLDER'],
-            user=user_obj,
-            filename=filestream.filename,
-            filestream=filestream.stream,
-        )
+        filenames = []
+        for filestream in flask.request.files.getlist('filestream'):
+            filenames.append(filestream.filename)
+            new_filename = pagure.lib.add_attachment(
+                repo=repo,
+                issue=issue,
+                attachmentfolder=APP.config['ATTACHMENTS_FOLDER'],
+                user=user_obj,
+                filename=filestream.filename,
+                filestream=filestream.stream,
+            )
 
         return flask.jsonify({
             'output': 'ok',
-            'filename': new_filename.split('-', 1)[1],
-            'filelocation': flask.url_for(
-                'view_issue_raw_file',
-                repo=repo.name,
-                username=username,
-                namespace=repo.namespace,
-                filename=new_filename,
-            )
+            'filenames': [
+                filename.split('-', 1)[1]
+                for filename in filenames],
+            'filelocations': [
+                flask.url_for(
+                    'view_issue_raw_file',
+                    repo=repo.name,
+                    username=username,
+                    namespace=repo.namespace,
+                    filename=new_filename,
+                )
+                for new_filename in filenames
+            ]
         })
     else:
         return flask.jsonify({'output': 'notok'})

From 1cb715ee2041638ea0b0e67592407d4f1ef3a97d Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: Jun 07 2017 09:53:02 +0000
Subject: [PATCH 2/7] Fix typo in the logging


Signed-off-by: Pierre-Yves Chibon <pingou@pingoured.fr>

---

diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py
index c0ce5bb..778f463 100644
--- a/pagure/lib/__init__.py
+++ b/pagure/lib/__init__.py
@@ -2503,7 +2503,7 @@ def reset_status_pull_request(session, project):
 def add_attachment(repo, issue, attachmentfolder, user, filename, filestream):
     ''' Add a file to the attachments folder of repo and update git. '''
     _log.info(
-        'Addinf file: %s to the git repo: %s',
+        'Adding file: %s to the git repo: %s',
         repo.path, werkzeug.secure_filename(filename))
 
     # Prefix the filename with a timestamp:

From 3887318d8f62d5cea5d3ff9c6e87c320d69a458c Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: Jun 07 2017 09:53:02 +0000
Subject: [PATCH 3/7] Fixes path to the files uploaded or linked to


Signed-off-by: Pierre-Yves Chibon <pingou@pingoured.fr>

---

diff --git a/pagure/lib/git.py b/pagure/lib/git.py
index 6d79166..55b36fa 100644
--- a/pagure/lib/git.py
+++ b/pagure/lib/git.py
@@ -871,7 +871,7 @@ def _add_file_to_git(repo, issue, attachmentfolder, ticketfolder, user,
         os.mkdir(folder_path)
 
     # Copy from attachments directory
-    src = os.path.join(attachmentfolder, repo.fullname, filename)
+    src = os.path.join(attachmentfolder, repo.fullname, 'files', filename)
     shutil.copyfile(src, file_path)
 
     # Retrieve the list of files that changed
diff --git a/pagure/ui/issues.py b/pagure/ui/issues.py
index deb93c9..e5337c9 100644
--- a/pagure/ui/issues.py
+++ b/pagure/ui/issues.py
@@ -1251,7 +1251,6 @@ def upload_issue(repo, issueid, username=None, namespace=None):
     if form.validate_on_submit():
         filenames = []
         for filestream in flask.request.files.getlist('filestream'):
-            filenames.append(filestream.filename)
             new_filename = pagure.lib.add_attachment(
                 repo=repo,
                 issue=issue,
@@ -1260,6 +1259,7 @@ def upload_issue(repo, issueid, username=None, namespace=None):
                 filename=filestream.filename,
                 filestream=filestream.stream,
             )
+            filenames.append(new_filename)
 
         return flask.jsonify({
             'output': 'ok',
@@ -1272,7 +1272,7 @@ def upload_issue(repo, issueid, username=None, namespace=None):
                     repo=repo.name,
                     username=username,
                     namespace=repo.namespace,
-                    filename=new_filename,
+                    filename='files/%s' % new_filename,
                 )
                 for new_filename in filenames
             ]

From 8be363bef5c5ea30925a7876cc49afc6333658f8 Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: Jun 07 2017 09:53:02 +0000
Subject: [PATCH 4/7] Add some more logging to the tasks performed by the worker


Signed-off-by: Pierre-Yves Chibon <pingou@pingoured.fr>

---

diff --git a/pagure/lib/tasks.py b/pagure/lib/tasks.py
index dbbcf98..411dee0 100644
--- a/pagure/lib/tasks.py
+++ b/pagure/lib/tasks.py
@@ -28,6 +28,7 @@ from pagure import APP
 import pagure.lib
 import pagure.lib.git
 
+logging.config.dictConfig(APP.config.get('LOGGING') or {'version': 1})
 _log = logging.getLogger(__name__)
 
 
@@ -359,7 +360,9 @@ def merge_pull_request(name, namespace, user, requestid, user_merger):
     with project.lock('WORKER'):
         request = pagure.lib.search_pull_requests(
             session, project_id=project.id, requestid=requestid)
-
+        _log.debug(
+            'Merging pull-request: %/#%s', request.project.fullname,
+            request.id)
         pagure.lib.git.merge_pull_request(
             session, request, user_merger, APP.config['REQUESTS_FOLDER'])
 
@@ -379,9 +382,16 @@ def add_file_to_git(name, namespace, user, user_attacher, issueuid, filename):
         issue = pagure.lib.get_issue_by_uid(session, issueuid)
         user_attacher = pagure.lib.search_user(session, username=user_attacher)
 
+        from_folder = APP.config['ATTACHMENTS_FOLDER']
+        to_folder = APP.config['TICKETS_FOLDER']
+        _log.info(
+            'Adding file %s from %s to %s', filename, from_folder, to_folder)
         pagure.lib.git._add_file_to_git(
-            project, issue, APP.config['ATTACHMENTS_FOLDER'],
-            APP.config['TICKETS_FOLDER'], user_attacher, filename)
+            project, issue,
+            from_folder,
+            to_folder,
+            user_attacher,
+            filename)
 
     session.remove()
     gc_clean()

From d265db336e6cd3b49004829fcc08ca08f2177af9 Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: Jun 07 2017 09:53:02 +0000
Subject: [PATCH 5/7] Adjust the default logging config to the one run in prod


Signed-off-by: Pierre-Yves Chibon <pingou@pingoured.fr>

---

diff --git a/pagure/default_config.py b/pagure/default_config.py
index fca3695..21b0318 100644
--- a/pagure/default_config.py
+++ b/pagure/default_config.py
@@ -305,8 +305,8 @@ LOGGING = {
     'loggers': {
         'pagure': {
             'handlers': ['console'],
-            'level': 'INFO',
-            'propagate': False
+            'level': 'DEBUG',
+            'propagate': True
         },
         'flask': {
             'handlers': ['console'],
@@ -318,5 +318,15 @@ LOGGING = {
             'level': 'WARN',
             'propagate': False
         },
+        'binaryornot': {
+            'handlers': ['console'],
+            'level': 'WARN',
+            'propagate': True
+        },
+        'pagure.lib.encoding_utils': {
+            'handlers': ['console'],
+            'level': 'WARN',
+            'propagate': False
+        },
     }
 }

From e85e3283f28ba6b851062441574caf70d2f6c5c4 Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: Jun 07 2017 09:53:02 +0000
Subject: [PATCH 6/7] Log when we're pushing to the remote


---

diff --git a/pagure/lib/git.py b/pagure/lib/git.py
index 55b36fa..c1f11aa 100644
--- a/pagure/lib/git.py
+++ b/pagure/lib/git.py
@@ -916,6 +916,7 @@ def _add_file_to_git(repo, issue, attachmentfolder, ticketfolder, user,
     master_ref = new_repo.lookup_reference('HEAD').resolve()
     refname = '%s:%s' % (master_ref.name, master_ref.name)
 
+    _log.info('Pushing to %s: %s', ori_remote.name, refname)
     PagureRepo.push(ori_remote, refname)
 
     # Remove the clone

From 766620b3af4e9f4f2d1e3fa255163d7a39ad816d Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: Jun 07 2017 10:15:46 +0000
Subject: [PATCH 7/7] Fix and improve testing uploading a file to a ticket


This commit adds unit-tests for uploading multiple files at once in the
UI as well as rework how the testing for single upload is done so that
it is tested even when pyclamd is not installed/available.

Signed-off-by: Pierre-Yves Chibon <pingou@pingoured.fr>

---

diff --git a/tests/test_pagure_flask_ui_issues.py b/tests/test_pagure_flask_ui_issues.py
index 972f28f..48212f5 100644
--- a/tests/test_pagure_flask_ui_issues.py
+++ b/tests/test_pagure_flask_ui_issues.py
@@ -42,9 +42,6 @@ class PagureFlaskIssuestests(tests.Modeltests):
         super(PagureFlaskIssuestests, self).setUp()
 
         pagure.APP.config['TESTING'] = True
-        # TODO: Figure a way to enable this test on jenkins
-        if not os.environ.get('BUILD_ID'):
-            pagure.APP.config['VIRUS_SCAN_ATTACHMENTS'] = True
         pagure.SESSION = self.session
         pagure.ui.SESSION = self.session
         pagure.ui.app.SESSION = self.session
@@ -1571,8 +1568,6 @@ class PagureFlaskIssuestests(tests.Modeltests):
     @patch('pagure.lib.notify.send_email')
     def test_upload_issue(self, p_send_email, p_ugt):
         """ Test the upload_issue endpoint. """
-        if not pyclamd:
-            raise SkipTest()
         p_send_email.return_value = True
         p_ugt.return_value = True
 
@@ -1628,6 +1623,75 @@ class PagureFlaskIssuestests(tests.Modeltests):
             exp = {'output': 'notok'}
             self.assertDictEqual(json_data, exp)
 
+            # Attach a file to a ticket
+            with open(os.path.join(tests.HERE, 'placebo.png'), 'rb') as stream:
+                data = {
+                    'csrf_token': csrf_token,
+                    'filestream': stream,
+                    'enctype': 'multipart/form-data',
+                }
+                output = self.app.post(
+                    '/test/issue/1/upload', data=data, follow_redirects=True)
+            self.assertEqual(output.status_code, 200)
+            json_data = json.loads(output.data)
+
+            folder = os.path.dirname(
+                os.path.abspath(__file__))[1:].replace('/', '_')
+            exp = {
+                'filelocations': [
+                    '/test/issue/raw/files/8a06845923010b27bfd8'
+                    'e7e75acff7badc40d1021b4994e01f5e11ca40bc3a'
+                    'be-%s_placebo.png' % folder
+                ],
+                'filenames': ['%s_placebo.png' % folder],
+                'output': 'ok'
+            }
+            self.assertDictEqual(json_data, exp)
+
+        # Project w/o issue tracker
+        repo = pagure.get_authorized_project(self.session, 'test')
+        repo.settings = {'issue_tracker': False}
+        self.session.add(repo)
+        self.session.commit()
+
+        with tests.user_set(pagure.APP, user):
+            output = self.app.post('/test/issue/1/upload')
+            self.assertEqual(output.status_code, 404)
+
+    @patch.dict('pagure.APP.config', {'PR_ONLY': True})
+    @patch('pagure.lib.git.update_git')
+    @patch('pagure.lib.notify.send_email')
+    def test_upload_issue_virus(self, p_send_email, p_ugt):
+        """ Test the upload_issue endpoint. """
+        if not pyclamd:
+            raise SkipTest()
+        p_send_email.return_value = True
+        p_ugt.return_value = True
+
+        tests.create_projects(self.session)
+        tests.create_projects_git(
+            os.path.join(self.path, 'repos'), bare=True)
+        tests.create_projects_git(
+            os.path.join(self.path, 'tickets'), bare=True)
+
+        # Create issues to play with
+        repo = pagure.get_authorized_project(self.session, 'test')
+        msg = pagure.lib.new_issue(
+            session=self.session,
+            repo=repo,
+            title='Test issue',
+            content='We should work on this',
+            user='pingou',
+            ticketfolder=None
+        )
+        self.session.commit()
+        self.assertEqual(msg.title, 'Test issue')
+
+        user = tests.FakeUser()
+        user.username = 'pingou'
+        with tests.user_set(pagure.APP, user):
+            csrf_token = self.get_csrf()
+
             # TODO: Figure a way to enable this test on jenkins
             # Try to attach a virus
             if not os.environ.get('BUILD_ID'):
@@ -1649,15 +1713,47 @@ class PagureFlaskIssuestests(tests.Modeltests):
                     }
                     self.assertDictEqual(json_data, exp)
 
-            # Attach a file to a ticket
+    @patch('pagure.lib.git.update_git')
+    @patch('pagure.lib.notify.send_email')
+    def test_upload_issue_two_files(self, p_send_email, p_ugt):
+        """ Test the upload_issue endpoint with two files. """
+        p_send_email.return_value = True
+        p_ugt.return_value = True
+
+        tests.create_projects(self.session)
+        tests.create_projects_git(
+            os.path.join(self.path, 'repos'), bare=True)
+        tests.create_projects_git(
+            os.path.join(self.path, 'tickets'), bare=True)
+
+        # Create issues to play with
+        repo = pagure.get_authorized_project(self.session, 'test')
+        msg = pagure.lib.new_issue(
+            session=self.session,
+            repo=repo,
+            title='Test issue',
+            content='We should work on this',
+            user='pingou',
+            ticketfolder=None
+        )
+        self.session.commit()
+        self.assertEqual(msg.title, 'Test issue')
+
+        user = tests.FakeUser()
+        user.username = 'pingou'
+        with tests.user_set(pagure.APP, user):
+            csrf_token = self.get_csrf()
+
+            # Attach two files to a ticket
             with open(os.path.join(tests.HERE, 'placebo.png'), 'rb') as stream:
-                data = {
-                    'csrf_token': csrf_token,
-                    'filestream': stream,
-                    'enctype': 'multipart/form-data',
-                }
-                output = self.app.post(
-                    '/test/issue/1/upload', data=data, follow_redirects=True)
+                with open(os.path.join(tests.HERE, 'placebo.png'), 'rb') as stream2:
+                    data = {
+                        'csrf_token': csrf_token,
+                        'filestream': [stream, stream2],
+                        'enctype': 'multipart/form-data',
+                    }
+                    output = self.app.post(
+                        '/test/issue/1/upload', data=data, follow_redirects=True)
             self.assertEqual(output.status_code, 200)
             json_data = json.loads(output.data)
 
@@ -1665,23 +1761,21 @@ class PagureFlaskIssuestests(tests.Modeltests):
                 os.path.abspath(__file__))[1:].replace('/', '_')
             exp = {
                 'output': 'ok',
-                'filelocation': '/test/issue/raw/8a06845923010b27bfd8'
-                                'e7e75acff7badc40d1021b4994e01f5e11ca40bc3a'
-                                'be-%s_placebo.png' % folder,
-                'filename': '%s_placebo.png' % folder,
+                'filelocations': [
+                    '/test/issue/raw/files/8a06845923010b27bfd8'
+                    'e7e75acff7badc40d1021b4994e01f5e11ca40bc3a'
+                    'be-%s_placebo.png' % folder,
+                    '/test/issue/raw/files/8a06845923010b27bfd8'
+                    'e7e75acff7badc40d1021b4994e01f5e11ca40bc3a'
+                    'be-%s_placebo.png' % folder,
+                ],
+                'filenames': [
+                    '%s_placebo.png' % folder,
+                    '%s_placebo.png' % folder
+                ],
             }
             self.assertDictEqual(json_data, exp)
 
-        # Project w/o issue tracker
-        repo = pagure.get_authorized_project(self.session, 'test')
-        repo.settings = {'issue_tracker': False}
-        self.session.add(repo)
-        self.session.commit()
-
-        with tests.user_set(pagure.APP, user):
-            output = self.app.post('/test/issue/1/upload')
-            self.assertEqual(output.status_code, 404)
-
     def test_view_issue_raw_file_empty(self):
         """ Test the view_issue_raw_file endpoint. """
         # Create the project and git repos

