From aa40c6a7e490a7907d7afe87e35ce49ea4e0d842 Mon Sep 17 00:00:00 2001 From: ionutgoldan Date: Tue, 3 Apr 2018 16:15:27 +0300 Subject: [PATCH] Bug 1376829 - Add notes to alert summaries (#3345) --- .../api/test_performance_alertsummary_api.py | 2 + .../0006_add_alert_summary_notes.py | 20 +++ treeherder/perf/models.py | 2 + .../webapp/api/performance_serializers.py | 3 +- ui/css/perf.css | 4 + ui/js/controllers/perf/alerts.js | 51 +++++- ui/js/models/perf/alerts.js | 15 ++ ui/js/perf.js | 2 + ui/partials/perf/alertsctrl.html | 63 ++++--- ui/partials/perf/editnotesctrl.html | 28 ++++ ui/vendor/ng-text-truncate.js | 157 ++++++++++++++++++ 11 files changed, 321 insertions(+), 26 deletions(-) create mode 100644 treeherder/perf/migrations/0006_add_alert_summary_notes.py create mode 100644 ui/partials/perf/editnotesctrl.html create mode 100644 ui/vendor/ng-text-truncate.js diff --git a/tests/webapp/api/test_performance_alertsummary_api.py b/tests/webapp/api/test_performance_alertsummary_api.py index a205c44ce..c7a2c975c 100644 --- 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', diff --git a/treeherder/perf/migrations/0006_add_alert_summary_notes.py b/treeherder/perf/migrations/0006_add_alert_summary_notes.py new file mode 100644 index 000000000..17e2cb5f3 --- /dev/null +++ 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), + ), + ] diff --git a/treeherder/perf/models.py b/treeherder/perf/models.py index d9c61d582..750c83f13 100644 --- 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 diff --git a/treeherder/webapp/api/performance_serializers.py b/treeherder/webapp/api/performance_serializers.py index 1169d87b3..62c6bc36f 100644 --- 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): diff --git a/ui/css/perf.css b/ui/css/perf.css index 48c6143e7..f5a1e98b4 100644 --- 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; diff --git a/ui/js/controllers/perf/alerts.js b/ui/js/controllers/perf/alerts.js index 0087c6ad0..63f15d97c 100644 --- 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 diff --git a/ui/js/models/perf/alerts.js b/ui/js/models/perf/alerts.js index 67c9689f3..13d4f1896 100644 --- 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 diff --git a/ui/js/perf.js b/ui/js/perf.js index 5267cb912..242d36377 100644 --- 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 ]); diff --git a/ui/partials/perf/alertsctrl.html b/ui/partials/perf/alertsctrl.html index f3417d94b..b394cdaec 100644 --- a/ui/partials/perf/alertsctrl.html +++ b/ui/partials/perf/alertsctrl.html @@ -88,6 +88,12 @@
  • Unlink from bug
    • +
  • + + Add notes + Edit notes + +
  • Re-open
    • @@ -195,31 +201,40 @@

    -
    - - - - - - - + + + + + + +
    +
    +

    +

    diff --git a/ui/partials/perf/editnotesctrl.html b/ui/partials/perf/editnotesctrl.html new file mode 100644 index 000000000..1ec4a5433 --- /dev/null +++ b/ui/partials/perf/editnotesctrl.html @@ -0,0 +1,28 @@ +

    +
    +

    {{title}}

    + +
    +
    +
    + +
    +
    +
    + +
    +
    diff --git a/ui/vendor/ng-text-truncate.js b/ui/vendor/ng-text-truncate.js new file mode 100644 index 000000000..85ae4e11b --- /dev/null +++ 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( "" + + $scope.text.substr( 0, threshould ) + + "..." + + "" + + " " + ($scope.customMoreLabel ? $scope.customMoreLabel : "More") + + "" + + "" + + $scope.text.substring( threshould ) + + "" + + " " + ($scope.customLessLabel ? $scope.customLessLabel : "Less") + + "" + + "" + + "" ); + $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( "" + + splitText.slice( 0, threshould ).join( " " ) + " " + + "..." + + "" + + " " + ($scope.customMoreLabel ? $scope.customMoreLabel : "More") + + "" + + "" + + splitText.slice( threshould, splitText.length ).join( " " ) + + "" + + " " + ($scope.customLessLabel ? $scope.customLessLabel : "Less") + + "" + + "" + + "" ); + $compile( el )( $scope ); + $element.append( el ); + + } else { + $element.append( splitText.slice( 0, threshould ).join( " " ) + "..." ); + } + } + }; + }]); +}));