diff --git a/app/controllers/journals_controller.rb b/app/controllers/journals_controller.rb index 8e7bcab40..fd344c000 100644 --- a/app/controllers/journals_controller.rb +++ b/app/controllers/journals_controller.rb @@ -46,7 +46,7 @@ class JournalsController < ApplicationController end def diff - @issue = @journal.issue + @issue = @journal.journalized if params[:detail_id].present? @detail = @journal.details.find_by_id(params[:detail_id]) else @@ -100,8 +100,9 @@ class JournalsController < ApplicationController private def find_journal - @journal = Journal.visible.find(params[:id]) - @project = @journal.journalized.project + # Only issue journals are handled by this controller for now + @journal = Journal.where(:journalized_type => 'Issue').visible.find(params[:id]) + @project = @journal.project rescue ActiveRecord::RecordNotFound render_404 end diff --git a/app/helpers/journals_helper.rb b/app/helpers/journals_helper.rb index 4092b7b0f..206df1d0c 100644 --- a/app/helpers/journals_helper.rb +++ b/app/helpers/journals_helper.rb @@ -30,7 +30,7 @@ module JournalsHelper def render_journal_actions(issue, journal, options={}) links = [] dropbown_links = [] - indice = journal.indice || journal.issue.visible_journals_with_index.find{|j| j.id == journal.id}&.indice + indice = journal.indice || journal.journalized.visible_journals_with_index.find{|j| j.id == journal.id}&.indice dropbown_links << copy_object_url_link(issue_url(issue, anchor: "note-#{indice}", only_path: false)) if journal.attachments.size > 1 diff --git a/app/models/issue.rb b/app/models/issue.rb index ec778a2ac..25952c6f8 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -38,7 +38,6 @@ class Issue < ApplicationRecord belongs_to :priority, :class_name => 'IssuePriority' belongs_to :category, :class_name => 'IssueCategory' - has_many :journals, :as => :journalized, :dependent => :destroy, :inverse_of => :journalized has_many :time_entries, :dependent => :destroy has_and_belongs_to_many :changesets, lambda {order("#{Changeset.table_name}.committed_on ASC, #{Changeset.table_name}.id ASC")} @@ -48,6 +47,12 @@ class Issue < ApplicationRecord acts_as_attachable :after_add => :attachment_added, :after_remove => :attachment_removed acts_as_customizable acts_as_watchable + acts_as_journalized :activity_type => 'issues', + :permissions => { + :edit_notes => :edit_issue_notes, + :edit_own_notes => :edit_own_issue_notes, + :add_watchers => :add_issue_watchers + } acts_as_searchable :columns => ['subject', "#{table_name}.description"], :preload => [:project, :status, :tracker], :scope => lambda {|options| options[:open_issues] ? self.open : self.all} @@ -1304,8 +1309,8 @@ class Issue < ApplicationRecord def self.load_visible_last_updated_by(issues, user=User.current) if issues.any? issue_ids = issues.map(&:id) - journal_ids = Journal.joins(issue: :project). - where(:journalized_type => 'Issue', :journalized_id => issue_ids). + journal_ids = Journal.joins_journalized(Issue). + where(:journalized_id => issue_ids). where(Journal.visible_notes_condition(user, :skip_pre_condition => true)). group(:journalized_id). maximum(:id). @@ -1323,8 +1328,8 @@ class Issue < ApplicationRecord def self.load_visible_last_notes(issues, user=User.current) if issues.any? issue_ids = issues.map(&:id) - journal_ids = Journal.joins(issue: :project). - where(:journalized_type => 'Issue', :journalized_id => issue_ids). + journal_ids = Journal.joins_journalized(Issue). + where(:journalized_id => issue_ids). where(Journal.visible_notes_condition(user, :skip_pre_condition => true)). where.not(notes: ''). group(:journalized_id). @@ -1721,6 +1726,68 @@ class Issue < ApplicationRecord end end + def self.journal_visible_join_condition(user=User.current, options={}) + "(#{visible_condition(user, options)}) AND #{Journal.visible_notes_condition(user, :skip_pre_condition => true)}" + end + + def self.journal_activity_preload + [{:journalized => [:project, :tracker]}, :user] + end + + def self.journal_activity_scope(scope) + scope.joins("LEFT OUTER JOIN #{JournalDetail.table_name} ON #{JournalDetail.table_name}.journal_id = #{Journal.table_name}.id"). + where("#{JournalDetail.table_name}.prop_key = 'status_id' OR #{Journal.table_name}.notes <> ''").distinct + end + + def journal_event_title(journal) + status = (s = journal_new_status(journal)) ? " (#{s})" : nil + "#{tracker} ##{id}#{status}: #{subject}" + end + + def journal_event_type(journal) + if (status = journal_new_status(journal)) + status.is_closed? ? 'issue-closed' : 'issue-edit' + else + 'issue-note' + end + end + + def journal_relation_visible?(detail, user) + Issue.find_by_id(detail.value || detail.old_value).try(:visible?, user) + end + + def journal_new_status(journal) + s = journal.new_value_for('status_id') + s ? IssueStatus.find_by_id(s.to_i) : nil + end + + def journal_notify(journal) + if Setting.notified_events.include?('issue_updated') || + (Setting.notified_events.include?('issue_note_added') && journal.notes.present?) || + (Setting.notified_events.include?('issue_status_updated') && journal_new_status(journal).present?) || + (Setting.notified_events.include?('issue_assigned_to_updated') && journal.detail_for_attribute('assigned_to_id').present?) || + (Setting.notified_events.include?('issue_priority_updated') && journal.new_value_for('priority_id').present?) || + (Setting.notified_events.include?('issue_fixed_version_updated') && journal.detail_for_attribute('fixed_version_id').present?) || + (Setting.notified_events.include?('issue_attachment_added') && journal.details.any? {|d| d.property == 'attachment' && d.value }) + Mailer.deliver_issue_edit(journal) + end + end + + def journal_process_watchers(journal) + user = journal.user + if user.is_a?(User) && + user.pref.auto_watch_on?('issue_contributed_to') && + journal_valid_watcher?(user) + set_watcher(user, true) + end + + if assigned_to.is_a?(User) && + assigned_to.pref.auto_watch_on?('issue_assigned_to_me') && + journal_valid_watcher?(assigned_to) + set_watcher(assigned_to, true) + end + end + private def user_tracker_permission?(user, permission) diff --git a/app/models/issue_query.rb b/app/models/issue_query.rb index 3431750f6..57c54669a 100644 --- a/app/models/issue_query.rb +++ b/app/models/issue_query.rb @@ -477,13 +477,14 @@ class IssueQuery < Query # Returns the journals # Valid options are :order, :offset, :limit def journals(options={}) - Journal.visible. - joins(:issue => [:project, :status]). + Journal.joins_journalized(Issue). + visible. + joins("INNER JOIN #{IssueStatus.table_name} ON #{IssueStatus.table_name}.id = #{Issue.table_name}.status_id"). where(statement). order(options[:order]). limit(options[:limit]). offset(options[:offset]). - preload(:details, :user, {:issue => [:project, :author, :tracker, :status]}). + preload(:details, :user, {:journalized => [:project, :author, :tracker, :status]}). to_a rescue ::ActiveRecord::StatementInvalid => e raise StatementInvalid.new(e.message) diff --git a/app/models/journal.rb b/app/models/journal.rb index 8435ec069..78460ce14 100644 --- a/app/models/journal.rb +++ b/app/models/journal.rb @@ -21,44 +21,39 @@ class Journal < ApplicationRecord include Redmine::SafeAttributes include Redmine::Reaction::Reactable - 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 :journalized, :polymorphic => true, :inverse_of => :journals belongs_to :user belongs_to :updated_by, :class_name => 'User' + has_many :details, lambda {order(:id)}, :class_name => "JournalDetail", :dependent => :delete_all, :inverse_of => :journal attr_accessor :indice + # Returns journals of klass joined with the table of klass (and its project) + def self.joins_journalized(klass) + where(:journalized_type => klass.polymorphic_name).joins(*klass.journal_joins) + end + + def self.activity_scope_for(klass) + klass.journal_activity_scope(joins_journalized(klass).preload(*klass.journal_activity_preload)) + end + + def self.register_activity_provider(klass) + # Resolve the class at runtime so that the scope survives code reloading + class_name = klass.name + acts_as_activity_provider( + :type => klass.journalized_options[:activity_type], + :author_key => :user_id, + :scope => proc { activity_scope_for(class_name.constantize) } + ) + end + acts_as_event( - :title => - Proc.new do |o| - status = ((s = o.new_status) ? " (#{s})" : nil) - "#{o.issue.tracker} ##{o.issue.id}#{status}: #{o.issue.subject}" - end, - :description => :notes, - :author => :user, - :group => :issue, - :type => - Proc.new do |o| - (s = o.new_status) ? (s.is_closed? ? 'issue-closed' : 'issue-edit') : 'issue-note' - end, - :url => - Proc.new do |o| - {:controller => 'issues', :action => 'show', :id => o.issue.id, :anchor => "change-#{o.id}"} - end - ) - acts_as_activity_provider( - :type => 'issues', - :author_key => :user_id, - :scope => - proc do - preload({:issue => :project}, {:issue => :tracker}, :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 <> '')").distinct - end + title: proc {|o| o.journalized.journal_event_title(o) }, + description: :notes, + author: :user, + group: :journalized, + type: proc {|o| o.journalized.journal_event_type(o) }, + url: proc {|o| o.journalized.journal_event_url(o) } ) acts_as_mentionable :attributes => ['notes'] before_create :split_private_notes @@ -69,9 +64,21 @@ class Journal < ApplicationRecord user = args.shift || User.current options = args.shift || {} - joins(:issue => :project). - where(Issue.visible_condition(user, options)). - where(Journal.visible_notes_condition(user, :skip_pre_condition => true)) + classes = Redmine::Acts::Journalized.journalized_classes + # Only consider the types the current scope is restricted to (eg. activity) + if (types = Array(where_values_hash['journalized_type'])).any? + classes = classes.select {|klass| types.include?(klass.polymorphic_name)} + end + + if classes.size == 1 && (condition = classes.first.journal_visible_join_condition(user, options)) + # Joining the journalized table lets the database use the same query + # plans as before journals were decoupled from issues + joins_journalized(classes.first).where(condition) + elsif classes.any? + where(classes.map {|klass| "(#{klass.journal_visible_condition(user, options)})"}.join(' OR ')) + else + none + end end) safe_attributes( @@ -79,7 +86,7 @@ class Journal < ApplicationRecord :if => lambda {|journal, user| journal.new_record? || journal.editable_by?(user)}) safe_attributes( 'private_notes', - :if => lambda {|journal, user| user.allowed_to?(:set_notes_private, journal.project)}) + :if => lambda {|journal, user| journal.journalized&.journal_private_notes_allowed?(user)}) safe_attributes 'updated_by' # Returns a SQL condition to filter out journals with notes that are not visible to user @@ -109,13 +116,10 @@ class Journal < ApplicationRecord notes.blank? && details.empty? end - def journalized - if journalized_type == 'Issue' && association(:issue).loaded? - # Avoid extra query by using preloaded association - issue - else - super - end + # @deprecated Use #journalized instead + def issue + Rails.application.deprecators[:redmine].warn "Journal#issue is deprecated and will be removed. Use Journal#journalized instead." + journalized if journalized_type == 'Issue' end # Returns journal details that are visible to user @@ -124,7 +128,7 @@ class Journal < ApplicationRecord 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) + journalized&.journal_relation_visible?(detail, user) else true end @@ -139,8 +143,7 @@ class Journal < ApplicationRecord # 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 + journalized&.journal_new_status(self) end def new_value_for(prop) @@ -148,11 +151,11 @@ class Journal < ApplicationRecord 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))) + journalized.present? && journalized.journal_editable_by?(self, usr) end def project - journalized.respond_to?(:project) ? journalized.project : nil + journalized&.journal_project end def attachments @@ -163,7 +166,7 @@ class Journal < ApplicationRecord end def visible?(*) - journalized.visible?(*) + journalized.present? && journalized.journal_visible?(*) end def attachments_visible? @@ -188,11 +191,8 @@ class Journal < ApplicationRecord 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 + notified = journalized.journal_notified_users + select_journal_visible_user(notified) end def recipients @@ -200,7 +200,7 @@ class Journal < ApplicationRecord end def notified_watchers - notified = journalized.notified_watchers + notified = journalized.journal_notified_watchers select_journal_visible_user(notified) end @@ -235,8 +235,10 @@ class Journal < ApplicationRecord @attributes_before_change = journalized.journalized_attribute_names.index_with do |attribute| journalized.send(attribute) end - @custom_values_before_change = journalized.custom_field_values.to_h do |c| - [c.custom_field_id, c.value] + if journalized.respond_to?(:custom_field_values) + @custom_values_before_change = journalized.custom_field_values.to_h do |c| + [c.custom_field_id, c.value] + end end end self @@ -354,45 +356,16 @@ class Journal < ApplicationRecord end def add_watcher - if user.is_a?(User) && - user.pref.auto_watch_on?('issue_contributed_to') && - valid_watcher?(user) - journalized.set_watcher(user, true) - end - - assignee = journalized.assigned_to - if assignee.is_a?(User) && - assignee.pref.auto_watch_on?('issue_assigned_to_me') && - valid_watcher?(assignee) - journalized.set_watcher(assignee, true) - end - end - - def valid_watcher?(user) - user.active? && - user.allowed_to?(:add_issue_watchers, journalized.project) && - journalized.valid_watcher?(user) && - !journalized.watched_by?(user) + journalized.journal_process_watchers(self) 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?) || - (Setting.notified_events.include?('issue_fixed_version_updated') && detail_for_attribute('fixed_version_id').present?) || - (Setting.notified_events.include?('issue_attachment_added') && details.any? {|d| d.property == 'attachment' && d.value }) - ) - Mailer.deliver_issue_edit(self) - end + journalized.journal_notify(self) if notify? end def select_journal_visible_user(notified) if private_notes? - notified = notified.select {|user| user.allowed_to?(:view_private_notes, journalized.project)} + notified = notified.select {|user| journalized.journal_user_can_view_private_notes?(user)} end notified end diff --git a/app/views/journals/_notes_form.html.erb b/app/views/journals/_notes_form.html.erb index 4213d6613..1e44f0d84 100644 --- a/app/views/journals/_notes_form.html.erb +++ b/app/views/journals/_notes_form.html.erb @@ -18,4 +18,4 @@

<%= submit_tag l(:button_save) %> <%= link_to l(:button_cancel), '#', :onclick => "$('#journal-#{@journal.id}-form').remove(); $('#journal-#{@journal.id}-notes').show(); return false;" %>

<% end %> -<%= wikitoolbar_for "journal_#{@journal.id}_notes", preview_issue_path(:project_id => @project, :issue_id => @journal.issue) %> +<%= wikitoolbar_for "journal_#{@journal.id}_notes", preview_issue_path(:project_id => @project, :issue_id => @journal.journalized) %> diff --git a/app/views/journals/index.builder b/app/views/journals/index.builder index a7ddd71c4..9d6fc74dc 100644 --- a/app/views/journals/index.builder +++ b/app/views/journals/index.builder @@ -10,7 +10,7 @@ xml.feed "xmlns" => "http://www.w3.org/2005/Atom" do xml.updated((@journals.first ? @journals.first.event_datetime : Time.now).xmlschema) xml.author {xml.name "#{Setting.app_title}"} @journals.each do |change| - issue = change.issue + issue = change.journalized xml.entry do xml.title "#{issue.project.name} - #{issue.tracker.name} ##{issue.id}: #{issue.subject}" xml.link "rel" => "alternate", "href" => issue_url(issue) diff --git a/app/views/journals/update.js.erb b/app/views/journals/update.js.erb index 2ab11cac9..230333821 100644 --- a/app/views/journals/update.js.erb +++ b/app/views/journals/update.js.erb @@ -2,9 +2,9 @@ $("#change-<%= @journal.id %>").remove(); <% else %> $("#change-<%= @journal.id %>").attr('class', '<%= @journal.css_classes %>'); - $("#change-<%= @journal.id %> .journal-actions").html('<%= escape_javascript(render_journal_actions(@journal.issue, @journal, :reply_links => authorize_for('issues', 'edit'))) %>'); + $("#change-<%= @journal.id %> .journal-actions").html('<%= escape_javascript(render_journal_actions(@journal.journalized, @journal, :reply_links => authorize_for('issues', 'edit'))) %>'); $("#journal-<%= @journal.id %>-private_notes").replaceWith('<%= escape_javascript(render_private_notes_indicator(@journal)) %>'); - $("#journal-<%= @journal.id %>-notes").replaceWith('<%= escape_javascript(render_notes(@journal.issue, @journal, :reply_links => authorize_for('issues', 'edit'))) %>'); + $("#journal-<%= @journal.id %>-notes").replaceWith('<%= escape_javascript(render_notes(@journal.journalized, @journal, :reply_links => authorize_for('issues', 'edit'))) %>'); $("#journal-<%= @journal.id %>-notes").show(); $("#journal-<%= @journal.id %>-form").remove(); var journal_header = $("#change-<%= @journal.id %>>div.note>h4.journal-header>.journal-info"); diff --git a/lib/plugins/acts_as_journalized/init.rb b/lib/plugins/acts_as_journalized/init.rb new file mode 100644 index 000000000..bf0eb1891 --- /dev/null +++ b/lib/plugins/acts_as_journalized/init.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +# Redmine - project management software +# Copyright (C) 2006- 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_relative 'lib/acts_as_journalized' +Rails.application.reloader.to_prepare do + ApplicationRecord.send(:include, Redmine::Acts::Journalized) +end diff --git a/lib/plugins/acts_as_journalized/lib/acts_as_journalized.rb b/lib/plugins/acts_as_journalized/lib/acts_as_journalized.rb new file mode 100644 index 000000000..27f007270 --- /dev/null +++ b/lib/plugins/acts_as_journalized/lib/acts_as_journalized.rb @@ -0,0 +1,275 @@ +# frozen_string_literal: true + +# Redmine - project management software +# Copyright (C) 2006- 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. + +module Redmine + module Acts + # Makes a model journalized, i.e. its changes and notes are recorded + # as Journal records. + # + # acts_as_journalized( + # :activity_type => 'issues', + # :attributes => %w(subject description), + # :permissions => { + # :edit_notes => :edit_issue_notes, + # :edit_own_notes => :edit_own_issue_notes, + # :add_watchers => :add_issue_watchers + # } + # ) + # + # Options: + # * :activity_type - registers the journals of the model as activity + # provider of the given event type (optional) + # * :attributes - names of the attributes to journalize, unless the model + # defines #journalized_attribute_names + # * :permissions - permissions used by the default implementations, see + # DEFAULT_PERMISSIONS. A nil permission is never granted. + # + # The defaults assume that the model has a visible scope and a visible? + # method, and optionally a project association. Every journal_* method + # can be overridden by the model for specific behavior. + module Journalized + DEFAULT_PERMISSIONS = { + :edit_notes => nil, + :edit_own_notes => nil, + :view_private_notes => :view_private_notes, + :set_notes_private => :set_notes_private, + :add_watchers => nil + }.freeze + + @journalized_type_names = [] + + class << self + # Registers a journalized type by its name, so that journals of this + # type are taken into account even if the class is not loaded yet + def register(class_name) + name = class_name.to_s + @journalized_type_names << name unless @journalized_type_names.include?(name) + end + + def unregister(class_name) + @journalized_type_names.delete(class_name.to_s) + end + + def journalized_type_names + @journalized_type_names.dup + end + + # Returns the classes of the registered journalized types + def journalized_classes + @journalized_type_names.filter_map do |name| + klass = name.safe_constantize + klass if klass.respond_to?(:journalized_options) + end + end + end + + def self.included(base) + base.extend ClassMethods + end + + module ClassMethods + def acts_as_journalized(options = {}) + options.assert_valid_keys(:activity_type, :attributes, :permissions) + permissions = options[:permissions] || {} + permissions.assert_valid_keys(*DEFAULT_PERMISSIONS.keys) + + class_attribute :journalized_options, :instance_writer => false + self.journalized_options = { + :activity_type => options[:activity_type]&.to_s, + :attributes => Array(options[:attributes]).map(&:to_s), + :permissions => DEFAULT_PERMISSIONS.merge(permissions) + } + + has_many :journals, :as => :journalized, :dependent => :destroy, :inverse_of => :journalized + + extend SingletonMethods + include InstanceMethods + + Redmine::Acts::Journalized.register(polymorphic_name) + Journal.register_activity_provider(self) if journalized_options[:activity_type] + end + end + + module SingletonMethods + # Returns a SQL condition on the tables joined by journal_joins that + # restricts journals to the ones visible to user, including private + # notes, or nil if the model does not provide one. + # + # Journal.visible uses it when only the journals of this type are + # queried. Joins usually perform better than the EXISTS subquery of + # journal_visible_condition, which is used otherwise. + def journal_visible_join_condition(user=User.current, options={}) + nil + end + + # Returns a SQL condition that restricts journals to the journals + # of this type that are visible to user, including private notes + def journal_visible_condition(user=User.current, options={}) + return '1=0' unless respond_to?(:visible) + + journals = Journal.table_name + records = visible(user, options). + where("#{table_name}.#{primary_key} = #{journals}.journalized_id"). + where(journal_private_notes_condition(user, options)) + "#{journals}.journalized_type = #{connection.quote(polymorphic_name)}" \ + " AND EXISTS (#{records.select('1').to_sql})" + end + + # Returns a SQL condition to filter out journals with private notes + # that are not visible to user + def journal_private_notes_condition(user=User.current, options={}) + journals = Journal.table_name + permission = journalized_options[:permissions][:view_private_notes] + allowed = + if user.admin? + '1=1' + elsif permission && (reflection = journal_project_reflection) + project_condition = + Project.allowed_to_condition(user, permission, options.merge(:skip_pre_condition => true)) + "#{table_name}.#{reflection.foreign_key} IN" \ + " (SELECT #{Project.table_name}.id FROM #{Project.table_name} WHERE #{project_condition})" + else + '1=0' + end + sanitize_sql_for_conditions( + ["(#{journals}.private_notes = ? OR #{journals}.user_id = ? OR (#{allowed}))", false, user.id] + ) + end + + # Returns the SQL joins from the journals table to the table of this + # type and, if any, to the projects table. The journals must also be + # restricted to this type, see Journal.joins_journalized + def journal_joins + journals = Journal.table_name + joins = ["INNER JOIN #{table_name} ON #{table_name}.#{primary_key} = #{journals}.journalized_id"] + if (reflection = journal_project_reflection) + joins << "INNER JOIN #{Project.table_name} ON #{Project.table_name}.id = #{table_name}.#{reflection.foreign_key}" + end + joins + end + + # Returns the scope of the activity events. The given scope contains + # the journals of this type only, joined with journal_joins + def journal_activity_scope(scope) + scope + end + + def journal_activity_preload + journal_project_reflection ? [{:journalized => :project}, :user] : [:user] + end + + def journal_project_reflection + reflection = reflect_on_association(:project) + reflection if reflection&.belongs_to? + end + end + + module InstanceMethods + # Event API + def journal_event_title(journal) + "#{self.class.model_name.human} ##{id}" + end + + def journal_event_type(journal) + 'journal-update' + end + + def journal_event_url(journal) + {:controller => self.class.model_name.plural, :action => 'show', :id => id, :anchor => "change-#{journal.id}"} + end + + # Called after a journal is created, if notifications are enabled + def journal_notify(journal) + end + + # Called before a journal is created + def journal_process_watchers(journal) + end + + # Attributes whose changes are journalized + def journalized_attribute_names + journalized_options[:attributes] + end + + def journal_notified_users + respond_to?(:notified_users) ? notified_users : [] + end + + def journal_notified_watchers + respond_to?(:notified_watchers) ? notified_watchers : [] + end + + def journal_visible?(user=User.current) + respond_to?(:visible?) && visible?(user) + end + + def journal_project + respond_to?(:project) ? project : nil + end + + def journal_editable_by?(journal, user) + return false unless user&.logged? + + journal_allowed_to?(user, :edit_notes) || + (journal.user == user && journal_allowed_to?(user, :edit_own_notes)) + end + + def journal_user_can_view_private_notes?(user) + journal_allowed_to?(user, :view_private_notes) + end + + def journal_private_notes_allowed?(user) + journal_allowed_to?(user, :set_notes_private) + end + + def journal_valid_watcher?(user) + user.active? && + journal_allowed_to?(user, :add_watchers) && + respond_to?(:valid_watcher?) && valid_watcher?(user) && + !watched_by?(user) + end + + # Returns true if the relation of the given journal detail is visible to user + def journal_relation_visible?(detail, user) + false + end + + # Returns the new status if the journal contains a status change, otherwise nil + def journal_new_status(journal) + nil + end + + private + + # Checks the permission configured for key (see DEFAULT_PERMISSIONS) + # on the journalized project, or globally if there is no project + def journal_allowed_to?(user, key) + permission = journalized_options[:permissions][key] + return false if permission.nil? || user.nil? + + if (project = journal_project) + user.allowed_to?(permission, project) + else + user.allowed_to?(permission, nil, :global => true) + end + end + end + end + end +end diff --git a/lib/redmine/preparation.rb b/lib/redmine/preparation.rb index b3e36583f..4138128c9 100644 --- a/lib/redmine/preparation.rb +++ b/lib/redmine/preparation.rb @@ -25,6 +25,9 @@ module Redmine ApplicationRecord.include Redmine::Acts::Webhookable ApplicationRecord.include Redmine::I18n + # Journalized types, registered before their classes are loaded + Redmine::Acts::Journalized.register 'Issue' + Scm::Base.add "Subversion" Scm::Base.add "Mercurial" Scm::Base.add "Cvs" diff --git a/test/unit/changeset_test.rb b/test/unit/changeset_test.rb index 7a814fe34..766a951a8 100644 --- a/test/unit/changeset_test.rb +++ b/test/unit/changeset_test.rb @@ -313,7 +313,7 @@ class ChangesetTest < ActiveSupport::TestCase end assert issue.reload.closed? journal = Journal.order(id: :desc).first - assert_equal issue, journal.issue + assert_equal issue, journal.journalized assert_include "Applied in changeset ecookbook:r12345.", journal.notes end end diff --git a/test/unit/journal_test.rb b/test/unit/journal_test.rb index 4f130990a..7c0583a33 100644 --- a/test/unit/journal_test.rb +++ b/test/unit/journal_test.rb @@ -26,7 +26,7 @@ class JournalTest < ActiveSupport::TestCase end def test_journalized_is_an_issue - issue = @journal.issue + issue = @journal.journalized assert_kind_of Issue, issue assert_equal 1, issue.id end @@ -181,7 +181,7 @@ class JournalTest < ActiveSupport::TestCase # Anonymous user should see issues of public projects only journals = Journal.visible(User.anonymous).to_a assert journals.any? - assert_nil journals.detect {|journal| !journal.issue.project.is_public?} + assert_nil journals.detect {|journal| !journal.project.is_public?} # Anonymous user should not see issues without permission Role.anonymous.remove_permission!(:view_issues) journals = Journal.visible(User.anonymous).to_a @@ -194,7 +194,7 @@ class JournalTest < ActiveSupport::TestCase # Non member user should see issues of public projects only journals = Journal.visible(user).to_a assert journals.any? - assert_nil journals.detect {|journal| !journal.issue.project.is_public?} + assert_nil journals.detect {|journal| !journal.project.is_public?} # Non member user should not see issues without permission Role.non_member.remove_permission!(:view_issues) user.reload @@ -205,7 +205,7 @@ class JournalTest < ActiveSupport::TestCase user.reload journals = Journal.visible(user).to_a assert journals.any? - assert_nil journals.detect {|journal| journal.issue.project_id != 1} + assert_nil journals.detect {|journal| journal.journalized.project_id != 1} end def test_visible_scope_for_admin @@ -215,7 +215,7 @@ class JournalTest < ActiveSupport::TestCase journals = Journal.visible(user).to_a assert journals.any? # Admin should see issues on private projects that admin does not belong to - assert journals.detect {|journal| !journal.issue.project.is_public?} + assert journals.detect {|journal| !journal.project.is_public?} end def test_preload_journals_details_custom_fields_should_set_custom_field_instance_variable @@ -268,7 +268,7 @@ class JournalTest < ActiveSupport::TestCase visible_issue = Issue.generate! hidden_issue = Issue.generate!(:is_private => true) - journal = Journal.new + journal = Journal.new(journalized: issue) journal.details << JournalDetail.new(:property => 'relation', :prop_key => 'relates', :value => visible_issue.id) journal.details << JournalDetail.new(:property => 'relation', :prop_key => 'relates', :value => hidden_issue.id) diff --git a/test/unit/lib/redmine/acts/journalized_test.rb b/test/unit/lib/redmine/acts/journalized_test.rb new file mode 100644 index 000000000..c49fe3cda --- /dev/null +++ b/test/unit/lib/redmine/acts/journalized_test.rb @@ -0,0 +1,109 @@ +# frozen_string_literal: true + +# Redmine - project management software +# Copyright (C) 2006- 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_relative '../../../../test_helper' + +class Redmine::Acts::JournalizedTest < ActiveSupport::TestCase + def setup + User.current = nil + # Use News as a second journalized type + News.acts_as_journalized unless News.respond_to?(:journalized_options) + Redmine::Acts::Journalized.register('News') + @news = News.find(1) + end + + def teardown + Redmine::Acts::Journalized.unregister('News') + end + + def test_issue_should_be_registered + assert_include 'Issue', Redmine::Acts::Journalized.journalized_type_names + assert_include Issue, Redmine::Acts::Journalized.journalized_classes + end + + def test_acts_as_journalized_should_reject_invalid_options + assert_raise ArgumentError do + Class.new(ApplicationRecord) { acts_as_journalized :foo => 'bar' } + end + end + + def test_visible_should_include_journals_of_each_journalized_type + journal = Journal.create!(:journalized => @news, :user => User.find(2), :notes => 'News note') + journals = Journal.visible(User.anonymous) + + assert_include journal, journals + assert journals.where(:journalized_type => 'Issue').exists? + assert_equal Journal.where(:journalized_type => 'Issue').visible(User.anonymous).sort, + journals.where(:journalized_type => 'Issue').sort + end + + def test_visible_should_not_include_journals_of_invisible_records + journal = Journal.create!(:journalized => @news, :user => User.find(2), :notes => 'News note') + Role.anonymous.remove_permission!(:view_news) + + assert_not_include journal, Journal.visible(User.anonymous) + assert_not journal.visible?(User.anonymous) + end + + def test_visible_should_not_include_private_notes_without_permission + journal = Journal.create!(:journalized => @news, :user => User.find(2), :notes => 'Private', :private_notes => true) + + assert_not_include journal, Journal.visible(User.anonymous) + assert_include journal, Journal.visible(User.find(2)) + assert_include journal, Journal.visible(User.find(1)) + end + + def test_notified_users_should_exclude_users_not_allowed_to_view_private_notes + journal = Journal.new(:journalized => @news, :user => User.find(2), :notes => 'Private', :private_notes => true) + Role.find(1).remove_permission!(:view_private_notes) + + assert journal.notified_users.none? {|user| user.roles_for_project(@news.project).include?(Role.find(1)) && !user.admin?} + end + + def test_editable_by_should_be_false_without_configured_permission + journal = Journal.create!(:journalized => @news, :user => User.find(2), :notes => 'News note') + + assert_not journal.editable_by?(User.find(2)) + end + + def test_visible_should_join_journalized_table_when_restricted_to_one_type + sql = Journal.where(:journalized_type => 'Issue').visible(User.anonymous).to_sql + assert_include 'INNER JOIN issues', sql + assert_not_include 'EXISTS (SELECT 1 FROM "issues"', sql + end + + def test_visible_should_use_exists_conditions_for_several_types + sql = Journal.visible(User.anonymous).to_sql + assert_not_include 'INNER JOIN issues', sql + assert_include "journals.journalized_type = 'News' AND EXISTS", sql + end + + def test_visible_with_join_and_exists_conditions_should_return_the_same_issue_journals + user = User.find(2) + assert_equal Journal.where(:journalized_type => 'Issue').visible(user).ids.sort, + Journal.where(:journalized_type => 'Issue').where(Issue.journal_visible_condition(user)).ids.sort + end + + def test_joins_journalized_should_join_journalized_table_and_project + journals = Journal.joins_journalized(Issue).where(:issues => {:project_id => 1}) + + assert journals.any? + assert journals.all? {|journal| journal.journalized.is_a?(Issue) && journal.project.id == 1} + end +end diff --git a/test/unit/mail_handler_test.rb b/test/unit/mail_handler_test.rb index beb332e55..45f321eed 100644 --- a/test/unit/mail_handler_test.rb +++ b/test/unit/mail_handler_test.rb @@ -944,7 +944,7 @@ class MailHandlerTest < ActiveSupport::TestCase assert_equal Issue.find(2), journal.journalized assert_match /This is reply/, journal.notes assert_equal false, journal.private_notes - assert_equal 'Feature request', journal.issue.tracker.name + assert_equal 'Feature request', journal.journalized.tracker.name end def test_update_issue_should_accept_issue_id_after_space_inside_brackets @@ -977,11 +977,11 @@ class MailHandlerTest < ActiveSupport::TestCase 'start_date', 'due_date', 'float field']) assert journal.is_a?(Journal) - issue = Issue.find(journal.issue.id) + issue = Issue.find(journal.journalized.id) assert_equal User.find_by_login('jsmith'), journal.user assert_equal Issue.find(2), journal.journalized assert_match /This is reply/, journal.notes - assert_equal 'Feature request', journal.issue.tracker.name + assert_equal 'Feature request', journal.journalized.tracker.name assert_equal IssueStatus.find_by_name("Resolved"), issue.status assert_equal '2010-01-01', issue.start_date.to_s assert_equal '2010-12-31', issue.due_date.to_s @@ -1042,8 +1042,8 @@ class MailHandlerTest < ActiveSupport::TestCase ) assert journal.is_a?(Journal) assert_match /This is reply/, journal.notes - assert_equal 'Feature request', journal.issue.tracker.name - assert_equal 'Normal', journal.issue.priority.name + assert_equal 'Feature request', journal.journalized.tracker.name + assert_equal 'Normal', journal.journalized.priority.name end def test_update_issue_should_add_cc_as_watchers diff --git a/test/unit/mailer_test.rb b/test/unit/mailer_test.rb index ab880ef45..272404626 100644 --- a/test/unit/mailer_test.rb +++ b/test/unit/mailer_test.rb @@ -362,7 +362,7 @@ class MailerTest < ActiveSupport::TestCase def test_issue_edit_message_id journal = Journal.find(3) - journal.issue = Issue.find(2) + journal.journalized = Issue.find(2) Mailer.deliver_issue_edit(journal) mail = last_email