Merge pull request #1409 from riyad/update-votes

Update votes for issues and merge requests

Merge pull request #1409 from riyad/update-votes
Update votes for issues and merge requests
Dmitriy Zaporozhets
2 parents d8f6d38d 0bfcc574
Showing 17 changed files with 256 additions and 66 deletions Show diff stats
app/assets/stylesheets/common.scss
app/models/issue.rb
app/models/merge_request.rb
app/models/note.rb
app/roles/upvote.rb
app/roles/votes.rb
app/views/issues/_show.html.haml
app/views/issues/show.html.haml
app/views/merge_requests/_merge_request.html.haml
app/views/merge_requests/show/_mr_title.html.haml
app/views/votes/_votes_block.html.haml
app/views/votes/_votes_inline.html.haml
spec/models/issue_spec.rb
spec/models/merge_request_spec.rb
spec/models/note_spec.rb
spec/roles/upvote_spec.rb
spec/roles/votes_spec.rb
@@ -415,13 +415,48 @@ p.time {
   }
 }
-.upvotes {
-  font-size: 14px;
-  font-weight: bold;
-  color: #468847;
-  text-align: right;
-  padding: 4px;
-  margin: 2px;
+.votes {
+  font-size: 13px;
+  line-height: 15px;
+  .progress {
+    height: 4px;
+    margin: 0;
+    .bar {
+      float: left;
+      height: 100%;
+    }
+    .bar-success {
+      background-color: #468847;
+      @include bg-gradient(#62C462, #51A351);
+    }
+    .bar-danger {
+      background-color: #B94A48;
+      @include bg-gradient(#EE5F5B, #BD362F);
+    }
+  }
+  .upvotes {
+    display: inline-block;
+    color: #468847;
+  }
+  .downvotes {
+    display: inline-block;
+    color: #B94A48;
+  }
+}
+.votes-block {
+  margin: 14px 6px 6px 0;
+  .downvotes {
+    float: right;
+  }
+}
+.votes-inline {
+  display: inline-block;
+  margin: 0 8px;
+  .progress {
+    display: inline-block;
+    padding: 0 0 2px;
+    width: 45px;
+  }
 }
 /* Fix for readme code (stopped it from being yellow) */
 class Issue < ActiveRecord::Base
   include IssueCommonality
-  include Upvote
+  include Votes
   acts_as_taggable_on :labels
@@ -2,7 +2,7 @@ require File.join(Rails.root, &quot;app/models/commit&quot;)
 class MergeRequest < ActiveRecord::Base
   include IssueCommonality
-  include Upvote
+  include Votes
   BROKEN_DIFF = "--broken-diff"
@@ -105,6 +105,12 @@ class Note &lt; ActiveRecord::Base
   def upvote?
     note.start_with?('+1') || note.start_with?(':+1:')
   end
+
+  # Returns true if this is a downvote note,
+  # otherwise false is returned
+  def downvote?
+    note.start_with?('-1') || note.start_with?(':-1:')
+  end
 end
 # == Schema Information
 #
@@ -1,6 +0,0 @@
-module Upvote
-  # Return the number of +1 comments (upvotes)
-  def upvotes
-    notes.select(&:upvote?).size
-  end
-end
@@ -0,0 +1,32 @@
+module Votes
+  # Return the number of +1 comments (upvotes)
+  def upvotes
+    notes.select(&:upvote?).size
+  end
+
+  def upvotes_in_percent
+    if votes_count.zero?
+      0
+    else
+      100.0 / votes_count * upvotes
+    end
+  end
+
+  # Return the number of -1 comments (downvotes)
+  def downvotes
+    notes.select(&:downvote?).size
+  end
+
+  def downvotes_in_percent
+    if votes_count.zero?
+      0
+    else
+      100.0 - upvotes_in_percent
+    end
+  end
+
+  # Return the total number of votes
+  def votes_count
+    upvotes + downvotes
+  end
+end
@@ -34,5 +34,5 @@
     - else
       &nbsp;
-    - if issue.upvotes > 0
-      %span.badge.badge-success= "+#{issue.upvotes}"
+    - if issue.votes_count > 0
+      = render 'votes/votes_inline', votable: issue
@@ -8,22 +8,22 @@
   %span.right
     - if can?(current_user, :admin_project, @project) || @issue.author == current_user
       - if @issue.closed
-        = link_to 'Reopen', project_issue_path(@project, @issue, issue: {closed: false }, status_only: true), method: :put,  class: "btn small"
+        = link_to 'Reopen', project_issue_path(@project, @issue, issue: {closed: false }, status_only: true), method: :put,  class: "btn grouped success"
       - else
-        = link_to 'Close', project_issue_path(@project, @issue, issue: {closed: true }, status_only: true), method: :put, class: "btn small", title: "Close Issue"
+        = link_to 'Close', project_issue_path(@project, @issue, issue: {closed: true }, status_only: true), method: :put, class: "btn grouped danger", title: "Close Issue"
     - if can?(current_user, :admin_project, @project) || @issue.author == current_user
-      = link_to edit_project_issue_path(@project, @issue), class: "btn small" do
+      = link_to edit_project_issue_path(@project, @issue), class: "btn grouped" do
         %i.icon-edit
         Edit
-    %br
-    - if @issue.upvotes > 0
-      .upvotes#upvotes= "+#{pluralize @issue.upvotes, 'upvote'}"
+.right
+  .span3#votes= render 'votes/votes_block', votable: @issue
 .back_link
   = link_to project_issues_path(@project) do
     &larr; To issues list
+
 .main_box
   .top_box_content
     %h4
@@ -23,5 +23,6 @@
     authored by #{merge_request.author_name}
     = time_ago_in_words(merge_request.created_at)
     ago
-    - if merge_request.upvotes > 0
-      %span.badge.badge-success= "+#{merge_request.upvotes}"
+
+    - if merge_request.votes_count > 0
+      = render 'votes/votes_inline', votable: merge_request
@@ -23,10 +23,8 @@
           %i.icon-edit
           Edit
-    %br
-    - if @merge_request.upvotes > 0
-      .upvotes#upvotes= "+#{pluralize @merge_request.upvotes, 'upvote'}"
-
+.right
+  .span3#votes= render 'votes/votes_block', votable: @merge_request
 .back_link
   = link_to project_merge_requests_path(@project) do
@@ -0,0 +1,6 @@
+.votes.votes-block
+  .progress
+    .bar.bar-success{style: "width: #{votable.upvotes_in_percent}%;"}
+    .bar.bar-danger{style: "width: #{votable.downvotes_in_percent}%;"}
+  .upvotes= "#{votable.upvotes} up"
+  .downvotes= "#{votable.downvotes} down"
@@ -0,0 +1,6 @@
+.votes.votes-inline
+  .upvotes= votable.upvotes
+  .progress
+    .bar.bar-success{style: "width: #{votable.upvotes_in_percent}%;"}
+    .bar.bar-danger{style: "width: #{votable.downvotes_in_percent}%;"}
+  .downvotes= votable.downvotes
@@ -12,7 +12,7 @@ describe Issue do
   describe 'modules' do
     it { should include_module(IssueCommonality) }
-    it { should include_module(Upvote) }
+    it { should include_module(Votes) }
   end
   subject { Factory.create(:issue) }
@@ -8,6 +8,6 @@ describe MergeRequest do
   describe 'modules' do
     it { should include_module(IssueCommonality) }
-    it { should include_module(Upvote) }
+    it { should include_module(Votes) }
   end
 end
@@ -24,6 +24,13 @@ describe Note do
     it "recognizes a neutral note" do
       note = Factory(:note, note: "This is not a +1 note")
       note.should_not be_upvote
+      note.should_not be_downvote
+    end
+
+    it "recognizes a neutral emoji note" do
+      note = build(:note, note: "I would :+1: this, but I don't want to")
+      note.should_not be_upvote
+      note.should_not be_downvote
     end
     it "recognizes a +1 note" do
@@ -31,19 +38,19 @@ describe Note do
       note.should be_upvote
     end
-    it "recognizes a -1 note as no vote" do
-      note = Factory(:note, note: "-1 for this")
-      note.should_not be_upvote
-    end
-
     it "recognizes a +1 emoji as a vote" do
       note = build(:note, note: ":+1: for this")
       note.should be_upvote
     end
-    it "recognizes a neutral emoji note" do
-      note = build(:note, note: "I would :+1: this, but I don't want to")
-      note.should_not be_upvote
+    it "recognizes a -1 note" do
+      note = Factory(:note, note: "-1 for this")
+      note.should be_downvote
+    end
+
+    it "recognizes a -1 emoji as a vote" do
+      note = build(:note, note: ":-1: for this")
+      note.should be_downvote
     end
   end
@@ -1,27 +0,0 @@
-require 'spec_helper'
-
-describe Issue, "Upvote" do
-  let(:issue) { create(:issue) }
-
-  it "with no notes has a 0/0 score" do
-    issue.upvotes.should == 0
-  end
-
-  it "should recognize non-+1 notes" do
-    issue.notes << create(:note, note: "No +1 here")
-    issue.should have(1).note
-    issue.notes.first.upvote?.should be_false
-    issue.upvotes.should == 0
-  end
-
-  it "should recognize a single +1 note" do
-    issue.notes << create(:note, note: "+1 This is awesome")
-    issue.upvotes.should == 1
-  end
-
-  it "should recognize multiple +1 notes" do
-    issue.notes << create(:note, note: "+1 This is awesome")
-    issue.notes << create(:note, note: "+1 I want this")
-    issue.upvotes.should == 2
-  end
-end
@@ -0,0 +1,132 @@
+require 'spec_helper'
+
+describe Issue do
+  let(:issue) { create(:issue) }
+
+  describe "#upvotes" do
+    it "with no notes has a 0/0 score" do
+      issue.upvotes.should == 0
+    end
+
+    it "should recognize non-+1 notes" do
+      issue.notes << create(:note, note: "No +1 here")
+      issue.should have(1).note
+      issue.notes.first.upvote?.should be_false
+      issue.upvotes.should == 0
+    end
+
+    it "should recognize a single +1 note" do
+      issue.notes << create(:note, note: "+1 This is awesome")
+      issue.upvotes.should == 1
+    end
+
+    it "should recognize multiple +1 notes" do
+      issue.notes << create(:note, note: "+1 This is awesome")
+      issue.notes << create(:note, note: "+1 I want this")
+      issue.upvotes.should == 2
+    end
+  end
+
+  describe "#downvotes" do
+    it "with no notes has a 0/0 score" do
+      issue.downvotes.should == 0
+    end
+
+    it "should recognize non--1 notes" do
+      issue.notes << create(:note, note: "Almost got a -1")
+      issue.should have(1).note
+      issue.notes.first.downvote?.should be_false
+      issue.downvotes.should == 0
+    end
+
+    it "should recognize a single -1 note" do
+      issue.notes << create(:note, note: "-1 This is bad")
+      issue.downvotes.should == 1
+    end
+
+    it "should recognize multiple -1 notes" do
+      issue.notes << create(:note, note: "-1 This is bad")
+      issue.notes << create(:note, note: "-1 Away with this")
+      issue.downvotes.should == 2
+    end
+  end
+
+  describe "#votes_count" do
+    it "with no notes has a 0/0 score" do
+      issue.votes_count.should == 0
+    end
+
+    it "should recognize non notes" do
+      issue.notes << create(:note, note: "No +1 here")
+      issue.should have(1).note
+      issue.votes_count.should == 0
+    end
+
+    it "should recognize a single +1 note" do
+      issue.notes << create(:note, note: "+1 This is awesome")
+      issue.votes_count.should == 1
+    end
+
+    it "should recognize a single -1 note" do
+      issue.notes << create(:note, note: "-1 This is bad")
+      issue.votes_count.should == 1
+    end
+
+    it "should recognize multiple notes" do
+      issue.notes << create(:note, note: "+1 This is awesome")
+      issue.notes << create(:note, note: "-1 This is bad")
+      issue.notes << create(:note, note: "+1 I want this")
+      issue.votes_count.should == 3
+    end
+  end
+
+  describe "#upvotes_in_percent" do
+    it "with no notes has a 0% score" do
+      issue.upvotes_in_percent.should == 0
+    end
+
+    it "should count a single 1 note as 100%" do
+      issue.notes << create(:note, note: "+1 This is awesome")
+      issue.upvotes_in_percent.should == 100
+    end
+
+    it "should count multiple +1 notes as 100%" do
+      issue.notes << create(:note, note: "+1 This is awesome")
+      issue.notes << create(:note, note: "+1 I want this")
+      issue.upvotes_in_percent.should == 100
+    end
+
+    it "should count fractions for multiple +1 and -1 notes correctly" do
+      issue.notes << create(:note, note: "+1 This is awesome")
+      issue.notes << create(:note, note: "+1 I want this")
+      issue.notes << create(:note, note: "-1 This is bad")
+      issue.notes << create(:note, note: "+1 me too")
+      issue.upvotes_in_percent.should == 75
+    end
+  end
+
+  describe "#downvotes_in_percent" do
+    it "with no notes has a 0% score" do
+      issue.downvotes_in_percent.should == 0
+    end
+
+    it "should count a single -1 note as 100%" do
+      issue.notes << create(:note, note: "-1 This is bad")
+      issue.downvotes_in_percent.should == 100
+    end
+
+    it "should count multiple -1 notes as 100%" do
+      issue.notes << create(:note, note: "-1 This is bad")
+      issue.notes << create(:note, note: "-1 Away with this")
+      issue.downvotes_in_percent.should == 100
+    end
+
+    it "should count fractions for multiple +1 and -1 notes correctly" do
+      issue.notes << create(:note, note: "+1 This is awesome")
+      issue.notes << create(:note, note: "+1 I want this")
+      issue.notes << create(:note, note: "-1 This is bad")
+      issue.notes << create(:note, note: "+1 me too")
+      issue.downvotes_in_percent.should == 25
+    end
+  end
+end
	@@ -415,13 +415,48 @@ p.time {		@@ -415,13 +415,48 @@ p.time {
415	}	415	}
416	}	416	}
417		417
418	-.upvotes {
419	- font-size: 14px;
420	- font-weight: bold;
421	- color: #468847;
422	- text-align: right;
423	- padding: 4px;
424	- margin: 2px;	418	+.votes {
		419	+ font-size: 13px;
		420	+ line-height: 15px;
		421	+ .progress {
		422	+ height: 4px;
		423	+ margin: 0;
		424	+ .bar {
		425	+ float: left;
		426	+ height: 100%;
		427	+ }
		428	+ .bar-success {
		429	+ background-color: #468847;
		430	+ @include bg-gradient(#62C462, #51A351);
		431	+ }
		432	+ .bar-danger {
		433	+ background-color: #B94A48;
		434	+ @include bg-gradient(#EE5F5B, #BD362F);
		435	+ }
		436	+ }
		437	+ .upvotes {
		438	+ display: inline-block;
		439	+ color: #468847;
		440	+ }
		441	+ .downvotes {
		442	+ display: inline-block;
		443	+ color: #B94A48;
		444	+ }
		445	+}
		446	+.votes-block {
		447	+ margin: 14px 6px 6px 0;
		448	+ .downvotes {
		449	+ float: right;
		450	+ }
		451	+}
		452	+.votes-inline {
		453	+ display: inline-block;
		454	+ margin: 0 8px;
		455	+ .progress {
		456	+ display: inline-block;
		457	+ padding: 0 0 2px;
		458	+ width: 45px;
		459	+ }
425	}	460	}
426		461
427	/* Fix for readme code (stopped it from being yellow) */	462	/* Fix for readme code (stopped it from being yellow) */
1	class Issue < ActiveRecord::Base	1	class Issue < ActiveRecord::Base
2	include IssueCommonality	2	include IssueCommonality
3	- include Upvote	3	+ include Votes
4		4
5	acts_as_taggable_on :labels	5	acts_as_taggable_on :labels
6		6
	@@ -2,7 +2,7 @@ require File.join(Rails.root, "app/models/commit")		@@ -2,7 +2,7 @@ require File.join(Rails.root, "app/models/commit")
2		2
3	class MergeRequest < ActiveRecord::Base	3	class MergeRequest < ActiveRecord::Base
4	include IssueCommonality	4	include IssueCommonality
5	- include Upvote	5	+ include Votes
6		6
7	BROKEN_DIFF = "--broken-diff"	7	BROKEN_DIFF = "--broken-diff"
8		8
	@@ -105,6 +105,12 @@ class Note < ActiveRecord::Base		@@ -105,6 +105,12 @@ class Note < ActiveRecord::Base
105	def upvote?	105	def upvote?
106	note.start_with?('+1') \|\| note.start_with?(':+1:')	106	note.start_with?('+1') \|\| note.start_with?(':+1:')
107	end	107	end
		108	+
		109	+ # Returns true if this is a downvote note,
		110	+ # otherwise false is returned
		111	+ def downvote?
		112	+ note.start_with?('-1') \|\| note.start_with?(':-1:')
		113	+ end
108	end	114	end
109	# == Schema Information	115	# == Schema Information
110	#	116	#
	@@ -1,6 +0,0 @@	@@ -1,6 +0,0 @@
1	-module Upvote
2	- # Return the number of +1 comments (upvotes)
3	- def upvotes
4	- notes.select(&:upvote?).size
5	- end
6	-end
@@ -0,0 +1,32 @@		@@ -0,0 +1,32 @@
	1	+module Votes
	2	+ # Return the number of +1 comments (upvotes)
	3	+ def upvotes
	4	+ notes.select(&:upvote?).size
	5	+ end
	6	+
	7	+ def upvotes_in_percent
	8	+ if votes_count.zero?
	9	+ 0
	10	+ else
	11	+ 100.0 / votes_count * upvotes
	12	+ end
	13	+ end
	14	+
	15	+ # Return the number of -1 comments (downvotes)
	16	+ def downvotes
	17	+ notes.select(&:downvote?).size
	18	+ end
	19	+
	20	+ def downvotes_in_percent
	21	+ if votes_count.zero?
	22	+ 0
	23	+ else
	24	+ 100.0 - upvotes_in_percent
	25	+ end
	26	+ end
	27	+
	28	+ # Return the total number of votes
	29	+ def votes_count
	30	+ upvotes + downvotes
	31	+ end
	32	+end
	@@ -34,5 +34,5 @@		@@ -34,5 +34,5 @@
34	- else	34	- else
35		35
36		36
37	- - if issue.upvotes > 0
38	- %span.badge.badge-success= "+#{issue.upvotes}"	37	+ - if issue.votes_count > 0
		38	+ = render 'votes/votes_inline', votable: issue
@@ -0,0 +1,6 @@		@@ -0,0 +1,6 @@
	1	+.votes.votes-block
	2	+ .progress
	3	+ .bar.bar-success{style: "width: #{votable.upvotes_in_percent}%;"}
	4	+ .bar.bar-danger{style: "width: #{votable.downvotes_in_percent}%;"}
	5	+ .upvotes= "#{votable.upvotes} up"
	6	+ .downvotes= "#{votable.downvotes} down"
@@ -0,0 +1,6 @@		@@ -0,0 +1,6 @@
	1	+.votes.votes-inline
	2	+ .upvotes= votable.upvotes
	3	+ .progress
	4	+ .bar.bar-success{style: "width: #{votable.upvotes_in_percent}%;"}
	5	+ .bar.bar-danger{style: "width: #{votable.downvotes_in_percent}%;"}
	6	+ .downvotes= votable.downvotes
	@@ -12,7 +12,7 @@ describe Issue do		@@ -12,7 +12,7 @@ describe Issue do
12		12
13	describe 'modules' do	13	describe 'modules' do
14	it { should include_module(IssueCommonality) }	14	it { should include_module(IssueCommonality) }
15	- it { should include_module(Upvote) }	15	+ it { should include_module(Votes) }
16	end	16	end
17		17
18	subject { Factory.create(:issue) }	18	subject { Factory.create(:issue) }