Merge branch 'dev' into feature/project-status-widget-the-second-approach

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'

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)

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

frontend/src/app/modules/fields/openproject-fields.module.ts

@@ -53,6 +53,7 @@ import {EditFormComponent} from "core-app/modules/fields/edit/edit-form/edit-for
 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 {ProjectStatusEditFieldComponent} from "core-app/modules/fields/edit/field-types/project-status-edit-field.component";
+import {PortalCleanupService} from "core-app/modules/fields/display/display-portal/portal-cleanup.service";
 
 @NgModule({
   imports: [
@@ -70,6 +71,7 @@ import {ProjectStatusEditFieldComponent} from "core-app/modules/fields/edit/fiel
   providers: [
     EditingPortalService,
     UserFieldPortalService,
+    PortalCleanupService,
     DisplayFieldService,
     EditFieldService,
     SelectAutocompleterRegisterService,

frontend/src/app/modules/grids/widgets/project-details/project-details.component.html

@@ -9,10 +9,19 @@
 </widget-header>
 
 <div class="grid--widget-content">
-  <div class="attributes-map"
-       #contentContainer>
-  </div>
-  <no-results *ngIf="noFields"
-              [title]="text.noResults">
-  </no-results>
+  <edit-form *ngIf="(project$ | async) as project"
+             [resource]="project">
+    <div class="attributes-map">
+      <ng-container *ngFor="let cf of customFields">
+        <div class="attributes-map--key">
+          {{ cf.label }}
+        </div>
+        <div class="attributes-map--value">
+          <editable-attribute-field [resource]="project"
+                                    [fieldName]="cf.key">
+          </editable-attribute-field>
+        </div>
+      </ng-container>
+    </div>
+  </edit-form>
 </div>

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<ProjectResource>;
 
   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<DisplayField> = [];
-    // 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<DisplayField>) {
-    fields.sort((a, b) => { return a.label.localeCompare(b.label); });
-  }
-
-  private renderFields(fields:Array<DisplayField>) {
-    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();
   }
 }

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

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

spec/support/edit_fields/multi_select_field.rb

@@ -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

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