From 821132066592c88203d0b5feaf47345ce80564de Mon Sep 17 00:00:00 2001
From: Monty Taylor <mordred@inaugust.com>
Date: Sun, 18 Mar 2018 09:56:39 -0500
Subject: [PATCH] Stop falling back to job name for missing url

We set url to the job name if it's missing, but a job name is not a url
nor is it additional information. Then, in dashboard displays
we have display code that sets url to null if it matches the job name.

Instead of fixing the data at the display layer, let's set it to null in
the first place. So that we can update the dashboard code to remove the
workarounds, let's also run a migration to update the previously saved
build data.

Change-Id: I80ce26de4abc15720d7e37aee73049423584d1b9
---
 ...ge-missing-build-url-6df030ae3d8793ae.yaml |  7 ++++
 ...1bfe22_change_missing_build_url_to_null.py | 41 +++++++++++++++++++
 zuul/model.py                                 |  2 +-
 zuul/reporter/__init__.py                     |  6 +++
 4 files changed, 55 insertions(+), 1 deletion(-)
 create mode 100644 releasenotes/notes/change-missing-build-url-6df030ae3d8793ae.yaml
 create mode 100644 zuul/driver/sql/alembic/versions/52d49e1bfe22_change_missing_build_url_to_null.py

diff --git a/releasenotes/notes/change-missing-build-url-6df030ae3d8793ae.yaml b/releasenotes/notes/change-missing-build-url-6df030ae3d8793ae.yaml
new file mode 100644
index 0000000000..6a6f0baaa6
--- /dev/null
+++ b/releasenotes/notes/change-missing-build-url-6df030ae3d8793ae.yaml
@@ -0,0 +1,7 @@
+---
+upgrade:
+  - |
+    Previously the SqlReporter would record the job name in the database in
+    place of the url if the url was empty. This has now been updated to store
+    a null in the database for that case. Zuul will automatically run a
+    database migration to correct old values.
diff --git a/zuul/driver/sql/alembic/versions/52d49e1bfe22_change_missing_build_url_to_null.py b/zuul/driver/sql/alembic/versions/52d49e1bfe22_change_missing_build_url_to_null.py
new file mode 100644
index 0000000000..9457e995bb
--- /dev/null
+++ b/zuul/driver/sql/alembic/versions/52d49e1bfe22_change_missing_build_url_to_null.py
@@ -0,0 +1,41 @@
+# Copyright 2018 Red Hat, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+"""Change missing build url to null
+
+Revision ID: 52d49e1bfe22
+Revises: e0eda5d09eae
+Create Date: 2018-03-18 09:30:23.343650
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '52d49e1bfe22'
+down_revision = '32e28a297c3e'
+branch_labels = None
+depends_on = None
+
+BUILD_TABLE = 'zuul_build'
+
+from alembic import op
+
+
+def upgrade(table_prefix=''):
+    op.execute(
+        'UPDATE {table_name} SET log_url=NULL WHERE log_url=job_name'.format(
+            table_name=table_prefix + BUILD_TABLE))
+
+
+def downgrade(table_prefix=''):
+    raise Exception("Downgrades not supported")
diff --git a/zuul/model.py b/zuul/model.py
index 1d34af4339..bb8e3b491c 100644
--- a/zuul/model.py
+++ b/zuul/model.py
@@ -2854,7 +2854,7 @@ class QueueItem(object):
             self.log.exception("Error while parsing url for job %s:"
                                % (job,))
         if not url:
-            url = default_url or build.url or job.name
+            url = default_url or build.url or None
         return (result, url)
 
     def formatJSON(self, websocket_url=None):
diff --git a/zuul/reporter/__init__.py b/zuul/reporter/__init__.py
index da3c606b41..d7dc67bea0 100644
--- a/zuul/reporter/__init__.py
+++ b/zuul/reporter/__init__.py
@@ -251,6 +251,12 @@ class BaseReporter(object, metaclass=abc.ABCMeta):
             else:
                 error = ''
             name = job.name + ' '
+            # TODO(mordred) The gerrit consumption interface depends on
+            # something existing in the url field and don't have a great
+            # behavior defined for url being none/missing. Put name into
+            # the url field to match old behavior until we can deal with
+            # the gerrit-side piece as well
+            url = url or job.name
             jobs_fields.append((name, url, result, error, elapsed, voting))
         return jobs_fields