diff options
| -rw-r--r-- | app/controllers/issues_controller.rb | 1 | ||||
| -rw-r--r-- | app/helpers/issues_helper.rb | 16 | ||||
| -rw-r--r-- | app/models/journal.rb | 20 | ||||
| -rw-r--r-- | app/models/journal_detail.rb | 6 | ||||
| -rw-r--r-- | test/unit/journal_test.rb | 24 |
5 files changed, 56 insertions, 11 deletions
diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index ef778865d..a0f296487 100644 --- a/app/controllers/issues_controller.rb +++ b/app/controllers/issues_controller.rb @@ -103,6 +103,7 @@ class IssuesController < ApplicationController @journals = @issue.journals.includes(:user, :details).reorder("#{Journal.table_name}.id ASC").all @journals.each_with_index {|j,i| j.indice = i+1} @journals.reject!(&:private_notes?) unless User.current.allowed_to?(:view_private_notes, @issue.project) + Journal.preload_journals_details_custom_fields(@journals) # TODO: use #select! when ruby1.8 support is dropped @journals.reject! {|journal| !journal.notes? && journal.visible_details.empty?} @journals.reverse! if User.current.wants_comments_in_reverse_order? diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index bc7fbab85..614c2f90d 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -261,23 +261,23 @@ module IssuesHelper values_by_field = {} details.each do |detail| if detail.property == 'cf' - field_id = detail.prop_key - field = CustomField.find_by_id(field_id) + field = detail.custom_field if field && field.multiple? - values_by_field[field_id] ||= {:added => [], :deleted => []} + values_by_field[field] ||= {:added => [], :deleted => []} if detail.old_value - values_by_field[field_id][:deleted] << detail.old_value + values_by_field[field][:deleted] << detail.old_value end if detail.value - values_by_field[field_id][:added] << detail.value + values_by_field[field][:added] << detail.value end next end end strings << show_detail(detail, no_html, options) end - values_by_field.each do |field_id, changes| - detail = JournalDetail.new(:property => 'cf', :prop_key => field_id) + values_by_field.each do |field, changes| + detail = JournalDetail.new(:property => 'cf', :prop_key => field.id.to_s) + detail.instance_variable_set "@custom_field", field if changes[:added].any? detail.value = changes[:added] strings << show_detail(detail, no_html, options) @@ -320,7 +320,7 @@ module IssuesHelper old_value = l(detail.old_value == "0" ? :general_text_No : :general_text_Yes) unless detail.old_value.blank? end when 'cf' - custom_field = CustomField.find_by_id(detail.prop_key) + custom_field = detail.custom_field if custom_field multiple = custom_field.multiple? label = custom_field.name diff --git a/app/models/journal.rb b/app/models/journal.rb index d7712fa1f..eb4f9499f 100644 --- a/app/models/journal.rb +++ b/app/models/journal.rb @@ -58,9 +58,7 @@ class Journal < ActiveRecord::Base def visible_details(user=User.current) details.select do |detail| if detail.property == 'cf' - field_id = detail.prop_key - field = CustomField.find_by_id(field_id) - field && field.visible_by?(project, user) + detail.custom_field && detail.custom_field.visible_by?(project, user) elsif detail.property == 'relation' Issue.find_by_id(detail.value || detail.old_value).try(:visible?, user) else @@ -146,6 +144,22 @@ class Journal < ActiveRecord::Base notified_watchers.map(&:mail) end + # Sets @custom_field instance variable on journals details using a single query + def self.preload_journals_details_custom_fields(journals) + field_ids = journals.map(&:details).flatten.select {|d| d.property == 'cf'}.map(&:prop_key).uniq + if field_ids.any? + fields_by_id = CustomField.find_all_by_id(field_ids).inject({}) {|h, f| h[f.id] = f; h} + journals.each do |journal| + journal.details.each do |detail| + if detail.property == 'cf' + detail.instance_variable_set "@custom_field", fields_by_id[detail.prop_key.to_i] + end + end + end + end + journals + end + private def split_private_notes diff --git a/app/models/journal_detail.rb b/app/models/journal_detail.rb index 1ec195733..9484a2a5f 100644 --- a/app/models/journal_detail.rb +++ b/app/models/journal_detail.rb @@ -19,6 +19,12 @@ class JournalDetail < ActiveRecord::Base belongs_to :journal before_save :normalize_values + def custom_field + if property == 'cf' + @custom_field ||= CustomField.find_by_id(prop_key) + end + end + private def normalize_values diff --git a/test/unit/journal_test.rb b/test/unit/journal_test.rb index e3184bf21..af3a30c9e 100644 --- a/test/unit/journal_test.rb +++ b/test/unit/journal_test.rb @@ -155,6 +155,20 @@ class JournalTest < ActiveSupport::TestCase assert journals.detect {|journal| !journal.issue.project.is_public?} end + def test_preload_journals_details_custom_fields_should_set_custom_field_instance_variable + d = JournalDetail.new(:property => 'cf', :prop_key => '2') + journals = [Journal.new(:details => [d])] + + d.expects(:instance_variable_set).with("@custom_field", CustomField.find(2)).once + Journal.preload_journals_details_custom_fields(journals) + end + + def test_preload_journals_details_custom_fields_with_empty_set + assert_nothing_raised do + Journal.preload_journals_details_custom_fields([]) + end + end + def test_details_should_normalize_dates j = JournalDetail.create!(:old_value => Date.parse('2012-11-03'), :value => Date.parse('2013-01-02')) j.reload @@ -176,6 +190,16 @@ class JournalTest < ActiveSupport::TestCase assert_equal '0', j.value end + def test_custom_field_should_return_custom_field_for_cf_detail + d = JournalDetail.new(:property => 'cf', :prop_key => '2') + assert_equal CustomField.find(2), d.custom_field + end + + def test_custom_field_should_return_nil_for_non_cf_detail + d = JournalDetail.new(:property => 'subject') + assert_equal nil, d.custom_field + end + def test_visible_details_should_include_relations_to_visible_issues_only issue = Issue.generate! visible_issue = Issue.generate! |