Some cleanup of code

2 files changed, 24 insertions, 16 deletions

diff --git a/help/app/models/ticket.rb b/help/app/models/ticket.rb
index 7192cb3..fa056b4 100644
--- a/help/app/models/ticket.rb
+++ b/help/app/models/ticket.rb
@@ -141,25 +141,13 @@ class Ticket < CouchRest::Model::Base
 
   def self.for_user(user, options = {}, is_admin = false)
 
-    # TODO: This is obviously super tedious. we will refactor later.
     # TODO: thought i  should reverse keys for descending, but that didn't work. look into whether that should be tweaked, and whether it works okay with pagination (seems to now...)
-    # TODO: Time.now + 2.days is to catch tickets created in future. shouldn't happen but does on my computer now, so this at least catches for now.
-    # TODO handle default values correctly:
-    options[:open_status] = 'open' if !options[:open_status] #hacky. redo this when handling defaults correctly
-    options[:sort_order] = 'updated_at_desc' if !options[:sort_order] #hacky. redo this when handling defaults correctly
 
     options[:user_id] = user.id
     options[:is_admin] = is_admin
 
     @selection = TicketSelection.new(options)
-
-    #TODO: can this be more succinct?
-    if @selection.order
-      @tickets = Ticket.send(@selection.finder_method).startkey(@selection.startkey).endkey(@selection.endkey).send(@selection.order)
-    else
-      @tickets = Ticket.send(@selection.finder_method).startkey(@selection.startkey).endkey(@selection.endkey)
-    end
-
+    @selection.tickets
   end
 
   #def self.tickets_by_commenter(user_id)#, options = {})
diff --git a/help/app/models/ticket_selection.rb b/help/app/models/ticket_selection.rb
index 77720f7..cb20f5b 100644
--- a/help/app/models/ticket_selection.rb
+++ b/help/app/models/ticket_selection.rb
@@ -2,18 +2,33 @@ class TicketSelection
 
   def initialize(options = {})
     @options = options
+    @options[:open_status] ||= 'open'
+    @options[:sort_order] ||= 'updated_at_desc'
+
+  end
+
+  def tickets
+    #TODO: can this be more succinct?
+    if order
+      Ticket.send(finder_method).startkey(startkey).endkey(endkey).send(order)
+    else
+      Ticket.send(finder_method).startkey(startkey).endkey(endkey)
+    end
   end
 
+  protected
+
+
   def finder_method
     method = 'by_'
-    method += 'includes_post_by_and_' if !@options[:is_admin] or (@options[:admin_status] == 'mine')
+    method += 'includes_post_by_and_' if only_mine?
     method += 'is_open_and_' if @options[:open_status] != 'all'
     method += @options[:sort_order].sub(/_(de|a)sc$/, '')
   end
 
   def startkey
     startkeys = []
-    startkeys << @options[:user_id] if !@options[:is_admin] or (@options[:admin_status] == 'mine')
+    startkeys << @options[:user_id] if only_mine?
     startkeys << (@options[:open_status] == 'open') if @options[:open_status] != 'all'
     startkeys << 0
     startkeys = startkeys.join if startkeys.length == 1 #want string not array if just one thing in array
@@ -21,7 +36,7 @@ class TicketSelection
   end
 
   def endkey
-    endtime = Time.now + 2.days #TODO
+    endtime = Time.now + 2.days #TODO. this obviously isn't ideal
     if self.startkey.is_a?(Array)
       endkeys = self.startkey
       endkeys.pop
@@ -35,4 +50,9 @@ class TicketSelection
     'descending' if @options[:sort_order].end_with? 'desc'
   end
 
+
+  def only_mine?
+    !@options[:is_admin] or (@options[:admin_status] == 'mine')
+  end
+
 end