Correct comment behavior with optional notifications

This allows submitting comments with and without notification
for the created journal.

Using hyperagent resource on activities, the resource `addComment` is
replaced when executing `fetch` on the link.
This breaks subsequent comment requests. This is fixed by requesting
manually on the resource link using `$http`.

This commit also provides the functionality required to comment with or
without notification propagation, and changes the backend to comply to
the APIv3 specification, which defines comments to be sent as a JSON
`{ comment: { raw: 'my comment' } }`.

This commit also provides:

* Displaying the comment input on overview depending on sort setting
* Disable inplace controls when comment is empty
* correctly hiding the comment based on link availability

app/assets/stylesheets/content/_in_place_editing.sass

@@ -29,6 +29,8 @@
 $inplace-edit--border-color: #ddd
 $inplace-edit--dark-background: #f8f8f8
 $inplace-edit--color--very-dark: #cacaca
+$inplace-edit--color--disabled: #999
+$inplace-edit--bg-color--disabled: #eee
 $inplace-edit--color-highlight: $primary-color
 $inplace-edit--selected-date-border-color: $primary-color-dark
 
@@ -278,6 +280,15 @@ a.inplace-editing--trigger-link,
     color: #888
     font-size: 1rem
 
+  // Disabled comment submit styles when empty
+  .inplace-edit--control--save[disabled],
+  .inplace-edit--control--send[disabled]
+    background-color: $inplace-edit--bg-color--disabled
+
+    span
+      color: $inplace-edit--color--disabled
+      cursor: not-allowed
+
 // this aligns the title for the WP
 .work-packages--details--title
   margin-left: -0.375rem

config/locales/js-en.yml

@@ -333,6 +333,7 @@ en:
       image: "Image"
     work_packages:
       button_clear: "Clear"
+      comment_send_failed: "An error has occured. Could not submit the comment."
       description_filter: "Filter"
       description_enter_text: "Enter text"
       description_options_hide: "Hide options"

doc/apiv3-documentation.apib

@@ -4542,7 +4542,7 @@ Gets a list of revisions that are linked to this work package, e.g., because it
             }
 
 
-## Work Package activities [/api/v3/work_packages/{id}/activities]
+## Work Package activities [/api/v3/work_packages/{id}/activities{?notify}]
 
 + Model
     + Body
@@ -4636,6 +4636,10 @@ updated activity.
 + Parameters
     + id (required, integer, `1`) ... Work package id
 
+    + notify = `true` (optional, boolean, `false`) ... Indicates whether change notifications (e.g. via E-Mail) should be sent.
+    Note that this controls notifications for all users interested in changes to the work package (e.g. watchers, author and assignee),
+    not just the current user.
+
 + Request (application/json)
 
         {

frontend/app/services/activity-service.js

@@ -29,16 +29,14 @@
 module.exports = function(HALAPIResource, $http, PathHelper){
 
   var ActivityService = {
-    createComment: function(workPackage, activities, descending, comment) {
+    createComment: function(workPackage, activities, comment, notify) {
-      var options = {
-        ajax: {
-          method: "POST",
-          data: JSON.stringify({ comment: comment }),
-          contentType: "application/json; charset=utf-8"
-        }
-      };
 
-      return workPackage.links.addComment.fetch(options);
+      return $http({
+        url: URI(workPackage.links.addComment.url()).addSearch('notify', notify).toString(),
+        method: 'POST',
+        data: JSON.stringify({ comment: comment }),
+        headers: { 'Content-Type': 'application/json; charset=UTF-8' }
+      });
     },
 
     updateComment: function(activity, comment) {

frontend/app/templates/work_packages/comment_field.html

@@ -1,4 +1,4 @@
-<div class="work-package-field work-packages--activity--add-comment">
+<div class="work-package-field work-packages--activity--add-comment" ng-show="fieldController.canAddComment">
   <inplace-editor-main-pane>
     <div class="inplace-edit--read" ng-if="!fieldController.isEditing">
       <accessible-by-keyboard
@@ -8,7 +8,7 @@
         execute="fieldController.startEditing()">
         <span class="inplace-edit--read-value"
               ng-class="{'-default': fieldController.isEmpty()}">
-          <span ng-bind-html="fieldController.placeholder"></span>
+          <span ng-bind="fieldController.placeholder"></span>
         </span>
         <span class="inplace-edit--icon-wrapper">
             <icon-wrapper icon-name="edit" icon-title="{{ fieldController.editTitle }}">
@@ -22,11 +22,15 @@
         </div>
         <div class="inplace-edit--dashboard">
           <div class="inplace-edit--controls" ng-hide="fieldController.state.isBusy || !fieldController.isActive()">
-            <accessible-by-keyboard execute="fieldController.submit(false)" class="inplace-edit--control inplace-edit--control--save">
+            <accessible-by-keyboard execute="fieldController.submit(false)"
+                                    ng-disabled="fieldController.isEmpty()"
+                                    class="inplace-edit--control inplace-edit--control--save">
               <icon-wrapper icon-name="yes" icon-title="{{ fieldController.saveTitle }}">
               </icon-wrapper>
             </accessible-by-keyboard>
-            <accessible-by-keyboard execute="fieldController.submit(true)" class="inplace-edit--control -icons-2 inplace-edit--control--send">
+            <accessible-by-keyboard execute="fieldController.submit(true)"
+                                    ng-disabled="fieldController.isEmpty()"
+                                    class="inplace-edit--control -icons-2 inplace-edit--control--send">
               <span title="{{ fieldController.saveAndSendTitle }}">
                 <i class="icon-yes"></i>
                 <i class="icon-mail"></i>

frontend/app/templates/work_packages/inplace_editor/custom/editable/wiki_textarea.html

@@ -7,7 +7,7 @@
             name="value"
             ng-disabled="fieldController.state.isBusy"
             ng-required="fieldController.isRequired"
-            ng-model="$parent.fieldController.writeValue.raw"
+            ng-model="fieldController.writeValue.raw"
             title="{{ fieldController.editTitle }}">
     </textarea>
     <div class="inplace-edit--preview" ng-if="customEditorController.isPreview && !fieldController.state.isBusy">

frontend/app/templates/work_packages/tabs/activity.html

@@ -1,7 +1,6 @@
 <exclusive-edit>
-  <div ng-if="activitiesSortedInDescendingOrder === true">
+  <div ng-show="activitiesSortedInDescendingOrder === true">
     <work-package-comment work-package="workPackage"
-                          activities="activities"
                           autocomplete-path="{{ autocompletePath }}">
     </work-package-comment>
   </div>
@@ -28,9 +27,8 @@
       </li>
     </ul>
   </div>
-  <div ng-if="activitiesSortedInDescendingOrder === false">
+  <div ng-show="activitiesSortedInDescendingOrder === false">
     <work-package-comment work-package="workPackage"
-                          activities="activities"
                           autocomplete-path="{{ autocompletePath }}">
     </work-package-comment>
   </div>

frontend/app/templates/work_packages/tabs/overview.html

@@ -57,6 +57,11 @@
   </div>
 
   <exclusive-edit>
+    <div ng-show="activitiesSortedInDescendingOrder === true">
+      <work-package-comment work-package="workPackage"
+                            autocomplete-path="{{ autocompletePath }}">
+      </work-package-comment>
+    </div>
     <ul class='work-package-details-activities-list'>
       <li ng-repeat="activity in activities | latestItems:activitiesSortedInDescendingOrder:3"
           class="work-package-details-activities-activity"
@@ -72,9 +77,10 @@
         </activity-entry>
       </li>
     </ul>
-    <work-package-comment work-package="workPackage"
+    <div ng-show="activitiesSortedInDescendingOrder === false">
-                          activities="activities"
+      <work-package-comment work-package="workPackage"
-                          autocomplete-path="{{ autocompletePath }}">
+                            autocomplete-path="{{ autocompletePath }}">
-    </work-package-comment>
+      </work-package-comment>
+    </div>
   </exclusive-edit>
 </div>

frontend/app/ui_components/index.js

@@ -31,14 +31,6 @@ angular.module('openproject.uiComponents')
     './accessible-by-keyboard-directive')])
   .directive('accessibleCheckbox', [require('./accessible-checkbox-directive')])
   .directive('accessibleElement', [require('./accessible-element-directive')])
-  .directive('activityComment', [
-    '$timeout',
-    'I18n',
-    'ActivityService',
-    'ConfigurationService',
-    'AutoCompleteHelper',
-    require('./activity-comment-directive')
-  ])
   .directive('authoring', ['I18n', 'PathHelper', 'TimezoneService', require(
     './authoring-directive')])
   .directive('backUrl', [require('./back-url-directive')])

frontend/app/work_packages/directives/index.js

@@ -67,6 +67,7 @@ angular.module('openproject.workPackages.directives')
     'ActivityService',
     'ConfigurationService',
     'AutoCompleteHelper',
+    'NotificationsService',
     require('./work-package-comment-directive')
   ])
   .directive('workPackageField', require('./work-package-field-directive'))

frontend/app/work_packages/directives/inplace_editor/inplace-editor-main-pane-directive.js

@@ -30,7 +30,7 @@ module.exports = function() {
   return {
     transclude: true,
     replace: true,
-    scope: {},
+    scope: false,
     templateUrl: '/templates/work_packages/inplace_editor/main_pane.html',
     controller: function($scope, $timeout) {
       // controller is invoked before linker
@@ -51,8 +51,5 @@ module.exports = function() {
       });
     },
     controllerAs: 'mainPaneController',
-    link: function(scope, element, attrs, fieldController) {
-      scope.fieldController = fieldController;
-    }
   };
 };

frontend/app/work_packages/directives/work-package-comment-directive.js

@@ -34,19 +34,24 @@ module.exports = function(
   I18n,
   ActivityService,
   ConfigurationService,
-  AutoCompleteHelper) {
+  AutoCompleteHelper,
+  NotificationsService) {
 
   function commentFieldDirectiveController($scope, $element) {
     var ctrl = this;
     ctrl.state = EditableFieldsState;
     ctrl.field = 'activity-comment';
-    ctrl.state.isBusy = false;
+
-    ctrl.isEditing = ctrl.state.forcedEditState;
     ctrl.editTitle = I18n.t('js.inplace.button_edit', { attribute: I18n.t('js.label_comment') });
     ctrl.placeholder = I18n.t('js.label_add_comment_title');
+    ctrl.title = I18n.t('js.label_add_comment_title');
+
+    ctrl.state.isBusy = false;
+    ctrl.isEditing = ctrl.state.forcedEditState;
+    ctrl.canAddComment = !!ctrl.workPackage.links.addComment;
 
     ctrl.isEmpty = function() {
-      return WorkPackageFieldService.isEmpty(EditableFieldsState.workPackage, ctrl.field);
+      return ctrl.writeValue === undefined || !ctrl.writeValue.raw;
     };
 
     ctrl.isEditable = function() {
@@ -54,27 +59,30 @@ module.exports = function(
     };
 
     ctrl.submit = function(notify) {
-      if (ctrl.writeValue === undefined) {
+      if (ctrl.isEmpty()) {
-        /** Error handling */
         return;
       }
 
+      ctrl.state.isBusy = true;
       ActivityService.createComment(
-        $scope.workPackage,
+        ctrl.workPackage,
-        $scope.activities,
+        ctrl.writeValue,
-        ConfigurationService.commentsSortedInDescendingOrder(),
+        notify
-        ctrl.writeValue.raw
       ).then(function(response) {
         $scope.$emit('workPackageRefreshRequired', '');
         ctrl.discardEditing();
         return response;
       }, function(error) {
-        console.log(error);
+        NotificationsService.addError(I18n.t('js.comment_send_failed'))
+        ctrl.state.isBusy = false;
       });
-    }
+    };
 
-    ctrl.startEditing = function() {
+    ctrl.startEditing = function(writeValue) {
       ctrl.isEditing = true;
+      ctrl.writeValue = writeValue || { raw: '' };
+      ctrl.markActive();
+
       $timeout(function() {
         var inputElement = $element.find('.focus-input');
         FocusHelper.focus(inputElement);
@@ -89,8 +97,9 @@ module.exports = function(
     };
 
     ctrl.discardEditing = function() {
-      ctrl.isEditing = false;
       delete ctrl.writeValue;
+      ctrl.isEditing = false;
+      ctrl.state.isBusy = false;
     };
 
     ctrl.isActive = function() {
@@ -123,20 +132,11 @@ module.exports = function(
     bindToController: true,
     templateUrl: '/templates/work_packages/comment_field.html',
     scope: {
-      workPackage: '=',
+      workPackage: '='
-      activities: '='
     },
     controller: commentFieldDirectiveController,
     link: function(scope, element, attrs, exclusiveEditController) {
-      exclusiveEditController.setCreator(scope);
+      exclusiveEditController.setCreator(scope.fieldController);
-
-      // TODO: WorkPackage is not applied from attribute scope?
-      scope.workPackage = scope.$parent.workPackage;
-      scope.title = I18n.t('js.label_add_comment_title');
-      scope.buttonTitle = I18n.t('js.label_add_comment');
-      scope.buttonCancel = I18n.t('js.button_cancel');
-      scope.canAddComment = !!scope.workPackage.links.addComment;
-      scope.activity = { comment: '' };
 
       $timeout(function() {
         AutoCompleteHelper.enableTextareaAutoCompletion(

frontend/app/work_packages/tabs/exclusive-edit-directive.js

@@ -51,8 +51,7 @@ module.exports = function() {
       };
 
       this.quoteComment = function(text) {
-        creator.fieldController.writeValue = { raw: text };
+        creator.startEditing({ raw: text });
-        creator.fieldController.startEditing();
       };
     }
   };

frontend/tests/unit/tests/ui_components/activity-comment-directive-test.js

@@ -63,6 +63,8 @@ describe('activityCommentDirective', function() {
       scope.$digest();
     };
 
+    workPackageFieldService.isEmpty = sinon.stub().returns(true);
+
     ActivityService = _ActivityService_;
     var createComments = sinon.stub(ActivityService, 'createComment');
     commentCreation = q.defer();
@@ -98,7 +100,7 @@ describe('activityCommentDirective', function() {
       });
 
       it('should not display the comments form', function() {
-        expect(element.find('.activity-comment').length).to.equal(0);
+        expect(element.find('.work-packages--activity--add-comment').hasClass('ng-hide')).to.equal(true);
       });
     });
 
@@ -116,50 +118,32 @@ describe('activityCommentDirective', function() {
         expect(commentSection.length).to.equal(1);
       });
 
-      it('should provide a label next to the comments field', function() {
+      describe('when clicking the inplace edit' function() {
-        var label = commentSection.find('label[for=' + commentField.attr('id') + ']');
+        beforeEach(function() {
+          element.find('.work-packages--activity--add-comment .inplace-edit--write-value').click();
+        });
 
-        expect(label.text().trim()).to.equal('trans_title');
+        it('should provide a label next to the comments field', function() {
-      });
+          var label = commentSection.find('label[for=' + commentField.attr('id') + ']');
 
-      it('should display a placeholder in the comments field', function() {
+          expect(label.text().trim()).to.equal('trans_title');
-        expect(commentField.attr('placeholder')).to.equal('trans_title');
+        });
-      });
 
-      it('does not allow sending comment with an empty message', function() {
+        it('should display a placeholder in the comments field', function() {
-        var saveButton = commentSection.find('button');
+          expect(commentField.attr('placeholder')).to.equal('trans_title');
+        });
 
-        commentField.val('');
+        it('does not allow sending comment with an empty message', function() {
-        commentField.change();
+          var saveButton = commentSection.find('button');
-        expect(saveButton.prop('disabled')).to.be.true;
 
-        commentField.val('a useful comment');
+          commentField.val('');
-        commentField.change();
+          commentField.change();
-        expect(saveButton.prop('disabled')).to.be.false;
+          expect(saveButton.prop('disabled')).to.be.true;
-      });
 
-      it('does prevent double posts', function() {
+          commentField.val('a useful comment');
-        var saveButton = commentSection.find('button');
+          commentField.change();
-
+          expect(saveButton.prop('disabled')).to.be.false;
-        // comments can be saved when there is text to post
+        });
-        commentField.val('a useful comment');
-        commentField.change();
-        expect(saveButton.prop('disabled')).to.be.false;
-
-        // while sending the comment, one cannot send another comment
-        saveButton.click();
-        expect(saveButton.scope().$parent.processingComment).to.be.true;
-        expect(saveButton.scope().$parent.activity.comment).to.equal('a useful comment');
-        expect(commentField.prop('disabled')).to.be.true;
-        expect(saveButton.prop('disabled')).to.be.true;
-
-        // after sending, we can send comments again
-        commentCreation.resolve();
-        scope.$digest();
-        expect(saveButton.scope().$parent.processingComment).to.be.false;
-        expect(saveButton.scope().$parent.activity.comment).to.equal('');
-        expect(commentField.val()).to.equal('');
-        expect(commentField.prop('disabled')).to.be.false;
       });
     });
   });

lib/api/v3/activities/activities_by_work_package_api.rb

@@ -34,10 +34,15 @@ module API
       class ActivitiesByWorkPackageAPI < ::API::OpenProjectAPI
         resource :activities do
           helpers do
-            def save_work_package(work_package)
+            def save_work_package(work_package, send_notifications)
-              if work_package.save
+              update_service = UpdateWorkPackageService.new(
-                journals = ::Journal::AggregatedJournal.aggregated_journals(
+                user: current_user,
-                  journable: work_package)
+                work_package: work_package,
+                send_notifications: send_notifications
+              )
+
+              if update_service.save
+                journals = ::Journal::AggregatedJournal.aggregated_journals(journable: work_package)
                 Activities::ActivityRepresenter.new(journals.last, current_user: current_user)
               else
                 fail ::API::Errors::ErrorBase.create_and_merge_errors(work_package.errors)
@@ -54,17 +59,17 @@ module API
           end
 
           params do
-            requires :comment, type: String
+            requires :comment, type: Hash
           end
           post do
-            authorize({ controller: :journals, action: :new },
+            authorize({ controller: :journals, action: :new }, context: @work_package.project) do
-                      context: @work_package.project) do
               raise ::API::Errors::NotFound.new
             end
 
-            @work_package.journal_notes = params[:comment]
+            @work_package.journal_notes = params[:comment][:raw]
 
-            save_work_package(@work_package)
+            send_notifications = !(params.has_key?(:notify) && params[:notify] == 'false')
+            save_work_package(@work_package, send_notifications)
           end
         end
       end