diff --git a/.gitignore b/.gitignore index 181d655..427d0e0 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,3 @@ .~lock* +*~ +#*# diff --git a/app/controllers/importer_controller.rb b/app/controllers/importer_controller.rb index 8c72278..23e61b3 100644 --- a/app/controllers/importer_controller.rb +++ b/app/controllers/importer_controller.rb @@ -27,7 +27,7 @@ def match # Delete existing iip to ensure there can't be two iips for a user ImportInProgress.delete_all(["user_id = ?",User.current.id]) # save import-in-progress data - iip = ImportInProgress.find_or_create_by_user_id(User.current.id) + iip = ImportInProgress.find_or_create_by(user_id: User.current.id) iip.quote_char = params[:wrapper] iip.col_sep = params[:splitter] iip.encoding = params[:encoding] @@ -71,20 +71,7 @@ def result # used for bookkeeping flash.delete(:error) - @handle_count = 0 - @update_count = 0 - @skip_count = 0 - @failed_count = 0 - @failed_issues = Hash.new - @messages = Array.new - @affect_projects_issues = Hash.new - # This is a cache of previously inserted issues indexed by the value - # the user provided in the unique column - @issue_by_unique_attr = Hash.new - # Cache of user id by login - @user_by_login = Hash.new - # Cache of Version by name - @version_id_by_name = Hash.new + init_globals # Used to optimize some work that has to happen inside the loop unique_attr_checked = false @@ -100,7 +87,6 @@ def result "This import cannot be completed" return end - # which options were turned on? update_issue = params[:update_issue] update_other_project = params[:update_other_project] @@ -220,175 +206,29 @@ def result rescue ActiveRecord::RecordNotFound log_failure(row, "Warning: When adding issue #{@failed_count+1} below," \ " the #{@unfound_class} #{@unfound_key} was not found") - next - end - - # translate unique_attr if it's a custom field -- only on the first issue - if !unique_attr_checked - if unique_field && !ISSUE_ATTRS.include?(unique_attr.to_sym) - issue.available_custom_fields.each do |cf| - if cf.name == unique_attr - unique_attr = "cf_#{cf.id}" - break - end - end - end - unique_attr_checked = true + raise RowFailed end - if update_issue - begin - issue = issue_for_unique_attr(unique_attr,row[unique_field],row) - - # ignore other project's issue or not - if issue.project_id != @project.id && !update_other_project - @skip_count += 1 - next - end - - # ignore closed issue except reopen - if issue.status.is_closed? - if status == nil || status.is_closed? - @skip_count += 1 - next - end - end - - # init journal - note = row[journal_field] || '' - journal = issue.init_journal(author || User.current, - note || '') - - @update_count += 1 + begin - rescue NoIssueForUniqueValue - if ignore_non_exist - @skip_count += 1 - next - else - log_failure(row, - "Warning: Could not update issue #{@failed_count+1} below," \ - " no match for the value #{row[unique_field]} were found") - next - end + unique_attr = translate_unique_attr(issue, unique_field, unique_attr, unique_attr_checked) - rescue MultipleIssuesForUniqueValue - log_failure("Warning: Could not update issue #{@failed_count+1} below," \ - " multiple matches for the value #{row[unique_field]} were found") - next - end - end + issue, journal = handle_issue_update(issue, row, author, status, update_other_project, journal_field, + unique_attr, unique_field, ignore_non_exist, update_issue) - project ||= Project.find_by_id(issue.project_id) + project ||= Project.find_by_id(issue.project_id) - if @affect_projects_issues.has_key?(project.name) - @affect_projects_issues[project.name] += 1 - else - @affect_projects_issues[project.name] = 1 - end + update_project_issues_stat(project) - # required attributes - issue.status_id = status != nil ? status.id : issue.status_id - issue.priority_id = priority != nil ? priority.id : issue.priority_id - issue.subject = fetch("subject", row) || issue.subject - - # optional attributes - issue.description = fetch("description", row) || issue.description - issue.category_id = category != nil ? category.id : issue.category_id - - if fetch("start_date", row).present? - issue.start_date = Date.parse(fetch("start_date", row)) - end - issue.due_date = if row[@attrs_map["due_date"]].blank? - nil - else - Date.parse(row[@attrs_map["due_date"]]) - end - issue.assigned_to_id = assigned_to.id if assigned_to - issue.fixed_version_id = fixed_version_id if fixed_version_id - issue.done_ratio = row[@attrs_map["done_ratio"]] || issue.done_ratio - issue.estimated_hours = row[@attrs_map["estimated_hours"]] || issue.estimated_hours - - # parent issues - begin - parent_value = row[@attrs_map["parent_issue"]] - if parent_value && (parent_value.length > 0) - issue.parent_issue_id = issue_for_unique_attr(unique_attr,parent_value,row).id - end - rescue NoIssueForUniqueValue - if ignore_non_exist - @skip_count += 1 - else - @failed_count += 1 - @failed_issues[@failed_count] = row - @messages << "Warning: When setting the parent for issue #{@failed_count} below,"\ - " no matches for the value #{parent_value} were found" - next - end - rescue MultipleIssuesForUniqueValue - @failed_count += 1 - @failed_issues[@failed_count] = row - @messages << "Warning: When setting the parent for issue #{@failed_count} below," \ - " multiple matches for the value #{parent_value} were found" + assign_issue_attrs(issue, category, fixed_version_id, assigned_to, status, row, priority) + handle_parent_issues(issue, row, ignore_non_exist, unique_attr) + handle_custom_fields(add_versions, issue, project, row) + handle_watchers(issue, row, watchers) + rescue RowFailed next end - # custom fields - custom_failed_count = 0 - issue.custom_field_values = issue.available_custom_fields.inject({}) do |h, cf| - value = row[@attrs_map[cf.name]] - unless value.blank? - begin - value = case cf.field_format - when 'user' - user_id_for_login!(value).to_s - when 'version' - version_id_for_name!(project,value,add_versions).to_s - when 'date' - value.to_date.to_s(:db) - else - value - end - h[cf.id] = value - rescue - if custom_failed_count == 0 - custom_failed_count += 1 - @failed_count += 1 - @failed_issues[@failed_count] = row - end - @messages << "Warning: When trying to set custom field #{cf.name}" \ - " on issue #{@failed_count} below, value #{value} was invalid" - end - end - h - end - next if custom_failed_count > 0 - # watchers - watcher_failed_count = 0 - if watchers - addable_watcher_users = issue.addable_watcher_users - watchers.split(',').each do |watcher| - begin - watcher_user = user_id_for_login!(watcher) - if issue.watcher_users.include?(watcher_user) - next - end - if addable_watcher_users.include?(watcher_user) - issue.add_watcher(watcher_user) - end - rescue ActiveRecord::RecordNotFound - if watcher_failed_count == 0 - @failed_count += 1 - @failed_issues[@failed_count] = row - end - watcher_failed_count += 1 - @messages << "Warning: When trying to add watchers on issue" \ - " #{@failed_count} below, User #{watcher} was not found" - end - end - end - next if watcher_failed_count > 0 begin issue_saved = issue.save @@ -413,7 +253,7 @@ def result if send_emails if update_issue if Setting.notified_events.include?('issue_updated') \ - && (!issue.current_journal.empty?) + && (!issue.current_journal.empty?) Mailer.deliver_issue_edit(issue.current_journal) end @@ -475,6 +315,206 @@ def result ImportInProgress.delete_all(["created < ?",Time.new - 3*24*60*60]) end + def translate_unique_attr(issue, unique_field, unique_attr, unique_attr_checked) + # translate unique_attr if it's a custom field -- only on the first issue + if !unique_attr_checked + if unique_field && !ISSUE_ATTRS.include?(unique_attr.to_sym) + issue.available_custom_fields.each do |cf| + if cf.name == unique_attr + unique_attr = "cf_#{cf.id}" + break + end + end + end + unique_attr_checked = true + end + unique_attr + end + + def handle_issue_update(issue, row, author, status, update_other_project, journal_field, unique_attr, unique_field, ignore_non_exist, update_issue) + if update_issue + begin + issue = issue_for_unique_attr(unique_attr, row[unique_field], row) + + # ignore other project's issue or not + if issue.project_id != @project.id && !update_other_project + @skip_count += 1 + raise RowFailed + end + + # ignore closed issue except reopen + if issue.status.is_closed? + if status == nil || status.is_closed? + @skip_count += 1 + raise RowFailed + end + end + + # init journal + note = row[journal_field] || '' + journal = issue.init_journal(author || User.current, + note || '') + journal.notify = false #disable journal's notification to use custom one down below + @update_count += 1 + + rescue NoIssueForUniqueValue + if ignore_non_exist + @skip_count += 1 + raise RowFailed + else + log_failure(row, + "Warning: Could not update issue #{@failed_count+1} below," \ + " no match for the value #{row[unique_field]} were found") + raise RowFailed + end + + rescue MultipleIssuesForUniqueValue + log_failure(row, + "Warning: Could not update issue #{@failed_count+1} below," \ + " multiple matches for the value #{row[unique_field]} were found") + raise RowFailed + end + end + return issue, journal + end + + def update_project_issues_stat(project) + if @affect_projects_issues.has_key?(project.name) + @affect_projects_issues[project.name] += 1 + else + @affect_projects_issues[project.name] = 1 + end + end + + def assign_issue_attrs(issue, category, fixed_version_id, assigned_to, status, row, priority) + # required attributes + issue.status_id = status != nil ? status.id : issue.status_id + issue.priority_id = priority != nil ? priority.id : issue.priority_id + issue.subject = fetch("subject", row) || issue.subject + + # optional attributes + issue.description = fetch("description", row) || issue.description + issue.category_id = category != nil ? category.id : issue.category_id + + if fetch("start_date", row).present? + issue.start_date = Date.parse(fetch("start_date", row)) + end + issue.due_date = if row[@attrs_map["due_date"]].blank? + nil + else + Date.parse(row[@attrs_map["due_date"]]) + end + issue.assigned_to_id = assigned_to.id if assigned_to + issue.fixed_version_id = fixed_version_id if fixed_version_id + issue.done_ratio = row[@attrs_map["done_ratio"]] || issue.done_ratio + issue.estimated_hours = row[@attrs_map["estimated_hours"]] || issue.estimated_hours + end + + def handle_parent_issues(issue, row, ignore_non_exist, unique_attr) + begin + parent_value = row[@attrs_map["parent_issue"]] + if parent_value && (parent_value.length > 0) + issue.parent_issue_id = issue_for_unique_attr(unique_attr, parent_value, row).id + end + rescue NoIssueForUniqueValue + if ignore_non_exist + @skip_count += 1 + else + @failed_count += 1 + @failed_issues[@failed_count] = row + @messages << "Warning: When setting the parent for issue #{@failed_count} below,"\ + " no matches for the value #{parent_value} were found" + raise RowFailed + end + rescue MultipleIssuesForUniqueValue + @failed_count += 1 + @failed_issues[@failed_count] = row + @messages << "Warning: When setting the parent for issue #{@failed_count} below," \ + " multiple matches for the value #{parent_value} were found" + raise RowFailed + end + end + + def init_globals + @handle_count = 0 + @update_count = 0 + @skip_count = 0 + @failed_count = 0 + @failed_issues = Hash.new + @messages = Array.new + @affect_projects_issues = Hash.new + # This is a cache of previously inserted issues indexed by the value + # the user provided in the unique column + @issue_by_unique_attr = Hash.new + # Cache of user id by login + @user_by_login = Hash.new + # Cache of Version by name + @version_id_by_name = Hash.new + end + + def handle_watchers(issue, row, watchers) + watcher_failed_count = 0 + if watchers + addable_watcher_users = issue.addable_watcher_users + watchers.split(',').each do |watcher| + begin + watcher_user = user_for_login!(watcher) + if issue.watcher_users.include?(watcher_user) + next + end + if addable_watcher_users.include?(watcher_user) + issue.add_watcher(watcher_user) + end + rescue ActiveRecord::RecordNotFound + if watcher_failed_count == 0 + @failed_count += 1 + @failed_issues[@failed_count] = row + end + watcher_failed_count += 1 + @messages << "Warning: When trying to add watchers on issue" \ + " #{@failed_count} below, User #{watcher} was not found" + end + end + end + raise RowFailed if watcher_failed_count > 0 + end + + def handle_custom_fields(add_versions, issue, project, row) + custom_failed_count = 0 + issue.custom_field_values = issue.available_custom_fields.inject({}) do |h, cf| + value = row[@attrs_map[cf.name]] + unless value.blank? + if cf.multiple + h[cf.id] = process_multivalue_custom_field(issue, cf, value) + else + begin + value = case cf.field_format + when 'user' + user_id_for_login!(value).to_s + when 'version' + version_id_for_name!(project, value, add_versions).to_s + when 'date' + value.to_date.to_s(:db) + else + value + end + h[cf.id] = value + rescue + if custom_failed_count == 0 + custom_failed_count += 1 + @failed_count += 1 + @failed_issues[@failed_count] = row + end + @messages << "Warning: When trying to set custom field #{cf.name}" \ + " on issue #{@failed_count} below, value #{value} was invalid" + end + end + end + h + end + raise RowFailed if custom_failed_count > 0 + end + private def fetch(key, row) @@ -603,10 +643,9 @@ def issue_for_unique_attr(unique_attr, attr_value, row_data) "'#{attr_value}' in issue #{@failed_count} has duplicate record" raise MultipleIssuesForUniqueValue, "Unique field #{unique_attr} with" \ " value '#{attr_value}' has duplicate record" + elsif issues.size == 0 || issues[0].nil? + raise NoIssueForUniqueValue, "No issue with #{unique_attr} of '#{attr_value}' found" else - if issues.size == 0 - raise NoIssueForUniqueValue, "No issue with #{unique_attr} of '#{attr_value}' found" - end issues.first end end @@ -629,6 +668,7 @@ def user_for_login!(login) end @user_by_login[login] end + def user_id_for_login!(login) user = user_for_login!(login) user ? user.id : nil @@ -641,7 +681,7 @@ def user_id_for_login!(login) # will create a new version and save it when it doesn't exist yet. def version_id_for_name!(project,name,add_versions) if !@version_id_by_name.has_key?(name) - version = Version.find_by_project_id_and_name(project.id, name) + version = project.shared_versions.find_by_name(name) if !version if name && (name.length > 0) && add_versions version = project.versions.build(:name=>name) @@ -657,4 +697,18 @@ def version_id_for_name!(project,name,add_versions) @version_id_by_name[name] end + def process_multivalue_custom_field(issue, custom_field, csv_val) + csv_val.split(',').map(&:strip).map do |val| + if custom_field.field_format == 'version' + version = version_id_for_name!(project, val, add_versions) + version.id + else + val + end + end + end + + class RowFailed < Exception + end + end diff --git a/app/models/import_in_progress.rb b/app/models/import_in_progress.rb index 864b500..141e6d4 100644 --- a/app/models/import_in_progress.rb +++ b/app/models/import_in_progress.rb @@ -6,6 +6,8 @@ class ImportInProgress < ActiveRecord::Base before_save :encode_csv_data + attr_accessible :user_id + private def encode_csv_data return if self.csv_data.blank? diff --git a/test/functional/importer_controller_test.rb b/test/functional/importer_controller_test.rb new file mode 100644 index 0000000..8d52b1b --- /dev/null +++ b/test/functional/importer_controller_test.rb @@ -0,0 +1,246 @@ +require File.expand_path('../../test_helper', __FILE__) + +class ImporterControllerTest < ActionController::TestCase + def setup + @project = Project.create! :name => 'foo' + @tracker = Tracker.new(:name => 'Defect') + @tracker.default_status = IssueStatus.find_or_create_by!(name: 'New') + @tracker.save! + @project.trackers << @tracker + @project.save! + @role = Role.create! :name => 'ADMIN', :permissions => [:import, :view_issues] + @user = create_user!(@role, @project) + @iip = create_iip_for_multivalues!(@user, @project) + @issue = create_issue!(@project, @user) + create_custom_fields!(@issue) + create_versions!(@project) + User.stubs(:current).returns(@user) + end + + test 'should handle multiple values for versions' do + assert issue_has_none_of_these_multival_versions?(@issue, + ['Admin', '2013-09-25']) + post :result, build_params + assert_response :success + @issue.reload + assert issue_has_all_these_multival_versions?(@issue, ['Admin', '2013-09-25']) + end + + test 'should handle multiple values' do + assert issue_has_none_of_these_multifield_vals?(@issue, ['tag1', 'tag2']) + post :result, build_params + assert_response :success + @issue.reload + assert issue_has_all_these_multifield_vals?(@issue, ['tag1', 'tag2']) + end + + test 'should handle single-value fields' do + assert_equal 'foobar', @issue.subject + post :result, build_params + assert_response :success + @issue.reload + assert_equal 'barfooz', @issue.subject + end + + test 'should create issue if none exists' do + Issue.delete_all + assert_equal 0, Issue.count + post :result, build_params(:update_issue => nil) + assert_response :success + assert_equal 1, Issue.count + issue = Issue.first + assert_equal 'barfooz', issue.subject + end + + test 'should send email when Send email notifications checkbox is checked' do + assert_equal 'foobar', @issue.subject + Mailer.expects(:deliver_issue_edit) + + post :result, build_params(:send_emails => 'true') + assert_response :success + @issue.reload + assert_equal 'barfooz', @issue.subject + end + + test 'should NOT send email when Send email notifications checkbox is unchecked' do + assert_equal 'foobar', @issue.subject + Mailer.expects(:deliver_issue_edit).never + + post :result, build_params(:send_emails => nil) + assert_response :success + @issue.reload + assert_equal 'barfooz', @issue.subject + end + + test 'should add watchers' do + assert issue_has_none_of_these_watchers?(@issue, [@user, @user.parent]) + post :result, build_params + assert_response :success + @issue.reload + assert issue_has_all_of_these_watchers?(@issue, [@user, @user.parent]) + end + + protected + + def build_params(opts={}) + @iip.reload + opts.reverse_merge( + :import_timestamp => @iip.created.strftime("%Y-%m-%d %H:%M:%S"), + :update_issue => 'true', + :unique_field => '#', + :project_id => @project.id, + :fields_map => { + '#' => 'id', + 'Subject' => 'subject', + 'Tags' => 'Tags', + 'Affected versions' => 'Affected versions', + 'Priority' => 'priority', + 'Tracker' => 'tracker', + 'Status' => 'status', + 'Watchers' => 'watchers' + } + ) + end + + def issue_has_all_these_multival_versions?(issue, version_names) + find_version_ids(version_names).all? do |version_to_find| + versions_for(issue).include?(version_to_find) + end + end + + def issue_has_none_of_these_multival_versions?(issue, version_names) + find_version_ids(version_names).none? do |version_to_find| + versions_for(issue).include?(version_to_find) + end + end + + def issue_has_none_of_these_watchers?(issue, watchers) + watchers.none? do |watcher| + issue.watcher_users.include?(watcher) + end + end + + def issue_has_all_of_these_watchers?(issue, watchers) + watchers.all? do |watcher| + issue.watcher_users.include?(watcher) + end + end + + def find_version_ids(version_names) + version_names.map do |name| + Version.find_by_name!(name).id.to_s + end + end + + def versions_for(issue) + versions_field = CustomField.find_by_name! 'Affected versions' + value_objs = issue.custom_values.where(custom_field_id: versions_field.id) + values = value_objs.map(&:value) + end + + def issue_has_all_these_multifield_vals?(issue, vals_to_find) + vals_to_find.all? do |val_to_find| + multifield_vals_for(issue).include?(val_to_find) + end + end + + def issue_has_none_of_these_multifield_vals?(issue, vals_to_find) + vals_to_find.none? do |val_to_find| + multifield_vals_for(issue).include?(val_to_find) + end + end + + def multifield_vals_for(issue) + multival_field = CustomField.find_by_name! 'Tags' + value_objs = issue.custom_values.where(custom_field_id: multival_field.id) + values = value_objs.map(&:value) + end + + def create_user!(role, project) + user = User.new :admin => true, + :login => 'bob', + :firstname => 'Bob', + :lastname => 'Loblaw', + :mail => 'bob.loblaw@example.com' + user.login = 'bob' + sponsor = User.new :admin => true, + :firstname => 'A', + :lastname => 'H', + :mail => 'a@example.com' + sponsor.login = 'alice' + sponsor.parent = sponsor + + user.parent = sponsor + membership = user.memberships.build(:project => project) + membership.roles << role + membership.principal = user + + membership = sponsor.memberships.build(:project => project) + membership.roles << role + membership.principal = sponsor + sponsor.save! + user.save! + user + end + + def create_iip_for_multivalues!(user, project) + create_iip!('CustomFieldMultiValues', user, project) + end + + def create_iip!(filename, user, project) + iip = ImportInProgress.new + iip.user = user + iip.csv_data = get_csv(filename) + #iip.created = DateTime.new(2001,2,3,4,5,6,'+7') + iip.created = DateTime.now + iip.encoding = 'U' + iip.col_sep = ',' + iip.quote_char = '"' + iip.save! + iip + end + + def create_issue!(project, author) + issue = Issue.new + issue.id = 70385 + issue.project = project + issue.subject = 'foobar' + issue.create_priority!(name: 'Critical') + issue.tracker = project.trackers.first + issue.author = author + issue.status = IssueStatus.find_or_create_by!(name: 'New') + issue.save! + issue + end + + def create_custom_fields!(issue) + versions_field = create_multivalue_field!('Affected versions', + 'version', + issue.project) + multival_field = create_multivalue_field!('Tags', + 'list', + issue.project, + %w(tag1 tag2)) + issue.tracker.custom_fields << versions_field + issue.tracker.custom_fields << multival_field + issue.tracker.save! + end + + def create_multivalue_field!(name, format, project, possible_vals = []) + field = IssueCustomField.new :name => name, :multiple => true + field.field_format = format + field.projects << project + field.possible_values = possible_vals if possible_vals + field.save! + field + end + + def create_versions!(project) + project.versions.create! :name => 'Admin', :status => 'open' + project.versions.create! :name => '2013-09-25', :status => 'open' + end + + def get_csv(filename) + File.read(File.expand_path("../../samples/#{filename}.csv", __FILE__)) + end +end diff --git a/test/samples/CustomFieldMultiValues.csv b/test/samples/CustomFieldMultiValues.csv new file mode 100644 index 0000000..083d6e5 --- /dev/null +++ b/test/samples/CustomFieldMultiValues.csv @@ -0,0 +1,2 @@ +#,Project,Tracker,Parent item,Status,Priority,Subject,Author,Visibility,Updated,Category,Target version,Start date,Due date,Created,Closed,Related items,Priority.,Priority:,Priority*,Found in version,Platform.fv,Cycle.fv,Affected versions,Platform,Cycle,Severity,Occurrence,Responsible,Found In-Market,Ext. Ref,Type,Tags,Private,Watchers +70385,RDQ,Defect,"",New,Critical,barfooz,Bob Loblaw,"",2015-02-20 15:25,"","",2015-01-30,"",2015-01-30 11:08,"","","","",Critical,2013-09-25,"","","Admin, 2013-09-25","","",Critical,Always,Anonymous,"","","","tag1, tag2",Yes,"alice,bob" diff --git a/test/test_helper.rb b/test/test_helper.rb new file mode 100644 index 0000000..6f04415 --- /dev/null +++ b/test/test_helper.rb @@ -0,0 +1 @@ +require File.expand_path('../../../../test/test_helper', __FILE__) diff --git a/test/unit/import_in_progress_test.rb b/test/unit/import_in_progress_test.rb index 613ef6a..343bc00 100644 --- a/test/unit/import_in_progress_test.rb +++ b/test/unit/import_in_progress_test.rb @@ -1,10 +1,4 @@ require File.dirname(__FILE__) + '/../test_helper' class ImportInProgressTest < ActiveSupport::TestCase - fixtures :import_in_progresses - - # Replace this with your real tests. - def test_truth - assert true - end end