diff options
| -rw-r--r-- | CHANGELOG | 1 | ||||
| -rw-r--r-- | app/assets/stylesheets/generic/typography.scss | 4 | ||||
| -rw-r--r-- | app/models/concerns/issuable.rb | 16 | ||||
| -rw-r--r-- | app/models/note.rb | 17 | ||||
| -rw-r--r-- | app/views/projects/notes/_note.html.haml | 26 | ||||
| -rw-r--r-- | spec/lib/votes_spec.rb | 100 |
6 files changed, 128 insertions, 36 deletions
diff --git a/CHANGELOG b/CHANGELOG index f804d658aab..e3fc463621c 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -15,6 +15,7 @@ v 7.8.0 - - - Show tags in commit view (Hannes Rosenögger) + - Only count a user's vote once on a merge request or issue (Michael Clarke) - - - diff --git a/app/assets/stylesheets/generic/typography.scss b/app/assets/stylesheets/generic/typography.scss index 385a627b4be..58243bc5ba2 100644 --- a/app/assets/stylesheets/generic/typography.scss +++ b/app/assets/stylesheets/generic/typography.scss @@ -128,3 +128,7 @@ a:focus { textarea.js-gfm-input { font-family: $monospace_font; } + +.strikethrough { + text-decoration: line-through; +}
\ No newline at end of file diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index f49708fd6eb..b8bee0d0ec0 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -88,7 +88,7 @@ module Issuable # Return the number of -1 comments (downvotes) def downvotes - notes.select(&:downvote?).size + filter_superceded_votes(notes.select(&:downvote?), notes).size end def downvotes_in_percent @@ -101,7 +101,7 @@ module Issuable # Return the number of +1 comments (upvotes) def upvotes - notes.select(&:upvote?).size + filter_superceded_votes(notes.select(&:upvote?), notes).size end def upvotes_in_percent @@ -154,4 +154,16 @@ module Issuable self.labels << label end end + + private + + def filter_superceded_votes(votes, notes) + filteredvotes = [] + votes + votes.each do |vote| + if vote.superceded?(notes) + filteredvotes.delete(vote) + end + end + filteredvotes + end end diff --git a/app/models/note.rb b/app/models/note.rb index e99bc2668d6..1b7e412e9c5 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -459,6 +459,23 @@ class Note < ActiveRecord::Base ) end + def superceded?(notes) + return false unless vote? + notes.each do |note| + next if note == self + if note.vote? && + self[:author_id] == note[:author_id] && + self[:created_at] <= note[:created_at] + return true + end + end + false + end + + def vote? + upvote? || downvote? + end + def votable? for_issue? || (for_merge_request? && !for_diff_line?) end diff --git a/app/views/projects/notes/_note.html.haml b/app/views/projects/notes/_note.html.haml index 691c169b620..88c7b7ccf1a 100644 --- a/app/views/projects/notes/_note.html.haml +++ b/app/views/projects/notes/_note.html.haml @@ -28,14 +28,24 @@ %span.note-last-update = note_timestamp(note) - - if note.upvote? - %span.vote.upvote.label.label-success - %i.fa.fa-thumbs-up - \+1 - - if note.downvote? - %span.vote.downvote.label.label-danger - %i.fa.fa-thumbs-down - \-1 + - if note.superceded?(@notes) + - if note.upvote? + %span.vote.upvote.label.label-gray.strikethrough + %i.fa.fa-thumbs-up + \+1 + - if note.downvote? + %span.vote.downvote.label.label-gray.strikethrough + %i.fa.fa-thumbs-down + \-1 + - else + - if note.upvote? + %span.vote.upvote.label.label-success + %i.fa.fa-thumbs-up + \+1 + - if note.downvote? + %span.vote.downvote.label.label-danger + %i.fa.fa-thumbs-down + \-1 .note-body diff --git a/spec/lib/votes_spec.rb b/spec/lib/votes_spec.rb index a3c353d5eab..63692814b97 100644 --- a/spec/lib/votes_spec.rb +++ b/spec/lib/votes_spec.rb @@ -20,11 +20,17 @@ describe Issue, 'Votes' do issue.upvotes.should == 1 end - it "should recognize multiple +1 notes" do - add_note "+1 This is awesome" - add_note "+1 I want this" + it 'should recognize multiple +1 notes' do + add_note '+1 This is awesome', create(:user) + add_note '+1 I want this', create(:user) issue.upvotes.should == 2 end + + it 'should not count 2 +1 votes from the same user' do + add_note '+1 This is awesome' + add_note '+1 I want this' + issue.upvotes.should == 1 + end end describe "#downvotes" do @@ -45,8 +51,8 @@ describe Issue, 'Votes' do end it "should recognize multiple -1 notes" do - add_note "-1 This is bad" - add_note "-1 Away with this" + add_note('-1 This is bad', create(:user)) + add_note('-1 Away with this', create(:user)) issue.downvotes.should == 2 end end @@ -73,11 +79,17 @@ describe Issue, 'Votes' do end it "should recognize multiple notes" do - add_note "+1 This is awesome" - add_note "-1 This is bad" - add_note "+1 I want this" + add_note('+1 This is awesome', create(:user)) + add_note('-1 This is bad', create(:user)) + add_note('+1 I want this', create(:user)) issue.votes_count.should == 3 end + + it 'should not count 2 -1 votes from the same user' do + add_note '-1 This is suspicious' + add_note '-1 This is bad' + issue.votes_count.should == 1 + end end describe "#upvotes_in_percent" do @@ -90,17 +102,17 @@ describe Issue, 'Votes' do issue.upvotes_in_percent.should == 100 end - it "should count multiple +1 notes as 100%" do - add_note "+1 This is awesome" - add_note "+1 I want this" + it 'should count multiple +1 notes as 100%' do + add_note('+1 This is awesome', create(:user)) + add_note('+1 I want this', create(:user)) issue.upvotes_in_percent.should == 100 end - it "should count fractions for multiple +1 and -1 notes correctly" do - add_note "+1 This is awesome" - add_note "+1 I want this" - add_note "-1 This is bad" - add_note "+1 me too" + it 'should count fractions for multiple +1 and -1 notes correctly' do + add_note('+1 This is awesome', create(:user)) + add_note('+1 I want this', create(:user)) + add_note('-1 This is bad', create(:user)) + add_note('+1 me too', create(:user)) issue.upvotes_in_percent.should == 75 end end @@ -115,22 +127,58 @@ describe Issue, 'Votes' do issue.downvotes_in_percent.should == 100 end - it "should count multiple -1 notes as 100%" do - add_note "-1 This is bad" - add_note "-1 Away with this" + it 'should count multiple -1 notes as 100%' do + add_note('-1 This is bad', create(:user)) + add_note('-1 Away with this', create(:user)) issue.downvotes_in_percent.should == 100 end - it "should count fractions for multiple +1 and -1 notes correctly" do - add_note "+1 This is awesome" - add_note "+1 I want this" - add_note "-1 This is bad" - add_note "+1 me too" + it 'should count fractions for multiple +1 and -1 notes correctly' do + add_note('+1 This is awesome', create(:user)) + add_note('+1 I want this', create(:user)) + add_note('-1 This is bad', create(:user)) + add_note('+1 me too', create(:user)) issue.downvotes_in_percent.should == 25 end end - def add_note(text) - issue.notes << create(:note, note: text, project: issue.project) + describe '#filter_superceded_votes' do + + it 'should count a users vote only once amongst multiple votes' do + add_note('-1 This needs work before I will accept it') + add_note('+1 I want this', create(:user)) + add_note('+1 This is is awesome', create(:user)) + add_note('+1 this looks good now') + add_note('+1 This is awesome', create(:user)) + add_note('+1 me too', create(:user)) + issue.downvotes.should == 0 + issue.upvotes.should == 5 + end + + it 'should count each users vote only once' do + add_note '-1 This needs work before it will be accepted' + add_note '+1 I like this' + add_note '+1 I still like this' + add_note '+1 I really like this' + add_note '+1 Give me this now!!!!' + p issue.downvotes.should == 0 + p issue.upvotes.should == 1 + end + + it 'should count a users vote only once without caring about comments' do + add_note '-1 This needs work before it will be accepted' + add_note 'Comment 1' + add_note 'Another comment' + add_note '+1 vote' + add_note 'final comment' + p issue.downvotes.should == 0 + p issue.upvotes.should == 1 + end + + end + + def add_note(text, author = issue.author) + issue.notes << create(:note, note: text, project: issue.project, + author_id: author.id) end end |