Fix code mentor's comment

df3204f9 · Huỳnh Thiên Phước · d3ddaa64 · df3204f9 · d3ddaa64 · df3204f9
Commit df3204f9 authored Aug 11, 2020 by Huỳnh Thiên Phước
10 changed files
--- a/app/controllers/jobs_controller.rb
+++ b/app/controllers/jobs_controller.rb
 class JobsController < ApplicationController
+  before_action :set_job, only: [:show]
  before_action :use_variables
  def index
-    @jobs_list = Job.all_job.page(params[:page]).per(20)
+    @jobs_list = Job.all.page(params[:page]).per(Job::LIMIT_PAGE)
  end
  def city_jobs
    @city = City.find_by(converted_name: params[:converted_name])
-    @jobs_list = @city.jobs.all_job.page(params[:page]).per(20)
+    @jobs_list = @city.jobs.page(params[:page]).per(Job::LIMIT_PAGE)
    @result_for_job = @city.jobs.count
  end
  def industry_jobs
    @industry = Industry.find_by(converted_name: params[:converted_name])
-    @jobs_list = @industry.jobs.all_job.page(params[:page]).per(20)
+    @jobs_list = @industry.jobs.page(params[:page]).per(Job::LIMIT_PAGE)
    @result_for_job = @industry.jobs.count
  end
  def company_jobs
    @company = Company.find_by(converted_name: params[:converted_name])
-    @jobs_list = @company.jobs.all_job.page(params[:page]).per(20)
+    @jobs_list = @company.jobs.page(params[:page]).per(Job::LIMIT_PAGE)
    @result_for_job = @company.jobs.count
+    redirect_to jobs_path unless @company
  end
-  def detail
+  def show
-    @job_details = Job.find(params[:id])
+    redirect_to jobs_path unless @job
+  end
+  private
+  def set_job
+    @job ||= Job.find_by(params[:id])
  end
  def use_variables
@@ -33,3 +42,4 @@ class JobsController < ApplicationController
    @total_job = Job.count
  end
 end
--- a/app/helpers/convert.rb
+++ b/app/helpers/convert.rb
-module ConvertsHelper
-  def to_convert(str)
-    str.mb_chars.normalize(:kd).gsub(/[Đđ]/, 'd').gsub(/[^\x00-\x7F]/,'').gsub(/[\W+]/,' ').downcase.to_s.split(' ').join('-')
-  end
-end
--- a/app/models/application_record.rb
+++ b/app/models/application_record.rb
 class ApplicationRecord < ActiveRecord::Base
  self.abstract_class = true
+  def convert_attribute
+    self.converted_name = normalize_attribute.mb_chars.normalize(:kd).gsub(/[Đđ]/, 'd').gsub(/[^\x00-\x7F]/,'').gsub(/[\W+]/,' ').downcase.to_s.split(' ').join('-')
+  end
 end
--- a/app/models/city.rb
+++ b/app/models/city.rb
 class City < ApplicationRecord
-  before_save :convert_city
+  before_save :convert_attribute
  has_many :city_jobs
  has_many :jobs, through: :city_jobs
@@ -9,7 +9,9 @@ class City < ApplicationRecord
  scope :top_city, -> { joins(:jobs).group(:city_id).order('count(job_id) DESC').limit(9) }
  scope :location, ->(number) { joins(:jobs).group(:city_id).order('count(job_id) DESC').where(location: number) }
-  def convert_city
+  private
-    self.converted_name = Convert.to_convert("#{name} #{rand(10000)}")
+  def normalize_attribute
+    name
  end
 end
--- a/app/models/company.rb
+++ b/app/models/company.rb
 class Company < ApplicationRecord
-  before_save :convert_company
+  before_save :convert_attribute
  has_many :jobs
-  def convert_company
+  private
-    self.converted_name = Convert.to_convert("#{name} #{rand(10000)}")
+  def normalize_attribute
+    name
  end
 end
--- a/app/models/industry.rb
+++ b/app/models/industry.rb
 class Industry < ApplicationRecord
-  before_save :convert_industry
+  before_save :convert_attribute
  has_many :industry_jobs
  has_many :jobs, through: :industry_jobs
  scope :top_industry, -> { joins(:jobs).group(:industry_id).order('count(job_id) DESC').limit(9) }
  scope :all_industry, -> { joins(:jobs).group(:industry_id).order('count(job_id) DESC') }
-  def convert_industry
+  private
-    self.converted_name = Convert.to_convert("#{name} #{rand(10000)}")
+  def normalize_attribute
+    name
  end
 end
--- a/app/models/job.rb
+++ b/app/models/job.rb
 class Job < ApplicationRecord
-  before_save :convert_job
+  before_save :convert_attribute
  belongs_to :company
  has_many :city_jobs
  has_many :cities, through: :city_jobs
@@ -16,15 +16,18 @@ class Job < ApplicationRecord
  has_many :histories
  has_many :users, through: :histories
-  scope :limit_job, -> { includes(:cities, :company).order(created_at: :desc).limit(5) }
+  LIMIT_PAGE = 20
-  scope :all_job, -> { limit(20).order(created_at: :desc) }
-  def convert_job
+  scope :limit_job, -> { includes(:cities, :company).order(created_at: :desc).limit(5) }
-    converted_name = Convert.to_convert("#{title} #{rand(10000)}")
-  end
  def company_name
    company&.name
  end
+  private
+  def normalize_attribute
+    title
+  end
 end
--- a/app/views/jobs/index.html.erb
+++ b/app/views/jobs/index.html.erb
@@ -6,9 +6,6 @@
  <div class="row total-jobs">
    <div class="col-4"><strong>Total: <%= @total_job %> jobs</strong></div>
  </div>
-  <div class="paginate-jobs">
-    <%= paginate @jobs_list, left: 3, right: 3 %>
-  </div>
  <%= render "jobs/pagination" %>
  <div class="job-list">
    <%= render partial: "job", collection: @jobs_list, as: :job %>

--- a/app/views/jobs/detail.html.erb
+++ b/app/views/jobs/detail.html.erb
@@ -2,21 +2,21 @@
  <div class="details-banner">
    <div class="job-details-banner">
      <div class="job-info">
-        <div><strong><%= @job_details.title %></strong></div>
+        <div><strong><%= @job.title %></strong></div>
        <div>
-          <%= link_to @job_details.company_name, company_jobs_path(converted_name: @job_details.company.converted_name), class: 'company' %>
+          <%= link_to @job.company_name, company_jobs_path(converted_name: @job.company.converted_name), class: 'company' %>
        </div>
        <div class="breadcrumb">
          <%= link_to "TOP", root_path  %>&ensp;/&ensp;
          <div>
-            <% @job_details.cities.each do |city| %>
+            <% @job.cities.each do |city| %>
              <%= link_to city.name, city_jobs_path(converted_name: city.converted_name) , class: 'location' %>
-            <% end %>&ensp;/&ensp;
+            <% end %>/&ensp;
          </div>
-          <% @job_details.industries.each do |industry| %>
+          <% @job.industries.each do |industry| %>
            <%= link_to industry.name, industry_jobs_path(converted_name: industry.converted_name), class: 'industry' %>
-          <% end %>&ensp;/&ensp;
+          <% end %>/&ensp;
-          <%= @job_details.title.truncate_words(5) %>
+          <%= @job.title.truncate_words(5) %>
        </div>
        <%= link_to '#' do %>
          <div class="apply-job">
@@ -28,42 +28,42 @@
  </div>
  <div class="details">
    <div class="job-infomation">
-      <h1><%= @job_details.title %></h1>
+      <h1><%= @job.title %></h1>
      <div class="row info border border-dark">
        <div class="col-6">
          <div class="city">
            <strong>Location:</strong>
-            <% @job_details.cities.each do |city| %>
+            <% @job.cities.each do |city| %>
              <%= link_to city.name, city_jobs_path(converted_name: city.converted_name), class: 'location' %>
            <% end %>
          </div>
          <div class="created-day">
-            <strong>Created at</strong>: <%= @job_details.created_at.strftime('%d/%m/%Y') %>
+            <strong>Created at</strong>: <%= @job.created_at.strftime('%d/%m/%Y') %>
          </div>
          <div class="industry">
-            <strong>Industry:</strong> <%= @job_details.industries.map(&:name).join(', ') %>
+            <strong>Industry:</strong> <%= @job.industries.map(&:name).join(', ') %>
          </div>
        </div>
        <div class="col-6">
          <div class="salary-detail">
-            <strong>Salary:</strong> <%= @job_details.salary %>
+            <strong>Salary:</strong> <%= @job.salary %>
          </div>
-          <% if @job_details.experience.present? %>
+          <% if @job.experience.present? %>
            <div class="experience">
-               <strong>Experience:</strong> <%= @job_details.experience %>
+               <strong>Experience:</strong> <%= @job.experience %>
            </div>
          <% end %>
          <div class="level">
-            <strong>Level:</strong> <%= @job_details.level %>
+            <strong>Level:</strong> <%= @job.level %>
          </div>
          <div class="expiration-date">
-            <strong>Expiration date:</strong> <%= @job_details.expiration_date %>
+            <strong>Expiration date:</strong> <%= @job.expiration_date %>
          </div>
        </div>
      </div>
    </div>
    <div class="job-description">
-      <%= @job_details.description.html_safe %>
+      <%= @job.description.html_safe %>
    </div>
  </div>
  <div class="row under-descrip">

--- a/config/routes.rb
+++ b/config/routes.rb
 Rails.application.routes.draw do
  # For details on the DSL available within this file, see http://guides.rubyonrails.org/routing.html
  resources :jobs
+  get 'detail/:id', action: :show, controller: 'jobs' , as: :job_detail
  get 'jobs/city/:converted_name', to: 'jobs#city_jobs', as: :city_jobs
  get 'jobs/industry/:converted_name', to: 'jobs#industry_jobs', as: :industry_jobs
  get 'jobs/company/:converted_name', to: 'jobs#company_jobs', as: :company_jobs
-  get 'detail/:id', to: 'jobs#detail', as: :job_detail
  resources :top_pages
  resources :industries