Multi Tag controller action now accepts lists also (#4606)

* Multiple tag controller action accepts lists also now

* Few minor changes

* routes modified

* User not logged in

* minor change

app/controllers/subscription_controller.rb

@@ -119,11 +119,16 @@ def digest
   end
 
   def multiple_add
-    unless params[:names]
+    if !params[:names] || params[:names] == ''
       flash[:notice] = "Please enter tags for subscription in the url."
       redirect_to "/subscriptions" + "?_=" + Time.now.to_i.to_s
+      return
+    end
+    if params[:names].is_a? String
+      tag_list = params[:names].split(',')
+    else
+      tag_list = params[:names]
     end
-    tag_list = params[:names].split(',')
     # should be logged in to subscribe
     if current_user
       # assume tag, for now
@@ -170,8 +175,8 @@ def multiple_add
         # user or node subscription
       end
     else
-      flash[:warning] = "You must be logged in to subscribe for email updates."
-      redirect_to "/login"
+      flash[:warning] = "You must be logged in to subscribe for email updates!"
+      redirect_to "/login?return_to=" + request.fullpath
     end
   end
 

app/views/tag/_tags.html.erb

@@ -1,7 +1,6 @@
 <ul class= "list-inline">
   <% tags.each do |tag| %>
     <% if tag.class == NodeTag %>
-
       <% if power_tag ^ tag.name.include?(':') # XOR operator??? %>
         <li><span id="tag_<%= tag.tid %>" class="label <%= label_name %>" style="cursor:pointer" data-toggle="popover" data-trigger="focus" data-count=0 data-placement="top" data-content="<p style='text-align:center;'><a href='/tag/<%= tag.name %>'><%= Tag.tagged_node_count(tag.name) || 0 %> notes</a> - <a href='/contributors/<%= tag.name %>'><%= Tag.contributors(tag.name).count %> people <br></a></p> <p style='text-align:center;font-size:12px;'><%if tag.description %><%= tag.description %> |<% end %> created by <a href='/profile/<%= tag.try(:author).try(:username) %>'><%= tag.try(:author).try(:username) %></a> <%= time_ago_in_words(Time.at(tag.date)) %> ago </p> <div style='text-align:center;'><a href='/subscribe/tag/<%= tag.name %>' class='btn btn-primary'>Follow</a></div>" data-html="true" title="<%= tag.name %>">
           <%= tag.name %>

config/routes.rb

@@ -87,8 +87,9 @@
   get 'subscriptions' => 'subscription#index'
   get 'subscriptions/digest' => 'subscription#digest'
   get 'subscribe/multiple/:type/:names' => 'subscription#multiple_add'
-  put 'subscribe/multiple/:type/:names' => 'subscription#multiple_add'
-
+  post 'subscribe/multiple/:type/:names' => 'subscription#multiple_add'
+  get 'subscribe/multiple/:type' => 'subscription#multiple_add'
+  post 'subscribe/multiple/:type' => 'subscription#multiple_add'
   get 'wiki/stale' => 'wiki#stale'
   get 'wiki/new' => 'wiki#new'
   get 'wiki/replace/:id' => 'wiki#replace'

test/functional/subscription_controller_test.rb

@@ -37,4 +37,18 @@ def setup
     assert users(:bob).following(:kites)
     assert users(:bob).following(:balloon)
   end
+
+  test 'should not subscribe to multiple tags in case of empty string' do
+    UserSession.create(users(:bob))
+    assert users(:bob).following(:awesome)
+    get :multiple_add, params: { type: 'tag', names: '' }
+    assert_response :redirect
+    assert_equal "Please enter tags for subscription in the url.", flash[:notice]
+  end
+
+  test 'user is not logged in and tries to subscribe multiple tags' do
+    get :multiple_add, params: { type: 'tag', names: 'kites,balloon' }
+    assert_redirected_to '/login?return_to=/subscribe/multiple/tag/kites,balloon'
+    assert_equal "You must be logged in to subscribe for email updates!", flash[:warning]
+  end
 end