From ff7fe2d12dec6c76b5a90b2c95cca81152cc5d42 Mon Sep 17 00:00:00 2001 From: Felipe Artur Date: Tue, 6 Jun 2017 15:53:31 -0300 Subject: [PATCH] Remove Drag and drop and sorting from milestone view --- app/assets/javascripts/milestone.js | 17 -- app/assets/stylesheets/pages/milestone.scss | 6 +- .../projects/milestones_controller.rb | 18 +-- app/models/concerns/issuable.rb | 2 - app/models/milestone.rb | 32 ---- .../shared/milestones/_issuables.html.haml | 4 +- changelogs/unreleased/issue_20900.yml | 4 + lib/api/milestones.rb | 4 +- spec/features/milestones/milestones_spec.rb | 153 +++++++++--------- 9 files changed, 87 insertions(+), 153 deletions(-) create mode 100644 changelogs/unreleased/issue_20900.yml diff --git a/app/assets/javascripts/milestone.js b/app/assets/javascripts/milestone.js index 07ede5ee913..8778ed0124e 100644 --- a/app/assets/javascripts/milestone.js +++ b/app/assets/javascripts/milestone.js @@ -84,7 +84,6 @@ 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 +93,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); 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/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/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/views/shared/milestones/_issuables.html.haml b/app/views/shared/milestones/_issuables.html.haml index 8af3bd597c5..f683b50a62a 100644 --- a/app/views/shared/milestones/_issuables.html.haml +++ b/app/views/shared/milestones/_issuables.html.haml @@ -11,8 +11,8 @@ = number_with_delimiter(issuables.size) - 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.recent, as: :issuable, locals: { show_project_name: show_project_name, show_full_project_name: show_full_project_name } diff --git a/changelogs/unreleased/issue_20900.yml b/changelogs/unreleased/issue_20900.yml new file mode 100644 index 00000000000..71586013b8b --- /dev/null +++ b/changelogs/unreleased/issue_20900.yml @@ -0,0 +1,4 @@ +--- +title: Remove Drag and drop and sorting from milestone view +merge_request: +author: diff --git a/lib/api/milestones.rb b/lib/api/milestones.rb index a3ea619a2fb..e26011db174 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: 'created_desc' } 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: 'created_desc' } merge_requests = MergeRequestsFinder.new(current_user, finder_params).execute diff --git a/spec/features/milestones/milestones_spec.rb b/spec/features/milestones/milestones_spec.rb index c8a4d23f695..1537b0f472a 100644 --- a/spec/features/milestones/milestones_spec.rb +++ b/spec/features/milestones/milestones_spec.rb @@ -1,109 +1,108 @@ -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 +>>>>>>> Remove Drag and drop and sorting from milestone view - 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 +<<<<<<< 5f42009f8dcc29d559ee415e92c88858e361f063 def create_and_drag_issue(params = {}) @issue = create(:issue, params.merge(title: 'Foo', project: project, milestone: milestone)) +======= +# def create_and_drag_issue(params = {}) +# create(:issue, params.merge(title: 'Foo', project: project, milestone: milestone)) +>>>>>>> Remove Drag and drop and sorting from milestone view - 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