Merge remote-tracking branch 'origin/release/10.4' into dev

app/assets/javascripts/custom-fields.js

@@ -44,6 +44,7 @@
         possibleValues        = $('#custom_field_possible_values_attributes'),
         defaultValueFields    = $('#custom_field_default_value_attributes'),
         spanDefaultText       = $('#default_value_text'),
+        spanDefaultLongText   = $('#default_value_long_text'),
         spanDefaultBool       = $('#default_value_bool'),
         textOrientationField  = $('#custom_field_text_orientation');
 
@@ -66,10 +67,10 @@
           unsearchable = function() { searchable.attr('checked', false).hide(); };
 
       // defaults (reset these fields before doing anything else)
-      $.each([spanDefaultBool, spanDefaultText, multiSelect, textOrientationField], function(idx, element) {
+      $.each([spanDefaultBool, spanDefaultLongText, spanDefaultText, multiSelect, textOrientationField], function(idx, element) {
         deactivate(element);
       });
-      show(defaultValueFields);
+      activate(defaultValueFields);
       activate(spanDefaultText);
 
       switch (format.val()) {
@@ -109,6 +110,8 @@
           unsearchable();
           break;
         case 'text':
+          activate(spanDefaultLongText);
+          deactivate(spanDefaultText);
           show(lengthField, regexpField, searchable, textOrientationField);
           deactivate(possibleValues);
           activate(textOrientationField);
@@ -122,6 +125,7 @@
 
     // assign the switch format function to the select field
     format.on('change', toggleFormat).trigger('change');
+    toggleFormat();
   });
 
   $(function() {

app/helpers/browser_helper.rb

@@ -20,6 +20,9 @@ module BrowserHelper
       # Older version of safari
       return true if browser.safari? && version < 12
 
+      # Older version of EDGE
+      return true if browser.edge? && version < 18
+
       false
     end
   end

app/models/project.rb

@@ -407,80 +407,94 @@ class Project < ActiveRecord::Base
     parents | descendants # Set union
   end
 
-  # Returns an auto-generated project identifier based on the last identifier used
-  def self.next_identifier
-    p = Project.newest.first
-    p.nil? ? nil : p.identifier.to_s.succ
-  end
+  class << self
+    # Returns an auto-generated project identifier based on the last identifier used
+    def next_identifier
+      p = Project.newest.first
+      p.nil? ? nil : p.identifier.to_s.succ
+    end
 
-  # builds up a project hierarchy helper structure for use with #project_tree_from_hierarchy
-  #
-  # it expects a simple list of projects with a #lft column (awesome_nested_set)
-  # and returns a hierarchy based on #lft
-  #
-  # the result is a nested list of root level projects that contain their child projects
-  # but, each entry is actually a ruby hash wrapping the project and child projects
-  # the keys are :project and :children where :children is in the same format again
-  #
-  #   result = [ root_level_project_info_1, root_level_project_info_2, ... ]
-  #
-  # where each entry has the form
-  #
-  #   project_info = { project: the_project, children: [ child_info_1, child_info_2, ... ] }
-  #
-  # if a project has no children the :children array is just empty
-  #
-  def self.build_projects_hierarchy(projects)
-    ancestors = []
-    result    = []
+    # builds up a project hierarchy helper structure for use with #project_tree_from_hierarchy
+    #
+    # it expects a simple list of projects with a #lft column (awesome_nested_set)
+    # and returns a hierarchy based on #lft
+    #
+    # the result is a nested list of root level projects that contain their child projects
+    # but, each entry is actually a ruby hash wrapping the project and child projects
+    # the keys are :project and :children where :children is in the same format again
+    #
+    #   result = [ root_level_project_info_1, root_level_project_info_2, ... ]
+    #
+    # where each entry has the form
+    #
+    #   project_info = { project: the_project, children: [ child_info_1, child_info_2, ... ] }
+    #
+    # if a project has no children the :children array is just empty
+    #
+    def build_projects_hierarchy(projects)
+      ancestors = []
+      result    = []
 
-    projects.sort_by(&:lft).each do |project|
-      while ancestors.any? && !project.is_descendant_of?(ancestors.last[:project])
-        # before we pop back one level, we sort the child projects by name
-        ancestors.last[:children] = ancestors.last[:children].sort_by { |h| h[:project].name.downcase if h[:project].name }
-        ancestors.pop
+      projects.sort_by(&:lft).each do |project|
+        while ancestors.any? && !project.is_descendant_of?(ancestors.last[:project])
+          # before we pop back one level, we sort the child projects by name
+          ancestors.last[:children] = sort_by_name(ancestors.last[:children])
+          ancestors.pop
+        end
+
+        current_hierarchy = { project: project, children: [] }
+        current_tree      = ancestors.any? ? ancestors.last[:children] : result
+
+        current_tree << current_hierarchy
+        ancestors << current_hierarchy
       end
 
-      current_hierarchy = { project: project, children: [] }
-      current_tree      = ancestors.any? ? ancestors.last[:children] : result
-
-      current_tree << current_hierarchy
-      ancestors << current_hierarchy
+      # When the last project is deeply nested, we need to sort
+      # all layers we are in.
+      ancestors.each do |level|
+        level[:children] = sort_by_name(level[:children])
+      end
+      # we need one extra element to ensure sorting at the end
+      # at the end the root level must be sorted as well
+      sort_by_name(result)
     end
 
-    # at the end the root level must be sorted as well
-    result.sort_by { |h| h[:project].name&.downcase }
-  end
-
-  def self.project_tree_from_hierarchy(projects_hierarchy, level, &block)
-    projects_hierarchy.each do |hierarchy|
-      project = hierarchy[:project]
-      children = hierarchy[:children]
-      yield project, level
-      # recursively show children
-      project_tree_from_hierarchy(children, level + 1, &block) if children.any?
+    def project_tree_from_hierarchy(projects_hierarchy, level, &block)
+      projects_hierarchy.each do |hierarchy|
+        project = hierarchy[:project]
+        children = hierarchy[:children]
+        yield project, level
+        # recursively show children
+        project_tree_from_hierarchy(children, level + 1, &block) if children.any?
+      end
     end
-  end
 
-  # Yields the given block for each project with its level in the tree
-  def self.project_tree(projects, &block)
-    projects_hierarchy = build_projects_hierarchy(projects)
-    project_tree_from_hierarchy(projects_hierarchy, 0, &block)
-  end
-
-  def self.project_level_list(projects)
-    list = []
-    project_tree(projects) do |project, level|
-      element = {
-        project: project,
-        level: level
-      }
-
-      element.merge!(yield(project)) if block_given?
-
-      list << element
+    # Yields the given block for each project with its level in the tree
+    def project_tree(projects, &block)
+      projects_hierarchy = build_projects_hierarchy(projects)
+      project_tree_from_hierarchy(projects_hierarchy, 0, &block)
+    end
+
+    def project_level_list(projects)
+      list = []
+      project_tree(projects) do |project, level|
+        element = {
+          project: project,
+          level: level
+        }
+
+        element.merge!(yield(project)) if block_given?
+
+        list << element
+      end
+      list
+    end
+
+    private
+
+    def sort_by_name(project_hashes)
+      project_hashes.sort_by { |h| h[:project].name&.downcase }
     end
-    list
   end
 
   def allowed_permissions

app/models/type/attribute_groups.rb

@@ -136,7 +136,6 @@ module Type::AttributeGroups
     self.attribute_groups_objects = nil
   end
 
-
   private
 
   def write_attribute_groups_objects

app/services/base_type_service.rb

@@ -104,7 +104,6 @@ class BaseTypeService
 
   def transform_attribute_groups(groups)
     groups.map do |group|
-
       if group['type'] == 'query'
         transform_query_group(group)
       else

app/views/copy_projects/copy_settings/_copy_associations.html.erb

@@ -34,7 +34,7 @@ See docs/COPYRIGHT.rdoc for more details.
              locals: { name: "queries", checked: true, label: l(:label_query_plural),
                           count: project.queries.count } %>
   <%= render partial: "copy_projects/copy_settings/block_checkbox",
-             locals: { name: "boards", checked: false, label: l(:label_forum_plural),
+             locals: { name: "forums", checked: false, label: l(:label_forum_plural),
                           count: project.forums.count } %>
   <%= render partial: "copy_projects/copy_settings/block_checkbox",
              locals: { name: "members", checked: true, label: l(:label_member_plural),

app/views/custom_fields/_form.html.erb

@@ -87,15 +87,31 @@ See docs/COPYRIGHT.rdoc for more details.
   <% end %>
   <div id="custom_field_default_value_attributes">
     <div class="form--field" id="default_value_text">
-      <% if @custom_field.new_record? || @custom_field.field_format != 'bool' %>
-        <%= f.text_field(:default_value, container_class: '-wide') %>
+      <% if @custom_field.new_record? || !%w[text bool].include?(@custom_field.field_format) %>
+        <%= f.text_field :default_value,
+                         id: 'custom_fields_default_value_text',
+                         for: 'custom_fields_default_value_text',
+                         container_class: '-wide' %>
       <% end %>
     </div>
     <div class="form--field" style="display:none" id="default_value_bool">
       <% if @custom_field.new_record? || @custom_field.field_format == 'bool' %>
-        <%= f.check_box(:default_value) %>
+        <%= f.check_box :default_value,
+                        id: 'custom_fields_default_value_bool',
+                        for: 'custom_fields_default_value_bool' %>
       <% end %>
     </div>
+    <div class="form--field" style="display:none" id="default_value_long_text">
+      <% if @custom_field.new_record? || @custom_field.field_format == 'text' %>
+        <%= f.text_area :default_value,
+                        id: 'custom_fields_default_value_longtext',
+                        for: 'custom_fields_default_value_longtext',
+                        cols: 100,
+                        rows: 20,
+                        class: 'wiki-edit',
+                        with_text_formatting: true %>
+    <% end %>
+    </div>
   </div>
   <%= call_hook(:view_custom_fields_form_upper_box, custom_field: @custom_field, form: f) %>
 </section>

app/views/types/form/_form_configuration.html.erb

@@ -89,6 +89,7 @@ See docs/COPYRIGHT.rdoc for more details.
 <div class="grid-block">
   <div class="generic-table--action-buttons">
     <%= styled_button_tag t(@type.new_record? ? :button_create : :button_save),
+                          data: { disable_with: t(@type.new_record? ? :button_create : :button_save) },
                           class: 'form-configuration--save -highlight -with-icon icon-checkmark' %>
   </div>
 </div>

docs/release-notes/9-0-4/README.md

@@ -1,8 +1,8 @@
 ---
-title: OpenProject 9.0.3
+title: OpenProject 9.0.4
 sidebar_navigation:
-    title: 9.0.3
-release_version: 9.0.3
+    title: 9.0.4
+release_version: 9.0.4
 release_date: 2019-07-23
 ---
 
@@ -20,4 +20,4 @@ Thanks to David Haintz from the SEC Consult Vulnerability Lab (https://www.sec-c
 
 #### Contributions
 
-Thanks to David Haintz from [SEC Consult Vulnerability Lab](https://www.sec-consult.com/) for identifying and responsibly disclosing the identified issues.
+Thanks to David Haintz from [SEC Consult Vulnerability Lab](https://www.sec-consult.com/) for identifying and responsibly disclosing the identified issues.

frontend/src/app/components/user/user-link/user-link.component.spec.ts

@@ -42,6 +42,11 @@ describe('UserLinkComponent component test', () => {
     t: (key:string, args:any) => `Author: ${args.user}`
   };
 
+  let app:UserLinkComponent;
+  let fixture:ComponentFixture<UserLinkComponent>;
+  let element:HTMLElement;
+  let user:UserResource;
+
   beforeEach(async(() => {
 
     // noinspection JSIgnoredPromiseFromCall
@@ -54,32 +59,50 @@ describe('UserLinkComponent component test', () => {
         { provide: PathHelperService, useValue: PathHelperStub },
       ]
     }).compileComponents();
+
+    fixture = TestBed.createComponent(UserLinkComponent);
+    app = fixture.debugElement.componentInstance;
+    element = fixture.elementRef.nativeElement;
   }));
 
   describe('inner element', function() {
-    let app:UserLinkComponent;
-    let fixture:ComponentFixture<UserLinkComponent>
-    let element:HTMLElement;
+    describe('with the uer having the showUserPath attribute', function() {
+      beforeEach(async(() => {
+        user = {
+          name: 'First Last',
+          showUserPath: '/users/1'
+        } as UserResource;
 
-    let user = {
-      name: 'First Last',
-      href: '/api/v3/users/1',
-      idFromLink: '1',
-    } as UserResource;
+        app.user = user;
+        fixture.detectChanges();
+      }));
 
-    it('should render an inner link with specified classes', function() {
-      fixture = TestBed.createComponent(UserLinkComponent);
-      app = fixture.debugElement.componentInstance;
-      element = fixture.elementRef.nativeElement;
+      it('should render an inner link with specified classes', function () {
+        const link = element.querySelector('a')!;
 
-      app.user = user;
-      fixture.detectChanges();
+        expect(link.textContent).toEqual('First Last');
+        expect(link.getAttribute('title')).toEqual('Author: First Last');
+        expect(link.getAttribute('href')).toEqual('/users/1');
+      });
+    });
 
-      const link = element.querySelector('a')!;
+    describe('with the user not having the showUserPath attribute', function() {
+      beforeEach(async(() => {
+        user = {
+          name: 'First Last',
+          showUserPath: null
+        } as UserResource;
 
-      expect(link.textContent).toEqual('First Last');
-      expect(link.getAttribute('title')).toEqual('Author: First Last');
-      expect(link.getAttribute('href')).toEqual('/users/1');
+        app.user = user;
+        fixture.detectChanges();
+      }));
+
+      it('renders only the name', function () {
+        const link = element.querySelector('a');
+
+        expect(link).toBeNull();
+        expect(element.textContent).toEqual(' First Last ');
+      });
     });
   });
 });

frontend/src/app/components/user/user-link/user-link.component.ts

@@ -26,7 +26,7 @@
 // See docs/COPYRIGHT.rdoc for more details.
 //++
 
-import {Component, Inject, Input} from '@angular/core';
+import {ChangeDetectionStrategy, Component, Input} from '@angular/core';
 import {UserResource} from 'core-app/modules/hal/resources/user-resource';
 import {I18nService} from 'core-app/modules/common/i18n/i18n.service';
 import {PathHelperService} from 'core-app/modules/common/path-helper/path-helper.service';
@@ -34,26 +34,32 @@ import {PathHelperService} from 'core-app/modules/common/path-helper/path-helper
 @Component({
   selector: 'user-link',
   template: `
-    <a [attr.href]="href"
+    <a *ngIf="href"
+       [attr.href]="href"
        [attr.title]="label"
-       [textContent]="user.name">
+       [textContent]="name">
     </a>
-  `
+    <ng-container *ngIf="!href">
+      {{ name }}
+    <ng-container>
+  `,
+  changeDetection: ChangeDetectionStrategy.OnPush
 })
 export class UserLinkComponent {
   @Input() user:UserResource;
 
-  public href:string;
-  public label:string;
-  public name:string;
-
-  constructor(readonly pathHelper:PathHelperService,
-              readonly I18n:I18nService) {
+  constructor(readonly I18n:I18nService) {
   }
 
-  ngOnInit() {
-    this.href = this.pathHelper.userPath(this.user.idFromLink);
-    this.name = this.user.name;
-    this.label = this.I18n.t('js.label_author', { user: this.name });
+  public get href() {
+    return this.user && this.user.showUserPath;
+  }
+
+  public get name() {
+    return this.user && this.user.name;
+  }
+
+  public get label() {
+    return this.I18n.t('js.label_author', { user: this.name });
   }
 }

frontend/src/app/components/wp-activity/user/user-activity.component.html

@@ -8,13 +8,9 @@
                data-class-list="avatar">
   </user-avatar>
 
-  <span class="user" *ngIf="userActive">
-    <a [attr.href]="userPath"
-       [attr.aria-label]="userLabel"
-       [textContent]="userName">
-    </a>
+  <span class="user">
+    <user-link [user]="user"></user-link>
   </span>
-  <span class="user" *ngIf="!userActive">{{ userName }}</span>
   <span class="date">
     {{ isInitial ? text.label_created_on : text.label_updated_on }}
     <op-date-time [dateTimeValue]="activity.createdAt"></op-date-time>

frontend/src/app/components/wp-activity/user/user-activity.component.ts

@@ -64,10 +64,8 @@ export class UserActivityComponent extends WorkPackageCommentFieldHandler implem
   public userCanQuote = false;
 
   public userId:string | number;
+  public user:UserResource;
   public userName:string;
-  public userActive:boolean;
-  public userPath:string | null;
-  public userLabel:string;
   public userAvatar:string;
   public details:any[] = [];
   public isComment:boolean;
@@ -121,12 +119,10 @@ export class UserActivityComponent extends WorkPackageCommentFieldHandler implem
     this.userCacheService
       .require(this.activity.user.idFromLink)
       .then((user:UserResource) => {
+        this.user = user;
         this.userId = user.id!;
         this.userName = user.name;
-        this.userActive = user.isActive;
         this.userAvatar = user.avatar;
-        this.userPath = user.showUser.href;
-        this.userLabel = this.I18n.t('js.label_author', {user: this.userName});
         this.cdRef.detectChanges();
       });
   }

frontend/src/app/modules/admin/types/type-form-configuration.component.ts

@@ -80,9 +80,22 @@ export class TypeFormConfigurationComponent extends UntilDestroyedMixin implemen
     this.form = jQuery(this.element).closest('form');
     this.submit = this.form.find('.form-configuration--save');
 
+    // In the following we are triggering the form submit ourselves to work around
+    // a firefox shortcoming. But to avoid double submits which are sometimes not canceled fast
+    // enough, we need to memoize whether we have already submitted.
+    let submitted = false;
+
+    this.form.on('submit', (event) => {
+      submitted = true;
+    });
+
     // Capture mousedown on button because firefox breaks blur on click
     this.submit.on('mousedown', (event) => {
-      setTimeout(() => this.form.trigger('submit'), 50);
+      setTimeout(() => {
+        if (!submitted) {
+          this.form.trigger('submit');
+        }
+      }, 50);
       return true;
     });
 

frontend/src/app/modules/hal/resources/user-resource.ts

@@ -55,7 +55,7 @@ export class UserResource extends HalResource {
   }
 
   public get showUserPath() {
-    return this.showUser.$link.href;
+    return this.showUser ? this.showUser.$link.href : null;
   }
 
   public get isActive() {

frontend/src/app/modules/work_packages/routing/wp-full-view/wp-full-view.component.ts

@@ -26,10 +26,8 @@
 // See docs/COPYRIGHT.rdoc for more details.
 // ++
 
-import {UserResource} from 'core-app/modules/hal/resources/user-resource';
 import {WorkPackageResource} from 'core-app/modules/hal/resources/work-package-resource';
 import {StateService} from '@uirouter/core';
-import {TypeResource} from 'core-app/modules/hal/resources/type-resource';
 import {Component, Injector, OnInit} from '@angular/core';
 import {WorkPackageViewSelectionService} from 'core-app/modules/work_packages/routing/wp-view-base/view-services/wp-view-selection.service';
 import {WorkPackageSingleViewBase} from "core-app/modules/work_packages/routing/wp-view-base/work-package-single-view.base";
@@ -48,13 +46,6 @@ export class WorkPackagesFullViewComponent extends WorkPackageSingleViewBase imp
   public displayWatchButton:boolean;
   public watchers:any;
 
-  // Properties
-  public type:TypeResource;
-  public author:UserResource;
-  public authorPath:string;
-  public authorActive:boolean;
-  public attachments:any;
-
   // More menu
   public permittedActions:any;
   public actionsAvailable:any;
@@ -102,16 +93,5 @@ export class WorkPackagesFullViewComponent extends WorkPackageSingleViewBase imp
     if (wp.watchers) {
       this.watchers = (wp.watchers as any).elements;
     }
-
-    // Type
-    this.type = wp.type;
-
-    // Author
-    this.author = wp.author;
-    this.authorPath = this.author.showUserPath as string;
-    this.authorActive = this.author.isActive;
-
-    // Attachments
-    this.attachments = wp.attachments.elements;
   }
 }

lib/api/v3/users/user_representer.rb

@@ -48,6 +48,8 @@ module API
         self_link
 
         link :showUser do
+          next if represented.locked?
+
           {
             href: api_v3_paths.show_user(represented.id),
             type: 'text/html'

spec/features/custom_fields/create_long_text_spec.rb

@@ -0,0 +1,66 @@
+require 'spec_helper'
+require 'support/pages/custom_fields'
+
+describe 'custom fields', js: true do
+  let(:user) { FactoryBot.create :admin }
+  let(:cf_page) { Pages::CustomFields.new }
+  let(:editor) { ::Components::WysiwygEditor.new '#default_value_long_text' }
+  let(:type) { FactoryBot.create :type_task }
+  let(:project) { FactoryBot.create :project, enabled_module_names: %i[work_package_tracking], types: [type] }
+
+  let(:wp_page) { Pages::FullWorkPackageCreate.new project: project }
+
+  let(:default_text) do
+    <<~MARKDOWN
+      # This is an exemplary test
+
+      **Foo bar**
+
+    MARKDOWN
+  end
+
+  before do
+    login_as(user)
+  end
+
+  describe "creating a new long text custom field" do
+    before do
+      cf_page.visit!
+      click_on "Create a new custom field"
+    end
+
+    it "creates a new bool custom field" do
+      cf_page.set_name "New Field"
+      cf_page.select_format "Long text"
+
+      sleep 1
+
+      editor.set_markdown default_text
+
+      cf_page.set_all_projects true
+      click_on "Save"
+
+      expect(page).to have_text("Successful creation")
+      expect(page).to have_text("New Field")
+
+      cf = CustomField.last
+      expect(cf.field_format).to eq 'text'
+
+      # textareas get carriage returns entered
+      expect(cf.default_value.gsub("\r\n", "\n").strip).to eq default_text.strip
+
+      type.custom_fields << cf
+      type.save!
+
+
+      wp_page.visit!
+      wp_editor = TextEditorField.new(page, 'description', selector: ".inline-edit--container.customField#{cf.id}")
+      wp_editor.expect_active!
+
+      wp_editor.ckeditor.in_editor do |container, _|
+        expect(container).to have_selector('h1', text: 'This is an exemplary test')
+        expect(container).to have_selector('strong', text: 'Foo bar')
+      end
+    end
+  end
+end

spec/lib/api/v3/users/user_representer_spec.rb

@@ -29,7 +29,8 @@
 require 'spec_helper'
 
 describe ::API::V3::Users::UserRepresenter do
-  let(:user) { FactoryBot.build_stubbed(:user, status: 1) }
+  let(:status) { Principal::STATUSES[:active] }
+  let(:user) { FactoryBot.build_stubbed(:user, status: status) }
   let(:current_user) { FactoryBot.build_stubbed(:user) }
   let(:representer) { described_class.new(user, current_user: current_user) }
 
@@ -150,9 +151,19 @@ describe ::API::V3::Users::UserRepresenter do
         expect(subject).to have_json_path('_links/self/href')
       end
 
-      it_behaves_like 'has an untitled link' do
-        let(:link) { 'showUser' }
-        let(:href) { "/users/#{user.id}" }
+      context 'showUser' do
+        it_behaves_like 'has an untitled link' do
+          let(:link) { 'showUser' }
+          let(:href) { "/users/#{user.id}" }
+        end
+
+        context 'with a locked user' do
+          let(:status) { Principal::STATUSES[:locked] }
+
+          it_behaves_like 'has no link' do
+            let(:link) { 'showUser' }
+          end
+        end
       end
 
       context 'when regular current_user' do

spec/support/pages/custom_fields.rb

@@ -43,7 +43,11 @@ module Pages
     end
 
     def set_default_value(value)
-      find("#custom_field_default_value").set value
+      fill_in 'custom_field[default_value]', with: value
+    end
+
+    def set_all_projects(value)
+      find('#custom_field_is_for_all').set value
     end
 
     def has_form_element?(name)