From e0e8c14c2aefd58267f7676b0342f04a37b08185 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Fri, 12 Nov 2010 11:34:53 +0000 Subject: [PATCH] Makes MailHandler accept all issue attributes and custom fields that can be set/updated (#4071, #4807, #5622, #6110). git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@4394 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/models/issue.rb | 11 ++- app/models/mail_handler.rb | 89 +++++++++---------- .../mail_handler/ticket_on_given_project.eml | 3 + .../mail_handler/ticket_reply_with_status.eml | 1 + test/unit/mail_handler_test.rb | 5 ++ 5 files changed, 59 insertions(+), 50 deletions(-) diff --git a/app/models/issue.rb b/app/models/issue.rb index d47ab74ad..c7885129e 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -248,19 +248,22 @@ class Issue < ActiveRecord::Base def safe_attributes=(attrs, user=User.current) return unless attrs.is_a?(Hash) - new_statuses_allowed = new_statuses_allowed_to(user) - # User can change issue attributes only if he has :edit permission or if a workflow transition is allowed if new_record? || user.allowed_to?(:edit_issues, project) attrs = attrs.reject {|k,v| !SAFE_ATTRIBUTES.include?(k)} - elsif new_statuses_allowed.any? + elsif new_statuses_allowed_to(user).any? attrs = attrs.reject {|k,v| !SAFE_ATTRIBUTES_ON_TRANSITION.include?(k)} else return end + # Tracker must be set before since new_statuses_allowed_to depends on it. + if t = attrs.delete('tracker_id') + self.tracker_id = t + end + if attrs['status_id'] - unless new_statuses_allowed.collect(&:id).include?(attrs['status_id'].to_i) + unless new_statuses_allowed_to(user).collect(&:id).include?(attrs['status_id'].to_i) attrs.delete('status_id') end end diff --git a/app/models/mail_handler.rb b/app/models/mail_handler.rb index 35e339687..5688711d4 100644 --- a/app/models/mail_handler.rb +++ b/app/models/mail_handler.rb @@ -116,36 +116,20 @@ class MailHandler < ActionMailer::Base # Creates a new issue def receive_issue project = target_project - tracker = (get_keyword(:tracker) && project.trackers.find_by_name(get_keyword(:tracker))) || project.trackers.find(:first) - category = (get_keyword(:category) && project.issue_categories.find_by_name(get_keyword(:category))) - priority = (get_keyword(:priority) && IssuePriority.find_by_name(get_keyword(:priority))) - status = (get_keyword(:status) && IssueStatus.find_by_name(get_keyword(:status))) - assigned_to = (get_keyword(:assigned_to, :override => true) && find_user_from_keyword(get_keyword(:assigned_to, :override => true))) - due_date = get_keyword(:due_date, :override => true) - start_date = get_keyword(:start_date, :override => true) - # check permission unless @@handler_options[:no_permission_check] raise UnauthorizedAction unless user.allowed_to?(:add_issues, project) end - issue = Issue.new(:author => user, :project => project, :tracker => tracker, :category => category, :priority => priority, :due_date => due_date, :start_date => start_date, :assigned_to => assigned_to) - # check workflow - if status && issue.new_statuses_allowed_to(user).include?(status) - issue.status = status - end - issue.subject = email.subject.chomp[0,255] + issue = Issue.new(:author => user, :project => project) + issue.safe_attributes = issue_attributes_from_keywords(issue) + issue.safe_attributes = {'custom_field_values' => custom_field_values_from_keywords(issue)} + issue.subject = email.subject.to_s.chomp[0,255] if issue.subject.blank? issue.subject = '(no subject)' end - # custom fields - issue.custom_field_values = issue.available_custom_fields.inject({}) do |h, c| - if value = get_keyword(c.name, :override => true) - h[c.id] = value - end - h - end issue.description = cleaned_up_text_body + # add To and Cc as watchers before saving so the watchers can reply to Redmine add_watchers(issue) issue.save! @@ -154,41 +138,19 @@ class MailHandler < ActionMailer::Base issue end - def target_project - # TODO: other ways to specify project: - # * parse the email To field - # * specific project (eg. Setting.mail_handler_target_project) - target = Project.find_by_identifier(get_keyword(:project)) - raise MissingInformation.new('Unable to determine target project') if target.nil? - target - end - # Adds a note to an existing issue def receive_issue_reply(issue_id) - status = (get_keyword(:status) && IssueStatus.find_by_name(get_keyword(:status))) - due_date = get_keyword(:due_date, :override => true) - start_date = get_keyword(:start_date, :override => true) - assigned_to = (get_keyword(:assigned_to, :override => true) && find_user_from_keyword(get_keyword(:assigned_to, :override => true))) - issue = Issue.find_by_id(issue_id) return unless issue # check permission unless @@handler_options[:no_permission_check] raise UnauthorizedAction unless user.allowed_to?(:add_issue_notes, issue.project) || user.allowed_to?(:edit_issues, issue.project) - raise UnauthorizedAction unless status.nil? || user.allowed_to?(:edit_issues, issue.project) end - - # add the note + journal = issue.init_journal(user, cleaned_up_text_body) + issue.safe_attributes = issue_attributes_from_keywords(issue) + issue.safe_attributes = {'custom_field_values' => custom_field_values_from_keywords(issue)} add_attachments(issue) - # check workflow - if status && issue.new_statuses_allowed_to(user).include?(status) - issue.status = status - end - issue.start_date = start_date if start_date - issue.due_date = due_date if due_date - issue.assigned_to = assigned_to if assigned_to - issue.save! logger.info "MailHandler: issue ##{issue.id} updated by #{user}" if logger && logger.info journal @@ -263,6 +225,41 @@ class MailHandler < ActionMailer::Base end end end + + def target_project + # TODO: other ways to specify project: + # * parse the email To field + # * specific project (eg. Setting.mail_handler_target_project) + target = Project.find_by_identifier(get_keyword(:project)) + raise MissingInformation.new('Unable to determine target project') if target.nil? + target + end + + # Returns a Hash of issue attributes extracted from keywords in the email body + def issue_attributes_from_keywords(issue) + { + 'tracker_id' => ((k = get_keyword(:tracker)) && issue.project.trackers.find_by_name(k).try(:id)) || issue.project.trackers.find(:first).try(:id), + 'status_id' => (k = get_keyword(:status)) && IssueStatus.find_by_name(k).try(:id), + 'priority_id' => (k = get_keyword(:priority)) && IssuePriority.find_by_name(k).try(:id), + 'category_id' => (k = get_keyword(:category)) && issue.project.issue_categories.find_by_name(k).try(:id), + 'assigned_to_id' => (k = get_keyword(:assigned_to, :override => true)) && find_user_from_keyword(k).try(:id), + 'fixed_version_id' => (k = get_keyword(:fixed_version, :override => true)) && issue.project.shared_versions.find_by_name(k).try(:id), + 'start_date' => get_keyword(:start_date, :override => true), + 'due_date' => get_keyword(:due_date, :override => true), + 'estimated_hours' => get_keyword(:estimated_hours, :override => true), + 'done_ratio' => get_keyword(:done_ratio, :override => true), + }.delete_if {|k, v| v.blank? } + end + + # Returns a Hash of issue custom field values extracted from keywords in the email body + def custom_field_values_from_keywords(customized) + customized.custom_field_values.inject({}) do |h, v| + if value = get_keyword(v.custom_field.name, :override => true) + h[v.custom_field.id.to_s] = value + end + h + end + end # Returns the text/plain part of the email # If not found (eg. HTML-only email), returns the body with tags removed diff --git a/test/fixtures/mail_handler/ticket_on_given_project.eml b/test/fixtures/mail_handler/ticket_on_given_project.eml index cfc90b534..39790f287 100644 --- a/test/fixtures/mail_handler/ticket_on_given_project.eml +++ b/test/fixtures/mail_handler/ticket_on_given_project.eml @@ -54,4 +54,7 @@ Status: Resolved due date: 2010-12-31 Start Date:2010-01-01 Assigned to: John Smith +fixed version: alpha +estimated hours: 2.5 +done ratio: 30 diff --git a/test/fixtures/mail_handler/ticket_reply_with_status.eml b/test/fixtures/mail_handler/ticket_reply_with_status.eml index e7d68c3db..66ca730ac 100644 --- a/test/fixtures/mail_handler/ticket_reply_with_status.eml +++ b/test/fixtures/mail_handler/ticket_reply_with_status.eml @@ -29,6 +29,7 @@ Status: Resolved due date: 2010-12-31 Start Date:2010-01-01 Assigned to: jsmith@somenet.foo +searchable field: Updated custom value ------=_NextPart_000_0067_01C8D3CE.711F9CC0 Content-Type: text/html; diff --git a/test/unit/mail_handler_test.rb b/test/unit/mail_handler_test.rb index e59d78be9..ac8e10433 100644 --- a/test/unit/mail_handler_test.rb +++ b/test/unit/mail_handler_test.rb @@ -30,6 +30,7 @@ class MailHandlerTest < ActiveSupport::TestCase :workflows, :trackers, :projects_trackers, + :versions, :enumerations, :issue_categories, :custom_fields, @@ -59,6 +60,9 @@ class MailHandlerTest < ActiveSupport::TestCase assert_equal '2010-01-01', issue.start_date.to_s assert_equal '2010-12-31', issue.due_date.to_s assert_equal User.find_by_login('jsmith'), issue.assigned_to + assert_equal Version.find_by_name('alpha'), issue.fixed_version + assert_equal 2.5, issue.estimated_hours + assert_equal 30, issue.done_ratio # keywords should be removed from the email body assert !issue.description.match(/^Project:/i) assert !issue.description.match(/^Status:/i) @@ -269,6 +273,7 @@ class MailHandlerTest < ActiveSupport::TestCase assert_equal '2010-01-01', issue.start_date.to_s assert_equal '2010-12-31', issue.due_date.to_s assert_equal User.find_by_login('jsmith'), issue.assigned_to + assert_equal 'Updated custom value', issue.custom_value_for(CustomField.find_by_name('Searchable field')).value end def test_add_issue_note_should_send_email_notification -- 2.39.5