From 5434db2393ce68bc408871eeb514b3c5db5add28 Mon Sep 17 00:00:00 2001
From: Clement Verna <cverna@tutanota.com>
Date: Mar 19 2018 11:50:35 +0000
Subject: Make the ci job an independent value.


This commit creates an attribute for the ci_job
that allows not to depend on the url parsing to
get the job name. This will make it easier
also to trigger more than one job name per project
in the future.

Signed-off-by: Clement Verna <cverna@tutanota.com>

---

diff --git a/alembic/versions/e18d5b78d782_add_ci_job_attribute_to_the_hook_pagure_.py b/alembic/versions/e18d5b78d782_add_ci_job_attribute_to_the_hook_pagure_.py
new file mode 100644
index 0000000..bb1bc45
--- /dev/null
+++ b/alembic/versions/e18d5b78d782_add_ci_job_attribute_to_the_hook_pagure_.py
@@ -0,0 +1,27 @@
+"""Add ci_job attribute to the hook_pagure_ci table
+
+Revision ID: e18d5b78d782
+Revises: 22fb5256f555
+Create Date: 2018-03-16 11:51:04.613420
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = 'e18d5b78d782'
+down_revision = '22fb5256f555'
+
+from alembic import op
+import sqlalchemy as sa
+
+
+def upgrade():
+    ''' Add ci_job column to projects table'''
+    op.add_column(
+        'hook_pagure_ci',
+        sa.Column('ci_job', sa.String(255), nullable=True, unique=False)
+    )
+
+
+def downgrade():
+    ''' Revert the ci_job column added'''
+    op.drop_column('hook_pagure_ci', 'ci_job')
diff --git a/pagure/hooks/pagure_ci.py b/pagure/hooks/pagure_ci.py
index b1c8cc6..1d61e3a 100644
--- a/pagure/hooks/pagure_ci.py
+++ b/pagure/hooks/pagure_ci.py
@@ -50,6 +50,10 @@ class PagureCITable(BASE):
         sa.String(255),
         nullable=True,
         unique=False)
+    ci_job = sa.Column(
+        sa.String(255),
+        nullable=True,
+        unique=False)
     active = sa.Column(sa.Boolean, nullable=False, default=False)
 
     project = relation(
@@ -97,6 +101,11 @@ class PagureCiForm(FlaskForm):
         'URL to the project on the CI service',
         [RequiredIf('active'), wtforms.validators.Length(max=255)],
     )
+
+    ci_job = wtforms.TextField(
+        'Name of the job to trigger',
+        [RequiredIf('active'), wtforms.validators.Length(max=255)],
+    )
     active = wtforms.BooleanField(
         'Active',
         [wtforms.validators.Optional()]
@@ -126,7 +135,7 @@ class PagureCi(BaseHook):
     form = PagureCiForm
     db_object = PagureCITable
     backref = 'ci_hook'
-    form_fields = ['ci_type', 'ci_url', 'active']
+    form_fields = ['ci_type', 'ci_url', 'ci_job', 'active']
 
     @classmethod
     def set_up(cls, project):
diff --git a/pagure/lib/lib_ci.py b/pagure/lib/lib_ci.py
index 12e6fa4..eda1dc7 100644
--- a/pagure/lib/lib_ci.py
+++ b/pagure/lib/lib_ci.py
@@ -34,9 +34,8 @@ def process_jenkins_build(session, project, build_id, requestfolder):
     """
     import jenkins
     # Jenkins Base URL
-    jenk = jenkins.Jenkins(project.ci_hook.ci_url.split('/job/')[0])
-    jenkins_name = project.ci_hook.ci_url.split(
-        '/job/', 1)[1].split('/', 1)[0]
+    jenk = jenkins.Jenkins(project.ci_hook.ci_url)
+    jenkins_name = project.ci_hook.ci_job
     build_info = jenk.get_build_info(jenkins_name, build_id)
     result = build_info.get('result')
     url = build_info['url']
@@ -85,7 +84,7 @@ def process_jenkins_build(session, project, build_id, requestfolder):
     session.commit()
 
 
-def trigger_jenkins_build(project_path, url, token, branch, pr_id):
+def trigger_jenkins_build(project_path, url, job, token, branch, pr_id):
     """ Trigger a build on a jenkins instance."""
     try:
         import jenkins
@@ -100,23 +99,18 @@ def trigger_jenkins_build(project_path, url, token, branch, pr_id):
         pagure_config['GIT_URL_GIT'].rstrip('/'),
         project_path)
 
-    url = url.rstrip('/')
-    # Jenkins Base URL
-    base_url, name = url.split('/job/', 1)
-    jenkins_name = name.rstrip('/').replace('/job/', '/')
-
     data = {
         'cause': pr_id,
         'REPO': repo,
         'BRANCH': branch
     }
 
-    server = jenkins.Jenkins(base_url)
+    server = jenkins.Jenkins(url)
     _log.info('Pagure-CI: Triggering at: %s for: %s - data: %s' % (
-        base_url, jenkins_name, data))
+        url, job, data))
     try:
         server.build_job(
-            name=jenkins_name,
+            name=job,
             parameters=data,
             token=token
         )
diff --git a/pagure/lib/tasks_services.py b/pagure/lib/tasks_services.py
index 3b7eefe..f8c4f0c 100644
--- a/pagure/lib/tasks_services.py
+++ b/pagure/lib/tasks_services.py
@@ -364,13 +364,16 @@ def trigger_ci_build(self, project_name, pr_id, branch, ci_type):
 
         if project.is_fork:
             url = project.parent.ci_hook.ci_url
+            job = project.parent.ci_hook.ci_job
             token = project.parent.ci_hook.pagure_ci_token
         else:
             url = project.ci_hook.ci_url
+            job = project.ci_hook.ci_job
             token = project.ci_hook.pagure_ci_token
 
         trigger_jenkins_build(project_path=project.path,
                               url=url,
+                              job=job,
                               token=token,
                               branch=branch,
                               pr_id=pr_id)
diff --git a/pagure/templates/_formhelper.html b/pagure/templates/_formhelper.html
index eaf44a2..a1d98a1 100644
--- a/pagure/templates/_formhelper.html
+++ b/pagure/templates/_formhelper.html
@@ -68,8 +68,8 @@
 
 {% macro render_field_in_row(field, after="") %}
 <tr>
-    <td>{{ field.label }}</td>
-    <td>{{ field(**kwargs)|safe }}</td>
+    <td style="padding-right:2em">{{ field.label }}</td>
+    <td>{{ field(class="form-control")|safe }}</td>
 {% if after %} <td>{{ after }}</td>{% endif %}
 {% if field.errors %}
 {% for error in field.errors %}
diff --git a/tests/test_pagure_flask_ui_plugins_pagure_ci.py b/tests/test_pagure_flask_ui_plugins_pagure_ci.py
index 0fafc72..b2fd1ad 100644
--- a/tests/test_pagure_flask_ui_plugins_pagure_ci.py
+++ b/tests/test_pagure_flask_ui_plugins_pagure_ci.py
@@ -42,11 +42,14 @@ class PagureFlaskPluginPagureCItests(tests.SimplePagureTest):
                 'test project #1        </div>', output.data)
             self.assertTrue('<h3>Pagure CI settings</h3>' in output.data)
             self.assertIn(
-                '<td><label for="ci_url">URL to the project on the CI '
-                'service</label></td>' , output.data)
+                '<label for="ci_url">URL to the project on the CI '
+                'service</label>', output.data)
             self.assertIn(
-                '<input id="active" name="active" type="checkbox" value="y">',
-                output.data)
+                '<label for="ci_job">Name of the job to trigger'
+                '</label>', output.data)
+            self.assertIn(
+                '<input class="form-control" id="active" name="active" '
+                'type="checkbox" value="y">', output.data)
 
             csrf_token = output.data.split(
                 'name="csrf_token" type="hidden" value="')[1].split('">')[0]
@@ -60,17 +63,21 @@ class PagureFlaskPluginPagureCItests(tests.SimplePagureTest):
                 'test project #1        </div>', output.data)
             self.assertTrue('<h3>Pagure CI settings</h3>' in output.data)
             self.assertIn(
-                '<td><label for="ci_url">URL to the project on the CI '
-                'service</label></td>' , output.data)
+                '<label for="ci_url">URL to the project on the CI '
+                'service</label>', output.data)
             self.assertIn(
-                '<input id="active" name="active" type="checkbox" value="y">',
-                output.data)
+                '<label for="ci_job">Name of the job to trigger'
+                '</label>', output.data)
+            self.assertIn(
+                '<input class="form-control" id="active" name="active" '
+                'type="checkbox" value="y">', output.data)
 
             # Activate hook
             data = {
                 'active': 'y',
                 'ci_url': 'https://jenkins.fedoraproject.org',
                 'ci_type': 'jenkins',
+                'ci_job': 'test/job'
             }
             # CSRF Token missing
             output = self.app.post(
@@ -81,11 +88,14 @@ class PagureFlaskPluginPagureCItests(tests.SimplePagureTest):
                 'test project #1        </div>', output.data)
             self.assertTrue('<h3>Pagure CI settings</h3>' in output.data)
             self.assertIn(
-                '<td><label for="ci_url">URL to the project on the CI '
-                'service</label></td>' , output.data)
+                '<label for="ci_url">URL to the project on the CI '
+                'service</label>', output.data)
             self.assertIn(
-                '<input checked id="active" name="active" type="checkbox" '
-                'value="y">', output.data)
+                '<label for="ci_job">Name of the job to trigger'
+                '</label>', output.data)
+            self.assertIn(
+                '<input checked class="form-control" id="active" name="active" '
+                'type="checkbox" value="y">', output.data)
 
             data['csrf_token'] = csrf_token
 
@@ -110,11 +120,14 @@ class PagureFlaskPluginPagureCItests(tests.SimplePagureTest):
                 'test project #1        </div>', output.data)
             self.assertTrue('<h3>Pagure CI settings</h3>' in output.data)
             self.assertIn(
-                '<td><label for="ci_url">URL to the project on the CI '
-                'service</label></td>' , output.data)
-            self.assertTrue(
-                '<input checked id="active" name="active" type="checkbox" value="y">'
-                in output.data)
+                '<label for="ci_url">URL to the project on the CI '
+                'service</label>', output.data)
+            self.assertIn(
+                '<label for="ci_job">Name of the job to trigger'
+                '</label>', output.data)
+            self.assertIn(
+                '<input checked class="form-control" id="active" name="active" '
+                'type="checkbox" value="y">', output.data)
             self.assertIn(
                 '<pre>\nhttps://pagure.org/api/0/ci/jenkins/test/',
                 output.data)
@@ -140,11 +153,14 @@ class PagureFlaskPluginPagureCItests(tests.SimplePagureTest):
                 'test project #1        </div>', output.data)
             self.assertTrue('<h3>Pagure CI settings</h3>' in output.data)
             self.assertIn(
-                '<td><label for="ci_url">URL to the project on the CI '
-                'service</label></td>' , output.data)
+                '<label for="ci_url">URL to the project on the CI '
+                'service</label>', output.data)
             self.assertIn(
-                '<input id="active" name="active" type="checkbox" '
-                'value="y">', output.data)
+                '<label for="ci_job">Name of the job to trigger'
+                '</label>', output.data)
+            self.assertIn(
+                '<input class="form-control" id="active" name="active" '
+                'type="checkbox" value="y">', output.data)
 
             # Missing the required ci_url
             data = {'csrf_token': csrf_token, 'active': 'y'}
@@ -159,12 +175,16 @@ class PagureFlaskPluginPagureCItests(tests.SimplePagureTest):
             self.assertFalse(
                 '</button>\n                      Hook activated' in output.data)
             self.assertIn(
-                '<td><input id="ci_url" name="ci_url" type="text" value="">'
+                '<td><input class="form-control" id="ci_url" name="ci_url" type="text" value="">'
+                '</td>\n<td class="errors">This field is required.</td>',
+                output.data)
+            self.assertIn(
+                '<td><input class="form-control" id="ci_job" name="ci_job" type="text" value="">'
                 '</td>\n<td class="errors">This field is required.</td>',
                 output.data)
             self.assertIn(
-                '<input checked id="active" name="active" type="checkbox" '
-                'value="y">', output.data)
+                '<input checked class="form-control" id="active" name="active" '
+                'type="checkbox" value="y">', output.data)
 
     def test_plugin_pagure_ci_namespaced(self):
         """ Test the pagure ci plugin on/off endpoint. """
@@ -181,11 +201,14 @@ class PagureFlaskPluginPagureCItests(tests.SimplePagureTest):
                 'namespaced test project        </div>', output.data)
             self.assertTrue('<h3>Pagure CI settings</h3>' in output.data)
             self.assertIn(
-                '<td><label for="ci_url">URL to the project on the CI '
-                'service</label></td>' , output.data)
+                '<label for="ci_url">URL to the project on the CI '
+                'service</label>', output.data)
             self.assertIn(
-                '<input id="active" name="active" type="checkbox" value="y">',
-                output.data)
+                '<label for="ci_job">Name of the job to trigger'
+                '</label>', output.data)
+            self.assertIn(
+                '<input class="form-control" id="active" name="active" '
+                'type="checkbox" value="y">', output.data)
 
             csrf_token = output.data.split(
                 'name="csrf_token" type="hidden" value="')[1].split('">')[0]
@@ -194,6 +217,7 @@ class PagureFlaskPluginPagureCItests(tests.SimplePagureTest):
             data = {
                 'active': 'y',
                 'ci_url': 'https://jenkins.fedoraproject.org',
+                'ci_job': 'test/job',
                 'ci_type': 'jenkins',
                 'csrf_token': csrf_token,
             }
@@ -219,11 +243,14 @@ class PagureFlaskPluginPagureCItests(tests.SimplePagureTest):
                 'namespaced test project        </div>', output.data)
             self.assertTrue('<h3>Pagure CI settings</h3>' in output.data)
             self.assertIn(
-                '<td><label for="ci_url">URL to the project on the CI '
-                'service</label></td>' , output.data)
+                '<label for="ci_url">URL to the project on the CI '
+                'service</label>', output.data)
+            self.assertIn(
+                '<label for="ci_job">Name of the job to trigger'
+                '</label>', output.data)
             self.assertTrue(
-                '<input checked id="active" name="active" type="checkbox" value="y">'
-                in output.data)
+                '<input checked class="form-control" id="active" name="active" '
+                'type="checkbox" value="y">' in output.data)
             self.assertIn(
                 '<pre>\nhttps://pagure.org/api/0/ci/jenkins/somenamespace/test3/',
                 output.data)