Commit ce07545086402039cfd914577efb8728d0488687
1 parent
32d3f0fb
Exists in
web_steps_improvements
and in
9 other branches
Refactor CommunitiesBlock footer into helper/view
This is one more step into decoupling HTML generation from models. All models with such method are expected to go through this refactor.
Showing
5 changed files
with
74 additions
and
48 deletions
Show diff stats
app/helpers/boxes_helper.rb
| @@ -97,10 +97,20 @@ module BoxesHelper | @@ -97,10 +97,20 @@ module BoxesHelper | ||
| 97 | end | 97 | end |
| 98 | end | 98 | end |
| 99 | 99 | ||
| 100 | + def render_block_footer(block) | ||
| 101 | + template_name = block.class.name.underscore.gsub('_block', '') | ||
| 102 | + template_filename = "#{template_name}.html.erb" | ||
| 103 | + if File.exists? Rails.root.join('app', 'views', 'blocks', 'footers', template_filename) | ||
| 104 | + render :file => "blocks/footers/#{template_name}", :locals => { :block => block } | ||
| 105 | + else | ||
| 106 | + nil | ||
| 107 | + end | ||
| 108 | + end | ||
| 109 | + | ||
| 100 | def display_block_content(block, main_content = nil) | 110 | def display_block_content(block, main_content = nil) |
| 101 | content = block.main? ? wrap_main_content(main_content) : render_block_content(block) | 111 | content = block.main? ? wrap_main_content(main_content) : render_block_content(block) |
| 102 | result = extract_block_content(content) | 112 | result = extract_block_content(content) |
| 103 | - footer_content = extract_block_content(block.footer) | 113 | + footer_content = extract_block_content(block.respond_to?(:footer) ? block.footer : render_block_footer(block)) # FIXME: this ternary conditional should be removed after all block footer methods get refatored into helpers and views |
| 104 | unless footer_content.blank? | 114 | unless footer_content.blank? |
| 105 | footer_content = content_tag('div', footer_content, :class => 'block-footer-content' ) | 115 | footer_content = content_tag('div', footer_content, :class => 'block-footer-content' ) |
| 106 | end | 116 | end |
app/models/communities_block.rb
| @@ -27,15 +27,6 @@ class CommunitiesBlock < ProfileListBlock | @@ -27,15 +27,6 @@ class CommunitiesBlock < ProfileListBlock | ||
| 27 | owner.profile_suggestions.of_community.enabled.limit(3).includes(:suggestion) | 27 | owner.profile_suggestions.of_community.enabled.limit(3).includes(:suggestion) |
| 28 | end | 28 | end |
| 29 | 29 | ||
| 30 | - def footer | ||
| 31 | - owner = self.owner | ||
| 32 | - suggestions = self.suggestions | ||
| 33 | - return '' unless owner.kind_of?(Profile) || owner.kind_of?(Environment) | ||
| 34 | - proc do | ||
| 35 | - render :file => 'blocks/communities_footer', :locals => { :owner => owner, :suggestions => suggestions } | ||
| 36 | - end | ||
| 37 | - end | ||
| 38 | - | ||
| 39 | def profiles | 30 | def profiles |
| 40 | owner.communities | 31 | owner.communities |
| 41 | end | 32 | end |
app/views/blocks/communities_footer.html.erb
| @@ -1,17 +0,0 @@ | @@ -1,17 +0,0 @@ | ||
| 1 | -<% if owner.kind_of?(Profile) %> | ||
| 2 | - <%= link_to s_('communities|View all'), {:profile => owner.identifier, :controller => 'profile', :action => 'communities'}, :class => 'view-all' %> | ||
| 3 | -<% elsif owner.kind_of?(Environment) %> | ||
| 4 | - <%= link_to s_('communities|View all'), {:controller => 'search', :action => 'communities'}, :class => 'view-all' %> | ||
| 5 | -<% end %> | ||
| 6 | - | ||
| 7 | -<% if user && user == profile && suggestions && !suggestions.empty? %> | ||
| 8 | - <div class='suggestions-block common-profile-list-block'> | ||
| 9 | - <h4 class='block-subtitle'><%= _('Some suggestions for you') %></h4> | ||
| 10 | - <div class='profiles-suggestions'> | ||
| 11 | - <%= render :partial => 'shared/profile_suggestions_list', :locals => { :suggestions => suggestions, :collection => :communities_suggestions, :per_page => 3 } %> | ||
| 12 | - </div> | ||
| 13 | - <div class='more-suggestions'> | ||
| 14 | - <%= link_to _('See all suggestions'), profile.communities_suggestions_url %> | ||
| 15 | - </div> | ||
| 16 | - </div> | ||
| 17 | -<% end %> |
| @@ -0,0 +1,19 @@ | @@ -0,0 +1,19 @@ | ||
| 1 | +<% if block.owner.kind_of?(Profile) || block.owner.kind_of?(Environment) %> | ||
| 2 | + <% if block.owner.kind_of?(Profile) %> | ||
| 3 | + <%= link_to s_('communities|View all'), {:profile => block.owner.identifier, :controller => 'profile', :action => 'communities'}, :class => 'view-all' %> | ||
| 4 | + <% elsif block.owner.kind_of?(Environment) %> | ||
| 5 | + <%= link_to s_('communities|View all'), {:controller => 'search', :action => 'communities'}, :class => 'view-all' %> | ||
| 6 | + <% end %> | ||
| 7 | + | ||
| 8 | + <% if user && user == profile && block.suggestions && !block.suggestions.empty? %> | ||
| 9 | + <div class='suggestions-block common-profile-list-block'> | ||
| 10 | + <h4 class='block-subtitle'><%= _('Some suggestions for you') %></h4> | ||
| 11 | + <div class='profiles-suggestions'> | ||
| 12 | + <%= render :partial => 'shared/profile_suggestions_list', :locals => { :suggestions => block.suggestions, :collection => :communities_suggestions, :per_page => 3 } %> | ||
| 13 | + </div> | ||
| 14 | + <div class='more-suggestions'> | ||
| 15 | + <%= link_to _('See all suggestions'), profile.communities_suggestions_url %> | ||
| 16 | + </div> | ||
| 17 | + </div> | ||
| 18 | + <% end %> | ||
| 19 | +<% end %> |
test/unit/communities_block_test.rb
| @@ -27,14 +27,44 @@ class CommunitiesBlockTest < ActiveSupport::TestCase | @@ -27,14 +27,44 @@ class CommunitiesBlockTest < ActiveSupport::TestCase | ||
| 27 | assert_same list, block.profiles | 27 | assert_same list, block.profiles |
| 28 | end | 28 | end |
| 29 | 29 | ||
| 30 | + should 'list non-public communities' do | ||
| 31 | + user = create_user('testuser').person | ||
| 32 | + | ||
| 33 | + public_community = fast_create(Community, :environment_id => Environment.default.id) | ||
| 34 | + public_community.add_member(user) | ||
| 35 | + | ||
| 36 | + private_community = fast_create(Community, :environment_id => Environment.default.id, :public_profile => false) | ||
| 37 | + private_community.add_member(user) | ||
| 38 | + | ||
| 39 | + block = CommunitiesBlock.new | ||
| 40 | + block.expects(:owner).at_least_once.returns(user) | ||
| 41 | + | ||
| 42 | + assert_equivalent [public_community, private_community], block.profiles | ||
| 43 | + end | ||
| 44 | + | ||
| 45 | +end | ||
| 46 | + | ||
| 47 | +require 'boxes_helper' | ||
| 48 | + | ||
| 49 | +class CommunitiesBlockViewTest < ActionView::TestCase | ||
| 50 | + include BoxesHelper | ||
| 51 | + | ||
| 30 | should 'support profile as block owner' do | 52 | should 'support profile as block owner' do |
| 31 | profile = Profile.new | 53 | profile = Profile.new |
| 54 | + profile.identifier = 42 | ||
| 55 | + | ||
| 56 | + ActionView::Base.any_instance.stubs(:user).with(anything).returns(profile) | ||
| 57 | + ActionView::Base.any_instance.stubs(:profile).with(anything).returns(profile) | ||
| 32 | 58 | ||
| 33 | block = CommunitiesBlock.new | 59 | block = CommunitiesBlock.new |
| 34 | block.expects(:owner).returns(profile).at_least_once | 60 | block.expects(:owner).returns(profile).at_least_once |
| 35 | 61 | ||
| 36 | - self.expects(:render).with(:file => 'blocks/communities_footer', :locals => { :owner => profile, :suggestions => block.suggestions }) | ||
| 37 | - instance_eval(&block.footer) | 62 | + footer = render_block_footer(block) |
| 63 | + | ||
| 64 | + assert_tag_in_string footer, tag: 'a', attributes: {href: '/profile/42/communities'} | ||
| 65 | + | ||
| 66 | + ActionView::Base.any_instance.unstub(:user) | ||
| 67 | + ActionView::Base.any_instance.unstub(:profile) | ||
| 38 | end | 68 | end |
| 39 | 69 | ||
| 40 | should 'support environment as block owner' do | 70 | should 'support environment as block owner' do |
| @@ -42,31 +72,24 @@ class CommunitiesBlockTest < ActiveSupport::TestCase | @@ -42,31 +72,24 @@ class CommunitiesBlockTest < ActiveSupport::TestCase | ||
| 42 | block = CommunitiesBlock.new | 72 | block = CommunitiesBlock.new |
| 43 | block.expects(:owner).returns(env).at_least_once | 73 | block.expects(:owner).returns(env).at_least_once |
| 44 | 74 | ||
| 45 | - self.expects(:render).with(:file => 'blocks/communities_footer', :locals => { :owner => env, :suggestions => block.suggestions }) | ||
| 46 | - instance_eval(&block.footer) | ||
| 47 | - end | ||
| 48 | - | ||
| 49 | - should 'give empty footer on unsupported owner type' do | ||
| 50 | - block = CommunitiesBlock.new | ||
| 51 | - block.expects(:owner).returns(1).at_least_once | 75 | + profile = Profile.new |
| 76 | + profile.identifier = 42 | ||
| 52 | 77 | ||
| 53 | - self.expects(:render).with(anything).never | ||
| 54 | - assert_equal '', block.footer | ||
| 55 | - end | 78 | + ActionView::Base.any_instance.stubs(:user).with(anything).returns(profile) |
| 79 | + ActionView::Base.any_instance.stubs(:profile).with(anything).returns(profile) | ||
| 56 | 80 | ||
| 57 | - should 'list non-public communities' do | ||
| 58 | - user = create_user('testuser').person | 81 | + footer = render_block_footer(block) |
| 59 | 82 | ||
| 60 | - public_community = fast_create(Community, :environment_id => Environment.default.id) | ||
| 61 | - public_community.add_member(user) | 83 | + assert_tag_in_string footer, tag: 'a', attributes: {href: '/search/communities'} |
| 62 | 84 | ||
| 63 | - private_community = fast_create(Community, :environment_id => Environment.default.id, :public_profile => false) | ||
| 64 | - private_community.add_member(user) | 85 | + ActionView::Base.any_instance.unstub(:user) |
| 86 | + ActionView::Base.any_instance.unstub(:profile) | ||
| 87 | + end | ||
| 65 | 88 | ||
| 89 | + should 'give empty footer on unsupported owner type' do | ||
| 66 | block = CommunitiesBlock.new | 90 | block = CommunitiesBlock.new |
| 67 | - block.expects(:owner).at_least_once.returns(user) | 91 | + block.expects(:owner).returns(1).at_least_once |
| 68 | 92 | ||
| 69 | - assert_equivalent [public_community, private_community], block.profiles | 93 | + assert_equal '', render_block_footer(block) |
| 70 | end | 94 | end |
| 71 | - | ||
| 72 | end | 95 | end |