From 581b75e3e601fa95667ed99a9ae74faf00a773b3 Mon Sep 17 00:00:00 2001 From: Henriette Dinger Date: Tue, 1 Oct 2019 15:27:45 +0200 Subject: [PATCH 1/3] Use editable fields for project details widget --- .../project-details.component.html | 21 ++- .../project-details.component.ts | 124 ++++-------------- 2 files changed, 40 insertions(+), 105 deletions(-) diff --git a/frontend/src/app/modules/grids/widgets/project-details/project-details.component.html b/frontend/src/app/modules/grids/widgets/project-details/project-details.component.html index 6ba8a201c2..4f36fa126e 100644 --- a/frontend/src/app/modules/grids/widgets/project-details/project-details.component.html +++ b/frontend/src/app/modules/grids/widgets/project-details/project-details.component.html @@ -9,10 +9,19 @@
-
-
- - + +
+ +
+ {{ cf.label }} +
+
+ + +
+ +
+
diff --git a/frontend/src/app/modules/grids/widgets/project-details/project-details.component.ts b/frontend/src/app/modules/grids/widgets/project-details/project-details.component.ts index bc481cd4a9..a1650bb3d3 100644 --- a/frontend/src/app/modules/grids/widgets/project-details/project-details.component.ts +++ b/frontend/src/app/modules/grids/widgets/project-details/project-details.component.ts @@ -26,53 +26,50 @@ // See doc/COPYRIGHT.rdoc for more details. // ++ -import {Component, OnInit, ChangeDetectionStrategy, ChangeDetectorRef, Injector, ViewChild, ElementRef} from '@angular/core'; +import { + ChangeDetectionStrategy, + ChangeDetectorRef, + Component, + ElementRef, + Injector, + OnInit, + ViewChild +} from '@angular/core'; import {AbstractWidgetComponent} from "app/modules/grids/widgets/abstract-widget.component"; import {I18nService} from "core-app/modules/common/i18n/i18n.service"; import {ProjectDmService} from "core-app/modules/hal/dm-services/project-dm.service"; import {CurrentProjectService} from "core-components/projects/current-project.service"; import {SchemaResource} from "core-app/modules/hal/resources/schema-resource"; -import {DisplayFieldContext, DisplayFieldService} from "core-app/modules/fields/display/display-field.service"; -import {ProjectResource} from "core-app/modules/hal/resources/project-resource"; -import {PortalCleanupService} from 'core-app/modules/fields/display/display-portal/portal-cleanup.service'; -import {DisplayField} from "core-app/modules/fields/display/display-field.module"; -import {WorkPackageViewHighlightingService} from "core-app/modules/work_packages/routing/wp-view-base/view-services/wp-view-highlighting.service"; -import {IsolatedQuerySpace} from "core-app/modules/work_packages/query-space/isolated-query-space"; import {ProjectCacheService} from "core-components/projects/project-cache.service"; - -export const emptyPlaceholder = '-'; +import {Observable} from "rxjs"; +import {ProjectResource} from "core-app/modules/hal/resources/project-resource"; +import {HalResourceEditingService} from "core-app/modules/fields/edit/services/hal-resource-editing.service"; @Component({ templateUrl: './project-details.component.html', changeDetection: ChangeDetectionStrategy.OnPush, providers: [ - // required by the displayField service to render the fields - PortalCleanupService, - WorkPackageViewHighlightingService, - IsolatedQuerySpace + HalResourceEditingService ] }) export class WidgetProjectDetailsComponent extends AbstractWidgetComponent implements OnInit { @ViewChild('contentContainer', { static: true }) readonly contentContainer:ElementRef; - public noFields = false; - - public text = { - noResults: this.i18n.t('js.grid.widgets.project_details.no_results'), - }; + public customFields:{key:string, label:string}[] = []; + public project$:Observable; constructor(protected readonly i18n:I18nService, protected readonly injector:Injector, protected readonly projectDm:ProjectDmService, protected readonly projectCache:ProjectCacheService, protected readonly currentProject:CurrentProjectService, - protected readonly displayField:DisplayFieldService, - protected readonly cdr:ChangeDetectorRef) { + protected readonly cdRef:ChangeDetectorRef) { super(i18n, injector); } ngOnInit() { this.loadAndRender(); + this.project$ = this.projectCache.requireAndStream(this.currentProject.id!); } public get isEditable() { @@ -80,96 +77,25 @@ export class WidgetProjectDetailsComponent extends AbstractWidgetComponent imple } private loadAndRender() { - Promise.all( - [this.loadCurrentProject(), - this.loadProjectSchema()] - ) - .then(([project, schema]) => { - this.renderCFs(project, schema as SchemaResource); - - this.redraw(); + Promise.all([ + this.loadProjectSchema() + ]) + .then(([schema]) => { + this.setCustomFields(schema); }); } - private loadCurrentProject() { - return this.projectCache.require(this.currentProject.id as string); - } - - public get isLoaded() { - return this.projectCache.state(this.currentProject.id as string).value; - } - private loadProjectSchema() { return this.projectDm.schema(); } - private renderCFs(project:ProjectResource, schema:SchemaResource) { - const cfFields = this.collectFieldsForCfs(project, schema); - - this.noFields = cfFields.length === 0; - - this.sortFieldsLexicographically(cfFields); - this.renderFields(cfFields); - } - - private collectFieldsForCfs(project:ProjectResource, schema:SchemaResource) { - let displayFields:Array = []; - // passing an arbitrary context to displayField.getField only to satisfy the interface - let context:DisplayFieldContext = {injector: this.injector, container: 'table', options: []}; - + private setCustomFields(schema:SchemaResource) { Object.entries(schema).forEach(([key, keySchema]) => { if (key.match(/customField\d+/)) { - let field = this.displayField.getField(project, key, keySchema, context); - - displayFields.push(field); + this.customFields.push({key: key, label: keySchema.name }); } }); - return displayFields; - } - - private sortFieldsLexicographically(fields:Array) { - fields.sort((a, b) => { return a.label.localeCompare(b.label); }); - } - - private renderFields(fields:Array) { - this.contentContainer.nativeElement.innerHTML = ''; - - fields.forEach(field => { - this.renderKeyValue(field); - }); - } - - private renderKeyValue(field:DisplayField) { - this.contentContainer.nativeElement.appendChild(this.labelElement(field)); - this.contentContainer.nativeElement.appendChild(this.valueElement(field)); - } - - private labelElement(field:DisplayField) { - const label = document.createElement('div'); - label.classList.add('attributes-map--key'); - label.innerText = field.label; - - return label; - } - - private valueElement(field:DisplayField) { - const value = document.createElement('div'); - value.classList.add('attributes-map--value'); - field.render(value, this.getText(field)); - - return value; - } - - private getText(field:DisplayField):string { - if (field.isEmpty()) { - return emptyPlaceholder; - } else { - return field.valueString; - } - } - - private redraw() { - this.cdr.detectChanges(); + this.cdRef.detectChanges(); } } From bfb23a27103040176b2ef506418f561bc6b0ba60 Mon Sep 17 00:00:00 2001 From: Henriette Dinger Date: Fri, 4 Oct 2019 14:14:00 +0200 Subject: [PATCH 2/3] Add test for inline editing of project custom fields --- .../content/_attributes_key_value.sass | 1 + .../fields/openproject-fields.module.ts | 2 + .../spec/features/project_details_spec.rb | 112 ++++++++++++++---- .../multi_value_custom_field_spec.rb | 2 +- .../support/edit_fields/multi_select_field.rb | 29 ----- spec/support/edit_fields/select_edit_field.rb | 12 ++ 6 files changed, 102 insertions(+), 56 deletions(-) delete mode 100644 spec/support/edit_fields/multi_select_field.rb create mode 100644 spec/support/edit_fields/select_edit_field.rb diff --git a/app/assets/stylesheets/content/_attributes_key_value.sass b/app/assets/stylesheets/content/_attributes_key_value.sass index 593660fc65..2addf04e60 100644 --- a/app/assets/stylesheets/content/_attributes_key_value.sass +++ b/app/assets/stylesheets/content/_attributes_key_value.sass @@ -84,6 +84,7 @@ .attributes-map--key @include text-shortener font-weight: bold + line-height: 27px .attributes-map.-minimal-keys & max-width: 200px diff --git a/frontend/src/app/modules/fields/openproject-fields.module.ts b/frontend/src/app/modules/fields/openproject-fields.module.ts index c656c90ac9..c40cd82b45 100644 --- a/frontend/src/app/modules/fields/openproject-fields.module.ts +++ b/frontend/src/app/modules/fields/openproject-fields.module.ts @@ -52,6 +52,7 @@ import {SelectAutocompleterRegisterService} from "core-app/modules/fields/edit/f import {EditFormComponent} from "core-app/modules/fields/edit/edit-form/edit-form.component"; import {WorkPackageEditFieldComponent} from "core-app/modules/fields/edit/field-types/work-package-edit-field.component"; import {EditableAttributeFieldComponent} from "core-app/modules/fields/edit/field/editable-attribute-field.component"; +import {PortalCleanupService} from "core-app/modules/fields/display/display-portal/portal-cleanup.service"; @NgModule({ imports: [ @@ -69,6 +70,7 @@ import {EditableAttributeFieldComponent} from "core-app/modules/fields/edit/fiel providers: [ EditingPortalService, UserFieldPortalService, + PortalCleanupService, DisplayFieldService, EditFieldService, SelectAutocompleterRegisterService, diff --git a/modules/dashboards/spec/features/project_details_spec.rb b/modules/dashboards/spec/features/project_details_spec.rb index 61db38a691..2e3e4fc815 100644 --- a/modules/dashboards/spec/features/project_details_spec.rb +++ b/modules/dashboards/spec/features/project_details_spec.rb @@ -64,6 +64,12 @@ describe 'Project details widget on dashboard', type: :feature, js: true do manage_dashboards] end + let(:editing_permissions) do + %i[view_dashboards + manage_dashboards + edit_project] + end + let(:role) do FactoryBot.create(:role, permissions: permissions) end @@ -71,6 +77,13 @@ describe 'Project details widget on dashboard', type: :feature, js: true do let(:user) do FactoryBot.create(:user, member_in_project: project, member_through_role: role) end + let(:second_user) do + FactoryBot.create(:user, + member_in_project: project, + member_with_permissions: editing_permissions, + firstname: 'Cool', + lastname: 'Guy') + end let(:other_user) do FactoryBot.create(:user) end @@ -78,37 +91,84 @@ describe 'Project details widget on dashboard', type: :feature, js: true do Pages::Dashboard.new(project) end - before do - login_as user - + def add_project_details_widget dashboard_page.visit! - end - - it 'can add the widget and see the description in it' do dashboard_page.add_widget(1, 1, :within, "Project details") sleep(0.1) + end + + def change_cf_value(cf, old_value, new_value) + # Open description field + cf.activate! + sleep(0.1) + + # Change the value + cf.expect_value(old_value) + cf.set_value new_value + cf.save! unless cf.field_type === 'create-autocompleter' + + # The edit field is toggled and the value saved. + expect(page).to have_content(new_value) + expect(page).to have_selector(cf.selector) + expect(page).not_to have_selector(cf.input_selector) + end + + context 'without editing permissions' do + before do + login_as user + add_project_details_widget + end + + it 'can add the widget, but not edit the custom fields' do + # As the user lacks the manage_public_queries and save_queries permission, no other widget is present + details_widget = Components::Grids::GridArea.new('.grid--area.-widgeted:nth-of-type(1)') + + within(details_widget.area) do + # Expect values + expect(page) + .to have_content("#{int_cf.name}\n5") + expect(page) + .to have_content("#{bool_cf.name}\nyes") + expect(page) + .to have_content("#{version_cf.name}\n#{system_version.name}") + expect(page) + .to have_content("#{float_cf.name}\n4.5") + expect(page) + .to have_content("#{text_cf.name}\nSome long text") + expect(page) + .to have_content("#{string_cf.name}\nSome small text") + expect(page) + .to have_content("#{date_cf.name}\n#{Date.today.strftime('%m/%d/%Y')}") + expect(page) + .to have_content("#{user_cf.name}\n#{user.name.split.map(&:first).join}#{user.name}") + + # The fields are not editable + field = EditField.new dashboard_page, "customField#{bool_cf.id}" + field.activate! expect_open: false + end + end + end + + context 'with editing permissions' do + before do + user + login_as second_user + add_project_details_widget + end + + it 'can edit the custom fields' do + int_field = EditField.new dashboard_page, "customField#{int_cf.id}" + change_cf_value int_field, "5", "3" + + string_field = EditField.new dashboard_page, "customField#{string_cf.id}" + change_cf_value string_field, 'Some small text', 'Some new text' + + text_field = TextEditorField.new dashboard_page, "customField#{text_cf.id}" + change_cf_value text_field, 'Some long text', 'Some very long text' - # As the user lacks the manage_public_queries and save_queries permission, no other widget is present - details_widget = Components::Grids::GridArea.new('.grid--area.-widgeted:nth-of-type(1)') - - within(details_widget.area) do - expect(page) - .to have_content("#{int_cf.name}\n5") - expect(page) - .to have_content("#{bool_cf.name}\nyes") - expect(page) - .to have_content("#{version_cf.name}\n#{system_version.name}") - expect(page) - .to have_content("#{float_cf.name}\n4.5") - expect(page) - .to have_content("#{text_cf.name}\nSome long text") - expect(page) - .to have_content("#{string_cf.name}\nSome small text") - expect(page) - .to have_content("#{date_cf.name}\n#{Date.today.strftime('%m/%d/%Y')}") - expect(page) - .to have_content("#{user_cf.name}\n#{user.name.split.map(&:first).join}#{user.name}") + user_field = SelectField.new dashboard_page, "customField#{user_cf.id}" + change_cf_value user_field, user.name, second_user.name end end end diff --git a/spec/features/custom_fields/multi_value_custom_field_spec.rb b/spec/features/custom_fields/multi_value_custom_field_spec.rb index 42708c88ab..d2afe78f16 100644 --- a/spec/features/custom_fields/multi_value_custom_field_spec.rb +++ b/spec/features/custom_fields/multi_value_custom_field_spec.rb @@ -147,7 +147,7 @@ describe "multi select custom values", clear_cache: true, js: true do # Open split view split_view = wp_table.open_split_view work_package - field = MultiSelectField.new(split_view.container, "customField#{custom_field.id}") + field = SelectField.new(split_view.container, "customField#{custom_field.id}") field.activate! field.unset_value "ham", true diff --git a/spec/support/edit_fields/multi_select_field.rb b/spec/support/edit_fields/multi_select_field.rb deleted file mode 100644 index 1cee1584ad..0000000000 --- a/spec/support/edit_fields/multi_select_field.rb +++ /dev/null @@ -1,29 +0,0 @@ -require_relative './edit_field' - -class MultiSelectField < EditField - - def multiselect? - field_container.has_selector?('.inline-edit--toggle-multiselect .icon-minus2') - end - - def expect_save_button(enabled: true) - if enabled - expect(field_container).to have_no_selector("#{control_link}[disabled]") - else - expect(field_container).to have_selector("#{control_link}[disabled]") - end - end - - def save! - submit_by_click - end - - def field_type - 'create-autocompleter' - end - - def control_link(action = :save) - raise 'Invalid link' unless [:save, :cancel].include?(action) - ".inplace-edit--control--#{action}" - end -end diff --git a/spec/support/edit_fields/select_edit_field.rb b/spec/support/edit_fields/select_edit_field.rb new file mode 100644 index 0000000000..efc00d5676 --- /dev/null +++ b/spec/support/edit_fields/select_edit_field.rb @@ -0,0 +1,12 @@ +require_relative './edit_field' + +class SelectField < EditField + def expect_value(value) + input = context.find(input_selector + ' .ng-value-label') + expect(input.text).to eq(value) + end + + def field_type + 'create-autocompleter' + end +end From 1e4b090076576838df1f49c3db5f0f957167c600 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Wed, 9 Oct 2019 11:34:59 +0200 Subject: [PATCH 3/3] Bump danger --- Gemfile | 2 +- Gemfile.lock | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Gemfile b/Gemfile index 85aa2fe01c..86fc1afcb0 100644 --- a/Gemfile +++ b/Gemfile @@ -263,7 +263,7 @@ group :development, :test do gem 'pry-stack_explorer', '~> 0.4.9.2' # Dangerfile scanner on travis and locally - gem 'danger', '~> 6.0.9' + gem 'danger', '~> 6.1.0' # Brakeman scanner gem 'brakeman', '~> 4.6.1' diff --git a/Gemfile.lock b/Gemfile.lock index 2d9b3c8e0d..72af474be9 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -429,7 +429,7 @@ GEM cucumber-wire (0.0.1) daemons (1.3.1) dalli (2.7.10) - danger (6.0.9) + danger (6.1.0) claide (~> 1.0) claide-plugins (>= 0.9.2) colored2 (~> 3.1) @@ -491,7 +491,7 @@ GEM railties (>= 3.0.0) faker (1.9.1) i18n (>= 0.7) - faraday (0.15.4) + faraday (0.17.0) multipart-post (>= 1.2, < 3) faraday-http-cache (2.0.0) faraday (~> 0.8) @@ -973,7 +973,7 @@ DEPENDENCIES cucumber-rails (~> 1.8.0) daemons dalli (~> 2.7.10) - danger (~> 6.0.9) + danger (~> 6.1.0) danger-brakeman dashboards! database_cleaner (~> 1.6)