From 9372126ab1bb1f7d85cc2ebf17eb5fabd49880bd Mon Sep 17 00:00:00 2001 From: Daniela Feitosa Date: Thu, 18 Sep 2014 08:11:12 -0300 Subject: [PATCH] [suggestions] Some enhancements --- app/helpers/application_helper.rb | 4 ++-- app/models/communities_block.rb | 1 + app/views/blocks/communities.html.erb | 4 +--- app/views/shared/_profile_connections.html.erb | 31 +++++++++++++++++-------------- app/views/shared/_profile_suggestions_list.html.erb | 26 +++++++++++++++++++------- public/javascripts/add-and-join.js | 31 +++++++++++++++---------------- public/stylesheets/application.css | 3 +-- test/functional/friends_controller_test.rb | 12 +++--------- test/functional/memberships_controller_test.rb | 6 +++--- test/unit/communities_block_test.rb | 18 +++++++++--------- 10 files changed, 71 insertions(+), 65 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 1036a64..8f638c3 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.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) diff --git a/app/models/communities_block.rb b/app/models/communities_block.rb index 2e8a7b0..1f2b9cc 100644 --- a/app/models/communities_block.rb +++ b/app/models/communities_block.rb @@ -22,6 +22,7 @@ class CommunitiesBlock < 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 diff --git a/app/views/blocks/communities.html.erb b/app/views/blocks/communities.html.erb index 08a74a1..45a467a 100644 --- a/app/views/blocks/communities.html.erb +++ b/app/views/blocks/communities.html.erb @@ -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? %>

<%= _('Some suggestions for you') %>

diff --git a/app/views/shared/_profile_connections.html.erb b/app/views/shared/_profile_connections.html.erb index 8ee0b09..9fa78e4 100644 --- a/app/views/shared/_profile_connections.html.erb +++ b/app/views/shared/_profile_connections.html.erb @@ -1,20 +1,23 @@ -

<%= _("Profiles in common:") if profiles.present? %>

-
-
    - <% profiles.each do |profile| %> -
  • - <%= link_to_profile profile_image(profile) + '
    ' + profile.short_name, - profile.identifier, :class => 'profile-link' %> -
  • - <% end %> -
-
- -
+<% if profiles.present? %> +

<%= _("Profiles in common:") %>

+
+
    + <% profiles.each do |profile| %> +
  • + <%= link_to_profile profile_image(profile) + '
    ' + profile.short_name, + profile.identifier, :class => 'profile-link' %> +
  • + <% end %> +
+
+
+<% end %> -

<%= _("Tags in common:") if tags.present? %>

+<% if tags.present? %> +

<%= _("Tags in common:") %>

    <% tags.each do |tag| %>
  • <%= tag.name %>
  • <% end %>
+<% end %> diff --git a/app/views/shared/_profile_suggestions_list.html.erb b/app/views/shared/_profile_suggestions_list.html.erb index 104976e..d4737aa 100644 --- a/app/views/shared/_profile_suggestions_list.html.erb +++ b/app/views/shared/_profile_suggestions_list.html.erb @@ -10,14 +10,22 @@
    <% suggestions.each do |s| %>
  • - <%= 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 %>
    <%= 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 %>
  • diff --git a/public/javascripts/add-and-join.js b/public/javascripts/add-and-join.js index adbf579..3c90af2 100644 --- a/public/javascripts/add-and-join.js +++ b/public/javascripts/add-and-join.js @@ -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); + }); +} diff --git a/public/stylesheets/application.css b/public/stylesheets/application.css index 5105891..4e2b86f 100644 --- a/public/stylesheets/application.css +++ b/public/stylesheets/application.css @@ -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 } diff --git a/test/functional/friends_controller_test.rb b/test/functional/friends_controller_test.rb index 1271584..18f73b7 100644 --- a/test/functional/friends_controller_test.rb +++ b/test/functional/friends_controller_test.rb @@ -75,16 +75,10 @@ class FriendsControllerTest < 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 < 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 diff --git a/test/functional/memberships_controller_test.rb b/test/functional/memberships_controller_test.rb index 9b9f9ba..4047a34 100644 --- a/test/functional/memberships_controller_test.rb +++ b/test/functional/memberships_controller_test.rb @@ -340,7 +340,7 @@ class MembershipsControllerTest < 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 < 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 < 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 diff --git a/test/unit/communities_block_test.rb b/test/unit/communities_block_test.rb index 472199f..7a05808 100644 --- a/test/unit/communities_block_test.rb +++ b/test/unit/communities_block_test.rb @@ -27,30 +27,30 @@ class CommunitiesBlockTest < 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 -- libgit2 0.21.2