Commit c30065be6679c968b9c9edfca2bd60e428588a1e
Committed by
Gabriel Silva
1 parent
4314409d
Exists in
ratings_minor_fixes
and in
3 other branches
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>
Showing
11 changed files
with
137 additions
and
57 deletions
Show diff stats
plugins/organization_ratings/features/rate_community.feature
| ... | ... | @@ -19,7 +19,7 @@ Feature: rate_community |
| 19 | 19 | And I am logged in as "joaosilva" |
| 20 | 20 | |
| 21 | 21 | @selenium |
| 22 | - Scenario: display rate button inside average block | |
| 22 | + Scenario: display rate button and total ratings inside average block | |
| 23 | 23 | Given I am on mycommunity's homepage |
| 24 | 24 | Then I should see "Rate this Community" within ".average-rating-block" |
| 25 | 25 | And I should see "Be the first to rate" within ".average-rating-block" |
| ... | ... | @@ -34,3 +34,9 @@ Feature: rate_community |
| 34 | 34 | When I follow "Rate this Community" |
| 35 | 35 | Then I should see "Joao Silva" within ".star-profile-name" |
| 36 | 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 < ApplicationRecord |
| 13 | 13 | validates :organization_id, :person_id, |
| 14 | 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 | 18 | self.organization.is_admin?(person)) |
| 20 | - end | |
| 21 | 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 | 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 | 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 | 34 | end |
| 35 | + { average: average, total: total } | |
| 35 | 36 | end |
| 36 | 37 | |
| 37 | 38 | end | ... | ... |
plugins/organization_ratings/lib/ratings_helper.rb
| ... | ... | @@ -12,4 +12,14 @@ module RatingsHelper |
| 12 | 12 | ratings = OrganizationRating.where(organization_id: profile_id).order("created_at DESC") |
| 13 | 13 | end |
| 14 | 14 | end |
| 15 | -end | |
| 16 | 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 | 1 | .star-container { |
| 2 | 2 | width: 100%; |
| 3 | - height: 20px; | |
| 3 | + height: 22px; | |
| 4 | 4 | } |
| 5 | 5 | |
| 6 | 6 | .star-negative, .star-positive { |
| ... | ... | @@ -80,7 +80,16 @@ |
| 80 | 80 | |
| 81 | 81 | .organization-average-rating-container .star-container { |
| 82 | 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 | 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 | 188 | assert_no_tag :tag => 'p', :content => /Report waiting for approva/, :attributes => {:class =>/comment-rejected-msg/} |
| 189 | 189 | assert_tag :tag => 'p', :content => /comment accepted/, :attributes => {:class =>/comment-body/} |
| 190 | 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 | 211 | end | ... | ... |
plugins/organization_ratings/test/unit/organization_rating_test.rb
| ... | ... | @@ -35,55 +35,36 @@ class OrganizationRatingTest < ActiveSupport::TestCase |
| 35 | 35 | assert_equal false, organization_rating2.errors[:value].include?("must be between 1 and 5") |
| 36 | 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 | 39 | CreateOrganizationRatingComment.create!( |
| 44 | 40 | :organization_rating_id => @rating.id, |
| 45 | 41 | :target => @community, |
| 46 | 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 | 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 | 48 | CreateOrganizationRatingComment.create!( |
| 54 | 49 | :organization_rating_id => @rating.id, |
| 55 | 50 | :target => @community, |
| 56 | 51 | :requestor => @person) |
| 57 | 52 | CreateOrganizationRatingComment.last.cancel |
| 58 | - assert_not @rating.task_active? | |
| 53 | + assert_equal Task::Status::CANCELLED, @rating.task_status | |
| 59 | 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 | 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 | 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 | 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 | 68 | end |
| 88 | 69 | |
| 89 | 70 | test "Create task for create a rating comment" do |
| ... | ... | @@ -109,7 +90,7 @@ class OrganizationRatingTest < ActiveSupport::TestCase |
| 109 | 90 | assert community.tasks.include?(create_organization_rating_comment) |
| 110 | 91 | end |
| 111 | 92 | |
| 112 | - test "Should calculate community's rating average" do | |
| 93 | + test "Should calculate community's rating statistics" do | |
| 113 | 94 | community = fast_create Community |
| 114 | 95 | p1 = fast_create Person, :name=>"Person 1" |
| 115 | 96 | p2 = fast_create Person, :name=>"Person 2" |
| ... | ... | @@ -119,11 +100,13 @@ class OrganizationRatingTest < ActiveSupport::TestCase |
| 119 | 100 | OrganizationRating.create! :value => 3, :organization => community, :person => p2 |
| 120 | 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 | 106 | p4 = fast_create Person, :name=>"Person 4" |
| 125 | 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 | 111 | end |
| 129 | 112 | end | ... | ... |
plugins/organization_ratings/test/unit/ratings_helper_test.rb
| ... | ... | @@ -3,6 +3,7 @@ require 'ratings_helper' |
| 3 | 3 | |
| 4 | 4 | class RatingsHelperTest < ActiveSupport::TestCase |
| 5 | 5 | include RatingsHelper |
| 6 | + include ActionView::Helpers::TagHelper | |
| 6 | 7 | |
| 7 | 8 | def setup |
| 8 | 9 | |
| ... | ... | @@ -12,6 +13,12 @@ class RatingsHelperTest < ActiveSupport::TestCase |
| 12 | 13 | @person = create_user('testuser').person |
| 13 | 14 | @community = Community.create(:name => "TestCommunity") |
| 14 | 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 | 22 | end |
| 16 | 23 | |
| 17 | 24 | should "get the ratings of a community ordered by most recent ratings" do |
| ... | ... | @@ -32,6 +39,7 @@ class RatingsHelperTest < ActiveSupport::TestCase |
| 32 | 39 | |
| 33 | 40 | ratings_array << most_recent_rating |
| 34 | 41 | ratings_array << first_rating |
| 42 | + ratings_array << @rating | |
| 35 | 43 | |
| 36 | 44 | assert_equal @organization_ratings_config.order, "recent" |
| 37 | 45 | assert_equal ratings_array, get_ratings(@community.id) |
| ... | ... | @@ -57,7 +65,42 @@ class RatingsHelperTest < ActiveSupport::TestCase |
| 57 | 65 | |
| 58 | 66 | ratings_array << second_rating |
| 59 | 67 | ratings_array << first_rating |
| 68 | + ratings_array << @rating | |
| 60 | 69 | |
| 61 | 70 | assert_equal ratings_array, get_ratings(@community.id) |
| 62 | 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 | 106 | end | ... | ... |
plugins/organization_ratings/views/blocks/average_rating.html.erb
| 1 | -<% average_rating = OrganizationRating.average_rating block.owner.id %> | |
| 1 | +<% statistics = OrganizationRating.statistics_for_profile block.owner %> | |
| 2 | 2 | |
| 3 | 3 | <div class="organization-average-rating-container"> |
| 4 | - <% if average_rating %> | |
| 4 | + <% if statistics[:average] %> | |
| 5 | 5 | <div class="star-rate-text"> |
| 6 | 6 | <%= _("Rating: ") %> |
| 7 | 7 | </div> |
| 8 | 8 | |
| 9 | 9 | <div class="star-container"> |
| 10 | 10 | <% (1..5).each do |star_number| %> |
| 11 | - <% if star_number <= average_rating %> | |
| 11 | + <% if star_number <= statistics[:average] %> | |
| 12 | 12 | <div class="medium-star-positive"></div> |
| 13 | 13 | <% else %> |
| 14 | 14 | <div class="medium-star-negative"></div> |
| 15 | 15 | <% end %> |
| 16 | 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> | |
| 17 | 26 | </div> |
| 18 | 27 | <% else %> |
| 19 | 28 | <div class="rating-invitation"> |
| ... | ... | @@ -24,4 +33,4 @@ |
| 24 | 33 | <div class="rate-this-organization"> |
| 25 | 34 | <%= link_to _("Rate this %s") % _(block.owner.class.name), url_for(:controller => "organization_ratings_plugin_profile", :action => "new_rating", :profile => block.owner.identifier) %> |
| 26 | 35 | </div> |
| 27 | -</div> | |
| 28 | 36 | \ No newline at end of file |
| 37 | +</div> | ... | ... |
plugins/organization_ratings/views/blocks/organization_ratings.html.erb
| ... | ... | @@ -14,7 +14,7 @@ |
| 14 | 14 | <%= render :partial => 'shared/make_report_block' %> |
| 15 | 15 | |
| 16 | 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 | 18 | </div> |
| 19 | 19 | </div> |
| 20 | 20 | <% end %> | ... | ... |
plugins/organization_ratings/views/organization_ratings_plugin_profile/new_rating.html.erb
| ... | ... | @@ -9,7 +9,7 @@ |
| 9 | 9 | </div> |
| 10 | 10 | <% end %> |
| 11 | 11 | |
| 12 | -<div class="ratings-list"> | |
| 12 | +<div class="ratings-list" id="ratings-list"> | |
| 13 | 13 | <% @users_ratings.each do |user_rate| %> |
| 14 | 14 | <%= render :partial => "shared/user_rating_container", :locals => {:user_rate => user_rate} %> |
| 15 | 15 | <% end %> | ... | ... |
plugins/organization_ratings/views/shared/_user_rating_container.html.erb
| 1 | +<% extend RatingsHelper %> | |
| 1 | 2 | <div class="user-rating-block"> |
| 2 | 3 | <div class="star-profile-information"> |
| 3 | 4 | <div class="star-profile-image"> |
| ... | ... | @@ -25,9 +26,7 @@ |
| 25 | 26 | </div> |
| 26 | 27 | |
| 27 | 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 | 30 | <% if user_rate.comment.present? %> |
| 32 | 31 | <p class="comment-body"> <%= user_rate.comment.body %> </p> |
| 33 | 32 | <% end %> | ... | ... |
-
mentioned in commit 2926f7ed17f5fad829c4724e47ebfc7b70fa47d8