Merge branch 'release/11.1' into dev

app/models/group.rb

@@ -40,7 +40,7 @@ class Group < Principal
   alias_attribute(:groupname, :lastname)
   validates_presence_of :groupname
   validate :uniqueness_of_groupname
-  validates_length_of :groupname, maximum: 30
+  validates_length_of :groupname, maximum: 256
 
   # HACK: We want to have the :preference association on the Principal to allow
   # for eager loading preferences.

app/models/user.rb

@@ -120,9 +120,9 @@ class User < Principal
   # Login must contain letters, numbers, underscores only
   validates_format_of :login, with: /\A[a-z0-9_\-@\.+ ]*\z/i
   validates_length_of :login, maximum: 256
-  validates_length_of :firstname, :lastname, maximum: 30
+  validates_length_of :firstname, :lastname, maximum: 256
   validates_format_of :mail, with: /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\z/i, allow_blank: true
-  validates_length_of :mail, maximum: 60, allow_nil: true
+  validates_length_of :mail, maximum: 256, allow_nil: true
   validates_confirmation_of :password, allow_nil: true
   validates_inclusion_of :mail_notification, in: MAIL_NOTIFICATION_OPTIONS.map(&:first), allow_blank: true
 

db/migrate/20210127134438_alter_user_attributes_max_length.rb

@@ -0,0 +1,13 @@
+class AlterUserAttributesMaxLength < ActiveRecord::Migration[6.0]
+  def up
+    change_column :users, :firstname, :string, limit: nil
+    change_column :users, :lastname, :string, limit: nil
+    change_column :users, :mail, :string, limit: nil
+  end
+
+  def down
+    change_column :users, :firstname, :string, limit: 30
+    change_column :users, :lastname, :string, limit: 30
+    change_column :users, :mail, :string, limit: 60
+  end
+end

frontend/src/app/components/modals/export-modal/wp-table-export.modal.ts

@@ -8,7 +8,7 @@ import {HalLink} from "core-app/modules/hal/hal-link/hal-link";
 import {I18nService} from "core-app/modules/common/i18n/i18n.service";
 import {OpModalLocalsToken} from "core-components/op-modals/op-modal.service";
 import * as URI from 'urijs';
-import {HttpClient} from '@angular/common/http';
+import {HttpClient, HttpErrorResponse} from '@angular/common/http';
 import {LoadingIndicatorService} from "core-app/modules/common/loading-indicator/loading-indicator.service";
 import {Observable} from 'rxjs';
 import {NotificationsService} from "core-app/modules/common/notifications/notifications.service";
@@ -105,8 +105,21 @@ export class WpTableExportModal extends OpModalComponent implements OnInit {
     this.service.show(JobStatusModal, 'global', { jobId: jobId });
   }
 
-  private handleError(error:string) {
-    this.notifications.addError(error || this.I18n.t('js.error.internal'));
+  private handleError(error:HttpErrorResponse) {
+    // There was an error but the status code is actually a 200.
+    // If that is the case the response's content-type probably does not match
+    // the expected type (json).
+    // Currently this happens e.g. when exporting Atom which actually is not an export
+    // but rather a feed to follow.
+    if (error.status === 200 && error.url) {
+      window.open(error.url);
+    } else {
+      this.showError(error);
+    }
+  }
+
+  private showError(error:HttpErrorResponse) {
+    this.notifications.addError(error.message || this.I18n.t('js.error.internal'));
   }
 
   private addColumnsToHref(href:string) {

modules/ldap_groups/app/models/ldap_groups/synchronized_group.rb

@@ -18,6 +18,7 @@ module LdapGroups
 
     validates_presence_of :dn
     validates_presence_of :group
+    validates_associated :group
     validates_presence_of :auth_source
 
     before_destroy :remove_all_members

modules/ldap_groups/lib/open_project/ldap_groups/synchronize_filter.rb

@@ -77,7 +77,7 @@ module OpenProject::LdapGroups
         Group.where(id: sync.group_id).update_all(lastname: name)
       else
         # Create an OpenProject group
-        sync.group = Group.find_or_initialize_by(groupname: name)
+        sync.group = Group.find_or_create_by!(groupname: name)
       end
     end
 

spec/features/work_packages/export_spec.rb

@@ -242,4 +242,31 @@ describe 'work package export', type: :feature do
       end
     end
   end
+
+  # Atom exports are not downloaded. In fact, it is not even a download but rather
+  # a feed one can follow.
+  context 'Atom export', js: true do
+    let(:export_type) { 'Atom' }
+    context 'with default filter' do
+      before do
+        work_packages_page.visit_index
+        filters.expect_filter_count 1
+        filters.open
+      end
+
+      it 'shows an xml with work packages' do
+        settings_menu.open_and_choose 'Export ...'
+
+        # The feed is opened in a new tab
+        new_window = window_opened_by { click_on export_type }
+
+        within_window new_window do
+          expect(page).to have_text(wp_1.description)
+          expect(page).to have_text(wp_2.description)
+          expect(page).to have_text(wp_3.description)
+          expect(page).to have_text(wp_4.description)
+        end
+      end
+    end
+  end
 end

spec/models/group_spec.rb

@@ -49,6 +49,29 @@ describe Group, type: :model do
     expect(g.save).to eq true
   end
 
+  describe 'with long but allowed attributes' do
+    it 'is valid' do
+      group.groupname = 'a' * 256
+      expect(group).to be_valid
+      expect(group.save).to be_truthy
+    end
+  end
+
+  describe 'with a name too long' do
+    it 'is invalid' do
+      group.groupname = 'a' * 257
+      expect(group).not_to be_valid
+      expect(group.save).to be_falsey
+    end
+  end
+
+  describe 'a user with and overly long firstname (> 256 chars)' do
+    it 'is invalid' do
+      user.firstname = 'a' * 257
+      expect(user).not_to be_valid
+      expect(user.save).to be_falsey
+    end
+  end
 
   describe 'from legacy specs' do
     let!(:roles) { FactoryBot.create_list :role, 2 }

spec/models/user_spec.rb

@@ -98,6 +98,34 @@ describe User, type: :model do
     end
   end
 
+  describe 'with long but allowed attributes' do
+    it 'is valid' do
+      user.firstname = 'a' * 256
+      user.lastname = 'b' * 256
+      user.mail = 'fo' + ('o' * 237) + '@mail.example.com'
+      expect(user).to be_valid
+      expect(user.save).to be_truthy
+    end
+  end
+
+
+  describe 'a user with and overly long firstname (> 256 chars)' do
+    it 'is invalid' do
+      user.firstname = 'a' * 257
+      expect(user).not_to be_valid
+      expect(user.save).to be_falsey
+    end
+  end
+
+  describe 'a user with and overly long lastname (> 256 chars)' do
+    it 'is invalid' do
+      user.lastname = 'a' * 257
+      expect(user).not_to be_valid
+      expect(user.save).to be_falsey
+    end
+  end
+
+
   describe 'login whitespace' do
     before do
       user.login = login

spec_legacy/unit/mail_handler_spec.rb

@@ -383,11 +383,8 @@ describe MailHandler, type: :model do
       ['jsmith@example.net', 'John'] => ['jsmith@example.net', 'John', '-'],
       ['jsmith@example.net', 'John Smith'] => ['jsmith@example.net', 'John', 'Smith'],
       ['jsmith@example.net', 'John Paul Smith'] => ['jsmith@example.net', 'John', 'Paul Smith'],
-      # TODO: implement https://github.com/redmine/redmine/commit/a00f04886fac78e489bb030d20414ebdf10841e3
-      # ['jsmith@example.net', 'AVeryLongFirstnameThatExceedsTheMaximumLength Smith'] => ['jsmith@example.net', 'AVeryLongFirstnameThatExceedsT', 'Smith'],
-      # ['jsmith@example.net', 'John AVeryLongLastnameThatExceedsTheMaximumLength'] => ['jsmith@example.net', 'John', 'AVeryLongLastnameThatExceedsTh']
-      ['jsmith@example.net', 'AVeryLongFirstnameThatExceedsTheMaximumLength Smith'] => ['jsmith@example.net', '-', 'Smith'],
-      ['jsmith@example.net', 'John AVeryLongLastnameThatExceedsTheMaximumLength'] => ['jsmith@example.net', 'John', '-']
+      ['jsmith@example.net', 'AVeryLongFirstnameThatNoLongerExceedsTheMaximumLength Smith'] => ['jsmith@example.net', 'AVeryLongFirstnameThatNoLongerExceedsTheMaximumLength', 'Smith'],
+      ['jsmith@example.net', 'John AVeryLongLastnameThatNoLongerExceedsTheMaximumLength'] => ['jsmith@example.net', 'John', 'AVeryLongLastnameThatNoLongerExceedsTheMaximumLength']
     }
 
     to_test.each do |attrs, expected|