From 0dff5ea784b849e118a7045a5bf185788fcb6d43 Mon Sep 17 00:00:00 2001 From: benni Date: Sun, 16 Dec 2012 19:03:04 +0100 Subject: [PATCH] Fixed some bugs in tasks, apple feature: * Update ordergroup stats when task is destroyed. * Removed assigned caching attribute in task object. * A lot of eager loading for tasks controller. --- app/controllers/tasks_controller.rb | 13 ++++++++---- app/helpers/tasks_helper.rb | 5 ++--- app/models/assignment.rb | 10 --------- app/models/ordergroup.rb | 13 +++++++----- app/models/task.rb | 21 ++++++++++--------- app/views/orders/show.html.haml | 6 +++--- app/views/tasks/_archive_tasks.html.haml | 4 +--- app/views/tasks/show.haml | 10 +++++---- ...121216180646_remove_assigned_from_tasks.rb | 9 ++++++++ db/schema.rb | 7 +++---- 10 files changed, 52 insertions(+), 46 deletions(-) create mode 100644 db/migrate/20121216180646_remove_assigned_from_tasks.rb diff --git a/app/controllers/tasks_controller.rb b/app/controllers/tasks_controller.rb index 62e29d8e..2d1937f0 100644 --- a/app/controllers/tasks_controller.rb +++ b/app/controllers/tasks_controller.rb @@ -3,8 +3,8 @@ class TasksController < ApplicationController #auto_complete_for :user, :nick def index - @non_group_tasks = Task.non_group - @groups = Workgroup.all + @non_group_tasks = Task.non_group.includes(assignments: :user) + @groups = Workgroup.includes(open_tasks: {assignments: :user}) end def user @@ -50,7 +50,12 @@ class TasksController < ApplicationController end def destroy - Task.find(params[:id]).destroy + task = Task.find(params[:id]) + # Save user_ids to update apple statistics after destroy + user_ids = task.user_ids + task.destroy + task.update_ordergroup_stats(user_ids) + redirect_to tasks_url, :notice => "Aufgabe wurde gelöscht" end @@ -79,7 +84,7 @@ class TasksController < ApplicationController # Shows all tasks, which are already done def archive - @tasks = Task.done.page(params[:page]).per(@per_page) + @tasks = Task.done.page(params[:page]).per(@per_page).order('tasks.updated_on DESC').includes(assignments: :user) end # shows workgroup (normal group) to edit weekly_tasks_template diff --git a/app/helpers/tasks_helper.rb b/app/helpers/tasks_helper.rb index 9d002c4d..9081ccaa 100644 --- a/app/helpers/tasks_helper.rb +++ b/app/helpers/tasks_helper.rb @@ -9,9 +9,8 @@ module TasksHelper # generate colored number of still required users def highlighted_required_users(task) unless task.enough_users_assigned? - still_required = task.required_users - task.assignments.select { |ass| ass.accepted }.size - content_tag :span, still_required, class: 'badge badge-important', - title: "Es fehlen #{still_required} Mitstreiterinnen!" + content_tag :span, task.still_required_users, class: 'badge badge-important', + title: "Es fehlen #{task.still_required_users} Mitstreiterinnen!" end end end diff --git a/app/models/assignment.rb b/app/models/assignment.rb index 04127d7e..9781b7b0 100644 --- a/app/models/assignment.rb +++ b/app/models/assignment.rb @@ -2,16 +2,6 @@ class Assignment < ActiveRecord::Base belongs_to :user belongs_to :task - - # after user is assigned mark task as assigned - def after_create - self.task.update_attribute(:assigned, true) - end - - # update assigned-attribute - def after_destroy - self.task.update_attribute(:assigned, false) if self.task.assignments.empty? - end end diff --git a/app/models/ordergroup.rb b/app/models/ordergroup.rb index efaae4de..49649062 100644 --- a/app/models/ordergroup.rb +++ b/app/models/ordergroup.rb @@ -6,6 +6,9 @@ # * account_balance (decimal) # * account_updated (datetime) class Ordergroup < Group + + APPLE_MONTH_AGO = 6 # How many month back we will count tasks and orders sum + acts_as_paranoid # Avoid deleting the ordergroup for consistency of order-results serialize :stats @@ -56,11 +59,10 @@ class Ordergroup < Group end def update_stats! - time = 6.month.ago # Get hours for every job of each user in period - jobs = users.sum { |u| u.tasks.done.sum(:duration, :conditions => ["updated_on > ?", time]) } + jobs = users.sum { |u| u.tasks.done.sum(:duration, :conditions => ["updated_on > ?", APPLE_MONTH_AGO.month.ago]) } # Get group_order.price for every finished order in this period - orders_sum = group_orders.includes(:order).merge(Order.finished).where('orders.ends >= ?', time).sum(:price) + orders_sum = group_orders.includes(:order).merge(Order.finished).where('orders.ends >= ?', APPLE_MONTH_AGO.month.ago).sum(:price) @readonly = false # Dirty hack, avoid getting RecordReadOnly exception when called in task after_save callback. A rails bug? update_attribute(:stats, {:jobs_size => jobs, :orders_sum => orders_sum}) @@ -79,12 +81,13 @@ class Ordergroup < Group # If the the option stop_ordering_under is set, the ordergroup is only allowed to participate in an order, # when the apples value is above the configured amount. # The restriction can be deactivated for each ordergroup. - # Only ordergroups, which have participated in more than 5 order are affected + # Only ordergroups, which have participated in more than 5 orders in total and more than 2 orders in apple time period def not_enough_apples? FoodsoftConfig[:stop_ordering_under].present? and !ignore_apple_restriction and apples < FoodsoftConfig[:stop_ordering_under] and - group_orders.count > 5 + group_orders.count > 5 and + group_orders.joins(:order).merge(Order.finished).where('orders.ends >= ?', APPLE_MONTH_AGO.month.ago).count > 2 end # Global average diff --git a/app/models/task.rb b/app/models/task.rb index e957d2f1..bef3a44e 100644 --- a/app/models/task.rb +++ b/app/models/task.rb @@ -37,11 +37,12 @@ class Task < ActiveRecord::Base where(["tasks.due_date >= ? AND tasks.due_date <= ?", Time.now, number.days.from_now]) end - # count tasks with no responsible person + # count tasks with not enough responsible people # tasks for groups the user is not a member are ignored def self.unassigned_tasks_for(user) - Task.undone.where(assigned: false).includes(:workgroup).all.select do |task| - !task.workgroup or user.member_of?(task.workgroup) + undone.includes(:assignments, workgroup: :memberships).select do |task| + !task.enough_users_assigned? and + (!task.workgroup or task.workgroup.memberships.detect { |m| m.user_id == user.id }) end end @@ -54,7 +55,11 @@ class Task < ActiveRecord::Base end def enough_users_assigned? - assignments.find_all_by_accepted(true).size >= required_users ? true : false + assignments.to_a.count(&:accepted) >= required_users ? true : false + end + + def still_required_users + required_users - assignments.to_a.count(&:accepted) end # Get users from comma seperated ids @@ -95,12 +100,8 @@ class Task < ActiveRecord::Base @user_list ||= users.collect(&:id).join(", ") end - private - - def update_ordergroup_stats - if done - Ordergroup.joins(:users).where(users: {id: user_ids}).each(&:update_stats!) - end + def update_ordergroup_stats(user_ids = self.user_ids) + Ordergroup.joins(:users).where(users: {id: user_ids}).each(&:update_stats!) end end diff --git a/app/views/orders/show.html.haml b/app/views/orders/show.html.haml index 2e6f163d..275d2a7e 100644 --- a/app/views/orders/show.html.haml +++ b/app/views/orders/show.html.haml @@ -28,15 +28,15 @@ .form-actions - if @order.open? = link_to "Bearbeiten", edit_order_path(@order), class: 'btn' - = link_to 'Beenden!', finish_order_path(@order), method: :post, - confirm: "Willst Du wirklich die Bestellung beenden?\nEs gibt kein zurück..", class: 'btn' + = link_to 'Beenden!', finish_order_path(@order), method: :post, class: 'btn btn-success', + confirm: "Willst Du wirklich die Bestellung beenden?\nEs gibt kein zurück.." - unless @order.closed? = link_to "Löschen", @order, confirm: "Willst du wirklich die Bestellung löschen?", method: :delete, class: 'btn btn-danger' - unless @order.open? %ul.nav.nav-pills - %li.active= update_articles_link(@order, "Artikelübersicht", :default) + %li= update_articles_link(@order, "Artikelübersicht", :default) %li= update_articles_link(@order, "Sortiert nach Gruppen", :groups) %li= update_articles_link(@order, "Sortiert nach Artikeln", :articles) %li= link_to 'Kommentare', '#comments' diff --git a/app/views/tasks/_archive_tasks.html.haml b/app/views/tasks/_archive_tasks.html.haml index 431667f7..13df6503 100644 --- a/app/views/tasks/_archive_tasks.html.haml +++ b/app/views/tasks/_archive_tasks.html.haml @@ -14,6 +14,4 @@ %tr %td= task.due_date unless task.due_date.nil? %td= link_to "#{task.name} (#{task.duration}h)", :controller => "tasks", :action => "show", :id => task - %td - - unless task.users.empty? - = task.users.map(&:nick).join(", ") \ No newline at end of file + %td= task_assignments task \ No newline at end of file diff --git a/app/views/tasks/show.haml b/app/views/tasks/show.haml index d74bcb9f..82328739 100644 --- a/app/views/tasks/show.haml +++ b/app/views/tasks/show.haml @@ -5,10 +5,12 @@ %dl.dl-horizontal %dt Name %dd= @task.name - %dt Beschreibung - %dd= simple_format(@task.description) - %dt Fälligkeitsdatum - %dd= format_date(@task.due_date) + - if @task.description.present? + %dt Beschreibung + %dd= simple_format(@task.description) + - if @task.due_date.present? + %dt Fälligkeitsdatum + %dd= format_date(@task.due_date) %dt Dauer in Stunden %dd= @task.duration %dt Verantwortliche Menschen diff --git a/db/migrate/20121216180646_remove_assigned_from_tasks.rb b/db/migrate/20121216180646_remove_assigned_from_tasks.rb new file mode 100644 index 00000000..97fc6477 --- /dev/null +++ b/db/migrate/20121216180646_remove_assigned_from_tasks.rb @@ -0,0 +1,9 @@ +class RemoveAssignedFromTasks < ActiveRecord::Migration + def up + remove_column :tasks, :assigned + end + + def down + add_column :tasks, :assigned, :boolean + end +end diff --git a/db/schema.rb b/db/schema.rb index 423da9a8..5ac449d4 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20121112093327) do +ActiveRecord::Schema.define(:version => 20121216180646) do create_table "article_categories", :force => true do |t| t.string "name", :default => "", :null => false @@ -88,8 +88,8 @@ ActiveRecord::Schema.define(:version => 20121112093327) do t.datetime "failed_at" t.string "locked_by" t.string "queue" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false end add_index "delayed_jobs", ["priority", "run_at"], :name => "delayed_jobs_priority" @@ -330,7 +330,6 @@ ActiveRecord::Schema.define(:version => 20121112093327) do t.date "due_date" t.boolean "done", :default => false t.integer "workgroup_id" - t.boolean "assigned", :default => false t.datetime "created_on", :null => false t.datetime "updated_on", :null => false t.integer "required_users", :default => 1