Merge pull request #49 from anyone-oslo/remove-username

Remove username

app/controllers/admin/password_resets_controller.rb

@@ -9,7 +9,7 @@ class PasswordResetsController < Admin::AdminController
     layout "admin"
 
     def create
-      @user = find_user_by_email(params[:username])
+      @user = find_user_by_email(params[:email])
       if @user
         @password_reset_token = @user.password_reset_tokens.create
         deliver_password_reset(@user, @password_reset_token)
@@ -50,7 +50,7 @@ def deliver_password_reset(user, password_reset)
     def find_user_by_email(email)
       return unless email
 
-      User.login_name(params[:username])
+      User.find_by_email(params[:email])
     end
 
     def user_params

app/controllers/sessions_controller.rb

@@ -2,7 +2,7 @@
 
 class SessionsController < ::ApplicationController
   def create
-    user = find_user(params[:username], params[:password])
+    user = find_user(params[:email], params[:password])
     authenticate!(user) if user
 
     if logged_in?
@@ -21,7 +21,7 @@ def destroy
 
   protected
 
-  def find_user(username, password)
-    User.authenticate(username, password: password) if username && password
+  def find_user(email, password)
+    User.authenticate(email, password: password) if email && password
   end
 end

app/models/user.rb

@@ -23,8 +23,6 @@ class User < ApplicationRecord
 
   serialize :persistent_data
 
-  validates :username, presence: true, uniqueness: { case_sensitive: false }
-
   validates :name, presence: true
 
   validates :email,
@@ -37,7 +35,6 @@ class User < ApplicationRecord
 
   validate :confirm_password_must_match
 
-  before_validation :ensure_username
   before_validation :hash_password
   before_create :ensure_first_user_has_all_roles
 
@@ -47,18 +44,13 @@ class User < ApplicationRecord
 
   class << self
     def authenticate(email, password:)
-      user = User.login_name(email)
+      user = User.find_by_email(email)
       user if user.try { |u| u.authenticate!(password) }
     end
 
     def find_by_email(str)
       find_by("LOWER(email) = ?", str.to_s.downcase)
     end
-
-    # Finds a user by either username or email address.
-    def login_name(string)
-      find_by(username: string.to_s) || find_by_email(string)
-    end
   end
 
   def authenticate!(password)
@@ -102,10 +94,6 @@ def encrypt_password(password)
     BCrypt::Password.create(password)
   end
 
-  def ensure_username
-    self.username ||= email
-  end
-
   def ensure_first_user_has_all_roles
     return if User.any?
 

app/views/admin/users/login.html.erb

@@ -19,7 +19,7 @@
     <%= form_tag session_path do %>
       <p>
         <label>Email address</label>
-        <%= text_field_tag :username, '' %>
+        <%= text_field_tag :email, '' %>
       </p>
       <p>
         <label>Password</label>
@@ -54,7 +54,7 @@
         and we'll send you a link where you can reset your password.
       </p>
       <p>
-        <%= text_field_tag :username, '' %>
+        <%= text_field_tag :email, '' %>
       </p>
       <p>
         <button type="submit">

app/views/admin/users/new_password.html.erb

@@ -13,7 +13,7 @@
 </div>
 <%= form_tag(reset_password_admin_users_path, method: :post) do %>
   <p>
-    <%= text_field_tag :username, '', size: 32 %>
+    <%= text_field_tag :email, '', size: 32 %>
     <%= submit_tag "Find" %>
   </p>
 </form>

db/migrate/20220615160300_remove_username.rb

@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+class RemoveUsername < ActiveRecord::Migration[7.0]
+  def change
+    remove_column :users, :username, :string
+  end
+end

spec/controllers/admin/password_resets_controller_spec.rb

@@ -20,7 +20,7 @@
 
       before do
         perform_enqueued_jobs do
-          post :create, params: { username: user.email }
+          post :create, params: { email: user.email }
         end
       end
 
@@ -50,7 +50,7 @@
     end
 
     context "with a non-existant user" do
-      before { post :create, params: { username: "[email protected]" } }
+      before { post :create, params: { email: "[email protected]" } }
 
       it { is_expected.to redirect_to(login_admin_users_url) }
 

spec/internal/db/schema.rb

@@ -10,8 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2021_02_10_235200) do
-
+ActiveRecord::Schema[7.0].define(version: 2022_06_15_160300) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_trgm"
   enable_extension "plpgsql"
@@ -21,8 +20,8 @@
     t.string "filename", null: false
     t.string "content_type", null: false
     t.integer "content_length", null: false
-    t.datetime "created_at"
-    t.datetime "updated_at"
+    t.datetime "created_at", precision: nil
+    t.datetime "updated_at", precision: nil
     t.string "content_hash", null: false
     t.integer "user_id"
   end
@@ -31,8 +30,8 @@
     t.string "name"
     t.string "slug"
     t.integer "position"
-    t.datetime "created_at", null: false
-    t.datetime "updated_at", null: false
+    t.datetime "created_at", precision: nil, null: false
+    t.datetime "updated_at", precision: nil, null: false
     t.index ["slug"], name: "index_categories_on_slug"
   end
 
@@ -41,12 +40,12 @@
     t.integer "attempts", default: 0
     t.text "handler"
     t.text "last_error"
-    t.datetime "run_at"
-    t.datetime "locked_at"
-    t.datetime "failed_at"
+    t.datetime "run_at", precision: nil
+    t.datetime "locked_at", precision: nil
+    t.datetime "failed_at", precision: nil
     t.string "locked_by"
-    t.datetime "created_at", null: false
-    t.datetime "updated_at", null: false
+    t.datetime "created_at", precision: nil, null: false
+    t.datetime "updated_at", precision: nil, null: false
     t.string "queue"
   end
 
@@ -64,8 +63,8 @@
     t.integer "crop_height", null: false
     t.integer "crop_start_x", null: false
     t.integer "crop_start_y", null: false
-    t.datetime "created_at", null: false
-    t.datetime "updated_at", null: false
+    t.datetime "created_at", precision: nil, null: false
+    t.datetime "updated_at", precision: nil, null: false
     t.index ["image_id", "image_type", "format", "width", "height", "crop_width", "crop_height", "crop_start_x", "crop_start_y"], name: "dynamic_image_variants_by_format_and_size", unique: true
     t.index ["image_id", "image_type"], name: "dynamic_image_variants_by_image"
     t.index ["image_type", "image_id"], name: "index_dynamic_image_variants_on_image"
@@ -74,8 +73,8 @@
   create_table "images", id: :serial, force: :cascade do |t|
     t.string "filename", null: false
     t.string "content_type", null: false
-    t.datetime "created_at"
-    t.datetime "updated_at"
+    t.datetime "created_at", precision: nil
+    t.datetime "updated_at", precision: nil
     t.string "content_hash", null: false
     t.integer "content_length", null: false
     t.string "colorspace", null: false
@@ -92,17 +91,17 @@
   create_table "invite_roles", id: :serial, force: :cascade do |t|
     t.integer "invite_id"
     t.string "name"
-    t.datetime "created_at"
-    t.datetime "updated_at"
+    t.datetime "created_at", precision: nil
+    t.datetime "updated_at", precision: nil
   end
 
   create_table "invites", id: :serial, force: :cascade do |t|
     t.integer "user_id"
     t.string "email"
     t.string "token"
-    t.datetime "sent_at"
-    t.datetime "created_at"
-    t.datetime "updated_at"
+    t.datetime "sent_at", precision: nil
+    t.datetime "created_at", precision: nil
+    t.datetime "updated_at", precision: nil
   end
 
   create_table "localizations", id: :serial, force: :cascade do |t|
@@ -111,8 +110,8 @@
     t.string "name"
     t.string "locale"
     t.text "value"
-    t.datetime "created_at"
-    t.datetime "updated_at"
+    t.datetime "created_at", precision: nil
+    t.datetime "updated_at", precision: nil
     t.index ["localizable_id", "localizable_type", "name", "locale"], name: "index_textbits_on_locale", unique: true
     t.index ["localizable_id", "localizable_type"], name: "index_localizations_on_localizable_id_and_localizable_type"
   end
@@ -128,8 +127,8 @@
     t.bigint "page_id"
     t.bigint "attachment_id"
     t.integer "position"
-    t.datetime "created_at", null: false
-    t.datetime "updated_at", null: false
+    t.datetime "created_at", precision: nil, null: false
+    t.datetime "updated_at", precision: nil, null: false
     t.index ["attachment_id"], name: "index_page_files_on_attachment_id"
     t.index ["page_id"], name: "index_page_files_on_page_id"
   end
@@ -147,8 +146,8 @@
     t.integer "page_id"
     t.string "locale"
     t.string "path"
-    t.datetime "created_at", null: false
-    t.datetime "updated_at", null: false
+    t.datetime "created_at", precision: nil, null: false
+    t.datetime "updated_at", precision: nil, null: false
     t.index ["locale", "path"], name: "index_page_paths_on_locale_and_path", unique: true
   end
 
@@ -157,23 +156,23 @@
     t.integer "position"
     t.string "byline"
     t.string "template"
-    t.datetime "created_at"
-    t.datetime "updated_at"
+    t.datetime "created_at", precision: nil
+    t.datetime "updated_at", precision: nil
     t.integer "user_id"
     t.integer "status", default: 0, null: false
     t.boolean "feed_enabled", default: false, null: false
-    t.datetime "published_at"
+    t.datetime "published_at", precision: nil
     t.string "redirect_to"
     t.integer "image_id"
     t.string "image_link"
     t.boolean "news_page", default: false, null: false
     t.boolean "autopublish", default: false, null: false
     t.string "unique_name"
-    t.datetime "last_comment_at"
+    t.datetime "last_comment_at", precision: nil
     t.boolean "pinned", default: false, null: false
     t.integer "meta_image_id"
-    t.datetime "starts_at"
-    t.datetime "ends_at"
+    t.datetime "starts_at", precision: nil
+    t.datetime "ends_at", precision: nil
     t.boolean "all_day", default: false, null: false
     t.index ["ends_at"], name: "index_pages_on_ends_at"
     t.index ["parent_page_id"], name: "index_pages_on_parent_page_id"
@@ -187,16 +186,16 @@
   create_table "password_reset_tokens", id: :serial, force: :cascade do |t|
     t.integer "user_id"
     t.string "token"
-    t.datetime "expires_at"
-    t.datetime "created_at"
-    t.datetime "updated_at"
+    t.datetime "expires_at", precision: nil
+    t.datetime "created_at", precision: nil
+    t.datetime "updated_at", precision: nil
   end
 
   create_table "roles", id: :serial, force: :cascade do |t|
     t.integer "user_id"
     t.string "name"
-    t.datetime "created_at"
-    t.datetime "updated_at"
+    t.datetime "created_at", precision: nil
+    t.datetime "updated_at", precision: nil
     t.index ["user_id"], name: "index_roles_on_user_id"
   end
 
@@ -209,9 +208,9 @@
     t.text "content"
     t.text "tags"
     t.boolean "published", default: true, null: false
-    t.datetime "created_at", precision: 6, null: false
-    t.datetime "updated_at", precision: 6, null: false
-    t.datetime "record_updated_at", null: false
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
+    t.datetime "record_updated_at", precision: nil, null: false
     t.string "tsv_config", default: "simple_unaccent", null: false
     t.tsvector "tsv"
     t.index ["name"], name: "search_documents_trgm_idx", opclass: :gin_trgm_ops, using: :gin
@@ -235,13 +234,12 @@
   end
 
   create_table "users", id: :serial, force: :cascade do |t|
-    t.string "username"
     t.string "hashed_password"
     t.string "name"
     t.string "email"
-    t.datetime "last_login_at"
+    t.datetime "last_login_at", precision: nil
     t.integer "created_by"
-    t.datetime "created_at"
+    t.datetime "created_at", precision: nil
     t.text "persistent_data"
     t.boolean "activated", default: false, null: false
     t.integer "image_id"

spec/models/user_spec.rb

@@ -15,7 +15,6 @@
   it { is_expected.to validate_presence_of(:email) }
   it { is_expected.to validate_presence_of(:name) }
 
-  it { is_expected.to validate_uniqueness_of(:username).case_insensitive }
   it { is_expected.to validate_uniqueness_of(:email).case_insensitive }
 
   it { is_expected.to allow_value("[email protected]").for(:email) }
@@ -25,12 +24,6 @@
   it { is_expected.to allow_value("long enough").for(:password) }
   it { is_expected.not_to allow_value("eep").for(:password) }
 
-  context "when email is blank" do
-    subject { build(:user, email: nil) }
-
-    it { is_expected.to validate_presence_of(:username) }
-  end
-
   describe "password validation" do
     subject { user.valid? }
 

spec/support/features/login_helper.rb

@@ -13,7 +13,7 @@ def login_as(user = nil)
   def login_with(email, password)
     visit login_admin_users_path
     within ".password.login-tab" do
-      fill_in "username", with: email
+      fill_in "email", with: email
       fill_in "password", with: password
       click_button "Sign in"
     end