Bug 1443777 - Bug links from old alert summaries are broken (#3560)

Make issue_tracker column non nullable and fix broken tests

tests/conftest.py

@@ -140,6 +140,16 @@ def test_repository(transactional_db):
     return r
 
 
+@pytest.fixture
+def test_issue_tracker(transactional_db):
+    from treeherder.perf.models import IssueTracker
+
+    return IssueTracker.objects.create(
+        name="Bugzilla",
+        task_base_url="https://bugzilla.mozilla.org/show_bug.cgi?id="
+    )
+
+
 @pytest.fixture
 def test_repository_2(test_repository):
     from treeherder.model.models import Repository
@@ -604,7 +614,7 @@ def text_log_error_lines(test_job, failure_lines):
 
 
 @pytest.fixture
-def test_perf_alert_summary(test_repository, push_stored, test_perf_framework):
+def test_perf_alert_summary(test_repository, push_stored, test_perf_framework, test_issue_tracker):
     from treeherder.perf.models import PerformanceAlertSummary
     return PerformanceAlertSummary.objects.create(
         repository=test_repository,

tests/etl/test_perf_data_adapters.py

@@ -371,7 +371,7 @@ def test_same_signature_multiple_performance_frameworks(test_repository,
                                {'shouldAlert': True, 'alertChangeType': 'absolute'},
                                False, True),
                         ])
-def test_alert_generation(test_repository,
+def test_alert_generation(test_repository, test_issue_tracker,
                           failure_classifications, generic_reference_data,
                           alerts_enabled_repository,
                           add_suite_value, extra_suite_metadata,
@@ -475,8 +475,8 @@ def test_framework_not_enabled(test_repository,
     assert 0 == PerformanceDatum.objects.all().count()
 
 
-def test_last_updated(test_repository, failure_classifications,
-                      generic_reference_data):
+def test_last_updated(test_repository, test_issue_tracker,
+                      failure_classifications, generic_reference_data):
     _generate_perf_data_range(test_repository,
                               generic_reference_data,
                               reverse_push_range=True)

tests/perfalert/test_create_alerts.py

@@ -34,7 +34,7 @@ def _verify_alert(alertid, expected_push_id, expected_prev_push_id,
 
 
 def _generate_performance_data(test_repository, test_perf_signature,
-                               generic_reference_data,
+                               test_issue_tracker, generic_reference_data,
                                base_timestamp, start_id, value, amount):
     for (t, v) in zip([i for i in range(start_id, start_id + amount)],
                       [value for i in range(start_id, start_id + amount)]):
@@ -56,16 +56,21 @@ def _generate_performance_data(test_repository, test_perf_signature,
 
 
 def test_detect_alerts_in_series(test_repository,
+                                 test_issue_tracker,
                                  failure_classifications,
                                  generic_reference_data,
                                  test_perf_signature):
 
     base_time = time.time()  # generate it based off current time
     INTERVAL = 30
-    _generate_performance_data(test_repository, test_perf_signature,
+    _generate_performance_data(test_repository,
+                               test_perf_signature,
+                               test_issue_tracker,
                                generic_reference_data,
                                base_time, 1, 0.5, int(INTERVAL / 2))
-    _generate_performance_data(test_repository, test_perf_signature,
+    _generate_performance_data(test_repository,
+                               test_perf_signature,
+                               test_issue_tracker,
                                generic_reference_data,
                                base_time, int(INTERVAL / 2) + 1, 1.0, int(INTERVAL / 2))
 
@@ -86,7 +91,9 @@ def test_detect_alerts_in_series(test_repository,
                   PerformanceAlertSummary.UNTRIAGED, None)
 
     # add data that should be enough to generate a new alert if we rerun
-    _generate_performance_data(test_repository, test_perf_signature,
+    _generate_performance_data(test_repository,
+                               test_perf_signature,
+                               test_issue_tracker,
                                generic_reference_data,
                                base_time, (INTERVAL+1), 2.0, INTERVAL)
     generate_new_alerts_in_series(test_perf_signature)
@@ -99,8 +106,8 @@ def test_detect_alerts_in_series(test_repository,
 
 
 def test_detect_alerts_in_series_with_retriggers(
-        test_repository, failure_classifications,
-        generic_reference_data, test_perf_signature):
+        test_repository, test_issue_tracker,
+        failure_classifications, generic_reference_data, test_perf_signature):
 
     # sometimes we detect an alert in the middle of a series
     # where there are retriggers, make sure we handle this case
@@ -111,15 +118,21 @@ def test_detect_alerts_in_series_with_retriggers(
     # mix)
     base_time = time.time()  # generate it based off current time
     for i in range(20):
-        _generate_performance_data(test_repository, test_perf_signature,
+        _generate_performance_data(test_repository,
+                                   test_perf_signature,
+                                   test_issue_tracker,
                                    generic_reference_data,
                                    base_time, 1, 0.5, 1)
     for i in range(5):
-        _generate_performance_data(test_repository, test_perf_signature,
+        _generate_performance_data(test_repository,
+                                   test_perf_signature,
+                                   test_issue_tracker,
                                    generic_reference_data,
                                    base_time, 2, 0.5, 1)
     for i in range(15):
-        _generate_performance_data(test_repository, test_perf_signature,
+        _generate_performance_data(test_repository,
+                                   test_perf_signature,
+                                   test_issue_tracker,
                                    generic_reference_data,
                                    base_time, 2, 1.0, 1)
 
@@ -130,14 +143,18 @@ def test_detect_alerts_in_series_with_retriggers(
 
 
 def test_no_alerts_with_old_data(
-        test_repository, failure_classifications,
-        generic_reference_data, test_perf_signature):
+        test_repository, test_issue_tracker,
+        failure_classifications, generic_reference_data, test_perf_signature):
     base_time = 0  # 1970, too old!
     INTERVAL = 30
-    _generate_performance_data(test_repository, test_perf_signature,
+    _generate_performance_data(test_repository,
+                               test_perf_signature,
+                               test_issue_tracker,
                                generic_reference_data,
                                base_time, 1, 0.5, int(INTERVAL / 2))
-    _generate_performance_data(test_repository, test_perf_signature,
+    _generate_performance_data(test_repository,
+                               test_perf_signature,
+                               test_issue_tracker,
                                generic_reference_data,
                                base_time, int(INTERVAL / 2) + 1, 1.0, int(INTERVAL / 2))
 
@@ -148,8 +165,8 @@ def test_no_alerts_with_old_data(
 
 
 def test_custom_alert_threshold(
-        test_repository, failure_classifications,
-        generic_reference_data, test_perf_signature):
+        test_repository, test_issue_tracker,
+        failure_classifications, generic_reference_data, test_perf_signature):
 
     test_perf_signature.alert_threshold = 200.0
     test_perf_signature.save()
@@ -159,13 +176,19 @@ def test_custom_alert_threshold(
     # of 200% that should only generate 1
     INTERVAL = 60
     base_time = time.time()
-    _generate_performance_data(test_repository, test_perf_signature,
+    _generate_performance_data(test_repository,
+                               test_perf_signature,
+                               test_issue_tracker,
                                generic_reference_data,
                                base_time, 1, 0.5, int(INTERVAL / 3))
-    _generate_performance_data(test_repository, test_perf_signature,
+    _generate_performance_data(test_repository,
+                               test_perf_signature,
+                               test_issue_tracker,
                                generic_reference_data,
                                base_time, int(INTERVAL / 3) + 1, 0.6, int(INTERVAL / 3))
-    _generate_performance_data(test_repository, test_perf_signature,
+    _generate_performance_data(test_repository,
+                               test_perf_signature,
+                               test_issue_tracker,
                                generic_reference_data,
                                base_time, 2 * int(INTERVAL / 3) + 1, 2.0, int(INTERVAL / 3))
 
@@ -178,6 +201,7 @@ def test_custom_alert_threshold(
 @pytest.mark.parametrize(('new_value', 'expected_num_alerts'),
                          [(1.0, 1), (0.25, 0)])
 def test_alert_change_type_absolute(test_repository,
+                                    test_issue_tracker,
                                     failure_classifications,
                                     generic_reference_data,
                                     test_perf_signature, new_value,
@@ -191,10 +215,10 @@ def test_alert_change_type_absolute(test_repository,
     base_time = time.time()  # generate it based off current time
     INTERVAL = 30
     _generate_performance_data(test_repository, test_perf_signature,
-                               generic_reference_data,
+                               test_issue_tracker, generic_reference_data,
                                base_time, 1, 0.5, int(INTERVAL / 2))
     _generate_performance_data(test_repository, test_perf_signature,
-                               generic_reference_data,
+                               test_issue_tracker, generic_reference_data,
                                base_time, int(INTERVAL / 2) + 1, new_value,
                                int(INTERVAL / 2))
 

tests/webapp/api/test_performance_alertsummary_api.py

@@ -171,8 +171,8 @@ def test_alert_summaries_put(client, test_repository, test_perf_signature,
     assert PerformanceAlertSummary.objects.get(id=1).status == 1
 
 
-def test_alert_summary_post(client, test_repository, push_stored,
-                            test_perf_signature, test_user, test_sheriff):
+def test_alert_summary_post(client, test_repository, test_issue_tracker,
+                            push_stored, test_perf_signature, test_user, test_sheriff):
     # this blob should be sufficient to create a new alert summary (assuming
     # the user of this API is authorized to do so!)
     post_blob = {

treeherder/perf/migrations/0009_non_nullable_issue_tracker.py

@@ -0,0 +1,21 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.13 on 2018-05-23 08:07
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('perf', '0008_add_confirming_state'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='performancealertsummary',
+            name='issue_tracker',
+            field=models.ForeignKey(default=1, on_delete=django.db.models.deletion.PROTECT, to='perf.IssueTracker'),
+        ),
+    ]

treeherder/perf/models.py

@@ -238,7 +238,7 @@ class PerformanceAlertSummary(models.Model):
 
     issue_tracker = models.ForeignKey(IssueTracker,
                                       on_delete=models.PROTECT,
-                                      null=True)
+                                      default=1)  # Bugzilla
 
     def update_status(self):
         autodetermined_status = self.autodetermine_status()

ui/js/controllers/perf/alerts.js

@@ -316,7 +316,7 @@ perf.controller('AlertsCtrl', [
             });
         };
         $scope.unlinkBug = function (alertSummary) {
-            alertSummary.assignBug(null, null).then(function () {
+            alertSummary.unassignBug().then(function () {
                 updateAlertVisibility();
             });
         };

ui/js/models/perf/alerts.js

@@ -96,6 +96,7 @@ treeherder.factory('PhAlerts', [
             };
         });
         AlertSummary.prototype.getIssueTrackerUrl = function () {
+            if (!this.bug_number) { return; }
             if (this.issue_tracker) {
                 const issueTrackerUrl = _.find(issueTrackers, { id: this.issue_tracker }).issueTrackerUrl;
                 return issueTrackerUrl + this.bug_number;
@@ -228,6 +229,13 @@ treeherder.factory('PhAlerts', [
                                  return alertSummary.update();
                              });
         };
+        AlertSummary.prototype.unassignBug = function () {
+            const alertSummary = this;
+            return $http.put(getApiUrl(`/performance/alertsummary/${this.id}/`),
+                             { bug_number: null }).then(function () {
+                                 return alertSummary.update();
+                             });
+        };
         AlertSummary.prototype.modifySelectedAlerts = function (modification) {
             this.allSelected = false;