Commit 0520ee44 authored by Felipe Artur's avatar Felipe Artur

Improve method names and add more specs

parent 85cdde8d
module Milestoneish module Milestoneish
def closed_items_count(user) def closed_items_count(user)
memoize_per_user(user, :closed_items_count) do memoize_per_user(user, :closed_items_count) do
(count_issues_by_state(user)['closed']&.length || 0) + merge_requests.closed_and_merged.size (count_issues_by_state(user)['closed'] || 0) + merge_requests.closed_and_merged.size
end end
end end
...@@ -12,7 +12,7 @@ module Milestoneish ...@@ -12,7 +12,7 @@ module Milestoneish
end end
def total_issues_count(user) def total_issues_count(user)
issues_visible_to_user(user).length count_issues_by_state(user).values.sum
end end
def complete?(user) def complete?(user)
...@@ -44,6 +44,14 @@ module Milestoneish ...@@ -44,6 +44,14 @@ module Milestoneish
end end
end end
def sorted_issues(user)
issues_visible_to_user(user).preload_associations.sort('label_priority')
end
def sorted_merge_requests
merge_requests.sort('label_priority')
end
def upcoming? def upcoming?
start_date && start_date.future? start_date && start_date.future?
end end
...@@ -62,17 +70,11 @@ module Milestoneish ...@@ -62,17 +70,11 @@ module Milestoneish
due_date && due_date.past? due_date && due_date.past?
end end
def sorted_merge_requests
merge_requests.sort('label_priority')
end
private private
def count_issues_by_state(user) def count_issues_by_state(user)
memoize_per_user(user, :count_issues_by_state) do memoize_per_user(user, :count_issues_by_state) do
# Need to group and count using ruby array to not break issues_visible_to_user(user).reorder(nil).group(:state).count
# label ordering. Also it saves a SQL query.
issues_visible_to_user(user).to_a.group_by(&:state)
end end
end end
...@@ -85,6 +87,6 @@ module Milestoneish ...@@ -85,6 +87,6 @@ module Milestoneish
# override in a class that includes this module to get a faster query # override in a class that includes this module to get a faster query
# from IssuesFinder # from IssuesFinder
def issues_finder_params def issues_finder_params
{ sort: 'label_priority' } {}
end end
end end
class DashboardMilestone < GlobalMilestone class DashboardMilestone < GlobalMilestone
def issues_finder_params def issues_finder_params
super.merge({ authorized_only: true }) { authorized_only: true }
end end
end end
...@@ -14,6 +14,6 @@ class GroupMilestone < GlobalMilestone ...@@ -14,6 +14,6 @@ class GroupMilestone < GlobalMilestone
end end
def issues_finder_params def issues_finder_params
super.merge({ group_id: group.id }) { group_id: group.id }
end end
end end
...@@ -187,6 +187,6 @@ class Milestone < ActiveRecord::Base ...@@ -187,6 +187,6 @@ class Milestone < ActiveRecord::Base
end end
def issues_finder_params def issues_finder_params
super.merge({ project_id: project_id }) { project_id: project_id }
end end
end end
...@@ -5,7 +5,6 @@ class IssuableEntity < Grape::Entity ...@@ -5,7 +5,6 @@ class IssuableEntity < Grape::Entity
expose :description expose :description
expose :lock_version expose :lock_version
expose :milestone_id expose :milestone_id
expose :position
expose :state expose :state
expose :title expose :title
expose :updated_by_id expose :updated_by_id
......
...@@ -68,10 +68,10 @@ ...@@ -68,10 +68,10 @@
.sidebar-collapsed-icon .sidebar-collapsed-icon
%strong %strong
= icon('hashtag', 'aria-hidden': 'true') = icon('hashtag', 'aria-hidden': 'true')
%span= milestone.issues_visible_to_user(current_user).length %span= milestone.issues_visible_to_user(current_user).count
.title.hide-collapsed .title.hide-collapsed
Issues Issues
%span.badge= milestone.issues_visible_to_user(current_user).length %span.badge= milestone.issues_visible_to_user(current_user).count
- if project && can?(current_user, :create_issue, project) - if project && can?(current_user, :create_issue, project)
= link_to new_namespace_project_issue_path(project.namespace, project, issue: { milestone_id: milestone.id }), class: "pull-right", title: "New Issue" do = link_to new_namespace_project_issue_path(project.namespace, project, issue: { milestone_id: milestone.id }), class: "pull-right", title: "New Issue" do
New issue New issue
...@@ -79,11 +79,11 @@ ...@@ -79,11 +79,11 @@
%span.milestone-stat %span.milestone-stat
= link_to milestones_browse_issuables_path(milestone, type: :issues) do = link_to milestones_browse_issuables_path(milestone, type: :issues) do
Open: Open:
= milestone.issues_visible_to_user(current_user).opened.length = milestone.issues_visible_to_user(current_user).opened.count
%span.milestone-stat %span.milestone-stat
= link_to milestones_browse_issuables_path(milestone, type: :issues, state: 'closed') do = link_to milestones_browse_issuables_path(milestone, type: :issues, state: 'closed') do
Closed: Closed:
= milestone.issues_visible_to_user(current_user).closed.length = milestone.issues_visible_to_user(current_user).closed.count
.block.merge-requests .block.merge-requests
.sidebar-collapsed-icon .sidebar-collapsed-icon
......
...@@ -31,7 +31,7 @@ ...@@ -31,7 +31,7 @@
.tab-content.milestone-content .tab-content.milestone-content
- if milestone.is_a?(GlobalMilestone) || can?(current_user, :read_issue, @project) - if milestone.is_a?(GlobalMilestone) || can?(current_user, :read_issue, @project)
.tab-pane.active#tab-issues{ data: { sort_endpoint: (sort_issues_namespace_project_milestone_path(@project.namespace, @project, @milestone) if @project && current_user) } } .tab-pane.active#tab-issues{ data: { sort_endpoint: (sort_issues_namespace_project_milestone_path(@project.namespace, @project, @milestone) if @project && current_user) } }
= render 'shared/milestones/issues_tab', issues: milestone.issues_visible_to_user(current_user).preload_associations, show_project_name: show_project_name, show_full_project_name: show_full_project_name = render 'shared/milestones/issues_tab', issues: milestone.sorted_issues(current_user), show_project_name: show_project_name, show_full_project_name: show_full_project_name
.tab-pane#tab-merge-requests .tab-pane#tab-merge-requests
-# loaded async -# loaded async
= render "shared/milestones/tab_loading" = render "shared/milestones/tab_loading"
......
...@@ -50,7 +50,7 @@ ...@@ -50,7 +50,7 @@
- project_name = group ? ms.project.name : ms.project.name_with_namespace - project_name = group ? ms.project.name : ms.project.name_with_namespace
= link_to project_name, namespace_project_milestone_path(ms.project.namespace, ms.project, ms) = link_to project_name, namespace_project_milestone_path(ms.project.namespace, ms.project, ms)
%td %td
= ms.issues_visible_to_user(current_user).opened.length = ms.issues_visible_to_user(current_user).opened.count
%td %td
- if ms.closed? - if ms.closed?
Closed Closed
......
...@@ -28,13 +28,13 @@ describe Milestone, 'Milestoneish' do ...@@ -28,13 +28,13 @@ describe Milestone, 'Milestoneish' do
project.team << [guest, :guest] project.team << [guest, :guest]
end end
describe '#issues_visible_to_user' do describe '#sorted_issues' do
it 'sorts issues by label priority' do it 'sorts issues by label priority' do
issue.labels << label_1 issue.labels << label_1
security_issue_1.labels << label_2 security_issue_1.labels << label_2
closed_issue_1.labels << label_3 closed_issue_1.labels << label_3
issues = milestone.issues_visible_to_user(member) issues = milestone.sorted_issues(member)
expect(issues.first).to eq(issue) expect(issues.first).to eq(issue)
expect(issues.second).to eq(security_issue_1) expect(issues.second).to eq(security_issue_1)
...@@ -43,14 +43,10 @@ describe Milestone, 'Milestoneish' do ...@@ -43,14 +43,10 @@ describe Milestone, 'Milestoneish' do
end end
describe '#sorted_merge_requests' do describe '#sorted_merge_requests' do
let(:merge_request_1) { create(:merge_request, :simple, source_project: project, source_branch: 'branch_1', milestone: milestone) }
let(:merge_request_2) { create(:merge_request, :simple, source_project: project, source_branch: 'branch_2', milestone: milestone) }
let(:merge_request_3) { create(:merge_request, :simple, source_project: project, source_branch: 'branch_3', milestone: milestone) }
it 'sorts merge requests by label priority' do it 'sorts merge requests by label priority' do
merge_request_2.labels << label_1 merge_request_1 = create(:labeled_merge_request, labels: [label_2], source_project: project, source_branch: 'branch_1', milestone: milestone)
merge_request_1.labels << label_2 merge_request_2 = create(:labeled_merge_request, labels: [label_1], source_project: project, source_branch: 'branch_2', milestone: milestone)
merge_request_3.labels << label_3 merge_request_3 = create(:labeled_merge_request, labels: [label_3], source_project: project, source_branch: 'branch_3', milestone: milestone)
merge_requests = milestone.sorted_merge_requests merge_requests = milestone.sorted_merge_requests
......
...@@ -3,7 +3,6 @@ require 'spec_helper' ...@@ -3,7 +3,6 @@ require 'spec_helper'
describe GroupMilestone, models: true do describe GroupMilestone, models: true do
let(:group) { create(:group) } let(:group) { create(:group) }
let(:project) { create(:empty_project, group: group) } let(:project) { create(:empty_project, group: group) }
let(:project_2) { create(:empty_project, group: group) }
let(:project_milestone) do let(:project_milestone) do
create(:milestone, title: "Milestone v1.2", project: project) create(:milestone, title: "Milestone v1.2", project: project)
end end
......
...@@ -5,6 +5,9 @@ describe API::Milestones do ...@@ -5,6 +5,9 @@ describe API::Milestones do
let!(:project) { create(:empty_project, namespace: user.namespace ) } let!(:project) { create(:empty_project, namespace: user.namespace ) }
let!(:closed_milestone) { create(:closed_milestone, project: project, title: 'version1', description: 'closed milestone') } let!(:closed_milestone) { create(:closed_milestone, project: project, title: 'version1', description: 'closed milestone') }
let!(:milestone) { create(:milestone, project: project, title: 'version2', description: 'open milestone') } let!(:milestone) { create(:milestone, project: project, title: 'version2', description: 'open milestone') }
let(:label_1) { create(:label, title: 'label_1', project: project, priority: 1) }
let(:label_2) { create(:label, title: 'label_2', project: project, priority: 2) }
let(:label_3) { create(:label, title: 'label_3', project: project) }
before do before do
project.team << [user, :developer] project.team << [user, :developer]
...@@ -228,6 +231,18 @@ describe API::Milestones do ...@@ -228,6 +231,18 @@ describe API::Milestones do
expect(json_response.first['milestone']['title']).to eq(milestone.title) expect(json_response.first['milestone']['title']).to eq(milestone.title)
end end
it 'returns project issues sorted by label priority' do
issue_1 = create(:labeled_issue, project: project, milestone: milestone, labels: [label_3])
issue_2 = create(:labeled_issue, project: project, milestone: milestone, labels: [label_1])
issue_3 = create(:labeled_issue, project: project, milestone: milestone, labels: [label_2])
get api("/projects/#{project.id}/milestones/#{milestone.id}/issues", user)
expect(json_response.first['id']).to eq(issue_2.id)
expect(json_response.second['id']).to eq(issue_3.id)
expect(json_response.third['id']).to eq(issue_1.id)
end
it 'matches V4 response schema for a list of issues' do it 'matches V4 response schema for a list of issues' do
get api("/projects/#{project.id}/milestones/#{milestone.id}/issues", user) get api("/projects/#{project.id}/milestones/#{milestone.id}/issues", user)
...@@ -244,8 +259,8 @@ describe API::Milestones do ...@@ -244,8 +259,8 @@ describe API::Milestones do
describe 'confidential issues' do describe 'confidential issues' do
let(:public_project) { create(:empty_project, :public) } let(:public_project) { create(:empty_project, :public) }
let(:milestone) { create(:milestone, project: public_project) } let(:milestone) { create(:milestone, project: public_project) }
let(:issue) { create(:issue, project: public_project, position: 2) } let(:issue) { create(:issue, project: public_project) }
let(:confidential_issue) { create(:issue, confidential: true, project: public_project, position: 1) } let(:confidential_issue) { create(:issue, confidential: true, project: public_project) }
before do before do
public_project.team << [user, :developer] public_project.team << [user, :developer]
...@@ -285,7 +300,10 @@ describe API::Milestones do ...@@ -285,7 +300,10 @@ describe API::Milestones do
expect(json_response.map { |issue| issue['id'] }).to include(issue.id) expect(json_response.map { |issue| issue['id'] }).to include(issue.id)
end end
it 'returns issues ordered by position asc' do it 'returns issues ordered by label priority' do
issue.labels << label_2
confidential_issue.labels << label_1
get api("/projects/#{public_project.id}/milestones/#{milestone.id}/issues", user) get api("/projects/#{public_project.id}/milestones/#{milestone.id}/issues", user)
expect(response).to have_http_status(200) expect(response).to have_http_status(200)
...@@ -299,8 +317,8 @@ describe API::Milestones do ...@@ -299,8 +317,8 @@ describe API::Milestones do
end end
describe 'GET /projects/:id/milestones/:milestone_id/merge_requests' do describe 'GET /projects/:id/milestones/:milestone_id/merge_requests' do
let(:merge_request) { create(:merge_request, source_project: project, position: 2) } let(:merge_request) { create(:merge_request, source_project: project) }
let(:another_merge_request) { create(:merge_request, :simple, source_project: project, position: 1) } let(:another_merge_request) { create(:merge_request, :simple, source_project: project) }
before do before do
milestone.merge_requests << merge_request milestone.merge_requests << merge_request
...@@ -318,6 +336,18 @@ describe API::Milestones do ...@@ -318,6 +336,18 @@ describe API::Milestones do
expect(json_response.first['milestone']['title']).to eq(milestone.title) expect(json_response.first['milestone']['title']).to eq(milestone.title)
end end
it 'returns project merge_requests sorted by label priority' do
merge_request_1 = create(:labeled_merge_request, source_branch: 'branch_1', source_project: project, milestone: milestone, labels: [label_2])
merge_request_2 = create(:labeled_merge_request, source_branch: 'branch_2', source_project: project, milestone: milestone, labels: [label_1])
merge_request_3 = create(:labeled_merge_request, source_branch: 'branch_3', source_project: project, milestone: milestone, labels: [label_3])
get api("/projects/#{project.id}/milestones/#{milestone.id}/merge_requests", user)
expect(json_response.first['id']).to eq(merge_request_2.id)
expect(json_response.second['id']).to eq(merge_request_1.id)
expect(json_response.third['id']).to eq(merge_request_3.id)
end
it 'returns a 404 error if milestone id not found' do it 'returns a 404 error if milestone id not found' do
get api("/projects/#{project.id}/milestones/1234/merge_requests", user) get api("/projects/#{project.id}/milestones/1234/merge_requests", user)
...@@ -339,6 +369,8 @@ describe API::Milestones do ...@@ -339,6 +369,8 @@ describe API::Milestones do
it 'returns merge_requests ordered by position asc' do it 'returns merge_requests ordered by position asc' do
milestone.merge_requests << another_merge_request milestone.merge_requests << another_merge_request
another_merge_request.labels << label_1
merge_request.labels << label_2
get api("/projects/#{project.id}/milestones/#{milestone.id}/merge_requests", user) get api("/projects/#{project.id}/milestones/#{milestone.id}/merge_requests", user)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment