various small refactorings

app/helpers/feedlist_helper.rb

@@ -27,12 +27,16 @@ module FeedlistHelper
     return html.html_safe
   end
 
+  def all_feed_links(object, symbol)
+    feed_links([:rss, :txt, :ical], { :controller=> 'todos', :action => 'index', symbol => object.to_param }, content_tag(:strong, object.name))
+  end
+
   def all_feed_links_for_project(project)
-    feed_links([:rss, :txt, :ical], { :controller=> 'todos', :action => 'index', :project_id => project.to_param }, content_tag(:strong, project.name))
+    all_feed_links(project, :project_id)  
   end
 
   def all_feed_links_for_context(context)
-    feed_links([:rss, :txt, :ical], { :controller=> 'todos', :action => 'index', :context_id => context.to_param }, content_tag(:strong, context.name))
+    all_feed_links(context, :context_id)
   end
 
   protected
@@ -44,5 +48,6 @@ module FeedlistHelper
   def user_token_hash
     { :token => current_user.token }
   end
+
     
 end

app/models/message_gateway.rb

@@ -3,27 +3,14 @@ class MessageGateway < ActionMailer::Base
   extend ActionView::Helpers::SanitizeHelper::ClassMethods
   
   def receive(email)
-    user = get_user_from_email_address(email)
+    user = get_receiving_user_from_email_address(email)
     return false if user.nil?
+    return false unless check_sender_is_in_mailmap(user, email)
     
     context = user.prefs.sms_context
-    description = nil
+    todo_params = get_todo_params(email)
-    notes = nil
-    
-    if email.multipart?
-      description = get_text_or_nil(email.subject)
-      notes = get_first_text_plain_part(email)
-    else
-      if email.subject.blank?
-        description = get_decoded_text_or_nil(email.body)
-        notes = nil
-      else
-        description = get_text_or_nil(email.subject)
-        notes = get_decoded_text_or_nil(email.body)
-      end
-    end
 
-    todo_builder = TodoFromRichMessage.new(user, context.id, description, notes)
+    todo_builder = TodoFromRichMessage.new(user, context.id, todo_params[:description], todo_params[:notes])
     todo = todo_builder.construct
     todo.save!
     Rails.logger.info "Saved email as todo for user #{user.login} in context #{context.name}"
@@ -32,32 +19,60 @@ class MessageGateway < ActionMailer::Base
   
   private
   
-  def get_address(email)
+  def get_todo_params(email)
-    return SITE_CONFIG['email_dispatch'] == 'to' ?  email.to[0] :  email.from[0]
+    params = {}
-  end
+    
-  
+    if email.multipart?
-  def get_user_from_env_setting
+      params[:description] = get_text_or_nil(email.subject)
+      params[:notes]       = get_first_text_plain_part(email)
+    else
+      if email.subject.blank?
+        params[:description] = get_decoded_text_or_nil(email.body)
+        params[:notes]       = nil
+      else
+        params[:description] = get_text_or_nil(email.subject)
+        params[:notes]       = get_decoded_text_or_nil(email.body)
+      end
+    end
+    params
+  end    
+
+  def get_receiving_user_from_email_address(email)
+    SITE_CONFIG['email_dispatch'] == 'single_user' ? get_receiving_user_from_env_setting : get_receiving_user_from_mail_header(email)
+  end  
+
+  def get_receiving_user_from_env_setting
     Rails.logger.info "All received email goes to #{ENV['TRACKS_MAIL_RECEIVER']}"
     user = User.where(:login => ENV['TRACKS_MAIL_RECEIVER']).first
     Rails.logger.info "WARNING: Unknown user set for TRACKS_MAIL_RECEIVER (#{ENV['TRACKS_MAIL_RECEIVER']})" if user.nil?
     return user
   end
   
-  def get_user_from_mail_header(email)
+  def get_receiving_user_from_mail_header(email)
-    address = get_address(email)
+    user = get_receiving_user_from_sms_email( get_address(email) )
-    Rails.logger.info "Looking for user with email #{address}"
+    Rails.logger.info(user.nil? ? "User unknown": "Email belongs to #{user.login}")
-    user = User.where("preferences.sms_email" => address.strip).includes(:preference).first
-    if user.nil?
-      user = User.where("preferences.sms_email" => address.strip[1.100]).includes(:preference).first
-    end
-    if user.present? and !sender_is_in_mailmap?(user,email)
-      Rails.logger.warn "#{email.from[0]} not found in mailmap for #{user.login}"
-      return nil
-    end
-    Rails.logger.info(!user.nil? ? "Email belongs to #{user.login}" : "User unknown")
     return user
   end
 
+  def get_address(email)
+    return SITE_CONFIG['email_dispatch'] == 'to' ?  email.to[0] :  email.from[0]
+  end
+
+  def get_receiving_user_from_sms_email(address)
+    Rails.logger.info "Looking for user with email #{address}"
+    user = User.where("preferences.sms_email" => address.strip).includes(:preference).first
+    user = User.where("preferences.sms_email" => address.strip[1.100]).includes(:preference).first if user.nil?
+    return user
+  end    
+
+  def check_sender_is_in_mailmap(user, email)
+    if user.present? and !sender_is_in_mailmap?(user,email)
+      Rails.logger.warn "#{email.from[0]} not found in mailmap for #{user.login}"
+      return false
+    end
+    return true
+  end    
+
   def sender_is_in_mailmap?(user,email)
     if SITE_CONFIG['mailmap'].is_a? Hash and SITE_CONFIG['email_dispatch'] == 'to'
       # Look for the sender in the map of allowed senders
@@ -69,11 +84,6 @@ class MessageGateway < ActionMailer::Base
     end
   end
 
-
-  def get_user_from_email_address(email)
-    SITE_CONFIG['email_dispatch'] == 'single_user' ? get_user_from_env_setting : get_user_from_mail_header(email)
-  end
-
   def get_text_or_nil(text)
     return text ? sanitize(text.strip) : nil
   end

lib/is_taggable.rb

@@ -80,7 +80,11 @@ module IsTaggable
         end
       end
 
-      def tag_cast_to_string obj
+      def tag_cast_to_string(obj)
+        tag_array_from_obj(obj).flatten.compact.map(&:downcase).uniq
+      end
+
+      def tag_array_from_obj(obj)
         case obj
         when Array
           obj.map! { |item| get_tag_name_from_item(item) }
@@ -88,7 +92,7 @@ module IsTaggable
           obj.split(Tag::DELIMITER).map { |tag_name| tag_name.strip.squeeze(" ") }
         else
           raise "Invalid object of class #{obj.class} as tagging method parameter"
-        end.flatten.compact.map(&:downcase).uniq
+        end
       end
             
     end

test/models/is_taggable_test.rb

@@ -42,5 +42,33 @@ class IsTaggableTest < ActiveSupport::TestCase
     t.tag_with "a, b, e, f"
     assert_equal "a, b, e, f", t.tag_list, "should add e and f and remove c and d"
   end
+
+  def test_editing_using_taglist
+    t = Todo.create(:description => "test", :context => Context.first)
+    t.tag_list = "a, b, c"
+    assert_equal 3, t.tags.count
+    assert_equal "a, b, c", t.tag_list
+
+
+    t.tag_list = "d, e"
+    assert_equal 2, t.tags.count
+    assert_equal "d, e", t.tag_list
+  end
   
+  def test_tag_cast_to_string
+    t = Todo.create(:description => "test", :context => Context.first)
+
+    obj = "tag"
+    assert_equal ["tag"], t.tag_cast_to_string(obj)
+
+    obj = ["tag1", Tag.new(name: "tag2")]
+    assert_equal ["tag1", "tag2"], t.tag_cast_to_string(obj)
+
+    obj = {a: "hash"}
+    assert_raise(RuntimeError) { t.tag_cast_to_string(obj) } 
+
+    obj = ["string", {a: "hash"}]
+    assert_raise(RuntimeError) { t.tag_cast_to_string(obj) } 
+  end
+
 end