Commit 8d8b82127f2563ee24a7621793b19c9c984c199e
1 parent
ed11ca13
Exists in
master
and in
4 other branches
Count +1 votes in issue notes.
Showing
6 changed files
with
81 additions
and
15 deletions
Show diff stats
app/models/issue.rb
@@ -45,6 +45,11 @@ class Issue < ActiveRecord::Base | @@ -45,6 +45,11 @@ class Issue < ActiveRecord::Base | ||
45 | def new? | 45 | def new? |
46 | today? && created_at == updated_at | 46 | today? && created_at == updated_at |
47 | end | 47 | end |
48 | + | ||
49 | + # Return the number of +1 comments (upvotes) | ||
50 | + def upvotes | ||
51 | + notes.select(&:upvote?).size | ||
52 | + end | ||
48 | end | 53 | end |
49 | # == Schema Information | 54 | # == Schema Information |
50 | # | 55 | # |
app/models/note.rb
@@ -47,27 +47,27 @@ class Note < ActiveRecord::Base | @@ -47,27 +47,27 @@ class Note < ActiveRecord::Base | ||
47 | end | 47 | end |
48 | 48 | ||
49 | def target | 49 | def target |
50 | - if noteable_type == "Commit" | 50 | + if noteable_type == "Commit" |
51 | project.commit(noteable_id) | 51 | project.commit(noteable_id) |
52 | - else | 52 | + else |
53 | noteable | 53 | noteable |
54 | end | 54 | end |
55 | # Temp fix to prevent app crash | 55 | # Temp fix to prevent app crash |
56 | # if note commit id doesnt exist | 56 | # if note commit id doesnt exist |
57 | - rescue | 57 | + rescue |
58 | nil | 58 | nil |
59 | end | 59 | end |
60 | 60 | ||
61 | # Check if we can notify commit author | 61 | # Check if we can notify commit author |
62 | # with email about our comment | 62 | # with email about our comment |
63 | # | 63 | # |
64 | - # If commit author email exist in project | ||
65 | - # and commit author is not passed user we can | 64 | + # If commit author email exist in project |
65 | + # and commit author is not passed user we can | ||
66 | # send email to him | 66 | # send email to him |
67 | # | 67 | # |
68 | # params: | 68 | # params: |
69 | # user - current user | 69 | # user - current user |
70 | - # | 70 | + # |
71 | # return: | 71 | # return: |
72 | # Boolean | 72 | # Boolean |
73 | # | 73 | # |
@@ -81,12 +81,18 @@ class Note < ActiveRecord::Base | @@ -81,12 +81,18 @@ class Note < ActiveRecord::Base | ||
81 | end | 81 | end |
82 | 82 | ||
83 | def commit_author | 83 | def commit_author |
84 | - @commit_author ||= | ||
85 | - project.users.find_by_email(target.author_email) || | 84 | + @commit_author ||= |
85 | + project.users.find_by_email(target.author_email) || | ||
86 | project.users.find_by_name(target.author_name) | 86 | project.users.find_by_name(target.author_name) |
87 | - rescue | 87 | + rescue |
88 | nil | 88 | nil |
89 | end | 89 | end |
90 | + | ||
91 | + # Returns true if this is an upvote note, | ||
92 | + # otherwise false is returned | ||
93 | + def upvote? | ||
94 | + note =~ /^\+1/ ? true : false | ||
95 | + end | ||
90 | end | 96 | end |
91 | # == Schema Information | 97 | # == Schema Information |
92 | # | 98 | # |
app/views/issues/_show.html.haml
app/views/issues/show.html.haml
@@ -35,6 +35,9 @@ | @@ -35,6 +35,9 @@ | ||
35 | %cite.cgray and currently assigned to | 35 | %cite.cgray and currently assigned to |
36 | = image_tag gravatar_icon(@issue.assignee_email), :width => 16, :class => "lil_av" | 36 | = image_tag gravatar_icon(@issue.assignee_email), :width => 16, :class => "lil_av" |
37 | %strong.author= link_to_issue_assignee(@issue) | 37 | %strong.author= link_to_issue_assignee(@issue) |
38 | + | ||
39 | + - if @issue.upvotes > 0 | ||
40 | + %span.label.success= "+#{@issue.upvotes}" | ||
38 | 41 | ||
39 | %hr | 42 | %hr |
40 | 43 |
spec/models/issue_spec.rb
@@ -24,6 +24,37 @@ describe Issue do | @@ -24,6 +24,37 @@ describe Issue do | ||
24 | :assignee => Factory(:user), | 24 | :assignee => Factory(:user), |
25 | :project => Factory.create(:project)).should be_valid } | 25 | :project => Factory.create(:project)).should be_valid } |
26 | 26 | ||
27 | + describe "plus 1" do | ||
28 | + let(:project) { Factory(:project) } | ||
29 | + subject { | ||
30 | + Factory.create(:issue, | ||
31 | + :author => Factory(:user), | ||
32 | + :assignee => Factory(:user), | ||
33 | + :project => project) | ||
34 | + } | ||
35 | + | ||
36 | + it "with no notes has a 0/0 score" do | ||
37 | + subject.upvotes.should == 0 | ||
38 | + end | ||
39 | + | ||
40 | + it "should recognize non-+1 notes" do | ||
41 | + subject.notes << Factory(:note, note: "No +1 here", project: Factory(:project, path: 'plusone', code: 'plusone')) | ||
42 | + subject.should have(1).note | ||
43 | + subject.notes.first.upvote?.should be_false | ||
44 | + subject.upvotes.should == 0 | ||
45 | + end | ||
46 | + | ||
47 | + it "should recognize a single +1 note" do | ||
48 | + subject.notes << Factory(:note, note: "+1 This is awesome", project: Factory(:project, path: 'plusone', code: 'plusone')) | ||
49 | + subject.upvotes.should == 1 | ||
50 | + end | ||
51 | + | ||
52 | + it "should recognize a multiple +1 notes" do | ||
53 | + subject.notes << Factory(:note, note: "+1 This is awesome", project: Factory(:project, path: 'plusone', code: 'plusone')) | ||
54 | + subject.notes << Factory(:note, note: "+1 I want this", project: Factory(:project, path: 'plustwo', code: 'plustwo')) | ||
55 | + subject.upvotes.should == 2 | ||
56 | + end | ||
57 | + end | ||
27 | end | 58 | end |
28 | # == Schema Information | 59 | # == Schema Information |
29 | # | 60 | # |
spec/models/note_spec.rb
@@ -20,10 +20,29 @@ describe Note do | @@ -20,10 +20,29 @@ describe Note do | ||
20 | Note.today.where_values.should == ["created_at >= '#{Date.today}'"] | 20 | Note.today.where_values.should == ["created_at >= '#{Date.today}'"] |
21 | end | 21 | end |
22 | end | 22 | end |
23 | - | ||
24 | - describe "Commit notes" do | ||
25 | 23 | ||
26 | - before do | 24 | + describe "Voting score" do |
25 | + let(:project) { Factory(:project) } | ||
26 | + | ||
27 | + it "recognizes a neutral note" do | ||
28 | + note = Factory(:note, project: project, note: "This is not a +1 note") | ||
29 | + note.should_not be_upvote | ||
30 | + end | ||
31 | + | ||
32 | + it "recognizes a +1 note" do | ||
33 | + note = Factory(:note, project: project, note: "+1 for this") | ||
34 | + note.should be_upvote | ||
35 | + end | ||
36 | + | ||
37 | + it "recognizes a -1 note as no vote" do | ||
38 | + note = Factory(:note, project: project, note: "-1 for this") | ||
39 | + note.should_not be_upvote | ||
40 | + end | ||
41 | + end | ||
42 | + | ||
43 | + describe "Commit notes" do | ||
44 | + | ||
45 | + before do | ||
27 | @note = Factory :note, | 46 | @note = Factory :note, |
28 | :project => project, | 47 | :project => project, |
29 | :noteable_id => commit.id, | 48 | :noteable_id => commit.id, |
@@ -36,12 +55,12 @@ describe Note do | @@ -36,12 +55,12 @@ describe Note do | ||
36 | end | 55 | end |
37 | end | 56 | end |
38 | 57 | ||
39 | - describe "Pre-line commit notes" do | ||
40 | - before do | 58 | + describe "Pre-line commit notes" do |
59 | + before do | ||
41 | @note = Factory :note, | 60 | @note = Factory :note, |
42 | :project => project, | 61 | :project => project, |
43 | :noteable_id => commit.id, | 62 | :noteable_id => commit.id, |
44 | - :noteable_type => "Commit", | 63 | + :noteable_type => "Commit", |
45 | :line_code => "0_16_1" | 64 | :line_code => "0_16_1" |
46 | end | 65 | end |
47 | 66 |