From ecfd77aff3d2fbda93c38ef15681321e1f0981b6 Mon Sep 17 00:00:00 2001
From: Giulia Naponiello <gnaponie@redhat.com>
Date: Apr 24 2018 10:48:39 +0000
Subject: Comment required for submitting waivers


The comment parameter is now required when you submit a new waiver.
Both for API and CLI. Provided some test.

---

diff --git a/tests/test_api_v10.py b/tests/test_api_v10.py
index 8908db9..bac37e6 100644
--- a/tests/test_api_v10.py
+++ b/tests/test_api_v10.py
@@ -87,6 +87,30 @@ def test_create_waiver_with_original_spec_nvr_subject(mocked_get_user, mocked_re
     assert res_data['comment'] == 'it broke'
 
 
+@patch('waiverdb.api_v1.get_resultsdb_result')
+@patch('waiverdb.auth.get_user', return_value=('foo', {}))
+def test_create_waiver_without_comment(mocked_get_user, mocked_resultsdb, client,
+                                       session):
+    mocked_resultsdb.return_value = {
+        'data': {
+            'original_spec_nvr': ['somedata'],
+        },
+        'testcase': {'name': 'sometest'}
+    }
+
+    data = {
+        'result_id': 123,
+        'product_version': 'fool-1',
+        'waived': True,
+    }
+    r = client.post('/api/v1.0/waivers/', data=json.dumps(data),
+                    content_type='application/json')
+    res_data = json.loads(r.get_data(as_text=True))
+    assert r.status_code == 400
+    res_data = json.loads(r.get_data(as_text=True))
+    assert res_data['message'] == 'Comment is a required argument.'
+
+
 @patch('waiverdb.api_v1.get_resultsdb_result', side_effect=HTTPError(response=Mock(status=404)))
 @patch('waiverdb.auth.get_user', return_value=('foo', {}))
 def test_create_waiver_with_unknown_result_id(mocked_get_user, mocked_resultsdb, client, session):
diff --git a/tests/test_cli.py b/tests/test_cli.py
index 627f537..46f028c 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -123,7 +123,7 @@ oidc_scopes=
 resultsdb_api_url=http://localhost:5001/api/v2.0
         """)
     runner = CliRunner()
-    args = ['-C', p.strpath, '-p', 'fedora-26']
+    args = ['-C', p.strpath, '-p', 'fedora-26', '-c', 'comment']
     result = runner.invoke(waiverdb_cli, args)
     assert result.exit_code == 1
     assert result.output == 'Error: Please specify one subject\n'
@@ -142,12 +142,31 @@ oidc_scopes=
 resultsdb_api_url=http://localhost:5001/api/v2.0
         """)
     runner = CliRunner()
-    args = ['-C', p.strpath, '-p', 'fedora-26', '-s', 'subject']
+    args = ['-C', p.strpath, '-p', 'fedora-26', '-s', 'subject', '-c', 'comment']
     result = runner.invoke(waiverdb_cli, args)
     assert result.exit_code == 1
     assert result.output == 'Error: Please specify testcase\n'
 
 
+def test_no_comment(tmpdir):
+    p = tmpdir.join('client.conf')
+    p.write("""
+[waiverdb]
+auth_method=OIDC
+api_url=http://localhost:5004/api/v1.0
+oidc_id_provider=https://id.stg.fedoraproject.org/openidc/
+oidc_client_id=waiverdb
+oidc_scopes=
+    openid
+resultsdb_api_url=http://localhost:5001/api/v2.0
+        """)
+    runner = CliRunner()
+    args = ['-C', p.strpath, '-p', 'fedora-26', '-s', 'subject', '-t', 'testcase']
+    result = runner.invoke(waiverdb_cli, args)
+    assert result.exit_code == 1
+    assert result.output == 'Error: Please specify comment\n'
+
+
 def test_oidc_auth_is_enabled(tmpdir):
     # Skip if waiverdb is rebuilt for an environment where GSSAPI
     # authentication is used and python-openidc-client is not available.
diff --git a/waiverdb/api_v1.py b/waiverdb/api_v1.py
index ffe7a9b..0a08e07 100644
--- a/waiverdb/api_v1.py
+++ b/waiverdb/api_v1.py
@@ -250,6 +250,8 @@ class WaiversResource(Resource):
         if not args['subject'] or not args['testcase']:
             raise BadRequest('Either result_id or subject/testcase '
                              'are required arguments.')
+        if not args['comment']:
+            raise BadRequest('Comment is a required argument.')
 
         waiver = Waiver(args['subject'], args['testcase'], user,
                         args['product_version'], args['waived'], args['comment'], proxied_by)
diff --git a/waiverdb/cli.py b/waiverdb/cli.py
index bef7481..8b986d6 100644
--- a/waiverdb/cli.py
+++ b/waiverdb/cli.py
@@ -92,6 +92,8 @@ def cli(comment, waived, product_version, testcase, subject, result_id, config_f
     result_ids = result_id
     if not product_version:
         raise click.ClickException('Please specify product version')
+    if not comment:
+        raise click.ClickException('Please specify comment')
     if result_ids and (subject or testcase):
         raise click.ClickException('Please specify result_id or subject/testcase. Not both')
     if not result_ids and not subject: