Bug 1376829 - Add notes to alert summaries (#3345)

2018-04-03 16:15:27 +03:00 · 2018-04-03 16:15:27 +03:00 · aa40c6a7e4
--- a/tests/webapp/api/test_performance_alertsummary_api.py
+++ b/tests/webapp/api/test_performance_alertsummary_api.py
@ -72,6 +72,7 @@ def test_alert_summaries_get(webapp, test_perf_alert_summary,
        'alerts',
        'bug_number',
        'issue_tracker',
+        'notes',
        'framework',
        'id',
        'last_updated',
@ -115,6 +116,7 @@ def test_alert_summaries_get_onhold(webapp, test_perf_alert_summary,
        'alerts',
        'bug_number',
        'issue_tracker',
+        'notes',
        'framework',
        'id',
        'last_updated',
--- a/treeherder/perf/migrations/0006_add_alert_summary_notes.py
+++ b/treeherder/perf/migrations/0006_add_alert_summary_notes.py
@ -0,0 +1,20 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.10 on 2018-03-08 14:53
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('perf', '0005_permit_github_links'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='performancealertsummary',
+            name='notes',
+            field=models.TextField(null=True, blank=True),
+        ),
+    ]
--- a/treeherder/perf/models.py
+++ b/treeherder/perf/models.py
@ -205,6 +205,8 @@ class PerformanceAlertSummary(models.Model):

    manually_created = models.BooleanField(default=False)

+    notes = models.TextField(null=True, blank=True)
+
    last_updated = models.DateTimeField(db_index=True)

    UNTRIAGED = 0
--- a/treeherder/webapp/api/performance_serializers.py
+++ b/treeherder/webapp/api/performance_serializers.py
@ -120,7 +120,8 @@ class PerformanceAlertSummarySerializer(serializers.ModelSerializer):
        model = PerformanceAlertSummary
        fields = ['id', 'push_id', 'prev_push_id',
                  'last_updated', 'repository', 'framework', 'alerts',
-                  'related_alerts', 'status', 'bug_number', 'issue_tracker']
+                  'related_alerts', 'status', 'bug_number',
+                  'issue_tracker', 'notes']


 class PerformanceBugTemplateSerializer(serializers.ModelSerializer):
--- a/ui/css/perf.css
+++ b/ui/css/perf.css
@ -410,6 +410,10 @@ span.compare-notsure {
  border-bottom: 1px solid transparent;
 }

+.notes-preview {
+    white-space: pre-wrap;
+}
+
 .alert-summary-header-element {
    flex: none;
    padding: 8px;
--- a/ui/js/controllers/perf/alerts.js
+++ b/ui/js/controllers/perf/alerts.js
@ -1,5 +1,8 @@
+import angular from 'angular';
+
 import perf from '../../perf';
 import modifyAlertsCtrlTemplate from '../../../partials/perf/modifyalertsctrl.html';
+import editAlertSummaryNotesCtrlTemplate from '../../../partials/perf/editnotesctrl.html';
 import { getApiUrl } from "../../../helpers/urlHelper";

 perf.factory('PhBugs', [
@ -67,7 +70,39 @@ perf.controller(
                }
            });
        }]);
+perf.controller(
+    'EditAlertSummaryNotesCtrl', ['$scope', '$uibModalInstance', 'alertSummary',
+        function ($scope, $uibModalInstance, alertSummary) {
+            $scope.title = "Edit notes";
+            $scope.placeholder = "Leave notes here...";
+            $scope.error = false;
+            $scope.alertSummaryCopy = angular.copy(alertSummary);

+            $scope.saveChanges = function () {
+                $scope.modifying = true;
+                $scope.alertSummaryCopy.saveNotes().then(function () {
+                    _.merge(alertSummary, $scope.alertSummaryCopy);
+                    $scope.modifying = false;
+                    $scope.error = false;
+
+                    $uibModalInstance.close();
+                }, function () {
+                    $scope.error = true;
+                    $scope.modifying = false;
+                }
+                );
+            };
+
+            $scope.cancel = function () {
+                $uibModalInstance.dismiss('cancel');
+            };
+
+            $scope.$on('modal.closing', function (event) {
+                if ($scope.modifying) {
+                    event.preventDefault();
+                }
+            });
+        }]);
 perf.controller(
    'MarkDownstreamAlertsCtrl', ['$scope', '$uibModalInstance', '$q', 'alertSummary',
        'allAlertSummaries', 'phAlertStatusMap',
@ -168,6 +203,19 @@ perf.controller('AlertsCtrl', [
            return Math.min(Math.abs(percent)*5, 100);
        };

+        $scope.editAlertSummaryNotes = function (alertSummary) {
+            $uibModal.open({
+                template: editAlertSummaryNotesCtrlTemplate,
+                controller: 'EditAlertSummaryNotesCtrl',
+                size: 'md',
+                resolve: {
+                    alertSummary: function () {
+                        return alertSummary;
+                    }
+                }
+            });
+        };
+
        // can filter by alert statuses or just show everything
        $scope.statuses = _.map(phAlertSummaryStatusMap);
        $scope.statuses = $scope.statuses.concat({ id: -1, text: "all" });
@ -217,7 +265,6 @@ perf.controller('AlertsCtrl', [
                }
            });
            $scope.numFilteredAlertSummaries = $scope.alertSummaries.filter(summary => !summary.anyVisible).length;
-
        }

        // these methods handle the business logic of alert selection and
@ -374,6 +421,8 @@ perf.controller('AlertsCtrl', [
                _.defaults(resultSetToSummaryMap,
                           _.set({}, alertSummary.repository, {}));

+                alertSummary.originalNotes = alertSummary.notes;
+
                [alertSummary.push_id, alertSummary.prev_push_id].forEach(
                    function (resultSetId) {
                        // skip nulls
--- a/ui/js/models/perf/alerts.js
+++ b/ui/js/models/perf/alerts.js
@ -63,6 +63,10 @@ treeherder.factory('PhAlerts', [
            Object.assign(this, alertSummaryData);
            this._initializeAlerts(optionCollectionMap);
        };
+        AlertSummary.prototype.modify = function (modification) {
+            return $http.put(getApiUrl(`/performance/alertsummary/${this.id}/`),
+                             modification);
+        };
        _.forEach(phAlertSummaryStatusMap, function (status) {
            AlertSummary.prototype['is' + _.capitalize(status.text)] = function () {
                return this.status === status.id;
@ -217,6 +221,17 @@ treeherder.factory('PhAlerts', [
            return _.find(phAlertSummaryStatusMap, { id: this.status }).text;
        };

+        AlertSummary.prototype.saveNotes = function () {
+            const alertSummary = this;
+            return this.modify({ notes: this.notes }).then(() => {
+                                alertSummary.originalNotes = alertSummary.notes;
+                                alertSummary.notesChanged = false;
+                             });
+        };
+        AlertSummary.prototype.editingNotes = function () {
+            this.notesChanged = (this.notes !== this.originalNotes);
+        };
+
        function _getAlertSummary(id) {
            // get a specific alert summary
            // in order to cancel the http request, a canceller must be used
--- a/ui/js/perf.js
+++ b/ui/js/perf.js
@ -3,6 +3,7 @@ import angularClipboardModule from 'angular-clipboard';
 import uiBootstrap from 'angular1-ui-bootstrap4';
 import uiRouter from 'angular-ui-router';

+import ngTextTruncateModule from '../vendor/ng-text-truncate';
 import treeherderModule from './treeherder';

 export default angular.module('perf', [
@ -10,4 +11,5 @@ export default angular.module('perf', [
  uiBootstrap,
  treeherderModule.name,
  angularClipboardModule.name,
+  ngTextTruncateModule.name
 ]);
--- a/ui/partials/perf/alertsctrl.html
+++ b/ui/partials/perf/alertsctrl.html
@ -88,6 +88,12 @@
            <li role="menuitem" ng-show="alertSummary.bug_number && user.is_staff">
              <a ng-click="unlinkBug(alertSummary)" class="dropdown-item">Unlink from bug</a>
            </li>
+            <li role="menuitem" ng-show="user.is_staff">
+              <a ng-click="editAlertSummaryNotes(alertSummary)" class="dropdown-item">
+                <span ng-if="!alertSummary.notes">Add notes</span>
+                <span ng-if="alertSummary.notes">Edit notes</span>
+              </a>
+            </li>
            <li role="menuitem" ng-show="user.is_staff" ng-if="alertSummary.isResolved()">
              <a ng-click="alertSummary.markInvestigating()" class="dropdown-item">Re-open</a>
            </li>
@ -195,31 +201,40 @@
        </span>
      </p>
    </div>
-    <div class="card-body" uib-collapse="!anySelected(alertSummary.alerts)">
-      <button ng-if="anySelectedAndTriaged(alertSummary.alerts)" class="btn btn-warning" role="button"
-              ng-click="resetAlerts(alertSummary)" title="Reset selected alerts to untriaged">
-        Reset
-      </button>
-      <span ng-if="!anySelectedAndTriaged(alertSummary.alerts)">
-        <button class="btn btn-light-bordered" role="button"
-                ng-click="markAlertsAcknowledged(alertSummary)" title="Acknowledge selected alerts as valid">
-          <span class="fa fa-check"></span> Acknowledge
+    <div class="card-body" uib-collapse="!anySelected(alertSummary.alerts) && !(alertSummary.notes)">
+      <div ng-show="anySelected(alertSummary.alerts)">
+        <button ng-if="anySelectedAndTriaged(alertSummary.alerts)" class="btn btn-warning" role="button"
+                ng-click="resetAlerts(alertSummary)" title="Reset selected alerts to untriaged">
+          Reset
        </button>
-        <button class="btn btn-light-bordered" role="button"
-                ng-click="markAlertsInvalid(alertSummary)" title="Mark selected alerts as invalid">
-          <span class="fa fa-ban"></span> Mark invalid
-        </button>
-        <button class="btn btn-light-bordered" role="button"
-                ng-click="markAlertsDownstream(alertSummary)"
-                title="Mark selected alerts as downstream from an alert summary on another branch">
-          <span class="fa fa-level-down"></span> Mark downstream
-        </button>
-        <button class="btn btn-light-bordered" role="button"
-                ng-click="reassignAlerts(alertSummary)"
-                title="Reassign selected alerts to another alert summary on the same branch">
-          <span class="fa fa-arrow-circle-o-right"></span> Reassign
-        </button>
-      </span>
+        <span ng-if="!anySelectedAndTriaged(alertSummary.alerts)">
+          <button class="btn btn-light-bordered" role="button"
+                  ng-click="markAlertsAcknowledged(alertSummary)" title="Acknowledge selected alerts as valid">
+            <span class="fa fa-check"></span> Acknowledge
+          </button>
+          <button class="btn btn-light-bordered" role="button"
+                  ng-click="markAlertsInvalid(alertSummary)" title="Mark selected alerts as invalid">
+            <span class="fa fa-ban"></span> Mark invalid
+          </button>
+          <button class="btn btn-light-bordered" role="button"
+                  ng-click="markAlertsDownstream(alertSummary)"
+                  title="Mark selected alerts as downstream from an alert summary on another branch">
+            <span class="fa fa-level-down"></span> Mark downstream
+          </button>
+          <button class="btn btn-light-bordered" role="button"
+                  ng-click="reassignAlerts(alertSummary)"
+                  title="Reassign selected alerts to another alert summary on the same branch">
+            <span class="fa fa-arrow-circle-o-right"></span> Reassign
+          </button>
+        </span>
+      </div>
+      <div ng-show="alertSummary.notes">
+        <p class="notes-preview bg-light rounded mt-2"
+           ng-text-truncate="alertSummary.notes"
+           ng-tt-chars-threshold="40"
+           ng-tt-more-label="Show"
+           ng-tt-less-label="Hide"></p>
+      </div>
    </div>
  </div>
  <p class="text-muted" ng-if="numFilteredAlertSummaries > 0">
--- a/ui/partials/perf/editnotesctrl.html
+++ b/ui/partials/perf/editnotesctrl.html
@ -0,0 +1,28 @@
+<form name="modifyAlert">
+  <div class="modal-header">
+    <h3 class="modal-title">{{title}}</h3>
+    <button type="button" class="close" ng-model="closeButton" ng-show="!modifying" ng-click="cancel()">
+      <span aria-hidden="true">&times;</span><span class="sr-only">Close</span>
+    </button>
+  </div>
+  <div class="modal-body">
+    <div class="form-group" ng-class="{'has-danger': error}">
+      <textarea
+          class="form-control"
+          ng-model="alertSummaryCopy.notes"
+          ng-change="alertSummaryCopy.editingNotes()"
+          ng-disabled="modifying"
+          name="editableNotes"
+          placeholder="{{placeholder}}"
+          cols="50" rows="10"
+          autofocus>
+      </textarea>
+    </div>
+    </div>
+  <div class="modal-footer">
+    <button class="btn btn-primary-soft" ng-click="saveChanges()" ng-disabled="modifying || !alertSummaryCopy.notesChanged">
+      <span ng-if="!modifying">Save</span>
+      <span ng-if="modifying">Saving...</span>
+    </button>
+  </div>
+</form>
--- a/ui/vendor/ng-text-truncate.js
+++ b/ui/vendor/ng-text-truncate.js
@ -0,0 +1,157 @@
+// vendored in to use more modern module syntax, based off of:
+// https://www.npmjs.com/package/ng-text-truncate-2
+
+(function (root, factory) {
+    /* istanbul ignore next */
+    if (typeof define === 'function' && define.amd) {
+        define(['angular'], factory);
+    } else if (typeof module === 'object' && module.exports) {
+        module.exports = factory(require('angular'));
+    } else {
+        root.ngTextTruncate = factory(root.angular);
+  }
+}(this, function (angular) {
+    'use strict';
+
+
+    return angular.module( 'ngTextTruncate', [] )
+
+
+    .directive( "ngTextTruncate", [ "$compile", "ValidationServices", "CharBasedTruncation", "WordBasedTruncation",
+        function( $compile, ValidationServices, CharBasedTruncation, WordBasedTruncation ) {
+        return {
+            restrict: "A",
+            scope: {
+                text: "=ngTextTruncate",
+                charsThreshould: "@ngTtCharsThreshold",
+                wordsThreshould: "@ngTtWordsThreshold",
+                customMoreLabel: "@ngTtMoreLabel",
+                customLessLabel: "@ngTtLessLabel"
+            },
+            controller: ["$scope", "$element", "$attrs", function( $scope, $element, $attrs ) {
+                $scope.toggleShow = function() {
+                    $scope.open = !$scope.open;
+                };
+
+                $scope.useToggling = $attrs.ngTtNoToggling === undefined;
+            }],
+            link: function( $scope, $element, $attrs ) {
+                $scope.open = false;
+
+                ValidationServices.failIfWrongThreshouldConfig( $scope.charsThreshould, $scope.wordsThreshould );
+
+                var CHARS_THRESHOLD = parseInt( $scope.charsThreshould );
+                var WORDS_THRESHOLD = parseInt( $scope.wordsThreshould );
+
+                $scope.$watch( "text", function() {
+                    $element.empty();
+
+                    if( CHARS_THRESHOLD ) {
+                            if( $scope.text && CharBasedTruncation.truncationApplies( $scope, CHARS_THRESHOLD ) ) {
+                                CharBasedTruncation.applyTruncation( CHARS_THRESHOLD, $scope, $element );
+
+                            } else {
+                                $element.append( $scope.text );
+                            }
+
+                    } else {
+
+                        if( $scope.text && WordBasedTruncation.truncationApplies( $scope, WORDS_THRESHOLD ) ) {
+                            WordBasedTruncation.applyTruncation( WORDS_THRESHOLD, $scope, $element );
+
+                        } else {
+                            $element.append( $scope.text );
+                        }
+
+                    }
+                } );
+            }
+        };
+    }] )
+
+
+
+    .factory( "ValidationServices", function() {
+        return {
+            failIfWrongThreshouldConfig: function( firstThreshould, secondThreshould ) {
+                if( (! firstThreshould && ! secondThreshould) || (firstThreshould && secondThreshould) ) {
+                    throw "You must specify one, and only one, type of threshould (chars or words)";
+                }
+            }
+        };
+    })
+
+
+
+    .factory( "CharBasedTruncation", [ "$compile", function( $compile ) {
+        return {
+            truncationApplies: function( $scope, threshould ) {
+                return $scope.text.length > threshould;
+            },
+
+            applyTruncation: function( threshould, $scope, $element ) {
+                if( $scope.useToggling ) {
+                    var el = angular.element(    "<span>" +
+                                                    $scope.text.substr( 0, threshould ) +
+                                                    "<span ng-show='!open'>...</span>" +
+                                                    "<span class='btn-link ngTruncateToggleText' " +
+                                                        "ng-click='toggleShow()'" +
+                                                        "ng-show='!open'>" +
+                                                        " " + ($scope.customMoreLabel ? $scope.customMoreLabel : "More") +
+                                                    "</span>" +
+                                                    "<span ng-show='open'>" +
+                                                        $scope.text.substring( threshould ) +
+                                                        "<span class='btn-link ngTruncateToggleText'" +
+                                                              "ng-click='toggleShow()'>" +
+                                                            " " + ($scope.customLessLabel ? $scope.customLessLabel : "Less") +
+                                                        "</span>" +
+                                                    "</span>" +
+                                                "</span>" );
+                    $compile( el )( $scope );
+                    $element.append( el );
+
+                } else {
+                    $element.append( $scope.text.substr( 0, threshould ) + "..." );
+
+                }
+            }
+        };
+    }])
+
+
+
+    .factory( "WordBasedTruncation", [ "$compile", function( $compile ) {
+        return {
+            truncationApplies: function( $scope, threshould ) {
+                return $scope.text.split( " " ).length > threshould;
+            },
+
+            applyTruncation: function( threshould, $scope, $element ) {
+                var splitText = $scope.text.split( " " );
+                if( $scope.useToggling ) {
+                    var el = angular.element(    "<span>" +
+                                                    splitText.slice( 0, threshould ).join( " " ) + " " +
+                                                    "<span ng-show='!open'>...</span>" +
+                                                    "<span class='btn-link ngTruncateToggleText' " +
+                                                        "ng-click='toggleShow()'" +
+                                                        "ng-show='!open'>" +
+                                                        " " + ($scope.customMoreLabel ? $scope.customMoreLabel : "More") +
+                                                    "</span>" +
+                                                    "<span ng-show='open'>" +
+                                                        splitText.slice( threshould, splitText.length ).join( " " ) +
+                                                        "<span class='btn-link ngTruncateToggleText'" +
+                                                              "ng-click='toggleShow()'>" +
+                                                            " " + ($scope.customLessLabel ? $scope.customLessLabel : "Less") +
+                                                        "</span>" +
+                                                    "</span>" +
+                                                "</span>" );
+                    $compile( el )( $scope );
+                    $element.append( el );
+
+                } else {
+                    $element.append( splitText.slice( 0, threshould ).join( " " ) + "..." );
+                }
+            }
+        };
+    }]);
+}));