diff options
author | Sean McGivern <sean@mcgivern.me.uk> | 2017-06-16 16:29:35 +0000 |
---|---|---|
committer | Sean McGivern <sean@mcgivern.me.uk> | 2017-06-16 16:29:35 +0000 |
commit | c6701d45474acab6a2d687d2e2ffdaac5c0d5491 (patch) | |
tree | 5e18ab8b3e41bed293e4e1748f780c0fad94fd79 | |
parent | 451cbe1f69d65b24568a53ec7aa4a7fba2c46933 (diff) | |
parent | 0520ee44985528d3076df1208bda7c6c7ff8ec79 (diff) | |
download | gitlab-ce-c6701d45474acab6a2d687d2e2ffdaac5c0d5491.tar.gz |
Merge branch 'issue_20900' into 'master'
Remove issuable drag and drop and sorting from milestone view
Closes #20900
See merge request !11950
21 files changed, 182 insertions, 347 deletions
diff --git a/app/assets/javascripts/milestone.js b/app/assets/javascripts/milestone.js index 07ede5ee913..3e07ec4d0aa 100644 --- a/app/assets/javascripts/milestone.js +++ b/app/assets/javascripts/milestone.js @@ -4,87 +4,7 @@ (function() { this.Milestone = (function() { - Milestone.updateIssue = function(li, issue_url, data) { - return $.ajax({ - type: "PUT", - url: issue_url, - data: data, - success: function(_data) { - return Milestone.successCallback(_data, li); - }, - error: function(data) { - return new Flash("Issue update failed", 'alert'); - }, - dataType: "json" - }); - }; - - Milestone.sortIssues = function(url, data) { - return $.ajax({ - type: "PUT", - url, - data: data, - success: function(_data) { - return Milestone.successCallback(_data); - }, - error: function() { - return new Flash("Issues update failed", 'alert'); - }, - dataType: "json" - }); - }; - - Milestone.sortMergeRequests = function(url, data) { - return $.ajax({ - type: "PUT", - url, - data: data, - success: function(_data) { - return Milestone.successCallback(_data); - }, - error: function(data) { - return new Flash("Issue update failed", 'alert'); - }, - dataType: "json" - }); - }; - - Milestone.updateMergeRequest = function(li, merge_request_url, data) { - return $.ajax({ - type: "PUT", - url: merge_request_url, - data: data, - success: function(_data) { - return Milestone.successCallback(_data, li); - }, - error: function(data) { - return new Flash("Issue update failed", 'alert'); - }, - dataType: "json" - }); - }; - - Milestone.successCallback = function(data, element) { - const $avatarContainer = $(element).find('.assignee-icon'); - $avatarContainer.empty(); - - if (data.assignees && data.assignees.length > 0) { - const $avatars = data.assignees.map((assignee) => { - const img_tag = $('<img/>'); - img_tag.attr('src', assignee.avatar_url); - img_tag.addClass('avatar s16'); - return img_tag; - }); - - $avatarContainer.append($avatars); - } - }; - function Milestone() { - this.issuesSortEndpoint = $('#tab-issues').data('sort-endpoint'); - this.mergeRequestsSortEndpoint = $('#tab-merge-requests').data('sort-endpoint'); - - this.bindIssuesSorting(); this.bindTabsSwitching(); // Load merge request tab if it is active @@ -94,22 +14,6 @@ this.loadInitialTab(); } - Milestone.prototype.bindIssuesSorting = function() { - if (!this.issuesSortEndpoint) return; - - $('#issues-list-unassigned, #issues-list-ongoing, #issues-list-closed').each(function (i, el) { - this.createSortable(el, { - group: 'issue-list', - listEls: $('.issues-sortable-list'), - fieldName: 'issue', - sortCallback: (data) => { - Milestone.sortIssues(this.issuesSortEndpoint, data); - }, - updateCallback: Milestone.updateIssue, - }); - }.bind(this)); - }; - Milestone.prototype.bindTabsSwitching = function() { return $('a[data-toggle="tab"]').on('show.bs.tab', (e) => { const $target = $(e.target); @@ -119,69 +23,6 @@ }); }; - Milestone.prototype.bindMergeRequestSorting = function() { - if (!this.mergeRequestsSortEndpoint) return; - - $("#merge_requests-list-unassigned, #merge_requests-list-ongoing, #merge_requests-list-closed").each(function (i, el) { - this.createSortable(el, { - group: 'merge-request-list', - listEls: $(".merge_requests-sortable-list:not(#merge_requests-list-merged)"), - fieldName: 'merge_request', - sortCallback: (data) => { - Milestone.sortMergeRequests(this.mergeRequestsSortEndpoint, data); - }, - updateCallback: Milestone.updateMergeRequest, - }); - }.bind(this)); - }; - - Milestone.prototype.createSortable = function(el, opts) { - return Sortable.create(el, { - group: opts.group, - filter: '.is-disabled', - forceFallback: true, - onStart: function(e) { - opts.listEls.css('min-height', e.item.offsetHeight); - }, - onEnd: function () { - opts.listEls.css("min-height", "0px"); - }, - onUpdate: function(e) { - var ids = this.toArray(), - data; - - if (ids.length) { - data = ids.map(function(id) { - return 'sortable_' + opts.fieldName + '[]=' + id; - }).join('&'); - - opts.sortCallback(data); - } - }, - onAdd: function (e) { - var data, issuableId, issuableUrl, newState; - newState = e.to.dataset.state; - issuableUrl = e.item.dataset.url; - data = (function() { - switch (newState) { - case 'ongoing': - return `${opts.fieldName}[assignee_ids][]=${gon.current_user_id}`; - case 'unassigned': - return `${opts.fieldName}[assignee_ids][]=0`; - case 'closed': - return opts.fieldName + '[state_event]=close'; - } - })(); - if (e.from.dataset.state === 'closed') { - data += '&' + opts.fieldName + '[state_event]=reopen'; - } - - opts.updateCallback(e.item, issuableUrl, data); - this.options.onUpdate.call(this, e); - } - }); - }; - Milestone.prototype.loadInitialTab = function() { const $target = $(`.js-milestone-tabs a[href="${location.hash}"]`); @@ -203,10 +44,6 @@ .done((data) => { $(tabElId).html(data.html); $target.addClass('is-loaded'); - - if (tabElId === '#tab-merge-requests') { - this.bindMergeRequestSorting(); - } }); } }; diff --git a/app/assets/stylesheets/pages/milestone.scss b/app/assets/stylesheets/pages/milestone.scss index 335e587b8f4..55e0ee1936e 100644 --- a/app/assets/stylesheets/pages/milestone.scss +++ b/app/assets/stylesheets/pages/milestone.scss @@ -111,8 +111,8 @@ } } -.issues-sortable-list, -.merge_requests-sortable-list { +.milestone-issues-list, +.milestone-merge_requests-list { .issuable-detail { display: block; margin-top: 7px; @@ -197,6 +197,4 @@ .issuable-row { background-color: $white-light; - cursor: -webkit-grab; - cursor: grab; } diff --git a/app/controllers/concerns/milestone_actions.rb b/app/controllers/concerns/milestone_actions.rb index b2536a1c949..1ff785ac2ca 100644 --- a/app/controllers/concerns/milestone_actions.rb +++ b/app/controllers/concerns/milestone_actions.rb @@ -6,7 +6,7 @@ module MilestoneActions format.html { redirect_to milestone_redirect_path } format.json do render json: tabs_json("shared/milestones/_merge_requests_tab", { - merge_requests: @milestone.merge_requests, + merge_requests: @milestone.sorted_merge_requests, show_project_name: true }) end diff --git a/app/controllers/projects/milestones_controller.rb b/app/controllers/projects/milestones_controller.rb index ae16f69955a..62410d7f57f 100644 --- a/app/controllers/projects/milestones_controller.rb +++ b/app/controllers/projects/milestones_controller.rb @@ -2,7 +2,7 @@ class Projects::MilestonesController < Projects::ApplicationController include MilestoneActions before_action :module_enabled - before_action :milestone, only: [:edit, :update, :destroy, :show, :sort_issues, :sort_merge_requests, :merge_requests, :participants, :labels] + before_action :milestone, only: [:edit, :update, :destroy, :show, :merge_requests, :participants, :labels] # Allow read any milestone before_action :authorize_read_milestone! @@ -85,22 +85,6 @@ class Projects::MilestonesController < Projects::ApplicationController end end - def sort_issues - @milestone.sort_issues(params['sortable_issue'].map(&:to_i)) - - render json: { saved: true } - end - - def sort_merge_requests - @merge_requests = @milestone.merge_requests.where(id: params['sortable_merge_request']) - @merge_requests.each do |merge_request| - merge_request.position = params['sortable_merge_request'].index(merge_request.id.to_s) + 1 - merge_request.save - end - - render json: { saved: true } - end - protected def milestone diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index ea10d004c9c..8e367576c9d 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -67,7 +67,6 @@ module Issuable scope :authored, ->(user) { where(author_id: user) } scope :recent, -> { reorder(id: :desc) } - scope :order_position_asc, -> { reorder(position: :asc) } scope :of_projects, ->(ids) { where(project_id: ids) } scope :of_milestones, ->(ids) { where(milestone_id: ids) } scope :with_milestone, ->(title) { left_joins_milestones.where(milestones: { title: title }) } @@ -139,7 +138,6 @@ module Issuable when 'upvotes_desc' then order_upvotes_desc when 'label_priority' then order_labels_priority(excluded_labels: excluded_labels) when 'priority' then order_due_date_and_labels_priority(excluded_labels: excluded_labels) - when 'position_asc' then order_position_asc else order_by(method) end diff --git a/app/models/concerns/milestoneish.rb b/app/models/concerns/milestoneish.rb index a3472af5c55..01599ce49c6 100644 --- a/app/models/concerns/milestoneish.rb +++ b/app/models/concerns/milestoneish.rb @@ -40,10 +40,18 @@ module Milestoneish def issues_visible_to_user(user) memoize_per_user(user, :issues_visible_to_user) do IssuesFinder.new(user, issues_finder_params) - .execute.includes(:assignees).where(milestone_id: milestoneish_ids) + .execute.preload(:assignees).where(milestone_id: milestoneish_ids) 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 diff --git a/app/models/issue.rb b/app/models/issue.rb index 693cc21bb40..f0f525aea21 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -9,6 +9,9 @@ class Issue < ActiveRecord::Base include Spammable include FasterCacheKeys include RelativePositioning + include IgnorableColumn + + ignore_column :position DueDateStruct = Struct.new(:title, :name).freeze NoDueDate = DueDateStruct.new('No Due Date', '0').freeze @@ -44,7 +47,7 @@ class Issue < ActiveRecord::Base scope :created_after, -> (datetime) { where("created_at >= ?", datetime) } - scope :include_associations, -> { includes(:labels, project: :namespace) } + scope :preload_associations, -> { preload(:labels, project: :namespace) } after_save :expire_etag_cache diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index dd155252ad5..8e8eaea3926 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -4,6 +4,9 @@ class MergeRequest < ActiveRecord::Base include Noteable include Referable include Sortable + include IgnorableColumn + + ignore_column :position belongs_to :target_project, class_name: "Project" belongs_to :source_project, class_name: "Project" diff --git a/app/models/milestone.rb b/app/models/milestone.rb index b04bed4c014..0a6fc064aec 100644 --- a/app/models/milestone.rb +++ b/app/models/milestone.rb @@ -164,38 +164,6 @@ class Milestone < ActiveRecord::Base write_attribute(:title, sanitize_title(value)) if value.present? end - # Sorts the issues for the given IDs. - # - # This method runs a single SQL query using a CASE statement to update the - # position of all issues in the current milestone (scoped to the list of IDs). - # - # Given the ids [10, 20, 30] this method produces a SQL query something like - # the following: - # - # UPDATE issues - # SET position = CASE - # WHEN id = 10 THEN 1 - # WHEN id = 20 THEN 2 - # WHEN id = 30 THEN 3 - # ELSE position - # END - # WHERE id IN (10, 20, 30); - # - # This method expects that the IDs given in `ids` are already Fixnums. - def sort_issues(ids) - pairs = [] - - ids.each_with_index do |id, index| - pairs << id - pairs << index + 1 - end - - conditions = 'WHEN id = ? THEN ? ' * ids.length - - issues.where(id: ids). - update_all(["position = CASE #{conditions} ELSE position END", *pairs]) - end - private def milestone_format_reference(format = :iid) 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/_issuables.html.haml b/app/views/shared/milestones/_issuables.html.haml index 8af3bd597c5..7175e275f95 100644 --- a/app/views/shared/milestones/_issuables.html.haml +++ b/app/views/shared/milestones/_issuables.html.haml @@ -8,11 +8,11 @@ = title - if show_counter .counter - = number_with_delimiter(issuables.size) + = number_with_delimiter(issuables.length) - class_prefix = dom_class(issuables).pluralize - %ul{ class: "well-list #{class_prefix}-sortable-list", id: "#{class_prefix}-list-#{id}", "data-state" => id } + %ul{ class: "well-list milestone-#{class_prefix}-list", id: "#{class_prefix}-list-#{id}" } = render partial: 'shared/milestones/issuable', - collection: issuables.order_position_asc, + collection: issuables, as: :issuable, locals: { show_project_name: show_project_name, show_full_project_name: show_full_project_name } diff --git a/app/views/shared/milestones/_tabs.html.haml b/app/views/shared/milestones/_tabs.html.haml index 6a6d817b344..4de8a6cb15f 100644 --- a/app/views/shared/milestones/_tabs.html.haml +++ b/app/views/shared/milestones/_tabs.html.haml @@ -31,12 +31,12 @@ .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).include_associations, show_project_name: show_project_name, show_full_project_name: show_full_project_name - .tab-pane#tab-merge-requests{ data: { sort_endpoint: (sort_merge_requests_namespace_project_milestone_path(@project.namespace, @project, @milestone) if @project && current_user) } } + = 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" - else - .tab-pane.active#tab-merge-requests{ data: { sort_endpoint: (sort_merge_requests_namespace_project_milestone_path(@project.namespace, @project, @milestone) if @project && current_user) } } + .tab-pane.active#tab-merge-requests -# loaded async = render "shared/milestones/tab_loading" .tab-pane#tab-participants diff --git a/changelogs/unreleased/issue_20900.yml b/changelogs/unreleased/issue_20900.yml new file mode 100644 index 00000000000..e8cef6d2bce --- /dev/null +++ b/changelogs/unreleased/issue_20900.yml @@ -0,0 +1,4 @@ +--- +title: Remove issues/merge requests drag n drop and sorting from milestone view +merge_request: +author: diff --git a/db/post_migrate/20170609183112_remove_position_from_issuables.rb b/db/post_migrate/20170609183112_remove_position_from_issuables.rb new file mode 100644 index 00000000000..4caaa2e83e8 --- /dev/null +++ b/db/post_migrate/20170609183112_remove_position_from_issuables.rb @@ -0,0 +1,8 @@ +class RemovePositionFromIssuables < ActiveRecord::Migration + DOWNTIME = false + + def change + remove_column :issues, :position, :integer + remove_column :merge_requests, :position, :integer + end +end diff --git a/db/schema.rb b/db/schema.rb index 956ca2278f4..803e36fba5a 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170607121233) do +ActiveRecord::Schema.define(version: 20170609183112) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -547,7 +547,6 @@ ActiveRecord::Schema.define(version: 20170607121233) do t.integer "project_id" t.datetime "created_at" t.datetime "updated_at" - t.integer "position", default: 0 t.string "branch_name" t.text "description" t.integer "milestone_id" @@ -738,7 +737,6 @@ ActiveRecord::Schema.define(version: 20170607121233) do t.integer "target_project_id", null: false t.integer "iid" t.text "description" - t.integer "position", default: 0 t.datetime "locked_at" t.integer "updated_by_id" t.text "merge_error" diff --git a/lib/api/milestones.rb b/lib/api/milestones.rb index a3ea619a2fb..3541d3c95fb 100644 --- a/lib/api/milestones.rb +++ b/lib/api/milestones.rb @@ -117,7 +117,7 @@ module API finder_params = { project_id: user_project.id, milestone_title: milestone.title, - sort: 'position_asc' + sort: 'label_priority' } issues = IssuesFinder.new(current_user, finder_params).execute @@ -140,7 +140,7 @@ module API finder_params = { project_id: user_project.id, milestone_title: milestone.title, - sort: 'position_asc' + sort: 'label_priority' } merge_requests = MergeRequestsFinder.new(current_user, finder_params).execute diff --git a/spec/features/dashboard/milestone_tabs_spec.rb b/spec/features/dashboard/milestone_tabs_spec.rb index 0c7b992c500..2c48939bf9c 100644 --- a/spec/features/dashboard/milestone_tabs_spec.rb +++ b/spec/features/dashboard/milestone_tabs_spec.rb @@ -23,7 +23,7 @@ describe 'Dashboard milestone tabs', :js, :feature do it 'loads merge requests async' do click_link 'Merge Requests' - expect(page).to have_selector('.merge_requests-sortable-list') + expect(page).to have_selector('.milestone-merge_requests-list') end it 'loads participants async' do diff --git a/spec/features/milestones/milestones_spec.rb b/spec/features/milestones/milestones_spec.rb index c8a4d23f695..9e117c8ed9f 100644 --- a/spec/features/milestones/milestones_spec.rb +++ b/spec/features/milestones/milestones_spec.rb @@ -1,109 +1,102 @@ -require 'rails_helper' +# require 'rails_helper' -describe 'Milestone draggable', feature: true, js: true do - include DragTo +# describe 'Milestone draggable', feature: true, js: true do +# include DragTo - let(:milestone) { create(:milestone, project: project, title: 8.14) } - let(:project) { create(:empty_project, :public) } - let(:user) { create(:user) } +# let(:milestone) { create(:milestone, project: project, title: 8.14) } +# let(:project) { create(:empty_project, :public) } +# let(:user) { create(:user) } - context 'issues' do - let(:issue) { page.find_by_id('issues-list-unassigned').find('li') } - let(:issue_target) { page.find_by_id('issues-list-ongoing') } +# context 'issues' do +# let(:issue) { page.find_by_id('issues-list-unassigned').find('li') } +# let(:issue_target) { page.find_by_id('issues-list-ongoing') } - it 'does not allow guest to drag issue' do - create_and_drag_issue +# it 'does not allow guest to drag issue' do +# create_and_drag_issue - expect(issue_target).not_to have_selector('.issuable-row') - end +# expect(issue_target).not_to have_selector('.issuable-row') +# end - it 'does not allow authorized user to drag issue' do - login_as(user) - create_and_drag_issue +# it 'does not allow authorized user to drag issue' do +# login_as(user) +# create_and_drag_issue - expect(issue_target).not_to have_selector('.issuable-row') - end +# expect(issue_target).not_to have_selector('.issuable-row') +# end - it 'allows author to drag issue' do - login_as(user) - create_and_drag_issue(author: user) +# it 'allows author to drag issue' do +# login_as(user) +# create_and_drag_issue(author: user) - expect(issue_target).to have_selector('.issuable-row') - end +# expect(issue_target).to have_selector('.issuable-row') +# end - it 'allows admin to drag issue' do - login_as(:admin) - create_and_drag_issue +# it 'allows admin to drag issue' do +# login_as(:admin) - expect(issue_target).to have_selector('.issuable-row') - end +# create_and_drag_issue - it 'assigns issue when it has been dragged to ongoing list' do - login_as(:admin) - create_and_drag_issue +# expect(issue_target).to have_selector('.issuable-row') +# end +# end - expect(@issue.reload.assignees).not_to be_empty - expect(page).to have_selector("#sortable_issue_#{@issue.iid} .assignee-icon img", count: 1) - end - end +# context 'merge requests' do +# let(:merge_request) { page.find_by_id('merge_requests-list-unassigned').find('li') } +# let(:merge_request_target) { page.find_by_id('merge_requests-list-ongoing') } - context 'merge requests' do - let(:merge_request) { page.find_by_id('merge_requests-list-unassigned').find('li') } - let(:merge_request_target) { page.find_by_id('merge_requests-list-ongoing') } +# it 'does not allow guest to drag merge request' do +# create_and_drag_merge_request - it 'does not allow guest to drag merge request' do - create_and_drag_merge_request +# expect(merge_request_target).not_to have_selector('.issuable-row') +# end - expect(merge_request_target).not_to have_selector('.issuable-row') - end +# it 'does not allow authorized user to drag merge request' do +# login_as(user) +# create_and_drag_merge_request - it 'does not allow authorized user to drag merge request' do - login_as(user) - create_and_drag_merge_request +# expect(merge_request_target).not_to have_selector('.issuable-row') +# end - expect(merge_request_target).not_to have_selector('.issuable-row') - end +# it 'allows author to drag merge request' do +# login_as(user) +# create_and_drag_merge_request(author: user) - it 'allows author to drag merge request' do - login_as(user) - create_and_drag_merge_request(author: user) +# expect(merge_request_target).to have_selector('.issuable-row') +# end - expect(merge_request_target).to have_selector('.issuable-row') - end +# it 'allows admin to drag merge request' do +# login_as(:admin) +# create_and_drag_merge_request - it 'allows admin to drag merge request' do - login_as(:admin) - create_and_drag_merge_request +# expect(merge_request_target).to have_selector('.issuable-row') +# end +# end - expect(merge_request_target).to have_selector('.issuable-row') - end - end +# def create_and_drag_issue(params = {}) +# create(:issue, params.merge(title: 'Foo', project: project, milestone: milestone)) - def create_and_drag_issue(params = {}) - @issue = create(:issue, params.merge(title: 'Foo', project: project, milestone: milestone)) +# visit namespace_project_milestone_path(project.namespace, project, milestone) +# scroll_into_view('.milestone-content') +# drag_to(selector: '.issues-sortable-list', list_to_index: 1) - visit namespace_project_milestone_path(project.namespace, project, milestone) - scroll_into_view('.milestone-content') - drag_to(selector: '.issues-sortable-list', list_to_index: 1) +# wait_for_requests +# end - wait_for_requests - end +# def create_and_drag_merge_request(params = {}) +# create(:merge_request, params.merge(title: 'Foo', source_project: project, target_project: project, milestone: milestone)) - def create_and_drag_merge_request(params = {}) - create(:merge_request, params.merge(title: 'Foo', source_project: project, target_project: project, milestone: milestone)) +# visit namespace_project_milestone_path(project.namespace, project, milestone) +# page.find("a[href='#tab-merge-requests']").click - visit namespace_project_milestone_path(project.namespace, project, milestone) - page.find("a[href='#tab-merge-requests']").click +# wait_for_requests - wait_for_requests +# scroll_into_view('.milestone-content') +# drag_to(selector: '.merge_requests-sortable-list', list_to_index: 1) - scroll_into_view('.milestone-content') - drag_to(selector: '.merge_requests-sortable-list', list_to_index: 1) +# wait_for_requests +# end - wait_for_requests - end - - def scroll_into_view(selector) - page.evaluate_script("document.querySelector('#{selector}').scrollIntoView();") - end -end +# def scroll_into_view(selector) +# page.evaluate_script("document.querySelector('#{selector}').scrollIntoView();") +# end +# end diff --git a/spec/models/concerns/milestoneish_spec.rb b/spec/models/concerns/milestoneish_spec.rb index 675b730c557..cefe7fb6fea 100644 --- a/spec/models/concerns/milestoneish_spec.rb +++ b/spec/models/concerns/milestoneish_spec.rb @@ -19,12 +19,43 @@ describe Milestone, 'Milestoneish' do let!(:closed_security_issue_3) { create(:issue, :confidential, :closed, project: project, author: author, milestone: milestone) } let!(:closed_security_issue_4) { create(:issue, :confidential, :closed, project: project, assignees: [assignee], milestone: milestone) } let!(:merge_request) { create(:merge_request, source_project: project, target_project: project, milestone: 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 << [member, :developer] project.team << [guest, :guest] end + 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.sorted_issues(member) + + expect(issues.first).to eq(issue) + expect(issues.second).to eq(security_issue_1) + expect(issues.third).not_to eq(closed_issue_1) + end + end + + describe '#sorted_merge_requests' do + it 'sorts merge requests by label priority' do + 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 + + expect(merge_requests.first).to eq(merge_request_2) + expect(merge_requests.second).to eq(merge_request_1) + expect(merge_requests.third).to eq(merge_request_3) + end + end + describe '#closed_items_count' do it 'does not count confidential issues for non project members' do expect(milestone.closed_items_count(non_member)).to eq 2 diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb index aa1ce89ffd7..20b96c08a8f 100644 --- a/spec/models/milestone_spec.rb +++ b/spec/models/milestone_spec.rb @@ -144,35 +144,6 @@ describe Milestone, models: true do end end - describe '#sort_issues' do - let(:milestone) { create(:milestone) } - - let(:issue1) { create(:issue, milestone: milestone, position: 1) } - let(:issue2) { create(:issue, milestone: milestone, position: 2) } - let(:issue3) { create(:issue, milestone: milestone, position: 3) } - let(:issue4) { create(:issue, position: 42) } - - it 'sorts the given issues' do - milestone.sort_issues([issue3.id, issue2.id, issue1.id]) - - issue1.reload - issue2.reload - issue3.reload - - expect(issue1.position).to eq(3) - expect(issue2.position).to eq(2) - expect(issue3.position).to eq(1) - end - - it 'ignores issues not part of the milestone' do - milestone.sort_issues([issue3.id, issue2.id, issue1.id, issue4.id]) - - issue4.reload - - expect(issue4.position).to eq(42) - end - end - describe '.search' do let(:milestone) { create(:milestone, title: 'foo', description: 'bar') } 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) |