From c3cda163adf8f98477ca5927272db562a16b9f29 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 28 2017 12:39:23 +0000 Subject: [PATCH 1/3] Support to upload multiple files when creating a new ticket and scan them With this commit we can select multiple files when creating a new ticket they will get uploaded and inserted into the content of the ticket as expected. We also fix scanning these attachments for Virus if this option is enabled. Fixes https://pagure.io/pagure/issue/2366 Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/templates/new_issue.html b/pagure/templates/new_issue.html index e03f36f..54dd49c 100644 --- a/pagure/templates/new_issue.html +++ b/pagure/templates/new_issue.html @@ -150,19 +150,14 @@ {% if authenticated and form %} $(document).ready(function() { // Set up the handler for the file input box. - $("#file-picker").on("change", function() { + $("#file-picker").on("change", function(evt) { + var files = evt.target.files; //doUpload("{{ form.csrf_token.current_token }}", this.files); var _txt = $("#issue_content").val(); if (_txt) { _txt += '\n'; } - var _loc = "{{ url_for('view_issue_raw_file', - repo=repo.name, - username=username, - namespace=repo.namespace, - filename='') }}" + this.file; - - $("#issue_content").val(_txt + ''); + $("#issue_content").val(_txt + '\n'.repeat(files.length)); }); // List username in @ drop-down diff --git a/pagure/ui/issues.py b/pagure/ui/issues.py index d7eaf13..76033a6 100644 --- a/pagure/ui/issues.py +++ b/pagure/ui/issues.py @@ -899,32 +899,38 @@ def new_issue(repo, username=None, namespace=None): ticketfolder=APP.config['TICKETS_FOLDER'], ) SESSION.commit() + # If there is a file attached, attach it. - filestream = flask.request.files.get('filestream') - if filestream and '' in issue.content: - new_filename = pagure.lib.add_attachment( - repo=repo, - issue=issue, - attachmentfolder=APP.config['ATTACHMENTS_FOLDER'], - user=user_obj, - filename=filestream.filename, - filestream=filestream.stream, - ) - # Replace the tag in the comment with the link - # to the actual image - filelocation = flask.url_for( - 'view_issue_raw_file', - repo=repo.name, - username=username, - namespace=repo.namespace, - filename=new_filename, - ) - new_filename = new_filename.split('-', 1)[1] - url = '[![%s](%s)](%s)' % ( - new_filename, filelocation, filelocation) - issue.content = issue.content.replace('', url) - SESSION.add(issue) - SESSION.commit() + form = pagure.forms.UploadFileForm() + if form.validate_on_submit(): + streams = flask.request.files.getlist('filestream') + n_img = issue.content.count('') + if n_img == len(streams): + for filestream in streams: + new_filename = pagure.lib.add_attachment( + repo=repo, + issue=issue, + attachmentfolder=APP.config['ATTACHMENTS_FOLDER'], + user=user_obj, + filename=filestream.filename, + filestream=filestream.stream, + ) + # Replace the tag in the comment with the + # link to the actual image + filelocation = flask.url_for( + 'view_issue_raw_file', + repo=repo.name, + username=username, + namespace=repo.namespace, + filename=new_filename, + ) + new_filename = new_filename.split('-', 1)[1] + url = '[![%s](%s)](%s)' % ( + new_filename, filelocation, filelocation) + issue.content = issue.content.replace( + '', url, 1) + SESSION.add(issue) + SESSION.commit() return flask.redirect(flask.url_for( '.view_issue', username=username, repo=repo.name, From 610f98b7a53a1bface3bab9c8557ad4aad247fa1 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 28 2017 12:39:23 +0000 Subject: [PATCH 2/3] Split the existing tests in two three tests for better isolation Signed-off-by: Pierre-Yves Chibon --- diff --git a/tests/test_pagure_flask_ui_issues.py b/tests/test_pagure_flask_ui_issues.py index ed9a1ef..ff51160 100644 --- a/tests/test_pagure_flask_ui_issues.py +++ b/tests/test_pagure_flask_ui_issues.py @@ -170,8 +170,7 @@ class PagureFlaskIssuestests(tests.Modeltests): '
\n New issue' in output.data) - csrf_token = output.data.split( - 'name="csrf_token" type="hidden" value="')[1].split('">')[0] + csrf_token = self.get_csrf() with open(os.path.join(tests.HERE, 'placebo.png'), 'r') as stream: data = { @@ -202,12 +201,26 @@ class PagureFlaskIssuestests(tests.Modeltests): '994e01f5e11ca40bc3abe', output.data) + @patch('pagure.lib.git.update_git') + @patch('pagure.lib.notify.send_email') + def test_new_issue_w_file_no_issue_tracker(self, p_send_email, p_ugt): + """ Test the new_issue endpoint with a file. """ + 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) + # 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() + user = tests.FakeUser() user.username = 'pingou' with tests.user_set(pagure.APP, user): with open(os.path.join(tests.HERE, 'placebo.png'), 'r') as stream: @@ -217,13 +230,26 @@ class PagureFlaskIssuestests(tests.Modeltests): 'status': 'Open', 'filestream': stream, 'enctype': 'multipart/form-data', - 'csrf_token': csrf_token, + 'csrf_token': self.get_csrf(), } output = self.app.post( '/test/new_issue', data=data, follow_redirects=True) self.assertEqual(output.status_code, 404) + @patch('pagure.lib.git.update_git') + @patch('pagure.lib.notify.send_email') + def test_new_issue_w_file_namespace(self, p_send_email, p_ugt): + """ Test the new_issue endpoint with a file. """ + 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) + # Project with a namespace user = tests.FakeUser() user.username = 'pingou' @@ -234,8 +260,7 @@ class PagureFlaskIssuestests(tests.Modeltests): '
\n New issue' in output.data) - csrf_token = output.data.split( - 'name="csrf_token" type="hidden" value="')[1].split('">')[0] + csrf_token = self.get_csrf() with open(os.path.join(tests.HERE, 'placebo.png'), 'r') as stream: data = { From 88abbee0efc516a434d0100718c58e1a037269ab Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 28 2017 12:39:23 +0000 Subject: [PATCH 3/3] Add tests for creating a new issue with multiple attachments Signed-off-by: Pierre-Yves Chibon --- diff --git a/tests/test_pagure_flask_ui_issues.py b/tests/test_pagure_flask_ui_issues.py index ff51160..672ffa6 100644 --- a/tests/test_pagure_flask_ui_issues.py +++ b/tests/test_pagure_flask_ui_issues.py @@ -294,6 +294,136 @@ class PagureFlaskIssuestests(tests.Modeltests): @patch('pagure.lib.git.update_git') @patch('pagure.lib.notify.send_email') + def test_new_issue_w_files(self, p_send_email, p_ugt): + """ Test the new_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) + + user = tests.FakeUser() + user.username = 'pingou' + with tests.user_set(pagure.APP, user): + output = self.app.get('/test/new_issue') + self.assertEqual(output.status_code, 200) + self.assertTrue( + '
\n New issue' + in output.data) + + csrf_token = self.get_csrf() + + with open( + os.path.join(tests.HERE, 'placebo.png'), 'r' + ) as stream: + with open( + os.path.join(tests.HERE, 'pagure.png'), 'r' + ) as stream2: + data = { + 'title': 'Test issue', + 'issue_content': 'We really should improve on this issue\n' + '\n', + 'status': 'Open', + 'filestream': [stream, stream2], + 'enctype': 'multipart/form-data', + 'csrf_token': csrf_token, + } + + output = self.app.post( + '/test/new_issue', data=data, follow_redirects=True) + + self.assertEqual(output.status_code, 200) + self.assertIn( + 'Issue #1: Test issue - test - Pagure', + output.data) + self.assertIn( + '', + output.data) + # Check the image was uploaded + self.assertIn( + 'href="/test/issue/raw/' + '8a06845923010b27bfd8e7e75acff7badc40d1021b4' + '994e01f5e11ca40bc3abe', + output.data) + self.assertIn( + 'href="/test/issue/raw/' + '6498a2de405546200b6144da56fc25d0a3976ae688d' + 'bfccaca609c8b4480523e', + output.data) + + @patch('pagure.lib.git.update_git') + @patch('pagure.lib.notify.send_email') + def test_new_issue_w_files_namespace(self, p_send_email, p_ugt): + """ Test the new_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) + + # Project with a namespace + user = tests.FakeUser() + user.username = 'pingou' + with tests.user_set(pagure.APP, user): + output = self.app.get('/somenamespace/test3/new_issue') + self.assertEqual(output.status_code, 200) + self.assertTrue( + '