[suggestions] Some enhancements

Added title on links Moved logic from view to model Added conditions to avoid displaying empty lists Removed code duplication on js

[suggestions] Some enhancements
Added title on links Moved logic from view to model Added conditions to avoid displaying empty lists Removed code duplication on js
Daniela Feitosa
1 parent 9cc4340d
Showing 10 changed files with 71 additions and 65 deletions Show diff stats
app/helpers/application_helper.rb
app/models/communities_block.rb
app/views/blocks/communities.html.erb
app/views/shared/_profile_connections.html.erb
app/views/shared/_profile_suggestions_list.html.erb
public/javascripts/add-and-join.js
public/stylesheets/application.css
test/functional/friends_controller_test.rb
test/functional/memberships_controller_test.rb
test/unit/communities_block_test.rb
@@ -1424,7 +1424,7 @@ module ApplicationHelper
   def profile_suggestion_profile_connections(suggestion)
     profiles = suggestion.profile_connections.first(4).map do |profile|
-      link_to(profile_image(profile, :icon), profile.url, :class => 'profile-suggestion-connection-icon', :title => profile.name)
+      link_to(profile_image(profile, :icon, :title => profile.name), profile.url, :class => 'profile-suggestion-connection-icon')
     end
     controller_target = suggestion.suggestion_type == 'Person' ? :friends : :memberships
@@ -1446,7 +1446,7 @@ module ApplicationHelper
     title = tags.join
     controller_target = suggestion.suggestion_type == 'Person' ? :friends : :memberships
-    tags << ' ' + link_to('...', {:controller => controller_target, :action => :connections, :id => suggestion.suggestion_id}, :class => 'more-tag-connections') if suggestion.tag_connections.count > 4
+    tags << ' ' + link_to('...', {:controller => controller_target, :action => :connections, :id => suggestion.suggestion_id}, :class => 'more-tag-connections', :title => _('See all connections')) if suggestion.tag_connections.count > 4
     if tags.present?
       content_tag(:div, tags.join, :class => 'tag-connections', :title => title)
@@ -22,6 +22,7 @@ class CommunitiesBlock &lt; ProfileListBlock
   def footer
     owner = self.owner
     suggestions = self.suggestions
+    return '' unless owner.kind_of?(Profile) || owner.kind_of?(Environment)
     proc do
       render :file => 'blocks/communities', :locals => { :owner => owner, :suggestions => suggestions }
     end
@@ -2,11 +2,9 @@
   <%= link_to s_('communities|View all'), {:profile => owner.identifier, :controller => 'profile', :action => 'communities'}, :class => 'view-all' %>
 <% elsif owner.kind_of?(Environment) %>
   <%= link_to s_('communities|View all'), {:controller => 'search', :action => 'communities'}, :class => 'view-all' %>
-<% else %>
-  ''
 <% end %>
-<% if user == profile && suggestions && !suggestions.empty? %>
+<% if user && user == profile && suggestions && !suggestions.empty? %>
   <div class='suggestions-block common-profile-list-block'>
     <h4 class='block-subtitle'><%= _('Some suggestions for you') %></h4>
     <div class='profiles-suggestions'>
-<h2><%= _("Profiles in common:") if profiles.present? %></h2>
-<div class="list-profile-connections">
-  <ul class="profile-list">
-    <% profiles.each do |profile| %>
-      <li>
-      <%= link_to_profile profile_image(profile) + '<br/>' + profile.short_name,
-                          profile.identifier, :class => 'profile-link' %>
-      </li>
-    <% end %>
-  </ul>
-</div>
-
-<br style="clear:both" />
+<% if profiles.present? %>
+  <h2><%= _("Profiles in common:") %></h2>
+  <div class="list-profile-connections">
+    <ul class="profile-list">
+      <% profiles.each do |profile| %>
+        <li>
+        <%= link_to_profile profile_image(profile) + '<br/>' + profile.short_name,
+                            profile.identifier, :class => 'profile-link' %>
+        </li>
+      <% end %>
+    </ul>
+  </div>
+  <br style="clear:both" />
+<% end %>
-<h2><%= _("Tags in common:") if tags.present? %></h2>
+<% if tags.present? %>
+  <h2><%= _("Tags in common:") %></h2>
   <ul>
     <% tags.each do |tag| %>
       <li><%= tag.name %></li>
     <% end %>
   </ul>
+<% end %>
@@ -10,14 +10,22 @@
   <ul class="profile-list">
     <% suggestions.each do |s| %>
       <li>
-        <%= link_to_profile profile_image(s.suggestion, :minor),
+        <%= link_to_profile profile_image(s.suggestion, :minor, :title => s.suggestion.short_name),
                             s.suggestion.identifier, :class => 'profile-link' %>
         <% if collection == :friends_suggestions %>
-          <%= link_to _('Add %s') % s.suggestion.short_name,
-                      s.suggestion.add_url, :class => 'accept-suggestion' %>
+          <%= link_to '+ %s' % s.suggestion.short_name,
+                        s.suggestion.add_url,
+                        :class => 'accept-suggestion',
+                        :title => _('Add suggestion'),
+                        :remote => true
+                      %>
         <% elsif collection == :communities_suggestions %>
-          <%= link_to _('Join %s') % s.suggestion.name,
-                      s.suggestion.add_url, :class => 'accept-suggestion' %>
+          <%= link_to '+ %s' % s.suggestion.name,
+                        s.suggestion.join_url,
+                        :class => 'accept-suggestion',
+                        :title => _('Add suggestion'),
+                        :remote => true
+                      %>
         <% end %>
         <div class='extra_info'>
           <%= profile_suggestion_profile_connections(s) %>
@@ -29,13 +37,17 @@
                 { :controller => 'friends', :action => 'remove_suggestion', :id => s.suggestion.identifier, :per_page => per_page },
                   :class => 'remove-suggestion',
                   :title => _('Remove suggestion'),
-                  :confirm => _('Are you sure you want to remove this suggestion?') %>
+                  :confirm => _('Are you sure you want to remove this suggestion?'),
+                  :remote => true
+                %>
           <% elsif collection == :communities_suggestions %>
               <%= link_to 'x',
                   { :controller => 'memberships', :action => 'remove_suggestion', :id => s.suggestion.identifier, :per_page => per_page },
                     :class => 'remove-suggestion',
                     :title => _('Remove suggestion'),
-                    :confirm => _('Are you sure you want to remove this suggestion?') %>
+                    :confirm => _('Are you sure you want to remove this suggestion?'),
+                    :remote => true
+                  %>
           <% end %>
         </div><!-- end class="controll" -->
       </li>
@@ -121,30 +121,29 @@ jQuery(function($) {
   })
   $(".remove-suggestion").live('click', function(){
-    clicked = $(this)
-    url = clicked.attr("href");
-    loading_for_button(this);
-    $.post(url, function(data){
-      clicked.fadeOut();
-      clicked.parents('.profiles-suggestions').html(data);
-    });
-    return false;
+    clicked = $(this);
+    removeSuggestionFromList(clicked);
   })
-  /* After adding a suggestion need to remove it from list */
   $(".accept-suggestion").live('click', function(){
     clicked = $(this)
     loading_for_button(this);
     url = clicked.attr("href");
-    remove_suggestion = clicked.parents('li').find('.remove-suggestion');
-    remove_url = remove_suggestion.attr('href')
-    $.post(remove_url, function(suggestions_data){
-      remove_suggestion.parents('.profiles-suggestions').html(suggestions_data);
-      $.post(url, function(add_data){
-        clicked.parents('li').fadeOut();
-      });
+    removeSuggestionFromList(clicked.parents('li').find('.remove-suggestion'));
+    $.post(url, function(add_data){
+      clicked.parents('li').fadeOut();
     });
     return false;
   })
 });
+
+/* Used after clicking on remove and after adding a suggestion */
+function removeSuggestionFromList( element ) {
+  url = element.attr("href");
+  loading_for_button(element);
+  jQuery.post(url, function(data){
+    element.fadeOut();
+    element.parents('.profiles-suggestions').html(data);
+  });
+}
@@ -1688,12 +1688,11 @@ a.button.disabled, input.disabled {
 #content .communities-block .block-footer-content .profiles-suggestions a.accept-suggestion,
 #content .communities-block .block-footer-content .profiles-suggestions a.remove-suggestion {
   position: relative;
-  z-index: 5;
 }
 #content .block-footer-content .profiles-suggestions a.accept-suggestion {
   padding-bottom: 3px;
-  display: inline-block;
+  display: block;
   color: #333;
   font-size: 11px
 }
@@ -75,16 +75,10 @@ class FriendsControllerTest &lt; ActionController::TestCase
     assert_no_tag :tag => 'a', :content => 'Suggest friends', :attributes => { :href => "/myprofile/testuser/friends/suggest" }
   end
-  should 'display link to list suggestions page' do
-    profile.profile_suggestions.create(:suggestion => friend)
-    get :index, :profile => 'testuser'
-    assert_tag :tag => 'a', :content => 'See more suggestions...', :attributes => { :href => "/myprofile/testuser/friends/suggest" }
-  end
-
   should 'display people suggestions' do
     profile.profile_suggestions.create(:suggestion => friend)
     get :suggest, :profile => 'testuser'
-    assert_tag :tag => 'a', :content => friend.name, :attributes => { :href => "/profile/#{friend.identifier}" }
+    assert_tag :tag => 'a', :content => "+ #{friend.name}", :attributes => { :href => "/profile/#{friend.identifier}/add" }
   end
   should 'display button to add friend suggestion' do
@@ -96,14 +90,14 @@ class FriendsControllerTest &lt; ActionController::TestCase
   should 'display button to remove people suggestion' do
     profile.profile_suggestions.create(:suggestion => friend)
     get :suggest, :profile => 'testuser'
-    assert_tag :tag => 'a', :attributes => { :href => "/myprofile/testuser/friends/remove_suggestion/#{friend.identifier}" }
+    assert_tag :tag => 'a', :attributes => { :href => /\/myprofile\/testuser\/friends\/remove_suggestion\/#{friend.identifier}/ }
   end
   should 'remove suggestion of friend' do
     suggestion = profile.profile_suggestions.create(:suggestion => friend)
     post :remove_suggestion, :profile => 'testuser', :id => friend.identifier
-    assert_redirected_to :action => 'suggest'
+    assert_response :success
     assert_equal false, ProfileSuggestion.find(suggestion.id).enabled
   end
@@ -340,7 +340,7 @@ class MembershipsControllerTest &lt; ActionController::TestCase
     community = fast_create(Community)
     profile.profile_suggestions.create(:suggestion => community)
     get :suggest, :profile => 'testuser'
-    assert_tag :tag => 'a', :content => community.name, :attributes => { :href => "/profile/#{community.identifier}" }
+    assert_tag :tag => 'a', :content => "+ #{community.name}", :attributes => { :href => "/profile/#{community.identifier}/join" }
   end
   should 'display button to join on community suggestion' do
@@ -354,7 +354,7 @@ class MembershipsControllerTest &lt; ActionController::TestCase
     community = fast_create(Community)
     profile.profile_suggestions.create(:suggestion => community)
     get :suggest, :profile => 'testuser'
-    assert_tag :tag => 'a', :attributes => { :href => "/myprofile/testuser/memberships/remove_suggestion/#{community.identifier}" }
+    assert_tag :tag => 'a', :attributes => { :href => /\/myprofile\/testuser\/memberships\/remove_suggestion\/#{community.identifier}/ }
   end
   should 'remove suggestion of community' do
@@ -362,7 +362,7 @@ class MembershipsControllerTest &lt; ActionController::TestCase
     suggestion = profile.profile_suggestions.create(:suggestion => community)
     post :remove_suggestion, :profile => 'testuser', :id => community.identifier
-    assert_redirected_to :action => 'suggest'
+    assert_response :success
     assert_equal false, ProfileSuggestion.find(suggestion.id).enabled
   end
@@ -27,30 +27,30 @@ class CommunitiesBlockTest &lt; ActiveSupport::TestCase
     assert_same list, block.profiles
   end
-  should 'link to all communities of profile' do
+  should 'support profile as block owner' do
     profile = Profile.new
-    profile.expects(:identifier).returns("theprofile")
     block = CommunitiesBlock.new
-    block.expects(:owner).returns(profile)
+    block.expects(:owner).returns(profile).at_least_once
-    expects(:link_to).with('View all', :controller => 'profile', :profile => 'theprofile', :action => 'communities')
+    self.expects(:render).with(:file => 'blocks/communities', :locals => { :owner => profile, :suggestions => block.suggestions })
     instance_eval(&block.footer)
   end
-  should 'support environment as owner' do
+  should 'support environment as block owner' do
     env = Environment.default
     block = CommunitiesBlock.new
-    block.expects(:owner).returns(env)
-
-    expects(:link_to).with('View all', :controller => "search", :action => 'communities')
+    block.expects(:owner).returns(env).at_least_once
+    self.expects(:render).with(:file => 'blocks/communities', :locals => { :owner => env, :suggestions => block.suggestions })
     instance_eval(&block.footer)
   end
   should 'give empty footer on unsupported owner type' do
     block = CommunitiesBlock.new
-    block.expects(:owner).returns(1)
+    block.expects(:owner).returns(1).at_least_once
+
+    self.expects(:render).with(anything).never
     assert_equal '', block.footer
   end
	@@ -1424,7 +1424,7 @@ module ApplicationHelper		@@ -1424,7 +1424,7 @@ module ApplicationHelper
1424		1424
1425	def profile_suggestion_profile_connections(suggestion)	1425	def profile_suggestion_profile_connections(suggestion)
1426	profiles = suggestion.profile_connections.first(4).map do \|profile\|	1426	profiles = suggestion.profile_connections.first(4).map do \|profile\|
1427	- link_to(profile_image(profile, :icon), profile.url, :class => 'profile-suggestion-connection-icon', :title => profile.name)	1427	+ link_to(profile_image(profile, :icon, :title => profile.name), profile.url, :class => 'profile-suggestion-connection-icon')
1428	end	1428	end
1429		1429
1430	controller_target = suggestion.suggestion_type == 'Person' ? :friends : :memberships	1430	controller_target = suggestion.suggestion_type == 'Person' ? :friends : :memberships
	@@ -1446,7 +1446,7 @@ module ApplicationHelper		@@ -1446,7 +1446,7 @@ module ApplicationHelper
1446	title = tags.join	1446	title = tags.join
1447		1447
1448	controller_target = suggestion.suggestion_type == 'Person' ? :friends : :memberships	1448	controller_target = suggestion.suggestion_type == 'Person' ? :friends : :memberships
1449	- tags << ' ' + link_to('...', {:controller => controller_target, :action => :connections, :id => suggestion.suggestion_id}, :class => 'more-tag-connections') if suggestion.tag_connections.count > 4	1449	+ tags << ' ' + link_to('...', {:controller => controller_target, :action => :connections, :id => suggestion.suggestion_id}, :class => 'more-tag-connections', :title => _('See all connections')) if suggestion.tag_connections.count > 4
1450		1450
1451	if tags.present?	1451	if tags.present?
1452	content_tag(:div, tags.join, :class => 'tag-connections', :title => title)	1452	content_tag(:div, tags.join, :class => 'tag-connections', :title => title)
	@@ -340,7 +340,7 @@ class MembershipsControllerTest < ActionController::TestCase		@@ -340,7 +340,7 @@ class MembershipsControllerTest < ActionController::TestCase
340	community = fast_create(Community)	340	community = fast_create(Community)
341	profile.profile_suggestions.create(:suggestion => community)	341	profile.profile_suggestions.create(:suggestion => community)
342	get :suggest, :profile => 'testuser'	342	get :suggest, :profile => 'testuser'
343	- assert_tag :tag => 'a', :content => community.name, :attributes => { :href => "/profile/#{community.identifier}" }	343	+ assert_tag :tag => 'a', :content => "+ #{community.name}", :attributes => { :href => "/profile/#{community.identifier}/join" }
344	end	344	end
345		345
346	should 'display button to join on community suggestion' do	346	should 'display button to join on community suggestion' do
	@@ -354,7 +354,7 @@ class MembershipsControllerTest < ActionController::TestCase		@@ -354,7 +354,7 @@ class MembershipsControllerTest < ActionController::TestCase
354	community = fast_create(Community)	354	community = fast_create(Community)
355	profile.profile_suggestions.create(:suggestion => community)	355	profile.profile_suggestions.create(:suggestion => community)
356	get :suggest, :profile => 'testuser'	356	get :suggest, :profile => 'testuser'
357	- assert_tag :tag => 'a', :attributes => { :href => "/myprofile/testuser/memberships/remove_suggestion/#{community.identifier}" }	357	+ assert_tag :tag => 'a', :attributes => { :href => /\/myprofile\/testuser\/memberships\/remove_suggestion\/#{community.identifier}/ }
358	end	358	end
359		359
360	should 'remove suggestion of community' do	360	should 'remove suggestion of community' do
	@@ -362,7 +362,7 @@ class MembershipsControllerTest < ActionController::TestCase		@@ -362,7 +362,7 @@ class MembershipsControllerTest < ActionController::TestCase
362	suggestion = profile.profile_suggestions.create(:suggestion => community)	362	suggestion = profile.profile_suggestions.create(:suggestion => community)
363	post :remove_suggestion, :profile => 'testuser', :id => community.identifier	363	post :remove_suggestion, :profile => 'testuser', :id => community.identifier
364		364
365	- assert_redirected_to :action => 'suggest'	365	+ assert_response :success
366	assert_equal false, ProfileSuggestion.find(suggestion.id).enabled	366	assert_equal false, ProfileSuggestion.find(suggestion.id).enabled
367	end	367	end
368		368