diff --git a/.rubocop_todo/layout/empty_line_after_magic_comment.yml b/.rubocop_todo/layout/empty_line_after_magic_comment.yml index 1cb49d2b31e1c5d63c82b6ae4c5cf3f39e7f8a36..e8ed8a66a3ba37d76764d20ec966723d977db3c1 100644 --- a/.rubocop_todo/layout/empty_line_after_magic_comment.yml +++ b/.rubocop_todo/layout/empty_line_after_magic_comment.yml @@ -94,7 +94,6 @@ Layout/EmptyLineAfterMagicComment: - 'ee/app/graphql/types/vulnerability_scanner_vendor_input_type.rb' - 'ee/app/helpers/ee/admin/identities_helper.rb' - 'ee/app/helpers/ee/hooks_helper.rb' - - 'ee/app/helpers/ee/routing/projects_helper.rb' - 'ee/app/helpers/ee/sorting_helper.rb' - 'ee/app/helpers/ee/sorting_titles_values_helper.rb' - 'ee/app/models/analytics/devops_adoption.rb' diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 9a1f4aae82902747e840da84ecd9f1aad2407a06..f10c1b96014c537c1e9376e0633fb09666addf8e 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -132,7 +132,7 @@ def new end def show - return super unless issue.show_as_work_item? && request.format.html? + return super if issue.require_legacy_views? || !html_request? if project&.work_items_consolidated_list_enabled?(current_user) if params[:vueroute].present? && request.path.include?('/designs/') @@ -480,7 +480,8 @@ def add_related_issue def create_vulnerability_issue_feedback(issue); end def redirect_if_work_item - return unless use_work_items_path?(issue) + return unless html_request? + return unless issue.show_as_work_item? query_params = request.query_parameters query_params = query_params.merge(edit: 'true') if action_name == 'edit' && can?(current_user, :update_issue, issue) diff --git a/app/helpers/routing/projects_helper.rb b/app/helpers/routing/projects_helper.rb index cc1bfac24014c8ff7a6831444e00e8fa7e1e0a56..2c0cd4e06d266b7024aac4a695e522fca9ef114c 100644 --- a/app/helpers/routing/projects_helper.rb +++ b/app/helpers/routing/projects_helper.rb @@ -23,7 +23,11 @@ def environment_delete_path(environment, *args) end def issue_path(entity, *args) - project_issue_path(entity.project, entity, *args) + if entity.show_as_work_item? + project_work_item_path(entity.project, entity, *args) + else + project_issue_path(entity.project, entity, *args) + end end def merge_request_path(entity, *args) @@ -35,7 +39,7 @@ def pipeline_path(pipeline, *args) end def issue_url(entity, *args) - if use_work_items_path?(entity) + if entity.show_as_work_item? work_item_url(entity, *args) else project_issue_url(entity.project, entity, *args) @@ -45,7 +49,7 @@ def issue_url(entity, *args) def work_item_url(entity, *args) return group_work_item_url(entity.namespace, entity.iid, *args) unless entity.project.present? - if use_issue_path?(entity) + if !entity.show_as_work_item? project_issue_url(entity.project, entity.iid, *args) else project_work_item_url(entity.project, entity.iid, *args) @@ -91,18 +95,6 @@ def toggle_subscription_path(entity, *args) toggle_subscription_project_merge_request_path(entity.project, entity) end end - - private - - def use_work_items_path?(issue) - return true if issue.project.blank? && issue.namespace.present? - - issue.issue_type == 'task' - end - - def use_issue_path?(work_item) - work_item.work_item_type.issue? || work_item.work_item_type.incident? || work_item.from_service_desk? - end end end diff --git a/app/models/issue.rb b/app/models/issue.rb index b459712130a5f64bcee9064c3703add24b004a67..8285babc8ecca49429895f815e0f746983465466 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -902,11 +902,28 @@ def group_epic_work_item? epic_work_item? && group_level? end - # Service Desk issues and incidents should not use the work item view, - # since these have not been migrated over to using the work items framework. - # These should continue to use the .../issues/... path and render as issues. + # Some Issues types/conditions were not fully migrated to WorkItems UI/workflows yet. + # On the other hand some other Issue types/conditions are only available through + # WorkItems UI/workflows. + # + # Overriden on EE def show_as_work_item? - !from_service_desk? && !work_item_type&.incident? + # WorkItems only views + return true if work_item_type&.task? + return true if work_item_type&.epic? + return true if project.blank? && namespace.present? + + # Legacy only views + return false if require_legacy_views? + + resource_parent.work_items_consolidated_list_enabled? + end + + # Legacy views/workflows only + # - Service Desk were not converted to the work items framework. + # - Incidents were not converted to the work items framework. + def require_legacy_views? + from_service_desk? || work_item_type&.incident? end def ensure_work_item_description diff --git a/app/views/notify/issue_moved_email.html.haml b/app/views/notify/issue_moved_email.html.haml index 666aa45540eac31a6db7171bc8cb2aa78deb3bee..158cbcffa07c024466194fc971171c2251a87d0e 100644 --- a/app/views/notify/issue_moved_email.html.haml +++ b/app/views/notify/issue_moved_email.html.haml @@ -2,6 +2,6 @@ = s_('Notify|Issue was moved to another project.') - if @can_access_project %p - = sprintf(s_('Notify|New issue: %{project_issue_url}'), { project_issue_url: link_to(@new_issue.title, project_issue_url(@new_project, @new_issue)) }).html_safe + = sprintf(s_('Notify|New issue: %{project_issue_url}'), { project_issue_url: link_to(@new_issue.title, issue_url(@new_issue)) }).html_safe - else = s_("Notify|You don't have access to the project.") diff --git a/ee/app/helpers/ee/routing/projects_helper.rb b/ee/app/helpers/ee/routing/projects_helper.rb deleted file mode 100644 index 87abe476b27857bc922b57e4dc724d52bd87952e..0000000000000000000000000000000000000000 --- a/ee/app/helpers/ee/routing/projects_helper.rb +++ /dev/null @@ -1,19 +0,0 @@ -# frozen_string_literal: true -module EE - module Routing - module ProjectsHelper - extend ::Gitlab::Utils::Override - - private - - override :use_work_items_path? - def use_work_items_path?(issue) - if issue.project&.okrs_mvc_feature_flag_enabled? && issue.licensed_feature_available?(:okrs) - return super || issue.work_item_type.objective? || issue.work_item_type.key_result? - end - - super - end - end - end -end diff --git a/ee/app/models/ee/issue.rb b/ee/app/models/ee/issue.rb index c2398ed42a3fe5b94589f6cf472dca2694c49d26..12ca9432f85ae6be2cce078cab187774365c663e 100644 --- a/ee/app/models/ee/issue.rb +++ b/ee/app/models/ee/issue.rb @@ -401,8 +401,26 @@ def supports_parent? supports_epic? && licensed_feature_available?(:epics) end + # # WorkItems only + # - OKRs (objectives and key results) are only works with WorkItems views. + # - Epics are only works with WorkItems views. + override :show_as_work_item? + def show_as_work_item? + if work_item_type&.objective? || work_item_type&.key_result? + return okr_available_and_enabled? + end + + return true if work_item_type&.epic? + + super + end + private + def okr_available_and_enabled? + licensed_feature_available?(:okrs) && resource_parent.okrs_mvc_feature_flag_enabled? + end + def blocking_issues_ids @blocking_issues_ids ||= self.class.related_link_class.blocking_issuables_ids_for(self) end diff --git a/ee/lib/ee/gitlab/url_builder.rb b/ee/lib/ee/gitlab/url_builder.rb index 5e16ddeade18f8625a0d5af85329814d773a5c12..47293cf93d2163f1e63d7f151b526d4512e7d432 100644 --- a/ee/lib/ee/gitlab/url_builder.rb +++ b/ee/lib/ee/gitlab/url_builder.rb @@ -23,7 +23,11 @@ def build(object, **options) instance.issue_url(object, **options) when WorkItem if object.epic_work_item? && object.project_id.nil? - instance.group_epic_url(object.namespace, object, **options) + if object.resource_parent.work_items_consolidated_list_enabled? + instance.work_item_url(object, **options) + else + instance.group_epic_url(object.namespace, object, **options) + end else super end @@ -59,7 +63,11 @@ def note_url(note, **options) noteable = note.noteable if note.for_epic? - instance.group_epic_url(noteable.group, noteable, anchor: dom_id(note), **options) + if noteable.group.work_items_consolidated_list_enabled? + instance.work_item_url(noteable, anchor: dom_id(note), **options) + else + instance.group_epic_url(noteable.group, noteable, anchor: dom_id(note), **options) + end elsif note.for_vulnerability? instance.project_security_vulnerability_url(noteable.project, noteable, anchor: dom_id(note), **options) elsif note.for_group_wiki? diff --git a/ee/spec/controllers/ee/projects/issues_controller_spec.rb b/ee/spec/controllers/ee/projects/issues_controller_spec.rb index 0e7b9fe5e306ddd0f374b2886a082f1bc25cdaed..c8142bc682f0e17ca83bc95ea706f28414a2f38a 100644 --- a/ee/spec/controllers/ee/projects/issues_controller_spec.rb +++ b/ee/spec/controllers/ee/projects/issues_controller_spec.rb @@ -120,27 +120,45 @@ def expected_params(params) describe 'GET #show' do let(:issue) { create(:issue, project: project) } - context 'when generate_description is licensed' do + context 'when work_item_planning_view: true' do before do - stub_licensed_features(generate_description: true) + stub_feature_flags(work_item_planning_view: true) end - it 'pushes generate_description licensed feature' do + it 'redirects to work item' do get :show, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } - expect(controller).to have_received(:push_licensed_feature).with(:generate_description, project) + expect(response).to redirect_to project_work_item_path(project, issue.iid) end end - context 'when generate_description is not licensed' do + context 'when work_item_planning_view: false' do before do - stub_licensed_features(generate_description: false) + stub_feature_flags(work_item_planning_view: false) end - it 'pushes generate_description licensed feature when user has permission regardless of license status' do - get :show, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } + context 'when generate_description is licensed' do + before do + stub_licensed_features(generate_description: true) + end - expect(controller).to have_received(:push_licensed_feature).with(:generate_description, project) + it 'pushes generate_description licensed feature' do + get :show, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } + + expect(controller).to have_received(:push_licensed_feature).with(:generate_description, project) + end + end + + context 'when generate_description is not licensed' do + before do + stub_licensed_features(generate_description: false) + end + + it 'pushes generate_description licensed feature when user has permission regardless of license status' do + get :show, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } + + expect(controller).to have_received(:push_licensed_feature).with(:generate_description, project) + end end end end @@ -723,6 +741,10 @@ def send_request let(:issue) { create(:issue, project: project) } let!(:discussion) { create(:discussion_note_on_issue, noteable: issue, project: issue.project) } + before do + stub_feature_flags(work_item_planning_view: false) + end + context 'with a related system note' do let(:confidential_issue) { create(:issue, :confidential, project: project) } let!(:system_note) { SystemNoteService.relate_issuable(issue, confidential_issue, user) } diff --git a/ee/spec/controllers/groups/epics_controller_spec.rb b/ee/spec/controllers/groups/epics_controller_spec.rb index dab60977a42d6f8e458118dbeee2ba4957df0efd..ad2bc3fd31e4f2156a80e2affc8b979137ba46aa 100644 --- a/ee/spec/controllers/groups/epics_controller_spec.rb +++ b/ee/spec/controllers/groups/epics_controller_spec.rb @@ -104,46 +104,52 @@ group.add_developer(user) end - context 'when issue note is returned' do + context 'when work_item_planning_view: false' do before do - SystemNoteService.epic_issue(epic, issue, user, :added) + stub_feature_flags(work_item_planning_view: false) end - shared_examples 'issue link presence' do - let(:issue) { create(:issue, project: project, description: "Project Issue") } + context 'when issue note is returned' do + before do + SystemNoteService.epic_issue(epic, issue, user, :added) + end + + shared_examples 'issue link presence' do + let(:issue) { create(:issue, project: project, description: "Project Issue") } - it 'the link to the issue is included' do - get :discussions, params: { group_id: group, id: epic.to_param } + it 'the link to the issue is included' do + get :discussions, params: { group_id: group, id: epic.to_param } - expect(response).to have_gitlab_http_status(:ok) - expect(json_response.size).to eq(1) - discussion = json_response[0] - notes = discussion["notes"] - expect(notes.size).to eq(1) - expect(notes[0]["note_html"]).to include(project_issue_path(project, issue)) + expect(response).to have_gitlab_http_status(:ok) + expect(json_response.size).to eq(1) + discussion = json_response[0] + notes = discussion["notes"] + expect(notes.size).to eq(1) + expect(notes[0]["note_html"]).to include(project_issue_path(project, issue)) + end end - end - describe 'project default namespace' do - it_behaves_like 'issue link presence' do - let(:project) { create(:project, :public) } + describe 'project default namespace' do + it_behaves_like 'issue link presence' do + let(:project) { create(:project, :public) } + end end - end - describe 'project group namespace' do - it_behaves_like 'issue link presence' do - let(:project) { create(:project, namespace: group) } + describe 'project group namespace' do + it_behaves_like 'issue link presence' do + let(:project) { create(:project, namespace: group) } + end end end - end - context 'setting notes filter' do - let(:issuable) { epic } - let(:issuable_parent) { group } - let!(:discussion_note) { create(:note, :system, noteable: issuable) } - let!(:discussion_comment) { create(:note, noteable: issuable) } + context 'setting notes filter' do + let(:issuable) { epic } + let(:issuable_parent) { group } + let!(:discussion_note) { create(:note, :system, noteable: issuable) } + let!(:discussion_comment) { create(:note, noteable: issuable) } - it_behaves_like 'issuable notes filter' + it_behaves_like 'issuable notes filter' + end end end diff --git a/ee/spec/lib/ee/gitlab/url_builder_spec.rb b/ee/spec/lib/ee/gitlab/url_builder_spec.rb index 6134eb85774a0bd231e99e75a6731ea5fc1ca451..8ff8e25dbd87dbca4d611d395f53d86b6acb6915 100644 --- a/ee/spec/lib/ee/gitlab/url_builder_spec.rb +++ b/ee/spec/lib/ee/gitlab/url_builder_spec.rb @@ -8,25 +8,60 @@ describe '.build' do using RSpec::Parameterized::TableSyntax + before do + stub_licensed_features(okrs: true) + end + + context 'when work_item_planning_view: false', feature_category: :team_planning do + where(:factory, :path_generator) do + :epic | ->(work_item) { "/groups/#{work_item.resource_parent.full_path}/-/epics/#{work_item.iid}" } + [:work_item, :epic, :group_level] | ->(work_item) { "/groups/#{work_item.resource_parent.full_path}/-/epics/#{work_item.iid}" } + [:work_item, :epic] | ->(work_item) { "/#{work_item.resource_parent.full_path}/-/work_items/#{work_item.iid}" } + + [:issue, :objective] | ->(work_item) { "/#{work_item.resource_parent.full_path}/-/work_items/#{work_item.iid}" } + [:issue, :key_result] | ->(work_item) { "/#{work_item.resource_parent.full_path}/-/work_items/#{work_item.iid}" } + + :note_on_epic | ->(note) { "/groups/#{note.noteable.resource_parent.full_path}/-/epics/#{note.noteable.iid}#note_#{note.id}" } + end + + with_them do + let(:object) { build_stubbed(*Array(factory)) } + let(:path) { path_generator.call(object) } + + before do + stub_feature_flags(work_item_planning_view: false) + end + + it 'returns the full URL' do + expect(subject.build(object)).to eq("#{Settings.gitlab['url']}#{path}") + end + + it 'returns only the path if only_path is set' do + expect(subject.build(object, only_path: true)).to eq(path) + end + end + end + where(:factory, :path_generator) do - :epic | ->(epic) { "/groups/#{epic.group.full_path}/-/epics/#{epic.iid}" } + :epic | ->(work_item) { "/groups/#{work_item.group.full_path}/-/epics/#{work_item.iid}" } + [:work_item, :epic] | ->(work_item) { "/#{work_item.project.full_path}/-/work_items/#{work_item.iid}" } + [:work_item, :epic, :group_level] | ->(work_item) { "/groups/#{work_item.namespace.full_path}/-/work_items/#{work_item.iid}" } + + [:issue, :objective] | ->(work_item) { "/#{work_item.project.full_path}/-/work_items/#{work_item.iid}" } + [:issue, :key_result] | ->(work_item) { "/#{work_item.project.full_path}/-/work_items/#{work_item.iid}" } + + :note_on_epic | ->(note) { "/groups/#{note.noteable.group.full_path}/-/work_items/#{note.noteable.iid}#note_#{note.id}" } + :note_on_vulnerability | ->(note) { "/#{note.project.full_path}/-/security/vulnerabilities/#{note.noteable.id}#note_#{note.id}" } + :epic_board | ->(epic_board) { "/groups/#{epic_board.group.full_path}/-/epic_boards/#{epic_board.id}" } :vulnerability | ->(vulnerability) { "/#{vulnerability.project.full_path}/-/security/vulnerabilities/#{vulnerability.id}" } :project_compliance_violation | ->(violation) { "/#{violation.project.full_path}/-/security/compliance_violations/#{violation.id}" } - :note_on_epic | ->(note) { "/groups/#{note.noteable.group.full_path}/-/epics/#{note.noteable.iid}#note_#{note.id}" } - :note_on_vulnerability | ->(note) { "/#{note.project.full_path}/-/security/vulnerabilities/#{note.noteable.id}#note_#{note.id}" } - - :group_wiki | ->(wiki) { "/groups/#{wiki.container.full_path}/-/wikis/home" } + :group_wiki | ->(wiki) { "/groups/#{wiki.container.full_path}/-/wikis/home" } :note_on_compliance_violation | ->(note) { "/#{note.project.full_path}/-/security/compliance_violations/#{note.noteable.id}#note_#{note.id}" } - [:issue, :objective] | ->(issue) { "/#{issue.project.full_path}/-/work_items/#{issue.iid}" } - [:issue, :key_result] | ->(issue) { "/#{issue.project.full_path}/-/work_items/#{issue.iid}" } - [:work_item, :epic, :group_level] | ->(epic_work_item) { "/groups/#{epic_work_item.namespace.full_path}/-/epics/#{epic_work_item.iid}" } - [:work_item, :epic] | ->(epic_work_item) { "/#{epic_work_item.project.full_path}/-/work_items/#{epic_work_item.iid}" } - [:issue, :key_result, :group_level] | ->(issue) { "/groups/#{issue.namespace.full_path}/-/work_items/#{issue.iid}" } :ai_catalog_agent | ->(item) { "/explore/ai-catalog/agents/#{item.id}" } @@ -37,9 +72,6 @@ with_them do let(:object) { build_stubbed(*Array(factory)) } let(:path) { path_generator.call(object) } - before do - stub_licensed_features(okrs: true) - end it 'returns the full URL' do expect(subject.build(object)).to eq("#{Settings.gitlab['url']}#{path}") diff --git a/ee/spec/models/issue_spec.rb b/ee/spec/models/issue_spec.rb index 069fd5842f1176773550b17a9282372ea8849311..5c3221c3113a31b607f8fee3ac1f6d6462e953a6 100644 --- a/ee/spec/models/issue_spec.rb +++ b/ee/spec/models/issue_spec.rb @@ -1526,4 +1526,35 @@ expect(issue.namespace_traversal_ids).to eq([root_group.id, group.id]) end end + + describe '#show_as_work_item?' do + subject(:issue_as_work_item) { issue.show_as_work_item? } + + where(:factory, :okr_ff, :okr_license, :result) do + [:work_item, :epic] | false | false | true + [:work_item, :objective] | false | false | false + [:work_item, :key_result] | false | false | false + [:work_item, :epic] | false | true | true + [:work_item, :objective] | false | true | false + [:work_item, :key_result] | false | true | false + [:work_item, :epic] | true | false | true + [:work_item, :objective] | true | false | false + [:work_item, :key_result] | true | false | false + [:work_item, :epic] | true | true | true + [:work_item, :objective] | true | true | true + [:work_item, :key_result] | true | true | true + end + + with_them do + let(:issue) { build_stubbed(*Array(factory)) } + + before do + stub_feature_flags(okrs_mvc: okr_ff) + + stub_licensed_features(okrs: okr_license) + end + + it { is_expected.to be result } + end + end end diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index ffdda48c80f7b3fa9d92d999e5646aa2c738319c..48f034682a917d1f925bfa730f96f5821b6a350e 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -171,102 +171,94 @@ project.add_developer(user) end - context 'issue email participants' do - context 'when issue is confidential' do - let(:issue) { create(:issue, project: project, confidential: true) } - let!(:participants) { create_list(:issue_email_participant, 2, issue: issue) } - - it "returns issue email participants" do - get :show, params: { namespace_id: project.namespace, project_id: project, id: issue.iid }, format: :json - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to include( - 'issue_email_participants' => contain_exactly( - { "email" => participants[0].email }, { "email" => participants[1].email } - ), - 'type' => 'ISSUE' - ) - end + context 'when work_item_planning_view: true' do + before do + stub_feature_flags(work_item_planning_view: true) end - context 'when issue is not confidential' do - it "returns empty email participants" do - get :show, params: { namespace_id: project.namespace, project_id: project, id: issue.iid }, format: :json + it 'redirects to work item' do + get :show, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to include('issue_email_participants' => []) - end + expect(response).to redirect_to project_work_item_path(project, issue.iid) end end - context 'when issue is not a task and work items feature flag is enabled' do - context 'when work_item_planning_view is enabled' do - before do - stub_feature_flags(work_item_planning_view: true) - end + context 'when work_item_planning_view: false' do + before do + stub_feature_flags(work_item_planning_view: false) + end - it 'redirects to work items route' do - get :show, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } + context 'issue email participants' do + context 'when issue is confidential' do + let(:issue) { create(:issue, project: project, confidential: true) } + let!(:participants) { create_list(:issue_email_participant, 2, issue: issue) } - expect(response).to redirect_to project_work_item_path(project, issue.iid) - end - end + it "returns issue email participants" do + get :show, params: { namespace_id: project.namespace, project_id: project, id: issue.iid }, format: :json - context 'when work_item_planning_view is disabled' do - before do - stub_feature_flags(work_item_planning_view: false) + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to include( + 'issue_email_participants' => contain_exactly( + { "email" => participants[0].email }, { "email" => participants[1].email } + ), + 'type' => 'ISSUE' + ) + end end - it 'does not redirect to work items route' do - get :show, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } + context 'when issue is not confidential' do + it "returns empty email participants" do + get :show, params: { namespace_id: project.namespace, project_id: project, id: issue.iid }, format: :json - expect(response).to render_template(:show) + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to include('issue_email_participants' => []) + end end end - end - context 'when issue is of type task' do - let(:query) { {} } + context 'when issue is of type task' do + let(:query) { {} } - let_it_be(:task) { create(:issue, :task, project: project) } + let_it_be(:task) { create(:issue, :task, project: project) } - shared_examples 'redirects to show work item page' do - it 'redirects to work item page using iid' do - make_request + shared_examples 'redirects to show work item page' do + it 'redirects to work item' do + make_request - expect(response).to redirect_to(project_work_item_path(project, task.iid, query)) + expect(response).to redirect_to(project_work_item_path(project, task.iid, query)) + end end - end - context 'show action' do - let(:query) { { query: 'any' } } + context 'show action' do + let(:query) { { query: 'any' } } - it_behaves_like 'redirects to show work item page' do - subject(:make_request) do - get :show, params: { namespace_id: project.namespace, project_id: project, id: task.iid, **query } + it_behaves_like 'redirects to show work item page' do + subject(:make_request) do + get :show, params: { namespace_id: project.namespace, project_id: project, id: task.iid, **query } + end end end - end - context 'edit action' do - let(:query) { { query: 'any', edit: 'true' } } + context 'edit action' do + let(:query) { { query: 'any', edit: 'true' } } - it_behaves_like 'redirects to show work item page' do - subject(:make_request) do - get :edit, params: { namespace_id: project.namespace, project_id: project, id: task.iid, query: 'any' } + it_behaves_like 'redirects to show work item page' do + subject(:make_request) do + get :edit, params: { namespace_id: project.namespace, project_id: project, id: task.iid, query: 'any' } + end end end - end - context 'update action' do - it_behaves_like 'redirects to show work item page' do - subject(:make_request) do - put :update, params: { - namespace_id: project.namespace, - project_id: project, - id: task.iid, - issue: { title: 'New title' } - } + context 'update action' do + it_behaves_like 'redirects to show work item page' do + subject(:make_request) do + put :update, params: { + namespace_id: project.namespace, + project_id: project, + id: task.iid, + issue: { title: 'New title' } + } + end end end end @@ -274,6 +266,40 @@ end describe 'GET #edit' do + context 'when work_item_planning_view: false' do + before do + stub_feature_flags(work_item_planning_view: false) + end + + context 'when visiting issues edit route and user can edit issue' do + before do + project.add_developer(user) + sign_in(user) + end + + it 'redirects to issues detail page with edit parameter' do + get :edit, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } + + expect(response).to redirect_to(project_issue_path(project, issue, edit: 'true')) + expect(response).to have_gitlab_http_status(:found) + end + end + + context 'when visiting issues edit route and user cannot edit issue' do + before do + project.add_guest(user) + sign_in(user) + end + + it 'redirects to issue detail page without edit parameter' do + get :edit, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } + + expect(response).to redirect_to(project_issue_path(project, issue, params: {})) + expect(response).to have_gitlab_http_status(:found) + end + end + end + context 'when visiting issues edit route and user can edit issue' do before do project.add_developer(user) @@ -283,7 +309,7 @@ it 'redirects to issues detail page with edit parameter' do get :edit, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } - expect(response).to redirect_to(project_issue_path(project, issue, edit: 'true')) + expect(response).to redirect_to(project_work_item_path(project, issue, edit: 'true')) expect(response).to have_gitlab_http_status(:found) end end @@ -297,7 +323,7 @@ it 'redirects to issue detail page without edit parameter' do get :edit, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } - expect(response).to redirect_to(project_issue_path(project, issue, params: {})) + expect(response).to redirect_to(project_work_item_path(project, issue, params: {})) expect(response).to have_gitlab_http_status(:found) end end @@ -310,7 +336,7 @@ sign_in(user) end - it 'redirects to work item detail page without edit parameter' do + it 'redirects to work item without edit parameter' do get :edit, params: { namespace_id: project.namespace, project_id: project, id: work_item_issue } expect(response).to redirect_to(project_work_item_path(project, work_item_issue.iid, params: {})) @@ -781,6 +807,10 @@ def go(id:) } end + before do + stub_feature_flags(work_item_planning_view: false) + end + context 'when an issue was edited' do before do project.add_developer(user) @@ -1541,6 +1571,7 @@ def post_new_issue_in_project expect(akismet_service).to receive_messages(submit_spam: true) end stub_application_setting(akismet_enabled: true) + stub_feature_flags(work_item_planning_view: false) end def post_spam @@ -1563,50 +1594,74 @@ def post_spam end describe "DELETE #destroy" do - context "when the user is a developer" do + let_it_be(:owner) { create(:user) } + let_it_be(:namespace) { create(:namespace, owner: owner) } + let_it_be(:project) { create(:project, namespace: namespace) } + + context 'when work_item_planning_view: true' do before do - sign_in(user) + sign_in(owner) + stub_feature_flags(work_item_planning_view: true) end - it "does not delete the issue, returning :not_found" do - delete :destroy, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } + it 'redirects to work item' do + delete :destroy, params: { + namespace_id: project.namespace, + project_id: project, + id: issue.iid, + destroy_confirm: true + } - expect(response).to have_gitlab_http_status(:not_found) + expect(response).to redirect_to project_work_item_path(project, issue.iid) end end - context "when the user is owner" do - let_it_be(:owner) { create(:user) } - let_it_be(:namespace) { create(:namespace, owner: owner) } - let_it_be(:project) { create(:project, namespace: namespace) } - + context 'when work_item_planning_view: false' do before do - sign_in(owner) + stub_feature_flags(work_item_planning_view: false) end - it "deletes the issue" do - delete :destroy, params: { namespace_id: project.namespace, project_id: project, id: issue.iid, destroy_confirm: true } + context "when the user is a developer" do + before do + sign_in(user) + end + + it "does not delete the issue, returning :not_found" do + delete :destroy, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } - expect(response).to have_gitlab_http_status(:see_other) - expect(controller).to set_flash[:notice].to(/The issue was successfully deleted\./) + expect(response).to have_gitlab_http_status(:not_found) + end end - it "prevents deletion if destroy_confirm is not set" do - expect(Gitlab::ErrorTracking).to receive(:track_exception).and_call_original + context "when the user is owner" do + before do + sign_in(owner) + end - delete :destroy, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } + it "deletes the issue" do + delete :destroy, params: { namespace_id: project.namespace, project_id: project, id: issue.iid, destroy_confirm: true } - expect(response).to have_gitlab_http_status(:found) - expect(controller).to set_flash[:notice].to('Destroy confirmation not provided for issue') - end + expect(response).to have_gitlab_http_status(:see_other) + expect(controller).to set_flash[:notice].to(/The issue was successfully deleted\./) + end - it "prevents deletion in JSON format if destroy_confirm is not set" do - expect(Gitlab::ErrorTracking).to receive(:track_exception).and_call_original + it "prevents deletion if destroy_confirm is not set" do + expect(Gitlab::ErrorTracking).to receive(:track_exception).and_call_original - delete :destroy, params: { namespace_id: project.namespace, project_id: project, id: issue.iid, format: 'json' } + delete :destroy, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } - expect(response).to have_gitlab_http_status(:unprocessable_entity) - expect(json_response).to eq({ 'errors' => 'Destroy confirmation not provided for issue' }) + expect(response).to have_gitlab_http_status(:found) + expect(controller).to set_flash[:notice].to('Destroy confirmation not provided for issue') + end + + it "prevents deletion in JSON format if destroy_confirm is not set" do + expect(Gitlab::ErrorTracking).to receive(:track_exception).and_call_original + + delete :destroy, params: { namespace_id: project.namespace, project_id: project, id: issue.iid, format: 'json' } + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(json_response).to eq({ 'errors' => 'Destroy confirmation not provided for issue' }) + end end end end @@ -1617,7 +1672,7 @@ def post_spam project.add_developer(user) end - subject do + subject(:make_request) do post(:toggle_award_emoji, params: { namespace_id: project.namespace, project_id: project, @@ -1628,28 +1683,44 @@ def post_spam let(:emoji_name) { AwardEmoji::THUMBS_UP } - it "toggles the award emoji" do - expect do - subject - end.to change { issue.award_emoji.count }.by(1) + context 'when work_item_planning_view: true' do + before do + stub_feature_flags(work_item_planning_view: true) + end - expect(response).to have_gitlab_http_status(:ok) + it 'redirects to work item' do + make_request + + expect(response).to redirect_to project_work_item_path(project, issue.iid) + end end - it "removes the already awarded emoji" do - create(:award_emoji, awardable: issue, name: emoji_name, user: user) + context 'when work_item_planning_view: false' do + before do + stub_feature_flags(work_item_planning_view: false) + end - expect { subject }.to change { AwardEmoji.count }.by(-1) + it "toggles the award emoji" do + expect { make_request }.to change { issue.award_emoji.count }.by(1) - expect(response).to have_gitlab_http_status(:ok) - end + expect(response).to have_gitlab_http_status(:ok) + end + + it "removes the already awarded emoji" do + create(:award_emoji, awardable: issue, name: emoji_name, user: user) + + expect { make_request }.to change { AwardEmoji.count }.by(-1) + + expect(response).to have_gitlab_http_status(:ok) + end - it 'marks Todos on the Issue as done' do - todo = create(:todo, target: issue, project: project, user: user) + it 'marks Todos on the Issue as done' do + todo = create(:todo, target: issue, project: project, user: user) - subject + make_request - expect(todo.reload).to be_done + expect(todo.reload).to be_done + end end end @@ -1886,13 +1957,31 @@ def get_service_desk(extra_params = {}) describe 'GET #discussions' do let!(:discussion) { create(:discussion_note_on_issue, noteable: issue, project: issue.project) } - context 'when authenticated' do + context 'when work_item_planning_view: true' do before do project.add_developer(user) sign_in(user) + stub_feature_flags(work_item_planning_view: true) + end + + it 'redirects to work item' do + get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } + + expect(response).to redirect_to project_work_item_path(project, issue.iid) + end + end + + context 'when work_item_planning_view: false' do + before do + stub_feature_flags(work_item_planning_view: false) end - context do + context 'when authenticated' do + before do + project.add_developer(user) + sign_in(user) + end + it_behaves_like 'discussions provider' do let!(:author) { create(:user) } let!(:project) { create(:project) } @@ -1912,124 +2001,124 @@ def get_service_desk(extra_params = {}) ] end end - end - - it 'returns discussion json' do - get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } - - expect(json_response.first.keys).to match_array(%w[id reply_id expanded notes diff_discussion discussion_path individual_note resolvable commit_id for_commit project_id confidential resolve_path resolved resolved_at resolved_by resolved_by_push]) - end - - it 'renders the author status html if there is a status' do - create(:user_status, user: discussion.author) - - get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } - - note_json = json_response.first['notes'].first - - expect(note_json['author']['status_tooltip_html']).to be_present - end - it 'does not cause an extra query for the status' do - control = ActiveRecord::QueryRecorder.new do + it 'returns discussion json' do get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } + + expect(json_response.first.keys).to match_array(%w[id reply_id expanded notes diff_discussion discussion_path individual_note resolvable commit_id for_commit project_id confidential resolve_path resolved resolved_at resolved_by resolved_by_push]) end - create(:user_status, user: discussion.author) - second_discussion = create(:discussion_note_on_issue, noteable: issue, project: issue.project, author: create(:user)) - create(:user_status, user: second_discussion.author) + it 'renders the author status html if there is a status' do + create(:user_status, user: discussion.author) - expect { get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } } - .not_to exceed_query_limit(control).with_threshold(9) - end + get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } - context 'when user is setting notes filters' do - let(:issuable) { issue } - let(:issuable_parent) { project } - let!(:discussion_note) { create(:discussion_note_on_issue, :system, noteable: issuable, project: project) } + note_json = json_response.first['notes'].first - it_behaves_like 'issuable notes filter' - end + expect(note_json['author']['status_tooltip_html']).to be_present + end - context 'with cross-reference system note', :request_store do - let_it_be(:new_issue) { create(:issue) } + it 'does not cause an extra query for the status' do + control = ActiveRecord::QueryRecorder.new do + get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } + end - let(:cross_reference) { "mentioned in #{new_issue.to_reference(issue.project)}" } + create(:user_status, user: discussion.author) + second_discussion = create(:discussion_note_on_issue, noteable: issue, project: issue.project, author: create(:user)) + create(:user_status, user: second_discussion.author) - before do - create(:discussion_note_on_issue, :system, noteable: issue, project: issue.project, note: cross_reference) + expect { get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } } + .not_to exceed_query_limit(control).with_threshold(9) end - it 'filters notes that the user should not see' do - get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } + context 'when user is setting notes filters' do + let(:issuable) { issue } + let(:issuable_parent) { project } + let!(:discussion_note) { create(:discussion_note_on_issue, :system, noteable: issuable, project: project) } - expect(json_response.count).to eq(1) + it_behaves_like 'issuable notes filter' end - it 'does not result in N+1 queries' do - # Instantiate the controller variables to ensure QueryRecorder has an accurate base count - get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } + context 'with cross-reference system note', :request_store do + let_it_be(:new_issue) { create(:issue) } - RequestStore.clear! + let(:cross_reference) { "mentioned in #{new_issue.to_reference(issue.project)}" } - control = ActiveRecord::QueryRecorder.new do - get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } + before do + create(:discussion_note_on_issue, :system, noteable: issue, project: issue.project, note: cross_reference) end - RequestStore.clear! + it 'filters notes that the user should not see' do + get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } - create_list(:discussion_note_on_issue, 2, :system, noteable: issue, project: issue.project, note: cross_reference) + expect(json_response.count).to eq(1) + end - expect do + it 'does not result in N+1 queries' do + # Instantiate the controller variables to ensure QueryRecorder has an accurate base count get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } - end.not_to exceed_query_limit(control) - end - context 'when reference is invalid' do - let(:cross_reference) { "mentioned in some/invalid/project#123" } + RequestStore.clear! - it 'does not include the system note' do - get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } + control = ActiveRecord::QueryRecorder.new do + get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } + end - expect(json_response.count).to eq(1) - end - end - end + RequestStore.clear! - context 'private project' do - let!(:branch_note) { create(:discussion_note_on_issue, :system, noteable: issue, project: project) } - let!(:commit_note) { create(:discussion_note_on_issue, :system, noteable: issue, project: project) } - let!(:branch_note_meta) { create(:system_note_metadata, note: branch_note, action: "branch") } - let!(:commit_note_meta) { create(:system_note_metadata, note: commit_note, action: "commit") } + create_list(:discussion_note_on_issue, 2, :system, noteable: issue, project: issue.project, note: cross_reference) - context 'user is allowed access' do - before do - project.add_member(user, :maintainer) + expect do + get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } + end.not_to exceed_query_limit(control) end - it 'displays all available notes' do - get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } + context 'when reference is invalid' do + let(:cross_reference) { "mentioned in some/invalid/project#123" } + + it 'does not include the system note' do + get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } - expect(json_response.length).to eq(3) + expect(json_response.count).to eq(1) + end end end - context 'user is a guest' do - let(:json_response_note_ids) do - json_response - .flat_map { |discussion| discussion["notes"] } - .map { |note| note["id"].to_i } - end + context 'private project' do + let!(:branch_note) { create(:discussion_note_on_issue, :system, noteable: issue, project: project) } + let!(:commit_note) { create(:discussion_note_on_issue, :system, noteable: issue, project: project) } + let!(:branch_note_meta) { create(:system_note_metadata, note: branch_note, action: "branch") } + let!(:commit_note_meta) { create(:system_note_metadata, note: commit_note, action: "commit") } - before do - project.add_guest(user) + context 'user is allowed access' do + before do + project.add_member(user, :maintainer) + end + + it 'displays all available notes' do + get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } + + expect(json_response.length).to eq(3) + end end - it 'does not display notes w/type listed in TYPES_RESTRICTED_BY_ACCESS_LEVEL' do - get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } + context 'user is a guest' do + let(:json_response_note_ids) do + json_response + .flat_map { |discussion| discussion["notes"] } + .map { |note| note["id"].to_i } + end - expect(json_response.length).to eq(2) - expect(json_response_note_ids).not_to include(branch_note.id) + before do + project.add_guest(user) + end + + it 'does not display notes w/type listed in TYPES_RESTRICTED_BY_ACCESS_LEVEL' do + get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } + + expect(json_response.length).to eq(2) + expect(json_response_note_ids).not_to include(branch_note.id) + end end end end diff --git a/spec/controllers/sent_notifications_controller_spec.rb b/spec/controllers/sent_notifications_controller_spec.rb index be9daf2c08914dcc223e86d05921705d0c31bac1..3910186d6e37cb2f232f9fd5e357698de9f995a6 100644 --- a/spec/controllers/sent_notifications_controller_spec.rb +++ b/spec/controllers/sent_notifications_controller_spec.rb @@ -300,9 +300,26 @@ def post_unsubscribe it_behaves_like 'unsubscribes a user' - it 'redirects to the issue page' do - force_unsubscribe - expect(response).to redirect_to(project_issue_path(project, issue)) + context 'when work_item_planning_view: true' do + before do + stub_feature_flags(work_item_planning_view: true) + end + + it 'redirects to the issue page' do + force_unsubscribe + expect(response).to redirect_to(project_work_item_path(project, issue)) + end + end + + context 'when work_item_planning_view: false' do + before do + stub_feature_flags(work_item_planning_view: false) + end + + it 'redirects to the issue page' do + force_unsubscribe + expect(response).to redirect_to(project_issue_path(project, issue)) + end end end @@ -377,15 +394,33 @@ def post_unsubscribe private_project.add_developer(user) end - before do - unsubscribe + context 'when work_item_planning_view: true' do + before do + stub_feature_flags(work_item_planning_view: true) + end + + it 'unsubscribes user and redirects to issue path' do + unsubscribe + + expect(response).to redirect_to(project_work_item_path(private_project, issue)) + end end - it 'unsubscribes user and redirects to issue path' do - expect(response).to redirect_to(project_issue_path(private_project, issue)) + context 'when work_item_planning_view: false' do + before do + stub_feature_flags(work_item_planning_view: false) + end + + it 'unsubscribes user and redirects to issue path' do + unsubscribe + + expect(response).to redirect_to(project_issue_path(private_project, issue)) + end end it 'does not delete the issue email participant for non-service-desk issue' do + unsubscribe + expect { unsubscribe }.not_to change { issue.issue_email_participants.count } end end @@ -429,9 +464,26 @@ def post_unsubscribe it_behaves_like 'unsubscribes a user' - it 'redirects to the issue page' do - post_unsubscribe - expect(response).to redirect_to(project_issue_path(project, issue)) + context 'when work_item_planning_view: true' do + before do + stub_feature_flags(work_item_planning_view: true) + end + + it 'redirects to the issue page' do + post_unsubscribe + expect(response).to redirect_to(project_work_item_path(project, issue)) + end + end + + context 'when work_item_planning_view: false' do + before do + stub_feature_flags(work_item_planning_view: false) + end + + it 'redirects to the issue page' do + post_unsubscribe + expect(response).to redirect_to(project_issue_path(project, issue)) + end end end end diff --git a/spec/factories/issues.rb b/spec/factories/issues.rb index c7bf9d1b028e1c1268f1e4021d1831b2a372737b..961b96c07b99d252c0d0e083e81184a73894b98c 100644 --- a/spec/factories/issues.rb +++ b/spec/factories/issues.rb @@ -127,6 +127,10 @@ association :work_item_type, :ticket end + trait :service_desk do + association :author, :support_bot + end + factory :incident do association :work_item_type, :incident diff --git a/spec/lib/gitlab/url_builder_spec.rb b/spec/lib/gitlab/url_builder_spec.rb index b6cd768bd38a63db898714d8e46f27a2d3d63958..3535a0f59219c3440e6a9fc8231264e15a481d3e 100644 --- a/spec/lib/gitlab/url_builder_spec.rb +++ b/spec/lib/gitlab/url_builder_spec.rb @@ -16,16 +16,54 @@ describe '.build' do using RSpec::Parameterized::TableSyntax + context 'when work_item_planning_view: false', feature_category: :team_planning do + before do + stub_feature_flags(work_item_planning_view: false) + end + + where(:factory, :path_generator) do + :issue | ->(issue) { "/#{issue.project.full_path}/-/issues/#{issue.iid}" } + [:issue, :task] | ->(issue) { "/#{issue.project.full_path}/-/work_items/#{issue.iid}" } + [:issue, :group_level] | ->(issue) { "/groups/#{issue.namespace.full_path}/-/work_items/#{issue.iid}" } + [:work_item, :issue] | ->(work_item) { "/#{work_item.project.full_path}/-/issues/#{work_item.iid}" } + [:work_item, :incident] | ->(work_item) { "/#{work_item.project.full_path}/-/issues/#{work_item.iid}" } + [:work_item, :task] | ->(work_item) { "/#{work_item.project.full_path}/-/work_items/#{work_item.iid}" } + [:work_item, :group_level] | ->(work_item) { "/groups/#{work_item.namespace.full_path}/-/work_items/#{work_item.iid}" } + :note_on_issue | ->(note) { "/#{note.project.full_path}/-/issues/#{note.noteable.iid}#note_#{note.id}" } + :discussion_note_on_issue | ->(note) { "/#{note.project.full_path}/-/issues/#{note.noteable.iid}#note_#{note.id}" } + end + + with_them do + let(:object) { build_stubbed(*Array(factory)) } + let(:path) { path_generator.call(object) } + + it 'returns the full URL' do + expect(subject.build(object)).to eq("#{Gitlab.config.gitlab.url}#{path}") + end + + it 'returns only the path if only_path is given' do + expect(subject.build(object, only_path: true)).to eq(path) + end + end + end + where(:factory, :path_generator) do :project | ->(project) { "/#{project.full_path}" } :board | ->(board) { "/#{board.project.full_path}/-/boards/#{board.id}" } :group_board | ->(board) { "/groups/#{board.group.full_path}/-/boards/#{board.id}" } :commit | ->(commit) { "/#{commit.project.full_path}/-/commit/#{commit.id}" } - :issue | ->(issue) { "/#{issue.project.full_path}/-/issues/#{issue.iid}" } - [:issue, :task] | ->(issue) { "/#{issue.project.full_path}/-/work_items/#{issue.iid}" } - [:work_item, :task] | ->(work_item) { "/#{work_item.project.full_path}/-/work_items/#{work_item.iid}" } - [:work_item, :issue] | ->(work_item) { "/#{work_item.project.full_path}/-/issues/#{work_item.iid}" } - [:work_item, :incident] | ->(work_item) { "/#{work_item.project.full_path}/-/issues/#{work_item.iid}" } + + # WorkItems/Issues + :issue | ->(work_item) { "/#{work_item.project.full_path}/-/work_items/#{work_item.iid}" } + [:work_item, :issue] | ->(work_item) { "/#{work_item.project.full_path}/-/work_items/#{work_item.iid}" } + [:work_item, :incident] | ->(work_item) { "/#{work_item.project.full_path}/-/issues/#{work_item.iid}" } + [:work_item, :task] | ->(work_item) { "/#{work_item.project.full_path}/-/work_items/#{work_item.iid}" } + [:issue, :task] | ->(work_item) { "/#{work_item.project.full_path}/-/work_items/#{work_item.iid}" } + [:issue, :group_level] | ->(work_item) { "/groups/#{work_item.namespace.full_path}/-/work_items/#{work_item.iid}" } + [:work_item, :group_level] | ->(work_item) { "/groups/#{work_item.namespace.full_path}/-/work_items/#{work_item.iid}" } + :note_on_issue | ->(note) { "/#{note.project.full_path}/-/work_items/#{note.noteable.iid}#note_#{note.id}" } + :discussion_note_on_issue | ->(note) { "/#{note.project.full_path}/-/work_items/#{note.noteable.iid}#note_#{note.id}" } + :merge_request | ->(merge_request) { "/#{merge_request.project.full_path}/-/merge_requests/#{merge_request.iid}" } :project_milestone | ->(milestone) { "/#{milestone.project.full_path}/-/milestones/#{milestone.iid}" } :project_snippet | ->(snippet) { "/#{snippet.project.full_path}/-/snippets/#{snippet.id}" } @@ -36,9 +74,6 @@ :ci_pipeline | ->(pipeline) { "/#{pipeline.project.full_path}/-/pipelines/#{pipeline.id}" } :design | ->(design) { "/#{design.project.full_path}/-/design_management/designs/#{design.id}/raw_image" } - [:issue, :group_level] | ->(issue) { "/groups/#{issue.namespace.full_path}/-/work_items/#{issue.iid}" } - [:work_item, :group_level] | ->(work_item) { "/groups/#{work_item.namespace.full_path}/-/work_items/#{work_item.iid}" } - :group | ->(group) { "/groups/#{group.full_path}" } :group_milestone | ->(milestone) { "/groups/#{milestone.group.full_path}/-/milestones/#{milestone.iid}" } @@ -51,9 +86,6 @@ :discussion_note_on_commit | ->(note) { "/#{note.project.full_path}/-/commit/#{note.commit_id}#note_#{note.id}" } :legacy_diff_note_on_commit | ->(note) { "/#{note.project.full_path}/-/commit/#{note.commit_id}#note_#{note.id}" } - :note_on_issue | ->(note) { "/#{note.project.full_path}/-/issues/#{note.noteable.iid}#note_#{note.id}" } - :discussion_note_on_issue | ->(note) { "/#{note.project.full_path}/-/issues/#{note.noteable.iid}#note_#{note.id}" } - :note_on_merge_request | ->(note) { "/#{note.project.full_path}/-/merge_requests/#{note.noteable.iid}#note_#{note.id}" } :diff_note_on_merge_request | ->(note) { "/#{note.project.full_path}/-/merge_requests/#{note.noteable.iid}#note_#{note.id}" } :discussion_note_on_merge_request | ->(note) { "/#{note.project.full_path}/-/merge_requests/#{note.noteable.iid}#note_#{note.id}" } diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 594f91306b1420abb4aa1e6d0b4a51932c922aef..a19982d8d9a0c0d4959a007939ef53fc93937b77 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -667,7 +667,7 @@ def invite_to_project(project, inviter:, user: nil) allow(Note).to receive(:find).with(note.id).and_return(note) end - shared_examples 'a discussion note email' do |model| + shared_examples 'a discussion note email' do |model, issuable_url: nil| let(:noteable_url_args) { {} } it_behaves_like 'it should have Gmail Actions links' @@ -689,7 +689,7 @@ def invite_to_project(project, inviter:, user: nil) end it 'contains an introduction' do - issuable_url = "project_#{note.noteable_type.underscore}_url" + issuable_url ||= "project_#{note.noteable_type.underscore}_url" issuable_url = "project_wiki_url" if note.for_wiki_page? anchor = "note_#{note.id}" @@ -796,7 +796,6 @@ def create_note subject { described_class.note_issue_email(recipient.id, note.id) } - it_behaves_like 'a discussion note email', :discussion_note_on_issue it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do let(:model) { issue } end @@ -815,8 +814,32 @@ def create_note is_expected.to have_referable_subject(issue, reply: true) end - it 'contains a link to the issue note' do - is_expected.to have_body_text note_on_issue_path + context 'when work_item_planning_view: false' do + before do + stub_feature_flags(work_item_planning_view: false) + end + + it_behaves_like 'a discussion note email', :discussion_note_on_issue + + it 'contains a link to the issue note' do + is_expected.to have_body_text note_on_issue_path + end + end + + context 'when work_item_planning_view: true' do + let(:note_on_issue_path) { project_work_item_path(project, issue, anchor: "note_#{note.id}") } + + before do + stub_feature_flags(work_item_planning_view: true) + end + + it_behaves_like 'a discussion note email', + :discussion_note_on_issue, + issuable_url: "project_work_item_url" + + it 'contains a link to the issue note' do + is_expected.to have_body_text note_on_issue_path + end end end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 1c13ce5a09cbf16debb944145585658d756ec0b4..9c5f53e9b3cf12693768bd38018c2c0f120ef210 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -2156,34 +2156,45 @@ describe '#show_as_work_item?' do subject(:issue_as_work_item) { issue.show_as_work_item? } - context 'with a service desk issue' do - let(:issue) { build_stubbed(:issue, project: reusable_project, author: support_bot) } - - it { is_expected.to be false } + where(:factory, :work_item_planning_view, :result) do + :issue | false | false + [:issue, :task] | false | true + [:issue, :group_level] | false | true + [:issue, :incident] | false | false + [:issue, :service_desk] | false | false + :issue | true | true + [:issue, :task] | true | true + [:issue, :group_level] | true | true + [:issue, :incident] | true | false + [:issue, :service_desk] | true | false end - context 'with an incident' do - let(:issue) { build_stubbed(:incident, project: reusable_project) } - - it { is_expected.to be false } - end + with_them do + let(:issue) { build_stubbed(*Array(factory)) } - context 'when work_item_type is not set' do - let(:issue) { build_stubbed(:issue, work_item_type: nil) } + before do + stub_feature_flags(work_item_planning_view: work_item_planning_view) + end - it { is_expected.to be true } + it { is_expected.to be result } end + end - context 'with a regular issue' do - let(:issue) { build_stubbed(:issue, project: reusable_project) } + describe '#require_legacy_views?' do + subject(:require_legacy_views) { issue.require_legacy_views? } - it { is_expected.to be true } + where(:factory, :result) do + :issue | false + [:issue, :task] | false + [:issue, :group_level] | false + [:issue, :incident] | true + [:issue, :service_desk] | true end - context 'with a task' do - let(:issue) { build_stubbed(:issue, :task, project: reusable_project) } + with_them do + let(:issue) { build_stubbed(*Array(factory)) } - it { is_expected.to be true } + it { is_expected.to be result } end end diff --git a/spec/presenters/merge_request_presenter_spec.rb b/spec/presenters/merge_request_presenter_spec.rb index d4a885e7cad3afab75eadf709d283e3cd7932455..85f36accb280dd61da0cce97d327aeb9e8476a22 100644 --- a/spec/presenters/merge_request_presenter_spec.rb +++ b/spec/presenters/merge_request_presenter_spec.rb @@ -157,12 +157,32 @@ describe '#closing_issues_links' do subject { described_class.new(resource, current_user: user).closing_issues_links } - it 'presents closing issues links' do - is_expected.to match("#{project.full_path}/-/issues/#{issue_a.iid}") + context 'when work_item_planning_view: false' do + before do + stub_feature_flags(work_item_planning_view: false) + end + + it 'presents closing issues links' do + is_expected.to match("#{project.full_path}/-/issues/#{issue_a.iid}") + end + + it 'does not present related issues links' do + is_expected.not_to match("#{project.full_path}/-/issues/#{issue_b.iid}") + end end - it 'does not present related issues links' do - is_expected.not_to match("#{project.full_path}/-/issues/#{issue_b.iid}") + context 'when work_item_planning_view: true' do + before do + stub_feature_flags(work_item_planning_view: true) + end + + it 'presents closing issues links' do + is_expected.to match("#{project.full_path}/-/work_items/#{issue_a.iid}") + end + + it 'does not present related issues links' do + is_expected.not_to match("#{project.full_path}/-/work_items/#{issue_b.iid}") + end end it 'appends status when closing issue is already closed' do @@ -177,12 +197,32 @@ .mentioned_issues_links end - it 'presents related issues links' do - is_expected.to match("#{project.full_path}/-/issues/#{issue_b.iid}") + context 'when work_item_planning_view: false' do + before do + stub_feature_flags(work_item_planning_view: false) + end + + it 'presents related issues links' do + is_expected.to match("#{project.full_path}/-/issues/#{issue_b.iid}") + end + + it 'does not present closing issues links' do + is_expected.not_to match("#{project.full_path}/-/issues/#{issue_a.iid}") + end end - it 'does not present closing issues links' do - is_expected.not_to match("#{project.full_path}/-/issues/#{issue_a.iid}") + context 'when work_item_planning_view: true' do + before do + stub_feature_flags(work_item_planning_view: true) + end + + it 'presents related issues links' do + is_expected.to match("#{project.full_path}/-/work_items/#{issue_b.iid}") + end + + it 'does not present closing issues links' do + is_expected.not_to match("#{project.full_path}/-/work_items/#{issue_a.iid}") + end end it 'appends status when mentioned issue is already closed' do diff --git a/spec/serializers/issue_board_entity_spec.rb b/spec/serializers/issue_board_entity_spec.rb index 04c283ddc2c87e2a7f87aec280b88ec08ab5353a..195290c3a1fdf3ada13039270ae99d828d59f5da 100644 --- a/spec/serializers/issue_board_entity_spec.rb +++ b/spec/serializers/issue_board_entity_spec.rb @@ -54,16 +54,40 @@ end describe 'real_path' do - it 'has an issue path' do - expect(subject[:real_path]).to eq(project_issue_path(project, resource.iid)) + context 'when work_item_planning_view: false' do + before do + stub_feature_flags(work_item_planning_view: false) + end + + it 'has an issue path' do + expect(subject[:real_path]).to eq(project_issue_path(project, resource.iid)) + end + + context 'when issue is of type task' do + let(:resource) { create(:issue, :task, project: project) } + + it 'has a work item path with iid' do + expect(subject[:real_path]).to eq(project_work_item_path(project, resource.iid)) + end + end end - context 'when issue is of type task' do - let(:resource) { create(:issue, :task, project: project) } + context 'when work_item_planning_view: true' do + before do + stub_feature_flags(work_item_planning_view: true) + end - it 'has a work item path with iid' do + it 'has an issue path' do expect(subject[:real_path]).to eq(project_work_item_path(project, resource.iid)) end + + context 'when issue is of type task' do + let(:resource) { create(:issue, :task, project: project) } + + it 'has a work item path with iid' do + expect(subject[:real_path]).to eq(project_work_item_path(project, resource.iid)) + end + end end end end diff --git a/spec/serializers/linked_project_issue_entity_spec.rb b/spec/serializers/linked_project_issue_entity_spec.rb index 070ddda2a8b7f2732915ef638699156a87dadde5..47c2d392d98ad35167c6b19ef874cf92042388e6 100644 --- a/spec/serializers/linked_project_issue_entity_spec.rb +++ b/spec/serializers/linked_project_issue_entity_spec.rb @@ -40,17 +40,43 @@ end describe 'path' do - it 'returns an issue path' do - expect(serialized_entity).to include(path: project_issue_path(related_issue.project, related_issue.iid)) + context 'when work_item_planning_view: false' do + before do + stub_feature_flags(work_item_planning_view: false) + end + + it 'returns an issue path' do + expect(serialized_entity[:path]) + .to eq(project_issue_path(related_issue.project, related_issue.iid)) + end + + context 'when related issue is a task' do + let_it_be(:issue_link) { create(:issue_link, target: create(:issue, :task)) } + + it 'returns a work items path using iid' do + expect(serialized_entity[:path]) + .to eq(project_work_item_path(related_issue.project, related_issue.iid)) + end + end end - context 'when related issue is a task' do - let_it_be(:issue_link) { create(:issue_link, target: create(:issue, :task)) } + context 'when work_item_planning_view: true' do + before do + stub_feature_flags(work_item_planning_view: true) + end + + it 'returns an issue path' do + expect(serialized_entity[:path]) + .to eq(project_work_item_path(related_issue.project, related_issue.iid)) + end + + context 'when related issue is a task' do + let_it_be(:issue_link) { create(:issue_link, target: create(:issue, :task)) } - it 'returns a work items path using iid' do - expect(serialized_entity).to include( - path: project_work_item_path(related_issue.project, related_issue.iid) - ) + it 'returns a work items path using iid' do + expect(serialized_entity[:path]) + .to eq(project_work_item_path(related_issue.project, related_issue.iid)) + end end end end diff --git a/spec/support/shared_examples/mailers/notify_shared_examples.rb b/spec/support/shared_examples/mailers/notify_shared_examples.rb index 8b56a550e71a1bbeaaca57709587e0f9f475fecc..c4a3975f9e2e2056bd0060c8cf239f46071bb75b 100644 --- a/spec/support/shared_examples/mailers/notify_shared_examples.rb +++ b/spec/support/shared_examples/mailers/notify_shared_examples.rb @@ -186,11 +186,27 @@ RSpec.shared_examples 'it should show Gmail Actions View Issue link' do |group_level = false| it_behaves_like 'it should have Gmail Actions links' - it 'show the correct view action text' do - if group_level + context 'when work_item_planning_view: false' do + before do + stub_feature_flags(work_item_planning_view: false) + end + + it 'show the correct view action text' do + if group_level + is_expected.to have_body_text('View Work item') + else + is_expected.to have_body_text('View Issue') + end + end + end + + context 'when work_item_planning_view: true' do + before do + stub_feature_flags(work_item_planning_view: true) + end + + it 'show the correct view action text' do is_expected.to have_body_text('View Work item') - else - is_expected.to have_body_text('View Issue') end end end