From ebfe19e8e7690598f86facc0bb18df4052468fc0 Mon Sep 17 00:00:00 2001 From: Brandon Labuschagne Date: Thu, 14 Mar 2019 12:32:07 +0200 Subject: [PATCH] Add limit of 128 characters to users name Truncate existing users names which exceed 128 characters Include test for truncating users names --- app/models/user.rb | 2 +- .../57493-add-limit-to-user-name.yml | 5 +++++ .../20190325080727_truncate_user_fullname.rb | 21 +++++++++++++++++++ .../migrations/truncate_user_fullname_spec.rb | 21 +++++++++++++++++++ spec/models/user_spec.rb | 5 +++++ 5 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/57493-add-limit-to-user-name.yml create mode 100644 db/migrate/20190325080727_truncate_user_fullname.rb create mode 100644 spec/migrations/truncate_user_fullname_spec.rb diff --git a/app/models/user.rb b/app/models/user.rb index b426d100537..259889995d3 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -159,7 +159,7 @@ class User < ApplicationRecord # Validations # # Note: devise :validatable above adds validations for :email and :password - validates :name, presence: true + validates :name, presence: true, length: { maximum: 128 } validates :email, confirmation: true validates :notification_email, presence: true validates :notification_email, devise_email: true, if: ->(user) { user.notification_email != user.email } diff --git a/changelogs/unreleased/57493-add-limit-to-user-name.yml b/changelogs/unreleased/57493-add-limit-to-user-name.yml new file mode 100644 index 00000000000..e6c78572d23 --- /dev/null +++ b/changelogs/unreleased/57493-add-limit-to-user-name.yml @@ -0,0 +1,5 @@ +--- +title: Set user.name limit to 128 characters +merge_request: 26146 +author: +type: changed diff --git a/db/migrate/20190325080727_truncate_user_fullname.rb b/db/migrate/20190325080727_truncate_user_fullname.rb new file mode 100644 index 00000000000..e5f88671eef --- /dev/null +++ b/db/migrate/20190325080727_truncate_user_fullname.rb @@ -0,0 +1,21 @@ +# rubocop:disable Migration/UpdateLargeTable +class TruncateUserFullname < ActiveRecord::Migration[5.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + truncated_name = Arel.sql('SUBSTRING(name from 1 for 128)') + where_clause = Arel.sql("LENGTH(name) > 128") + + update_column_in_batches(:users, :name, truncated_name) do |table, query| + query.where(where_clause) + end + end + + def down + # noop + end +end diff --git a/spec/migrations/truncate_user_fullname_spec.rb b/spec/migrations/truncate_user_fullname_spec.rb new file mode 100644 index 00000000000..17fd4d9f688 --- /dev/null +++ b/spec/migrations/truncate_user_fullname_spec.rb @@ -0,0 +1,21 @@ +require 'spec_helper' +require Rails.root.join('db', 'migrate', '20190325080727_truncate_user_fullname.rb') + +describe TruncateUserFullname, :migration do + let(:users) { table(:users) } + + let(:user_short) { create_user(name: 'abc', email: 'test_short@example.com') } + let(:user_long) { create_user(name: 'a' * 200 + 'z', email: 'test_long@example.com') } + + def create_user(params) + users.create!(params.merge(projects_limit: 0)) + end + + it 'truncates user full name to the first 128 characters' do + expect { migrate! }.to change { user_long.reload.name }.to('a' * 128) + end + + it 'does not truncate short names' do + expect { migrate! }.not_to change { user_short.reload.name.length } + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index b7e36748fa2..a45a2737b13 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -98,6 +98,11 @@ describe User do end describe 'validations' do + describe 'name' do + it { is_expected.to validate_presence_of(:name) } + it { is_expected.to validate_length_of(:name).is_at_most(128) } + end + describe 'username' do it 'validates presence' do expect(subject).to validate_presence_of(:username) -- GitLab