From 323487bee01c84ff032917efec276d9649ff1350 Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Mon, 18 Mar 2019 09:13:21 +0100 Subject: [PATCH 01/14] start on work_packages_table widget --- config/locales/js-en.yml | 2 + .../modules/grids/openproject-grids.module.ts | 7 ++ .../widgets/wp-table/wp-table.component.ts | 24 +++++ .../lib/grids/my_page_grid_registration.rb | 1 + .../my/my_page_work_package_table_spec.rb | 96 +++++++++++++++++++ 5 files changed, 130 insertions(+) create mode 100644 frontend/src/app/modules/grids/widgets/wp-table/wp-table.component.ts create mode 100644 modules/grids/spec/features/my/my_page_work_package_table_spec.rb diff --git a/config/locales/js-en.yml b/config/locales/js-en.yml index 7120cad7ae..95dca9fe05 100644 --- a/config/locales/js-en.yml +++ b/config/locales/js-en.yml @@ -208,6 +208,8 @@ en: title: 'Work packages created by me' work_packages_watched: title: 'Work packages watched by me' + work_packages_table: + title: 'Work package table' work_packages_calendar: title: 'Calendar' diff --git a/frontend/src/app/modules/grids/openproject-grids.module.ts b/frontend/src/app/modules/grids/openproject-grids.module.ts index cbc5d70dd9..5e2fe922ed 100644 --- a/frontend/src/app/modules/grids/openproject-grids.module.ts +++ b/frontend/src/app/modules/grids/openproject-grids.module.ts @@ -50,6 +50,7 @@ import {Ng2StateDeclaration, UIRouterModule} from '@uirouter/angular'; import {WidgetDocumentsComponent} from "core-app/modules/grids/widgets/documents/documents.component"; import {WidgetNewsComponent} from "core-app/modules/grids/widgets/news/news.component"; import {WidgetWpAccountableComponent} from './widgets/wp-accountable/wp-accountable.component'; +import {WidgetWpTableComponent} from "core-app/modules/grids/widgets/wp-table/wp-table.component"; export const GRID_ROUTES:Ng2StateDeclaration[] = [ { @@ -76,6 +77,7 @@ export const GRID_ROUTES:Ng2StateDeclaration[] = [ WidgetWpAccountableComponent, WidgetWpCreatedComponent, WidgetWpWatchedComponent, + WidgetWpTableComponent, WidgetWpCalendarComponent, WidgetTimeEntriesCurrentUserComponent]), @@ -100,6 +102,7 @@ export const GRID_ROUTES:Ng2StateDeclaration[] = [ WidgetWpCreatedComponent, WidgetWpWatchedComponent, WidgetWpCalendarComponent, + WidgetWpTableComponent, WidgetTimeEntriesCurrentUserComponent, AddGridWidgetModal, @@ -142,6 +145,10 @@ export function registerWidgets(injector:Injector) { identifier: 'work_packages_watched', component: WidgetWpWatchedComponent }, + { + identifier: 'work_packages_table', + component: WidgetWpTableComponent + }, { identifier: 'work_packages_calendar', component: WidgetWpCalendarComponent diff --git a/frontend/src/app/modules/grids/widgets/wp-table/wp-table.component.ts b/frontend/src/app/modules/grids/widgets/wp-table/wp-table.component.ts new file mode 100644 index 0000000000..c804914b69 --- /dev/null +++ b/frontend/src/app/modules/grids/widgets/wp-table/wp-table.component.ts @@ -0,0 +1,24 @@ +import {Component, OnInit} from "@angular/core"; +import {ApiV3FilterBuilder} from "core-components/api/api-v3/api-v3-filter-builder"; +import {WidgetWpListComponent} from "core-app/modules/grids/widgets/wp-widget/wp-widget.component"; + +@Component({ + templateUrl: '../wp-widget/wp-widget.component.html', + styleUrls: ['../wp-widget/wp-widget.component.css'] +}) +export class WidgetWpTableComponent extends WidgetWpListComponent implements OnInit { + public text = { title: this.i18n.t('js.grid.widgets.work_packages_table.title') }; + public queryProps:any; + + ngOnInit() { + super.ngOnInit(); + // TODO: adapt to arbitrary query + let filters = new ApiV3FilterBuilder(); + filters.add('watcher', '=', ["me"]); + filters.add('status', 'o', []); + + this.queryProps = {"columns[]":["id", "project", "type", "subject"], + "filters":filters.toJson()}; + + } +} diff --git a/modules/grids/lib/grids/my_page_grid_registration.rb b/modules/grids/lib/grids/my_page_grid_registration.rb index f71639413a..9ef0f99cbd 100644 --- a/modules/grids/lib/grids/my_page_grid_registration.rb +++ b/modules/grids/lib/grids/my_page_grid_registration.rb @@ -8,6 +8,7 @@ module Grids 'work_packages_watched', 'work_packages_created', 'work_packages_calendar', + 'work_packages_table', 'time_entries_current_user', 'documents', 'news' 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 new file mode 100644 index 0000000000..f765fdec10 --- /dev/null +++ b/modules/grids/spec/features/my/my_page_work_package_table_spec.rb @@ -0,0 +1,96 @@ +#-- copyright +# OpenProject is a project management system. +# Copyright (C) 2012-2018 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 docs/COPYRIGHT.rdoc for more details. +#++ + +require 'spec_helper' + +describe 'Arbitrary WorkPackage query table widget widget on my page', type: :feature, js: true do + let!(:type) { FactoryBot.create :type } + let!(:other_type) { FactoryBot.create :type } + let!(:priority) { FactoryBot.create :default_priority } + let!(:project) { FactoryBot.create :project, types: [type] } + let!(:other_project) { FactoryBot.create :project, types: [type] } + let!(:open_status) { FactoryBot.create :default_status } + let!(:type_work_package) do + FactoryBot.create :work_package, + project: project, + type: type, + author: user, + responsible: user + end + let!(:other_type_work_package) do + FactoryBot.create :work_package, + project: project, + type: other_type, + author: user, + responsible: user + end + + let(:role) { FactoryBot.create(:role, permissions: %i[view_work_packages add_work_packages]) } + + let(:user) do + FactoryBot.create(:user, + member_in_project: project, + member_through_role: role) + end + let(:my_page) do + Pages::My::Page.new + end + + before do + login_as user + + 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) + + my_page.add_widget(2, 3, "Work package table") + + sleep(0.2) + + filter_area = Components::Grids::GridArea.new('.grid--area', text: "Work package table") + 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, 7, 5) + ## enlarging the accountable area will have moved the created area down + created_area.expect_to_span(7, 4, 13, 6) + + #expect(accountable_area.area) + # .to have_selector('.subject', text: accountable_work_package.subject) + + #expect(accountable_area.area) + # .to have_no_selector('.subject', text: accountable_by_other_work_package.subject) + + #expect(accountable_area.area) + # .to have_no_selector('.subject', text: accountable_but_invisible_work_package.subject) + end +end From e63b1fa021469cc855afe97bd3dd0a87616070ce Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Tue, 19 Mar 2019 09:00:20 +0100 Subject: [PATCH 02/14] enable modifying the query - unpersisted yet --- .../widgets/wp-table/wp-table.component.ts | 41 +++++++++++--- .../wp-isolated-query-space.directive.ts | 2 +- .../my/my_page_work_package_table_spec.rb | 56 ++++++++++++++++--- .../components/work_packages/columns.rb | 3 - 4 files changed, 81 insertions(+), 21 deletions(-) diff --git a/frontend/src/app/modules/grids/widgets/wp-table/wp-table.component.ts b/frontend/src/app/modules/grids/widgets/wp-table/wp-table.component.ts index c804914b69..9f4433e9cb 100644 --- a/frontend/src/app/modules/grids/widgets/wp-table/wp-table.component.ts +++ b/frontend/src/app/modules/grids/widgets/wp-table/wp-table.component.ts @@ -1,24 +1,47 @@ -import {Component, OnInit} from "@angular/core"; +import {Component, OnInit, OnDestroy, ViewChild, AfterViewInit} from "@angular/core"; import {ApiV3FilterBuilder} from "core-components/api/api-v3/api-v3-filter-builder"; import {WidgetWpListComponent} from "core-app/modules/grids/widgets/wp-widget/wp-widget.component"; +import {WorkPackageTableConfiguration} from "core-components/wp-table/wp-table-configuration"; +import {QueryResource} from "core-app/modules/hal/resources/query-resource"; +import {I18nService} from "core-app/modules/common/i18n/i18n.service"; +import {IsolatedQuerySpace} from "core-app/modules/work_packages/query-space/isolated-query-space"; +import {untilComponentDestroyed} from 'ng2-rx-componentdestroyed'; +import {WorkPackageIsolatedQuerySpaceDirective} from "core-app/modules/work_packages/query-space/wp-isolated-query-space.directive"; @Component({ templateUrl: '../wp-widget/wp-widget.component.html', styleUrls: ['../wp-widget/wp-widget.component.css'] }) -export class WidgetWpTableComponent extends WidgetWpListComponent implements OnInit { +export class WidgetWpTableComponent extends WidgetWpListComponent implements OnInit, OnDestroy, AfterViewInit { public text = { title: this.i18n.t('js.grid.widgets.work_packages_table.title') }; - public queryProps:any; + public queryProps = {}; + + public configuration:Partial = { + actionsColumnEnabled: false, + columnMenuEnabled: true, + hierarchyToggleEnabled: true, + contextMenuEnabled: false + }; + + @ViewChild(WorkPackageIsolatedQuerySpaceDirective) public querySpaceDirective:WorkPackageIsolatedQuerySpaceDirective; ngOnInit() { super.ngOnInit(); - // TODO: adapt to arbitrary query - let filters = new ApiV3FilterBuilder(); - filters.add('watcher', '=', ["me"]); - filters.add('status', 'o', []); - this.queryProps = {"columns[]":["id", "project", "type", "subject"], - "filters":filters.toJson()}; + } + + ngAfterViewInit() { + this + .querySpaceDirective + .querySpace + .query + .values$() + .pipe( + untilComponentDestroyed(this) + ).subscribe(() => console.log('query updated')); + } + ngOnDestroy() { + // nothing to do } } diff --git a/frontend/src/app/modules/work_packages/query-space/wp-isolated-query-space.directive.ts b/frontend/src/app/modules/work_packages/query-space/wp-isolated-query-space.directive.ts index 2817aad886..d898496489 100644 --- a/frontend/src/app/modules/work_packages/query-space/wp-isolated-query-space.directive.ts +++ b/frontend/src/app/modules/work_packages/query-space/wp-isolated-query-space.directive.ts @@ -114,7 +114,7 @@ import {debugLog} from "core-app/helpers/debug_output"; export class WorkPackageIsolatedQuerySpaceDirective { constructor(private elementRef:ElementRef, - private querySpace:IsolatedQuerySpace, + public querySpace:IsolatedQuerySpace, private injector:Injector) { debugLog("Opening isolated query space %O in %O", injector, elementRef.nativeElement); } 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 f765fdec10..9b3fd5cd9c 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 @@ -28,7 +28,7 @@ require 'spec_helper' -describe 'Arbitrary WorkPackage query table widget widget on my page', type: :feature, js: true do +describe 'Arbitrary WorkPackage query table widget on my page', type: :feature, js: true do let!(:type) { FactoryBot.create :type } let!(:other_type) { FactoryBot.create :type } let!(:priority) { FactoryBot.create :default_priority } @@ -61,6 +61,10 @@ describe 'Arbitrary WorkPackage query table widget widget on my page', type: :fe Pages::My::Page.new end + let(:modal) { ::Components::WorkPackages::TableConfigurationModal.new } + let(:filters) { ::Components::WorkPackages::TableConfiguration::Filters.new } + let(:columns) { ::Components::WorkPackages::Columns.new } + before do login_as user @@ -81,16 +85,52 @@ describe 'Arbitrary WorkPackage query table widget widget on my page', type: :fe filter_area.resize_to(6, 4) filter_area.expect_to_span(2, 3, 7, 5) - ## enlarging the accountable area will have moved the created area down + ## enlarging the table area will have moved the created area down created_area.expect_to_span(7, 4, 13, 6) - #expect(accountable_area.area) - # .to have_selector('.subject', text: accountable_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) + + # 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 + + columns.remove 'Subject' + + 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 other_type is filtered out + expect(filter_area.area) + .to have_no_selector('.id', text: other_type_work_package.id) + + # The whole of the configuration survives a reload + # as it is persisted in the grid + + visit root_path + my_page.visit! + + filter_area = Components::Grids::GridArea.new('.grid--area', text: "Work package table") + expect(filter_area.area) + .to have_selector('.id', text: type_work_package.id) - #expect(accountable_area.area) - # .to have_no_selector('.subject', text: accountable_by_other_work_package.subject) + # as the Subject column is disabled + expect(filter_area.area) + .to have_no_selector('.subject', text: type_work_package.subject) - #expect(accountable_area.area) - # .to have_no_selector('.subject', text: accountable_but_invisible_work_package.subject) + # As other_type is filtered out + expect(filter_area.area) + .to have_no_selector('.id', text: other_type_work_package.id) end end diff --git a/spec/support/components/work_packages/columns.rb b/spec/support/components/work_packages/columns.rb index 5c6f770b93..2013df0a82 100644 --- a/spec/support/components/work_packages/columns.rb +++ b/spec/support/components/work_packages/columns.rb @@ -32,9 +32,6 @@ module Components include Capybara::DSL include RSpec::Matchers - def initialize - end - def expect_column_not_available(name) modal_open? or open_modal From 2aaced8c6a92580bb5098c5d531e4361167c3b85 Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Fri, 22 Mar 2019 11:02:04 +0100 Subject: [PATCH 03/14] persist changes to the wp-table widget --- .../components/wp-query/url-params-helper.ts | 3 ++- .../app/modules/grids/grid/area.service.ts | 7 +++++- .../modules/grids/grid/grid.component.html | 2 +- .../app/modules/grids/grid/grid.component.ts | 4 ++++ .../widgets/abstract-widget.component.ts | 4 +++- .../widgets/wp-table/wp-table.component.ts | 22 ++++++++++++++++--- .../widgets/wp-widget/wp-widget.component.ts | 1 - 7 files changed, 35 insertions(+), 8 deletions(-) diff --git a/frontend/src/app/components/wp-query/url-params-helper.ts b/frontend/src/app/components/wp-query/url-params-helper.ts index 1fff418901..455b3ea9c1 100644 --- a/frontend/src/app/components/wp-query/url-params-helper.ts +++ b/frontend/src/app/components/wp-query/url-params-helper.ts @@ -84,7 +84,8 @@ export class UrlParamsHelperService { private encodeColumns(paramsData:any, query:QueryResource) { paramsData.c = query.columns.map(function (column) { return column.id!; - }) + }); + return paramsData; } diff --git a/frontend/src/app/modules/grids/grid/area.service.ts b/frontend/src/app/modules/grids/grid/area.service.ts index c38ffcf7d1..d492997e34 100644 --- a/frontend/src/app/modules/grids/grid/area.service.ts +++ b/frontend/src/app/modules/grids/grid/area.service.ts @@ -48,13 +48,18 @@ export class GridAreaService { this.resource.widgets = this.widgetResources; this.resource.rowCount = this.numRows; + this.resource.columnCount = this.numColumns; if (save) { - this.gridDm.update(this.resource, this.schema); + this.saveGrid(); } } + public saveGrid() { + this.gridDm.update(this.resource, this.schema); + } + private buildGridAreas() { let cells:GridArea[] = []; diff --git a/frontend/src/app/modules/grids/grid/grid.component.html b/frontend/src/app/modules/grids/grid/grid.component.html index 87da0e8c75..30b0a9e65f 100644 --- a/frontend/src/app/modules/grids/grid/grid.component.html +++ b/frontend/src/app/modules/grids/grid/grid.component.html @@ -52,7 +52,7 @@ + [ndcDynamicOutputs]="widgetComponentOutput(area.widget)"> (); + constructor(protected i18n:I18nService) { } } diff --git a/frontend/src/app/modules/grids/widgets/wp-table/wp-table.component.ts b/frontend/src/app/modules/grids/widgets/wp-table/wp-table.component.ts index 9f4433e9cb..9e774e32a6 100644 --- a/frontend/src/app/modules/grids/widgets/wp-table/wp-table.component.ts +++ b/frontend/src/app/modules/grids/widgets/wp-table/wp-table.component.ts @@ -7,6 +7,8 @@ import {I18nService} from "core-app/modules/common/i18n/i18n.service"; import {IsolatedQuerySpace} from "core-app/modules/work_packages/query-space/isolated-query-space"; import {untilComponentDestroyed} from 'ng2-rx-componentdestroyed'; import {WorkPackageIsolatedQuerySpaceDirective} from "core-app/modules/work_packages/query-space/wp-isolated-query-space.directive"; +import {skip} from 'rxjs/operators'; +import {UrlParamsHelperService} from "core-components/wp-query/url-params-helper"; @Component({ templateUrl: '../wp-widget/wp-widget.component.html', @@ -14,7 +16,7 @@ import {WorkPackageIsolatedQuerySpaceDirective} from "core-app/modules/work_pack }) export class WidgetWpTableComponent extends WidgetWpListComponent implements OnInit, OnDestroy, AfterViewInit { public text = { title: this.i18n.t('js.grid.widgets.work_packages_table.title') }; - public queryProps = {}; + public queryProps:any = {}; public configuration:Partial = { actionsColumnEnabled: false, @@ -23,11 +25,18 @@ export class WidgetWpTableComponent extends WidgetWpListComponent implements OnI contextMenuEnabled: false }; + constructor(protected i18n:I18nService, + protected urlParamsHelper:UrlParamsHelperService) { + super(i18n); + } + @ViewChild(WorkPackageIsolatedQuerySpaceDirective) public querySpaceDirective:WorkPackageIsolatedQuerySpaceDirective; ngOnInit() { + if (this.resource.options.queryProps) { + this.queryProps = this.resource.options.queryProps; + } super.ngOnInit(); - } ngAfterViewInit() { @@ -37,8 +46,15 @@ export class WidgetWpTableComponent extends WidgetWpListComponent implements OnI .query .values$() .pipe( + // 2 because ... well it is a magic number and works + skip(2), untilComponentDestroyed(this) - ).subscribe(() => console.log('query updated')); + ).subscribe((query) => { + let queryProps = this.urlParamsHelper.buildV3GetQueryFromQueryResource(query); + + this.resource.options = { queryProps: queryProps }; + this.resourceChanged.emit(this.resource); + }); } ngOnDestroy() { diff --git a/frontend/src/app/modules/grids/widgets/wp-widget/wp-widget.component.ts b/frontend/src/app/modules/grids/widgets/wp-widget/wp-widget.component.ts index 815f58d566..1eb8d2d052 100644 --- a/frontend/src/app/modules/grids/widgets/wp-widget/wp-widget.component.ts +++ b/frontend/src/app/modules/grids/widgets/wp-widget/wp-widget.component.ts @@ -2,7 +2,6 @@ import {AbstractWidgetComponent} from "app/modules/grids/widgets/abstract-widget import {OnInit} from "@angular/core"; import { WorkPackageTableConfiguration, - WorkPackageTableConfigurationObject } from "core-components/wp-table/wp-table-configuration"; export class WidgetWpListComponent extends AbstractWidgetComponent implements OnInit { From 7433d65d9196d30641011962786e7b7809ec41e7 Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Tue, 26 Mar 2019 21:56:26 +0100 Subject: [PATCH 04/14] use backend saved query --- .../modules/grids/grid/add-widget.service.ts | 3 +- .../widgets/wp-table/wp-table.component.ts | 63 +++++++++++++++---- .../wp-widget/wp-widget.component.html | 2 +- .../my/my_page_work_package_table_spec.rb | 2 + 4 files changed, 57 insertions(+), 13 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 5cafa21154..9f85187540 100644 --- a/frontend/src/app/modules/grids/grid/add-widget.service.ts +++ b/frontend/src/app/modules/grids/grid/add-widget.service.ts @@ -92,7 +92,8 @@ export class GridAddWidgetService { startRow: area.startRow, endRow: area.endRow, startColumn: area.startColumn, - endColumn: area.endColumn + endColumn: area.endColumn, + options: {} }; let resource = this.halResource.createHalResource(source) as GridWidgetResource; diff --git a/frontend/src/app/modules/grids/widgets/wp-table/wp-table.component.ts b/frontend/src/app/modules/grids/widgets/wp-table/wp-table.component.ts index 9e774e32a6..2a4c824f88 100644 --- a/frontend/src/app/modules/grids/widgets/wp-table/wp-table.component.ts +++ b/frontend/src/app/modules/grids/widgets/wp-table/wp-table.component.ts @@ -1,14 +1,16 @@ import {Component, OnInit, OnDestroy, ViewChild, AfterViewInit} from "@angular/core"; -import {ApiV3FilterBuilder} from "core-components/api/api-v3/api-v3-filter-builder"; +import {ApiV3Filter, ApiV3FilterBuilder} from "core-components/api/api-v3/api-v3-filter-builder"; import {WidgetWpListComponent} from "core-app/modules/grids/widgets/wp-widget/wp-widget.component"; import {WorkPackageTableConfiguration} from "core-components/wp-table/wp-table-configuration"; import {QueryResource} from "core-app/modules/hal/resources/query-resource"; import {I18nService} from "core-app/modules/common/i18n/i18n.service"; -import {IsolatedQuerySpace} from "core-app/modules/work_packages/query-space/isolated-query-space"; import {untilComponentDestroyed} from 'ng2-rx-componentdestroyed'; import {WorkPackageIsolatedQuerySpaceDirective} from "core-app/modules/work_packages/query-space/wp-isolated-query-space.directive"; import {skip} from 'rxjs/operators'; import {UrlParamsHelperService} from "core-components/wp-query/url-params-helper"; +import {QueryFormDmService} from "core-app/modules/hal/dm-services/query-form-dm.service"; +import {QueryDmService} from "core-app/modules/hal/dm-services/query-dm.service"; +import {QueryFormResource} from "core-app/modules/hal/resources/query-form-resource"; @Component({ templateUrl: '../wp-widget/wp-widget.component.html', @@ -16,7 +18,8 @@ import {UrlParamsHelperService} from "core-components/wp-query/url-params-helper }) export class WidgetWpTableComponent extends WidgetWpListComponent implements OnInit, OnDestroy, AfterViewInit { public text = { title: this.i18n.t('js.grid.widgets.work_packages_table.title') }; - public queryProps:any = {}; + public queryId:string|null; + private queryForm:QueryFormResource; public configuration:Partial = { actionsColumnEnabled: false, @@ -26,17 +29,31 @@ export class WidgetWpTableComponent extends WidgetWpListComponent implements OnI }; constructor(protected i18n:I18nService, - protected urlParamsHelper:UrlParamsHelperService) { + protected urlParamsHelper:UrlParamsHelperService, + private readonly queryDm:QueryDmService, + private readonly queryFormDm:QueryFormDmService) { super(i18n); } @ViewChild(WorkPackageIsolatedQuerySpaceDirective) public querySpaceDirective:WorkPackageIsolatedQuerySpaceDirective; ngOnInit() { - if (this.resource.options.queryProps) { - this.queryProps = this.resource.options.queryProps; + if (!this.resource.options.queryId) { + this.createInitial() + .then((query) => { + this.resource.options = { queryId: query.id }; + + this.resourceChanged.emit(this.resource); + + this.queryId = query.id; + + super.ngOnInit(); + }); + } else { + this.queryId = this.resource.options.queryId as string; + + super.ngOnInit(); } - super.ngOnInit(); } ngAfterViewInit() { @@ -50,14 +67,38 @@ export class WidgetWpTableComponent extends WidgetWpListComponent implements OnI skip(2), untilComponentDestroyed(this) ).subscribe((query) => { - let queryProps = this.urlParamsHelper.buildV3GetQueryFromQueryResource(query); - - this.resource.options = { queryProps: queryProps }; - this.resourceChanged.emit(this.resource); + if (this.queryForm) { + this.queryDm.update(query, this.queryForm).toPromise(); + } else { + this.queryFormDm.load(query).then((form) => { + this.queryForm = form; + this.queryDm.update(query, form).toPromise(); + }); + } }); } ngOnDestroy() { // nothing to do } + + private createInitial():Promise { + return this.queryFormDm + .loadWithParams( + {pageSize: 0}, + undefined, + null, + this.buildQueryRequest() + ) + .then(form => { + const query = this.queryFormDm.buildQueryResource(form); + return this.queryDm.create(query, form); + }); + } + + private buildQueryRequest() { + return { + hidden: true + }; + } } diff --git a/frontend/src/app/modules/grids/widgets/wp-widget/wp-widget.component.html b/frontend/src/app/modules/grids/widgets/wp-widget/wp-widget.component.html index 4c514669a6..0ba32152d9 100644 --- a/frontend/src/app/modules/grids/widgets/wp-widget/wp-widget.component.html +++ b/frontend/src/app/modules/grids/widgets/wp-widget/wp-widget.component.html @@ -4,7 +4,7 @@ - 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 9b3fd5cd9c..b97a7c1c8f 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 @@ -115,6 +115,8 @@ describe 'Arbitrary WorkPackage query table widget on my page', type: :feature, expect(filter_area.area) .to have_no_selector('.id', text: other_type_work_package.id) + sleep(1) + # The whole of the configuration survives a reload # as it is persisted in the grid From 6eda5c318a0aa5774bcb080198eae12e1e7b941a Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Mon, 1 Apr 2019 10:21:32 +0200 Subject: [PATCH 05/14] allow setting a custom title --- app/assets/stylesheets/content/_grid.sass | 22 +++++++ config/locales/js-en.yml | 2 +- .../widgets/abstract-widget.component.ts | 2 +- .../widgets/wp-table/wp-table.component.html | 23 +++++++ .../widgets/wp-table/wp-table.component.ts | 64 +++++++++++++++---- .../widgets/wp-widget/wp-widget.component.css | 8 +++ .../wp-widget/wp-widget.component.html | 2 +- .../my/my_page_work_package_table_spec.rb | 27 ++++++-- spec/support/pages/my/page.rb | 2 +- 9 files changed, 133 insertions(+), 19 deletions(-) create mode 100644 frontend/src/app/modules/grids/widgets/wp-table/wp-table.component.html diff --git a/app/assets/stylesheets/content/_grid.sass b/app/assets/stylesheets/content/_grid.sass index e9d7f3fe24..a86f625f99 100644 --- a/app/assets/stylesheets/content/_grid.sass +++ b/app/assets/stylesheets/content/_grid.sass @@ -27,6 +27,7 @@ $grid--header-width: 20px .widget-box height: 100% + &.-resizing border: 1px solid $primary-color z-index: 100 @@ -46,6 +47,20 @@ $grid--header-width: 20px .cdk-drag-handle cursor: grab +.grid--area-drag-handle + margin-left: -19px + padding-right: 1px + margin-top: 3px + opacity: 0 + cursor: grab + + &:before + padding: 0 + color: #888 + + .grid--area.-widgeted:hover & + opacity: 1 + .grid--area-content height: 100% @@ -54,6 +69,13 @@ $grid--header-width: 20px flex-direction: column height: 100% + input[type="text"].toolbar--editable-toolbar + font-size: 18px + font-weight: normal + + .title-container + margin-bottom: 0px + .grid--widget-content height: 100% overflow-x: auto diff --git a/config/locales/js-en.yml b/config/locales/js-en.yml index 95dca9fe05..4f6dfe7a7c 100644 --- a/config/locales/js-en.yml +++ b/config/locales/js-en.yml @@ -209,7 +209,7 @@ en: work_packages_watched: title: 'Work packages watched by me' work_packages_table: - title: 'Work package table' + title: 'Work packages' work_packages_calendar: title: 'Calendar' diff --git a/frontend/src/app/modules/grids/widgets/abstract-widget.component.ts b/frontend/src/app/modules/grids/widgets/abstract-widget.component.ts index 9e42b24a71..562b62b69a 100644 --- a/frontend/src/app/modules/grids/widgets/abstract-widget.component.ts +++ b/frontend/src/app/modules/grids/widgets/abstract-widget.component.ts @@ -1,4 +1,4 @@ -import {HostBinding, Input, EventEmitter, Output} from "@angular/core"; +import {HostBinding, Input, EventEmitter, Output, HostListener} from "@angular/core"; import {GridWidgetResource} from "app/modules/hal/resources/grid-widget-resource"; import {I18nService} from "core-app/modules/common/i18n/i18n.service"; diff --git a/frontend/src/app/modules/grids/widgets/wp-table/wp-table.component.html b/frontend/src/app/modules/grids/widgets/wp-table/wp-table.component.html new file mode 100644 index 0000000000..944b47edb8 --- /dev/null +++ b/frontend/src/app/modules/grids/widgets/wp-table/wp-table.component.html @@ -0,0 +1,23 @@ +

+ + + + +

+ + + + + diff --git a/frontend/src/app/modules/grids/widgets/wp-table/wp-table.component.ts b/frontend/src/app/modules/grids/widgets/wp-table/wp-table.component.ts index 2a4c824f88..b29a9b7bc5 100644 --- a/frontend/src/app/modules/grids/widgets/wp-table/wp-table.component.ts +++ b/frontend/src/app/modules/grids/widgets/wp-table/wp-table.component.ts @@ -1,25 +1,26 @@ import {Component, OnInit, OnDestroy, ViewChild, AfterViewInit} from "@angular/core"; -import {ApiV3Filter, ApiV3FilterBuilder} from "core-components/api/api-v3/api-v3-filter-builder"; import {WidgetWpListComponent} from "core-app/modules/grids/widgets/wp-widget/wp-widget.component"; import {WorkPackageTableConfiguration} from "core-components/wp-table/wp-table-configuration"; import {QueryResource} from "core-app/modules/hal/resources/query-resource"; import {I18nService} from "core-app/modules/common/i18n/i18n.service"; import {untilComponentDestroyed} from 'ng2-rx-componentdestroyed'; import {WorkPackageIsolatedQuerySpaceDirective} from "core-app/modules/work_packages/query-space/wp-isolated-query-space.directive"; -import {skip} from 'rxjs/operators'; +import {skip, take} from 'rxjs/operators'; import {UrlParamsHelperService} from "core-components/wp-query/url-params-helper"; import {QueryFormDmService} from "core-app/modules/hal/dm-services/query-form-dm.service"; import {QueryDmService} from "core-app/modules/hal/dm-services/query-dm.service"; import {QueryFormResource} from "core-app/modules/hal/resources/query-form-resource"; @Component({ - templateUrl: '../wp-widget/wp-widget.component.html', + templateUrl: './wp-table.component.html', styleUrls: ['../wp-widget/wp-widget.component.css'] }) export class WidgetWpTableComponent extends WidgetWpListComponent implements OnInit, OnDestroy, AfterViewInit { public text = { title: this.i18n.t('js.grid.widgets.work_packages_table.title') }; public queryId:string|null; private queryForm:QueryFormResource; + public inFlight = false; + public query:QueryResource; public configuration:Partial = { actionsColumnEnabled: false, @@ -57,6 +58,19 @@ export class WidgetWpTableComponent extends WidgetWpListComponent implements OnI } ngAfterViewInit() { + this + .querySpaceDirective + .querySpace + .query + .values$() + .pipe( + take(1), + untilComponentDestroyed(this) + ).subscribe((query) => { + this.query = query; + this.queryId = query.id; + }); + this .querySpaceDirective .querySpace @@ -67,14 +81,8 @@ export class WidgetWpTableComponent extends WidgetWpListComponent implements OnI skip(2), untilComponentDestroyed(this) ).subscribe((query) => { - if (this.queryForm) { - this.queryDm.update(query, this.queryForm).toPromise(); - } else { - this.queryFormDm.load(query).then((form) => { - this.queryForm = form; - this.queryDm.update(query, form).toPromise(); - }); - } + this.queryId = query.id; + this.ensureFormAndSaveQuery(query); }); } @@ -82,6 +90,37 @@ export class WidgetWpTableComponent extends WidgetWpListComponent implements OnI // nothing to do } + private ensureFormAndSaveQuery(query:QueryResource) { + if (this.queryForm) { + this.saveQuery(query); + } else { + this.queryFormDm.load(query).then((form) => { + this.queryForm = form; + this.saveQuery(query); + }); + } + } + + public renameQuery(query:QueryResource, value:string) { + query.name = value; + + this.ensureFormAndSaveQuery(query); + } + + private saveQuery(query:QueryResource) { + this.inFlight = true; + + this + .queryDm + .update(query, this.queryForm) + .toPromise() + .then((query) => { + this.inFlight = false; + return query; + }) + .catch(() => this.inFlight = false); + } + private createInitial():Promise { return this.queryFormDm .loadWithParams( @@ -92,6 +131,9 @@ export class WidgetWpTableComponent extends WidgetWpListComponent implements OnI ) .then(form => { const query = this.queryFormDm.buildQueryResource(form); + // set a default title + query.name = this.text.title; + return this.queryDm.create(query, form); }); } diff --git a/frontend/src/app/modules/grids/widgets/wp-widget/wp-widget.component.css b/frontend/src/app/modules/grids/widgets/wp-widget/wp-widget.component.css index fdef700d0e..4bf0fded86 100644 --- a/frontend/src/app/modules/grids/widgets/wp-widget/wp-widget.component.css +++ b/frontend/src/app/modules/grids/widgets/wp-widget/wp-widget.component.css @@ -3,3 +3,11 @@ wp-embedded-table { flex: 1 1 auto; overflow: hidden; } + +.widget-box--header { + display: flex +} + +.widget-box--header .icon-context { + padding-top: 5px +} diff --git a/frontend/src/app/modules/grids/widgets/wp-widget/wp-widget.component.html b/frontend/src/app/modules/grids/widgets/wp-widget/wp-widget.component.html index 0ba32152d9..4c514669a6 100644 --- a/frontend/src/app/modules/grids/widgets/wp-widget/wp-widget.component.html +++ b/frontend/src/app/modules/grids/widgets/wp-widget/wp-widget.component.html @@ -4,7 +4,7 @@ - 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 b97a7c1c8f..5418b76cd0 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,7 +50,12 @@ describe 'Arbitrary WorkPackage query table widget on my page', type: :feature, responsible: user end - let(:role) { FactoryBot.create(:role, permissions: %i[view_work_packages add_work_packages]) } + let(:role) do + FactoryBot.create(:role, + permissions: %i[view_work_packages + add_work_packages + save_queries]) + end let(:user) do FactoryBot.create(:user, @@ -74,11 +79,13 @@ describe 'Arbitrary WorkPackage query table widget on my page', type: :feature, 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 package table") + my_page.add_widget(2, 3, "Work packages") sleep(0.2) - filter_area = Components::Grids::GridArea.new('.grid--area', text: "Work package table") + 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.expect_to_span(2, 3, 5, 4) @@ -115,6 +122,13 @@ describe 'Arbitrary WorkPackage query table widget on my page', type: :feature, 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 + sleep(1) # The whole of the configuration survives a reload @@ -123,7 +137,7 @@ describe 'Arbitrary WorkPackage query table widget on my page', type: :feature, visit root_path my_page.visit! - filter_area = Components::Grids::GridArea.new('.grid--area', text: "Work package table") + 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) @@ -134,5 +148,10 @@ describe 'Arbitrary WorkPackage query table widget on my page', type: :feature, # 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 diff --git a/spec/support/pages/my/page.rb b/spec/support/pages/my/page.rb index d10c8d4cac..8e80d09c0a 100644 --- a/spec/support/pages/my/page.rb +++ b/spec/support/pages/my/page.rb @@ -70,7 +70,7 @@ module Pages expect(page) .to have_content(I18n.t('js.grid.add_modal.choose_widget')) - page.find('.grid--addable-widget', text: name).click + page.find('.grid--addable-widget', text: Regexp.new("^#{name}$")).click end end From 43eed63cab7e599b2818eccc35345bbcf5904488 Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Thu, 4 Apr 2019 08:28:41 +0200 Subject: [PATCH 06/14] fix feature specs --- modules/grids/spec/features/my/my_page_assigned_to_me_spec.rb | 1 + .../spec/features/my/my_page_time_entries_current_user_spec.rb | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/modules/grids/spec/features/my/my_page_assigned_to_me_spec.rb b/modules/grids/spec/features/my/my_page_assigned_to_me_spec.rb index e1a0caf535..031b5b0587 100644 --- a/modules/grids/spec/features/my/my_page_assigned_to_me_spec.rb +++ b/modules/grids/spec/features/my/my_page_assigned_to_me_spec.rb @@ -111,6 +111,7 @@ describe 'Assigned to me embedded query on my page', type: :feature, js: true do hierarchies.disable_via_header hierarchies.expect_no_hierarchies + sleep(0.2) # re-enable hierarchies.enable_via_header diff --git a/modules/grids/spec/features/my/my_page_time_entries_current_user_spec.rb b/modules/grids/spec/features/my/my_page_time_entries_current_user_spec.rb index b67cfc6323..9e203f33cf 100644 --- a/modules/grids/spec/features/my/my_page_time_entries_current_user_spec.rb +++ b/modules/grids/spec/features/my/my_page_time_entries_current_user_spec.rb @@ -88,7 +88,7 @@ describe 'My page time entries current user widget spec', type: :feature, js: tr sleep(0.5) # within top-right area, add an additional widget - my_page.add_widget(1, 1, 'Spent time (last 7 days)') + my_page.add_widget(1, 1, 'Spent time \(last 7 days\)') calendar_area = Components::Grids::GridArea.new('.grid--area', text: 'Spent time (last 7 days)') calendar_area.expect_to_span(1, 1, 4, 3) From 1181d1d4ec2ceb5652e8f521010b517eef82f497 Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Thu, 4 Apr 2019 16:07:16 +0200 Subject: [PATCH 07/14] destroy query if table widget is removed --- modules/grids/app/models/grids/widget.rb | 10 ++++++ modules/grids/lib/grids/configuration.rb | 26 ++++++++++++++ .../lib/grids/my_page_grid_registration.rb | 4 +++ .../grids/spec/models/grids/my_page_spec.rb | 34 +++++++++++++++++-- 4 files changed, 71 insertions(+), 3 deletions(-) diff --git a/modules/grids/app/models/grids/widget.rb b/modules/grids/app/models/grids/widget.rb index bb968ec486..11035e75bd 100644 --- a/modules/grids/app/models/grids/widget.rb +++ b/modules/grids/app/models/grids/widget.rb @@ -34,5 +34,15 @@ module Grids belongs_to :grid serialize :options, Hash + + after_destroy :execute_after_destroy_strategy + + private + + def execute_after_destroy_strategy + proc = Grids::Configuration.widget_strategy(grid, identifier).after_destroy + + instance_exec(&proc) + end end end diff --git a/modules/grids/lib/grids/configuration.rb b/modules/grids/lib/grids/configuration.rb index 705cbb97bb..387786a70a 100644 --- a/modules/grids/lib/grids/configuration.rb +++ b/modules/grids/lib/grids/configuration.rb @@ -95,6 +95,10 @@ class Grids::Configuration (grid_classes || []).include?(grid) end + def widget_strategy(grid, identifier) + grid_register[grid.class.to_s].widget_strategy(identifier) + end + def writable?(grid, user) grid_register[grid.class.to_s]&.writable?(grid, user) end @@ -120,6 +124,18 @@ class Grids::Configuration end end + class WidgetStrategy + class << self + def after_destroy(proc = nil) + if proc + @after_destroy = proc + end + + @after_destroy ||= -> {} + end + end + end + class Registration class << self def grid_class(name_string = nil) @@ -146,6 +162,16 @@ class Grids::Configuration @widgets end + def widget_strategy(widget_name, &block) + @widget_strategies ||= {} + + if block_given? + @widget_strategies[widget_name.to_s] = Class.new(Grids::Configuration::WidgetStrategy, &block) + end + + @widget_strategies[widget_name.to_s] ||= Grids::Configuration::WidgetStrategy + end + def defaults(hash = nil) # This is called during code load, which # may not have the table available. diff --git a/modules/grids/lib/grids/my_page_grid_registration.rb b/modules/grids/lib/grids/my_page_grid_registration.rb index 9ef0f99cbd..c913852f55 100644 --- a/modules/grids/lib/grids/my_page_grid_registration.rb +++ b/modules/grids/lib/grids/my_page_grid_registration.rb @@ -13,6 +13,10 @@ module Grids 'documents', 'news' + widget_strategy 'work_packages_table' do + after_destroy -> { ::Query.find_by(id: options[:queryId])&.destroy } + end + defaults( row_count: 7, column_count: 4, diff --git a/modules/grids/spec/models/grids/my_page_spec.rb b/modules/grids/spec/models/grids/my_page_spec.rb index c1b7117a38..0cd315322c 100644 --- a/modules/grids/spec/models/grids/my_page_spec.rb +++ b/modules/grids/spec/models/grids/my_page_spec.rb @@ -31,18 +31,46 @@ require 'spec_helper' require_relative './shared_model' describe Grids::MyPage, type: :model do - let(:instance) { described_class.new } + let(:instance) { described_class.new(row_count: 5, column_count: 5) } let(:user) { FactoryBot.build_stubbed(:user) } it_behaves_like 'grid attributes' context 'attributes' do - let(:user) { FactoryBot.build_stubbed :user } - it '#user' do instance.user = user expect(instance.user) .to eql user end end + + context 'altering widgets' do + context 'when removing a work_packages_table widget' do + let(:user) { FactoryBot.create(:user) } + let(:query) do + FactoryBot.create(:query, + user: user, + hidden: true) + end + + before do + widget = Grids::Widget.new(identifier: 'work_packages_table', + start_row: 1, + end_row: 2, + start_column: 1, + end_column: 2, + options: { queryId: query.id }) + + instance.widgets = [widget] + instance.save! + end + + it 'removes the widget\'s query' do + instance.widgets = [] + + expect(Query.find_by(id: query.id)) + .to be_nil + end + end + end end From 9b1e7618f939c6bdf5da9c5bac9b4f099c51be8f Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Thu, 4 Apr 2019 16:12:14 +0200 Subject: [PATCH 08/14] extract into own files --- modules/grids/lib/grids/configuration.rb | 110 +-------------- .../lib/grids/configuration/registration.rb | 127 ++++++++++++++++++ .../grids/configuration/widget_strategy.rb | 43 ++++++ 3 files changed, 171 insertions(+), 109 deletions(-) create mode 100644 modules/grids/lib/grids/configuration/registration.rb create mode 100644 modules/grids/lib/grids/configuration/widget_strategy.rb diff --git a/modules/grids/lib/grids/configuration.rb b/modules/grids/lib/grids/configuration.rb index 387786a70a..47a5f7b07b 100644 --- a/modules/grids/lib/grids/configuration.rb +++ b/modules/grids/lib/grids/configuration.rb @@ -28,7 +28,7 @@ # See docs/COPYRIGHT.rdoc for more details. #++ -class Grids::Configuration +module Grids::Configuration class << self def register_grid(grid, klass) @@ -123,112 +123,4 @@ class Grids::Configuration @url_helpers ||= OpenProject::StaticRouting::StaticUrlHelpers.new end end - - class WidgetStrategy - class << self - def after_destroy(proc = nil) - if proc - @after_destroy = proc - end - - @after_destroy ||= -> {} - end - end - end - - class Registration - class << self - def grid_class(name_string = nil) - if name_string - @grid_class = name_string - end - - @grid_class - end - - def to_scope(path = nil) - if path - @to_scope = path - end - - @to_scope - end - - def widgets(*widgets) - if widgets.any? - @widgets = widgets - end - - @widgets - end - - def widget_strategy(widget_name, &block) - @widget_strategies ||= {} - - if block_given? - @widget_strategies[widget_name.to_s] = Class.new(Grids::Configuration::WidgetStrategy, &block) - end - - @widget_strategies[widget_name.to_s] ||= Grids::Configuration::WidgetStrategy - end - - def defaults(hash = nil) - # This is called during code load, which - # may not have the table available. - return unless Grids::Widget.table_exists? - - if hash - @defaults = hash - end - - params = @defaults.dup - params[:widgets] = (params[:widgets] || []).map do |widget| - Grids::Widget.new(widget) - end - - params - end - - def from_scope(_scope) - raise NotImplementedError - end - - def all_scopes - Array(url_helpers.send(@to_scope)) - end - - def visible(_user = User.current) - ::Grids::Grid - .where(type: grid_class) - end - - def writable?(_grid, _user) - true - end - - def register! - unless @grid_class - raise 'Need to define the grid class first. Use grid_class to do so.' - end - unless @widgets - raise 'Need to define at least one widget first. Use widgets to do so.' - end - unless @to_scope - raise 'Need to define a scope. Use to_scope to do so' - end - - Grids::Configuration.register_grid(@grid_class, self) - - widgets.each do |widget| - Grids::Configuration.register_widget(widget, @grid_class) - end - end - - private - - def url_helpers - @url_helpers ||= OpenProject::StaticRouting::StaticUrlHelpers.new - end - end - end end diff --git a/modules/grids/lib/grids/configuration/registration.rb b/modules/grids/lib/grids/configuration/registration.rb new file mode 100644 index 0000000000..95c0e0e201 --- /dev/null +++ b/modules/grids/lib/grids/configuration/registration.rb @@ -0,0 +1,127 @@ +#-- encoding: UTF-8 + +#-- copyright +# OpenProject is a project management system. +# Copyright (C) 2012-2018 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 docs/COPYRIGHT.rdoc for more details. +#++ + +module Grids::Configuration + class Registration + class << self + def grid_class(name_string = nil) + if name_string + @grid_class = name_string + end + + @grid_class + end + + def to_scope(path = nil) + if path + @to_scope = path + end + + @to_scope + end + + def widgets(*widgets) + if widgets.any? + @widgets = widgets + end + + @widgets + end + + def widget_strategy(widget_name, &block) + @widget_strategies ||= {} + + if block_given? + @widget_strategies[widget_name.to_s] = Class.new(Grids::Configuration::WidgetStrategy, &block) + end + + @widget_strategies[widget_name.to_s] ||= Grids::Configuration::WidgetStrategy + end + + def defaults(hash = nil) + # This is called during code load, which + # may not have the table available. + return unless Grids::Widget.table_exists? + + if hash + @defaults = hash + end + + params = @defaults.dup + params[:widgets] = (params[:widgets] || []).map do |widget| + Grids::Widget.new(widget) + end + + params + end + + def from_scope(_scope) + raise NotImplementedError + end + + def all_scopes + Array(url_helpers.send(@to_scope)) + end + + def visible(_user = User.current) + ::Grids::Grid + .where(type: grid_class) + end + + def writable?(_grid, _user) + true + end + + def register! + unless @grid_class + raise 'Need to define the grid class first. Use grid_class to do so.' + end + unless @widgets + raise 'Need to define at least one widget first. Use widgets to do so.' + end + unless @to_scope + raise 'Need to define a scope. Use to_scope to do so' + end + + Grids::Configuration.register_grid(@grid_class, self) + + widgets.each do |widget| + Grids::Configuration.register_widget(widget, @grid_class) + end + end + + private + + def url_helpers + @url_helpers ||= OpenProject::StaticRouting::StaticUrlHelpers.new + end + end + end +end diff --git a/modules/grids/lib/grids/configuration/widget_strategy.rb b/modules/grids/lib/grids/configuration/widget_strategy.rb new file mode 100644 index 0000000000..4c666e6f18 --- /dev/null +++ b/modules/grids/lib/grids/configuration/widget_strategy.rb @@ -0,0 +1,43 @@ +#-- encoding: UTF-8 + +#-- copyright +# OpenProject is a project management system. +# Copyright (C) 2012-2018 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 docs/COPYRIGHT.rdoc for more details. +#++ + +module Grids::Configuration + class WidgetStrategy + class << self + def after_destroy(proc = nil) + if proc + @after_destroy = proc + end + + @after_destroy ||= -> {} + end + end + end +end From f32393e43592334ee4025e675b026d3e6ab4e375 Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Thu, 4 Apr 2019 22:11:18 +0200 Subject: [PATCH 09/14] correctly denote allowed widgets in schema/form --- docs/api/apiv3/endpoints/grids.apib | 12 +++++++++++ .../app/contracts/grids/base_contract.rb | 20 ++++++++++++++----- .../grids/schemas/grid_schema_representer.rb | 5 +++++ modules/grids/lib/grids/configuration.rb | 6 ++++++ .../contracts/grids/create_contract_spec.rb | 13 ++++++++++++ .../schemas/grid_schema_representer_spec.rb | 13 +++--------- 6 files changed, 54 insertions(+), 15 deletions(-) diff --git a/docs/api/apiv3/endpoints/grids.apib b/docs/api/apiv3/endpoints/grids.apib index 7032f3ba46..9cfdcc0338 100644 --- a/docs/api/apiv3/endpoints/grids.apib +++ b/docs/api/apiv3/endpoints/grids.apib @@ -582,6 +582,18 @@ A page link must be provided in the body when calling this end point. "required": true, "hasDefault": false, "writable": true, + "_embedded": { + allowedValues: [ + { + "_type": "GridWidget", + "identifier": "work_packages_assigned" + }, + { + "_type": "GridWidget", + "identifier": "news" + } + ] + }, "_links": {} }, "_links": {} diff --git a/modules/grids/app/contracts/grids/base_contract.rb b/modules/grids/app/contracts/grids/base_contract.rb index 1058c5b4c0..238602fe49 100644 --- a/modules/grids/app/contracts/grids/base_contract.rb +++ b/modules/grids/app/contracts/grids/base_contract.rb @@ -63,12 +63,14 @@ module Grids Grid end - def assignable_values(_column, _user) - nil + def assignable_values(column, _user) + if column == :widgets + config.all_widget_identifiers(grid_class) + end end def edit_allowed? - Grids::Configuration.writable?(model, user) + config.writable?(model, user) end private @@ -81,10 +83,10 @@ module Grids end def validate_registered_widgets - return unless Grids::Configuration.registered_grid?(model.class) + return unless config.registered_grid?(grid_class) undestroyed_widgets.each do |widget| - next if Grids::Configuration.allowed_widget?(model.class, widget.identifier) + next if config.allowed_widget?(grid_class, widget.identifier) errors.add(:widgets, :inclusion) end @@ -177,5 +179,13 @@ module Grids def undestroyed_widgets model.widgets.reject(&:marked_for_destruction?) end + + def grid_class + model.class + end + + def config + Grids::Configuration + end end end diff --git a/modules/grids/app/controllers/api/v3/grids/schemas/grid_schema_representer.rb b/modules/grids/app/controllers/api/v3/grids/schemas/grid_schema_representer.rb index 8c2909d7de..ad5ffc594b 100644 --- a/modules/grids/app/controllers/api/v3/grids/schemas/grid_schema_representer.rb +++ b/modules/grids/app/controllers/api/v3/grids/schemas/grid_schema_representer.rb @@ -86,6 +86,11 @@ module API required: true, has_default: false, visibility: false, + values_callback: -> do + represented.assignable_values(:widgets, current_user).map do |identifier| + OpenStruct.new(identifier: identifier) + end + end, value_representer: ::API::V3::Grids::WidgetRepresenter, link_factory: false diff --git a/modules/grids/lib/grids/configuration.rb b/modules/grids/lib/grids/configuration.rb index 47a5f7b07b..1ef3f4781f 100644 --- a/modules/grids/lib/grids/configuration.rb +++ b/modules/grids/lib/grids/configuration.rb @@ -95,6 +95,12 @@ module Grids::Configuration (grid_classes || []).include?(grid) end + def all_widget_identifiers(grid) + registered_widget_by_identifier.select do |_, grid_classes| + grid_classes.include?(grid) + end.keys + end + def widget_strategy(grid, identifier) grid_register[grid.class.to_s].widget_strategy(identifier) end diff --git a/modules/grids/spec/contracts/grids/create_contract_spec.rb b/modules/grids/spec/contracts/grids/create_contract_spec.rb index 7e765048e8..5fc8b38a46 100644 --- a/modules/grids/spec/contracts/grids/create_contract_spec.rb +++ b/modules/grids/spec/contracts/grids/create_contract_spec.rb @@ -94,6 +94,19 @@ describe Grids::CreateContract do end end + context 'for widgets' do + it 'calls the grid configuration for the available values' do + widgets = double('widgets') + + allow(Grids::Configuration) + .to receive(:all_widget_identifiers) + .and_return(widgets) + + expect(instance.assignable_values(:widgets, user)) + .to eql widgets + end + end + context 'for something else' do it 'returns nil' do expect(instance.assignable_values(:something, user)) diff --git a/modules/grids/spec/lib/api/v3/grids/schemas/grid_schema_representer_spec.rb b/modules/grids/spec/lib/api/v3/grids/schemas/grid_schema_representer_spec.rb index 4fcf094ac6..da1c4eee72 100644 --- a/modules/grids/spec/lib/api/v3/grids/schemas/grid_schema_representer_spec.rb +++ b/modules/grids/spec/lib/api/v3/grids/schemas/grid_schema_representer_spec.rb @@ -38,14 +38,7 @@ describe ::API::V3::Grids::Schemas::GridSchemaRepresenter do let(:new_record) { true } let(:allowed_scopes) { %w(/some/path /some/other/path) } let(:allowed_widgets) do - [ - OpenStruct.new( - identifier: 'first_widget' - ), - OpenStruct.new( - identifier: 'second_widget' - ) - ] + %w(first_widget second_widget) end let(:contract) do contract = double('contract') @@ -172,9 +165,9 @@ describe ::API::V3::Grids::Schemas::GridSchemaRepresenter do end it 'embeds the allowed values' do - allowed_widgets.each_with_index do |widget, index| + allowed_widgets.each_with_index do |identifier, index| href_path = "#{path}/_embedded/allowedValues/#{index}/identifier" - is_expected.to be_json_eql(widget[:identifier].to_json).at_path(href_path) + is_expected.to be_json_eql(identifier.to_json).at_path(href_path) end end end From 1f01189ededb14a98b7f0b138891d0abbbddc4d9 Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Thu, 4 Apr 2019 22:40:08 +0200 Subject: [PATCH 10/14] 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 From 02e26879acaec8ef425bf2fdaf4dc77b6f557221 Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Fri, 5 Apr 2019 07:54:26 +0200 Subject: [PATCH 11/14] increase spec robustness --- modules/grids/spec/features/my/my_page_assigned_to_me_spec.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/modules/grids/spec/features/my/my_page_assigned_to_me_spec.rb b/modules/grids/spec/features/my/my_page_assigned_to_me_spec.rb index 031b5b0587..85331360f0 100644 --- a/modules/grids/spec/features/my/my_page_assigned_to_me_spec.rb +++ b/modules/grids/spec/features/my/my_page_assigned_to_me_spec.rb @@ -112,9 +112,12 @@ describe 'Assigned to me embedded query on my page', type: :feature, js: true do hierarchies.expect_no_hierarchies sleep(0.2) + # re-enable hierarchies.enable_via_header + sleep(0.2) + hierarchies.expect_mode_enabled hierarchies.expect_hierarchy_at assigned_work_package, collapsed: true end From aaa7216d806fe5dfa17d41a5d811777309407491 Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Fri, 5 Apr 2019 08:46:10 +0200 Subject: [PATCH 12/14] split my_page module off of grids --- Gemfile.lock | 7 + Gemfile.modules | 1 + .../grids/schemas/grid_schema_representer.rb | 3 +- modules/grids/bin/rails | 14 - modules/grids/lib/grids/engine.rb | 4 - .../contracts/grids/create_contract_spec.rb | 26 +- .../spec/contracts/grids/shared_examples.rb | 256 +---------- .../contracts/grids/update_contract_spec.rb | 2 +- modules/grids/spec/factories/grid_factory.rb | 24 - .../grid_payload_representer_parsing_spec.rb | 4 +- .../grids/grid_representer_rendering_spec.rb | 25 +- .../grids/grids_create_form_resource_spec.rb | 140 ------ .../api/v3/grids/grids_resource_spec.rb | 353 --------------- .../grids/grids_update_form_resource_spec.rb | 124 +----- .../services/grids/create_service_spec.rb | 4 +- .../grids/set_attributes_service_spec.rb | 2 +- .../services/grids/update_service_spec.rb | 2 +- modules/my_page/.gitignore | 7 + modules/my_page/Gemfile | 3 + .../app/models/grids/my_page.rb | 0 modules/my_page/lib/my_page.rb | 4 + modules/my_page/lib/my_page/engine.rb | 11 + .../lib/my_page/grid_registration.rb} | 4 +- modules/my_page/my_page.gemspec | 12 + .../contracts/grids/create_contract_spec.rb | 61 +++ .../spec/contracts/grids/shared_examples.rb | 320 ++++++++++++++ .../contracts/grids/update_contract_spec.rb | 39 ++ .../my_page/spec/factories/grid_factory.rb | 25 ++ .../spec/features/my/accountable_spec.rb} | 0 .../spec/features/my/assigned_to_me_spec.rb} | 0 .../spec/features/my/documents_spec.rb} | 0 .../spec/features/my/my_page_spec.rb | 0 .../spec/features/my/news_spec.rb} | 0 .../my/time_entries_current_user_spec.rb} | 0 .../features/my/work_package_table_spec.rb} | 0 .../spec/models/grids/my_page_spec.rb | 0 .../my_page/spec/models/grids/shared_model.rb | 77 ++++ .../grids/filters/scope_filter_spec.rb | 0 .../queries/grids/query_integration_spec.rb | 0 .../grids/grids_create_form_resource_spec.rb | 193 ++++++++ .../api/v3/grids/grids_resource_spec.rb | 414 ++++++++++++++++++ .../grids/grids_update_form_resource_spec.rb | 174 ++++++++ 42 files changed, 1383 insertions(+), 952 deletions(-) delete mode 100755 modules/grids/bin/rails create mode 100644 modules/my_page/.gitignore create mode 100644 modules/my_page/Gemfile rename modules/{grids => my_page}/app/models/grids/my_page.rb (100%) create mode 100644 modules/my_page/lib/my_page.rb create mode 100644 modules/my_page/lib/my_page/engine.rb rename modules/{grids/lib/grids/my_page_grid_registration.rb => my_page/lib/my_page/grid_registration.rb} (93%) create mode 100644 modules/my_page/my_page.gemspec create mode 100644 modules/my_page/spec/contracts/grids/create_contract_spec.rb create mode 100644 modules/my_page/spec/contracts/grids/shared_examples.rb create mode 100644 modules/my_page/spec/contracts/grids/update_contract_spec.rb create mode 100644 modules/my_page/spec/factories/grid_factory.rb rename modules/{grids/spec/features/my/my_page_accountable_spec.rb => my_page/spec/features/my/accountable_spec.rb} (100%) rename modules/{grids/spec/features/my/my_page_assigned_to_me_spec.rb => my_page/spec/features/my/assigned_to_me_spec.rb} (100%) rename modules/{grids/spec/features/my/my_page_documents_spec.rb => my_page/spec/features/my/documents_spec.rb} (100%) rename modules/{grids => my_page}/spec/features/my/my_page_spec.rb (100%) rename modules/{grids/spec/features/my/my_page_news_spec.rb => my_page/spec/features/my/news_spec.rb} (100%) rename modules/{grids/spec/features/my/my_page_time_entries_current_user_spec.rb => my_page/spec/features/my/time_entries_current_user_spec.rb} (100%) rename modules/{grids/spec/features/my/my_page_work_package_table_spec.rb => my_page/spec/features/my/work_package_table_spec.rb} (100%) rename modules/{grids => my_page}/spec/models/grids/my_page_spec.rb (100%) create mode 100644 modules/my_page/spec/models/grids/shared_model.rb rename modules/{grids => my_page}/spec/queries/grids/filters/scope_filter_spec.rb (100%) rename modules/{grids => my_page}/spec/queries/grids/query_integration_spec.rb (100%) create mode 100644 modules/my_page/spec/requests/api/v3/grids/grids_create_form_resource_spec.rb create mode 100644 modules/my_page/spec/requests/api/v3/grids/grids_resource_spec.rb create mode 100644 modules/my_page/spec/requests/api/v3/grids/grids_update_form_resource_spec.rb diff --git a/Gemfile.lock b/Gemfile.lock index e9579cee39..dc49639537 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -180,6 +180,12 @@ PATH openproject-meeting (1.0.0) icalendar (~> 2.5.0) +PATH + remote: modules/my_page + specs: + my_page (1.0.0) + grids + PATH remote: modules/my_project_page specs: @@ -970,6 +976,7 @@ DEPENDENCIES lograge (~> 0.10.0) meta-tags (~> 2.11.0) multi_json (~> 1.13.1) + my_page! mysql2 (~> 0.5.0) net-ldap (~> 0.16.0) newrelic_rpm diff --git a/Gemfile.modules b/Gemfile.modules index ff05bcd148..e6608033d8 100644 --- a/Gemfile.modules +++ b/Gemfile.modules @@ -40,6 +40,7 @@ group :opf_plugins do gem 'openproject-ldap_groups', path: 'modules/ldap_groups' gem 'grids', path: 'modules/grids' + gem 'my_page', path: 'modules/my_page' gem 'openproject-boards', path: 'modules/boards' gem 'openproject-bim_seeder', path: 'modules/bim_seeder', require: !!(ENV['OPENPROJECT_EDITION'] == 'bim') diff --git a/modules/grids/app/controllers/api/v3/grids/schemas/grid_schema_representer.rb b/modules/grids/app/controllers/api/v3/grids/schemas/grid_schema_representer.rb index ad5ffc594b..c8343832b1 100644 --- a/modules/grids/app/controllers/api/v3/grids/schemas/grid_schema_representer.rb +++ b/modules/grids/app/controllers/api/v3/grids/schemas/grid_schema_representer.rb @@ -76,8 +76,7 @@ module API value_representer: false, link_factory: ->(path) { { - href: path, - title: I18n.t(:label_my_page) + href: path } } diff --git a/modules/grids/bin/rails b/modules/grids/bin/rails deleted file mode 100755 index b676279b95..0000000000 --- a/modules/grids/bin/rails +++ /dev/null @@ -1,14 +0,0 @@ -#!/usr/bin/env ruby -# This command will automatically be run when you run "rails" with Rails gems -# installed from the root of your application. - -ENGINE_ROOT = File.expand_path('../..', __FILE__) -ENGINE_PATH = File.expand_path('../../lib/grids/engine', __FILE__) -APP_PATH = File.expand_path('../../test/dummy/config/application', __FILE__) - -# Set up gems listed in the Gemfile. -ENV['BUNDLE_GEMFILE'] ||= File.expand_path('../../Gemfile', __FILE__) -require 'bundler/setup' if File.exist?(ENV['BUNDLE_GEMFILE']) - -require 'rails/all' -require 'rails/engine/commands' diff --git a/modules/grids/lib/grids/engine.rb b/modules/grids/lib/grids/engine.rb index d0653721bb..5a91abee35 100644 --- a/modules/grids/lib/grids/engine.rb +++ b/modules/grids/lib/grids/engine.rb @@ -9,9 +9,5 @@ module Grids Queries::Register.filter query, Grids::Filters::ScopeFilter end - - config.to_prepare do - Grids::MyPageGridRegistration.register! - end end end diff --git a/modules/grids/spec/contracts/grids/create_contract_spec.rb b/modules/grids/spec/contracts/grids/create_contract_spec.rb index 16511c79c0..c2e6a18392 100644 --- a/modules/grids/spec/contracts/grids/create_contract_spec.rb +++ b/modules/grids/spec/contracts/grids/create_contract_spec.rb @@ -38,9 +38,11 @@ describe Grids::CreateContract do it_behaves_like 'shared grid contract attributes' describe 'type' do + let(:grid) { FactoryBot.build_stubbed(:grid, default_values) } + it_behaves_like 'is writable' do let(:attribute) { :type } - let(:value) { 'Grids::MyPage' } + let(:value) { 'Grids::Grid' } end end @@ -51,15 +53,6 @@ describe Grids::CreateContract do let(:attribute) { :user_id } let(:value) { 5 } end - - context 'for a Grids::MyPage' do - let(:grid) { FactoryBot.build_stubbed(:my_page, default_values) } - - it_behaves_like 'is writable' do - let(:attribute) { :user_id } - let(:value) { 5 } - end - end end describe 'project_id' do @@ -69,15 +62,6 @@ describe Grids::CreateContract do let(:attribute) { :project_id } let(:value) { 5 } end - - context 'for a Grids::MyPage' do - let(:grid) { FactoryBot.build_stubbed(:my_page, default_values) } - - it_behaves_like 'is not writable' do - let(:attribute) { :project_id } - let(:value) { 5 } - end - end end describe '#assignable_values' do @@ -104,12 +88,12 @@ describe Grids::CreateContract do allow(Grids::Configuration) .to receive(:allowed_widget?) - .with(Grids::MyPage, :widget1, user) + .with(Grids::Grid, :widget1, user) .and_return(true) allow(Grids::Configuration) .to receive(:allowed_widget?) - .with(Grids::MyPage, :widget2, user) + .with(Grids::Grid, :widget2, user) .and_return(false) expect(instance.assignable_values(:widgets, user)) diff --git a/modules/grids/spec/contracts/grids/shared_examples.rb b/modules/grids/spec/contracts/grids/shared_examples.rb index 4e9324db81..c01a216d75 100644 --- a/modules/grids/spec/contracts/grids/shared_examples.rb +++ b/modules/grids/spec/contracts/grids/shared_examples.rb @@ -39,7 +39,7 @@ shared_context 'grid contract' do } end let(:grid) do - FactoryBot.build_stubbed(:my_page, default_values) + FactoryBot.build_stubbed(:grid, default_values) end shared_examples_for 'validates positive integer' do @@ -113,261 +113,7 @@ shared_examples_for 'shared grid contract attributes' do end end - describe 'widgets' do - it_behaves_like 'is writable' do - let(:attribute) { :widgets } - let(:value) do - [ - Grids::Widget.new(start_row: 1, - end_row: 4, - start_column: 2, - end_column: 5, - identifier: 'work_packages_assigned') - ] - end - end - - context 'invalid identifier' do - before do - grid.widgets.build(start_row: 1, - end_row: 4, - start_column: 2, - end_column: 5, - identifier: 'bogus_identifier') - end - - it 'is invalid' do - expect(instance.validate) - .to be_falsey - end - - it 'notes the error' do - instance.validate - expect(instance.errors.details[:widgets]) - .to match_array [{ error: :inclusion }] - end - end - - context 'collisions between widgets' do - before do - grid.widgets.build(start_row: 1, - end_row: 3, - start_column: 1, - end_column: 3, - identifier: 'work_packages_assigned') - grid.widgets.build(start_row: 2, - end_row: 4, - start_column: 2, - end_column: 4, - identifier: 'work_packages_created') - end - - it 'is invalid' do - expect(instance.validate) - .to be_falsey - end - - it 'notes the error' do - instance.validate - expect(instance.errors.details[:widgets]) - .to match_array [{ error: :overlaps }, { error: :overlaps }] - end - end - - context 'widgets having the same start column as another\'s end column' do - before do - grid.widgets.build(start_row: 1, - end_row: 3, - start_column: 1, - end_column: 3, - identifier: 'work_packages_assigned') - grid.widgets.build(start_row: 1, - end_row: 3, - start_column: 3, - end_column: 4, - identifier: 'work_packages_created') - end - - it 'is valid' do - expect(instance.validate) - .to be_truthy - end - end - - context 'widgets having the same start row as another\'s end row' do - before do - grid.widgets.build(start_row: 1, - end_row: 3, - start_column: 1, - end_column: 3, - identifier: 'work_packages_assigned') - grid.widgets.build(start_row: 3, - end_row: 4, - start_column: 1, - end_column: 3, - identifier: 'work_packages_created') - end - - it 'is valid' do - expect(instance.validate) - .to be_truthy - end - end - - context 'widgets being outside (max) of the grid' do - before do - grid.widgets.build(start_row: 1, - end_row: grid.row_count + 2, - start_column: 1, - end_column: 3, - identifier: 'work_packages_assigned') - end - - it 'is invalid' do - expect(instance.validate) - .to be_falsey - end - - it 'notes the error' do - instance.validate - expect(instance.errors.details[:widgets]) - .to match_array [{ error: :outside }] - end - end - - context 'widgets being outside (min) of the grid' do - before do - grid.widgets.build(start_row: 1, - end_row: 2, - start_column: -1, - end_column: 3, - identifier: 'work_packages_assigned') - end - - it 'is invalid' do - expect(instance.validate) - .to be_falsey - end - - it 'notes the error' do - instance.validate - expect(instance.errors.details[:widgets]) - .to match_array [{ error: :outside }] - end - end - - context 'widgets spanning the whole grid' do - before do - grid.widgets.build(start_row: 1, - end_row: grid.row_count + 1, - start_column: 1, - end_column: grid.column_count + 1, - identifier: 'work_packages_assigned') - end - - it 'is valid' do - expect(instance.validate) - .to be_truthy - end - end - - context 'widgets having start after end column' do - before do - grid.widgets.build(start_row: 1, - end_row: 2, - start_column: 4, - end_column: 3, - identifier: 'work_packages_assigned') - end - - it 'is invalid' do - expect(instance.validate) - .to be_falsey - end - - it 'notes the error' do - instance.validate - expect(instance.errors.details[:widgets]) - .to match_array [{ error: :end_before_start }] - end - end - - context 'widgets having start after end row' do - before do - grid.widgets.build(start_row: 4, - end_row: 2, - start_column: 1, - end_column: 3, - identifier: 'work_packages_assigned') - end - - it 'is invalid' do - expect(instance.validate) - .to be_falsey - end - - it 'notes the error' do - instance.validate - expect(instance.errors.details[:widgets]) - .to match_array [{ error: :end_before_start }] - end - end - - context 'widgets having start equals end column' do - before do - grid.widgets.build(start_row: 1, - end_row: 2, - start_column: 4, - end_column: 3, - identifier: 'work_packages_assigned') - end - - it 'is invalid' do - expect(instance.validate) - .to be_falsey - end - - it 'notes the error' do - instance.validate - expect(instance.errors.details[:widgets]) - .to match_array [{ error: :end_before_start }] - end - end - - context 'widgets having start equals end row' do - before do - grid.widgets.build(start_row: 2, - end_row: 2, - start_column: 1, - end_column: 3, - identifier: 'work_packages_assigned') - end - - it 'is invalid' do - expect(instance.validate) - .to be_falsey - end - - it 'notes the error' do - instance.validate - expect(instance.errors.details[:widgets]) - .to match_array [{ error: :end_before_start }] - end - end - end - describe 'valid grid subclasses' do - context 'for a registered subclass' do - let(:grid) do - FactoryBot.build_stubbed(:my_page, default_values) - end - - it 'is valid' do - expect(instance.validate) - .to be_truthy - end - end - context 'for the Grid superclass itself' do let(:grid) do FactoryBot.build_stubbed(:grid, default_values) diff --git a/modules/grids/spec/contracts/grids/update_contract_spec.rb b/modules/grids/spec/contracts/grids/update_contract_spec.rb index 1202f02a1a..5ed4ddec1f 100644 --- a/modules/grids/spec/contracts/grids/update_contract_spec.rb +++ b/modules/grids/spec/contracts/grids/update_contract_spec.rb @@ -51,7 +51,7 @@ describe Grids::UpdateContract do instance.validate # scope because that is what type is called on the outside for grids expect(instance.errors.details[:scope]) - .to match_array [{ error: :error_readonly }] + .to match_array [{ error: :error_readonly }, { error: :inclusion }] end end diff --git a/modules/grids/spec/factories/grid_factory.rb b/modules/grids/spec/factories/grid_factory.rb index d0749efaff..2250ad598c 100644 --- a/modules/grids/spec/factories/grid_factory.rb +++ b/modules/grids/spec/factories/grid_factory.rb @@ -1,28 +1,4 @@ FactoryBot.define do factory :grid, class: Grids::Grid do end - - factory :my_page, class: Grids::MyPage do - user - row_count { 7 } - column_count { 4 } - widgets do - [ - Grids::Widget.new( - identifier: 'work_packages_assigned', - start_row: 1, - end_row: 7, - start_column: 1, - end_column: 3 - ), - Grids::Widget.new( - identifier: 'work_packages_created', - start_row: 1, - end_row: 7, - start_column: 3, - end_column: 5 - ) - ] - end - end end diff --git a/modules/grids/spec/lib/api/v3/grids/grid_payload_representer_parsing_spec.rb b/modules/grids/spec/lib/api/v3/grids/grid_payload_representer_parsing_spec.rb index 308d6b88ed..8cd431bb76 100644 --- a/modules/grids/spec/lib/api/v3/grids/grid_payload_representer_parsing_spec.rb +++ b/modules/grids/spec/lib/api/v3/grids/grid_payload_representer_parsing_spec.rb @@ -71,7 +71,7 @@ describe ::API::V3::Grids::GridPayloadRepresenter, 'parsing' do ], "_links" => { "scope" => { - "href" => my_page_path + "href" => 'some_path' } } } @@ -82,7 +82,7 @@ describe ::API::V3::Grids::GridPayloadRepresenter, 'parsing' do it 'updates page' do grid = representer.from_hash(hash) expect(grid.scope) - .to eql(my_page_path) + .to eql('some_path') end end end diff --git a/modules/grids/spec/lib/api/v3/grids/grid_representer_rendering_spec.rb b/modules/grids/spec/lib/api/v3/grids/grid_representer_rendering_spec.rb index b17313eabe..4b6477722a 100644 --- a/modules/grids/spec/lib/api/v3/grids/grid_representer_rendering_spec.rb +++ b/modules/grids/spec/lib/api/v3/grids/grid_representer_rendering_spec.rb @@ -33,7 +33,7 @@ describe ::API::V3::Grids::GridRepresenter, 'rendering' do let(:grid) do FactoryBot.build_stubbed( - :my_page, + :grid, row_count: 4, column_count: 5, widgets: [ @@ -68,6 +68,21 @@ describe ::API::V3::Grids::GridRepresenter, 'rendering' do let(:current_user) { FactoryBot.build_stubbed(:user) } let(:representer) { described_class.new(grid, current_user: current_user) } + let(:writable) { true } + let(:scope_path) { 'bogus_scope' } + + before do + allow(::Grids::Configuration) + .to receive(:writable?) + .with(grid, current_user) + .and_return(writable) + + allow(::Grids::Configuration) + .to receive(:to_scope) + .with(Grids::Grid, []) + .and_return(scope_path) + end + context 'generation' do subject(:generated) { representer.to_json } @@ -78,12 +93,6 @@ describe ::API::V3::Grids::GridRepresenter, 'rendering' do .at_path('_type') end - it 'identifies the url the grid is stored for' do - is_expected - .to be_json_eql(my_page_path.to_json) - .at_path('_links/scope/href') - end - it 'has an id' do is_expected .to be_json_eql(grid.id) @@ -180,7 +189,7 @@ describe ::API::V3::Grids::GridRepresenter, 'rendering' do context 'scope link' do it_behaves_like 'has an untitled link' do let(:link) { 'scope' } - let(:href) { my_page_path } + let(:href) { scope_path } let(:type) { "text/html" } it 'has a content type of html' do diff --git a/modules/grids/spec/requests/api/v3/grids/grids_create_form_resource_spec.rb b/modules/grids/spec/requests/api/v3/grids/grids_create_form_resource_spec.rb index 0252eb017f..6047409c80 100644 --- a/modules/grids/spec/requests/api/v3/grids/grids_create_form_resource_spec.rb +++ b/modules/grids/spec/requests/api/v3/grids/grids_create_form_resource_spec.rb @@ -61,16 +61,6 @@ describe "POST /api/v3/grids/form", type: :request, content_type: :json do .at_path('_type') end - it 'contains a Schema embedding the available values' do - expect(subject.body) - .to be_json_eql("Schema".to_json) - .at_path('_embedded/schema/_type') - - expect(subject.body) - .to be_json_eql(my_page_path.to_json) - .at_path('_embedded/schema/scope/_links/allowedValues/0/href') - end - it 'contains default data in the payload' do expected = { "rowCount": 4, @@ -95,135 +85,5 @@ describe "POST /api/v3/grids/form", type: :request, content_type: :json do expect(subject.body) .not_to have_json_path('_links/commit') end - - context 'with /my/page for the scope value' do - let(:params) do - { - '_links': { - 'scope': { - 'href': my_page_path - } - } - } - end - - it 'contains default data in the payload' do - expected = { - "rowCount": 7, - "columnCount": 4, - "options": {}, - "widgets": [ - { - "_type": "GridWidget", - identifier: 'work_packages_assigned', - "options": {}, - startRow: 1, - endRow: 7, - startColumn: 1, - endColumn: 3 - }, - { - "_type": "GridWidget", - identifier: 'work_packages_created', - "options": {}, - startRow: 1, - endRow: 7, - startColumn: 3, - endColumn: 5 - } - ], - "_links": { - "scope": { - "href": "/my/page", - "type": "text/html" - } - } - } - - expect(subject.body) - .to be_json_eql(expected.to_json) - .at_path('_embedded/payload') - end - - it 'has no validationErrors' do - expect(subject.body) - .to be_json_eql({}.to_json) - .at_path('_embedded/validationErrors') - end - - it 'has a commit link' do - expect(subject.body) - .to be_json_eql(api_v3_paths.grids.to_json) - .at_path('_links/commit/href') - end - end - - context 'with an unsupported widget identifier' do - let(:params) do - { - '_links': { - 'scope': { - 'href': my_page_path - } - }, - "widgets": [ - { - "_type": "GridWidget", - "identifier": "bogus_identifier", - "startRow": 4, - "endRow": 5, - "startColumn": 1, - "endColumn": 2 - } - ] - } - end - - it 'has a validationError on widget' do - expect(subject.body) - .to be_json_eql("Widgets is not set to one of the allowed values.".to_json) - .at_path('_embedded/validationErrors/widgets/message') - end - end - - context 'with name set' do - let(:params) do - { - name: 'My custom grid 1', - '_links': { - 'scope': { - 'href': my_page_path - } - } - } - end - - it 'feeds it back' do - expect(subject.body) - .to be_json_eql("My custom grid 1".to_json) - .at_path('_embedded/payload/name') - end - end - - context 'with options set' do - let(:params) do - { - options: { - foo: 'bar' - }, - '_links': { - 'scope': { - 'href': my_page_path - } - } - } - end - - it 'feeds them back' do - expect(subject.body) - .to be_json_eql("bar".to_json) - .at_path('_embedded/payload/options/foo') - end - end end end diff --git a/modules/grids/spec/requests/api/v3/grids/grids_resource_spec.rb b/modules/grids/spec/requests/api/v3/grids/grids_resource_spec.rb index 427df74513..3b7309406c 100644 --- a/modules/grids/spec/requests/api/v3/grids/grids_resource_spec.rb +++ b/modules/grids/spec/requests/api/v3/grids/grids_resource_spec.rb @@ -37,12 +37,6 @@ describe 'API v3 Grids resource', type: :request, content_type: :json do FactoryBot.create(:user) end - let(:my_page_grid) { FactoryBot.create(:my_page, user: current_user) } - let(:other_user) do - FactoryBot.create(:user) - end - let(:other_my_page_grid) { FactoryBot.create(:my_page, user: other_user) } - before do login_as(current_user) end @@ -52,369 +46,22 @@ describe 'API v3 Grids resource', type: :request, content_type: :json do describe '#get INDEX' do let(:path) { api_v3_paths.grids } - let(:stored_grids) do - my_page_grid - other_my_page_grid - end - - before do - stored_grids - - get path - end - - it 'responds with 200 OK' do - expect(subject.status).to eq(200) - end - - it 'sends a collection of grids but only those visible to the current user' do - expect(subject.body) - .to be_json_eql('Collection'.to_json) - .at_path('_type') - - expect(subject.body) - .to be_json_eql('Grid'.to_json) - .at_path('_embedded/elements/0/_type') - - expect(subject.body) - .to be_json_eql(1.to_json) - .at_path('total') - end - - context 'with a filter on the scope attribute' do - shared_let(:other_grid) do - grid = Grids::Grid.new(row_count: 20, - column_count: 20) - grid.save - - Grids::Grid - .where(id: grid.id) - .update_all(user_id: current_user.id) - - grid - end - - let(:stored_grids) do - my_page_grid - other_my_page_grid - other_grid - end - - let(:path) do - filter = [{ 'scope' => - { - 'operator' => '=', - 'values' => [my_page_path] - } }] - - "#{api_v3_paths.grids}?#{{ filters: filter.to_json }.to_query}" - end - - it 'responds with 200 OK' do - expect(subject.status).to eq(200) - end - - it 'sends only the my page of the current user' do - expect(subject.body) - .to be_json_eql('Collection'.to_json) - .at_path('_type') - - expect(subject.body) - .to be_json_eql('Grid'.to_json) - .at_path('_embedded/elements/0/_type') - - expect(subject.body) - .to be_json_eql(1.to_json) - .at_path('total') - end - end - end - - describe '#get' do - let(:path) { api_v3_paths.grid(my_page_grid.id) } - - let(:stored_grids) do - my_page_grid - end - before do - stored_grids - get path end it 'responds with 200 OK' do expect(subject.status).to eq(200) end - - it 'sends a grid block' do - expect(subject.body) - .to be_json_eql('Grid'.to_json) - .at_path('_type') - end - - it 'identifies the url the grid is stored for' do - expect(subject.body) - .to be_json_eql(my_page_path.to_json) - .at_path('_links/scope/href') - end - - context 'with the page not existing' do - let(:path) { api_v3_paths.grid(5) } - - it 'responds with 404 NOT FOUND' do - expect(subject.status).to eql 404 - end - end - - context 'with the grid belonging to someone else' do - let(:stored_grids) do - my_page_grid - other_my_page_grid - end - - let(:path) { api_v3_paths.grid(other_my_page_grid.id) } - - it 'responds with 404 NOT FOUND' do - expect(subject.status).to eql 404 - end - end - end - - describe '#patch' do - let(:path) { api_v3_paths.grid(my_page_grid.id) } - - let(:params) do - { - "rowCount": 10, - "name": 'foo', - "columnCount": 15, - "widgets": [{ - "identifier": "work_packages_assigned", - "startRow": 4, - "endRow": 8, - "startColumn": 2, - "endColumn": 5 - }] - }.with_indifferent_access - end - - let(:stored_grids) do - my_page_grid - end - - before do - stored_grids - - patch path, params.to_json, 'CONTENT_TYPE' => 'application/json' - end - - it 'responds with 200 OK' do - expect(subject.status).to eq(200) - end - - it 'returns the altered grid block' do - expect(subject.body) - .to be_json_eql('Grid'.to_json) - .at_path('_type') - expect(subject.body) - .to be_json_eql('foo'.to_json) - .at_path('name') - expect(subject.body) - .to be_json_eql(params['rowCount'].to_json) - .at_path('rowCount') - expect(subject.body) - .to be_json_eql(params['widgets'][0]['identifier'].to_json) - .at_path('widgets/0/identifier') - end - - it 'perists the changes' do - expect(my_page_grid.reload.row_count) - .to eql params['rowCount'] - end - - context 'with invalid params' do - let(:params) do - { - "rowCount": -5, - "columnCount": 15, - "widgets": [{ - "identifier": "work_packages_assigned", - "startRow": 4, - "endRow": 8, - "startColumn": 2, - "endColumn": 5 - }] - }.with_indifferent_access - end - - it 'responds with 422 and mentions the error' do - expect(subject.status).to eq 422 - - expect(subject.body) - .to be_json_eql('Error'.to_json) - .at_path('_type') - - expect(subject.body) - .to be_json_eql("Widgets is outside of the grid.".to_json) - .at_path('_embedded/errors/0/message') - - expect(subject.body) - .to be_json_eql("Number of rows must be greater than 0.".to_json) - .at_path('_embedded/errors/1/message') - end - - it 'does not persist the changes to widgets' do - expect(my_page_grid.reload.widgets.count) - .to eql Grids::MyPageGridRegistration.defaults[:widgets].size - end - end - - context 'with a scope param' do - let(:params) do - { - "_links": { - "scope": { - "href": '' - } - } - }.with_indifferent_access - end - - it 'responds with 422 and mentions the error' do - expect(subject.status).to eq 422 - - expect(subject.body) - .to be_json_eql('Error'.to_json) - .at_path('_type') - - expect(subject.body) - .to be_json_eql("You must not write a read-only attribute.".to_json) - .at_path('message') - - expect(subject.body) - .to be_json_eql("scope".to_json) - .at_path('_embedded/details/attribute') - end - end - - context 'with the page not existing' do - let(:path) { api_v3_paths.grid(5) } - - it 'responds with 404 NOT FOUND' do - expect(subject.status).to eql 404 - end - end - - context 'with the grid belonging to someone else' do - let(:stored_grids) do - my_page_grid - other_my_page_grid - end - - let(:path) { api_v3_paths.grid(other_my_page_grid.id) } - - it 'responds with 404 NOT FOUND' do - expect(subject.status).to eql 404 - end - end end describe '#post' do let(:path) { api_v3_paths.grids } - let(:params) do - { - "rowCount": 10, - "columnCount": 15, - "widgets": [{ - "identifier": "work_packages_assigned", - "startRow": 4, - "endRow": 8, - "startColumn": 2, - "endColumn": 5 - }], - "_links": { - "scope": { - "href": my_page_path - } - } - }.with_indifferent_access - end - before do post path, params.to_json, 'CONTENT_TYPE' => 'application/json' end - it 'responds with 201 CREATED' do - expect(subject.status).to eq(201) - end - - it 'returns the created grid block' do - expect(subject.body) - .to be_json_eql('Grid'.to_json) - .at_path('_type') - expect(subject.body) - .to be_json_eql(params['rowCount'].to_json) - .at_path('rowCount') - expect(subject.body) - .to be_json_eql(params['widgets'][0]['identifier'].to_json) - .at_path('widgets/0/identifier') - end - - it 'persists the grid' do - expect(Grids::Grid.count) - .to eql(1) - end - - context 'with invalid params' do - let(:params) do - { - "rowCount": -5, - "columnCount": "sdjfksdfsdfdsf", - "widgets": [{ - "identifier": "work_packages_assigned", - "startRow": 4, - "endRow": 8, - "startColumn": 2, - "endColumn": 5 - }], - "_links": { - "scope": { - "href": my_page_path - } - } - }.with_indifferent_access - end - - it 'responds with 422' do - expect(subject.status).to eq(422) - end - - it 'does not create a grid' do - expect(Grids::Grid.count) - .to eql(0) - end - - it 'returns the errors' do - expect(subject.body) - .to be_json_eql('Error'.to_json) - .at_path('_type') - - expect(subject.body) - .to be_json_eql("Widgets is outside of the grid.".to_json) - .at_path('_embedded/errors/0/message') - - expect(subject.body) - .to be_json_eql("Number of rows must be greater than 0.".to_json) - .at_path('_embedded/errors/1/message') - - expect(subject.body) - .to be_json_eql("Number of columns must be greater than 0.".to_json) - .at_path('_embedded/errors/2/message') - end - end - context 'without a page link' do let(:params) do { diff --git a/modules/grids/spec/requests/api/v3/grids/grids_update_form_resource_spec.rb b/modules/grids/spec/requests/api/v3/grids/grids_update_form_resource_spec.rb index 5d4fba3b4c..4110d6999e 100644 --- a/modules/grids/spec/requests/api/v3/grids/grids_update_form_resource_spec.rb +++ b/modules/grids/spec/requests/api/v3/grids/grids_update_form_resource_spec.rb @@ -37,10 +37,6 @@ describe "PATCH /api/v3/grids/:id/form", type: :request, content_type: :json do FactoryBot.create(:user) end - let(:grid) do - FactoryBot.create(:my_page, user: current_user) - end - let(:path) { api_v3_paths.grid_form(grid.id) } let(:params) { {} } subject(:response) { last_response } @@ -53,126 +49,8 @@ describe "PATCH /api/v3/grids/:id/form", type: :request, content_type: :json do post path, params.to_json, 'CONTENT_TYPE' => 'application/json' end - it 'returns 200 OK' do - expect(subject.status) - .to eql 200 - end - - it 'is of type form' do - expect(subject.body) - .to be_json_eql("Form".to_json) - .at_path('_type') - end - - it 'contains a Schema disallowing setting scope' do - expect(subject.body) - .to be_json_eql("Schema".to_json) - .at_path('_embedded/schema/_type') - - expect(subject.body) - .to be_json_eql(false.to_json) - .at_path('_embedded/schema/scope/writable') - end - - it 'contains the current data in the payload' do - expected = { - rowCount: 7, - columnCount: 4, - options: {}, - widgets: [ - { - "_type": "GridWidget", - identifier: 'work_packages_assigned', - options: {}, - startRow: 1, - endRow: 7, - startColumn: 1, - endColumn: 3 - }, - { - "_type": "GridWidget", - identifier: 'work_packages_created', - options: {}, - startRow: 1, - endRow: 7, - startColumn: 3, - endColumn: 5 - } - ], - "_links": { - "scope": { - "href": "/my/page", - "type": "text/html" - } - } - } - - expect(subject.body) - .to be_json_eql(expected.to_json) - .at_path('_embedded/payload') - end - - it 'has a commit link' do - expect(subject.body) - .to be_json_eql(api_v3_paths.grid(grid.id).to_json) - .at_path('_links/commit/href') - end - - context 'with some value for the scope value' do - let(:params) do - { - '_links': { - 'scope': { - 'href': '/some/path' - } - } - } - end - - it 'has a validation error on scope as the value is not writeable' do - expect(subject.body) - .to be_json_eql("You must not write a read-only attribute.".to_json) - .at_path('_embedded/validationErrors/scope/message') - end - end - - context 'with an unsupported widget identifier' do - let(:params) do - { - "widgets": [ - { - "_type": "GridWidget", - "identifier": "bogus_identifier", - "startRow": 4, - "endRow": 5, - "startColumn": 1, - "endColumn": 2 - } - ] - } - end - - it 'has a validationError on widget' do - expect(subject.body) - .to be_json_eql("Widgets is not set to one of the allowed values.".to_json) - .at_path('_embedded/validationErrors/widgets/message') - end - end - context 'for a non existing grid' do - let(:path) { api_v3_paths.grid_form(grid.id + 5) } - - it 'returns 404 NOT FOUND' do - expect(subject.status) - .to eql 404 - end - end - - context 'for another user\'s grid' do - let(:other_user) { FactoryBot.create(:user) } - let(:other_grid) { FactoryBot.create(:my_page, user: other_user) } - - let(:path) { api_v3_paths.grid_form(other_grid.id) } + let(:path) { api_v3_paths.grid_form(5) } it 'returns 404 NOT FOUND' do expect(subject.status) diff --git a/modules/grids/spec/services/grids/create_service_spec.rb b/modules/grids/spec/services/grids/create_service_spec.rb index 8b25eac30e..a052bfb6fd 100644 --- a/modules/grids/spec/services/grids/create_service_spec.rb +++ b/modules/grids/spec/services/grids/create_service_spec.rb @@ -50,7 +50,9 @@ describe Grids::CreateService, type: :model do end let(:scope) { "some/scope/url" } let(:call_attributes) { { scope: scope } } - let(:grid_class) { Grids::MyPage } + let(:grid_class) do + Grids::Grid + end let(:set_attributes_success) do true end diff --git a/modules/grids/spec/services/grids/set_attributes_service_spec.rb b/modules/grids/spec/services/grids/set_attributes_service_spec.rb index d420c29958..12edfcb0d1 100644 --- a/modules/grids/spec/services/grids/set_attributes_service_spec.rb +++ b/modules/grids/spec/services/grids/set_attributes_service_spec.rb @@ -56,7 +56,7 @@ describe Grids::SetAttributesService, type: :model do contract_class: contract_class) end let(:call_attributes) { {} } - let(:grid_class) { Grids::MyPage } + let(:grid_class) { Grids::Grid } let(:grid) do FactoryBot.build_stubbed(grid_class.name.demodulize.underscore.to_sym, widgets: []) end diff --git a/modules/grids/spec/services/grids/update_service_spec.rb b/modules/grids/spec/services/grids/update_service_spec.rb index 84e3319d12..f6a1df81b4 100644 --- a/modules/grids/spec/services/grids/update_service_spec.rb +++ b/modules/grids/spec/services/grids/update_service_spec.rb @@ -42,7 +42,7 @@ describe Grids::UpdateService, type: :model do contract_class: contract_class) end let(:call_attributes) { {} } - let(:grid_class) { Grids::MyPage } + let(:grid_class) { Grids::Grid } let(:set_attributes_success) do true end diff --git a/modules/my_page/.gitignore b/modules/my_page/.gitignore new file mode 100644 index 0000000000..71f82cb25d --- /dev/null +++ b/modules/my_page/.gitignore @@ -0,0 +1,7 @@ +.bundle/ +log/*.log +pkg/ +test/dummy/db/*.sqlite3 +test/dummy/db/*.sqlite3-journal +test/dummy/log/*.log +test/dummy/tmp/ diff --git a/modules/my_page/Gemfile b/modules/my_page/Gemfile new file mode 100644 index 0000000000..fa75df1563 --- /dev/null +++ b/modules/my_page/Gemfile @@ -0,0 +1,3 @@ +source 'https://rubygems.org' + +gemspec diff --git a/modules/grids/app/models/grids/my_page.rb b/modules/my_page/app/models/grids/my_page.rb similarity index 100% rename from modules/grids/app/models/grids/my_page.rb rename to modules/my_page/app/models/grids/my_page.rb diff --git a/modules/my_page/lib/my_page.rb b/modules/my_page/lib/my_page.rb new file mode 100644 index 0000000000..ab49814387 --- /dev/null +++ b/modules/my_page/lib/my_page.rb @@ -0,0 +1,4 @@ +require "my_page/engine" + +module MyPage +end diff --git a/modules/my_page/lib/my_page/engine.rb b/modules/my_page/lib/my_page/engine.rb new file mode 100644 index 0000000000..83a02d3b22 --- /dev/null +++ b/modules/my_page/lib/my_page/engine.rb @@ -0,0 +1,11 @@ +module MyPage + class Engine < ::Rails::Engine + isolate_namespace MyPage + + include OpenProject::Plugins::ActsAsOpEngine + + config.to_prepare do + MyPage::GridRegistration.register! + end + end +end diff --git a/modules/grids/lib/grids/my_page_grid_registration.rb b/modules/my_page/lib/my_page/grid_registration.rb similarity index 93% rename from modules/grids/lib/grids/my_page_grid_registration.rb rename to modules/my_page/lib/my_page/grid_registration.rb index 56f9c7c791..3f9290e77a 100644 --- a/modules/grids/lib/grids/my_page_grid_registration.rb +++ b/modules/my_page/lib/my_page/grid_registration.rb @@ -1,5 +1,5 @@ -module Grids - class MyPageGridRegistration < ::Grids::Configuration::Registration +module MyPage + class GridRegistration < ::Grids::Configuration::Registration grid_class 'Grids::MyPage' to_scope :my_page_path diff --git a/modules/my_page/my_page.gemspec b/modules/my_page/my_page.gemspec new file mode 100644 index 0000000000..b550f341b9 --- /dev/null +++ b/modules/my_page/my_page.gemspec @@ -0,0 +1,12 @@ +# encoding: UTF-8 + +Gem::Specification.new do |s| + s.name = "my_page" + s.version = '1.0.0' + s.authors = ["OpenProject"] + s.summary = "OpenProject MyPage." + + s.files = Dir["{app,config,db,lib}/**/*"] + + s.add_dependency 'grids' +end diff --git a/modules/my_page/spec/contracts/grids/create_contract_spec.rb b/modules/my_page/spec/contracts/grids/create_contract_spec.rb new file mode 100644 index 0000000000..41d9a31e13 --- /dev/null +++ b/modules/my_page/spec/contracts/grids/create_contract_spec.rb @@ -0,0 +1,61 @@ +#-- encoding: UTF-8 + +#-- 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. +#++ + +require 'spec_helper' +require_relative './shared_examples' + +describe Grids::CreateContract do + include_context 'grid contract' + include_context 'model contract' + + it_behaves_like 'shared grid contract attributes' + + describe 'user_id' do + context 'for a Grids::MyPage' do + let(:grid) { FactoryBot.build_stubbed(:my_page, default_values) } + + it_behaves_like 'is writable' do + let(:attribute) { :user_id } + let(:value) { 5 } + end + end + end + + describe 'project_id' do + context 'for a Grids::MyPage' do + let(:grid) { FactoryBot.build_stubbed(:my_page, default_values) } + + it_behaves_like 'is not writable' do + let(:attribute) { :project_id } + let(:value) { 5 } + end + end + end +end diff --git a/modules/my_page/spec/contracts/grids/shared_examples.rb b/modules/my_page/spec/contracts/grids/shared_examples.rb new file mode 100644 index 0000000000..441637b37f --- /dev/null +++ b/modules/my_page/spec/contracts/grids/shared_examples.rb @@ -0,0 +1,320 @@ +#-- encoding: UTF-8 + +#-- 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. +#++ + +shared_context 'grid contract' do + let(:user) { FactoryBot.build_stubbed(:user) } + let(:instance) { described_class.new(grid, user) } + let(:default_values) do + { + row_count: 6, + column_count: 7, + widgets: [] + } + end + let(:grid) do + FactoryBot.build_stubbed(:my_page, default_values) + end +end + +shared_examples_for 'shared grid contract attributes' do + include_context 'model contract' + let(:model) { grid } + + describe 'widgets' do + it_behaves_like 'is writable' do + let(:attribute) { :widgets } + let(:value) do + [ + Grids::Widget.new(start_row: 1, + end_row: 4, + start_column: 2, + end_column: 5, + identifier: 'work_packages_assigned') + ] + end + end + + context 'invalid identifier' do + before do + grid.widgets.build(start_row: 1, + end_row: 4, + start_column: 2, + end_column: 5, + identifier: 'bogus_identifier') + end + + it 'is invalid' do + expect(instance.validate) + .to be_falsey + end + + it 'notes the error' do + instance.validate + expect(instance.errors.details[:widgets]) + .to match_array [{ error: :inclusion }] + end + end + + context 'collisions between widgets' do + before do + grid.widgets.build(start_row: 1, + end_row: 3, + start_column: 1, + end_column: 3, + identifier: 'work_packages_assigned') + grid.widgets.build(start_row: 2, + end_row: 4, + start_column: 2, + end_column: 4, + identifier: 'work_packages_created') + end + + it 'is invalid' do + expect(instance.validate) + .to be_falsey + end + + it 'notes the error' do + instance.validate + expect(instance.errors.details[:widgets]) + .to match_array [{ error: :overlaps }, { error: :overlaps }] + end + end + + context 'widgets having the same start column as another\'s end column' do + before do + grid.widgets.build(start_row: 1, + end_row: 3, + start_column: 1, + end_column: 3, + identifier: 'work_packages_assigned') + grid.widgets.build(start_row: 1, + end_row: 3, + start_column: 3, + end_column: 4, + identifier: 'work_packages_created') + end + + it 'is valid' do + expect(instance.validate) + .to be_truthy + end + end + + context 'widgets having the same start row as another\'s end row' do + before do + grid.widgets.build(start_row: 1, + end_row: 3, + start_column: 1, + end_column: 3, + identifier: 'work_packages_assigned') + grid.widgets.build(start_row: 3, + end_row: 4, + start_column: 1, + end_column: 3, + identifier: 'work_packages_created') + end + + it 'is valid' do + expect(instance.validate) + .to be_truthy + end + end + + context 'widgets being outside (max) of the grid' do + before do + grid.widgets.build(start_row: 1, + end_row: grid.row_count + 2, + start_column: 1, + end_column: 3, + identifier: 'work_packages_assigned') + end + + it 'is invalid' do + expect(instance.validate) + .to be_falsey + end + + it 'notes the error' do + instance.validate + expect(instance.errors.details[:widgets]) + .to match_array [{ error: :outside }] + end + end + + context 'widgets being outside (min) of the grid' do + before do + grid.widgets.build(start_row: 1, + end_row: 2, + start_column: -1, + end_column: 3, + identifier: 'work_packages_assigned') + end + + it 'is invalid' do + expect(instance.validate) + .to be_falsey + end + + it 'notes the error' do + instance.validate + expect(instance.errors.details[:widgets]) + .to match_array [{ error: :outside }] + end + end + + context 'widgets spanning the whole grid' do + before do + grid.widgets.build(start_row: 1, + end_row: grid.row_count + 1, + start_column: 1, + end_column: grid.column_count + 1, + identifier: 'work_packages_assigned') + end + + it 'is valid' do + expect(instance.validate) + .to be_truthy + end + end + + context 'widgets having start after end column' do + before do + grid.widgets.build(start_row: 1, + end_row: 2, + start_column: 4, + end_column: 3, + identifier: 'work_packages_assigned') + end + + it 'is invalid' do + expect(instance.validate) + .to be_falsey + end + + it 'notes the error' do + instance.validate + expect(instance.errors.details[:widgets]) + .to match_array [{ error: :end_before_start }] + end + end + + context 'widgets having start after end row' do + before do + grid.widgets.build(start_row: 4, + end_row: 2, + start_column: 1, + end_column: 3, + identifier: 'work_packages_assigned') + end + + it 'is invalid' do + expect(instance.validate) + .to be_falsey + end + + it 'notes the error' do + instance.validate + expect(instance.errors.details[:widgets]) + .to match_array [{ error: :end_before_start }] + end + end + + context 'widgets having start equals end column' do + before do + grid.widgets.build(start_row: 1, + end_row: 2, + start_column: 4, + end_column: 3, + identifier: 'work_packages_assigned') + end + + it 'is invalid' do + expect(instance.validate) + .to be_falsey + end + + it 'notes the error' do + instance.validate + expect(instance.errors.details[:widgets]) + .to match_array [{ error: :end_before_start }] + end + end + + context 'widgets having start equals end row' do + before do + grid.widgets.build(start_row: 2, + end_row: 2, + start_column: 1, + end_column: 3, + identifier: 'work_packages_assigned') + end + + it 'is invalid' do + expect(instance.validate) + .to be_falsey + end + + it 'notes the error' do + instance.validate + expect(instance.errors.details[:widgets]) + .to match_array [{ error: :end_before_start }] + end + end + end + + describe 'valid grid subclasses' do + context 'for a registered subclass' do + let(:grid) do + FactoryBot.build_stubbed(:my_page, default_values) + end + + it 'is valid' do + expect(instance.validate) + .to be_truthy + end + end + + context 'for the Grid superclass itself' do + let(:grid) do + FactoryBot.build_stubbed(:grid, default_values) + end + + before do + instance.validate + end + + it 'is invalid for the grid superclass itself' do + expect(instance.errors.details[:scope]) + .to match_array [{ error: :inclusion }] + end + end + end +end diff --git a/modules/my_page/spec/contracts/grids/update_contract_spec.rb b/modules/my_page/spec/contracts/grids/update_contract_spec.rb new file mode 100644 index 0000000000..65cded75c0 --- /dev/null +++ b/modules/my_page/spec/contracts/grids/update_contract_spec.rb @@ -0,0 +1,39 @@ +#-- encoding: UTF-8 + +#-- 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. +#++ + +require 'spec_helper' +require_relative './shared_examples' + +describe Grids::UpdateContract do + include_context 'model contract' + include_context 'grid contract' + + it_behaves_like 'shared grid contract attributes' +end diff --git a/modules/my_page/spec/factories/grid_factory.rb b/modules/my_page/spec/factories/grid_factory.rb new file mode 100644 index 0000000000..d56d1e6c38 --- /dev/null +++ b/modules/my_page/spec/factories/grid_factory.rb @@ -0,0 +1,25 @@ +FactoryBot.define do + factory :my_page, class: Grids::MyPage do + user + row_count { 7 } + column_count { 4 } + widgets do + [ + Grids::Widget.new( + identifier: 'work_packages_assigned', + start_row: 1, + end_row: 7, + start_column: 1, + end_column: 3 + ), + Grids::Widget.new( + identifier: 'work_packages_created', + start_row: 1, + end_row: 7, + start_column: 3, + end_column: 5 + ) + ] + end + end +end diff --git a/modules/grids/spec/features/my/my_page_accountable_spec.rb b/modules/my_page/spec/features/my/accountable_spec.rb similarity index 100% rename from modules/grids/spec/features/my/my_page_accountable_spec.rb rename to modules/my_page/spec/features/my/accountable_spec.rb diff --git a/modules/grids/spec/features/my/my_page_assigned_to_me_spec.rb b/modules/my_page/spec/features/my/assigned_to_me_spec.rb similarity index 100% rename from modules/grids/spec/features/my/my_page_assigned_to_me_spec.rb rename to modules/my_page/spec/features/my/assigned_to_me_spec.rb diff --git a/modules/grids/spec/features/my/my_page_documents_spec.rb b/modules/my_page/spec/features/my/documents_spec.rb similarity index 100% rename from modules/grids/spec/features/my/my_page_documents_spec.rb rename to modules/my_page/spec/features/my/documents_spec.rb diff --git a/modules/grids/spec/features/my/my_page_spec.rb b/modules/my_page/spec/features/my/my_page_spec.rb similarity index 100% rename from modules/grids/spec/features/my/my_page_spec.rb rename to modules/my_page/spec/features/my/my_page_spec.rb diff --git a/modules/grids/spec/features/my/my_page_news_spec.rb b/modules/my_page/spec/features/my/news_spec.rb similarity index 100% rename from modules/grids/spec/features/my/my_page_news_spec.rb rename to modules/my_page/spec/features/my/news_spec.rb diff --git a/modules/grids/spec/features/my/my_page_time_entries_current_user_spec.rb b/modules/my_page/spec/features/my/time_entries_current_user_spec.rb similarity index 100% rename from modules/grids/spec/features/my/my_page_time_entries_current_user_spec.rb rename to modules/my_page/spec/features/my/time_entries_current_user_spec.rb diff --git a/modules/grids/spec/features/my/my_page_work_package_table_spec.rb b/modules/my_page/spec/features/my/work_package_table_spec.rb similarity index 100% rename from modules/grids/spec/features/my/my_page_work_package_table_spec.rb rename to modules/my_page/spec/features/my/work_package_table_spec.rb diff --git a/modules/grids/spec/models/grids/my_page_spec.rb b/modules/my_page/spec/models/grids/my_page_spec.rb similarity index 100% rename from modules/grids/spec/models/grids/my_page_spec.rb rename to modules/my_page/spec/models/grids/my_page_spec.rb diff --git a/modules/my_page/spec/models/grids/shared_model.rb b/modules/my_page/spec/models/grids/shared_model.rb new file mode 100644 index 0000000000..3e45334302 --- /dev/null +++ b/modules/my_page/spec/models/grids/shared_model.rb @@ -0,0 +1,77 @@ +#-- copyright +# OpenProject is a project management system. +# Copyright (C) 2012-2018 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 docs/COPYRIGHT.rdoc for more details. +#++ + +shared_examples_for 'grid attributes' do + describe 'attributes' do + it '#row_count' do + instance.row_count = 5 + expect(instance.row_count) + .to eql 5 + end + + it '#column_count' do + instance.column_count = 5 + expect(instance.column_count) + .to eql 5 + end + + it '#name' do + instance.name = 'custom 123' + expect(instance.name) + .to eql 'custom 123' + + # can be empty + instance.name = nil + expect(instance).to be_valid + end + + it '#options' do + value = { + some: 'value', + and: { + also: 1 + } + } + + instance.options = value + expect(instance.options) + .to eql value + end + + it '#widgets' do + widgets = [ + Grids::Widget.new(start_row: 2), + Grids::Widget.new(start_row: 5) + ] + + instance.widgets = widgets + expect(instance.widgets) + .to match_array widgets + end + end +end diff --git a/modules/grids/spec/queries/grids/filters/scope_filter_spec.rb b/modules/my_page/spec/queries/grids/filters/scope_filter_spec.rb similarity index 100% rename from modules/grids/spec/queries/grids/filters/scope_filter_spec.rb rename to modules/my_page/spec/queries/grids/filters/scope_filter_spec.rb diff --git a/modules/grids/spec/queries/grids/query_integration_spec.rb b/modules/my_page/spec/queries/grids/query_integration_spec.rb similarity index 100% rename from modules/grids/spec/queries/grids/query_integration_spec.rb rename to modules/my_page/spec/queries/grids/query_integration_spec.rb diff --git a/modules/my_page/spec/requests/api/v3/grids/grids_create_form_resource_spec.rb b/modules/my_page/spec/requests/api/v3/grids/grids_create_form_resource_spec.rb new file mode 100644 index 0000000000..3b8f97ed52 --- /dev/null +++ b/modules/my_page/spec/requests/api/v3/grids/grids_create_form_resource_spec.rb @@ -0,0 +1,193 @@ +#-- copyright +# OpenProject is a project management system. +# Copyright (C) 2012-2018 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 docs/COPYRIGHT.rdoc for more details. +#++ + +require 'spec_helper' +require 'rack/test' + +describe "POST /api/v3/grids/form", type: :request, content_type: :json do + include Rack::Test::Methods + include API::V3::Utilities::PathHelper + + shared_let(:current_user) do + FactoryBot.create(:user) + end + + let(:path) { api_v3_paths.create_grid_form } + let(:params) { {} } + subject(:response) { last_response } + + before do + login_as(current_user) + end + + describe '#post' do + before do + post path, params.to_json, 'CONTENT_TYPE' => 'application/json' + end + + it 'contains a Schema embedding the available values' do + expect(subject.body) + .to be_json_eql("Schema".to_json) + .at_path('_embedded/schema/_type') + + expect(subject.body) + .to be_json_eql(my_page_path.to_json) + .at_path('_embedded/schema/scope/_links/allowedValues/0/href') + end + + context 'with /my/page for the scope value' do + let(:params) do + { + '_links': { + 'scope': { + 'href': my_page_path + } + } + } + end + + it 'contains default data in the payload' do + expected = { + "rowCount": 7, + "columnCount": 4, + "options": {}, + "widgets": [ + { + "_type": "GridWidget", + identifier: 'work_packages_assigned', + "options": {}, + startRow: 1, + endRow: 7, + startColumn: 1, + endColumn: 3 + }, + { + "_type": "GridWidget", + identifier: 'work_packages_created', + "options": {}, + startRow: 1, + endRow: 7, + startColumn: 3, + endColumn: 5 + } + ], + "_links": { + "scope": { + "href": "/my/page", + "type": "text/html" + } + } + } + + expect(subject.body) + .to be_json_eql(expected.to_json) + .at_path('_embedded/payload') + end + + it 'has no validationErrors' do + expect(subject.body) + .to be_json_eql({}.to_json) + .at_path('_embedded/validationErrors') + end + + it 'has a commit link' do + expect(subject.body) + .to be_json_eql(api_v3_paths.grids.to_json) + .at_path('_links/commit/href') + end + end + + context 'with an unsupported widget identifier' do + let(:params) do + { + '_links': { + 'scope': { + 'href': my_page_path + } + }, + "widgets": [ + { + "_type": "GridWidget", + "identifier": "bogus_identifier", + "startRow": 4, + "endRow": 5, + "startColumn": 1, + "endColumn": 2 + } + ] + } + end + + it 'has a validationError on widget' do + expect(subject.body) + .to be_json_eql("Widgets is not set to one of the allowed values.".to_json) + .at_path('_embedded/validationErrors/widgets/message') + end + end + + context 'with name set' do + let(:params) do + { + name: 'My custom grid 1', + '_links': { + 'scope': { + 'href': my_page_path + } + } + } + end + + it 'feeds it back' do + expect(subject.body) + .to be_json_eql("My custom grid 1".to_json) + .at_path('_embedded/payload/name') + end + end + + context 'with options set' do + let(:params) do + { + options: { + foo: 'bar' + }, + '_links': { + 'scope': { + 'href': my_page_path + } + } + } + end + + it 'feeds them back' do + expect(subject.body) + .to be_json_eql("bar".to_json) + .at_path('_embedded/payload/options/foo') + end + end + end +end diff --git a/modules/my_page/spec/requests/api/v3/grids/grids_resource_spec.rb b/modules/my_page/spec/requests/api/v3/grids/grids_resource_spec.rb new file mode 100644 index 0000000000..7a43a68209 --- /dev/null +++ b/modules/my_page/spec/requests/api/v3/grids/grids_resource_spec.rb @@ -0,0 +1,414 @@ +#-- copyright +# OpenProject is a project management system. +# Copyright (C) 2012-2018 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 docs/COPYRIGHT.rdoc for more details. +#++ + +require 'spec_helper' +require 'rack/test' + +describe 'API v3 Grids resource', type: :request, content_type: :json do + include Rack::Test::Methods + include API::V3::Utilities::PathHelper + + shared_let(:current_user) do + FactoryBot.create(:user) + end + + let(:my_page_grid) { FactoryBot.create(:my_page, user: current_user) } + let(:other_user) do + FactoryBot.create(:user) + end + let(:other_my_page_grid) { FactoryBot.create(:my_page, user: other_user) } + + before do + login_as(current_user) + end + + subject(:response) { last_response } + + describe '#get INDEX' do + let(:path) { api_v3_paths.grids } + + let(:stored_grids) do + my_page_grid + other_my_page_grid + end + + before do + stored_grids + + get path + end + + it 'sends a collection of grids but only those visible to the current user' do + expect(subject.body) + .to be_json_eql('Collection'.to_json) + .at_path('_type') + + expect(subject.body) + .to be_json_eql('Grid'.to_json) + .at_path('_embedded/elements/0/_type') + + expect(subject.body) + .to be_json_eql(1.to_json) + .at_path('total') + end + + context 'with a filter on the scope attribute' do + shared_let(:other_grid) do + grid = Grids::Grid.new(row_count: 20, + column_count: 20) + grid.save + + Grids::Grid + .where(id: grid.id) + .update_all(user_id: current_user.id) + + grid + end + + let(:stored_grids) do + my_page_grid + other_my_page_grid + other_grid + end + + let(:path) do + filter = [{ 'scope' => + { + 'operator' => '=', + 'values' => [my_page_path] + } }] + + "#{api_v3_paths.grids}?#{{ filters: filter.to_json }.to_query}" + end + + it 'responds with 200 OK' do + expect(subject.status).to eq(200) + end + + it 'sends only the my page of the current user' do + expect(subject.body) + .to be_json_eql('Collection'.to_json) + .at_path('_type') + + expect(subject.body) + .to be_json_eql('Grid'.to_json) + .at_path('_embedded/elements/0/_type') + + expect(subject.body) + .to be_json_eql(1.to_json) + .at_path('total') + end + end + end + + describe '#get' do + let(:path) { api_v3_paths.grid(my_page_grid.id) } + + let(:stored_grids) do + my_page_grid + end + + before do + stored_grids + + get path + end + + it 'responds with 200 OK' do + expect(subject.status).to eq(200) + end + + it 'sends a grid block' do + expect(subject.body) + .to be_json_eql('Grid'.to_json) + .at_path('_type') + end + + it 'identifies the url the grid is stored for' do + expect(subject.body) + .to be_json_eql(my_page_path.to_json) + .at_path('_links/scope/href') + end + + context 'with the page not existing' do + let(:path) { api_v3_paths.grid(5) } + + it 'responds with 404 NOT FOUND' do + expect(subject.status).to eql 404 + end + end + + context 'with the grid belonging to someone else' do + let(:stored_grids) do + my_page_grid + other_my_page_grid + end + + let(:path) { api_v3_paths.grid(other_my_page_grid.id) } + + it 'responds with 404 NOT FOUND' do + expect(subject.status).to eql 404 + end + end + end + + describe '#patch' do + let(:path) { api_v3_paths.grid(my_page_grid.id) } + + let(:params) do + { + "rowCount": 10, + "name": 'foo', + "columnCount": 15, + "widgets": [{ + "identifier": "work_packages_assigned", + "startRow": 4, + "endRow": 8, + "startColumn": 2, + "endColumn": 5 + }] + }.with_indifferent_access + end + + let(:stored_grids) do + my_page_grid + end + + before do + stored_grids + + patch path, params.to_json, 'CONTENT_TYPE' => 'application/json' + end + + it 'responds with 200 OK' do + expect(subject.status).to eq(200) + end + + it 'returns the altered grid block' do + expect(subject.body) + .to be_json_eql('Grid'.to_json) + .at_path('_type') + expect(subject.body) + .to be_json_eql('foo'.to_json) + .at_path('name') + expect(subject.body) + .to be_json_eql(params['rowCount'].to_json) + .at_path('rowCount') + expect(subject.body) + .to be_json_eql(params['widgets'][0]['identifier'].to_json) + .at_path('widgets/0/identifier') + end + + it 'perists the changes' do + expect(my_page_grid.reload.row_count) + .to eql params['rowCount'] + end + + context 'with invalid params' do + let(:params) do + { + "rowCount": -5, + "columnCount": 15, + "widgets": [{ + "identifier": "work_packages_assigned", + "startRow": 4, + "endRow": 8, + "startColumn": 2, + "endColumn": 5 + }] + }.with_indifferent_access + end + + it 'responds with 422 and mentions the error' do + expect(subject.status).to eq 422 + + expect(subject.body) + .to be_json_eql('Error'.to_json) + .at_path('_type') + + expect(subject.body) + .to be_json_eql("Widgets is outside of the grid.".to_json) + .at_path('_embedded/errors/0/message') + + expect(subject.body) + .to be_json_eql("Number of rows must be greater than 0.".to_json) + .at_path('_embedded/errors/1/message') + end + + it 'does not persist the changes to widgets' do + expect(my_page_grid.reload.widgets.count) + .to eql MyPage::GridRegistration.defaults[:widgets].size + end + end + + context 'with a scope param' do + let(:params) do + { + "_links": { + "scope": { + "href": '' + } + } + }.with_indifferent_access + end + + it 'responds with 422 and mentions the error' do + expect(subject.status).to eq 422 + + expect(subject.body) + .to be_json_eql('Error'.to_json) + .at_path('_type') + + expect(subject.body) + .to be_json_eql("You must not write a read-only attribute.".to_json) + .at_path('message') + + expect(subject.body) + .to be_json_eql("scope".to_json) + .at_path('_embedded/details/attribute') + end + end + + context 'with the page not existing' do + let(:path) { api_v3_paths.grid(5) } + + it 'responds with 404 NOT FOUND' do + expect(subject.status).to eql 404 + end + end + + context 'with the grid belonging to someone else' do + let(:stored_grids) do + my_page_grid + other_my_page_grid + end + + let(:path) { api_v3_paths.grid(other_my_page_grid.id) } + + it 'responds with 404 NOT FOUND' do + expect(subject.status).to eql 404 + end + end + end + + describe '#post' do + let(:path) { api_v3_paths.grids } + + let(:params) do + { + "rowCount": 10, + "columnCount": 15, + "widgets": [{ + "identifier": "work_packages_assigned", + "startRow": 4, + "endRow": 8, + "startColumn": 2, + "endColumn": 5 + }], + "_links": { + "scope": { + "href": my_page_path + } + } + }.with_indifferent_access + end + + before do + post path, params.to_json, 'CONTENT_TYPE' => 'application/json' + end + + it 'responds with 201 CREATED' do + expect(subject.status).to eq(201) + end + + it 'returns the created grid block' do + expect(subject.body) + .to be_json_eql('Grid'.to_json) + .at_path('_type') + expect(subject.body) + .to be_json_eql(params['rowCount'].to_json) + .at_path('rowCount') + expect(subject.body) + .to be_json_eql(params['widgets'][0]['identifier'].to_json) + .at_path('widgets/0/identifier') + end + + it 'persists the grid' do + expect(Grids::Grid.count) + .to eql(1) + end + + context 'with invalid params' do + let(:params) do + { + "rowCount": -5, + "columnCount": "sdjfksdfsdfdsf", + "widgets": [{ + "identifier": "work_packages_assigned", + "startRow": 4, + "endRow": 8, + "startColumn": 2, + "endColumn": 5 + }], + "_links": { + "scope": { + "href": my_page_path + } + } + }.with_indifferent_access + end + + it 'responds with 422' do + expect(subject.status).to eq(422) + end + + it 'does not create a grid' do + expect(Grids::Grid.count) + .to eql(0) + end + + it 'returns the errors' do + expect(subject.body) + .to be_json_eql('Error'.to_json) + .at_path('_type') + + expect(subject.body) + .to be_json_eql("Widgets is outside of the grid.".to_json) + .at_path('_embedded/errors/0/message') + + expect(subject.body) + .to be_json_eql("Number of rows must be greater than 0.".to_json) + .at_path('_embedded/errors/1/message') + + expect(subject.body) + .to be_json_eql("Number of columns must be greater than 0.".to_json) + .at_path('_embedded/errors/2/message') + end + end + end +end diff --git a/modules/my_page/spec/requests/api/v3/grids/grids_update_form_resource_spec.rb b/modules/my_page/spec/requests/api/v3/grids/grids_update_form_resource_spec.rb new file mode 100644 index 0000000000..fdca625f0d --- /dev/null +++ b/modules/my_page/spec/requests/api/v3/grids/grids_update_form_resource_spec.rb @@ -0,0 +1,174 @@ +#-- copyright +# OpenProject is a project management system. +# Copyright (C) 2012-2018 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 docs/COPYRIGHT.rdoc for more details. +#++ + +require 'spec_helper' +require 'rack/test' + +describe "PATCH /api/v3/grids/:id/form", type: :request, content_type: :json do + include Rack::Test::Methods + include API::V3::Utilities::PathHelper + + shared_let(:current_user) do + FactoryBot.create(:user) + end + + let(:grid) do + FactoryBot.create(:my_page, user: current_user) + end + let(:path) { api_v3_paths.grid_form(grid.id) } + let(:params) { {} } + subject(:response) { last_response } + + before do + login_as(current_user) + end + + describe '#post' do + before do + post path, params.to_json, 'CONTENT_TYPE' => 'application/json' + end + + it 'returns 200 OK' do + expect(subject.status) + .to eql 200 + end + + it 'is of type form' do + expect(subject.body) + .to be_json_eql("Form".to_json) + .at_path('_type') + end + + it 'contains a Schema disallowing setting scope' do + expect(subject.body) + .to be_json_eql("Schema".to_json) + .at_path('_embedded/schema/_type') + + expect(subject.body) + .to be_json_eql(false.to_json) + .at_path('_embedded/schema/scope/writable') + end + + it 'contains the current data in the payload' do + expected = { + rowCount: 7, + columnCount: 4, + options: {}, + widgets: [ + { + "_type": "GridWidget", + identifier: 'work_packages_assigned', + options: {}, + startRow: 1, + endRow: 7, + startColumn: 1, + endColumn: 3 + }, + { + "_type": "GridWidget", + identifier: 'work_packages_created', + options: {}, + startRow: 1, + endRow: 7, + startColumn: 3, + endColumn: 5 + } + ], + "_links": { + "scope": { + "href": "/my/page", + "type": "text/html" + } + } + } + + expect(subject.body) + .to be_json_eql(expected.to_json) + .at_path('_embedded/payload') + end + + it 'has a commit link' do + expect(subject.body) + .to be_json_eql(api_v3_paths.grid(grid.id).to_json) + .at_path('_links/commit/href') + end + + context 'with some value for the scope value' do + let(:params) do + { + '_links': { + 'scope': { + 'href': '/some/path' + } + } + } + end + + it 'has a validation error on scope as the value is not writeable' do + expect(subject.body) + .to be_json_eql("You must not write a read-only attribute.".to_json) + .at_path('_embedded/validationErrors/scope/message') + end + end + + context 'with an unsupported widget identifier' do + let(:params) do + { + "widgets": [ + { + "_type": "GridWidget", + "identifier": "bogus_identifier", + "startRow": 4, + "endRow": 5, + "startColumn": 1, + "endColumn": 2 + } + ] + } + end + + it 'has a validationError on widget' do + expect(subject.body) + .to be_json_eql("Widgets is not set to one of the allowed values.".to_json) + .at_path('_embedded/validationErrors/widgets/message') + end + end + + context 'for another user\'s grid' do + let(:other_user) { FactoryBot.create(:user) } + let(:other_grid) { FactoryBot.create(:my_page, user: other_user) } + + let(:path) { api_v3_paths.grid_form(other_grid.id) } + + it 'returns 404 NOT FOUND' do + expect(subject.status) + .to eql 404 + end + end + end +end From 6e315c45fe39fbcb07da9d906c8b3eaba11fbe45 Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Mon, 8 Apr 2019 08:11:39 +0200 Subject: [PATCH 13/14] have dedicated controller in my_page engine --- app/controllers/application_controller.rb | 11 +++--- .../concerns/redirect_after_login.rb | 2 +- app/controllers/my_controller.rb | 6 --- app/helpers/meta_tags_helper.rb | 1 - config/initializers/menus.rb | 5 ++- config/routes.rb | 3 +- .../routing/my-page/my-page.component.ts | 13 ++++++- lib/redmine/menu_manager/menu_helper.rb | 17 +++++++-- .../menu_manager/top_menu/projects_menu.rb | 6 ++- .../controllers/my_page/angular_controller.rb | 37 +++++++++++++++++++ .../views/my_page/angular/no_menu.html.erb | 2 - modules/my_page/config/routes.rb | 33 +++++++++++++++++ 12 files changed, 109 insertions(+), 27 deletions(-) create mode 100644 modules/my_page/app/controllers/my_page/angular_controller.rb rename app/views/my/page.html.erb => modules/my_page/app/views/my_page/angular/no_menu.html.erb (96%) create mode 100644 modules/my_page/config/routes.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 5c9f7e7895..71715ae460 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -263,12 +263,11 @@ class ApplicationController < ActionController::Base reset_session respond_to do |format| - format.any(:html, :atom) do redirect_to signin_path(back_url: login_back_url) end + format.any(:html, :atom) { redirect_to main_app.signin_path(back_url: login_back_url) } - auth_header = OpenProject::Authentication::WWWAuthenticate.response_header( - request_headers: request.headers) + auth_header = OpenProject::Authentication::WWWAuthenticate.response_header(request_headers: request.headers) - format.any(:xml, :js, :json) do + format.any(:xml, :js, :json) do head :unauthorized, 'X-Reason' => 'login needed', 'WWW-Authenticate' => auth_header @@ -581,9 +580,9 @@ class ApplicationController < ActionController::Base # Converts the errors on an ActiveRecord object into a common JSON format def object_errors_to_json(object) - object.errors.map { |attribute, error| + object.errors.map do |attribute, error| { attribute => error } - }.to_json + end.to_json end # Renders API response on validation failure diff --git a/app/controllers/concerns/redirect_after_login.rb b/app/controllers/concerns/redirect_after_login.rb index fd6c256a0f..57cc8c5209 100644 --- a/app/controllers/concerns/redirect_after_login.rb +++ b/app/controllers/concerns/redirect_after_login.rb @@ -45,7 +45,7 @@ module Concerns::RedirectAfterLogin if url = OpenProject::Configuration.after_login_default_redirect_url redirect_to url else - redirect_back_or_default controller: '/my', action: 'page' + redirect_back_or_default my_page_path end end diff --git a/app/controllers/my_controller.rb b/app/controllers/my_controller.rb index bef689220d..fba2fe7694 100644 --- a/app/controllers/my_controller.rb +++ b/app/controllers/my_controller.rb @@ -45,12 +45,6 @@ class MyController < ApplicationController menu_item :access_token, only: [:access_token] menu_item :mail_notifications, only: [:mail_notifications] - # Show user's page - def index - render action: 'page', layout: 'no_menu' - end - alias :page :index - def account; end def update_account diff --git a/app/helpers/meta_tags_helper.rb b/app/helpers/meta_tags_helper.rb index 93932c2f66..6be6a7e448 100644 --- a/app/helpers/meta_tags_helper.rb +++ b/app/helpers/meta_tags_helper.rb @@ -29,7 +29,6 @@ #++ module MetaTagsHelper - ## # Use meta-tags to output title and site name def output_title_and_meta_tags diff --git a/config/initializers/menus.rb b/config/initializers/menus.rb index e159e5b932..e48df8e692 100644 --- a/config/initializers/menus.rb +++ b/config/initializers/menus.rb @@ -69,7 +69,7 @@ end Redmine::MenuManager.map :account_menu do |menu| menu.push :my_page, - { controller: '/my', action: 'page' }, + :my_page_path, if: Proc.new { User.current.logged? } menu.push :my_account, { controller: '/my', action: 'account' }, @@ -77,7 +77,8 @@ Redmine::MenuManager.map :account_menu do |menu| menu.push :administration, { controller: '/users', action: 'index' }, if: Proc.new { User.current.admin? } - menu.push :logout, :signout_path, + menu.push :logout, + :signout_path, if: Proc.new { User.current.logged? } end diff --git a/config/routes.rb b/config/routes.rb index 14209f9d06..b19db74e54 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -516,10 +516,11 @@ OpenProject::Application.routes.draw do match '/oauth/revoke_application/:application_id' => 'oauth/grants#revoke_application', via: :post, as: 'revoke_my_oauth_application' end + mount MyPage::Engine, at: "/my/page" + scope controller: 'my' do get '/my/password', action: 'password' post '/my/change_password', action: 'change_password' - get '/my/page', action: 'page' get '/my/account', action: 'account' get '/my/settings', action: 'settings' diff --git a/frontend/src/app/components/routing/my-page/my-page.component.ts b/frontend/src/app/components/routing/my-page/my-page.component.ts index 64633f9c39..0f9014b3d2 100644 --- a/frontend/src/app/components/routing/my-page/my-page.component.ts +++ b/frontend/src/app/components/routing/my-page/my-page.component.ts @@ -4,17 +4,20 @@ import {GridResource} from "core-app/modules/hal/resources/grid-resource"; import {PathHelperService} from "core-app/modules/common/path-helper/path-helper.service"; import {HalResourceService} from "core-app/modules/hal/services/hal-resource.service"; import {I18nService} from "core-app/modules/common/i18n/i18n.service"; +import {Title} from '@angular/platform-browser'; @Component({ templateUrl: './my-page.component.html' }) export class MyPageComponent implements OnInit { - public text = { title: this.i18n.t('js.label_my_page') }; + public text = { title: this.i18n.t('js.label_my_page'), + html_title: this.i18n.t('js.label_my_page') }; constructor(readonly gridDm:GridDmService, readonly pathHelper:PathHelperService, readonly halResourceService:HalResourceService, - readonly i18n:I18nService) {} + readonly i18n:I18nService, + readonly title:Title) {} public grid:GridResource; @@ -24,6 +27,8 @@ export class MyPageComponent implements OnInit { .then((grid) => { this.grid = grid; }); + + this.setHtmlTitle(); } // If a page with the current page exists (scoped to the current user by the backend) @@ -71,4 +76,8 @@ export class MyPageComponent implements OnInit { }); }); } + + private setHtmlTitle() { + this.title.setTitle(this.text.html_title); + } } diff --git a/lib/redmine/menu_manager/menu_helper.rb b/lib/redmine/menu_manager/menu_helper.rb index 3af40b36f2..2ada3cf6cd 100644 --- a/lib/redmine/menu_manager/menu_helper.rb +++ b/lib/redmine/menu_manager/menu_helper.rb @@ -195,9 +195,8 @@ module Redmine::MenuManager::MenuHelper link_text << ' '.html_safe + op_icon(item.icon_after) if item.icon_after.present? html_options = item.html_options(selected: selected) html_options[:title] ||= selected ? t(:description_current_position) + caption : caption - link_to url, html_options do - link_text - end + + link_to link_text, main_app_url(url), html_options end def render_unattached_menu_item(menu_item, project) @@ -212,6 +211,7 @@ module Redmine::MenuManager::MenuHelper def current_menu_item_part_of_menu?(menu, project = nil) return true if no_menu_item_wiki_prefix? || wiki_prefix? + all_menu_items_for(menu, project).each do |node| return true if node.name == current_menu_item end @@ -231,6 +231,7 @@ module Redmine::MenuManager::MenuHelper items = [] iteratable.each do |node| next if node.name == :root + if allowed_node?(node, User.current, project) && visible_node?(menu, node) items << node if block_given? @@ -248,7 +249,7 @@ module Redmine::MenuManager::MenuHelper when Hash project.nil? ? item.url : { item.param => project }.merge(item.url) when Symbol - send(item.url) + main_app.send(item.url) else item.url end @@ -323,4 +324,12 @@ module Redmine::MenuManager::MenuHelper end badge end + + def main_app_url(url) + if url.is_a? Symbol + main_app.send(url) + else + main_app.url_for(url) + end + end end diff --git a/lib/redmine/menu_manager/top_menu/projects_menu.rb b/lib/redmine/menu_manager/top_menu/projects_menu.rb index 2d9ba14295..3c0b603c17 100644 --- a/lib/redmine/menu_manager/top_menu/projects_menu.rb +++ b/lib/redmine/menu_manager/top_menu/projects_menu.rb @@ -59,7 +59,7 @@ module Redmine::MenuManager::TopMenu::ProjectsMenu def project_index_item Redmine::MenuManager::MenuItem.new( :list_projects, - { controller: '/projects', action: 'index' }, + main_app.projects_path, caption: t(:label_project_view_all), icon: "icon-show-all-projects icon4", html: { @@ -71,7 +71,7 @@ module Redmine::MenuManager::TopMenu::ProjectsMenu def project_new_item Redmine::MenuManager::MenuItem.new( :new_project, - { controller: '/projects', action: 'new' }, + main_app.new_project_path, caption: Project.model_name.human, icon: "icon-add icon4", html: { @@ -82,4 +82,6 @@ module Redmine::MenuManager::TopMenu::ProjectsMenu if: Proc.new { User.current.allowed_to?(:add_project, nil, global: true) } ) end + + include OpenProject::StaticRouting::UrlHelpers end diff --git a/modules/my_page/app/controllers/my_page/angular_controller.rb b/modules/my_page/app/controllers/my_page/angular_controller.rb new file mode 100644 index 0000000000..010cdf4606 --- /dev/null +++ b/modules/my_page/app/controllers/my_page/angular_controller.rb @@ -0,0 +1,37 @@ +#-- encoding: UTF-8 + +#-- copyright +# OpenProject is a project management system. +# Copyright (C) 2012-2018 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 docs/COPYRIGHT.rdoc for more details. +#++ + +class MyPage::AngularController < ::ApplicationController + before_action :require_login + + def no_menu + render layout: 'no_menu' + end +end diff --git a/app/views/my/page.html.erb b/modules/my_page/app/views/my_page/angular/no_menu.html.erb similarity index 96% rename from app/views/my/page.html.erb rename to modules/my_page/app/views/my_page/angular/no_menu.html.erb index 2f6691a362..c6917e5a81 100644 --- a/app/views/my/page.html.erb +++ b/modules/my_page/app/views/my_page/angular/no_menu.html.erb @@ -27,6 +27,4 @@ See docs/COPYRIGHT.rdoc for more details. ++#%> -<% html_title(t(:label_my_page)) -%> - diff --git a/modules/my_page/config/routes.rb b/modules/my_page/config/routes.rb new file mode 100644 index 0000000000..f9235add1a --- /dev/null +++ b/modules/my_page/config/routes.rb @@ -0,0 +1,33 @@ +#-- encoding: UTF-8 + +#-- copyright +# OpenProject is a project management system. +# Copyright (C) 2012-2018 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 docs/COPYRIGHT.rdoc for more details. +#++ + +MyPage::Engine.routes.draw do + root to: 'angular#no_menu' +end From 680eae29c0d3e764c085e2893b367eb97558ee09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Tue, 9 Apr 2019 08:32:42 +0200 Subject: [PATCH 14/14] Fix missing path --- app/helpers/my_helper.rb | 36 ------------------------------------ config/initializers/menus.rb | 2 +- 2 files changed, 1 insertion(+), 37 deletions(-) delete mode 100644 app/helpers/my_helper.rb diff --git a/app/helpers/my_helper.rb b/app/helpers/my_helper.rb deleted file mode 100644 index 79a9a24410..0000000000 --- a/app/helpers/my_helper.rb +++ /dev/null @@ -1,36 +0,0 @@ -#-- encoding: UTF-8 -#-- copyright -# OpenProject is a project management system. -# Copyright (C) 2012-2018 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 docs/COPYRIGHT.rdoc for more details. -#++ - -module MyHelper - include WorkPackagesFilterHelper - - def deletion_info_path - url_for(:delete_my_account_info) - end -end diff --git a/config/initializers/menus.rb b/config/initializers/menus.rb index e48df8e692..ed879e8894 100644 --- a/config/initializers/menus.rb +++ b/config/initializers/menus.rb @@ -114,7 +114,7 @@ Redmine::MenuManager.map :my_menu do |menu| caption: I18n.t('activerecord.attributes.user.mail_notification'), icon: 'icon2 icon-news' - menu.push :delete_account, :deletion_info_path, + menu.push :delete_account, :delete_my_account_info_path, caption: I18n.t('account.delete'), param: :user_id, if: Proc.new { Setting.users_deletable_by_self? },