diff --git a/.rubocop.yml b/.rubocop.yml index 390ec45..5c14707 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,3 +1,5 @@ +require: rubocop-rails + AllCops: NewCops: enable Style/Documentation: diff --git a/Gemfile b/Gemfile index fa6d43c..c5080e4 100644 --- a/Gemfile +++ b/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 diff --git a/Gemfile.lock b/Gemfile.lock index ffc4120..b3a100b 100644 --- a/Gemfile.lock +++ b/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 diff --git a/README.md b/README.md index b6d3b18..1ed84ab 100644 --- a/README.md +++ b/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 diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index fe0a97f..560a185 100644 --- a/app/controllers/application_controller.rb +++ b/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 diff --git a/app/controllers/links_controller.rb b/app/controllers/links_controller.rb index eaaefa6..b1440d8 100644 --- a/app/controllers/links_controller.rb +++ b/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 diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 3a9720d..d0b1a49 100644 --- a/app/controllers/sessions_controller.rb +++ b/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 diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 731e550..b5038c5 100644 --- a/app/controllers/users_controller.rb +++ b/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 diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb deleted file mode 100644 index 15b06f0..0000000 --- a/app/helpers/application_helper.rb +++ /dev/null @@ -1,4 +0,0 @@ -# frozen_string_literal: true - -module ApplicationHelper -end diff --git a/app/helpers/links_helper.rb b/app/helpers/links_helper.rb deleted file mode 100644 index 038f107..0000000 --- a/app/helpers/links_helper.rb +++ /dev/null @@ -1,4 +0,0 @@ -# frozen_string_literal: true - -module LinksHelper -end diff --git a/app/helpers/sessions_helper.rb b/app/helpers/sessions_helper.rb index e019440..2bf3d03 100644 --- a/app/helpers/sessions_helper.rb +++ b/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 diff --git a/app/helpers/users_helper.rb b/app/helpers/users_helper.rb deleted file mode 100644 index 4dc909e..0000000 --- a/app/helpers/users_helper.rb +++ /dev/null @@ -1,4 +0,0 @@ -# frozen_string_literal: true - -module UsersHelper -end diff --git a/app/models/link.rb b/app/models/link.rb index d1ab8df..98df1d7 100644 --- a/app/models/link.rb +++ b/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 diff --git a/app/models/user.rb b/app/models/user.rb index 881a3df..084f405 100644 --- a/app/models/user.rb +++ b/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 diff --git a/app/views/links/_index.html.erb b/app/views/links/_index.html.erb index 518802f..3e61cfc 100644 --- a/app/views/links/_index.html.erb +++ b/app/views/links/_index.html.erb @@ -1,4 +1,4 @@ -<% if logged_in? %> +<% unless @current_user.nil? %>

My links

<% @current_user.links.order(created_at: :desc).each do |link| %> diff --git a/app/views/sessions/index.html.erb b/app/views/sessions/index.html.erb index 813ab34..6cc4a7e 100644 --- a/app/views/sessions/index.html.erb +++ b/app/views/sessions/index.html.erb @@ -1,6 +1,6 @@
- <% unless logged_in? %> + <% if @current_user.nil? %> diff --git a/app/views/users/_show.html.erb b/app/views/users/_show.html.erb index 36b138e..a93312d 100644 --- a/app/views/users/_show.html.erb +++ b/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" %>

You are Logged In, <%= @current_user.username %>

<%end%> \ No newline at end of file diff --git a/config/environments/development.rb b/config/environments/development.rb index 93bea55..e98bb6d 100644 --- a/config/environments/development.rb +++ b/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 diff --git a/db/migrate/20210615193108_add_username_index_to_users.rb b/db/migrate/20210615193108_add_username_index_to_users.rb new file mode 100644 index 0000000..7075d14 --- /dev/null +++ b/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 diff --git a/db/migrate/20210615200050_unique_slug_index.rb b/db/migrate/20210615200050_unique_slug_index.rb new file mode 100644 index 0000000..5f0de4a --- /dev/null +++ b/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 diff --git a/db/schema.rb b/db/schema.rb index 43b7b21..df38dfe 100644 --- a/db/schema.rb +++ b/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 diff --git a/test/helpers/sessions_helper_test.rb b/test/helpers/sessions_helper_test.rb deleted file mode 100644 index 6a20fdc..0000000 --- a/test/helpers/sessions_helper_test.rb +++ /dev/null @@ -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