Commit d53e71f1d625bb9cc9e6756dc7dd297d9259bff0
1 parent
7024df4e
Exists in
web_steps_improvements
and in
8 other branches
Remove PeopleBlock#footer
This method was producing view code from within the model which is a violation of MVC. Part of the code was extracted into the new helper and most of it became a view. This follow the standard for using BoxesHelper#render_block_footer.
Showing
9 changed files
with
78 additions
and
158 deletions
Show diff stats
plugins/people_block/lib/friends_block.rb
@@ -20,14 +20,6 @@ class FriendsBlock < PeopleBlockBase | @@ -20,14 +20,6 @@ class FriendsBlock < PeopleBlockBase | ||
20 | owner.suggested_profiles.of_person.enabled.limit(3).includes(:suggestion) | 20 | owner.suggested_profiles.of_person.enabled.limit(3).includes(:suggestion) |
21 | end | 21 | end |
22 | 22 | ||
23 | - def footer | ||
24 | - profile = self.owner | ||
25 | - suggestions = self.suggestions | ||
26 | - proc do | ||
27 | - render :file => 'blocks/footers/friends', :locals => { :profile => profile, :suggestions => suggestions } | ||
28 | - end | ||
29 | - end | ||
30 | - | ||
31 | def self.expire_on | 23 | def self.expire_on |
32 | { :profile => [:profile] } | 24 | { :profile => [:profile] } |
33 | end | 25 | end |
plugins/people_block/lib/members_block.rb
@@ -20,15 +20,6 @@ class MembersBlock < PeopleBlockBase | @@ -20,15 +20,6 @@ class MembersBlock < PeopleBlockBase | ||
20 | role ? owner.members.with_role(role.id) : owner.members | 20 | role ? owner.members.with_role(role.id) : owner.members |
21 | end | 21 | end |
22 | 22 | ||
23 | - def footer | ||
24 | - profile = self.owner | ||
25 | - role_key = visible_role | ||
26 | - s = show_join_leave_button | ||
27 | - proc do | ||
28 | - render :file => 'blocks/footers/members', :locals => { :profile => profile, :show_join_leave_button => s, :role_key => role_key} | ||
29 | - end | ||
30 | - end | ||
31 | - | ||
32 | def role | 23 | def role |
33 | visible_role && !visible_role.empty? ? Role.find_by_key_and_environment_id(visible_role, owner.environment) : nil | 24 | visible_role && !visible_role.empty? ? Role.find_by_key_and_environment_id(visible_role, owner.environment) : nil |
34 | end | 25 | end |
plugins/people_block/lib/people_block.rb
@@ -15,11 +15,4 @@ class PeopleBlock < PeopleBlockBase | @@ -15,11 +15,4 @@ class PeopleBlock < PeopleBlockBase | ||
15 | def profiles | 15 | def profiles |
16 | owner.people | 16 | owner.people |
17 | end | 17 | end |
18 | - | ||
19 | - def footer | ||
20 | - proc do | ||
21 | - render :file => 'blocks/footers/people' | ||
22 | - end | ||
23 | - end | ||
24 | - | ||
25 | end | 18 | end |
plugins/people_block/lib/people_block_base.rb
@@ -42,58 +42,6 @@ class PeopleBlockBase < Block | @@ -42,58 +42,6 @@ class PeopleBlockBase < Block | ||
42 | profiles.visible.count | 42 | profiles.visible.count |
43 | end | 43 | end |
44 | 44 | ||
45 | -=begin | ||
46 | - def content(args={}) | ||
47 | - profiles = self.profile_list | ||
48 | - title = self.view_title | ||
49 | - | ||
50 | - if !self.name.blank? && !self.address.blank? | ||
51 | - name = self.name | ||
52 | - expanded_address = expand_address(self.address) | ||
53 | - end | ||
54 | - | ||
55 | - proc do | ||
56 | - count = 0 | ||
57 | - list = profiles.map {|item| | ||
58 | - count += 1 | ||
59 | - send(:profile_image_link, item, :minor ) | ||
60 | - }.join("\n") | ||
61 | - if list.empty? | ||
62 | - list = content_tag 'div', c_('None'), :class => 'common-profile-list-block-none' | ||
63 | - else | ||
64 | - if !name.blank? && !expanded_address.blank? | ||
65 | - list << content_tag( | ||
66 | - 'div', | ||
67 | - content_tag( | ||
68 | - 'li', | ||
69 | - content_tag( | ||
70 | - 'div', | ||
71 | - link_to( | ||
72 | - content_tag('span', name, :class => 'banner-span' ), | ||
73 | - expanded_address, | ||
74 | - :title => name | ||
75 | - ), | ||
76 | - :class => 'banner-div' | ||
77 | - ), | ||
78 | - :class => 'vcard' | ||
79 | - ), | ||
80 | - :class => 'common-profile-list-block' | ||
81 | - ) | ||
82 | - end | ||
83 | - list = content_tag 'ul', list | ||
84 | - end | ||
85 | - block_title(title) + content_tag('div', list + tag('br', :style => 'clear:both')) | ||
86 | - end | ||
87 | - end | ||
88 | -=end | ||
89 | - def expand_address(address) | ||
90 | - if address !~ /^[a-z]+:\/\// && address !~ /^\// | ||
91 | - 'http://' + address | ||
92 | - else | ||
93 | - address | ||
94 | - end | ||
95 | - end | ||
96 | - | ||
97 | def extra_option | 45 | def extra_option |
98 | { } | 46 | { } |
99 | end | 47 | end |
plugins/people_block/test/unit/friends_block_test.rb
@@ -60,19 +60,6 @@ class FriendsBlockTest < ActionView::TestCase | @@ -60,19 +60,6 @@ class FriendsBlockTest < ActionView::TestCase | ||
60 | assert_equal 20, block.limit | 60 | assert_equal 20, block.limit |
61 | end | 61 | end |
62 | 62 | ||
63 | - should 'link to "all friends"' do | ||
64 | - person1 = create_user('mytestperson').person | ||
65 | - | ||
66 | - block = FriendsBlock.new | ||
67 | - block.stubs(:suggestions).returns([]) | ||
68 | - block.expects(:owner).returns(person1).at_least_once | ||
69 | - | ||
70 | - instance_eval(&block.footer) | ||
71 | - assert_select 'a.view-all' do |elements| | ||
72 | - assert_select '[href=/profile/mytestperson/friends]' | ||
73 | - end | ||
74 | - end | ||
75 | - | ||
76 | should 'count number of owner friends' do | 63 | should 'count number of owner friends' do |
77 | owner = fast_create(Person) | 64 | owner = fast_create(Person) |
78 | friend1 = fast_create(Person) | 65 | friend1 = fast_create(Person) |
@@ -156,4 +143,14 @@ class FriendsBlockViewTest < ActionView::TestCase | @@ -156,4 +143,14 @@ class FriendsBlockViewTest < ActionView::TestCase | ||
156 | assert_match(/#{friend1.name}/, content) | 143 | assert_match(/#{friend1.name}/, content) |
157 | assert_match(/#{friend2.name}/, content) | 144 | assert_match(/#{friend2.name}/, content) |
158 | end | 145 | end |
146 | + | ||
147 | + should 'link to "all friends"' do | ||
148 | + person1 = create_user('mytestperson').person | ||
149 | + | ||
150 | + block = FriendsBlock.new | ||
151 | + block.stubs(:suggestions).returns([]) | ||
152 | + block.expects(:owner).returns(person1).at_least_once | ||
153 | + | ||
154 | + assert_tag_in_string render_block_footer(block), tag: 'a', attributes: {class: 'view-all', href: '/profile/mytestperson/friends' } | ||
155 | + end | ||
159 | end | 156 | end |
plugins/people_block/test/unit/members_block_test.rb
@@ -119,57 +119,6 @@ class MembersBlockTest < ActionView::TestCase | @@ -119,57 +119,6 @@ class MembersBlockTest < ActionView::TestCase | ||
119 | assert_equal 1, block.profile_count | 119 | assert_equal 1, block.profile_count |
120 | end | 120 | end |
121 | 121 | ||
122 | - should 'provide link to members page without a visible_role selected' do | ||
123 | - profile = create_user('mytestuser').person | ||
124 | - block = MembersBlock.new | ||
125 | - block.box = profile.boxes.first | ||
126 | - block.save! | ||
127 | - | ||
128 | - instance_eval(&block.footer) | ||
129 | - assert_select 'a.view-all' do |elements| | ||
130 | - assert_select "[href=/profile/mytestuser/members#members-tab]" | ||
131 | - end | ||
132 | - end | ||
133 | - | ||
134 | - should 'provide link to members page when visible_role is profile_member' do | ||
135 | - profile = create_user('mytestuser').person | ||
136 | - block = MembersBlock.new | ||
137 | - block.box = profile.boxes.first | ||
138 | - block.visible_role = 'profile_member' | ||
139 | - block.save! | ||
140 | - | ||
141 | - instance_eval(&block.footer) | ||
142 | - assert_select 'a.view-all' do |elements| | ||
143 | - assert_select '[href=/profile/mytestuser/members#members-tab]' | ||
144 | - end | ||
145 | - end | ||
146 | - | ||
147 | - should 'provide link to members page when visible_role is profile_moderator' do | ||
148 | - profile = create_user('mytestuser').person | ||
149 | - block = MembersBlock.new | ||
150 | - block.box = profile.boxes.first | ||
151 | - block.visible_role = 'profile_moderator' | ||
152 | - block.save! | ||
153 | - | ||
154 | - instance_eval(&block.footer) | ||
155 | - assert_select 'a.view-all' do |elements| | ||
156 | - assert_select '[href=/profile/mytestuser/members#members-tab]' | ||
157 | - end | ||
158 | - end | ||
159 | - | ||
160 | - should 'provide link to admins page when visible_role is profile_admin' do | ||
161 | - profile = create_user('mytestuser').person | ||
162 | - block = MembersBlock.new | ||
163 | - block.box = profile.boxes.first | ||
164 | - block.visible_role = 'profile_admin' | ||
165 | - block.save! | ||
166 | - | ||
167 | - instance_eval(&block.footer) | ||
168 | - assert_select 'a.view-all' do |elements| | ||
169 | - assert_select '[href=/profile/mytestuser/members#admins-tab]' | ||
170 | - end | ||
171 | - end | ||
172 | - | ||
173 | should 'provide a role to be displayed (and default to nil)' do | 122 | should 'provide a role to be displayed (and default to nil)' do |
174 | env = fast_create(Environment) | 123 | env = fast_create(Environment) |
175 | env.boxes << Box.new | 124 | env.boxes << Box.new |
@@ -306,4 +255,55 @@ class MembersBlockViewTest < ActionView::TestCase | @@ -306,4 +255,55 @@ class MembersBlockViewTest < ActionView::TestCase | ||
306 | assert_match(/#{person1.name}/, content) | 255 | assert_match(/#{person1.name}/, content) |
307 | assert_match(/#{person2.name}/, content) | 256 | assert_match(/#{person2.name}/, content) |
308 | end | 257 | end |
258 | + | ||
259 | + should 'provide link to members page without a visible_role selected' do | ||
260 | + profile = create_user('mytestuser').person | ||
261 | + block = MembersBlock.new | ||
262 | + block.box = profile.boxes.first | ||
263 | + block.save! | ||
264 | + | ||
265 | + render_block_footer(block) | ||
266 | + assert_select 'a.view-all' do |elements| | ||
267 | + assert_select "[href=/profile/mytestuser/members#members-tab]" | ||
268 | + end | ||
269 | + end | ||
270 | + | ||
271 | + should 'provide link to members page when visible_role is profile_member' do | ||
272 | + profile = create_user('mytestuser').person | ||
273 | + block = MembersBlock.new | ||
274 | + block.box = profile.boxes.first | ||
275 | + block.visible_role = 'profile_member' | ||
276 | + block.save! | ||
277 | + | ||
278 | + render_block_footer(block) | ||
279 | + assert_select 'a.view-all' do |elements| | ||
280 | + assert_select '[href=/profile/mytestuser/members#members-tab]' | ||
281 | + end | ||
282 | + end | ||
283 | + | ||
284 | + should 'provide link to members page when visible_role is profile_moderator' do | ||
285 | + profile = create_user('mytestuser').person | ||
286 | + block = MembersBlock.new | ||
287 | + block.box = profile.boxes.first | ||
288 | + block.visible_role = 'profile_moderator' | ||
289 | + block.save! | ||
290 | + | ||
291 | + render_block_footer(block) | ||
292 | + assert_select 'a.view-all' do |elements| | ||
293 | + assert_select '[href=/profile/mytestuser/members#members-tab]' | ||
294 | + end | ||
295 | + end | ||
296 | + | ||
297 | + should 'provide link to admins page when visible_role is profile_admin' do | ||
298 | + profile = create_user('mytestuser').person | ||
299 | + block = MembersBlock.new | ||
300 | + block.box = profile.boxes.first | ||
301 | + block.visible_role = 'profile_admin' | ||
302 | + block.save! | ||
303 | + | ||
304 | + render_block_footer(block) | ||
305 | + assert_select 'a.view-all' do |elements| | ||
306 | + assert_select '[href=/profile/mytestuser/members#admins-tab]' | ||
307 | + end | ||
308 | + end | ||
309 | end | 309 | end |
plugins/people_block/test/unit/people_block_test.rb
@@ -85,17 +85,6 @@ class PeopleBlockTest < ActionView::TestCase | @@ -85,17 +85,6 @@ class PeopleBlockTest < ActionView::TestCase | ||
85 | end | 85 | end |
86 | 86 | ||
87 | 87 | ||
88 | - should 'link to "all people"' do | ||
89 | - env = fast_create(Environment) | ||
90 | - block = PeopleBlock.new | ||
91 | - | ||
92 | - instance_eval(&block.footer) | ||
93 | - assert_select 'a.view-all' do |elements| | ||
94 | - assert_select '[href=/search/people]' | ||
95 | - end | ||
96 | - end | ||
97 | - | ||
98 | - | ||
99 | should 'count number of public and private people' do | 88 | should 'count number of public and private people' do |
100 | owner = fast_create(Environment) | 89 | owner = fast_create(Environment) |
101 | private_p = fast_create(Person, :public_profile => false, :environment_id => owner.id) | 90 | private_p = fast_create(Person, :public_profile => false, :environment_id => owner.id) |
@@ -146,4 +135,14 @@ class PeopleBlockViewTest < ActionView::TestCase | @@ -146,4 +135,14 @@ class PeopleBlockViewTest < ActionView::TestCase | ||
146 | assert_match(/#{person1.name}/, content) | 135 | assert_match(/#{person1.name}/, content) |
147 | assert_match(/#{person2.name}/, content) | 136 | assert_match(/#{person2.name}/, content) |
148 | end | 137 | end |
138 | + | ||
139 | + should 'link to "all people"' do | ||
140 | + env = fast_create(Environment) | ||
141 | + block = PeopleBlock.new | ||
142 | + | ||
143 | + render_block_footer(block) | ||
144 | + assert_select 'a.view-all' do |elements| | ||
145 | + assert_select '[href=/search/people]' | ||
146 | + end | ||
147 | + end | ||
149 | end | 148 | end |
plugins/people_block/views/blocks/footers/friends.html.erb
1 | -<%= link_to s_('friends|View all'), {:profile => profile.identifier, :controller => 'profile', :action => 'friends'}, :class => 'view-all' %> | 1 | +<%= link_to s_('friends|View all'), {:profile => block.owner.identifier, :controller => 'profile', :action => 'friends'}, :class => 'view-all' %> |
2 | 2 | ||
3 | -<% if !suggestions.empty? && user == profile %> | 3 | +<% if !block.suggestions.empty? && user == block.owner %> |
4 | <div class='suggestions-block common-profile-list-block'> | 4 | <div class='suggestions-block common-profile-list-block'> |
5 | <h4 class='block-subtitle'><%= _('Some suggestions for you') %></h4> | 5 | <h4 class='block-subtitle'><%= _('Some suggestions for you') %></h4> |
6 | <div class='profiles-suggestions'> | 6 | <div class='profiles-suggestions'> |
7 | - <%= render :partial => 'shared/profile_suggestions_list', :locals => { :suggestions => suggestions, :collection => :friends_suggestions } %> | 7 | + <%= render :partial => 'shared/profile_suggestions_list', :locals => { :suggestions => block.suggestions, :collection => :friends_suggestions } %> |
8 | </div> | 8 | </div> |
9 | <div class='more-suggestions'> | 9 | <div class='more-suggestions'> |
10 | - <%= link_to _('See all suggestions'), profile.people_suggestions_url %> | 10 | + <%= link_to _('See all suggestions'), block.owner.people_suggestions_url %> |
11 | </div> | 11 | </div> |
12 | </div> | 12 | </div> |
13 | <% end %> | 13 | <% end %> |
plugins/people_block/views/blocks/footers/members.html.erb
1 | -<% anchor = role_key == "profile_admin" ? "admins-tab" : "members-tab" %> | ||
2 | -<%= link_to c_('View all'), {:profile => profile.identifier, :controller => 'profile', :action => 'members', :anchor =>anchor }, :class => 'view-all' %> | 1 | +<% anchor = block.visible_role == "profile_admin" ? "admins-tab" : "members-tab" %> |
2 | +<%= link_to c_('View all'), {:profile => block.owner.identifier, :controller => 'profile', :action => 'members', :anchor =>anchor }, :class => 'view-all' %> | ||
3 | 3 | ||
4 | -<% if show_join_leave_button %> | 4 | +<% if block.show_join_leave_button %> |
5 | <%= render :partial => 'blocks/profile_info_actions/join_leave_community' %> | 5 | <%= render :partial => 'blocks/profile_info_actions/join_leave_community' %> |
6 | <% end %> | 6 | <% end %> |