From 9820bc36f17675d0ba59dbcb78cccaf84c7b4d20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Thu, 3 May 2018 12:14:18 +0200 Subject: [PATCH 1/4] [27576] Conflict-check only main wiki items from same project (#6294) The wiki renaming feature includes a check for duplicate menu items since https://github.com/opf/openproject/pull/4866. However this check tests for ALL wiki menu items in all projects, while we only need to check for items in the current project https://community.openproject.com/wp/27576 [ci skip] --- app/controllers/wiki_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/wiki_controller.rb b/app/controllers/wiki_controller.rb index 2ed82f7b09..868f657c5f 100644 --- a/app/controllers/wiki_controller.rb +++ b/app/controllers/wiki_controller.rb @@ -256,7 +256,7 @@ class WikiController < ApplicationController def wiki_root_menu_items MenuItems::WikiMenuItem - .where(parent_id: nil) + .main_items(@wiki.id) .map { |it| OpenStruct.new name: it.name, caption: it.title, item: it } end From bf957d2d4a45eef5ac8ee3c4b328f7cbfae9081c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Thu, 3 May 2018 14:38:09 +0200 Subject: [PATCH 2/4] [27457] Lift time_entry 1k hours restriction (#6295) https://community.openproject.com/wp/27457 [ci skip] --- app/models/time_entry.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/time_entry.rb b/app/models/time_entry.rb index 1f23cab48e..09c7b50cee 100644 --- a/app/models/time_entry.rb +++ b/app/models/time_entry.rb @@ -126,7 +126,7 @@ class TimeEntry < ActiveRecord::Base private def validate_hours_are_in_range - errors.add :hours, :invalid if hours && (hours < 0 || hours >= 1000) + errors.add :hours, :invalid if hours && hours < 0 end def validate_project_is_set From d591e8ae4c3b508b8feddaa24e7898e2c29c9574 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Thu, 3 May 2018 15:45:05 +0200 Subject: [PATCH 3/4] [27600] Add link to query_form to represent allowing new queries to be saved (#6293) When an existing query (and its form) is loaded, members with permissions to create new queries, but NOT to update existing ones can not save existing queries as new ones. This is due to the `create`, `update` and `commit` (on form) links are dependent on the query resource itself, not on general permissions of the queries project. https://community.openproject.com/wp/27600 [ci skip] --- app/policies/query_policy.rb | 8 +- .../settings-menu/settings-menu.controller.ts | 10 +- .../settings-menu/settings-menu.service.html | 4 +- .../options-dropdown-menu-controller-test.js | 411 ------------------ lib/api/v3/queries/form_representer.rb | 13 + 5 files changed, 30 insertions(+), 416 deletions(-) delete mode 100644 frontend/tests/unit/tests/work_packages/controllers/menus/options-dropdown-menu-controller-test.js diff --git a/app/policies/query_policy.rb b/app/policies/query_policy.rb index d83c8d28b5..bb583ae47e 100644 --- a/app/policies/query_policy.rb +++ b/app/policies/query_policy.rb @@ -36,6 +36,7 @@ class QueryPolicy < BasePolicy update: persisted_and_own_or_public?(cached_query), destroy: persisted_and_own_or_public?(cached_query), create: create_allowed?(cached_query), + create_new: create_new_allowed?(cached_query), publicize: publicize_allowed?(cached_query), depublicize: depublicize_allowed?(cached_query), star: persisted_and_own_or_public?(cached_query), @@ -58,8 +59,11 @@ class QueryPolicy < BasePolicy end def create_allowed?(query) - query.new_record? && - save_queries_allowed?(query) + query.new_record? && create_new_allowed?(query) + end + + def create_new_allowed?(query) + save_queries_allowed?(query) end def publicize_allowed?(query) diff --git a/frontend/app/components/context-menus/settings-menu/settings-menu.controller.ts b/frontend/app/components/context-menus/settings-menu/settings-menu.controller.ts index 62cc1a7d07..919935ecc6 100644 --- a/frontend/app/components/context-menus/settings-menu/settings-menu.controller.ts +++ b/frontend/app/components/context-menus/settings-menu/settings-menu.controller.ts @@ -164,7 +164,7 @@ function SettingsDropdownMenuController($scope:IMyScope, // Modals $scope.showSaveAsModal = function (event:JQueryEventObject) { event.stopPropagation(); - if (allowFormAction(event, 'commit')) { + if (allowFormAction(event, 'create_new')) { showExistingQueryModal.call(saveModal, event); updateFocusInModal('save-query-name'); } @@ -247,6 +247,14 @@ function SettingsDropdownMenuController($scope:IMyScope, return AuthorisationService.cannot('query', 'updateImmediately'); }; + $scope.showSaveAsModalInvalid = function () { + if (form) { + return !!form.$links.create_new; + } + + return AuthorisationService.cannot('query', 'updateImmediately'); + }; + $scope.saveQueryInvalid = function () { return AuthorisationService.cannot('query', 'updateImmediately'); }; diff --git a/frontend/app/components/context-menus/settings-menu/settings-menu.service.html b/frontend/app/components/context-menus/settings-menu/settings-menu.service.html index c596dde54c..3471c6c162 100644 --- a/frontend/app/components/context-menus/settings-menu/settings-menu.service.html +++ b/frontend/app/components/context-menus/settings-menu/settings-menu.service.html @@ -59,8 +59,8 @@ {{ I18n.t('js.toolbar.settings.save') }}
  • + inaccessible-by-tab="showSaveAsModalInvalid()" + ng-class="{'inactive': showSaveAsModalInvalid()}"> {{ I18n.t('js.toolbar.settings.save_as') }}
  • diff --git a/frontend/tests/unit/tests/work_packages/controllers/menus/options-dropdown-menu-controller-test.js b/frontend/tests/unit/tests/work_packages/controllers/menus/options-dropdown-menu-controller-test.js deleted file mode 100644 index 5e90930c08..0000000000 --- a/frontend/tests/unit/tests/work_packages/controllers/menus/options-dropdown-menu-controller-test.js +++ /dev/null @@ -1,411 +0,0 @@ -//-- copyright -// OpenProject is a project management system. -// Copyright (C) 2012-2017 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-2017 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. -//++ - -/*jshint expr: true*/ - -var reactivestates = require("reactivestates"); - -describe('optionsDropdown Directive', function() { - var compile, - element, - rootScope, - scope, - I18n, - AuthorisationService, - wpTableSum, - wpTableGroupBy, - columnsModal, - sortByModal, - groupByModal, - exportModal, - wpTableHierarchies, - states, - query, - form; - - beforeEach(angular.mock.module('openproject.models', - 'openproject.workPackages', - 'openproject.api', - 'openproject.layout', - 'openproject.services')); - - beforeEach(angular.mock.module('openproject.templates', function($provide) { - wpTableSum = { - isEnabled: false - }; - - wpTableHierarchies = { - isEnabled: false - }; - - wpTableGroupBy = { - isEnabled: false - } - - columnsModal = {}; - sortByModal = {}; - groupByModal = {}; - exportModal = {}; - - query = { - id: 5 - }; - - var queryValues = { - takeUntil: function(condition) { - return { - subscribe: function(func) { - func(query); - } - } - } - }; - - form = {} - - var formValues = { - takeUntil: function(condition) { - return { - subscribe: function(func) { - func(form); - } - } - } - }; - - states = { - query: { - resource: { - values$: function() { - return queryValues; - } - }, - form: { - values$: function() { - return formValues; - } - } - - }, - table: { - timelineVisible: { - value: {} - }, - stopAllSubscriptions: [false] - } - }; - - $provide.constant('wpTableSum', wpTableSum); - $provide.constant('wpTableHierarchies', wpTableHierarchies); - $provide.constant('wpTableGroupBy', wpTableGroupBy); - $provide.constant('columnsModal', columnsModal); - $provide.constant('sortingModal', sortByModal); - $provide.constant('groupingModal', groupByModal); - $provide.constant('exportModal', exportModal); - $provide.constant('states', states); - })); - - beforeEach(inject(function($rootScope, $compile) { - var optionsDropdownHtml; - optionsDropdownHtml = '
    '; - - element = angular.element(optionsDropdownHtml); - rootScope = $rootScope; - scope = $rootScope.$new(); - compile = function() { - angular.element(document).find('body').append(element); - $compile(element)(scope); - element.find('button').click(); - scope.$apply(); - }; - })); - - beforeEach(inject(function(_AuthorisationService_, _I18n_){ - AuthorisationService = _AuthorisationService_; - - I18n = _I18n_; - - var stub = sinon.stub(I18n, 't'); - - stub.withArgs('js.label_save_as').returns('Save as'); - stub.withArgs('js.toolbar.settings.columns').returns('Columns ...'); - stub.withArgs('js.toolbar.settings.sort_by').returns('Sort by ...'); - stub.withArgs('js.toolbar.settings.group_by').returns('Group by ...'); - stub.withArgs('js.toolbar.settings.display_sums').returns('Display sums'); - stub.withArgs('js.toolbar.settings.hide_sums').returns('Hide sums'); - stub.withArgs('js.toolbar.settings.display_hierarchy').returns('Display hierarchy'); - stub.withArgs('js.toolbar.settings.hide_hierarchy').returns('Hide hierarchy'); - stub.withArgs('js.toolbar.settings.export').returns('Export ...'); - })); - - afterEach(function() { - I18n.t.restore(); - element.remove(); - }); - - describe('element', function() { - var getMenuItem = function(name) { - var menuLinks = element.find('a.menu-item'); - - return _.find(menuLinks, function(item) { - return angular.element(item).text().indexOf(name) !== -1 || - angular.element(item).find('span').text().indexOf(name) !== -1; - }); - } - - it('should render a div', function() { - expect(element.prop('tagName')).to.equal('DIV'); - }); - - describe('Columns', function() { - var getColumnsMenuItem = function() { - return getMenuItem(I18n.t('js.toolbar.settings.columns')); - } - - it('has a "Columns" menu item', function() { - compile(); - - var item = getColumnsMenuItem(); - - expect(!!item).to.be.true; - }); - - it('activates the columns modal on click', function() { - compile(); - - var item = getColumnsMenuItem(); - - var spy = sinon.spy(); - - columnsModal['activate'] = spy; - - angular.element(item).click(); - - expect(spy).to.have.been.calledWith(); - }); - }) - - describe('Sort by', function() { - var getSortByMenuItem = function() { - return getMenuItem(I18n.t('js.toolbar.settings.sort_by')); - }; - - it('has a "Sort by" menu item', function() { - compile(); - - var item = getSortByMenuItem(); - - expect(!!item).to.be.true; - }); - - it('activates the columns modal on click', function() { - compile(); - - var item = getSortByMenuItem(); - - var spy = sinon.spy(); - - sortByModal['activate'] = spy; - - angular.element(item).click(); - - expect(spy).to.have.been.calledWith(); - }); - }) - - describe('Group by', function() { - var getGroupByMenuItem = function() { - return getMenuItem(I18n.t('js.toolbar.settings.group_by')); - }; - - it('has a "Group by" menu item', function() { - compile(); - - var item = getGroupByMenuItem(); - - expect(!!item).to.be.true; - }); - - it('activates the columns modal on click', function() { - compile(); - - var item = getGroupByMenuItem(); - - var spy = sinon.spy(); - - groupByModal['activate'] = spy; - - angular.element(item).click(); - - expect(spy).to.have.been.calledWith(); - }); - }) - - describe('sums', function() { - var getSumsMenuItem = function() { - return getMenuItem(I18n.t('js.toolbar.settings.display_sums')); - } - - it('has a "sums" menu item', function() { - compile(); - - var item = getSumsMenuItem(); - - expect(!!item).to.be.true; - }); - - it('displays not sumed', function() { - compile(); - - var item = getSumsMenuItem(); - - expect(angular.element(item).find('.no-icon').length).to.eq(1); - }); - - it('displays sumed if the service tells it to', function() { - wpTableSum['isEnabled'] = true; - - compile(); - - var item = getSumsMenuItem(); - - expect(angular.element(item).find('.icon-checkmark').length).to.eq(1); - }); - - it('forwards to the service on click', function() { - compile(); - - var item = getSumsMenuItem(); - - var spy = sinon.spy(); - - wpTableSum['toggle'] = spy; - - angular.element(item).click(); - - expect(spy).to.have.been.calledWith(); - }); - }); - - describe('hierarchy', function() { - var getHierarchyMenuItem = function() { - return getMenuItem(I18n.t('js.toolbar.settings.display_hierarchy')); - } - - it('has a "hierarchy" menu item', function() { - compile(); - - var item = getHierarchyMenuItem(); - - expect(!!item).to.be.true; - }); - - it('displays not active', function() { - compile(); - - var item = getHierarchyMenuItem(); - - expect(angular.element(item).find('.icon-no-hierarchy').length).to.eq(1); - }); - - it('displays active if the service tells it to', function() { - wpTableHierarchies['isEnabled'] = true; - - compile(); - - // named differently if active - var item = getMenuItem(I18n.t('js.toolbar.settings.hide_hierarchy')); - - expect(angular.element(item).find('.icon-hierarchy').length).to.eq(1); - }); - - it('forwards to the service on click', function() { - compile(); - - var item = getHierarchyMenuItem(); - - var spy = sinon.spy(); - - wpTableHierarchies['setEnabled'] = spy; - - angular.element(item).click(); - - expect(spy).to.have.been.calledWith(true); - }); - }); - - describe('Export', function() { - var getExportMenuItem = function() { - return getMenuItem(I18n.t('js.toolbar.settings.export')); - } - - var authorisation; - - beforeEach(function() { - authorisation = sinon.stub(AuthorisationService, 'can'); - authorisation.returns(false); - }); - - afterEach(function() { - AuthorisationService.can.restore(); - }); - - it('has a "Export" menu item', function() { - compile(); - - var item = getExportMenuItem(); - - expect(!!item).to.be.true; - }); - - it('activates the export modal on click', function() { - authorisation.withArgs('work_packages', 'representations').returns(true); - - compile(); - - var item = getExportMenuItem(); - - var spy = sinon.spy(); - - exportModal['activate'] = spy; - - angular.element(item).click(); - - expect(spy).to.have.been.calledWith(); - }); - - it('is inactive when permissions are lacking', function() { - compile(); - - var item = getExportMenuItem(); - - expect(angular.element(item).filter('.inactive').length).to.eq(1); - }); - }) - }); -}); diff --git a/lib/api/v3/queries/form_representer.rb b/lib/api/v3/queries/form_representer.rb index 7731764b76..4d29fb4eaa 100644 --- a/lib/api/v3/queries/form_representer.rb +++ b/lib/api/v3/queries/form_representer.rb @@ -55,6 +55,15 @@ module API end end + link :create_new do + if allow_create_as_new? + { + href: api_v3_paths.queries, + method: :post + } + end + end + def commit_action raise NotImplementedError, "subclass responsibility" end @@ -89,6 +98,10 @@ module API def allow_save? QueryPolicy.new(current_user).allowed? represented, commit_action end + + def allow_create_as_new? + QueryPolicy.new(current_user).allowed? represented, :create_new + end end end end From 46563e1a70d682906b58fb6481451bb8b089d180 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 7 May 2018 11:07:23 +0200 Subject: [PATCH 4/4] [27462] Pass wiki title to new page if it does not exist (#6299) When creating a wiki page from the text with, e.g., `[[Foo Bar]]`, the user gets redirected to wiki#show for a page `foo-bar` (the slug of the title above). He thus loses the title attribute he entered previously. Instead we can pass the title to the new wiki page IFF the page does not exist. https://community.openproject.com/wp/27462 [ci skip] --- app/controllers/wiki_controller.rb | 4 ++-- lib/open_project/text_formatting.rb | 13 ++++++++++--- spec/features/security/angular_xss_spec.rb | 4 ++-- spec/lib/open_project/text_formatting_spec.rb | 6 +++--- spec_legacy/functional/wiki_controller_spec.rb | 2 +- spec_legacy/unit/helpers/application_helper_spec.rb | 2 +- 6 files changed, 19 insertions(+), 12 deletions(-) diff --git a/app/controllers/wiki_controller.rb b/app/controllers/wiki_controller.rb index 868f657c5f..3339fa4ede 100644 --- a/app/controllers/wiki_controller.rb +++ b/app/controllers/wiki_controller.rb @@ -135,7 +135,7 @@ class WikiController < ApplicationController if @page.new_record? if User.current.allowed_to?(:edit_wiki_pages, @project) && editable? edit - render action: 'edit' + render action: 'new' else render_404 end @@ -403,7 +403,7 @@ class WikiController < ApplicationController private def wiki_page_title - params[:id] + params[:title] || params[:id] end def find_wiki diff --git a/lib/open_project/text_formatting.rb b/lib/open_project/text_formatting.rb index 17a8be999c..5d3ac1718b 100644 --- a/lib/open_project/text_formatting.rb +++ b/lib/open_project/text_formatting.rb @@ -283,15 +283,22 @@ module OpenProject page = CGI.unescapeHTML(page) # check if page exists wiki_page = link_project.wiki.find_page(page) - wiki_title = wiki_page.nil? ? page : wiki_page.title + default_wiki_title = wiki_page.nil? ? page : wiki_page.title + wiki_title = title || default_wiki_title url = case options[:wiki_links] when :local; "#{title}.html" when :anchor; "##{title}" # used for single-file wiki export else wiki_page_id = wiki_page.nil? ? page.to_url : wiki_page.slug - url_for(only_path: only_path, controller: '/wiki', action: 'show', project_id: link_project, id: wiki_page_id, anchor: anchor) + url_for(only_path: only_path, + controller: '/wiki', + action: 'show', + project_id: link_project, + id: wiki_page_id, + title: wiki_page.nil? ? wiki_title.strip : nil, + anchor: anchor) end - link_to(h(title || wiki_title), url, class: ('wiki-page' + (wiki_page ? '' : ' new'))) + link_to(h(wiki_title), url, class: ('wiki-page' + (wiki_page ? '' : ' new'))) else # project or wiki doesn't exist all diff --git a/spec/features/security/angular_xss_spec.rb b/spec/features/security/angular_xss_spec.rb index a905718d21..32fab2ac61 100644 --- a/spec/features/security/angular_xss_spec.rb +++ b/spec/features/security/angular_xss_spec.rb @@ -116,7 +116,7 @@ describe 'Angular expression escaping', type: :feature do let(:content) { find '#content_text' } let(:preview) { find '#preview' } let(:btn_preview) { find '#wiki_form-preview' } - let(:btn_cancel) { find '#wiki_form a.button', text: I18n.t(:button_cancel) } + let(:btn_save) { find '.button.-highlight', text: I18n.t(:button_save) } before do login_as(user) @@ -130,7 +130,7 @@ describe 'Angular expression escaping', type: :feature do expect(preview.text).not_to include '{{ $root.DOUBLE_LEFT_CURLY_BRACE }}' expect(preview.text).to match /\{\{[\s\w]+\}\}/ - btn_cancel.click + btn_save.click end end diff --git a/spec/lib/open_project/text_formatting_spec.rb b/spec/lib/open_project/text_formatting_spec.rb index a3bfd53cca..4413df22e7 100644 --- a/spec/lib/open_project/text_formatting_spec.rb +++ b/spec/lib/open_project/text_formatting_spec.rb @@ -440,13 +440,13 @@ describe OpenProject::TextFormatting do context 'Wiki link to an unknown page' do subject { format_text('[[Unknown page]]') } - it { is_expected.to be_html_eql("

    Unknown page

    ") } + it { is_expected.to be_html_eql("

    Unknown page

    ") } end context 'Wiki page link to an unknown page' do subject { format_text('[[Unknown page|404]]') } - it { is_expected.to be_html_eql("

    404

    ") } + it { is_expected.to be_html_eql("

    404

    ") } end context "Link to another project's wiki" do @@ -476,7 +476,7 @@ describe OpenProject::TextFormatting do context 'Link to an unknown wiki page in another project' do subject { format_text('[[onlinestore:Unknown page]]') } - it { is_expected.to be_html_eql("

    Unknown page

    ") } + it { is_expected.to be_html_eql("

    Unknown page

    ") } end context 'Struck through link to wiki page' do diff --git a/spec_legacy/functional/wiki_controller_spec.rb b/spec_legacy/functional/wiki_controller_spec.rb index b0028a813a..57ab74c142 100644 --- a/spec_legacy/functional/wiki_controller_spec.rb +++ b/spec_legacy/functional/wiki_controller_spec.rb @@ -91,7 +91,7 @@ describe WikiController, type: :controller do session[:user_id] = 2 get :show, params: { project_id: 1, id: 'Unexistent page' } assert_response :success - assert_template 'edit' + assert_template 'new' end it 'should create page' do diff --git a/spec_legacy/unit/helpers/application_helper_spec.rb b/spec_legacy/unit/helpers/application_helper_spec.rb index dcd4ecae36..7216d09513 100644 --- a/spec_legacy/unit/helpers/application_helper_spec.rb +++ b/spec_legacy/unit/helpers/application_helper_spec.rb @@ -347,7 +347,7 @@ EXPECTED FactoryGirl.create :wiki_page_with_content, wiki: @project.wiki, title: 'Last page' to_test = { "|[[Page|Link title]]|[[Other Page|Other title]]|\n|Cell 21|[[Last page]]|" => - "Link title" + + "Link title" + "Other title" + "Cell 21Last page" }