From a06410d995e1200c7734b2d50e68506320d13dd0 Mon Sep 17 00:00:00 2001
From: Aishwarya Subramanian <asubramanian@gitlab.com>
Date: Mon, 26 Aug 2019 18:20:18 +0000
Subject: Using before_save method instead of setter

Removed unused method for name setter method
---
 app/models/user.rb                                   | 12 +++++++++++-
 changelogs/unreleased/user_name_migration.yml        |  5 +++++
 .../20190820163320_add_first_last_name_to_user.rb    | 14 ++++++++++++++
 db/schema.rb                                         |  4 +++-
 spec/models/user_spec.rb                             | 20 ++++++++++++++++++++
 5 files changed, 53 insertions(+), 2 deletions(-)
 create mode 100644 changelogs/unreleased/user_name_migration.yml
 create mode 100644 db/migrate/20190820163320_add_first_last_name_to_user.rb

diff --git a/app/models/user.rb b/app/models/user.rb
index 6131a8dc710..6107aaa7fca 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -161,6 +161,8 @@ class User < ApplicationRecord
   #
   # Note: devise :validatable above adds validations for :email and :password
   validates :name, presence: true, length: { maximum: 128 }
+  validates :first_name, length: { maximum: 255 }
+  validates :last_name, length: { maximum: 255 }
   validates :email, confirmation: true
   validates :notification_email, presence: true
   validates :notification_email, devise_email: true, if: ->(user) { user.notification_email != user.email }
@@ -881,7 +883,15 @@ class User < ApplicationRecord
   end
 
   def first_name
-    name.split.first unless name.blank?
+    read_attribute(:first_name) || begin
+      name.split(' ').first unless name.blank?
+    end
+  end
+
+  def last_name
+    read_attribute(:last_name) || begin
+      name.split(' ').drop(1).join(' ') unless name.blank?
+    end
   end
 
   def projects_limit_left
diff --git a/changelogs/unreleased/user_name_migration.yml b/changelogs/unreleased/user_name_migration.yml
new file mode 100644
index 00000000000..11d6ff2e8b2
--- /dev/null
+++ b/changelogs/unreleased/user_name_migration.yml
@@ -0,0 +1,5 @@
+---
+title: Add First and Last name columns to User model
+merge_request: 31985
+author:
+type: added
diff --git a/db/migrate/20190820163320_add_first_last_name_to_user.rb b/db/migrate/20190820163320_add_first_last_name_to_user.rb
new file mode 100644
index 00000000000..0ea465fc2e2
--- /dev/null
+++ b/db/migrate/20190820163320_add_first_last_name_to_user.rb
@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddFirstLastNameToUser < ActiveRecord::Migration[5.2]
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  def change
+    add_column(:users, :first_name, :string, null: true, limit: 255)
+    add_column(:users, :last_name, :string, null: true, limit: 255)
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 944d23e5365..36bf2371994 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2019_08_15_093949) do
+ActiveRecord::Schema.define(version: 2019_08_20_163320) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_trgm"
@@ -3511,6 +3511,8 @@ ActiveRecord::Schema.define(version: 2019_08_15_093949) do
     t.text "note"
     t.integer "roadmap_layout", limit: 2
     t.integer "bot_type", limit: 2
+    t.string "first_name", limit: 255
+    t.string "last_name", limit: 255
     t.index ["accepted_term_id"], name: "index_users_on_accepted_term_id"
     t.index ["admin"], name: "index_users_on_admin"
     t.index ["bot_type"], name: "index_users_on_bot_type"
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 46b86e8393d..8338d2b5b39 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -103,6 +103,14 @@ describe User do
       it { is_expected.to validate_length_of(:name).is_at_most(128) }
     end
 
+    describe 'first name' do
+      it { is_expected.to validate_length_of(:first_name).is_at_most(255) }
+    end
+
+    describe 'last name' do
+      it { is_expected.to validate_length_of(:last_name).is_at_most(255) }
+    end
+
     describe 'username' do
       it 'validates presence' do
         expect(subject).to validate_presence_of(:username)
@@ -678,6 +686,18 @@ describe User do
     end
   end
 
+  describe 'name getters' do
+    let(:user) { create(:user, name: 'Kane Martin William') }
+
+    it 'derives first name from full name, if not present' do
+      expect(user.first_name).to eq('Kane')
+    end
+
+    it 'derives last name from full name, if not present' do
+      expect(user.last_name).to eq('Martin William')
+    end
+  end
+
   describe '#highest_role' do
     let(:user) { create(:user) }
 
-- 
cgit v1.2.1