#447452: models: manager urgent flag (transitions, previous setting, validations)

Author: tokland

app/models/comment.rb

@@ -39,7 +39,7 @@ def previous_assigned
     :reject_if => lambda { |google_docs| google_docs['title'].blank? || google_docs['url'].blank? }
 
   attr_accessible :body, :status, :assigned, :hours, :human_hours, :billable,
-                  :upload_ids, :uploads_attributes, :due_on, :google_docs_attributes, :private_ids, :is_private
+                  :upload_ids, :uploads_attributes, :due_on, :urgent, :google_docs_attributes, :private_ids, :is_private
 
   attr_accessor :is_importing
   attr_accessor :private_ids
@@ -246,6 +246,7 @@ def cleanup_task
     if @last_comment_in_task
       self.target.assigned_id = previous_assigned_id
       self.target.due_on = previous_due_on
+      self.target.urgent = previous_urgent
       self.target.status = previous_status || Task::STATUSES[:open]
       self.target.save!
     end

app/models/comment/conversions.rb

@@ -54,6 +54,8 @@ def to_api_hash(options = {})
       base[:status] = status
       base[:due_on] = due_on
       base[:previous_due_on] = previous_due_on
+      base[:urgent] = urgent
+      base[:previous_urgent] = previous_urgent
     end
 
     if Array(options[:include]).include?(:uploads) && uploads.any?

app/models/comment/tasks.rb

@@ -5,7 +5,7 @@ def previously_closed?
   end
 
   def transition?
-    status_transition? || assigned_transition? || due_on_change?
+    status_transition? || assigned_transition? || due_on_change? || urgent_change?
   end
 
   def initial_status?
@@ -20,6 +20,10 @@ def due_on_change?
     due_on != previous_due_on
   end
 
+  def urgent_change?
+    urgent != previous_urgent
+  end
+
   def assigned_transition?
     assigned_id != previous_assigned_id
   end
@@ -60,4 +64,4 @@ def previous_status_name
     Task::STATUS_NAMES[previous_status]
   end
 
-end
+end

app/models/task.rb

@@ -24,7 +24,7 @@ class Task < RoleRecord
   accepts_nested_attributes_for :comments, :allow_destroy => false,
     :reject_if => lambda { |comment| %w[is_private body hours human_hours uploads_attributes google_docs_attributes].all? { |k| comment[k].blank? } }
 
-  attr_accessible :name, :assigned_id, :status, :due_on, :comments_attributes, :user, :task_list_id
+  attr_accessible :name, :assigned_id, :status, :due_on, :comments_attributes, :user, :task_list_id, :urgent
 
   validates_presence_of :user
   validates_presence_of :task_list
@@ -49,14 +49,15 @@ class Task < RoleRecord
   before_save :save_completed_at
   before_validation :remember_comment_created, :on => :update
   before_save :update_google_calendar_event, :if => lambda {|t| t.assigned.try(:user) || !t.google_calendar_url_token.blank? }
+  before_validation :nilize_due_on_for_urgent_tasks
 
   def assigned
     @assigned ||= assigned_id ? Person.with_deleted.find_by_id(assigned_id) : nil
   end
 
   def track_changes?
     (new_record? and not status_new?) or
-    (updating_user and (status_changed? or assigned_id_changed? or due_on_changed?))
+    (updating_user and (status_changed? or assigned_id_changed? or due_on_changed? or urgent_changed?))
   end
 
   def archived?
@@ -299,7 +300,7 @@ def set_comments_author # before_save
   end
 
   def remember_comment_created # before_update
-    @comment_created = comments.any?(&:new_record?) || assigned_id_changed? || status_changed? || due_on_changed?
+    @comment_created = comments.any?(&:new_record?) || assigned_id_changed? || status_changed? || due_on_changed? || urgent_changed?
     true
   end
 
@@ -331,6 +332,11 @@ def save_changes_to_comment # before_save
       comment.previous_due_on = self.due_on_was if due_on_changed?
     end
 
+    if urgent_changed? or self.new_record?
+      comment.urgent = self.urgent
+      comment.previous_urgent = self.urgent_was if urgent_changed?
+    end
+
     @saved_changes_to_comment = true
     true
   end
@@ -465,4 +471,8 @@ def delete_old_events_if_required
       self.google_calendar_url_token = nil
     end
   end
+
+  def nilize_due_on_for_urgent_tasks
+    self.due_on = nil if self.urgent?
+  end  
 end

app/models/user.rb

@@ -260,7 +260,7 @@ def pending_tasks
     Rails.cache.fetch("pending_tasks.#{id}") do
       active_project_ids.empty? ? [] :
         Task.where(:status => Task::ACTIVE_STATUS_CODES).where(:assigned_id => active_project_ids).order('ID desc').includes(:project).
-             sort { |a,b| (a.due_on || 1.week.from_now.to_date) <=> (b.due_on || 1.year.from_now.to_date) }
+             sort { |a,b| [a.urgent? ? 0 : 1, (a.due_on || 1.week.from_now.to_date)] <=> [b.urgent? ? 0 : 1, (b.due_on || 1.year.from_now.to_date)] }
     end
   end
 

spec/models/task_spec.rb

@@ -57,6 +57,14 @@
       task.status_name = 'silly'
     }.should raise_error(ArgumentError)
   end
+  
+  it "should nilizie due_on only when urgent flag is set" do
+    task1 = Factory(:task, :due_on => Time.now, :urgent => false)
+    task1.due_on.should_not be_nil
+
+    task2 = Factory(:task, :due_on => Time.now, :urgent => true)
+    task2.due_on.should be_nil
+  end
 
   describe "assigning tasks" do
     before do

spec/models/user_spec.rb

@@ -449,6 +449,17 @@
       @task.project.update_attribute :archived, true
       @user.pending_tasks.should be_empty
     end
+    it "should list active tasks sorted by (urgent, due_on ASC)" do
+      @task.assign_to(@user)
+      @task2 = Factory.create(:task, :due_on => 1.minute.from_now)
+      @task3 = Factory.create(:task, :due_on => 2.days.from_now)
+      @task4 = Factory.create(:task, :urgent => true)
+      [@task2, @task3, @task4].each do |task|
+        task.project.add_user(@user)
+        task.assign_to(@user)
+      end      
+      @user.pending_tasks.should == [@task4, @task2, @task3, @task]
+    end
   end
 
   describe "#assigned_tasks_count" do