Commit 88447821 authored by Eulyeon Ko's avatar Eulyeon Ko

Make iteration title option with iteration cadence

User facing changes:

- Iteration title won't be included in notification emails.
- Board list title for iteration will display the iteration's period.
- Iterations created in advance for a cadence won't have titles (behind
  FF.)

Internal changes:

- Title is no longer a required attribute for Iteration model

Timebox concern previously hosted the validation logic for title
attribute for both Milestone and Iteration.
Since title is only required and needs to be validated for milestone,
the validation logic is moved from Timebox concern into Milestone model.
For Iteration model, only sanitization needs to be done for title.

- User must specify a title unless using the FF iteration_cadences when
creating/updating a new iteration.

Changelog: changed

Apply backend reviewer suggestion
parent 3235302b
...@@ -44,7 +44,6 @@ module Timebox ...@@ -44,7 +44,6 @@ module Timebox
validates :group, presence: true, unless: :project validates :group, presence: true, unless: :project
validates :project, presence: true, unless: :group validates :project, presence: true, unless: :group
validates :title, presence: true
validate :timebox_type_check validate :timebox_type_check
validate :start_date_should_be_less_than_due_date, if: proc { |m| m.start_date.present? && m.due_date.present? } validate :start_date_should_be_less_than_due_date, if: proc { |m| m.start_date.present? && m.due_date.present? }
......
...@@ -35,6 +35,7 @@ class Milestone < ApplicationRecord ...@@ -35,6 +35,7 @@ class Milestone < ApplicationRecord
scope :with_api_entity_associations, -> { preload(project: [:project_feature, :route, namespace: :route]) } scope :with_api_entity_associations, -> { preload(project: [:project_feature, :route, namespace: :route]) }
scope :order_by_dates_and_title, -> { order(due_date: :asc, start_date: :asc, title: :asc) } scope :order_by_dates_and_title, -> { order(due_date: :asc, start_date: :asc, title: :asc) }
validates :title, presence: true
validates_associated :milestone_releases, message: -> (_, obj) { obj[:value].map(&:errors).map(&:full_messages).join(",") } validates_associated :milestone_releases, message: -> (_, obj) { obj[:value].map(&:errors).map(&:full_messages).join(",") }
validate :uniqueness_of_title, if: :title_changed? validate :uniqueness_of_title, if: :title_changed?
......
--- ---
name: iteration_cadences name: iteration_cadences
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/54822 introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/54822
rollout_issue_url: rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/354878
milestone: '13.10' milestone: '13.10'
type: development type: development
group: group::project management group: group::project management
......
...@@ -12142,7 +12142,7 @@ Represents an iteration object. ...@@ -12142,7 +12142,7 @@ Represents an iteration object.
| <a id="iterationsequence"></a>`sequence` | [`Int!`](#int) | Sequence number for the iteration when you sort the containing cadence's iterations by the start and end date. The earliest starting and ending iteration is assigned 1. | | <a id="iterationsequence"></a>`sequence` | [`Int!`](#int) | Sequence number for the iteration when you sort the containing cadence's iterations by the start and end date. The earliest starting and ending iteration is assigned 1. |
| <a id="iterationstartdate"></a>`startDate` | [`Time`](#time) | Timestamp of the iteration start date. | | <a id="iterationstartdate"></a>`startDate` | [`Time`](#time) | Timestamp of the iteration start date. |
| <a id="iterationstate"></a>`state` | [`IterationState!`](#iterationstate) | State of the iteration. | | <a id="iterationstate"></a>`state` | [`IterationState!`](#iterationstate) | State of the iteration. |
| <a id="iterationtitle"></a>`title` | [`String!`](#string) | Title of the iteration. | | <a id="iterationtitle"></a>`title` | [`String`](#string) | Title of the iteration. Title must be specified unless iteration_cadences feature flag is enabled. |
| <a id="iterationupdatedat"></a>`updatedAt` | [`Time!`](#time) | Timestamp of last iteration update. | | <a id="iterationupdatedat"></a>`updatedAt` | [`Time!`](#time) | Timestamp of last iteration update. |
| <a id="iterationwebpath"></a>`webPath` | [`String!`](#string) | Web path of the iteration. | | <a id="iterationwebpath"></a>`webPath` | [`String!`](#string) | Web path of the iteration. |
| <a id="iterationweburl"></a>`webUrl` | [`String!`](#string) | Web URL of the iteration. | | <a id="iterationweburl"></a>`webUrl` | [`String!`](#string) | Web URL of the iteration. |
...@@ -63,6 +63,10 @@ module Mutations ...@@ -63,6 +63,10 @@ module Mutations
raise Gitlab::Graphql::Errors::ArgumentError, 'The list of iteration attributes is empty' raise Gitlab::Graphql::Errors::ArgumentError, 'The list of iteration attributes is empty'
end end
if !parent.iteration_cadences_feature_flag_enabled? && args[:title].blank?
raise Gitlab::Graphql::Errors::ArgumentError, "Title can't be blank"
end
# Currently there is a single iteration cadence per group, so if `iterations_cadence_id` argument is not provided # Currently there is a single iteration cadence per group, so if `iterations_cadence_id` argument is not provided
# we assign iteration to the only cadence in the group(see `Iteration#set_iterations_cadence`). # we assign iteration to the only cadence in the group(see `Iteration#set_iterations_cadence`).
# Once we introduce cadence CRUD support we need to specify to which iteration cadence a given iteration # Once we introduce cadence CRUD support we need to specify to which iteration cadence a given iteration
......
...@@ -51,6 +51,7 @@ module Mutations ...@@ -51,6 +51,7 @@ module Mutations
args[:id] = id_from_args(args) args[:id] = id_from_args(args)
parent = resolve_group(full_path: args[:group_path]).try(:sync) parent = resolve_group(full_path: args[:group_path]).try(:sync)
validate_title_argument!(parent, args)
iteration = authorized_find!(parent: parent, id: args[:id]) iteration = authorized_find!(parent: parent, id: args[:id])
response = ::Iterations::UpdateService.new(parent, current_user, args).execute(iteration) response = ::Iterations::UpdateService.new(parent, current_user, args).execute(iteration)
...@@ -76,6 +77,12 @@ module Mutations ...@@ -76,6 +77,12 @@ module Mutations
raise Gitlab::Graphql::Errors::ArgumentError, 'The list of iteration attributes is empty' if args.except(:group_path, :id).empty? raise Gitlab::Graphql::Errors::ArgumentError, 'The list of iteration attributes is empty' if args.except(:group_path, :id).empty?
end end
def validate_title_argument!(parent, args)
if !parent.iteration_cadences_feature_flag_enabled? && args[:title].blank?
raise Gitlab::Graphql::Errors::ArgumentError, "Title can't be blank"
end
end
# Originally accepted a raw model id. Now accept a gid, but allow a raw id # Originally accepted a raw model id. Now accept a gid, but allow a raw id
# for backward compatibility # for backward compatibility
def id_from_args(args) def id_from_args(args)
......
...@@ -20,8 +20,8 @@ module Types ...@@ -20,8 +20,8 @@ module Types
field :sequence, GraphQL::Types::Int, null: false, field :sequence, GraphQL::Types::Int, null: false,
description: "Sequence number for the iteration when you sort the containing cadence's iterations by the start and end date. The earliest starting and ending iteration is assigned 1." description: "Sequence number for the iteration when you sort the containing cadence's iterations by the start and end date. The earliest starting and ending iteration is assigned 1."
field :title, GraphQL::Types::String, null: false, field :title, GraphQL::Types::String, null: true,
description: 'Title of the iteration.' description: 'Title of the iteration. Title must be specified unless iteration_cadences feature flag is enabled.'
field :description, GraphQL::Types::String, null: true, field :description, GraphQL::Types::String, null: true,
description: 'Description of the iteration.' description: 'Description of the iteration.'
......
...@@ -42,7 +42,7 @@ module EE ...@@ -42,7 +42,7 @@ module EE
scope :in_iteration_scope, ->(iteration_scope) { joins(:iteration).merge(iteration_scope) } scope :in_iteration_scope, ->(iteration_scope) { joins(:iteration).merge(iteration_scope) }
scope :in_iteration_cadences, ->(iteration_cadences) { joins(:iteration).where(sprints: { iterations_cadence_id: iteration_cadences }) } scope :in_iteration_cadences, ->(iteration_cadences) { joins(:iteration).where(sprints: { iterations_cadence_id: iteration_cadences }) }
scope :with_iteration_title, ->(iteration_title) { joins(:iteration).where(sprints: { title: iteration_title }) } scope :with_iteration_title, ->(iteration_title) { joins(:iteration).where(sprints: { title: iteration_title }) }
scope :without_iteration_title, ->(iteration_title) { left_outer_joins(:iteration).where('sprints.title != ? OR sprints.id IS NULL', iteration_title) } scope :without_iteration_title, ->(iteration_title) { left_outer_joins(:iteration).where('sprints.title IS DISTINCT FROM ? OR sprints.id IS NULL', iteration_title) }
scope :on_status_page, -> do scope :on_status_page, -> do
joins(project: :status_page_setting) joins(project: :status_page_setting)
.where(status_page_settings: { enabled: true }) .where(status_page_settings: { enabled: true })
......
...@@ -182,6 +182,14 @@ module EE ...@@ -182,6 +182,14 @@ module EE
"#{iterations_cadence.title} #{period}" "#{iterations_cadence.title} #{period}"
end end
def title=(value)
if value.blank?
write_attribute(:title, nil)
else
super
end
end
def state def state
STATE_ENUM_MAP.key(state_enum) STATE_ENUM_MAP.key(state_enum)
end end
...@@ -194,10 +202,10 @@ module EE ...@@ -194,10 +202,10 @@ module EE
group || project group || project
end end
# Show just the title when we manage to find an iteration, without the reference pattern, # Show display_text when we manage to find an iteration, without the reference pattern,
# since it's long and unsightly. # since it's long and unsightly.
def reference_link_text(from = nil) def reference_link_text(from = nil)
self.title display_text
end end
def supports_timebox_charts? def supports_timebox_charts?
...@@ -247,8 +255,8 @@ module EE ...@@ -247,8 +255,8 @@ module EE
def timebox_format_reference(format = :id) def timebox_format_reference(format = :id)
raise ::ArgumentError, _('Unknown format') unless [:id, :name].include?(format) raise ::ArgumentError, _('Unknown format') unless [:id, :name].include?(format)
if format == :name if format == :name && title.present?
super %("#{title}")
else else
id id
end end
...@@ -322,6 +330,7 @@ module EE ...@@ -322,6 +330,7 @@ module EE
errors.add(:group, s_('is not valid. The iteration group has to match the iteration cadence group.')) errors.add(:group, s_('is not valid. The iteration group has to match the iteration cadence group.'))
end end
# TODO: remove this as part of https://gitlab.com/gitlab-org/gitlab/-/issues/354878
def uniqueness_of_title def uniqueness_of_title
relation = self.class.where(iterations_cadence_id: self.iterations_cadence) relation = self.class.where(iterations_cadence_id: self.iterations_cadence)
title_exists = relation.find_by_title(title) title_exists = relation.find_by_title(title)
......
...@@ -67,7 +67,7 @@ module EE ...@@ -67,7 +67,7 @@ module EE
when 'milestone' when 'milestone'
milestone.title milestone.title
when 'iteration' when 'iteration'
iteration.title iteration.display_text
else else
super super
end end
......
...@@ -68,8 +68,7 @@ module Iterations ...@@ -68,8 +68,7 @@ module Iterations
group_id: cadence.group_id, group_id: cadence.group_id,
start_date: start_date, start_date: start_date,
due_date: due_date, due_date: due_date,
state_enum: Iteration::STATE_ENUM_MAP[::Iteration.compute_state(start_date, due_date)], state_enum: Iteration::STATE_ENUM_MAP[::Iteration.compute_state(start_date, due_date)]
title: "Iteration #{iteration_number}"
} }
end end
......
%p %p
= _('Iteration changed to') = _('Iteration changed to')
%strong= link_to(@iteration.display_text, @iteration_url) %strong= link_to(@iteration.display_text, @iteration_url)
= "(#{@iteration.title})"
<%= _('Iteration changed to') %> <%= @iteration.display_text %> (<%= @iteration.title %>) ( <%= @iteration_url %> ) <%= _('Iteration changed to') %> <%= @iteration.display_text %> ( <%= @iteration_url %> )
...@@ -24,7 +24,7 @@ module EE ...@@ -24,7 +24,7 @@ module EE
relation = [id_relation, iteration_relation].compact relation = [id_relation, iteration_relation].compact
return ::Iteration.none if relation.all?(::Iteration.none) return ::Iteration.none if relation.all?(::Iteration.none)
::Iteration.from_union(relation).includes(:project, :group) # rubocop: disable CodeReuse/ActiveRecord ::Iteration.from_union(relation).includes(:project, :group, :iterations_cadence) # rubocop: disable CodeReuse/ActiveRecord
end end
def find_object(parent_object, id) def find_object(parent_object, id)
......
...@@ -6,7 +6,6 @@ FactoryBot.define do ...@@ -6,7 +6,6 @@ FactoryBot.define do
end end
factory :iteration do factory :iteration do
title
start_date { generate(:sequential_date) } start_date { generate(:sequential_date) }
due_date { generate(:sequential_date) } due_date { generate(:sequential_date) }
...@@ -19,6 +18,10 @@ FactoryBot.define do ...@@ -19,6 +18,10 @@ FactoryBot.define do
resource_parent { nil } resource_parent { nil }
end end
trait :with_title do
title
end
trait :upcoming do trait :upcoming do
state_enum { Iteration::STATE_ENUM_MAP[:upcoming] } state_enum { Iteration::STATE_ENUM_MAP[:upcoming] }
end end
......
...@@ -64,7 +64,7 @@ RSpec.describe 'User adds milestone/iterations lists', :js, :aggregate_failures ...@@ -64,7 +64,7 @@ RSpec.describe 'User adds milestone/iterations lists', :js, :aggregate_failures
it 'creates iteration column' do it 'creates iteration column' do
add_list('Iteration', iteration.period) add_list('Iteration', iteration.period)
expect(page).to have_selector('.board', text: iteration.title) expect(page).to have_selector('.board', text: iteration.display_text)
expect(find('.board:nth-child(2) .board-card')).to have_content(issue_with_iteration.title) expect(find('.board:nth-child(2) .board-card')).to have_content(issue_with_iteration.title)
end end
end end
......
...@@ -73,9 +73,9 @@ RSpec.describe 'Filter issues by iteration', :js do ...@@ -73,9 +73,9 @@ RSpec.describe 'Filter issues by iteration', :js do
end end
end end
context 'when passing specific iteration by title' do context 'when passing specific iteration by period' do
before do before do
set_filter('iteration', iteration_1.title) set_filter('iteration', iteration_1.period)
end end
it_behaves_like 'filters issues by iteration' it_behaves_like 'filters issues by iteration'
...@@ -93,17 +93,17 @@ RSpec.describe 'Filter issues by iteration', :js do ...@@ -93,17 +93,17 @@ RSpec.describe 'Filter issues by iteration', :js do
before do before do
visit page_path visit page_path
set_negated_filter('iteration', iteration_title) set_negated_filter('iteration', iteration_item)
end end
context 'with specific iteration' do context 'with specific iteration' do
let(:iteration_title) { iteration_1.title } let(:iteration_item) { iteration_1.period }
it_behaves_like 'filters issues by negated iteration' it_behaves_like 'filters issues by negated iteration'
end end
context 'with current iteration' do context 'with current iteration' do
let(:iteration_title) { 'Current' } let(:iteration_item) { 'Current' }
it_behaves_like 'filters issues by negated iteration' it_behaves_like 'filters issues by negated iteration'
end end
...@@ -123,14 +123,14 @@ RSpec.describe 'Filter issues by iteration', :js do ...@@ -123,14 +123,14 @@ RSpec.describe 'Filter issues by iteration', :js do
click_link '= is' click_link '= is'
end end
it 'shows cadence titles, and iteration titles and dates', :aggregate_failures do it 'shows cadence titles, and iteration periods and dates', :aggregate_failures do
within '.gl-filtered-search-suggestion-list' do within '.gl-filtered-search-suggestion-list' do
# cadence 1 grouping # cadence 1 grouping
expect(page).to have_css('li:nth-child(6)', text: "#{iteration_1.period} #{iteration_1.title}") expect(page).to have_css('li:nth-child(6)', text: iteration_1.period)
expect(page).to have_css('li:nth-child(7)', text: "#{iteration_3.period} #{iteration_3.title}") expect(page).to have_css('li:nth-child(7)', text: iteration_3.period)
# cadence 2 grouping # cadence 2 grouping
expect(page).to have_css('li:nth-child(9)', text: cadence_2.title) expect(page).to have_css('li:nth-child(9)', text: cadence_2.title)
expect(page).to have_css('li:nth-child(10)', text: "#{iteration_2.period} #{iteration_2.title}") expect(page).to have_css('li:nth-child(10)', text: iteration_2.period)
end end
end end
end end
......
...@@ -171,16 +171,14 @@ RSpec.describe 'Issue Sidebar' do ...@@ -171,16 +171,14 @@ RSpec.describe 'Issue Sidebar' do
within '[data-testid="iteration-edit"]' do within '[data-testid="iteration-edit"]' do
expect(page).not_to have_text(iteration_cadence.title) expect(page).not_to have_text(iteration_cadence.title)
expect(page).to have_text(iteration.title) expect(page).to have_text(iteration.period)
expect(page).to have_text(iteration_period(iteration))
end end
select_iteration(iteration.title) select_iteration(iteration.period)
within '[data-testid="select-iteration"]' do within '[data-testid="select-iteration"]' do
expect(page).not_to have_text(iteration_cadence.title) expect(page).not_to have_text(iteration_cadence.title)
expect(page).to have_text(iteration.title) expect(page).to have_text(iteration.period)
expect(page).to have_text(iteration_period(iteration))
end end
find_and_click_edit_iteration find_and_click_edit_iteration
...@@ -215,7 +213,7 @@ RSpec.describe 'Issue Sidebar' do ...@@ -215,7 +213,7 @@ RSpec.describe 'Issue Sidebar' do
find_and_click_edit_iteration find_and_click_edit_iteration
page.within '[data-testid="iteration-edit"]' do page.within '[data-testid="iteration-edit"]' do
expect(page).not_to have_content iteration2.title expect(page).not_to have_content iteration2.period
end end
end end
end end
...@@ -234,16 +232,14 @@ RSpec.describe 'Issue Sidebar' do ...@@ -234,16 +232,14 @@ RSpec.describe 'Issue Sidebar' do
within '[data-testid="iteration-edit"]' do within '[data-testid="iteration-edit"]' do
expect(page).to have_text(iteration_cadence.title) expect(page).to have_text(iteration_cadence.title)
expect(page).to have_text(iteration.title) expect(page).to have_text(iteration.period)
expect(page).to have_text(iteration_period(iteration))
end end
select_iteration(iteration.title) select_iteration(iteration.period)
within '[data-testid="select-iteration"]' do within '[data-testid="select-iteration"]' do
expect(page).to have_text(iteration_cadence.title) expect(page).to have_text(iteration_cadence.title)
expect(page).to have_text(iteration.title) expect(page).to have_text(iteration.period)
expect(page).to have_text(iteration_period(iteration))
end end
find_and_click_edit_iteration find_and_click_edit_iteration
...@@ -278,7 +274,7 @@ RSpec.describe 'Issue Sidebar' do ...@@ -278,7 +274,7 @@ RSpec.describe 'Issue Sidebar' do
find_and_click_edit_iteration find_and_click_edit_iteration
page.within '[data-testid="iteration-edit"]' do page.within '[data-testid="iteration-edit"]' do
expect(page).not_to have_content iteration2.title expect(page).not_to have_content iteration2.period
end end
end end
end end
......
...@@ -235,7 +235,7 @@ RSpec.describe IssuesFinder do ...@@ -235,7 +235,7 @@ RSpec.describe IssuesFinder do
end end
context 'filter by iteration' do context 'filter by iteration' do
let_it_be(:iteration_1) { create(:iteration, group: group, start_date: 2.days.from_now, due_date: 3.days.from_now) } let_it_be(:iteration_1) { create(:iteration, :with_title, group: group, start_date: 2.days.from_now, due_date: 3.days.from_now) }
let_it_be(:iteration_2) { create(:iteration, group: group, start_date: 4.days.from_now, due_date: 5.days.from_now) } let_it_be(:iteration_2) { create(:iteration, group: group, start_date: 4.days.from_now, due_date: 5.days.from_now) }
let_it_be(:iteration_1_issue) { create(:issue, project: project1, iteration: iteration_1) } let_it_be(:iteration_1_issue) { create(:issue, project: project1, iteration: iteration_1) }
......
...@@ -62,7 +62,10 @@ ...@@ -62,7 +62,10 @@
"type": "integer" "type": "integer"
}, },
"title": { "title": {
"type": "string" "type": [
"string",
"null"
]
}, },
"description": { "description": {
"type": [ "type": [
......
...@@ -119,7 +119,7 @@ RSpec.describe Mutations::Boards::Lists::Create do ...@@ -119,7 +119,7 @@ RSpec.describe Mutations::Boards::Lists::Create do
new_list = subject[:list] new_list = subject[:list]
expect(new_list.title).to eq "#{iteration.title}" expect(new_list.title).to eq "#{iteration.display_text}"
expect(new_list.iteration_id).to eq iteration.id expect(new_list.iteration_id).to eq iteration.id
expect(new_list.position).to eq 0 expect(new_list.position).to eq 0
end end
......
...@@ -15,7 +15,7 @@ RSpec.describe Resolvers::BoardListIssuesResolver do ...@@ -15,7 +15,7 @@ RSpec.describe Resolvers::BoardListIssuesResolver do
let_it_be(:epic) { create(:epic, group: group) } let_it_be(:epic) { create(:epic, group: group) }
let_it_be(:iteration_cadence1) { create(:iterations_cadence, group: group) } let_it_be(:iteration_cadence1) { create(:iterations_cadence, group: group) }
let_it_be(:iteration_cadence2) { create(:iterations_cadence, group: group) } let_it_be(:iteration_cadence2) { create(:iterations_cadence, group: group) }
let_it_be(:iteration) { create(:iteration, group: group, start_date: 1.week.ago, due_date: 2.days.ago, iterations_cadence: iteration_cadence1) } let_it_be(:iteration) { create(:iteration, :with_title, group: group, start_date: 1.week.ago, due_date: 2.days.ago, iterations_cadence: iteration_cadence1) }
let_it_be(:current_iteration) { create(:iteration, group: group, start_date: Date.yesterday, due_date: 1.day.from_now, iterations_cadence: iteration_cadence2) } let_it_be(:current_iteration) { create(:iteration, group: group, start_date: Date.yesterday, due_date: 1.day.from_now, iterations_cadence: iteration_cadence2) }
let_it_be(:issue1) { create(:issue, project: project, labels: [label], weight: 3) } let_it_be(:issue1) { create(:issue, project: project, labels: [label], weight: 3) }
......
...@@ -168,25 +168,6 @@ RSpec.describe Banzai::Filter::References::IterationReferenceFilter do ...@@ -168,25 +168,6 @@ RSpec.describe Banzai::Filter::References::IterationReferenceFilter do
end end
end end
shared_examples 'references with HTML entities' do
before do
iteration.update!(title: '&lt;html&gt;')
end
it 'links to a valid reference' do
doc = reference_filter('See *iteration:"&lt;html&gt;"')
expect(doc.css('a').first.attr('href')).to eq urls.iteration_url(iteration)
expect(doc.text).to eq 'See <html>'
end
it 'ignores invalid iteration names and escapes entities' do
act = %(Iteration *iteration:"&lt;non valid&gt;")
expect(reference_filter(act).to_html).to eq act
end
end
shared_context 'group iterations' do shared_context 'group iterations' do
let(:reference) { iteration.to_reference(format: :name) } let(:reference) { iteration.to_reference(format: :name) }
...@@ -195,12 +176,6 @@ RSpec.describe Banzai::Filter::References::IterationReferenceFilter do ...@@ -195,12 +176,6 @@ RSpec.describe Banzai::Filter::References::IterationReferenceFilter do
it_behaves_like 'String-based single-word references' it_behaves_like 'String-based single-word references'
it_behaves_like 'String-based multi-word references in quotes' it_behaves_like 'String-based multi-word references in quotes'
it_behaves_like 'referencing a iteration in a link href' it_behaves_like 'referencing a iteration in a link href'
it_behaves_like 'references with HTML entities'
it_behaves_like 'HTML text with references' do
let(:resource) { iteration }
let(:resource_text) { resource.title }
end
it_behaves_like 'Integer-based references' do it_behaves_like 'Integer-based references' do
let(:reference) { iteration.to_reference(format: :id) } let(:reference) { iteration.to_reference(format: :id) }
...@@ -286,7 +261,7 @@ RSpec.describe Banzai::Filter::References::IterationReferenceFilter do ...@@ -286,7 +261,7 @@ RSpec.describe Banzai::Filter::References::IterationReferenceFilter do
context 'when iteration is open' do context 'when iteration is open' do
context 'group iterations' do context 'group iterations' do
let(:iteration) { create(:iteration, group: group) } let(:iteration) { create(:iteration, :with_title, group: group) }
include_context 'group iterations' include_context 'group iterations'
end end
...@@ -294,7 +269,7 @@ RSpec.describe Banzai::Filter::References::IterationReferenceFilter do ...@@ -294,7 +269,7 @@ RSpec.describe Banzai::Filter::References::IterationReferenceFilter do
context 'when iteration is closed' do context 'when iteration is closed' do
context 'group iterations' do context 'group iterations' do
let(:iteration) { create(:iteration, :closed, group: group) } let(:iteration) { create(:iteration, :with_title, :closed, group: group) }
include_context 'group iterations' include_context 'group iterations'
end end
...@@ -303,15 +278,15 @@ RSpec.describe Banzai::Filter::References::IterationReferenceFilter do ...@@ -303,15 +278,15 @@ RSpec.describe Banzai::Filter::References::IterationReferenceFilter do
context 'checking N+1' do context 'checking N+1' do
let_it_be(:group) { create(:group) } let_it_be(:group) { create(:group) }
let_it_be(:group2) { create(:group, parent: group) } let_it_be(:group2) { create(:group, parent: group) }
let_it_be(:iteration) { create(:iteration, group: group) } let_it_be(:iteration) { create(:iteration, :with_title, group: group) }
let_it_be(:iteration_reference) { iteration.to_reference(format: :name) } let_it_be(:iteration_reference) { iteration.to_reference(format: :name) }
let_it_be(:iteration2) { create(:iteration, group: group) } let_it_be(:iteration2) { create(:iteration, :with_title, group: group) }
let_it_be(:iteration2_reference) { iteration2.to_reference(format: :id) } let_it_be(:iteration2_reference) { iteration2.to_reference(format: :id) }
let_it_be(:iteration3) { create(:iteration, group: group2) } let_it_be(:iteration3) { create(:iteration, :with_title, group: group2) }
let_it_be(:iteration3_reference) { iteration3.to_reference(format: :name) } let_it_be(:iteration3_reference) { iteration3.to_reference(format: :name) }
it 'does not have N+1 per multiple references per group', :use_sql_query_cache, :aggregate_failures do it 'does not have N+1 per multiple references per group', :use_sql_query_cache, :aggregate_failures do
max_count = 3 max_count = 4
markdown = "#{iteration_reference}" markdown = "#{iteration_reference}"
# warm the cache # warm the cache
...@@ -330,7 +305,7 @@ RSpec.describe Banzai::Filter::References::IterationReferenceFilter do ...@@ -330,7 +305,7 @@ RSpec.describe Banzai::Filter::References::IterationReferenceFilter do
it 'has N+1 for multiple unique group references', :use_sql_query_cache do it 'has N+1 for multiple unique group references', :use_sql_query_cache do
markdown = "#{iteration_reference}" markdown = "#{iteration_reference}"
max_count = 3 max_count = 4
# warm the cache # warm the cache
reference_filter(markdown, { project: nil, group: group2 }) reference_filter(markdown, { project: nil, group: group2 })
......
...@@ -24,9 +24,8 @@ RSpec.describe Emails::Issues do ...@@ -24,9 +24,8 @@ RSpec.describe Emails::Issues do
it 'shows the iteration it was changed to' do it 'shows the iteration it was changed to' do
expect(subject).to have_body_text 'Iteration changed to' expect(subject).to have_body_text 'Iteration changed to'
expect(subject).to have_body_text 'Sep 30, 2022 - Oct 4, 2022' expect(subject).to have_body_text iteration.period
expect(subject).to have_body_text iteration.name expect(subject).not_to have_body_text iterations_cadence.title
expect(subject).not_to have_body_text 'Plan cadence'
end end
context 'when iteration_cadences FF enabled' do context 'when iteration_cadences FF enabled' do
...@@ -35,7 +34,7 @@ RSpec.describe Emails::Issues do ...@@ -35,7 +34,7 @@ RSpec.describe Emails::Issues do
end end
it 'shows the iteration it was changed to' do it 'shows the iteration it was changed to' do
expect(subject).to have_body_text 'Plan cadence Sep 30, 2022 - Oct 4, 2022' expect(subject).to have_body_text iteration.display_text
end end
end end
end end
......
...@@ -62,6 +62,16 @@ RSpec.describe Iteration do ...@@ -62,6 +62,16 @@ RSpec.describe Iteration do
end end
end end
describe '#period' do
let_it_be(:group) { create(:group) }
let_it_be(:iterations_cadence) { create(:iterations_cadence, group: group) }
let_it_be(:iteration) { create(:iteration, iterations_cadence: iterations_cadence, start_date: Date.new(2022, 9, 30), due_date: Date.new(2022, 10, 4)) }
subject { iteration.period }
it { is_expected.to eq('Sep 30, 2022 - Oct 4, 2022') }
end
describe '.reference_pattern' do describe '.reference_pattern' do
let_it_be(:group) { create(:group) } let_it_be(:group) { create(:group) }
let_it_be(:iteration_cadence) { create(:iterations_cadence, group: group) } let_it_be(:iteration_cadence) { create(:iterations_cadence, group: group) }
...@@ -371,6 +381,14 @@ RSpec.describe Iteration do ...@@ -371,6 +381,14 @@ RSpec.describe Iteration do
end end
end end
end end
describe 'title' do
subject { build(:iteration, iterations_cadence: iteration_cadence, title: '<img src=x onerror=prompt(1)>') }
it 'sanitizes user intput', :aggregate_failures do
expect(subject.title).to be_blank
end
end
end end
describe 'relations' do describe 'relations' do
...@@ -846,7 +864,7 @@ RSpec.describe Iteration do ...@@ -846,7 +864,7 @@ RSpec.describe Iteration do
expect(new_timebox).to be_valid expect(new_timebox).to be_valid
end end
it "does not accept the same title when in same cadence" do it "does not accept the same title when in the same cadence" do
new_timebox = described_class.new(group: group, iterations_cadence: cadence, title: timebox.title) new_timebox = described_class.new(group: group, iterations_cadence: cadence, title: timebox.title)
expect(new_timebox).not_to be_valid expect(new_timebox).not_to be_valid
......
...@@ -64,7 +64,7 @@ RSpec.describe List do ...@@ -64,7 +64,7 @@ RSpec.describe List do
end end
context 'when it is an iteration type' do context 'when it is an iteration type' do
let(:iteration) { build(:iteration, title: 'awesome-iteration', group: create(:group)) } let(:iteration) { build(:iteration, group: create(:group)) }
subject { described_class.new(list_type: :iteration, iteration: iteration, board: board) } subject { described_class.new(list_type: :iteration, iteration: iteration, board: board) }
...@@ -84,8 +84,8 @@ RSpec.describe List do ...@@ -84,8 +84,8 @@ RSpec.describe List do
end end
describe '#title' do describe '#title' do
it 'returns the iteration title' do it 'returns the iteration cadence and period as title' do
expect(subject.title).to eq('awesome-iteration') expect(subject.title).to eq(iteration.display_text)
end end
end end
end end
......
...@@ -161,7 +161,7 @@ RSpec.describe Issue do ...@@ -161,7 +161,7 @@ RSpec.describe Issue do
end end
context 'iterations' do context 'iterations' do
let_it_be(:iteration1) { create(:iteration) } let_it_be(:iteration1) { create(:iteration, :with_title) }
let_it_be(:iteration2) { create(:iteration) } let_it_be(:iteration2) { create(:iteration) }
let_it_be(:iteration1_issue) { create(:issue, iteration: iteration1) } let_it_be(:iteration1_issue) { create(:issue, iteration: iteration1) }
let_it_be(:iteration2_issue) { create(:issue, iteration: iteration2) } let_it_be(:iteration2_issue) { create(:issue, iteration: iteration2) }
......
...@@ -141,7 +141,7 @@ RSpec.describe 'Querying an Iteration' do ...@@ -141,7 +141,7 @@ RSpec.describe 'Querying an Iteration' do
context 'when ID argument is missing' do context 'when ID argument is missing' do
let(:query) do let(:query) do
graphql_query_for('iteration', {}, 'title') graphql_query_for('iteration', {}, 'id')
end end
it 'raises an exception' do it 'raises an exception' do
......
...@@ -143,11 +143,49 @@ RSpec.describe 'Creating an Iteration' do ...@@ -143,11 +143,49 @@ RSpec.describe 'Creating an Iteration' do
end end
end end
context 'with iterations_cadences FF enabled' do
before do
stub_feature_flags(iteration_cadences: true)
end
context 'when title is not given' do
let(:attributes) { { start_date: start_date, due_date: end_date } }
it 'creates an iteration' do
post_graphql_mutation(mutation, current_user: current_user)
iteration_hash = mutation_response['iteration']
aggregate_failures do
expect(iteration_hash['title']).to eq(nil)
expect(iteration_hash['startDate']).to eq(start_date)
expect(iteration_hash['dueDate']).to eq(end_date)
end
end
end
end
context 'with iterations_cadences FF disabled' do
before do
stub_feature_flags(iteration_cadences: false)
end
context 'when title is not given' do
let(:attributes) { { start_date: start_date, due_date: end_date } }
it_behaves_like 'a mutation that returns top-level errors',
errors: ["Title can't be blank"]
it 'does not create the iteration' do
expect { post_graphql_mutation(mutation, current_user: current_user) }.not_to change(Iteration, :count)
end
end
end
context 'when there are ActiveRecord validation errors' do context 'when there are ActiveRecord validation errors' do
let(:attributes) { { title: '' } } let(:attributes) { { description: '' } }
it_behaves_like 'a mutation that returns errors in the response', it_behaves_like 'a mutation that returns errors in the response',
errors: ["Start date can't be blank", "Due date can't be blank", "Title can't be blank"] errors: ["Start date can't be blank", "Due date can't be blank"]
it 'does not create the iteration' do it 'does not create the iteration' do
expect { post_graphql_mutation(mutation, current_user: current_user) }.not_to change(Iteration, :count) expect { post_graphql_mutation(mutation, current_user: current_user) }.not_to change(Iteration, :count)
......
...@@ -72,19 +72,57 @@ RSpec.describe 'Updating an Iteration' do ...@@ -72,19 +72,57 @@ RSpec.describe 'Updating an Iteration' do
# Let's check that the mutation response is good # Let's check that the mutation response is good
iteration_hash = mutation_response['iteration'] iteration_hash = mutation_response['iteration']
expect(iteration_hash['title']).to eq('title')
expect(iteration_hash['description']).to eq('some description') expect(iteration_hash['description']).to eq('some description')
expect(iteration_hash['startDate'].to_date).to eq(start_date.to_date) expect(iteration_hash['startDate'].to_date).to eq(start_date.to_date)
expect(iteration_hash['dueDate'].to_date).to eq(end_date.to_date) expect(iteration_hash['dueDate'].to_date).to eq(end_date.to_date)
# Let's also check that the object was updated properly # Let's also check that the object was updated properly
iteration.reload iteration.reload
expect(iteration.title).to eq('title')
expect(iteration.description).to eq('some description') expect(iteration.description).to eq('some description')
expect(iteration.start_date).to eq(start_date.to_date) expect(iteration.start_date).to eq(start_date.to_date)
expect(iteration.due_date).to eq(end_date.to_date) expect(iteration.due_date).to eq(end_date.to_date)
end end
context 'when updating title' do
context 'with iterations_cadences FF enabled' do
using RSpec::Parameterized::TableSyntax
before do
stub_feature_flags(iteration_cadences: true)
end
where(:title_before, :title_after, :expected_title) do
nil | "abc" | "abc"
"abc" | "def" | "def"
end
with_them do
let(:iteration) { create(:iteration, title: title_before, group: group, iterations_cadence: cadence) }
let(:attributes) { { title: title_after } }
it 'updates an iteration', :aggregate_failures do
post_graphql_mutation(mutation, current_user: current_user)
expect(mutation_response['iteration']['title']).to eq(expected_title)
expect(iteration.reload.title).to eq(expected_title)
end
end
end
context 'with iterations_cadences FF disabled' do
before do
stub_feature_flags(iteration_cadences: false)
end
context 'when title is not given' do
let(:attributes) { { title: "" } }
it_behaves_like 'a mutation that returns top-level errors',
errors: ["Title can't be blank"]
end
end
end
context 'when updating dates' do context 'when updating dates' do
let_it_be(:start_date) { 1.month.ago } let_it_be(:start_date) { 1.month.ago }
let_it_be(:end_date) { 1.month.ago + 1.day } let_it_be(:end_date) { 1.month.ago + 1.day }
...@@ -102,10 +140,6 @@ RSpec.describe 'Updating an Iteration' do ...@@ -102,10 +140,6 @@ RSpec.describe 'Updating an Iteration' do
expect(iteration.start_date).to eq(start_date.to_date) expect(iteration.start_date).to eq(start_date.to_date)
end end
it 'does not update the iteration title' do
expect { post_graphql_mutation(mutation, current_user: current_user) }.not_to change(iteration, :title)
end
context 'when another iteration with given dates overlap' do context 'when another iteration with given dates overlap' do
let_it_be(:another_iteration) { create(:iteration, group: group, iterations_cadence: cadence, start_date: start_date.strftime('%F'), due_date: end_date.strftime('%F') ) } let_it_be(:another_iteration) { create(:iteration, group: group, iterations_cadence: cadence, start_date: start_date.strftime('%F'), due_date: end_date.strftime('%F') ) }
...@@ -113,6 +147,8 @@ RSpec.describe 'Updating an Iteration' do ...@@ -113,6 +147,8 @@ RSpec.describe 'Updating an Iteration' do
errors: ["Dates cannot overlap with other existing Iterations within this iterations cadence"] errors: ["Dates cannot overlap with other existing Iterations within this iterations cadence"]
context 'with iterations_cadences FF disabled' do context 'with iterations_cadences FF disabled' do
let_it_be(:attributes) { { title: 'iteration', start_date: start_date.strftime('%F') } }
before do before do
stub_feature_flags(iteration_cadences: false) stub_feature_flags(iteration_cadences: false)
end end
......
...@@ -188,7 +188,7 @@ RSpec.describe API::Issues, :mailer do ...@@ -188,7 +188,7 @@ RSpec.describe API::Issues, :mailer do
end end
context 'filtering by iteration' do context 'filtering by iteration' do
let_it_be(:iteration_1) { create(:iteration, group: group, start_date: Date.current) } let_it_be(:iteration_1) { create(:iteration, :with_title, group: group, start_date: Date.current) }
let_it_be(:iteration_2) { create(:iteration, group: group) } let_it_be(:iteration_2) { create(:iteration, group: group) }
let_it_be(:iteration_1_issue) { create(:issue, project: group_project, iteration: iteration_1) } let_it_be(:iteration_1_issue) { create(:issue, project: group_project, iteration: iteration_1) }
let_it_be(:iteration_2_issue) { create(:issue, project: group_project, iteration: iteration_2) } let_it_be(:iteration_2_issue) { create(:issue, project: group_project, iteration: iteration_2) }
......
...@@ -161,8 +161,8 @@ RSpec.describe Iterations::Cadences::CreateIterationsInAdvanceService do ...@@ -161,8 +161,8 @@ RSpec.describe Iterations::Cadences::CreateIterationsInAdvanceService do
end end
context 'when cadence has iterations but all are in the past' do context 'when cadence has iterations but all are in the past' do
let_it_be(:past_iteration1) { create(:iteration, group: group, title: 'Iteration 1: some user note', iterations_cadence: automated_cadence, start_date: 3.weeks.ago, due_date: 2.weeks.ago)} let_it_be(:past_iteration1) { create(:iteration, group: group, title: 'Important iteration', iterations_cadence: automated_cadence, start_date: 3.weeks.ago, due_date: 2.weeks.ago)}
let_it_be(:past_iteration2) { create(:iteration, group: group, title: 'Iteration 2: some user note', iterations_cadence: automated_cadence, start_date: past_iteration1.due_date + 1.day, due_date: past_iteration1.due_date + 1.week)} let_it_be(:past_iteration2) { create(:iteration, group: group, iterations_cadence: automated_cadence, start_date: past_iteration1.due_date + 1.day, due_date: past_iteration1.due_date + 1.week)}
before do before do
automated_cadence.update!(iterations_in_advance: 2) automated_cadence.update!(iterations_in_advance: 2)
...@@ -181,15 +181,15 @@ RSpec.describe Iterations::Cadences::CreateIterationsInAdvanceService do ...@@ -181,15 +181,15 @@ RSpec.describe Iterations::Cadences::CreateIterationsInAdvanceService do
expect(automated_cadence.reload.last_run_date).to eq(automated_cadence.reload.iterations.last(2).first.due_date) expect(automated_cadence.reload.last_run_date).to eq(automated_cadence.reload.iterations.last(2).first.due_date)
end end
it 'sets the title correctly based on iterations count without modifying user-edited titles' do it 'does not modify the titles of the existing iterations (if they have any)' do
subject subject
expect(group.reload.iterations.due_date_order_asc.pluck(:title)).to eq([ expect(group.reload.iterations.due_date_order_asc.pluck(:title)).to eq([
'Iteration 1: some user note', 'Important iteration',
'Iteration 2: some user note', nil,
'Iteration 3', nil,
'Iteration 4', nil,
'Iteration 5' nil
]) ])
end end
......
...@@ -54,7 +54,7 @@ RSpec.describe Iterations::CreateService do ...@@ -54,7 +54,7 @@ RSpec.describe Iterations::CreateService do
end end
expect(response.error?).to be_truthy expect(response.error?).to be_truthy
expect(errors.messages).to match({ title: ["can't be blank"], due_date: ["can't be blank"], start_date: ["can't be blank"] }) expect(errors.messages).to match({ due_date: ["can't be blank"], start_date: ["can't be blank"] })
end end
end end
......
...@@ -34,7 +34,7 @@ ...@@ -34,7 +34,7 @@
"priority": { "type": ["integer", "null"] } "priority": { "type": ["integer", "null"] }
} }
}, },
"title": { "type": "string" }, "title": { "type": ["string", "null"] },
"position": { "type": ["integer", "null"] }, "position": { "type": ["integer", "null"] },
"max_issue_count": { "type": "integer" }, "max_issue_count": { "type": "integer" },
"max_issue_weight": { "type": "integer" }, "max_issue_weight": { "type": "integer" },
......
...@@ -65,6 +65,17 @@ RSpec.describe Milestone do ...@@ -65,6 +65,17 @@ RSpec.describe Milestone do
allow(subject).to receive(:set_iid).and_return(false) allow(subject).to receive(:set_iid).and_return(false)
end end
describe 'title' do
it { is_expected.to validate_presence_of(:title) }
it 'is invalid if title would be empty after sanitation', :aggregate_failures do
milestone = build(:milestone, project: project, title: '<img src=x onerror=prompt(1)>')
expect(milestone).not_to be_valid
expect(milestone.errors[:title]).to include("can't be blank")
end
end
describe 'milestone_releases' do describe 'milestone_releases' do
let(:milestone) { build(:milestone, project: project) } let(:milestone) { build(:milestone, project: project) }
......
...@@ -66,17 +66,6 @@ RSpec.shared_examples 'a timebox' do |timebox_type| ...@@ -66,17 +66,6 @@ RSpec.shared_examples 'a timebox' do |timebox_type|
end end
end end
describe 'title' do
it { is_expected.to validate_presence_of(:title) }
it 'is invalid if title would be empty after sanitation' do
timebox = build(timebox_type, *timebox_args, project: project, title: '<img src=x onerror=prompt(1)>')
expect(timebox).not_to be_valid
expect(timebox.errors[:title]).to include("can't be blank")
end
end
describe '#timebox_type_check' do describe '#timebox_type_check' do
it 'is invalid if it has both project_id and group_id' do it 'is invalid if it has both project_id and group_id' do
timebox = build(timebox_type, *timebox_args, group: group) timebox = build(timebox_type, *timebox_args, group: group)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment