summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorFelipe Artur <felipefac@gmail.com>2017-06-09 15:53:32 -0300
committerFelipe Artur <felipefac@gmail.com>2017-06-16 12:03:15 -0300
commit0520ee44985528d3076df1208bda7c6c7ff8ec79 (patch)
treec078ce331252d8c3ce6cd6e6f77f7646b5e143b1
parent85cdde8d3340dc0be28749225bf00923c84e5924 (diff)
downloadgitlab-ce-0520ee44985528d3076df1208bda7c6c7ff8ec79.tar.gz
Improve method names and add more specs
-rw-r--r--app/models/concerns/milestoneish.rb22
-rw-r--r--app/models/dashboard_milestone.rb2
-rw-r--r--app/models/group_milestone.rb2
-rw-r--r--app/models/milestone.rb2
-rw-r--r--app/serializers/issuable_entity.rb1
-rw-r--r--app/views/shared/milestones/_sidebar.html.haml8
-rw-r--r--app/views/shared/milestones/_tabs.html.haml2
-rw-r--r--app/views/shared/milestones/_top.html.haml2
-rw-r--r--spec/models/concerns/milestoneish_spec.rb14
-rw-r--r--spec/models/group_milestone_spec.rb1
-rw-r--r--spec/requests/api/milestones_spec.rb42
11 files changed, 63 insertions, 35 deletions
diff --git a/app/models/concerns/milestoneish.rb b/app/models/concerns/milestoneish.rb
index 0f4d4d841ba..01599ce49c6 100644
--- a/app/models/concerns/milestoneish.rb
+++ b/app/models/concerns/milestoneish.rb
@@ -1,7 +1,7 @@
module Milestoneish
def closed_items_count(user)
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
@@ -12,7 +12,7 @@ module Milestoneish
end
def total_issues_count(user)
- issues_visible_to_user(user).length
+ count_issues_by_state(user).values.sum
end
def complete?(user)
@@ -44,6 +44,14 @@ module Milestoneish
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?
start_date && start_date.future?
end
@@ -62,17 +70,11 @@ module Milestoneish
due_date && due_date.past?
end
- def sorted_merge_requests
- merge_requests.sort('label_priority')
- end
-
private
def count_issues_by_state(user)
memoize_per_user(user, :count_issues_by_state) do
- # Need to group and count using ruby array to not break
- # label ordering. Also it saves a SQL query.
- issues_visible_to_user(user).to_a.group_by(&:state)
+ issues_visible_to_user(user).reorder(nil).group(:state).count
end
end
@@ -85,6 +87,6 @@ module Milestoneish
# override in a class that includes this module to get a faster query
# from IssuesFinder
def issues_finder_params
- { sort: 'label_priority' }
+ {}
end
end
diff --git a/app/models/dashboard_milestone.rb b/app/models/dashboard_milestone.rb
index e2af4040870..646c1e5ce1a 100644
--- a/app/models/dashboard_milestone.rb
+++ b/app/models/dashboard_milestone.rb
@@ -1,5 +1,5 @@
class DashboardMilestone < GlobalMilestone
def issues_finder_params
- super.merge({ authorized_only: true })
+ { authorized_only: true }
end
end
diff --git a/app/models/group_milestone.rb b/app/models/group_milestone.rb
index 4db20470220..86d38e5468b 100644
--- a/app/models/group_milestone.rb
+++ b/app/models/group_milestone.rb
@@ -14,6 +14,6 @@ class GroupMilestone < GlobalMilestone
end
def issues_finder_params
- super.merge({ group_id: group.id })
+ { group_id: group.id }
end
end
diff --git a/app/models/milestone.rb b/app/models/milestone.rb
index 14647602c02..0a6fc064aec 100644
--- a/app/models/milestone.rb
+++ b/app/models/milestone.rb
@@ -187,6 +187,6 @@ class Milestone < ActiveRecord::Base
end
def issues_finder_params
- super.merge({ project_id: project_id })
+ { project_id: project_id }
end
end
diff --git a/app/serializers/issuable_entity.rb b/app/serializers/issuable_entity.rb
index 65b204d4dd2..bd5211b8e58 100644
--- a/app/serializers/issuable_entity.rb
+++ b/app/serializers/issuable_entity.rb
@@ -5,7 +5,6 @@ class IssuableEntity < Grape::Entity
expose :description
expose :lock_version
expose :milestone_id
- expose :position
expose :state
expose :title
expose :updated_by_id
diff --git a/app/views/shared/milestones/_sidebar.html.haml b/app/views/shared/milestones/_sidebar.html.haml
index e75edd750ff..9bb87640319 100644
--- a/app/views/shared/milestones/_sidebar.html.haml
+++ b/app/views/shared/milestones/_sidebar.html.haml
@@ -68,10 +68,10 @@
.sidebar-collapsed-icon
%strong
= 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
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)
= link_to new_namespace_project_issue_path(project.namespace, project, issue: { milestone_id: milestone.id }), class: "pull-right", title: "New Issue" do
New issue
@@ -79,11 +79,11 @@
%span.milestone-stat
= link_to milestones_browse_issuables_path(milestone, type: :issues) do
Open:
- = milestone.issues_visible_to_user(current_user).opened.length
+ = milestone.issues_visible_to_user(current_user).opened.count
%span.milestone-stat
= link_to milestones_browse_issuables_path(milestone, type: :issues, state: 'closed') do
Closed:
- = milestone.issues_visible_to_user(current_user).closed.length
+ = milestone.issues_visible_to_user(current_user).closed.count
.block.merge-requests
.sidebar-collapsed-icon
diff --git a/app/views/shared/milestones/_tabs.html.haml b/app/views/shared/milestones/_tabs.html.haml
index b1d24578daf..4de8a6cb15f 100644
--- a/app/views/shared/milestones/_tabs.html.haml
+++ b/app/views/shared/milestones/_tabs.html.haml
@@ -31,7 +31,7 @@
.tab-content.milestone-content
- 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) } }
- = 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
-# loaded async
= render "shared/milestones/tab_loading"
diff --git a/app/views/shared/milestones/_top.html.haml b/app/views/shared/milestones/_top.html.haml
index 36c2969a555..2562f085338 100644
--- a/app/views/shared/milestones/_top.html.haml
+++ b/app/views/shared/milestones/_top.html.haml
@@ -50,7 +50,7 @@
- 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)
%td
- = ms.issues_visible_to_user(current_user).opened.length
+ = ms.issues_visible_to_user(current_user).opened.count
%td
- if ms.closed?
Closed
diff --git a/spec/models/concerns/milestoneish_spec.rb b/spec/models/concerns/milestoneish_spec.rb
index 7616123b486..cefe7fb6fea 100644
--- a/spec/models/concerns/milestoneish_spec.rb
+++ b/spec/models/concerns/milestoneish_spec.rb
@@ -28,13 +28,13 @@ describe Milestone, 'Milestoneish' do
project.team << [guest, :guest]
end
- describe '#issues_visible_to_user' do
+ describe '#sorted_issues' do
it 'sorts issues by label priority' do
issue.labels << label_1
security_issue_1.labels << label_2
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.second).to eq(security_issue_1)
@@ -43,14 +43,10 @@ describe Milestone, 'Milestoneish' do
end
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
- merge_request_2.labels << label_1
- merge_request_1.labels << label_2
- merge_request_3.labels << label_3
+ merge_request_1 = create(:labeled_merge_request, labels: [label_2], source_project: project, source_branch: 'branch_1', milestone: milestone)
+ merge_request_2 = create(:labeled_merge_request, labels: [label_1], source_project: project, source_branch: 'branch_2', milestone: milestone)
+ 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
diff --git a/spec/models/group_milestone_spec.rb b/spec/models/group_milestone_spec.rb
index b81d985b15c..916afb7aaf5 100644
--- a/spec/models/group_milestone_spec.rb
+++ b/spec/models/group_milestone_spec.rb
@@ -3,7 +3,6 @@ require 'spec_helper'
describe GroupMilestone, models: true do
let(:group) { create(:group) }
let(:project) { create(:empty_project, group: group) }
- let(:project_2) { create(:empty_project, group: group) }
let(:project_milestone) do
create(:milestone, title: "Milestone v1.2", project: project)
end
diff --git a/spec/requests/api/milestones_spec.rb b/spec/requests/api/milestones_spec.rb
index 40934c25afc..ab5ea3e8f2c 100644
--- a/spec/requests/api/milestones_spec.rb
+++ b/spec/requests/api/milestones_spec.rb
@@ -5,6 +5,9 @@ describe API::Milestones do
let!(:project) { create(:empty_project, namespace: user.namespace ) }
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(: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
project.team << [user, :developer]
@@ -228,6 +231,18 @@ describe API::Milestones do
expect(json_response.first['milestone']['title']).to eq(milestone.title)
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
get api("/projects/#{project.id}/milestones/#{milestone.id}/issues", user)
@@ -244,8 +259,8 @@ describe API::Milestones do
describe 'confidential issues' do
let(:public_project) { create(:empty_project, :public) }
let(:milestone) { create(:milestone, project: public_project) }
- let(:issue) { create(:issue, project: public_project, position: 2) }
- let(:confidential_issue) { create(:issue, confidential: true, project: public_project, position: 1) }
+ let(:issue) { create(:issue, project: public_project) }
+ let(:confidential_issue) { create(:issue, confidential: true, project: public_project) }
before do
public_project.team << [user, :developer]
@@ -285,7 +300,10 @@ describe API::Milestones do
expect(json_response.map { |issue| issue['id'] }).to include(issue.id)
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)
expect(response).to have_http_status(200)
@@ -299,8 +317,8 @@ describe API::Milestones do
end
describe 'GET /projects/:id/milestones/:milestone_id/merge_requests' do
- let(:merge_request) { create(:merge_request, source_project: project, position: 2) }
- let(:another_merge_request) { create(:merge_request, :simple, source_project: project, position: 1) }
+ let(:merge_request) { create(:merge_request, source_project: project) }
+ let(:another_merge_request) { create(:merge_request, :simple, source_project: project) }
before do
milestone.merge_requests << merge_request
@@ -318,6 +336,18 @@ describe API::Milestones do
expect(json_response.first['milestone']['title']).to eq(milestone.title)
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
get api("/projects/#{project.id}/milestones/1234/merge_requests", user)
@@ -339,6 +369,8 @@ describe API::Milestones do
it 'returns merge_requests ordered by position asc' do
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)