From 1f01189ededb14a98b7f0b138891d0abbbddc4d9 Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Thu, 4 Apr 2019 22:40:08 +0200 Subject: [PATCH] wp widget only selectable with save_query permission --- .../modules/grids/grid/add-widget.service.ts | 9 +- .../app/modules/grids/grid/area.service.ts | 2 +- .../modules/grids/grid/grid.component.html | 2 +- .../modules/grids/widgets/add/add.modal.ts | 12 +- .../app/contracts/grids/base_contract.rb | 12 +- modules/grids/app/models/grids/widget.rb | 2 +- modules/grids/lib/grids/configuration.rb | 7 +- .../grids/configuration/widget_strategy.rb | 12 ++ .../lib/grids/my_page_grid_registration.rb | 2 + .../contracts/grids/create_contract_spec.rb | 16 ++- .../my/my_page_work_package_table_spec.rb | 127 +++++++++--------- spec/support/pages/my/page.rb | 28 +++- 12 files changed, 148 insertions(+), 83 deletions(-) diff --git a/frontend/src/app/modules/grids/grid/add-widget.service.ts b/frontend/src/app/modules/grids/grid/add-widget.service.ts index 9f85187540..3afb3ad5b9 100644 --- a/frontend/src/app/modules/grids/grid/add-widget.service.ts +++ b/frontend/src/app/modules/grids/grid/add-widget.service.ts @@ -8,6 +8,7 @@ import {GridWidgetArea} from "core-app/modules/grids/areas/grid-widget-area"; import {GridAreaService} from "core-app/modules/grids/grid/area.service"; import {GridDragAndDropService} from "core-app/modules/grids/grid/drag-and-drop.service"; import {GridResizeService} from "core-app/modules/grids/grid/resize.service"; +import {SchemaResource} from "core-app/modules/hal/resources/schema-resource"; @Injectable() export class GridAddWidgetService { @@ -27,9 +28,9 @@ export class GridAddWidgetService { this.layout.widgetAreaIds.includes(area.guid); } - public widget(area:GridArea) { + public widget(area:GridArea, schema:SchemaResource) { this - .select(area) + .select(area, schema) .then((widgetResource) => { // try to set it to a 2 x 3 layout // but shrink if that is outside the grid or @@ -75,9 +76,9 @@ export class GridAddWidgetService { }); } - private select(area:GridArea) { + private select(area:GridArea, schema:SchemaResource) { return new Promise((resolve, reject) => { - const modal = this.opModalService.show(AddGridWidgetModal, this.injector); + const modal = this.opModalService.show(AddGridWidgetModal, this.injector, { schema: schema }); modal.closingEvent.subscribe((modal:AddGridWidgetModal) => { let registered = modal.chosenWidget; diff --git a/frontend/src/app/modules/grids/grid/area.service.ts b/frontend/src/app/modules/grids/grid/area.service.ts index d492997e34..818fefa772 100644 --- a/frontend/src/app/modules/grids/grid/area.service.ts +++ b/frontend/src/app/modules/grids/grid/area.service.ts @@ -11,7 +11,7 @@ import {SchemaResource} from "core-app/modules/hal/resources/schema-resource"; export class GridAreaService { private resource:GridResource; - private schema:SchemaResource; + public schema:SchemaResource; public numColumns:number = 0; public numRows:number = 0; diff --git a/frontend/src/app/modules/grids/grid/grid.component.html b/frontend/src/app/modules/grids/grid/grid.component.html index 30b0a9e65f..6875dee761 100644 --- a/frontend/src/app/modules/grids/grid/grid.component.html +++ b/frontend/src/app/modules/grids/grid/grid.component.html @@ -82,7 +82,7 @@ [cdkDropListConnectedTo]="layout.widgetAreaIds">
+ (click)="add.widget(area, layout.schema)">
diff --git a/frontend/src/app/modules/grids/widgets/add/add.modal.ts b/frontend/src/app/modules/grids/widgets/add/add.modal.ts index fb6ce5d502..75e00a338d 100644 --- a/frontend/src/app/modules/grids/widgets/add/add.modal.ts +++ b/frontend/src/app/modules/grids/widgets/add/add.modal.ts @@ -26,7 +26,7 @@ export class AddGridWidgetModal extends OpModalComponent { } public get selectable() { - return this.widgetsService.registered.map((widget) => { + return this.eligibleWidgets.map((widget) => { return { identifier: widget.identifier, title: this.i18n.t(`js.grid.widgets.${widget.identifier}.title`), @@ -45,4 +45,14 @@ export class AddGridWidgetModal extends OpModalComponent { public trackWidgetBy(widget:WidgetRegistration) { return widget.identifier; } + + private get eligibleWidgets() { + let schemaWidgetIdentifiers = this.locals.schema.widgets.allowedValues.map((widget:any) => { + return widget.identifier; + }); + + return this.widgetsService.registered.filter((widget) => { + return schemaWidgetIdentifiers.includes(widget.identifier); + }); + } } diff --git a/modules/grids/app/contracts/grids/base_contract.rb b/modules/grids/app/contracts/grids/base_contract.rb index 238602fe49..f6a7dddd67 100644 --- a/modules/grids/app/contracts/grids/base_contract.rb +++ b/modules/grids/app/contracts/grids/base_contract.rb @@ -63,9 +63,9 @@ module Grids Grid end - def assignable_values(column, _user) + def assignable_values(column, user) if column == :widgets - config.all_widget_identifiers(grid_class) + all_allowed_widget_identifiers(user) end end @@ -86,7 +86,7 @@ module Grids return unless config.registered_grid?(grid_class) undestroyed_widgets.each do |widget| - next if config.allowed_widget?(grid_class, widget.identifier) + next if config.allowed_widget?(grid_class, widget.identifier, user) errors.add(:widgets, :inclusion) end @@ -180,6 +180,12 @@ module Grids model.widgets.reject(&:marked_for_destruction?) end + def all_allowed_widget_identifiers(user) + config.all_widget_identifiers(grid_class).select do |identifier| + config.allowed_widget?(grid_class, identifier, user) + end + end + def grid_class model.class end diff --git a/modules/grids/app/models/grids/widget.rb b/modules/grids/app/models/grids/widget.rb index 11035e75bd..f07a561d44 100644 --- a/modules/grids/app/models/grids/widget.rb +++ b/modules/grids/app/models/grids/widget.rb @@ -40,7 +40,7 @@ module Grids private def execute_after_destroy_strategy - proc = Grids::Configuration.widget_strategy(grid, identifier).after_destroy + proc = Grids::Configuration.widget_strategy(grid.class, identifier).after_destroy instance_exec(&proc) end diff --git a/modules/grids/lib/grids/configuration.rb b/modules/grids/lib/grids/configuration.rb index 1ef3f4781f..5f6e61670e 100644 --- a/modules/grids/lib/grids/configuration.rb +++ b/modules/grids/lib/grids/configuration.rb @@ -89,10 +89,11 @@ module Grids::Configuration @widget_register[identifier] = Array(grid_classes) end - def allowed_widget?(grid, identifier) + def allowed_widget?(grid, identifier, user) grid_classes = registered_widget_by_identifier[identifier] - (grid_classes || []).include?(grid) + (grid_classes || []).include?(grid) && + widget_strategy(grid, identifier)&.allowed?(user) end def all_widget_identifiers(grid) @@ -102,7 +103,7 @@ module Grids::Configuration end def widget_strategy(grid, identifier) - grid_register[grid.class.to_s].widget_strategy(identifier) + grid_register[grid.to_s]&.widget_strategy(identifier) end def writable?(grid, user) diff --git a/modules/grids/lib/grids/configuration/widget_strategy.rb b/modules/grids/lib/grids/configuration/widget_strategy.rb index 4c666e6f18..197afe5c98 100644 --- a/modules/grids/lib/grids/configuration/widget_strategy.rb +++ b/modules/grids/lib/grids/configuration/widget_strategy.rb @@ -38,6 +38,18 @@ module Grids::Configuration @after_destroy ||= -> {} end + + def allowed(proc = nil) + if proc + @allowed = proc + end + + @allowed ||= ->(_user) { true } + end + + def allowed?(user) + allowed.(user) + end end end end diff --git a/modules/grids/lib/grids/my_page_grid_registration.rb b/modules/grids/lib/grids/my_page_grid_registration.rb index c913852f55..56f9c7c791 100644 --- a/modules/grids/lib/grids/my_page_grid_registration.rb +++ b/modules/grids/lib/grids/my_page_grid_registration.rb @@ -15,6 +15,8 @@ module Grids widget_strategy 'work_packages_table' do after_destroy -> { ::Query.find_by(id: options[:queryId])&.destroy } + + allowed ->(user) { user.allowed_to_globally?(:save_queries) } end defaults( diff --git a/modules/grids/spec/contracts/grids/create_contract_spec.rb b/modules/grids/spec/contracts/grids/create_contract_spec.rb index 5fc8b38a46..16511c79c0 100644 --- a/modules/grids/spec/contracts/grids/create_contract_spec.rb +++ b/modules/grids/spec/contracts/grids/create_contract_spec.rb @@ -95,15 +95,25 @@ describe Grids::CreateContract do end context 'for widgets' do - it 'calls the grid configuration for the available values' do - widgets = double('widgets') + it 'calls the grid configuration for the available values but allows only those eligible' do + widgets = %i[widget1 widget2] allow(Grids::Configuration) .to receive(:all_widget_identifiers) .and_return(widgets) + allow(Grids::Configuration) + .to receive(:allowed_widget?) + .with(Grids::MyPage, :widget1, user) + .and_return(true) + + allow(Grids::Configuration) + .to receive(:allowed_widget?) + .with(Grids::MyPage, :widget2, user) + .and_return(false) + expect(instance.assignable_values(:widgets, user)) - .to eql widgets + .to match_array [:widget1] end end diff --git a/modules/grids/spec/features/my/my_page_work_package_table_spec.rb b/modules/grids/spec/features/my/my_page_work_package_table_spec.rb index 5418b76cd0..0cafef53dd 100644 --- a/modules/grids/spec/features/my/my_page_work_package_table_spec.rb +++ b/modules/grids/spec/features/my/my_page_work_package_table_spec.rb @@ -50,17 +50,12 @@ describe 'Arbitrary WorkPackage query table widget on my page', type: :feature, responsible: user end - let(:role) do - FactoryBot.create(:role, - permissions: %i[view_work_packages - add_work_packages - save_queries]) - end + let(:permissions) { %i[view_work_packages add_work_packages save_queries] } let(:user) do FactoryBot.create(:user, member_in_project: project, - member_through_role: role) + member_with_permissions: permissions) end let(:my_page) do Pages::My::Page.new @@ -76,82 +71,92 @@ describe 'Arbitrary WorkPackage query table widget on my page', type: :feature, my_page.visit! end - it 'can add the widget and see the work packages of the filtered for types' do - my_page.add_column(3, before_or_after: :before) + context 'with the permission to save queries' do + it 'can add the widget and see the work packages of the filtered for types' do + my_page.add_column(3, before_or_after: :before) - my_page.add_widget(2, 3, "Work packages") + my_page.add_widget(2, 3, "Work packages") - sleep(0.2) + sleep(1) - sleep(1) + filter_area = Components::Grids::GridArea.new('.grid--area.-widgeted:nth-of-type(3)') + created_area = Components::Grids::GridArea.new('.grid--area', text: "Work packages created by me") - filter_area = Components::Grids::GridArea.new('.grid--area.-widgeted:nth-of-type(3)') - created_area = Components::Grids::GridArea.new('.grid--area', text: "Work packages created by me") + filter_area.expect_to_span(2, 3, 5, 4) + filter_area.resize_to(6, 4) - filter_area.expect_to_span(2, 3, 5, 4) - filter_area.resize_to(6, 4) + filter_area.expect_to_span(2, 3, 7, 5) + ## enlarging the table area will have moved the created area down + created_area.expect_to_span(7, 4, 13, 6) - filter_area.expect_to_span(2, 3, 7, 5) - ## enlarging the table area will have moved the created area down - created_area.expect_to_span(7, 4, 13, 6) + # At the beginning, the default query is displayed + expect(filter_area.area) + .to have_selector('.subject', text: type_work_package.subject) - # At the beginning, the default query is displayed - expect(filter_area.area) - .to have_selector('.subject', text: type_work_package.subject) + expect(filter_area.area) + .to have_selector('.subject', text: other_type_work_package.subject) - expect(filter_area.area) - .to have_selector('.subject', text: other_type_work_package.subject) + # User has the ability to modify the query - # User has the ability to modify the query + modal.open_and_switch_to('Filters') + filters.expect_filter_count(2) + filters.add_filter_by('Type', 'is', type.name) + modal.save - modal.open_and_switch_to('Filters') - filters.expect_filter_count(2) - filters.add_filter_by('Type', 'is', type.name) - modal.save + columns.remove 'Subject' - columns.remove 'Subject' + expect(filter_area.area) + .to have_selector('.id', text: type_work_package.id) - expect(filter_area.area) - .to have_selector('.id', text: type_work_package.id) + # as the Subject column is disabled + expect(filter_area.area) + .to have_no_selector('.subject', text: type_work_package.subject) - # as the Subject column is disabled - expect(filter_area.area) - .to have_no_selector('.subject', text: type_work_package.subject) + # As other_type is filtered out + expect(filter_area.area) + .to have_no_selector('.id', text: other_type_work_package.id) - # As other_type is filtered out - expect(filter_area.area) - .to have_no_selector('.id', text: other_type_work_package.id) + scroll_to_element(filter_area.area) + within filter_area.area do + input = find('.editable-toolbar-title--input') + input.set('My WP Filter') + input.native.send_keys(:return) + end - scroll_to_element(filter_area.area) - within filter_area.area do - input = find('.editable-toolbar-title--input') - input.set('My WP Filter') - input.native.send_keys(:return) - end + sleep(1) - sleep(1) + # The whole of the configuration survives a reload + # as it is persisted in the grid - # The whole of the configuration survives a reload - # as it is persisted in the grid + visit root_path + my_page.visit! - visit root_path - my_page.visit! + filter_area = Components::Grids::GridArea.new('.grid--area.-widgeted:nth-of-type(3)') + expect(filter_area.area) + .to have_selector('.id', text: type_work_package.id) + + # as the Subject column is disabled + expect(filter_area.area) + .to have_no_selector('.subject', text: type_work_package.subject) - filter_area = Components::Grids::GridArea.new('.grid--area.-widgeted:nth-of-type(3)') - expect(filter_area.area) - .to have_selector('.id', text: type_work_package.id) + # As other_type is filtered out + expect(filter_area.area) + .to have_no_selector('.id', text: other_type_work_package.id) + + within filter_area.area do + expect(find('.editable-toolbar-title--input').value) + .to eql('My WP Filter') + end + end + end - # as the Subject column is disabled - expect(filter_area.area) - .to have_no_selector('.subject', text: type_work_package.subject) + context 'without the permission to save queries' do + let(:permissions) { %i[view_work_packages add_work_packages] } - # As other_type is filtered out - expect(filter_area.area) - .to have_no_selector('.id', text: other_type_work_package.id) + it 'cannot add the widget' do + my_page.add_column(3, before_or_after: :before) - within filter_area.area do - expect(find('.editable-toolbar-title--input').value) - .to eql('My WP Filter') + my_page.expect_unable_to_add_widget(2, 3, "Work packages") end end end diff --git a/spec/support/pages/my/page.rb b/spec/support/pages/my/page.rb index 8e80d09c0a..d188f2e634 100644 --- a/spec/support/pages/my/page.rb +++ b/spec/support/pages/my/page.rb @@ -62,11 +62,7 @@ module Pages end def add_widget(row_number, column_number, name) - area = area_of(row_number, column_number) - area.hover - area.find('.grid--widget-add').click - - within '.op-modal--portal' do + within_add_widget_modal(row_number, column_number) do expect(page) .to have_content(I18n.t('js.grid.add_modal.choose_widget')) @@ -74,9 +70,31 @@ module Pages end end + def expect_unable_to_add_widget(row_number, column_number, name) + within_add_widget_modal(row_number, column_number) do + expect(page) + .to have_content(I18n.t('js.grid.add_modal.choose_widget')) + + expect(page) + .not_to have_selector('.grid--addable-widget', text: Regexp.new("^#{name}$")) + end + end + def area_of(row_number, column_number) ::Components::Grids::GridArea.of(row_number, column_number).area end + + private + + def within_add_widget_modal(row_number, column_number) + area = area_of(row_number, column_number) + area.hover + area.find('.grid--widget-add').click + + within '.op-modal--portal' do + yield + end + end end end end