diff options
| -rw-r--r-- | app/assets/javascripts/milestone_select.js | 8 | ||||
| -rw-r--r-- | app/finders/issuable_finder.rb | 27 | ||||
| -rw-r--r-- | app/models/milestone.rb | 4 | ||||
| -rw-r--r-- | changelogs/unreleased/4221-board-milestone-should-persist-any-none-properly.yml | 5 | ||||
| -rw-r--r-- | db/migrate/20190703001120_default_milestone_to_nil.rb | 24 | ||||
| -rw-r--r-- | spec/finders/issues_finder_spec.rb | 4 | ||||
| -rw-r--r-- | spec/requests/api/issues/get_project_issues_spec.rb | 2 |
7 files changed, 23 insertions, 51 deletions
diff --git a/app/assets/javascripts/milestone_select.js b/app/assets/javascripts/milestone_select.js index 01ed27c49fb..43949d5cc86 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 ? __('Any Milestone') : null; + selectedMilestoneDefault = showAny ? '' : null; selectedMilestoneDefault = showNo && defaultNo ? __('No Milestone') : selectedMilestoneDefault; selectedMilestone = $dropdown.data('selected') || selectedMilestoneDefault; @@ -74,16 +74,14 @@ export default class MilestoneSelect { if (showAny) { extraOptions.push({ id: null, - // eslint-disable-next-line @gitlab/i18n/no-non-i18n-strings - name: 'Any', + name: null, title: __('Any Milestone'), }); } if (showNo) { extraOptions.push({ id: -1, - // eslint-disable-next-line @gitlab/i18n/no-non-i18n-strings - name: 'None', + name: __('No Milestone'), title: __('No Milestone'), }); } diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index 86970ae3219..1773ac2d508 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -484,19 +484,22 @@ class IssuableFinder # rubocop: disable CodeReuse/ActiveRecord def by_milestone(items) - 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]) + 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 end + + items end # rubocop: enable CodeReuse/ActiveRecord diff --git a/app/models/milestone.rb b/app/models/milestone.rb index 60266992ee1..2ad2838111e 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', -1) - Any = MilestoneStruct.new('Any Milestone', '', nil) + None = MilestoneStruct.new('No Milestone', 'No Milestone', 0) + Any = MilestoneStruct.new('Any Milestone', '', -1) 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 deleted file mode 100644 index d50c59bf607..00000000000 --- a/changelogs/unreleased/4221-board-milestone-should-persist-any-none-properly.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -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 deleted file mode 100644 index 6a1c3603d9d..00000000000 --- a/db/migrate/20190703001120_default_milestone_to_nil.rb +++ /dev/null @@ -1,24 +0,0 @@ -# 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 bcde730c40b..879ff01f294 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, issue2, issue3, issue4) + expect(issues).to contain_exactly(issue1) end it 'returns issues with any assigned milestone (deprecated)' do params[:milestone_title] = Milestone::Any.title - expect(issues).to contain_exactly(issue1, issue2, issue3, issue4) + expect(issues).to contain_exactly(issue1) 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 f11d8259d4a..f7ca6fd1e0a 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, confidential_issue.id, closed_issue.id]) + expect_paginated_array_response([issue.id, closed_issue.id]) end context 'without sort params' do |