refactor: ⚡ Username and slug unique indexes, before actions for set link and user

confirm_password_validation around action, unnecessary helpers removed, rubocop-rails suggestions applied

Add username index and unique slug index migrations

.rubocop.yml

@@ -1,3 +1,5 @@
+require: rubocop-rails
+
 AllCops:
   NewCops: enable
 Style/Documentation:

Gemfile

@@ -50,6 +50,8 @@ group :development do
   gem 'spring-watcher-listen', '~> 2.0.0'
   # A Ruby static code analyzer and formatter, based on the community Ruby style guide.
   gem 'rubocop', '~> 1.17'
+  # A RuboCop extension focused on enforcing Rails best practices and coding conventions.
+  gem 'rubocop-rails'
 end
 
 group :test do

Gemfile.lock

@@ -166,6 +166,10 @@ GEM
       unicode-display_width (>= 1.4.0, < 3.0)
     rubocop-ast (1.7.0)
       parser (>= 3.0.1.1)
+    rubocop-rails (2.10.1)
+      activesupport (>= 4.2.0)
+      rack (>= 1.1)
+      rubocop (>= 1.7.0, < 2.0)
     ruby-progressbar (1.11.0)
     ruby_dep (1.5.0)
     rubyzip (2.3.0)
@@ -248,6 +252,7 @@ DEPENDENCIES
   rails (~> 5.2.6)
   redis (~> 4.0)
   rubocop (~> 1.17)
+  rubocop-rails
   selenium-webdriver
   simplecov
   simplecov-console

README.md

@@ -27,24 +27,21 @@ docker-compose run --rm app bundle exec rubocop
 
 - Testing
 ```bash
-Finished in 2.326313s, 10.3168 runs/s, 10.7466 assertions/s.
-24 runs, 25 assertions, 0 failures, 0 errors, 0 skips
-Coverage report generated for Minitest to /usr/src/app/coverage. 79 / 79 LOC (100.0%) covered.
+Finished in 2.211344s, 9.9487 runs/s, 10.4009 assertions/s.
+22 runs, 23 assertions, 0 failures, 0 errors, 0 skips
+Coverage report generated for Minitest to /usr/src/app/coverage. 81 / 81 LOC (100.0%) covered.
 
-COVERAGE: 100.00% -- 79/79 lines in 11 files
-BRANCH COVERAGE: 100.00% -- 22/22 branches in 11 branches
+COVERAGE: 100.00% -- 81/81 lines in 8 files
+BRANCH COVERAGE: 100.00% -- 20/20 branches in 8 branches
 
 +----------+-------------------------------------------+-------+--------+---------+-----------------+----------+-----------------+------------------+
 | coverage | file                                      | lines | missed | missing | branch coverage | branches | branches missed | branches missing |
 +----------+-------------------------------------------+-------+--------+---------+-----------------+----------+-----------------+------------------+
-| 100.00%  | app/controllers/application_controller.rb | 2     | 0      |         | 100.00%         | 0        | 0               |                  |
-| 100.00%  | app/controllers/links_controller.rb       | 22    | 0      |         | 100.00%         | 8        | 0               |                  |
-| 100.00%  | app/controllers/sessions_controller.rb    | 14    | 0      |         | 100.00%         | 4        | 0               |                  |
-| 100.00%  | app/controllers/users_controller.rb       | 12    | 0      |         | 100.00%         | 4        | 0               |                  |
-| 100.00%  | app/helpers/application_helper.rb         | 1     | 0      |         | 100.00%         | 0        | 0               |                  |
-| 100.00%  | app/helpers/links_helper.rb               | 1     | 0      |         | 100.00%         | 0        | 0               |                  |
-| 100.00%  | app/helpers/sessions_helper.rb            | 7     | 0      |         | 100.00%         | 2        | 0               |                  |
-| 100.00%  | app/helpers/users_helper.rb               | 1     | 0      |         | 100.00%         | 0        | 0               |                  |
+| 100.00%  | app/controllers/application_controller.rb | 3     | 0      |         | 100.00%         | 0        | 0               |                  |
+| 100.00%  | app/controllers/links_controller.rb       | 23    | 0      |         | 100.00%         | 8        | 0               |                  |
+| 100.00%  | app/controllers/sessions_controller.rb    | 17    | 0      |         | 100.00%         | 4        | 0               |                  |
+| 100.00%  | app/controllers/users_controller.rb       | 16    | 0      |         | 100.00%         | 4        | 0               |                  |
+| 100.00%  | app/helpers/sessions_helper.rb            | 3     | 0      |         | 100.00%         | 0        | 0               |                  |
 | 100.00%  | app/models/application_record.rb          | 2     | 0      |         | 100.00%         | 0        | 0               |                  |
 | 100.00%  | app/models/link.rb                        | 13    | 0      |         | 100.00%         | 4        | 0               |                  |
 | 100.00%  | app/models/user.rb                        | 4     | 0      |         | 100.00%         | 0        | 0               |                  |
@@ -53,10 +50,10 @@ BRANCH COVERAGE: 100.00% -- 22/22 branches in 11 branches
 
 - Rubocop
 ```bash
-Inspecting 58 files
-..........................................................
+Inspecting 43 files
+...........................................
 
-58 files inspected, no offenses detected
+43 files inspected, no offenses detected
 ```
 
 - Production link

app/controllers/application_controller.rb

@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
 class ApplicationController < ActionController::Base
-  include SessionsHelper
+  def authenticate
+    @current_user = User.find_by(id: session[:user_id])
+  end
 end

app/controllers/links_controller.rb

@@ -1,22 +1,19 @@
 # frozen_string_literal: true
 
 class LinksController < ApplicationController
-  before_action :authenticate, only: [:create]
+  before_action :authenticate, only: %i[create]
+  before_action :set_link, only: %i[redirect counter]
 
   def redirect
-    @link = Link.find_by_slug(params[:slug])
-
     if @link
       @link.update(click_counter: @link.click_counter + 1)
       redirect_to @link.url
     else
-      render file: "#{Rails.root}/public/404", status: :not_found
+      render file: Rails.root.join('/public/404'), status: :not_found
     end
   end
 
   def counter
-    @link = Link.find_by_slug(params[:slug])
-
     if @link
       render json: @link.click_counter
     else
@@ -38,6 +35,10 @@ class LinksController < ApplicationController
 
   private
 
+  def set_link
+    @link = Link.find_by(slug: params[:slug])
+  end
+
   def link_params
     params.require(:link).permit(:url)
   end

app/controllers/sessions_controller.rb

@@ -2,11 +2,12 @@
 
 class SessionsController < ApplicationController
   before_action :authenticate, except: %i[create]
+  before_action :set_user, only: %i[create]
 
   def create
-    @user = User.find_by(username: session_params[:username])
     if @user&.authenticate(session_params[:password])
       session[:user_id] = @user.id
+      session[:username] = @user.username
       render json: nil, status: :ok
     else
       render json: { username: ['Credentials not valid, try again or create an account'] }, status: :unauthorized
@@ -20,6 +21,10 @@ class SessionsController < ApplicationController
 
   private
 
+  def set_user
+    @user = User.find_by(username: session_params[:username])
+  end
+
   def session_params
     params.permit(:username, :password)
   end

app/controllers/users_controller.rb

@@ -1,22 +1,29 @@
 # frozen_string_literal: true
 
 class UsersController < ApplicationController
-  def create
-    if user_params[:password] != params[:user][:confirm_password]
-      return render json: { password: ['Password not match with Confirm Password'] }, status: :bad_request
-    end
+  around_action :confirm_password_validation, only: %i[create]
 
+  def create
     @user = User.create(user_params)
     if @user.errors.any?
       render json: @user.errors, status: :unprocessable_entity
     else
       session[:user_id] = @user.id
+      session[:username] = @user.username
       render json: nil, status: :ok
     end
   end
 
   private
 
+  def confirm_password_validation
+    if user_params[:password] == params[:user][:confirm_password]
+      yield
+    else
+      render json: { password: ['Password not match with Confirm Password'] }, status: :bad_request
+    end
+  end
+
   def user_params
     params.require(:user).permit(:username, :password)
   end

app/helpers/application_helper.rb

@@ -1,4 +0,0 @@
-# frozen_string_literal: true
-
-module ApplicationHelper
-end

app/helpers/links_helper.rb

@@ -1,4 +0,0 @@
-# frozen_string_literal: true
-
-module LinksHelper
-end

app/helpers/sessions_helper.rb

@@ -1,15 +1,7 @@
 # frozen_string_literal: true
 
 module SessionsHelper
-  def authenticate
-    @current_user = User.find_by(id: session[:user_id])
-  end
-
-  def logged_in?
-    !@current_user.nil?
-  end
-
   def current_user_username
-    @current_user&.username
+    session[:username]
   end
 end

app/helpers/users_helper.rb

@@ -1,4 +0,0 @@
-# frozen_string_literal: true
-
-module UsersHelper
-end

app/models/link.rb

@@ -1,22 +1,22 @@
 # frozen_string_literal: true
 
 class Link < ApplicationRecord
-  validates_presence_of :url
-  validates_uniqueness_of :slug
+  validates :url, presence: true
+  validates :slug, uniqueness: true
 
   validates :url,
             format: { with: /\A#{URI::DEFAULT_PARSER.make_regexp(%w[http https])}\z/, message: 'invalid format' }
-  validates_length_of :url, within: 3..30_000, on: :create, message: 'max length is 30000'
+  validates :url, length: { within: 3..30_000, on: :create, message: 'max length is 30000' }
 
   before_validation :generate_slug
 
   def generate_slug(attempts = 0)
-    return if !slug.blank? || attempts == 3
+    return if slug.present? || attempts == 3
 
     # Number of combinations 62P6
     generated_slug = SecureRandom.alphanumeric(6)
 
-    if Link.where(slug: generated_slug).exists?
+    if Link.exists?(slug: generated_slug)
       generate_slug(attempts + 1)
     else
       self.slug = generated_slug

app/models/user.rb

@@ -1,8 +1,8 @@
 # frozen_string_literal: true
 
 class User < ApplicationRecord
-  validates_uniqueness_of :username
+  validates :username, uniqueness: true
   has_secure_password
 
-  has_many :links
+  has_many :links, dependent: :nullify
 end

app/views/links/_index.html.erb

@@ -1,4 +1,4 @@
-<% if logged_in? %>
+<% unless @current_user.nil? %>
 	<h2 class="text-2xl leading-6 font-medium text-gray-900 mx-4 my-6">My links</h2>
 	<div data-links-target="userLinks">
 		<% @current_user.links.order(created_at: :desc).each do |link| %>

app/views/sessions/index.html.erb

@@ -1,6 +1,6 @@
 <div data-controller="users">
   <div class="bg-gray-50 px-4 py-3 sm:px-6 sm:flex sm:flex-row-reverse">
-    <% unless logged_in? %>
+    <% if @current_user.nil? %>
       <button data-action="click->users#openLoginModal" class="mt-3 w-full inline-flex justify-center rounded-md border border-gray-300 shadow-sm px-4 py-2 bg-white text-base font-medium text-gray-700 hover:bg-gray-50 focus:outline-none focus:ring-2 focus:ring-offset-2 sm:mt-0 sm:ml-3 sm:w-auto sm:text-sm">
         Login
       </button>

app/views/users/_show.html.erb

@@ -1,4 +1,4 @@
-<% if logged_in? %>
+<% unless @current_user.nil? %>
 	<%= link_to "Logout", '/session/logout', data: { action: 'ajax:before->users#confirmLogout ajax:success->users#onSuccessLogout' }, remote: true, class: "mt-3 w-full inline-flex justify-center rounded-md border border-gray-300 shadow-sm px-4 py-2 bg-white text-base font-medium text-gray-700 hover:bg-gray-50 focus:outline-none focus:ring-2 focus:ring-offset-2 sm:mt-0 sm:ml-3 sm:w-auto sm:text-sm" %>
 	<h1 class="px-2 py-2 text-sm font-medium">You are Logged In, <%= @current_user.username %></h1>
 <%end%>

config/environments/development.rb

@@ -16,7 +16,7 @@ Rails.application.configure do
 
   # Enable/disable caching. By default caching is disabled.
   # Run rails dev:cache to toggle caching.
-  if Rails.root.join('tmp', 'caching-dev.txt').exist?
+  if Rails.root.join('tmp/caching-dev.txt').exist?
     config.action_controller.perform_caching = true
 
     config.cache_store = :memory_store

db/migrate/20210615193108_add_username_index_to_users.rb

@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+class AddUsernameIndexToUsers < ActiveRecord::Migration[5.2]
+  def change
+    add_index :users, :username, unique: true
+  end
+end

db/migrate/20210615200050_unique_slug_index.rb

@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+class UniqueSlugIndex < ActiveRecord::Migration[5.2]
+  def change
+    remove_index :links, column: :slug
+    add_index :links, :slug, unique: true
+  end
+end

db/schema.rb

@@ -1,5 +1,3 @@
-# frozen_string_literal: true
-
 # This file is auto-generated from the current state of the database. Instead
 # of editing this file, please use the migrations feature of Active Record to
 # incrementally modify your database, and then regenerate this schema definition.
@@ -12,27 +10,29 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20_210_614_114_837) do
+ActiveRecord::Schema.define(version: 2021_06_15_200050) do
+
   # These are extensions that must be enabled in order to support this database
-  enable_extension 'plpgsql'
+  enable_extension "plpgsql"
 
-  create_table 'links', force: :cascade do |t|
-    t.string 'slug', null: false
-    t.text 'url'
-    t.integer 'click_counter', default: 0
-    t.datetime 'created_at', null: false
-    t.datetime 'updated_at', null: false
-    t.bigint 'user_id'
-    t.index ['slug'], name: 'index_links_on_slug'
-    t.index ['user_id'], name: 'index_links_on_user_id'
+  create_table "links", force: :cascade do |t|
+    t.string "slug", null: false
+    t.text "url"
+    t.integer "click_counter", default: 0
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
+    t.bigint "user_id"
+    t.index ["slug"], name: "index_links_on_slug", unique: true
+    t.index ["user_id"], name: "index_links_on_user_id"
   end
 
-  create_table 'users', force: :cascade do |t|
-    t.string 'username'
-    t.string 'password_digest'
-    t.datetime 'created_at', null: false
-    t.datetime 'updated_at', null: false
+  create_table "users", force: :cascade do |t|
+    t.string "username"
+    t.string "password_digest"
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
+    t.index ["username"], name: "index_users_on_username", unique: true
   end
 
-  add_foreign_key 'links', 'users'
+  add_foreign_key "links", "users"
 end

test/helpers/sessions_helper_test.rb

@@ -1,14 +0,0 @@
-# frozen_string_literal: true
-
-require 'test_helper'
-
-class SessionsHelperTest < ActionView::TestCase
-  test 'Should return current_user username' do
-    @current_user = users(:one)
-    assert_equal @current_user.username, current_user_username
-  end
-
-  test 'Should return nil if current_user is nil' do
-    assert_nil current_user_username
-  end
-end