Unique twitter and github usernames #2061 (#2255)

* Set github and twitter usernames to nil if they are blank

* Verify github and twitter username in the Admin::UsersController (just in case)

* Add unique indexs on users twitter_username and github_username

app/controllers/admin/users_controller.rb

@@ -51,7 +51,14 @@ module Admin
         linkedin_url
       ]
       accessible << %i[password password_confirmation] if params[:user][:password].present?
-      params.require(:user).permit(accessible)
+      verify_usernames params.require(:user).permit(accessible)
+    end
+
+    # make sure usernames are not empty, to be able to use the database unique index
+    def verify_usernames(user_params)
+      user_params[:twitter_username] = nil if user_params[:twitter_username] == ""
+      user_params[:github_username] = nil if user_params[:github_username] == ""
+      user_params
     end
   end
 end

app/models/user.rb

@@ -143,6 +143,8 @@ class User < ApplicationRecord
   after_create :estimate_default_language!
   before_update :mentorship_status_update
   before_validation :set_username
+  # make sure usernames are not empty, to be able to use the database unique index
+  before_validation :verify_twitter_username, :verify_github_username
   before_validation :set_config_input
   before_validation :downcase_email
   before_validation :check_for_username_change
@@ -403,6 +405,14 @@ class User < ApplicationRecord
     Notification.send_welcome_notification(id)
   end
 
+  def verify_twitter_username
+    self.twitter_username = nil if twitter_username == ""
+  end
+
+  def verify_github_username
+    self.github_username = nil if github_username == ""
+  end
+
   def set_username
     set_temp_username if username.blank?
     self.username = username&.downcase

db/migrate/20190401100844_add_unique_index_to_users_twitter_usename.rb

@@ -0,0 +1,5 @@
+class AddUniqueIndexToUsersTwitterUsename < ActiveRecord::Migration[5.1]
+  def change
+    add_index :users, :twitter_username, unique: true
+  end
+end

db/migrate/20190401100850_add_unique_index_to_users_github_usename.rb

@@ -0,0 +1,5 @@
+class AddUniqueIndexToUsersGithubUsename < ActiveRecord::Migration[5.1]
+  def change
+    add_index :users, :github_username, unique: true
+  end
+end

db/schema.rb

@@ -484,6 +484,7 @@ ActiveRecord::Schema.define(version: 20190401213605) do
     t.index ["json_data"], name: "index_notifications_on_json_data", using: :gin
     t.index ["notifiable_id"], name: "index_notifications_on_notifiable_id"
     t.index ["notifiable_type"], name: "index_notifications_on_notifiable_type"
+    t.index ["user_id", "organization_id", "notifiable_id", "notifiable_type", "action"], name: "index_notifications_on_user_organization_notifiable_and_action", unique: true
     t.index ["user_id"], name: "index_notifications_on_user_id"
   end
 
@@ -888,9 +889,11 @@ ActiveRecord::Schema.define(version: 20190401213605) do
     t.datetime "workshop_expiration"
     t.index ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true
     t.index ["email"], name: "index_users_on_email", unique: true
+    t.index ["github_username"], name: "index_users_on_github_username", unique: true
     t.index ["language_settings"], name: "index_users_on_language_settings", using: :gin
     t.index ["organization_id"], name: "index_users_on_organization_id"
     t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true
+    t.index ["twitter_username"], name: "index_users_on_twitter_username", unique: true
     t.index ["username"], name: "index_users_on_username", unique: true
   end
 

spec/models/user_spec.rb

@@ -50,6 +50,27 @@ RSpec.describe User, type: :model do
     service.get_user
   end
 
+  describe "makes sure usernames are not blank" do
+    it "sets twitter username to nil" do
+      user = create(:user, twitter_username: "")
+      user.reload
+      expect(user.twitter_username).to eq(nil)
+    end
+
+    it "sets github username to nil" do
+      user = create(:user, github_username: "")
+      user.reload
+      expect(user.github_username).to eq(nil)
+    end
+
+    it "sets correct usernames if they are not blank" do
+      user = create(:user, github_username: "hello", twitter_username: "world")
+      user.reload
+      expect(user.github_username).to eq("hello")
+      expect(user.twitter_username).to eq("world")
+    end
+  end
+
   describe "validations" do
     it "gets a username after create" do
       expect(user.username).not_to eq(nil)