From ef66a4a57c8e7591ef5015d594d8bcc4a1077e17 Mon Sep 17 00:00:00 2001 From: Oswaldo Ferreira Date: Mon, 6 Aug 2018 16:38:37 -0300 Subject: Fix missing and duplicates on project milestone listing page --- app/models/concerns/issuable.rb | 2 +- app/models/concerns/sortable.rb | 1 + app/models/milestone.rb | 35 ++++++++++++---------- ...x-missing-and-duplicated-milestones-on-list.yml | 5 ++++ .../projects/milestones_controller_spec.rb | 33 ++++++++++++++++++-- 5 files changed, 57 insertions(+), 19 deletions(-) create mode 100644 changelogs/unreleased/osw-fix-missing-and-duplicated-milestones-on-list.yml diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index b93c1145f82..dfdc55b68d1 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -154,7 +154,7 @@ module Issuable end # Break ties with the ID column for pagination - sorted.order(id: :desc) + sorted.with_order_id_desc end def order_due_date_and_labels_priority(excluded_labels: []) diff --git a/app/models/concerns/sortable.rb b/app/models/concerns/sortable.rb index cb76ae971d4..409255fb68b 100644 --- a/app/models/concerns/sortable.rb +++ b/app/models/concerns/sortable.rb @@ -6,6 +6,7 @@ module Sortable extend ActiveSupport::Concern included do + scope :with_order_id_desc, -> { order(id: :desc) } scope :order_id_desc, -> { reorder(id: :desc) } scope :order_id_asc, -> { reorder(id: :asc) } scope :order_created_desc, -> { reorder(created_at: :desc) } diff --git a/app/models/milestone.rb b/app/models/milestone.rb index f2b2d291da9..cb1def1b422 100644 --- a/app/models/milestone.rb +++ b/app/models/milestone.rb @@ -146,22 +146,25 @@ class Milestone < ActiveRecord::Base end def self.sort_by_attribute(method) - case method.to_s - when 'due_date_asc' - reorder(Gitlab::Database.nulls_last_order('due_date', 'ASC')) - when 'due_date_desc' - reorder(Gitlab::Database.nulls_last_order('due_date', 'DESC')) - when 'name_asc' - reorder(Arel::Nodes::Ascending.new(arel_table[:title].lower)) - when 'name_desc' - reorder(Arel::Nodes::Descending.new(arel_table[:title].lower)) - when 'start_date_asc' - reorder(Gitlab::Database.nulls_last_order('start_date', 'ASC')) - when 'start_date_desc' - reorder(Gitlab::Database.nulls_last_order('start_date', 'DESC')) - else - order_by(method) - end + sorted = + case method.to_s + when 'due_date_asc' + reorder(Gitlab::Database.nulls_last_order('due_date', 'ASC')) + when 'due_date_desc' + reorder(Gitlab::Database.nulls_last_order('due_date', 'DESC')) + when 'name_asc' + reorder(Arel::Nodes::Ascending.new(arel_table[:title].lower)) + when 'name_desc' + reorder(Arel::Nodes::Descending.new(arel_table[:title].lower)) + when 'start_date_asc' + reorder(Gitlab::Database.nulls_last_order('start_date', 'ASC')) + when 'start_date_desc' + reorder(Gitlab::Database.nulls_last_order('start_date', 'DESC')) + else + order_by(method) + end + + sorted.with_order_id_desc end ## diff --git a/changelogs/unreleased/osw-fix-missing-and-duplicated-milestones-on-list.yml b/changelogs/unreleased/osw-fix-missing-and-duplicated-milestones-on-list.yml new file mode 100644 index 00000000000..62416b7f87e --- /dev/null +++ b/changelogs/unreleased/osw-fix-missing-and-duplicated-milestones-on-list.yml @@ -0,0 +1,5 @@ +--- +title: Fix missing and duplicates on project milestone listing page +merge_request: 21058 +author: +type: fixed diff --git a/spec/controllers/projects/milestones_controller_spec.rb b/spec/controllers/projects/milestones_controller_spec.rb index 6c2d1c7e92b..3190f1ce9d4 100644 --- a/spec/controllers/projects/milestones_controller_spec.rb +++ b/spec/controllers/projects/milestones_controller_spec.rb @@ -42,16 +42,45 @@ describe Projects::MilestonesController do describe "#index" do context "as html" do - before do - get :index, namespace_id: project.namespace.id, project_id: project.id + def render_index(project:, page:) + get :index, namespace_id: project.namespace.id, + project_id: project.id, + page: page end it "queries only projects milestones" do + render_index project: project, page: 1 + milestones = assigns(:milestones) expect(milestones.count).to eq(1) expect(milestones.where(project_id: nil)).to be_empty end + + it 'renders paginated milestones without missing or duplicates' do + allow(Milestone).to receive(:default_per_page).and_return(2) + create_list(:milestone, 5, project: project) + + render_index project: project, page: 1 + page_1_milestones = assigns(:milestones) + expect(page_1_milestones.size).to eq(2) + + render_index project: project, page: 2 + page_2_milestones = assigns(:milestones) + expect(page_2_milestones.size).to eq(2) + + render_index project: project, page: 3 + page_3_milestones = assigns(:milestones) + expect(page_3_milestones.size).to eq(2) + + rendered_milestone_ids = + page_1_milestones.pluck(:id) + + page_2_milestones.pluck(:id) + + page_3_milestones.pluck(:id) + + expect(rendered_milestone_ids) + .to match_array(project.milestones.pluck(:id)) + end end context "as json" do -- cgit v1.2.1