Re-factoring Course 49030

Gemfile.lock

@@ -222,6 +222,3 @@ DEPENDENCIES
   whenever (~> 0.9.4)
   will_paginate (~> 3.0.6)
   will_paginate-bootstrap
-
-BUNDLED WITH
-   1.10.6

app/controllers/courses_controller.rb

@@ -2,11 +2,7 @@ class CoursesController < ApplicationController
   load_and_authorize_resource except: [:show]
 
   def show
-    courses = current_user.courses
-    @course = courses.last
-    @subjects = @course.subjects
-    @user_subjects = current_user.courses_subjects
-    @activities = PublicActivity::Activity.order("created_at desc")
-    .where owner_id: current_user.id, trackable_id: @course.id
+    @course = current_user.courses.last
+    @activities = @course.load_acitvities current_user
   end
 end

app/controllers/supervisor/assign_supervisors_controller.rb

@@ -28,7 +28,7 @@ def load_course
   end
 
   def load_users
-    @users = User.supervisors
+    @users = User.supervisor
   end
 
   def course_params

app/controllers/supervisor/courses_controller.rb

@@ -24,23 +24,20 @@ def create
   end
 
   def show
-    @activities = PublicActivity::Activity.order("created_at desc")
-      .where trackable_type: "Course", trackable_id: @course.id
+    @activities = @course.load_activities
   end
 
   def edit
   end
 
   def update
     if course_params.has_key? :update_status
-      update_status!
-      redirect_to supervisor_course_path @course
+      @course.update_status!
+      redirect_to [:supervisor, @course]
+    elsif @course.update_attributes course_params
+      redirect_to [:supervisor, @course]
     else
-      if @course.update_attributes course_params
-        redirect_to supervisor_course_path @course
-      else
-        render :edit
-      end
+      render :edit
     end
   end
 
@@ -58,22 +55,12 @@ def load_course
   def course_params
     params.require(:course).permit :name, :description, :start_date, :end_date,
       :update_status, subject_ids: []
-  end
-
-  def update_status!
-    if @course.new?
-      @course.status = Course::STATUS[:started]
-    elsif @course.started?
-      @course.status = Course::STATUS[:finished]
-    end
-    @course.save!
-  end
+  end  
 
   def check_course
-    if (@course.started? && !course_params.has_key?(:update_status)) || 
-        @course.finished?
+    if valid_for_status_update? course_params.has_key?(:update_status)
       flash[:danger] = t ".danger"
       redirect_to supervisor_courses_path
     end
   end
-end
+end

app/controllers/supervisor/courses_subjects_controller.rb

@@ -39,7 +39,7 @@ def courses_subject_params
   def update_status!
     if @courses_subject.new?
       @courses_subject.status = CoursesSubject::STATUS[:started]
-      @courses_subject.users = @courses_subject.course.trainees      
+      @courses_subject.users = @courses_subject.course.trainees
     elsif @courses_subject.started?
       @courses_subject.status = CoursesSubject::STATUS[:finished]
     end

app/controllers/supervisor/users_controller.rb

@@ -2,10 +2,10 @@ class Supervisor::UsersController < ApplicationController
   def index
     @users = User.all.paginate page: params[:page], per_page: 15
     if params[:supervisor] == User::TYPES[:supervisor]
-      @search = User.supervisors.search params[:q]
+      @search = User.supervisor.search params[:q]
       @users = @search.result.paginate page: params[:page], per_page: 15
     elsif params[:supervisor] == User::TYPES[:trainee]
-      @search = User.trainees.search params[:q]
+      @search = User.trainee.search params[:q]
       @users = @search.result.paginate page: params[:page], per_page: 15
     end
   end

app/controllers/users_controller.rb

@@ -3,7 +3,7 @@ class UsersController < ApplicationController
   before_action :load_user, only: [:show, :update]
 
   def index
-    @search = User.trainees.search params[:q]
+    @search = User.trainee.search params[:q]
     @users = @search.result.paginate page: params[:page], per_page: 15
   end
   def show

app/models/course.rb

@@ -3,11 +3,7 @@ class Course < ActiveRecord::Base
   tracked owner: ->(controller, model) {controller && controller.current_user}
   acts_as_paranoid
 
-  STATUS = {
-    new: 0,
-    started: 1,
-    finished: 2
-  }
+  enum status: [:created, :started, :finished]
   attr_accessor :update_status, :assigned_users, :removed_users
 
   has_many :users_courses, dependent: :destroy
@@ -24,32 +20,38 @@ class Course < ActiveRecord::Base
   after_initialize :default_values
   scope :near_deadline, ->{where "end_date < ?", 5.days.since}
 
-  def trainees
-    User.joins(:courses).
-      where("users.supervisor = ? AND courses.id = ?", false, id)
+  User.roles.each_key do |key|
+    define_method "#{key}s" do
+      users.send key
+    end
   end
 
-  def supervisors
-    User.joins(:courses).
-      where("users.supervisor = ? AND courses.id = ?", true, id)
-  end
-
-  def new?
-    status == STATUS[:new]
+  def progress
+    return 0 if users_subjects.size.zero?
+    users_subjects.finished.size * 100 / users_subjects.size
   end
 
-  def started?
-    status == STATUS[:started]
+  def load_activities user = nil
+    if user
+      PublicActivity::Activity.order("created_at desc").
+        where owner_id: user.id, trackable_id: id
+    else
+      PublicActivity::Activity.order("created_at desc").
+        where trackable_type: "Course", trackable_id: id
+    end    
   end
 
-  def finished?
-    status == STATUS[:finished]
+  def update_status!
+    if created?
+      started!
+    elsif started?
+      finished!
+    end
+    save!
   end
 
-  def course_progress
-    return 0 if users_subjects.size.zero?
-    finished_user_subjects = users_subjects.where status: 1
-    finished_user_subjects.size * 100 / users_subjects.size
+  def valid_for_status_update? has_status
+    (started? && !has_status) || finished?
   end
 
   def start_must_be_before_end_date
@@ -58,7 +60,7 @@ def start_must_be_before_end_date
   end
 
   def default_values
-    assigned_users ||= []
-    removed_users ||= []
+    self.assigned_users ||= []
+    self.removed_users ||= []
   end
-end
+end

app/models/user.rb

@@ -3,10 +3,7 @@ class User < ActiveRecord::Base
   # :confirmable, :lockable, :timeoutable and :omniauthable
   acts_as_paranoid
 
-  TYPES = {
-    trainee: "0",
-    supervisor: "1"
-  }
+  enum role: [:trainee, :supervisor]
   devise :database_authenticatable, :registerable,
     :recoverable, :rememberable, :trackable, :validatable
 
@@ -19,14 +16,12 @@ class User < ActiveRecord::Base
   has_many :users_tasks, dependent: :destroy
   has_many :tasks, through: :users_tasks
 
-  scope :supervisors, -> {where supervisor: true}
-  scope :trainees, -> {where supervisor: false}
   scope :free,  -> {
-    joins(:courses).where.not courses: {status: Course::STATUS[:finished]}
+    joins(:courses).where.not courses: {status: Course.statuses[:finished]}
   }
   scope :assignable_trainees, ->(course) {
-    excluded_trainees_ids = free.trainees.ids - course.users.trainees.ids
-    where.not(id: excluded_trainees_ids).trainees
+    excluded_ids = free.ids - course.users.ids
+    where.not(id: excluded_ids).trainees
   }
 
   def full_name

app/models/users_subject.rb

@@ -1,33 +1,17 @@
 class UsersSubject < ActiveRecord::Base
   acts_as_paranoid
-  STATUS = {
-    new: 0,
-    started: 1,
-    finished: 2
-  }
+
+  enum status: [:created, :started, :finished]
   include PublicActivity::Model
   tracked owner: ->(controller, model) {controller && controller.current_user}
 
   belongs_to :user
   belongs_to :courses_subject
   belongs_to :subject
-  scope :finished, ->{where status: STATUS[:finished]}
 
   class << self
     def find_by_user user_id
       UsersSubject.find_by user_id: user_id, finished: true
     end
   end
-
-  def new?
-    status == STATUS[:new]
-  end
-
-  def started?
-    status == STATUS[:started]
-  end
-
-  def finished?
-    status == STATUS[:finished]
-  end
 end

app/views/courses/show.html.erb

@@ -2,10 +2,10 @@
 <br>
 <h4><%= t ".course_name" %> <%= @course.name.upcase %></h4>
 <br>
-<h4><%= @course.users.supervisors.count %><%= t ".no_of_supervisors" %></h4>
+<h4><%= @course.supervisors.count %><%= t ".no_of_supervisors" %></h4>
 <div class="progress">
-  <div class="progress-bar" role="progressbar" aria-valuenow="<%= @course.course_progress %>"
-    aria-valuemin="0"  aria-valuemax="100" style="width: <%= @course.course_progress %>%;">
+  <div class="progress-bar" role="progressbar" aria-valuenow="<%= @course.progress %>"
+    aria-valuemin="0"  aria-valuemax="100" style="width: <%= @course.progress %>%;">
   </div>
 </div>
 <table class="table table-striped">

app/views/supervisor/courses/_course.html.erb

@@ -3,13 +3,12 @@
   <td><%= course.description.truncate 100 %></td>
   <td><%= (I18n.localize course.start_date, format: :short) if course.start_date %></td>
   <td><%= (I18n.localize course.end_date, format: :short) if course.end_date %></td>
-  <td><%= course.users.supervisors.count %></td>
-  <td><%= course.users.trainees.count %></td>
+  <td><%= course.supervisors.count %></td>
+  <td><%= course.trainees.count %></td>
   <% if can? :manage, @course %>
-    <% if @course.new? %>
-      <td><%= link_to t(".edit"), edit_supervisor_course_path(course) %></td>
-    <% end %>
-    <% if @course.finished? %>
+    <% if @course.created? %>
+      <td><%= link_to t(".edit"), edit_supervisor_course_path(course) %></td>    
+    <% elsif @course.finished? %>
       <td><%= link_to t(".delete"), [:supervisor, @course], method: :delete,
         class: "btn btn-danger", data: { confirm: t(:confirm_message) } %></td>
     <% end %>

app/views/supervisor/courses/show.html.erb

@@ -2,7 +2,7 @@
 <%= form_for [:supervisor, @course] do |f| %>
   <%= f.hidden_field :update_status %>
   <% if can? :manage, @course %>
-    <% if @course.new? %>
+    <% if @course.created? %>
       <%= f.submit t(".start"), class: "btn btn-primary" %>
     <% elsif @course.started? %>
       <%= f.submit t(".finish"), class: "btn btn-primary" %>

app/workers/daily_report_worker.rb

@@ -2,7 +2,7 @@ class DailyReportWorker
   include Sidekiq::Worker
 
   def perform
-    User.supervisors.each do |supervisor|
+    User.supervisor.each do |supervisor|
       UserMailer.daily_report(supervisor.id).deliver_now
     end    
   end

db/migrate/20151201040850_add_supervisor_to_users.rb

@@ -1,5 +1,5 @@
 class AddSupervisorToUsers < ActiveRecord::Migration
   def change
-    add_column :users, :supervisor, :boolean, default: false
+    add_column :users, :role, :boolean, default: false
   end
 end

db/schema.rb

@@ -106,7 +106,7 @@
     t.string   "last_sign_in_ip",        limit: 255
     t.datetime "created_at",                                         null: false
     t.datetime "updated_at",                                         null: false
-    t.boolean  "supervisor",                         default: false
+    t.boolean  "role",                               default: false
     t.datetime "deleted_at"
   end
 

db/seeds.rb

@@ -3,12 +3,12 @@
                            last_name: "test",
                            email: "[email protected]",
                            password: "password",
-                           supervisor: true
+                           role: :supervisor
 supervisor2 = User.create! first_name: "test1",
                            last_name: "test1",
                            email: "[email protected]",
                            password: "password",
-                           supervisor: true
+                           role: :supervisor
 ## trainees
 99.times do |n|
   first_name = Faker::Name.first_name
@@ -38,16 +38,10 @@
     description: Faker::Lorem.paragraph,
     start_date: Faker::Date.backward,
     end_date: Faker::Date.forward,
-    status: 0
-  course.users = [supervisor1]
+    status: :created
 
-  5.times do |uc|
-    course.user_ids = course.user_ids << trainees.sample.id
-  end
-
-  3.times do |cs|
-    course.subject_ids = course.subject_ids << subjects.sample.id
-  end
+  course.subjects = subjects.sample 3  
+  course.users = [supervisor1] + trainees.sample(5)
   course.save!
 end