Commit 2926f7ed17f5fad829c4724e47ebfc7b70fa47d8
Committed by
Marcos Pereira
1 parent
67887fc5
Exists in
stable-spb-1.5
Organization Ratings improvements
- adds rejected status message for ratings - adds ratings count to be displayed on average ratings block - adds tests to new features and fixed old ones Signed-off-by: Tallys Martins <tallysmartins@gmail.com> Signed-off-by: Gabriel Silva <grabriel93.silva@gmail.com> (cherry picked from commit c30065be6679c968b9c9edfca2bd60e428588a1e)
Showing
11 changed files
with
138 additions
and
56 deletions
Show diff stats
plugins/organization_ratings/features/rate_community.feature
@@ -19,7 +19,7 @@ Feature: rate_community | @@ -19,7 +19,7 @@ Feature: rate_community | ||
19 | And I am logged in as "joaosilva" | 19 | And I am logged in as "joaosilva" |
20 | 20 | ||
21 | @selenium | 21 | @selenium |
22 | - Scenario: display rate button inside average block | 22 | + Scenario: display rate button and total ratings inside average block |
23 | Given I am on mycommunity's homepage | 23 | Given I am on mycommunity's homepage |
24 | Then I should see "Rate this Community" within ".average-rating-block" | 24 | Then I should see "Rate this Community" within ".average-rating-block" |
25 | And I should see "Be the first to rate" within ".average-rating-block" | 25 | And I should see "Be the first to rate" within ".average-rating-block" |
@@ -34,3 +34,9 @@ Feature: rate_community | @@ -34,3 +34,9 @@ Feature: rate_community | ||
34 | When I follow "Rate this Community" | 34 | When I follow "Rate this Community" |
35 | Then I should see "Joao Silva" within ".star-profile-name" | 35 | Then I should see "Joao Silva" within ".star-profile-name" |
36 | And I should see Joao Silva's profile image | 36 | And I should see Joao Silva's profile image |
37 | + | ||
38 | + Scenario: display total ratings inside average block | ||
39 | + Given I am on mycommunity's homepage | ||
40 | + When I follow "Rate this Community" | ||
41 | + Then I follow "Save" | ||
42 | + Then I should see "(1)" within ".total-ratings" |
plugins/organization_ratings/lib/organization_rating.rb
@@ -13,25 +13,26 @@ class OrganizationRating < ActiveRecord::Base | @@ -13,25 +13,26 @@ class OrganizationRating < ActiveRecord::Base | ||
13 | validates :organization_id, :person_id, | 13 | validates :organization_id, :person_id, |
14 | :presence => true | 14 | :presence => true |
15 | 15 | ||
16 | - def display_moderation_message person | ||
17 | - if person.present? | ||
18 | - task_active? && (person.is_admin? || person == self.person || | 16 | + def display_full_info_to? person |
17 | + (person.is_admin? || person == self.person || | ||
19 | self.organization.is_admin?(person)) | 18 | self.organization.is_admin?(person)) |
20 | - end | ||
21 | end | 19 | end |
22 | 20 | ||
23 | - def task_active? | ||
24 | - tasks = CreateOrganizationRatingComment.where(:target_id => self.organization.id, | ||
25 | - :status => Task::Status::ACTIVE) | ||
26 | - tasks.detect {|t| t.organization_rating_id == self.id}.present? | 21 | + def task_status |
22 | + tasks = CreateOrganizationRatingComment.where(:target_id => self.organization.id, :requestor_id => self.person.id) | ||
23 | + task = tasks.detect{ |t| t.organization_rating_id == self.id } | ||
24 | + task.status if task.present? | ||
27 | end | 25 | end |
28 | 26 | ||
29 | - def self.average_rating organization_id | ||
30 | - average = OrganizationRating.where(organization_id: organization_id).average(:value) | 27 | + def self.statistics_for_profile organization |
28 | + ratings = OrganizationRating.where(organization_id: organization) | ||
29 | + average = ratings.average(:value) | ||
30 | + total = ratings.size | ||
31 | 31 | ||
32 | if average | 32 | if average |
33 | - (average - average.truncate) >= 0.5 ? average.ceil : average.floor | 33 | + average = (average - average.truncate) >= 0.5 ? average.ceil : average.floor |
34 | end | 34 | end |
35 | + { average: average, total: total } | ||
35 | end | 36 | end |
36 | 37 | ||
37 | end | 38 | end |
plugins/organization_ratings/lib/ratings_helper.rb
@@ -12,4 +12,14 @@ module RatingsHelper | @@ -12,4 +12,14 @@ module RatingsHelper | ||
12 | ratings = OrganizationRating.where(organization_id: profile_id).order("created_at DESC") | 12 | ratings = OrganizationRating.where(organization_id: profile_id).order("created_at DESC") |
13 | end | 13 | end |
14 | end | 14 | end |
15 | -end | ||
16 | \ No newline at end of file | 15 | \ No newline at end of file |
16 | + | ||
17 | + def status_message_for(person, rating) | ||
18 | + if person.present? && rating.display_full_info_to?(person) | ||
19 | + if(rating.task_status == Task::Status::ACTIVE) | ||
20 | + content_tag(:p, _("Report waiting for approval"), class: "moderation-msg") | ||
21 | + elsif(rating.task_status == Task::Status::CANCELLED) | ||
22 | + content_tag(:p, _("Report rejected"), class: "rejected-msg") | ||
23 | + end | ||
24 | + end | ||
25 | + end | ||
26 | +end |
plugins/organization_ratings/public/style.css
1 | .star-container { | 1 | .star-container { |
2 | width: 100%; | 2 | width: 100%; |
3 | - height: 20px; | 3 | + height: 22px; |
4 | } | 4 | } |
5 | 5 | ||
6 | .star-negative, .star-positive { | 6 | .star-negative, .star-positive { |
@@ -80,7 +80,16 @@ | @@ -80,7 +80,16 @@ | ||
80 | 80 | ||
81 | .organization-average-rating-container .star-container { | 81 | .organization-average-rating-container .star-container { |
82 | float: left; | 82 | float: left; |
83 | - width: 120px; | 83 | + display: inline-flex; |
84 | + width: auto; | ||
85 | + max-width: 190px; | ||
86 | + | ||
87 | +} | ||
88 | + | ||
89 | +.total-ratings { | ||
90 | + font-size: 16px; | ||
91 | + margin-left: 5px; | ||
92 | + margin-right: 5px; | ||
84 | } | 93 | } |
85 | 94 | ||
86 | .organization-average-rating-container .rate-this-organization { | 95 | .organization-average-rating-container .rate-this-organization { |
plugins/organization_ratings/test/functional/organization_ratings_plugin_profile_controller_test.rb
@@ -188,4 +188,24 @@ class OrganizationRatingsPluginProfileControllerTest < ActionController::TestCas | @@ -188,4 +188,24 @@ class OrganizationRatingsPluginProfileControllerTest < ActionController::TestCas | ||
188 | assert_no_tag :tag => 'p', :content => /Report waiting for approva/, :attributes => {:class =>/comment-rejected-msg/} | 188 | assert_no_tag :tag => 'p', :content => /Report waiting for approva/, :attributes => {:class =>/comment-rejected-msg/} |
189 | assert_tag :tag => 'p', :content => /comment accepted/, :attributes => {:class =>/comment-body/} | 189 | assert_tag :tag => 'p', :content => /comment accepted/, :attributes => {:class =>/comment-body/} |
190 | end | 190 | end |
191 | + | ||
192 | + test "should display ratings count in average block" do | ||
193 | + average_block = AverageRatingBlock.new | ||
194 | + average_block.box = @community.boxes.find_by_position(1) | ||
195 | + average_block.save! | ||
196 | + | ||
197 | + OrganizationRating.stubs(:statistics_for_profile).returns({:average => 5, :total => 42}) | ||
198 | + get :new_rating, profile: @community.identifier | ||
199 | + assert_tag :tag => 'a', :content => /\(42\)/ | ||
200 | + end | ||
201 | + | ||
202 | + test "should display maximum ratings count in average block" do | ||
203 | + average_block = AverageRatingBlock.new | ||
204 | + average_block.box = @community.boxes.find_by_position(1) | ||
205 | + average_block.save! | ||
206 | + | ||
207 | + OrganizationRating.stubs(:statistics_for_profile).returns({:average => 5, :total => 999000}) | ||
208 | + get :new_rating, profile: @community.identifier | ||
209 | + assert_tag :tag => 'a', :content => /\(10000\+\)/ | ||
210 | + end | ||
191 | end | 211 | end |
plugins/organization_ratings/test/unit/organization_rating_test.rb
@@ -35,55 +35,36 @@ class OrganizationRatingTest < ActiveSupport::TestCase | @@ -35,55 +35,36 @@ class OrganizationRatingTest < ActiveSupport::TestCase | ||
35 | assert_equal false, organization_rating2.errors[:value].include?("must be between 1 and 5") | 35 | assert_equal false, organization_rating2.errors[:value].include?("must be between 1 and 5") |
36 | end | 36 | end |
37 | 37 | ||
38 | - test "false return when no active tasks for an Organization Rating" do | ||
39 | - assert_not @rating.task_active? | ||
40 | - end | ||
41 | - | ||
42 | - test "true return when an active task exists for an Organization Rating" do | 38 | + test "return rating task status" do |
43 | CreateOrganizationRatingComment.create!( | 39 | CreateOrganizationRatingComment.create!( |
44 | :organization_rating_id => @rating.id, | 40 | :organization_rating_id => @rating.id, |
45 | :target => @community, | 41 | :target => @community, |
46 | :requestor => @person) | 42 | :requestor => @person) |
47 | 43 | ||
48 | - assert_equal Task::Status::ACTIVE, CreateOrganizationRatingComment.last.status | ||
49 | - assert @rating.task_active? | 44 | + assert_equal Task::Status::ACTIVE, @rating.task_status |
50 | end | 45 | end |
51 | 46 | ||
52 | - test "return false when an cancelled task exists for an Organization Rating" do | 47 | + test "return rating task status when task is cancelled" do |
53 | CreateOrganizationRatingComment.create!( | 48 | CreateOrganizationRatingComment.create!( |
54 | :organization_rating_id => @rating.id, | 49 | :organization_rating_id => @rating.id, |
55 | :target => @community, | 50 | :target => @community, |
56 | :requestor => @person) | 51 | :requestor => @person) |
57 | CreateOrganizationRatingComment.last.cancel | 52 | CreateOrganizationRatingComment.last.cancel |
58 | - assert_not @rating.task_active? | 53 | + assert_equal Task::Status::CANCELLED, @rating.task_status |
59 | end | 54 | end |
60 | 55 | ||
61 | - test "display report moderation message to community admin" do | ||
62 | - moderator = create_user('moderator') | ||
63 | - @community.add_admin(moderator.person) | ||
64 | - @rating.stubs(:task_active?).returns(true) | ||
65 | - assert @rating.display_moderation_message(@adminuser) | 56 | + test "should display full info to admin" do |
57 | + @person.stubs(:is_admin?).returns(true) | ||
58 | + assert @rating.display_full_info_to?(@person) | ||
66 | end | 59 | end |
67 | 60 | ||
68 | - test "display report moderation message to owner" do | ||
69 | - @rating.stubs(:task_active?).returns(true) | ||
70 | - assert @rating.display_moderation_message(@person) | 61 | + test "should display full info to owner" do |
62 | + assert @rating.display_full_info_to?(@person) | ||
71 | end | 63 | end |
72 | 64 | ||
73 | - test "do not display report moderation message to regular user" do | 65 | + test "should not display full info to regular user" do |
74 | regular_person = fast_create(Person) | 66 | regular_person = fast_create(Person) |
75 | - @rating.stubs(:task_active?).returns(true) | ||
76 | - assert_not @rating.display_moderation_message(regular_person) | ||
77 | - end | ||
78 | - | ||
79 | - test "do not display report moderation message to not logged user" do | ||
80 | - @rating.stubs(:task_active?).returns(true) | ||
81 | - assert_not @rating.display_moderation_message(nil) | ||
82 | - end | ||
83 | - | ||
84 | - test "do not display report moderation message no active task exists" do | ||
85 | - @rating.stubs(:task_active?).returns(false) | ||
86 | - assert_not @rating.display_moderation_message(@person) | 67 | + assert_not @rating.display_full_info_to?(regular_person) |
87 | end | 68 | end |
88 | 69 | ||
89 | test "Create task for create a rating comment" do | 70 | test "Create task for create a rating comment" do |
@@ -109,7 +90,7 @@ class OrganizationRatingTest < ActiveSupport::TestCase | @@ -109,7 +90,7 @@ class OrganizationRatingTest < ActiveSupport::TestCase | ||
109 | assert community.tasks.include?(create_organization_rating_comment) | 90 | assert community.tasks.include?(create_organization_rating_comment) |
110 | end | 91 | end |
111 | 92 | ||
112 | - test "Should calculate community's rating average" do | 93 | + test "Should calculate community's rating statistics" do |
113 | community = fast_create Community | 94 | community = fast_create Community |
114 | p1 = fast_create Person, :name=>"Person 1" | 95 | p1 = fast_create Person, :name=>"Person 1" |
115 | p2 = fast_create Person, :name=>"Person 2" | 96 | p2 = fast_create Person, :name=>"Person 2" |
@@ -119,11 +100,13 @@ class OrganizationRatingTest < ActiveSupport::TestCase | @@ -119,11 +100,13 @@ class OrganizationRatingTest < ActiveSupport::TestCase | ||
119 | OrganizationRating.create! :value => 3, :organization => community, :person => p2 | 100 | OrganizationRating.create! :value => 3, :organization => community, :person => p2 |
120 | OrganizationRating.create! :value => 5, :organization => community, :person => p3 | 101 | OrganizationRating.create! :value => 5, :organization => community, :person => p3 |
121 | 102 | ||
122 | - assert_equal 3, OrganizationRating.average_rating(community) | 103 | + assert_equal 3, OrganizationRating.statistics_for_profile(community)[:average] |
104 | + assert_equal 3, OrganizationRating.statistics_for_profile(community)[:total] | ||
123 | 105 | ||
124 | p4 = fast_create Person, :name=>"Person 4" | 106 | p4 = fast_create Person, :name=>"Person 4" |
125 | OrganizationRating.create! :value => 4, :organization => community, :person => p4 | 107 | OrganizationRating.create! :value => 4, :organization => community, :person => p4 |
126 | 108 | ||
127 | - assert_equal 4, OrganizationRating.average_rating(community) | 109 | + assert_equal 4, OrganizationRating.statistics_for_profile(community)[:average] |
110 | + assert_equal 4, OrganizationRating.statistics_for_profile(community)[:total] | ||
128 | end | 111 | end |
129 | end | 112 | end |
plugins/organization_ratings/test/unit/ratings_helper_test.rb
@@ -3,6 +3,7 @@ require 'ratings_helper' | @@ -3,6 +3,7 @@ require 'ratings_helper' | ||
3 | 3 | ||
4 | class RatingsHelperTest < ActiveSupport::TestCase | 4 | class RatingsHelperTest < ActiveSupport::TestCase |
5 | include RatingsHelper | 5 | include RatingsHelper |
6 | + include ActionView::Helpers::TagHelper | ||
6 | 7 | ||
7 | def setup | 8 | def setup |
8 | 9 | ||
@@ -12,6 +13,12 @@ class RatingsHelperTest < ActiveSupport::TestCase | @@ -12,6 +13,12 @@ class RatingsHelperTest < ActiveSupport::TestCase | ||
12 | @person = create_user('testuser').person | 13 | @person = create_user('testuser').person |
13 | @community = Community.create(:name => "TestCommunity") | 14 | @community = Community.create(:name => "TestCommunity") |
14 | @organization_ratings_config = OrganizationRatingsConfig.instance | 15 | @organization_ratings_config = OrganizationRatingsConfig.instance |
16 | + @rating = fast_create(OrganizationRating, {:value => 1, | ||
17 | + :person_id => @person.id, | ||
18 | + :organization_id => @community.id, | ||
19 | + :created_at => DateTime.now, | ||
20 | + :updated_at => DateTime.now, | ||
21 | + }) | ||
15 | end | 22 | end |
16 | 23 | ||
17 | should "get the ratings of a community ordered by most recent ratings" do | 24 | should "get the ratings of a community ordered by most recent ratings" do |
@@ -32,6 +39,7 @@ class RatingsHelperTest < ActiveSupport::TestCase | @@ -32,6 +39,7 @@ class RatingsHelperTest < ActiveSupport::TestCase | ||
32 | 39 | ||
33 | ratings_array << most_recent_rating | 40 | ratings_array << most_recent_rating |
34 | ratings_array << first_rating | 41 | ratings_array << first_rating |
42 | + ratings_array << @rating | ||
35 | 43 | ||
36 | assert_equal @organization_ratings_config.order, "recent" | 44 | assert_equal @organization_ratings_config.order, "recent" |
37 | assert_equal ratings_array, get_ratings(@community.id) | 45 | assert_equal ratings_array, get_ratings(@community.id) |
@@ -57,7 +65,42 @@ class RatingsHelperTest < ActiveSupport::TestCase | @@ -57,7 +65,42 @@ class RatingsHelperTest < ActiveSupport::TestCase | ||
57 | 65 | ||
58 | ratings_array << second_rating | 66 | ratings_array << second_rating |
59 | ratings_array << first_rating | 67 | ratings_array << first_rating |
68 | + ratings_array << @rating | ||
60 | 69 | ||
61 | assert_equal ratings_array, get_ratings(@community.id) | 70 | assert_equal ratings_array, get_ratings(@community.id) |
62 | end | 71 | end |
72 | + | ||
73 | + test "display report moderation message to community admin" do | ||
74 | + @moderator = create_user('moderator').person | ||
75 | + @community.add_admin(@moderator) | ||
76 | + @rating.stubs(:task_status).returns(Task::Status::ACTIVE) | ||
77 | + assert status_message_for(@moderator, @rating).include?("Report waiting for approval") | ||
78 | + end | ||
79 | + | ||
80 | + test "display report moderation message to owner" do | ||
81 | + @rating.stubs(:task_status).returns(Task::Status::ACTIVE) | ||
82 | + assert status_message_for(@person, @rating).include?("Report waiting for approval") | ||
83 | + end | ||
84 | + | ||
85 | + test "display report rejected message to owner" do | ||
86 | + @rating.stubs(:task_status).returns(Task::Status::CANCELLED) | ||
87 | + assert status_message_for(@person, @rating).include?("Report rejected") | ||
88 | + end | ||
89 | + | ||
90 | + test "do not display report moderation message to regular user" do | ||
91 | + @regular_person = fast_create(Person) | ||
92 | + @rating.stubs(:task_status).returns(Task::Status::ACTIVE) | ||
93 | + assert_nil status_message_for(@regular_person, @rating) | ||
94 | + end | ||
95 | + | ||
96 | + test "return empty status message to not logged user" do | ||
97 | + @rating.stubs(:task_status).returns(Task::Status::ACTIVE) | ||
98 | + assert_nil status_message_for(nil, @rating) | ||
99 | + end | ||
100 | + | ||
101 | + test "do not display status message if report task is finished" do | ||
102 | + @rating.stubs(:task_status).returns(Task::Status::FINISHED) | ||
103 | + assert_nil status_message_for(@person, @rating) | ||
104 | + end | ||
105 | + | ||
63 | end | 106 | end |
plugins/organization_ratings/views/blocks/display_organization_average_rating.html.erb
1 | +<% statistics = OrganizationRating.statistics_for_profile block.owner %> | ||
2 | + | ||
1 | <div class="organization-average-rating-container"> | 3 | <div class="organization-average-rating-container"> |
2 | - <% if average_rating %> | 4 | + <% if statistics[:average] %> |
3 | <div class="star-rate-text"> | 5 | <div class="star-rate-text"> |
4 | <%= _("Rating: ") %> | 6 | <%= _("Rating: ") %> |
5 | </div> | 7 | </div> |
6 | 8 | ||
7 | <div class="star-container"> | 9 | <div class="star-container"> |
8 | <% (1..5).each do |star_number| %> | 10 | <% (1..5).each do |star_number| %> |
9 | - <% if star_number <= average_rating %> | 11 | + <% if star_number <= statistics[:average] %> |
10 | <div class="medium-star-positive"></div> | 12 | <div class="medium-star-positive"></div> |
11 | <% else %> | 13 | <% else %> |
12 | <div class="medium-star-negative"></div> | 14 | <div class="medium-star-negative"></div> |
13 | <% end %> | 15 | <% end %> |
14 | <% end %> | 16 | <% end %> |
17 | + <div class="total-ratings"> | ||
18 | + <%= link_to url_for(:controller => 'organization_ratings_plugin_profile', :action => 'new_rating', :anchor => 'ratings-list') do %> | ||
19 | + <% if(statistics[:total] > 10000) %> | ||
20 | + <%= "(10000+)" %> | ||
21 | + <% else %> | ||
22 | + <%= "(#{statistics[:total]})" %> | ||
23 | + <% end %> | ||
24 | + <% end %> | ||
25 | + </div> | ||
15 | </div> | 26 | </div> |
16 | <% else %> | 27 | <% else %> |
17 | <div class="rating-invitation"> | 28 | <div class="rating-invitation"> |
@@ -22,4 +33,4 @@ | @@ -22,4 +33,4 @@ | ||
22 | <div class="rate-this-organization"> | 33 | <div class="rate-this-organization"> |
23 | <%= link_to _("Rate this %s") % _(profile.class.name), url_for(:controller => "organization_ratings_plugin_profile", :action => "new_rating", :profile=>profile_identifier) %> | 34 | <%= link_to _("Rate this %s") % _(profile.class.name), url_for(:controller => "organization_ratings_plugin_profile", :action => "new_rating", :profile=>profile_identifier) %> |
24 | </div> | 35 | </div> |
25 | -</div> | ||
26 | \ No newline at end of file | 36 | \ No newline at end of file |
37 | +</div> |
plugins/organization_ratings/views/blocks/organization_ratings_block.html.erb
@@ -14,7 +14,7 @@ | @@ -14,7 +14,7 @@ | ||
14 | <%= render :partial => 'shared/make_report_block' %> | 14 | <%= render :partial => 'shared/make_report_block' %> |
15 | 15 | ||
16 | <div class="see-more"> | 16 | <div class="see-more"> |
17 | - <%= link_to _('See more'), url_for(:controller => 'organization_ratings_plugin_profile', :action => 'new_rating'), :class => 'icon-arrow-right-p' %> | 17 | + <%= link_to _('See more'), url_for(:controller => 'organization_ratings_plugin_profile', :action => 'new_rating', :anchor => 'ratings-list'), :class => 'icon-arrow-right-p' %> |
18 | </div> | 18 | </div> |
19 | </div> | 19 | </div> |
20 | <% end %> | 20 | <% end %> |
plugins/organization_ratings/views/organization_ratings_plugin_profile/new_rating.html.erb
@@ -9,7 +9,7 @@ | @@ -9,7 +9,7 @@ | ||
9 | </div> | 9 | </div> |
10 | <% end %> | 10 | <% end %> |
11 | 11 | ||
12 | -<div class="ratings-list"> | 12 | +<div class="ratings-list" id="ratings-list"> |
13 | <% @users_ratings.each do |user_rate| %> | 13 | <% @users_ratings.each do |user_rate| %> |
14 | <%= render :partial => "shared/user_rating_container", :locals => {:user_rate => user_rate} %> | 14 | <%= render :partial => "shared/user_rating_container", :locals => {:user_rate => user_rate} %> |
15 | <% end %> | 15 | <% end %> |
plugins/organization_ratings/views/shared/_user_rating_container.html.erb
1 | +<% extend RatingsHelper %> | ||
1 | <div class="user-rating-block"> | 2 | <div class="user-rating-block"> |
2 | <div class="star-profile-information"> | 3 | <div class="star-profile-information"> |
3 | <div class="star-profile-image"> | 4 | <div class="star-profile-image"> |
@@ -25,9 +26,7 @@ | @@ -25,9 +26,7 @@ | ||
25 | </div> | 26 | </div> |
26 | 27 | ||
27 | <div class="user-testimony"> | 28 | <div class="user-testimony"> |
28 | - <% if user_rate.display_moderation_message(user) %> | ||
29 | - <p class="moderation-msg"><%= _("Report waiting for approval") %></p> | ||
30 | - <% end %> | 29 | + <%= status_message_for(user, user_rate) %> |
31 | <% if user_rate.comment.present? %> | 30 | <% if user_rate.comment.present? %> |
32 | <p class="comment-body"> <%= user_rate.comment.body %> </p> | 31 | <p class="comment-body"> <%= user_rate.comment.body %> </p> |
33 | <% end %> | 32 | <% end %> |