From 5d7b46d523bb2310ac33d5b4c06137e9a3fd17aa Mon Sep 17 00:00:00 2001 From: charlie ablett Date: Wed, 31 Jul 2019 13:51:46 +0000 Subject: Port CE changes from EE - DB migration of board milestone values - issue finder & spec updates --- app/assets/javascripts/milestone_select.js | 8 ++++--- app/finders/issuable_finder.rb | 27 ++++++++++------------ app/models/milestone.rb | 4 ++-- ...-milestone-should-persist-any-none-properly.yml | 5 ++++ .../20190703001120_default_milestone_to_nil.rb | 24 +++++++++++++++++++ spec/finders/issues_finder_spec.rb | 4 ++-- .../requests/api/issues/get_project_issues_spec.rb | 2 +- 7 files changed, 51 insertions(+), 23 deletions(-) create mode 100644 changelogs/unreleased/4221-board-milestone-should-persist-any-none-properly.yml create mode 100644 db/migrate/20190703001120_default_milestone_to_nil.rb diff --git a/app/assets/javascripts/milestone_select.js b/app/assets/javascripts/milestone_select.js index 43949d5cc86..01ed27c49fb 100644 --- a/app/assets/javascripts/milestone_select.js +++ b/app/assets/javascripts/milestone_select.js @@ -55,7 +55,7 @@ export default class MilestoneSelect { const $sidebarCollapsedValue = $block.find('.sidebar-collapsed-icon'); const $value = $block.find('.value'); const $loading = $block.find('.block-loading').fadeOut(); - selectedMilestoneDefault = showAny ? '' : null; + selectedMilestoneDefault = showAny ? __('Any Milestone') : null; selectedMilestoneDefault = showNo && defaultNo ? __('No Milestone') : selectedMilestoneDefault; selectedMilestone = $dropdown.data('selected') || selectedMilestoneDefault; @@ -74,14 +74,16 @@ export default class MilestoneSelect { if (showAny) { extraOptions.push({ id: null, - name: null, + // eslint-disable-next-line @gitlab/i18n/no-non-i18n-strings + name: 'Any', title: __('Any Milestone'), }); } if (showNo) { extraOptions.push({ id: -1, - name: __('No Milestone'), + // eslint-disable-next-line @gitlab/i18n/no-non-i18n-strings + name: 'None', title: __('No Milestone'), }); } diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index 1773ac2d508..86970ae3219 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -484,22 +484,19 @@ class IssuableFinder # rubocop: disable CodeReuse/ActiveRecord def by_milestone(items) - if milestones? - if filter_by_no_milestone? - items = items.left_joins_milestones.where(milestone_id: [-1, nil]) - elsif filter_by_any_milestone? - items = items.any_milestone - elsif filter_by_upcoming_milestone? - upcoming_ids = Milestone.upcoming_ids(projects, related_groups) - items = items.left_joins_milestones.where(milestone_id: upcoming_ids) - elsif filter_by_started_milestone? - items = items.left_joins_milestones.merge(Milestone.started) - else - items = items.with_milestone(params[:milestone_title]) - end + return items unless milestones? + return items if filter_by_any_milestone? + + if filter_by_no_milestone? + items.left_joins_milestones.where(milestone_id: nil) + elsif filter_by_upcoming_milestone? + upcoming_ids = Milestone.upcoming_ids(projects, related_groups) + items.left_joins_milestones.where(milestone_id: upcoming_ids) + elsif filter_by_started_milestone? + items.left_joins_milestones.merge(Milestone.started) + else + items.with_milestone(params[:milestone_title]) end - - items end # rubocop: enable CodeReuse/ActiveRecord diff --git a/app/models/milestone.rb b/app/models/milestone.rb index 2ad2838111e..60266992ee1 100644 --- a/app/models/milestone.rb +++ b/app/models/milestone.rb @@ -4,8 +4,8 @@ class Milestone < ApplicationRecord # Represents a "No Milestone" state used for filtering Issues and Merge # Requests that have no milestone assigned. MilestoneStruct = Struct.new(:title, :name, :id) - None = MilestoneStruct.new('No Milestone', 'No Milestone', 0) - Any = MilestoneStruct.new('Any Milestone', '', -1) + None = MilestoneStruct.new('No Milestone', 'No Milestone', -1) + Any = MilestoneStruct.new('Any Milestone', '', nil) Upcoming = MilestoneStruct.new('Upcoming', '#upcoming', -2) Started = MilestoneStruct.new('Started', '#started', -3) diff --git a/changelogs/unreleased/4221-board-milestone-should-persist-any-none-properly.yml b/changelogs/unreleased/4221-board-milestone-should-persist-any-none-properly.yml new file mode 100644 index 00000000000..d50c59bf607 --- /dev/null +++ b/changelogs/unreleased/4221-board-milestone-should-persist-any-none-properly.yml @@ -0,0 +1,5 @@ +--- +title: For milestone filters, treat Any as No Filter (using null). Use -1 for No Milestone +merge_request: +author: +type: changed diff --git a/db/migrate/20190703001120_default_milestone_to_nil.rb b/db/migrate/20190703001120_default_milestone_to_nil.rb new file mode 100644 index 00000000000..6a1c3603d9d --- /dev/null +++ b/db/migrate/20190703001120_default_milestone_to_nil.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +class DefaultMilestoneToNil < ActiveRecord::Migration[5.1] + DOWNTIME = false + + def up + execute(update_board_milestones_query) + end + + def down + # no-op + end + + private + + # Only 105 records to update, as of 2019/07/18 + def update_board_milestones_query + <<~HEREDOC + UPDATE boards + SET milestone_id = NULL + WHERE boards.milestone_id = -1 + HEREDOC + end +end diff --git a/spec/finders/issues_finder_spec.rb b/spec/finders/issues_finder_spec.rb index 879ff01f294..bcde730c40b 100644 --- a/spec/finders/issues_finder_spec.rb +++ b/spec/finders/issues_finder_spec.rb @@ -113,13 +113,13 @@ describe IssuesFinder do let(:params) { { milestone_title: 'Any' } } it 'returns issues with any assigned milestone' do - expect(issues).to contain_exactly(issue1) + expect(issues).to contain_exactly(issue1, issue2, issue3, issue4) end it 'returns issues with any assigned milestone (deprecated)' do params[:milestone_title] = Milestone::Any.title - expect(issues).to contain_exactly(issue1) + expect(issues).to contain_exactly(issue1, issue2, issue3, issue4) end end diff --git a/spec/requests/api/issues/get_project_issues_spec.rb b/spec/requests/api/issues/get_project_issues_spec.rb index f7ca6fd1e0a..f11d8259d4a 100644 --- a/spec/requests/api/issues/get_project_issues_spec.rb +++ b/spec/requests/api/issues/get_project_issues_spec.rb @@ -389,7 +389,7 @@ describe API::Issues do it 'returns an array of issues with any milestone' do get api("#{base_url}/issues", user), params: { milestone: any_milestone_title } - expect_paginated_array_response([issue.id, closed_issue.id]) + expect_paginated_array_response([issue.id, confidential_issue.id, closed_issue.id]) end context 'without sort params' do -- cgit v1.2.1