jro_apps/redmine Commit - r13781:cb9d405db3ba

Fixed that journals are shown multiple times in activity (#19168)....

Jean-Philippe Lang -

r13781:cb9d405db3ba

parent child

Context file:

r13781:cb9d405db3ba

Collapse all files

app/models/journal.rb +1 -1

             # Redmine - project management software
             # Copyright (C) 2006-2015  Jean-Philippe Lang
             #
             # This program is free software; you can redistribute it and/or
             # modify it under the terms of the GNU General Public License
             # as published by the Free Software Foundation; either version 2
             # of the License, or (at your option) any later version.
             #
             # This program is distributed in the hope that it will be useful,
             # but WITHOUT ANY WARRANTY; without even the implied warranty of
             # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
             # GNU General Public License for more details.
             #
             # You should have received a copy of the GNU General Public License
             # along with this program; if not, write to the Free Software
             # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
             class Journal < ActiveRecord::Base
               belongs_to :journalized, :polymorphic => true
               # added as a quick fix to allow eager loading of the polymorphic association
               # since always associated to an issue, for now
               belongs_to :issue, :foreign_key => :journalized_id
               belongs_to :user
               has_many :details, :class_name => "JournalDetail", :dependent => :delete_all, :inverse_of => :journal
               attr_accessor :indice
               attr_protected :id
               acts_as_event :title => Proc.new {|o| status = ((s = o.new_status) ? " (#{s})" : nil); "#{o.issue.tracker} ##{o.issue.id}#{status}: #{o.issue.subject}" },
                             :description => :notes,
                             :author => :user,
                             :group => :issue,
                             :type => Proc.new {|o| (s = o.new_status) ? (s.is_closed? ? 'issue-closed' : 'issue-edit') : 'issue-note' },
                             :url => Proc.new {|o| {:controller => 'issues', :action => 'show', :id => o.issue.id, :anchor => "change-#{o.id}"}}
               acts_as_activity_provider :type => 'issues',
                                         :author_key => :user_id,
                                         :scope => preload({:issue => :project}, :user).
                                                   joins("LEFT OUTER JOIN #{JournalDetail.table_name} ON #{JournalDetail.table_name}.journal_id = #{Journal.table_name}.id").
                                                   where("#{Journal.table_name}.journalized_type = 'Issue' AND" +
-                                                        " (#{JournalDetail.table_name}.prop_key = 'status_id' OR #{Journal.table_name}.notes <> '')")
+                                                        " (#{JournalDetail.table_name}.prop_key = 'status_id' OR #{Journal.table_name}.notes <> '')").uniq
               before_create :split_private_notes
               after_create :send_notification
               scope :visible, lambda {|*args|
                 user = args.shift || User.current
                 joins(:issue => :project).
                   where(Issue.visible_condition(user, *args)).
                   where("(#{Journal.table_name}.private_notes = ? OR (#{Project.allowed_to_condition(user, :view_private_notes, *args)}))", false)
               }
               def initialize(*args)
                 super
                 if journalized
                   if journalized.new_record?
                     self.notify = false
                   else
                     start
                   end
                 end
               end
               def save(*args)
                 journalize_changes
                 # Do not save an empty journal
                 (details.empty? && notes.blank?) ? false : super
               end
               # Returns journal details that are visible to user
               def visible_details(user=User.current)
                 details.select do |detail|
                   if detail.property == 'cf'
                     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
                     true
                   end
                 end
               end
               def each_notification(users, &block)
                 if users.any?
                   users_by_details_visibility = users.group_by do |user|
                     visible_details(user)
                   end
                   users_by_details_visibility.each do |visible_details, users|
                     if notes? || visible_details.any?
                       yield(users)
                     end
                   end
                 end
               end
               # Returns the JournalDetail for the given attribute, or nil if the attribute
               # was not updated
               def detail_for_attribute(attribute)
                 details.detect {|detail| detail.prop_key == attribute}
               end
               # Returns the new status if the journal contains a status change, otherwise nil
               def new_status
                 s = new_value_for('status_id')
                 s ? IssueStatus.find_by_id(s.to_i) : nil
               end
               def new_value_for(prop)
                 detail_for_attribute(prop).try(:value)
               end
               def editable_by?(usr)
                 usr && usr.logged? && (usr.allowed_to?(:edit_issue_notes, project) || (self.user == usr && usr.allowed_to?(:edit_own_issue_notes, project)))
               end
               def project
                 journalized.respond_to?(:project) ? journalized.project : nil
               end
               def attachments
                 journalized.respond_to?(:attachments) ? journalized.attachments : nil
               end
               # Returns a string of css classes
               def css_classes
                 s = 'journal'
                 s << ' has-notes' unless notes.blank?
                 s << ' has-details' unless details.blank?
                 s << ' private-notes' if private_notes?
                 s
               end
               def notify?
                 @notify != false
               end
               def notify=(arg)
                 @notify = arg
               end
               def notified_users
                 notified = journalized.notified_users
                 if private_notes?
                   notified = notified.select {|user| user.allowed_to?(:view_private_notes, journalized.project)}
                 end
                 notified
               end
               def recipients
                 notified_users.map(&:mail)
               end
               def notified_watchers
                 notified = journalized.notified_watchers
                 if private_notes?
                   notified = notified.select {|user| user.allowed_to?(:view_private_notes, journalized.project)}
                 end
                 notified
               end
               def watcher_recipients
                 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.where(: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
               # Stores the values of the attributes and custom fields of the journalized object
               def start
                 if journalized
                   @attributes_before_change = journalized.journalized_attribute_names.inject({}) do |h, attribute|
                     h[attribute] = journalized.send(attribute)
                     h
                   end
                   @custom_values_before_change = journalized.custom_field_values.inject({}) do |h, c|
                     h[c.custom_field_id] = c.value
                     h
                   end
                 end
                 self
               end
               # Adds a journal detail for an attachment that was added or removed
               def journalize_attachment(attachment, added_or_removed)
                 key = (added_or_removed == :removed ? :old_value : :value)
                 details << JournalDetail.new(
                     :property => 'attachment',
                     :prop_key => attachment.id,
                     key => attachment.filename
                   )
               end
               # Adds a journal detail for an issue relation that was added or removed
               def journalize_relation(relation, added_or_removed)
                 key = (added_or_removed == :removed ? :old_value : :value)
                 details << JournalDetail.new(
                     :property  => 'relation',
                     :prop_key  => relation.relation_type_for(journalized),
                     key => relation.other_issue(journalized).try(:id)
                   )
               end
               private
               # Generates journal details for attribute and custom field changes
               def journalize_changes
                 # attributes changes
                 if @attributes_before_change
                   journalized.journalized_attribute_names.each {|attribute|
                     before = @attributes_before_change[attribute]
                     after = journalized.send(attribute)
                     next if before == after || (before.blank? && after.blank?)
                     add_attribute_detail(attribute, before, after)
                   }
                 end
                 if @custom_values_before_change
                   # custom fields changes
                   journalized.custom_field_values.each {|c|
                     before = @custom_values_before_change[c.custom_field_id]
                     after = c.value
                     next if before == after || (before.blank? && after.blank?)
                     if before.is_a?(Array) || after.is_a?(Array)
                       before = [before] unless before.is_a?(Array)
                       after = [after] unless after.is_a?(Array)
                       # values removed
                       (before - after).reject(&:blank?).each do |value|
                         add_custom_value_detail(c, value, nil)
                       end
                       # values added
                       (after - before).reject(&:blank?).each do |value|
                         add_custom_value_detail(c, nil, value)
                       end
                     else
                       add_custom_value_detail(c, before, after)
                     end
                   }
                 end
                 start
               end
               # Adds a journal detail for an attribute change
               def add_attribute_detail(attribute, old_value, value)
                 add_detail('attr', attribute, old_value, value)
               end
               # Adds a journal detail for a custom field value change
               def add_custom_value_detail(custom_value, old_value, value)
                 add_detail('cf', custom_value.custom_field_id, old_value, value)
               end
               # Adds a journal detail
               def add_detail(property, prop_key, old_value, value)
                 details << JournalDetail.new(
                     :property => property,
                     :prop_key => prop_key,
                     :old_value => old_value,
                     :value => value
                   )
               end
               def split_private_notes
                 if private_notes?
                   if notes.present?
                     if details.any?
                       # Split the journal (notes/changes) so we don't have half-private journals
                       journal = Journal.new(:journalized => journalized, :user => user, :notes => nil, :private_notes => false)
                       journal.details = details
                       journal.save
                       self.details = []
                       self.created_on = journal.created_on
                     end
                   else
                     # Blank notes should not be private
                     self.private_notes = false
                   end
                 end
                 true
               end
               def send_notification
                 if notify? && (Setting.notified_events.include?('issue_updated') ||
                     (Setting.notified_events.include?('issue_note_added') && notes.present?) ||
                     (Setting.notified_events.include?('issue_status_updated') && new_status.present?) ||
                     (Setting.notified_events.include?('issue_assigned_to_updated') && detail_for_attribute('assigned_to_id').present?) ||
                     (Setting.notified_events.include?('issue_priority_updated') && new_value_for('priority_id').present?)
                   )
                   Mailer.deliver_issue_edit(self)
                 end
               end
             end

test/unit/activity_test.rb +8 0

             # Redmine - project management software
             # Copyright (C) 2006-2015  Jean-Philippe Lang
             #
             # This program is free software; you can redistribute it and/or
             # modify it under the terms of the GNU General Public License
             # as published by the Free Software Foundation; either version 2
             # of the License, or (at your option) any later version.
             #
             # This program is distributed in the hope that it will be useful,
             # but WITHOUT ANY WARRANTY; without even the implied warranty of
             # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
             # GNU General Public License for more details.
             #
             # You should have received a copy of the GNU General Public License
             # along with this program; if not, write to the Free Software
             # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
             require File.expand_path('../../test_helper', __FILE__)
             class ActivityTest < ActiveSupport::TestCase
               fixtures :projects, :versions, :attachments, :users, :roles, :members, :member_roles, :issues, :journals, :journal_details,
                        :trackers, :projects_trackers, :issue_statuses, :enabled_modules, :enumerations, :boards, :messages, :time_entries,
                        :wikis, :wiki_pages, :wiki_contents, :wiki_content_versions
               def setup
                 @project = Project.find(1)
               end
               def test_activity_without_subprojects
                 events = find_events(User.anonymous, :project => @project)
                 assert_not_nil events
                 assert events.include?(Issue.find(1))
                 assert !events.include?(Issue.find(4))
                 # subproject issue
                 assert !events.include?(Issue.find(5))
               end
               def test_activity_with_subprojects
                 events = find_events(User.anonymous, :project => @project, :with_subprojects => 1)
                 assert_not_nil events
                 assert events.include?(Issue.find(1))
                 # subproject issue
                 assert events.include?(Issue.find(5))
               end
               def test_global_activity_anonymous
                 events = find_events(User.anonymous)
                 assert_not_nil events
                 assert events.include?(Issue.find(1))
                 assert events.include?(Message.find(5))
                 # Issue of a private project
                 assert !events.include?(Issue.find(4))
                 # Private issue and comment
                 assert !events.include?(Issue.find(14))
                 assert !events.include?(Journal.find(5))
               end
               def test_global_activity_logged_user
                 events = find_events(User.find(2)) # manager
                 assert_not_nil events
                 assert events.include?(Issue.find(1))
                 # Issue of a private project the user belongs to
                 assert events.include?(Issue.find(4))
               end
               def test_user_activity
                 user = User.find(2)
                 events = Redmine::Activity::Fetcher.new(User.anonymous, :author => user).events(nil, nil, :limit => 10)
                 assert(events.size > 0)
                 assert(events.size <= 10)
                 assert_nil(events.detect {|e| e.event_author != user})
               end
+              def test_journal_with_notes_and_changes_should_be_returned_once
+                f = Redmine::Activity::Fetcher.new(User.anonymous, :project => Project.find(1))
+                f.scope = ['issues']
+                events = f.events
+                assert_equal events, events.uniq
+              end
               def test_files_activity
                 f = Redmine::Activity::Fetcher.new(User.anonymous, :project => Project.find(1))
                 f.scope = ['files']
                 events = f.events
                 assert_kind_of Array, events
                 assert events.include?(Attachment.find_by_container_type_and_container_id('Project', 1))
                 assert events.include?(Attachment.find_by_container_type_and_container_id('Version', 1))
                 assert_equal [Attachment], events.collect(&:class).uniq
                 assert_equal %w(Project Version), events.collect(&:container_type).uniq.sort
               end
               def test_event_group_for_issue
                 issue = Issue.find(1)
                 assert_equal issue, issue.event_group
               end
               def test_event_group_for_journal
                 issue = Issue.find(1)
                 journal = issue.journals.first
                 assert_equal issue, journal.event_group
               end
               def test_event_group_for_issue_time_entry
                 time = TimeEntry.where(:issue_id => 1).first
                 assert_equal time.issue, time.event_group
               end
               def test_event_group_for_project_time_entry
                 time = TimeEntry.where(:issue_id => nil).first
                 assert_equal time, time.event_group
               end
               def test_event_group_for_message
                 message = Message.find(1)
                 reply = message.children.first
                 assert_equal message, message.event_group
                 assert_equal message, reply.event_group
               end
               def test_event_group_for_wiki_content_version
                 content = WikiContent::Version.find(1)
                 assert_equal content.page, content.event_group
               end
               class TestActivityProviderWithPermission
                 def self.activity_provider_options
                   {'test' => {:permission => :custom_permission}}
                 end
               end
               class TestActivityProviderWithNilPermission
                 def self.activity_provider_options
                   {'test' => {:permission => nil}}
                 end
               end
               class TestActivityProviderWithoutPermission
                 def self.activity_provider_options
                   {'test' => {}}
                 end
               end
               class MockUser
                 def initialize(*permissions)
                   @permissions = permissions
                 end
                 def allowed_to?(permission, *args)
                   @permissions.include?(permission)
                 end
               end
               def test_event_types_should_consider_activity_provider_permission
                 Redmine::Activity.register 'test', :class_name => 'ActivityTest::TestActivityProviderWithPermission'
                 user = MockUser.new(:custom_permission)
                 f = Redmine::Activity::Fetcher.new(user, :project => Project.find(1))
                 assert_include 'test', f.event_types
               ensure
                 Redmine::Activity.delete 'test'
               end
               def test_event_types_should_include_activity_provider_with_nil_permission
                 Redmine::Activity.register 'test', :class_name => 'ActivityTest::TestActivityProviderWithNilPermission'
                 user = MockUser.new()
                 f = Redmine::Activity::Fetcher.new(user, :project => Project.find(1))
                 assert_include 'test', f.event_types
               ensure
                 Redmine::Activity.delete 'test'
               end
               def test_event_types_should_use_default_permission_for_activity_provider_without_permission
                 Redmine::Activity.register 'test', :class_name => 'ActivityTest::TestActivityProviderWithoutPermission'
                 user = MockUser.new()
                 f = Redmine::Activity::Fetcher.new(user, :project => Project.find(1))
                 assert_not_include 'test', f.event_types
                 user = MockUser.new(:view_test)
                 f = Redmine::Activity::Fetcher.new(user, :project => Project.find(1))
                 assert_include 'test', f.event_types
               ensure
                 Redmine::Activity.delete 'test'
               end
               private
               def find_events(user, options={})
                 Redmine::Activity::Fetcher.new(user, options).events(Date.today - 30, Date.today + 1)
               end
             end

General Comments 0

Write
Preview

You need to be logged in to leave comments. Login now

No TODOs yet

	Site-wide shortcuts
/	Use quick search box
g h	Goto home page
g g	Goto my private gists page
g G	Goto my public gists page
g 0-9	Goto bookmarked items from 0-9
n r	New repository page
n g	New gist page

	Repositories
g s	Goto summary page
g c	Goto changelog page
g f	Goto files page
g F	Goto files page with file search activated
g p	Goto pull requests page
g o	Goto repository settings
g O	Goto repository access permissions settings
t s	Toggle sidebar on some pages