Merge pull request #4382 from oliverguenther/fix/23069/wp-context-menu

Fix invalid links to work package actions

frontend/app/components/context-menus/wp-context-menu/wp-context-menu.service.html

@@ -22,7 +22,7 @@
         class="{{action.icon}}">
       <a role="menuitem" href="" ng-click="triggerContextMenuAction(action.icon, action.link)">
         <i ng-class="['icon-action-menu', 'icon-' + action.icon]"></i>
-        <span ng-bind="I18n.t('js.button_' + action.icon)"/>
+        <span ng-bind="action.text"/>
       </a>
     </li>
   </ul>

frontend/app/components/context-menus/wp-context-menu/wp-context-menu.service.test.ts

@@ -83,9 +83,10 @@ describe('workPackageContextMenu', () => {
       update: '/work_packages/123/edit',
       move: '/work_packages/move/new?ids%5B%5D=123',
     };
-    var workPackage = Factory.build('PlanningElement', {
+    var workPackage = Factory.build('PlanningElement');
-      $links: actionLinks
+    workPackage.$source = { _links: actionLinks };
-    });
+    workPackage.$links = actionLinks;
+
     var directListElements;
 
     beforeEach(angular.mock.inject((_I18n_) => {
@@ -125,9 +126,9 @@ describe('workPackageContextMenu', () => {
       var actionLinks = {
         'delete': '/work_packages/bulk',
       };
-      var workPackage = Factory.build('PlanningElement', {
+      var workPackage = Factory.build('PlanningElement');
-        $links: actionLinks
+      workPackage.$source = { _links: actionLinks };
-      });
+      workPackage.$links = actionLinks;
 
       beforeEach(() => {
         $rootScope.rows = [];

frontend/app/components/wp-table/context-menu-helper/wp-context-menu-helper.service.js

@@ -26,17 +26,24 @@
 // See doc/COPYRIGHT.rdoc for more details.
 //++
 
-module.exports = function(PERMITTED_BULK_ACTIONS, WorkPackagesTableService, UrlParamsHelper) {
+angular
+  .module('openproject.workPackages.helpers')
+  .factory('WorkPackageContextMenuHelper', WorkPackageContextMenuHelper);
+
+function WorkPackageContextMenuHelper(PERMITTED_BULK_ACTIONS, WorkPackagesTableService, HookService, UrlParamsHelper) {
+
   function getPermittedActionLinks(workPackage, permittedActionConstants) {
     var singularPermittedActions = [];
 
-    var allowedActions = getAllowedActions(workPackage.$links, permittedActionConstants);
+    var allowedActions = getAllowedActions(workPackage, permittedActionConstants);
 
     angular.forEach(allowedActions, function(allowedAction) {
       singularPermittedActions.push({
         icon: allowedAction.icon,
+        text: allowedAction.text,
         link: workPackage
-                                              .$links[allowedAction.link]
+          .$source
+          ._links[allowedAction.link]
           .href
       });
     });
@@ -49,12 +56,13 @@ module.exports = function(PERMITTED_BULK_ACTIONS, WorkPackagesTableService, UrlP
 
     var permittedActions = _.filter(PERMITTED_BULK_ACTIONS, function(action) {
       return _.every(workPackages, function(workPackage) {
-        return getAllowedActions(workPackage.$links, [action]).length === 1;
+        return getAllowedActions(workPackage, [action]).length >= 1;
       });
     });
     angular.forEach(permittedActions, function(permittedAction) {
       bulkPermittedActions.push({
         icon: permittedAction.icon,
+        text: permittedAction.text,
         link: getBulkActionLink(permittedAction,
           workPackages)
       });
@@ -80,20 +88,28 @@ module.exports = function(PERMITTED_BULK_ACTIONS, WorkPackagesTableService, UrlP
     return link + '?' + queryParts.join('&');
   }
 
-  function getAllowedActions(links, actions) {
+  function getAllowedActions(workPackage, actions) {
     var allowedActions = [];
 
     angular.forEach(actions, function(action) {
-      if (links.hasOwnProperty(action.link)) {
+      if (workPackage.$links.hasOwnProperty(action.link)) {
+        action.text = I18n.t('js.button_' + action.icon);
         allowedActions.push(action);
       }
     });
 
+    angular.forEach(HookService.call('workPackageTableContextMenu'), function(action) {
+      if (workPackage.$links.hasOwnProperty(action.link)) {
+        var index = action.indexBy ? action.indexBy(allowedActions) : allowedActions.length;
+        allowedActions.splice(index, 0, action)
+      }
+    });
+
     return allowedActions;
   }
 
   var WorkPackageContextMenuHelper = {
-    getPermittedActions: function(workPackages, permittedActionConstants) {
+    getPermittedActions: function (workPackages, permittedActionConstants) {
       if (workPackages.length === 1) {
         return getPermittedActionLinks(workPackages[0], permittedActionConstants);
       } else if (workPackages.length > 1) {
@@ -103,4 +119,4 @@ module.exports = function(PERMITTED_BULK_ACTIONS, WorkPackagesTableService, UrlP
   };
 
   return WorkPackageContextMenuHelper;
-};
+}

frontend/app/work_packages/helpers/index.js

@@ -49,10 +49,6 @@ angular.module('openproject.workPackages.helpers')
       link: 'delete'
     }
   ])
-  .service('WorkPackageContextMenuHelper', ['PERMITTED_BULK_ACTIONS',
-    'WorkPackagesTableService', 'UrlParamsHelper', require(
-      './work-package-context-menu-helper')
-  ])
   .factory('WorkPackagesHelper', ['TimezoneService', 'currencyFilter',
     'CustomFieldHelper', require('./work-packages-helper')
   ])

frontend/tests/unit/tests/work_packages/helpers/work-package-context-menu-helper-test.js

@@ -102,9 +102,9 @@ describe('WorkPackageContextMenuHelper', function() {
       }
     };
 
-    var workPackage = Factory.build('PlanningElement', {
+    var workPackage = Factory.build('PlanningElement');
-      $links: actionLinks
+    workPackage.$source = { _links : actionLinks };
-    });
+    workPackage.$links = actionLinks;
 
     describe('when an array with a single work package is passed as an argument', function() {
       var workPackages = new Array(workPackage);
@@ -126,13 +126,15 @@ describe('WorkPackageContextMenuHelper', function() {
     });
 
     describe('when more than one work package is passed as an argument', function() {
-      var anotherWorkPackage = Factory.build('PlanningElement', {
+      var anotherWorkPackage = Factory.build('PlanningElement');
-            $links: {
+      anotherWorkPackage.$source = {
+        _links: {
           update: {
             href: '/work_packages/234/edit'
           }
         }
-          });
+      };
+      anotherWorkPackage.$links = { update: '/work_packages/234/edit' };
       var workPackages = [anotherWorkPackage, workPackage];
 
       beforeEach(inject(function(_WorkPackagesTableService_) {

spec/features/work_packages/table/context_menu_spec.rb

@@ -0,0 +1,86 @@
+require 'spec_helper'
+
+describe 'Work package table context menu', js: true do
+  let(:user) { FactoryGirl.create(:admin) }
+  let(:work_package) { FactoryGirl.create(:work_package) }
+
+  let(:wp_table) { Pages::WorkPackagesTable.new }
+  let(:menu) { Components::WorkPackagesContextMenu.new }
+
+  def goto_context_menu
+    # Go to table
+    wp_table.visit!
+    wp_table.expect_work_package_listed(work_package)
+
+    # Open context menu
+    menu.expect_closed
+    menu.open_for(work_package)
+  end
+
+  before do
+    login_as(user)
+    work_package
+  end
+
+  it 'provides a context menu for a single work package' do
+    # Open detail pane
+    goto_context_menu
+    menu.choose('Open details view')
+    expect(page).to have_selector('#work-package-subject', text: work_package.subject)
+
+    # Open full view
+    goto_context_menu
+    menu.choose('Open fullscreen view')
+    expect(page).to have_selector('.work-packages--show-view #work-package-subject',
+                                  text: work_package.subject)
+
+    # Open edit link
+    goto_context_menu
+    menu.choose('Edit')
+    expect(page).to have_selector('#inplace-edit--write-value--subject')
+    find('#work-packages--edit-actions-cancel').click
+    expect(page).to have_no_selector('#inplace-edit--write-value--subject')
+
+    # Open log time
+    goto_context_menu
+    menu.choose('Log time')
+    expect(page).to have_selector('h2', text: I18n.t(:label_spent_time))
+
+    # Open Move
+    goto_context_menu
+    menu.choose('Move')
+    expect(page).to have_selector('h2', text: I18n.t(:button_move))
+    expect(page).to have_selector('a.issue', text: "##{work_package.id}")
+
+    # Open Copy
+    goto_context_menu
+    menu.choose('Copy')
+    expect(page).to have_selector('h2', text: I18n.t(:button_copy))
+    expect(page).to have_selector('a.issue', text: "##{work_package.id}")
+
+    # Open Delete
+    goto_context_menu
+    menu.choose('Delete')
+    wp_table.dismiss_alert_dialog!
+  end
+
+  context 'multiple selected' do
+    let!(:work_package2) { FactoryGirl.create(:work_package) }
+
+    before do
+      # Go to table
+      wp_table.visit!
+      wp_table.expect_work_package_listed(work_package)
+      wp_table.expect_work_package_listed(work_package2)
+
+      # Select both
+      all('td.checkbox input').each { |el| el.set(true) }
+    end
+
+    it 'shows a subset of the available menu items' do
+      menu.open_for(work_package)
+      menu.expect_options ['Open details view', 'Open fullscreen view',
+                           'Edit', 'Copy', 'Move', 'Delete']
+    end
+  end
+end

spec/support/components/work_packages_context_menu.rb

@@ -0,0 +1,64 @@
+#-- copyright
+# OpenProject is a project management system.
+# Copyright (C) 2012-2015 the OpenProject Foundation (OPF)
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See doc/COPYRIGHT.rdoc for more details.
+#++
+
+module Components
+  class WorkPackagesContextMenu
+    include Capybara::DSL
+    include RSpec::Matchers
+
+    def open_for(work_package)
+      find("#work-package-#{work_package.id}").right_click
+      expect_open
+    end
+
+    def expect_open
+      expect(page).to have_selector(selector)
+    end
+
+    def expect_closed
+      expect(page).to have_no_selector(selector)
+    end
+
+    def choose(target)
+      find("#{selector} a", text: target).click
+    end
+
+    def expect_options(options)
+      expect_open
+      options.each do |text|
+        expect(page).to have_selector("#{selector} a", text: text)
+      end
+    end
+
+    private
+
+    def selector
+      '#work-package-context-menu'
+    end
+  end
+end