From 9823d00e0b13224ae9e820e7d3f9fade69201e99 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Wed, 17 Feb 2016 11:32:02 -0200 Subject: [PATCH] Add ability to see and sort on vote count from Issues and MR lists --- CHANGELOG | 1 + app/assets/stylesheets/pages/issues.scss | 2 +- .../stylesheets/pages/merge_requests.scss | 4 +- app/helpers/sorting_helper.rb | 18 ++++++ app/models/concerns/issuable.rb | 25 ++++++++ app/views/projects/issues/_issue.html.haml | 19 +++++++ .../merge_requests/_merge_request.html.haml | 19 +++++++ app/views/shared/_sort_dropdown.html.haml | 4 ++ features/project/issues/issues.feature | 10 ++++ features/project/merge_requests.feature | 11 ++++ features/steps/project/issues/issues.rb | 57 +++++++++++++++++++ features/steps/project/merge_requests.rb | 50 ++++++++++++++++ features/steps/shared/issuable.rb | 16 ++++++ spec/factories/notes.rb | 14 +++++ 14 files changed, 247 insertions(+), 3 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 74bc366d203..08a9b8df4a0 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -62,6 +62,7 @@ v 8.5.0 (unreleased) - Replaces "Create merge request" link with one to the "Merge Request" when one exists - Fix CI builds badge, add a new link to builds badge, deprecate the old one - Fix broken link to project in build notification emails + - Ability to see and sort on vote count from Issues and MR lists v 8.4.4 - Update omniauth-saml gem to 1.4.2 diff --git a/app/assets/stylesheets/pages/issues.scss b/app/assets/stylesheets/pages/issues.scss index 8694bd654a7..1cc853dd4f5 100644 --- a/app/assets/stylesheets/pages/issues.scss +++ b/app/assets/stylesheets/pages/issues.scss @@ -24,7 +24,7 @@ display: inline-block; } - .issue-no-comments { + .issue-no-comments, .issue-no-votes { opacity: 0.5; } } diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index f033ff15f88..6b497cd56ed 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -163,7 +163,7 @@ display: inline-block; } - .merge-request-no-comments { + .merge-request-no-comments, .merge-request-no-votes { opacity: 0.5; } } @@ -236,4 +236,4 @@ } } } -} \ No newline at end of file +} diff --git a/app/helpers/sorting_helper.rb b/app/helpers/sorting_helper.rb index 241179b0212..f9026b887da 100644 --- a/app/helpers/sorting_helper.rb +++ b/app/helpers/sorting_helper.rb @@ -11,6 +11,8 @@ module SortingHelper sort_value_largest_repo => sort_title_largest_repo, sort_value_recently_signin => sort_title_recently_signin, sort_value_oldest_signin => sort_title_oldest_signin, + sort_value_downvotes => sort_title_downvotes, + sort_value_upvotes => sort_title_upvotes } end @@ -54,6 +56,14 @@ module SortingHelper 'Oldest sign in' end + def sort_title_downvotes + 'Least popular' + end + + def sort_title_upvotes + 'Most popular' + end + def sort_value_oldest_updated 'updated_asc' end @@ -93,4 +103,12 @@ module SortingHelper def sort_value_oldest_signin 'oldest_sign_in' end + + def sort_value_downvotes + 'downvotes_desc' + end + + def sort_value_upvotes + 'upvotes_desc' + end end diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 5136d0196a5..e5f089fb8a0 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -69,10 +69,35 @@ module Issuable case method.to_s when 'milestone_due_asc' then order_milestone_due_asc when 'milestone_due_desc' then order_milestone_due_desc + when 'downvotes_desc' then order_downvotes_desc + when 'upvotes_desc' then order_upvotes_desc else order_by(method) end end + + def order_downvotes_desc + order_votes_desc('thumbsdown') + end + + def order_upvotes_desc + order_votes_desc('thumbsup') + end + + def order_votes_desc(award_emoji_name) + issuable_table = self.arel_table + note_table = Note.arel_table + + join_clause = issuable_table.join(note_table, Arel::Nodes::OuterJoin).on( + note_table[:noteable_id].eq(issuable_table[:id]).and( + note_table[:noteable_type].eq(self.name).and( + note_table[:is_award].eq(true).and(note_table[:note].eq(award_emoji_name)) + ) + ) + ).join_sources + + joins(join_clause).group(issuable_table[:id]).reorder("COUNT(notes.id) DESC") + end end def today? diff --git a/app/views/projects/issues/_issue.html.haml b/app/views/projects/issues/_issue.html.haml index f9cf4910df3..5b0edcfa978 100644 --- a/app/views/projects/issues/_issue.html.haml +++ b/app/views/projects/issues/_issue.html.haml @@ -15,6 +15,25 @@ %li = link_to_member(@project, issue.assignee, name: false, title: "Assigned to :name") + - upvotes, downvotes = issue.upvotes, issue.downvotes + - if upvotes > 0 || downvotes > 0 + %li + = icon('thumbs-up') + = upvotes + - else + %li{ class: 'issue-no-votes' } + = icon('thumbs-up') + = upvotes + + - if upvotes > 0 || downvotes > 0 + %li + = icon('thumbs-down') + = downvotes + - else + %li{ class: 'issue-no-votes' } + = icon('thumbs-down') + = downvotes + - note_count = issue.notes.user.count - if note_count > 0 %li diff --git a/app/views/projects/merge_requests/_merge_request.html.haml b/app/views/projects/merge_requests/_merge_request.html.haml index e25bf917b43..b230b3a0110 100644 --- a/app/views/projects/merge_requests/_merge_request.html.haml +++ b/app/views/projects/merge_requests/_merge_request.html.haml @@ -24,6 +24,25 @@ %li = link_to_member(merge_request.source_project, merge_request.assignee, name: false, title: "Assigned to :name") + - upvotes, downvotes = merge_request.upvotes, merge_request.downvotes + - if upvotes > 0 || downvotes > 0 + %li + = icon('thumbs-up') + = upvotes + - else + %li{ class: 'merge-request-no-votes' } + = icon('thumbs-up') + = upvotes + + - if upvotes > 0 || downvotes > 0 + %li + = icon('thumbs-down') + = downvotes + - else + %li{ class: 'merge-request-no-votes' } + = icon('thumbs-down') + = downvotes + - note_count = merge_request.mr_and_commit_notes.user.count - if note_count > 0 %li diff --git a/app/views/shared/_sort_dropdown.html.haml b/app/views/shared/_sort_dropdown.html.haml index f09ab25276d..e3a6a5a68b6 100644 --- a/app/views/shared/_sort_dropdown.html.haml +++ b/app/views/shared/_sort_dropdown.html.haml @@ -20,3 +20,7 @@ = sort_title_milestone_soon = link_to page_filter_path(sort: sort_value_milestone_later) do = sort_title_milestone_later + = link_to page_filter_path(sort: sort_value_upvotes) do + = sort_title_upvotes + = link_to page_filter_path(sort: sort_value_downvotes) do + = sort_title_downvotes diff --git a/features/project/issues/issues.feature b/features/project/issues/issues.feature index ca2399d85a9..89af58dcef3 100644 --- a/features/project/issues/issues.feature +++ b/features/project/issues/issues.feature @@ -88,6 +88,16 @@ Feature: Project Issues And I visit dashboard merge requests page Then The list should be sorted by "Oldest updated" + @javascript + Scenario: Sort issues by upvotes/downvotes + Given project "Shop" have "Bugfix" open issue + And issue "Release 0.4" have 2 upvotes and 1 downvote + And issue "Tweet control" have 1 upvote and 2 downvotes + And I sort the list by "Most popular" + Then The list should be sorted by "Most popular" + And I sort the list by "Least popular" + Then The list should be sorted by "Least popular" + @javascript Scenario: I search issue Given I fill in issue search with "Re" diff --git a/features/project/merge_requests.feature b/features/project/merge_requests.feature index 5995e787961..495f25f28e7 100644 --- a/features/project/merge_requests.feature +++ b/features/project/merge_requests.feature @@ -107,6 +107,17 @@ Feature: Project Merge Requests And I visit dashboard merge requests page Then The list should be sorted by "Oldest updated" + @javascript + Scenario: Sort merge requests by upvotes/downvotes + Given project "Shop" have "Bug NS-05" open merge request with diffs inside + And project "Shop" have "Bug NS-06" open merge request + And merge request "Bug NS-04" have 2 upvotes and 1 downvote + And merge request "Bug NS-06" have 1 upvote and 2 downvotes + And I sort the list by "Most popular" + Then The list should be sorted by "Most popular" + And I sort the list by "Least popular" + Then The list should be sorted by "Least popular" + @javascript Scenario: Visiting Merge Requests after commenting on diffs Given project "Shop" have "Bug NS-05" open merge request with diffs inside diff --git a/features/steps/project/issues/issues.rb b/features/steps/project/issues/issues.rb index 09a89e99831..54d64bacc52 100644 --- a/features/steps/project/issues/issues.rb +++ b/features/steps/project/issues/issues.rb @@ -174,6 +174,13 @@ class Spinach::Features::ProjectIssues < Spinach::FeatureSteps author: project.users.first) end + step 'project "Shop" have "Bugfix" open issue' do + create(:issue, + title: "Bugfix", + project: project, + author: project.users.first) + end + step 'project "Shop" have "Release 0.3" closed issue' do create(:closed_issue, title: "Release 0.3", @@ -181,6 +188,56 @@ class Spinach::Features::ProjectIssues < Spinach::FeatureSteps author: project.users.first) end + step 'issue "Release 0.4" have 2 upvotes and 1 downvote' do + issue = Issue.find_by(title: 'Release 0.4') + create_list(:upvote_note, 2, project: project, noteable: issue) + create(:downvote_note, project: project, noteable: issue) + end + + step 'issue "Tweet control" have 1 upvote and 2 downvotes' do + issue = Issue.find_by(title: 'Tweet control') + create(:upvote_note, project: project, noteable: issue) + create_list(:downvote_note, 2, project: project, noteable: issue) + end + + step 'The list should be sorted by "Least popular"' do + page.within '.issues-list' do + page.within 'li.issue:nth-child(1)' do + expect(page).to have_content 'Tweet control' + expect(page).to have_content '1 2' + end + + page.within 'li.issue:nth-child(2)' do + expect(page).to have_content 'Release 0.4' + expect(page).to have_content '2 1' + end + + page.within 'li.issue:nth-child(3)' do + expect(page).to have_content 'Bugfix' + expect(page).to have_content '0 0' + end + end + end + + step 'The list should be sorted by "Most popular"' do + page.within '.issues-list' do + page.within 'li.issue:nth-child(1)' do + expect(page).to have_content 'Release 0.4' + expect(page).to have_content '2 1' + end + + page.within 'li.issue:nth-child(2)' do + expect(page).to have_content 'Tweet control' + expect(page).to have_content '1 2' + end + + page.within 'li.issue:nth-child(3)' do + expect(page).to have_content 'Bugfix' + expect(page).to have_content '0 0' + end + end + end + step 'empty project "Empty Project"' do create :empty_project, name: 'Empty Project', namespace: @user.namespace end diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb index 337893e6209..2160d8645fd 100644 --- a/features/steps/project/merge_requests.rb +++ b/features/steps/project/merge_requests.rb @@ -138,6 +138,56 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps author: project.users.first) end + step 'merge request "Bug NS-04" have 2 upvotes and 1 downvote' do + merge_request = MergeRequest.find_by(title: 'Bug NS-04') + create_list(:upvote_note, 2, project: project, noteable: merge_request) + create(:downvote_note, project: project, noteable: merge_request) + end + + step 'merge request "Bug NS-06" have 1 upvote and 2 downvotes' do + merge_request = MergeRequest.find_by(title: 'Bug NS-06') + create(:upvote_note, project: project, noteable: merge_request) + create_list(:downvote_note, 2, project: project, noteable: merge_request) + end + + step 'The list should be sorted by "Least popular"' do + page.within '.mr-list' do + page.within 'li.merge-request:nth-child(1)' do + expect(page).to have_content 'Bug NS-06' + expect(page).to have_content '1 2' + end + + page.within 'li.merge-request:nth-child(2)' do + expect(page).to have_content 'Bug NS-04' + expect(page).to have_content '2 1' + end + + page.within 'li.merge-request:nth-child(3)' do + expect(page).to have_content 'Bug NS-05' + expect(page).to have_content '0 0' + end + end + end + + step 'The list should be sorted by "Most popular"' do + page.within '.mr-list' do + page.within 'li.merge-request:nth-child(1)' do + expect(page).to have_content 'Bug NS-04' + expect(page).to have_content '2 1' + end + + page.within 'li.merge-request:nth-child(2)' do + expect(page).to have_content 'Bug NS-06' + expect(page).to have_content '1 2' + end + + page.within 'li.merge-request:nth-child(3)' do + expect(page).to have_content 'Bug NS-05' + expect(page).to have_content '0 0' + end + end + end + step 'I click on the Changes tab' do page.within '.merge-request-tabs' do click_link 'Changes' diff --git a/features/steps/shared/issuable.rb b/features/steps/shared/issuable.rb index 2117feaedb8..ae10c6069a9 100644 --- a/features/steps/shared/issuable.rb +++ b/features/steps/shared/issuable.rb @@ -113,6 +113,22 @@ module SharedIssuable end end + step 'I sort the list by "Least popular"' do + find('button.dropdown-toggle.btn').click + + page.within('ul.dropdown-menu.dropdown-menu-align-right li') do + click_link 'Least popular' + end + end + + step 'I sort the list by "Most popular"' do + find('button.dropdown-toggle.btn').click + + page.within('ul.dropdown-menu.dropdown-menu-align-right li') do + click_link 'Most popular' + end + end + step 'The list should be sorted by "Oldest updated"' do page.within('div.dropdown.inline.prepend-left-10') do expect(page.find('button.dropdown-toggle.btn')).to have_content('Oldest updated') diff --git a/spec/factories/notes.rb b/spec/factories/notes.rb index 35a20adeef3..32c202891d8 100644 --- a/spec/factories/notes.rb +++ b/spec/factories/notes.rb @@ -34,6 +34,8 @@ FactoryGirl.define do factory :note_on_merge_request_diff, traits: [:on_merge_request, :on_diff] factory :note_on_project_snippet, traits: [:on_project_snippet] factory :system_note, traits: [:system] + factory :downvote_note, traits: [:award, :downvote] + factory :upvote_note, traits: [:award, :upvote] trait :on_commit do project @@ -65,6 +67,18 @@ FactoryGirl.define do system true end + trait :award do + is_award true + end + + trait :downvote do + note "thumbsdown" + end + + trait :upvote do + note "thumbsup" + end + trait :with_attachment do attachment { fixture_file_upload(Rails.root + "spec/fixtures/dk.png", "`/png") } end