From 06d8c96d54b4c9c04790c3281c508ec2f652d001 Mon Sep 17 00:00:00 2001 From: David Gil Date: Tue, 11 Aug 2015 20:47:41 +0200 Subject: [PATCH 1/4] Limiting max number of tags shown on debate cards ~ issue#144 --- app/helpers/application_helper.rb | 12 ++++++++++-- app/models/debate.rb | 9 +++++++++ app/views/debates/_debate.html.erb | 2 +- app/views/shared/_tags.html.erb | 6 ++++-- app/views/welcome/_featured_debate.html.erb | 2 +- 5 files changed, 25 insertions(+), 6 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 70828c01a..046890df8 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -1,7 +1,15 @@ module ApplicationHelper - def tags(debate) - debate.tag_list.sort.map { |tag| link_to sanitize(tag), debates_path(tag: tag) }.join('').html_safe + def tags(debate, limit = nil) + tag_names = debate.tag_list_with_limit(limit) + + tag_names.sort.map do |tag| + link_to sanitize(tag), debates_path(tag: tag) + end.join('').html_safe.tap do |output| + if limit && extra_tags = debate.tags_count_out_of_limit(limit) + output.concat(link_to("#{extra_tags}+", debate_path(debate))) + end + end end def percentage(vote, debate) diff --git a/app/models/debate.rb b/app/models/debate.rb index 26fd2dbf0..2d9dab815 100644 --- a/app/models/debate.rb +++ b/app/models/debate.rb @@ -42,6 +42,15 @@ class Debate < ActiveRecord::Base super.try :html_safe end + def tag_list_with_limit(limit = nil) + tags.most_used(limit).pluck :name + end + + def tags_count_out_of_limit(limit = nil) + count = tags.count - limit + count < 0 ? 0 : count + end + protected def sanitize_description diff --git a/app/views/debates/_debate.html.erb b/app/views/debates/_debate.html.erb index f8343e8be..56a5127c5 100644 --- a/app/views/debates/_debate.html.erb +++ b/app/views/debates/_debate.html.erb @@ -15,7 +15,7 @@ <%= link_to debate.description, debate %>
- <%= render "shared/tags", debate: debate %> + <%= render "shared/tags", debate: debate, limit: 5 %> diff --git a/app/views/shared/_tags.html.erb b/app/views/shared/_tags.html.erb index 8be6da579..93c94b571 100644 --- a/app/views/shared/_tags.html.erb +++ b/app/views/shared/_tags.html.erb @@ -1,3 +1,5 @@ +<%- limit ||= nil %> + <% if debate.tags.any? %> - <%= tags(debate) %> -<% end %> \ No newline at end of file + <%= tags(debate, limit) %> +<% end %> diff --git a/app/views/welcome/_featured_debate.html.erb b/app/views/welcome/_featured_debate.html.erb index 93626b4c7..a1992e337 100644 --- a/app/views/welcome/_featured_debate.html.erb +++ b/app/views/welcome/_featured_debate.html.erb @@ -14,7 +14,7 @@ <%= link_to featured_debate.description, featured_debate %>
- <%= render "shared/tags", debate: featured_debate %> + <%= render "shared/tags", debate: featured_debate, limit: 5 %>
From 41e5496d2b5dc31fdac4fa5949c126d9b97d1666 Mon Sep 17 00:00:00 2001 From: David Gil Date: Tue, 11 Aug 2015 23:56:16 +0200 Subject: [PATCH 2/4] add a couple of tests for the tag limit feature --- spec/features/debates_spec.rb | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/spec/features/debates_spec.rb b/spec/features/debates_spec.rb index b0c7a3a60..ccbee0a93 100644 --- a/spec/features/debates_spec.rb +++ b/spec/features/debates_spec.rb @@ -130,4 +130,37 @@ feature 'Debates' do expect(page).to have_content "Let's..." end + describe 'Limiting tags shown' do + let(:all_tags) { + ["Hacienda", "Economía", "Medio Ambiente", "Corrupción", "Fiestas populares", "Prensa", "Huelgas"] + } + let(:debate) { + create :debate, tag_list: all_tags + } + + scenario 'Index page show up to 5 tags per debate' do + debate + visible_tags = ["Medio Ambiente", "Corrupción", "Fiestas populares", "Prensa", "Huelgas"] + + visit debates_path + + expect(page).to have_selector('.debate', count: 1) + within('.debate') do + visible_tags.each do |tag| + expect(page).to have_content tag + end + expect(page).to have_content '2+' + end + end + + scenario 'Debate#show shows the full tag list' do + visit debate_path(debate) + + within("#debate-#{debate.id}") do + all_tags.each do |tag| + expect(page).to have_content tag + end + end + end + end end From 9a1bdb7aca088de52f0a32b55f0020074b70ed69 Mon Sep 17 00:00:00 2001 From: David Gil Date: Wed, 12 Aug 2015 16:24:35 +0200 Subject: [PATCH 3/4] move tags helper to view, fix case when less than 5 tags were present, tag list was showing a + link --- app/helpers/application_helper.rb | 13 ------------- app/models/debate.rb | 8 ++++++-- app/views/shared/_tags.html.erb | 10 +++++++++- spec/features/debates_spec.rb | 19 ++++++++++++++++--- 4 files changed, 31 insertions(+), 19 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 046890df8..cee6db199 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -1,17 +1,4 @@ module ApplicationHelper - - def tags(debate, limit = nil) - tag_names = debate.tag_list_with_limit(limit) - - tag_names.sort.map do |tag| - link_to sanitize(tag), debates_path(tag: tag) - end.join('').html_safe.tap do |output| - if limit && extra_tags = debate.tags_count_out_of_limit(limit) - output.concat(link_to("#{extra_tags}+", debate_path(debate))) - end - end - end - def percentage(vote, debate) return "0%" if debate.total_votes == 0 debate.send(vote).percent_of(debate.total_votes).to_s + "%" diff --git a/app/models/debate.rb b/app/models/debate.rb index 2d9dab815..aa7630cd5 100644 --- a/app/models/debate.rb +++ b/app/models/debate.rb @@ -47,8 +47,12 @@ class Debate < ActiveRecord::Base end def tags_count_out_of_limit(limit = nil) - count = tags.count - limit - count < 0 ? 0 : count + if limit + count = tags.count - limit + count < 0 ? 0 : count + else + 0 + end end protected diff --git a/app/views/shared/_tags.html.erb b/app/views/shared/_tags.html.erb index 93c94b571..6a61bd4b4 100644 --- a/app/views/shared/_tags.html.erb +++ b/app/views/shared/_tags.html.erb @@ -1,5 +1,13 @@ <%- limit ||= nil %> <% if debate.tags.any? %> - <%= tags(debate, limit) %> + + <% debate.tag_list_with_limit(limit).each do |tag| %> + <%= link_to sanitize(tag), debates_path(tag: tag) %> + <% end %> + + <% if debate.tags_count_out_of_limit(limit) > 0 %> + <%= link_to "#{debate.tags_count_out_of_limit(limit)}+", debate_path(debate) %> + <% end %> + <% end %> diff --git a/spec/features/debates_spec.rb b/spec/features/debates_spec.rb index ccbee0a93..de3e05943 100644 --- a/spec/features/debates_spec.rb +++ b/spec/features/debates_spec.rb @@ -138,14 +138,13 @@ feature 'Debates' do create :debate, tag_list: all_tags } - scenario 'Index page show up to 5 tags per debate' do + scenario 'Index page shows up to 5 tags per debate' do debate visible_tags = ["Medio Ambiente", "Corrupción", "Fiestas populares", "Prensa", "Huelgas"] visit debates_path - expect(page).to have_selector('.debate', count: 1) - within('.debate') do + within('.debate .tags') do visible_tags.each do |tag| expect(page).to have_content tag end @@ -153,6 +152,20 @@ feature 'Debates' do end end + scenario 'Index page shows 3 tags with no plus link' do + tag_list = ["Medio Ambiente", "Corrupción", "Fiestas populares"] + debate = create :debate, tag_list: tag_list + + visit debates_path + + within('.debate .tags') do + tag_list.each do |tag| + expect(page).to have_content tag + end + expect(page).not_to have_content '+' + end + end + scenario 'Debate#show shows the full tag list' do visit debate_path(debate) From 1227ce7c0802d9eabb01875d74231cdff17d10f6 Mon Sep 17 00:00:00 2001 From: David Gil Date: Wed, 12 Aug 2015 17:34:04 +0200 Subject: [PATCH 4/4] clean up tags_count_out_of_limit --- app/models/debate.rb | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/app/models/debate.rb b/app/models/debate.rb index aa7630cd5..fe75811fa 100644 --- a/app/models/debate.rb +++ b/app/models/debate.rb @@ -47,12 +47,10 @@ class Debate < ActiveRecord::Base end def tags_count_out_of_limit(limit = nil) - if limit - count = tags.count - limit - count < 0 ? 0 : count - else - 0 - end + return 0 unless limit + + count = tags.count - limit + count < 0 ? 0 : count end protected