From 0c8231ba17c796e5a778aa4200a01d9e4682ec94 Mon Sep 17 00:00:00 2001 From: Firdaws Farukh Date: Thu, 16 Feb 2023 19:04:39 +0300 Subject: [PATCH 1/7] Add test case for importing issues csv with milestone --- spec/fixtures/csv_complex.csv | 4 ++-- spec/services/issues/import_csv_service_spec.rb | 5 ++++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/spec/fixtures/csv_complex.csv b/spec/fixtures/csv_complex.csv index 60d8aa5d6f75ec..65b57a6b62a7bd 100644 --- a/spec/fixtures/csv_complex.csv +++ b/spec/fixtures/csv_complex.csv @@ -1,6 +1,6 @@ -title,description,due date +title,description,due date, milestone Issue in 中文,Test description, "Hello","World", "Title with quote""","Description /assign @csv_assignee -/estimate 1h",2022-06-28 +/estimate 1h",2022-06-28,15.10 diff --git a/spec/services/issues/import_csv_service_spec.rb b/spec/services/issues/import_csv_service_spec.rb index 660686cf805c2b..d3d7277e3e379e 100644 --- a/spec/services/issues/import_csv_service_spec.rb +++ b/spec/services/issues/import_csv_service_spec.rb @@ -14,6 +14,8 @@ described_class.new(user, project, uploader) end + let!(:test_milestone) { create(:milestone, project: project, title: '15.10') } + include_examples 'issuable import csv service', 'issue' do let(:issuables) { project.issues } let(:email_method) { :import_issues_csv_email } @@ -36,7 +38,8 @@ description: 'Description', time_estimate: 3600, assignees: include(assignee), - due_date: Date.new(2022, 6, 28) + due_date: Date.new(2022, 6, 28), + milestone_id: test_milestone.id ) ) end -- GitLab From 90838ed06c7c3c3ac6b9ce947e75f610ab255658 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 17 Feb 2023 13:48:54 +0000 Subject: [PATCH 2/7] Apply 1 suggestion(s) to 1 file(s) --- spec/fixtures/csv_complex.csv | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/fixtures/csv_complex.csv b/spec/fixtures/csv_complex.csv index 65b57a6b62a7bd..b42a5e99d88198 100644 --- a/spec/fixtures/csv_complex.csv +++ b/spec/fixtures/csv_complex.csv @@ -1,4 +1,4 @@ -title,description,due date, milestone +title,description,due date,milestone Issue in 中文,Test description, "Hello","World", "Title with quote""","Description -- GitLab From 129829edfd04d540596c6521208d2f1872d769cf Mon Sep 17 00:00:00 2001 From: Firdaws Farukh Date: Tue, 8 Aug 2023 15:14:02 +0100 Subject: [PATCH 3/7] Add milestone field to issues CSV import --- app/services/issuable/import_csv/base_service.rb | 3 ++- app/services/issues/build_service.rb | 6 ++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/app/services/issuable/import_csv/base_service.rb b/app/services/issuable/import_csv/base_service.rb index 95338374ca6b0a..8fb6fbab0f236e 100644 --- a/app/services/issuable/import_csv/base_service.rb +++ b/app/services/issuable/import_csv/base_service.rb @@ -12,7 +12,8 @@ def attributes_for(row) { title: row[:title], description: row[:description], - due_date: row[:due_date] + due_date: row[:due_date], + milestone: row[:milestone] } end diff --git a/app/services/issues/build_service.rb b/app/services/issues/build_service.rb index 63cad5939368b3..923516d29c2cb1 100644 --- a/app/services/issues/build_service.rb +++ b/app/services/issues/build_service.rb @@ -18,6 +18,7 @@ def execute(initialize_callbacks: true) @issue = model_klass.new(issue_params.merge(container_param)).tap do |issue| set_work_item_type(issue) + set_milestone(issue) initialize_callbacks!(issue) if initialize_callbacks end end @@ -96,6 +97,11 @@ def set_work_item_type(issue) end end + def set_milestone(issue) + milestone = Milestone.find_by(title: params[:milestone].strip) if params[:milestone].present? # rubocop: disable CodeReuse/ActiveRecord + issue.milestone_id = milestone.id if milestone + end + def model_klass ::Issue end -- GitLab From b657cde146347c58afb20062fdcfe6cf8b799300 Mon Sep 17 00:00:00 2001 From: Firdaws Farukh Date: Tue, 19 Sep 2023 16:03:44 +0300 Subject: [PATCH 4/7] Add test for milestone field in issues CSV import spec --- spec/fixtures/csv_gitlab_export.csv | 2 +- .../issuable_import_csv_service_shared_examples.rb | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/spec/fixtures/csv_gitlab_export.csv b/spec/fixtures/csv_gitlab_export.csv index 65422509eef4a4..a39260f9dfffb0 100644 --- a/spec/fixtures/csv_gitlab_export.csv +++ b/spec/fixtures/csv_gitlab_export.csv @@ -2,4 +2,4 @@ Issue ID,URL,Title,State,Description,Author,Author Username,Assignee,Assignee Us 1,http://localhost:3000/jashkenas/underscore/issues/1,Title,Open,,Elva Jerde,jamel,Tierra Effertz,aurora_hahn,No,No,,2020-01-17 10:36:26,2020-02-19 10:36:26,,v1.0,,"Brene,Cutlass,Escort,GVM",0,0,, 3,http://localhost:3000/jashkenas/underscore/issues/3,Nihil impedit neque quos totam ut aut enim cupiditate doloribus molestiae.,Open,Omnis aliquid sint laudantium quam.,Marybeth Goodwin,rocio.blanda,Annemarie Von,reynalda_howe,No,No,,2020-01-23 10:36:26,2020-02-19 10:36:27,,v1.0,,"Brene,Cutlass,Escort,GVM",0,0,, 34,http://localhost:3000/jashkenas/underscore/issues/34,Dismiss Cipher with no integrity,Open,,Marybeth Goodwin,rocio.blanda,"","",No,No,,2020-02-19 10:38:49,2020-02-19 10:38:49,,,,,0,0,, -35,http://localhost:3000/jashkenas/underscore/issues/35,Test Title,Open,Test Description,Marybeth Goodwin,rocio.blanda,"","",No,No,,2020-02-19 10:38:49,2020-02-19 10:38:49,,,,,0,0,, +35,http://localhost:3000/jashkenas/underscore/issues/35,Test Title,Open,Test Description,Marybeth Goodwin,rocio.blanda,"","",No,No,,2020-02-19 10:38:49,2020-02-19 10:38:49,,v1.0,,,0,0,, diff --git a/spec/support/shared_examples/services/issuable/issuable_import_csv_service_shared_examples.rb b/spec/support/shared_examples/services/issuable/issuable_import_csv_service_shared_examples.rb index 8a3ab07bbfe803..893d451b710ea3 100644 --- a/spec/support/shared_examples/services/issuable/issuable_import_csv_service_shared_examples.rb +++ b/spec/support/shared_examples/services/issuable/issuable_import_csv_service_shared_examples.rb @@ -41,7 +41,13 @@ it 'correctly sets the issuable attributes' do expect { subject }.to change { issuables.count }.by 4 - expect(issuables.reload).to include(have_attributes({ title: 'Test Title', description: 'Test Description' })) + expect(issuables.reload).to include(have_attributes({ title: 'Test Title', description: 'Test Description', + milestone_id: test_milestone.id })) + expect(issuables.reload).to include(have_attributes({ title: 'Title', milestone_id: test_milestone.id })) + expect(issuables.reload).to include(have_attributes( + { title: 'Nihil impedit neque quos totam ut aut enim cupiditate doloribus molestiae.', + description: 'Omnis aliquid sint laudantium quam.', + milestone_id: test_milestone.id })) end it_behaves_like 'importer with email notification' -- GitLab From 3045a6d6b4accad61c82f70601e7ada878316506 Mon Sep 17 00:00:00 2001 From: Firdaws Farukh Date: Tue, 19 Sep 2023 20:34:05 +0300 Subject: [PATCH 5/7] Fetch and pass milestone id when importing issues csv --- app/services/issuable/import_csv/base_service.rb | 15 ++++++++++++++- app/services/issues/build_service.rb | 6 ------ .../services/ee/issues/import_csv_service_spec.rb | 2 ++ 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/app/services/issuable/import_csv/base_service.rb b/app/services/issuable/import_csv/base_service.rb index 8fb6fbab0f236e..b8745a1109b4c4 100644 --- a/app/services/issuable/import_csv/base_service.rb +++ b/app/services/issuable/import_csv/base_service.rb @@ -13,7 +13,7 @@ def attributes_for(row) title: row[:title], description: row[:description], due_date: row[:due_date], - milestone: row[:milestone] + milestone_id: set_milestone_param(row[:milestone]) } end @@ -42,6 +42,19 @@ def preprocess_milestones! # collate errors here and throw errors results[:preprocess_errors][:milestone_errors] = result.payload end + + def set_milestone_param(title) + return unless title + + # Find the milestone_if using title in the project or its group and group ancestors + finder_params = { + project_ids: [project.id], + title: title.strip + } + finder_params[:group_ids] = project.group.self_and_ancestors.select(:id) if project.group + milestone = MilestonesFinder.new(finder_params).execute.first + milestone.id if milestone + end end end end diff --git a/app/services/issues/build_service.rb b/app/services/issues/build_service.rb index 923516d29c2cb1..63cad5939368b3 100644 --- a/app/services/issues/build_service.rb +++ b/app/services/issues/build_service.rb @@ -18,7 +18,6 @@ def execute(initialize_callbacks: true) @issue = model_klass.new(issue_params.merge(container_param)).tap do |issue| set_work_item_type(issue) - set_milestone(issue) initialize_callbacks!(issue) if initialize_callbacks end end @@ -97,11 +96,6 @@ def set_work_item_type(issue) end end - def set_milestone(issue) - milestone = Milestone.find_by(title: params[:milestone].strip) if params[:milestone].present? # rubocop: disable CodeReuse/ActiveRecord - issue.milestone_id = milestone.id if milestone - end - def model_klass ::Issue end diff --git a/ee/spec/services/ee/issues/import_csv_service_spec.rb b/ee/spec/services/ee/issues/import_csv_service_spec.rb index 1f08aa919615d6..ce28126b835645 100644 --- a/ee/spec/services/ee/issues/import_csv_service_spec.rb +++ b/ee/spec/services/ee/issues/import_csv_service_spec.rb @@ -14,6 +14,8 @@ described_class.new(user, project, uploader) end + let!(:test_milestone) { create(:milestone, project: project, title: '15.10') } + describe '#execute' do subject { service.execute } -- GitLab From 3c0afa1eeb79a5e650e29b6f54378fe0e514b472 Mon Sep 17 00:00:00 2001 From: Firdaws Farukh Date: Thu, 21 Sep 2023 17:49:29 +0300 Subject: [PATCH 6/7] Exclude test for milestone in requirements issue type --- ...ssuable_import_csv_service_shared_examples.rb | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/spec/support/shared_examples/services/issuable/issuable_import_csv_service_shared_examples.rb b/spec/support/shared_examples/services/issuable/issuable_import_csv_service_shared_examples.rb index 893d451b710ea3..76bafcd805f745 100644 --- a/spec/support/shared_examples/services/issuable/issuable_import_csv_service_shared_examples.rb +++ b/spec/support/shared_examples/services/issuable/issuable_import_csv_service_shared_examples.rb @@ -41,13 +41,17 @@ it 'correctly sets the issuable attributes' do expect { subject }.to change { issuables.count }.by 4 - expect(issuables.reload).to include(have_attributes({ title: 'Test Title', description: 'Test Description', - milestone_id: test_milestone.id })) - expect(issuables.reload).to include(have_attributes({ title: 'Title', milestone_id: test_milestone.id })) - expect(issuables.reload).to include(have_attributes( - { title: 'Nihil impedit neque quos totam ut aut enim cupiditate doloribus molestiae.', - description: 'Omnis aliquid sint laudantium quam.', + if issuable_type == 'issue' + expect(issuables.reload).to include(have_attributes({ title: 'Test Title', description: 'Test Description', milestone_id: test_milestone.id })) + expect(issuables.reload).to include(have_attributes({ title: 'Title', milestone_id: test_milestone.id })) + expect(issuables.reload).to include(have_attributes( + { title: 'Nihil impedit neque quos totam ut aut enim cupiditate doloribus molestiae.', + description: 'Omnis aliquid sint laudantium quam.', + milestone_id: test_milestone.id })) + else + expect(issuables.reload).to include(have_attributes({ title: 'Test Title', description: 'Test Description' })) + end end it_behaves_like 'importer with email notification' -- GitLab From 52abbaa5ab2630e459c067d7d7c2572b5379114e Mon Sep 17 00:00:00 2001 From: Firdaws Farukh Date: Fri, 29 Sep 2023 18:05:01 +0300 Subject: [PATCH 7/7] Pass available milestones from preprocessor instead of finding again --- .../preprocess_milestones_service.rb | 7 ++++--- .../issuable/import_csv/base_service.rb | 16 +++++----------- ...uable_import_csv_service_shared_examples.rb | 18 ++++++++++-------- 3 files changed, 19 insertions(+), 22 deletions(-) diff --git a/app/services/import_csv/preprocess_milestones_service.rb b/app/services/import_csv/preprocess_milestones_service.rb index 97fb381c58e07c..295c76184d3e7f 100644 --- a/app/services/import_csv/preprocess_milestones_service.rb +++ b/app/services/import_csv/preprocess_milestones_service.rb @@ -14,8 +14,9 @@ def initialize(user, project, provided_titles) attr_reader :user, :project, :provided_titles, :results, :milestone_errors def execute - available_milestones = find_milestones_by_titles - return ServiceResponse.success if provided_titles.sort == available_milestones.sort + result = find_milestones_by_titles + available_milestones = result.map(&:title).uniq.sort + return ServiceResponse.success(payload: result) if provided_titles.sort == available_milestones milestone_errors[:missing][:header] = 'Milestone' milestone_errors[:missing][:titles] = provided_titles.difference(available_milestones) || [] @@ -29,7 +30,7 @@ def find_milestones_by_titles title: provided_titles } finder_params[:group_ids] = project.group.self_and_ancestors.select(:id) if project.group - MilestonesFinder.new(finder_params).execute.map(&:title).uniq + MilestonesFinder.new(finder_params).execute end end end diff --git a/app/services/issuable/import_csv/base_service.rb b/app/services/issuable/import_csv/base_service.rb index b8745a1109b4c4..c26fb016f79ecb 100644 --- a/app/services/issuable/import_csv/base_service.rb +++ b/app/services/issuable/import_csv/base_service.rb @@ -13,7 +13,7 @@ def attributes_for(row) title: row[:title], description: row[:description], due_date: row[:due_date], - milestone_id: set_milestone_param(row[:milestone]) + milestone_id: find_milestone_by_title(row[:milestone]) } end @@ -35,25 +35,19 @@ def preprocess_milestones! # Pre-Process Milestone if header is present return unless csv_data.lines.first.downcase.include?('milestone') - provided_titles = with_csv_lines.filter_map { |row| row[:milestone]&.strip&.downcase }.uniq + provided_titles = with_csv_lines.filter_map { |row| row[:milestone]&.strip }.uniq result = ::ImportCsv::PreprocessMilestonesService.new(user, project, provided_titles).execute + @available_milestones = result.payload return if result.success? # collate errors here and throw errors results[:preprocess_errors][:milestone_errors] = result.payload end - def set_milestone_param(title) + def find_milestone_by_title(title) return unless title - # Find the milestone_if using title in the project or its group and group ancestors - finder_params = { - project_ids: [project.id], - title: title.strip - } - finder_params[:group_ids] = project.group.self_and_ancestors.select(:id) if project.group - milestone = MilestonesFinder.new(finder_params).execute.first - milestone.id if milestone + @available_milestones.find { |milestone| milestone.title == title.to_s.strip } if @available_milestones end end end diff --git a/spec/support/shared_examples/services/issuable/issuable_import_csv_service_shared_examples.rb b/spec/support/shared_examples/services/issuable/issuable_import_csv_service_shared_examples.rb index 76bafcd805f745..aa31bd2b60410f 100644 --- a/spec/support/shared_examples/services/issuable/issuable_import_csv_service_shared_examples.rb +++ b/spec/support/shared_examples/services/issuable/issuable_import_csv_service_shared_examples.rb @@ -30,7 +30,7 @@ context 'with a file generated by Gitlab CSV export' do let(:file) { fixture_file_upload('spec/fixtures/csv_gitlab_export.csv') } - let!(:test_milestone) { create(:milestone, project: project, title: 'v1.0') } + let_it_be(:test_milestone) { create(:milestone, project: project, title: 'v1.0') } it 'imports the CSV without errors' do expect(subject[:success]).to eq(4) @@ -42,13 +42,15 @@ expect { subject }.to change { issuables.count }.by 4 if issuable_type == 'issue' - expect(issuables.reload).to include(have_attributes({ title: 'Test Title', description: 'Test Description', - milestone_id: test_milestone.id })) - expect(issuables.reload).to include(have_attributes({ title: 'Title', milestone_id: test_milestone.id })) - expect(issuables.reload).to include(have_attributes( - { title: 'Nihil impedit neque quos totam ut aut enim cupiditate doloribus molestiae.', - description: 'Omnis aliquid sint laudantium quam.', - milestone_id: test_milestone.id })) + expect(issuables.reload).to include( + have_attributes({ title: 'Test Title', description: 'Test Description', milestone_id: test_milestone.id }), + have_attributes({ title: 'Title', milestone_id: test_milestone.id }), + have_attributes( + { title: 'Nihil impedit neque quos totam ut aut enim cupiditate doloribus molestiae.', + description: 'Omnis aliquid sint laudantium quam.', + milestone_id: test_milestone.id }) + ) + else expect(issuables.reload).to include(have_attributes({ title: 'Test Title', description: 'Test Description' })) end -- GitLab