From 64d0f57d853cc4ca5bb5e3a5f0306587fdd71c1a Mon Sep 17 00:00:00 2001 From: ulferts Date: Mon, 27 Apr 2020 08:04:28 +0200 Subject: [PATCH] Feature/aggregated activities (#8221) * use cte for aggregated journal * Revert "use cte for aggregated journal" This reverts commit 5fedefefdd8339c3a054a3f0f0c5085a72471758. * add another subselect that could later on be provided from the outside * allow passing a nukleous sql to aggregated journals * wip - using aggregated journal for activity * new sql for aggregated journals * start implementing new aggregated query * additional documentation * consolidate activity functionality * simplify by turing into instance methods * move activity fetcher out of redmine * remove verb verification made obsolete Without catchall routes, the dispatching handles it * remove duplicate authorize check * refactor activities controller * refactory activity fetcher * cache avatar file * sort choosable events * remove legacy spec covered by contemporary * speed up aggregated journals via CTE * instance var might never have been set * ensure the event_types are always transmitted * correctly reset the avatar cache * fix avatar fetcher expectation regarding wiki pages * adapt spec [ci skip] --- app/controllers/activities_controller.rb | 110 +++--- app/controllers/statuses_controller.rb | 8 - .../time_entries/reports_controller.rb | 2 +- app/controllers/timelog_controller.rb | 2 +- app/controllers/users_controller.rb | 4 +- app/controllers/wiki_controller.rb | 5 - .../activities/base_activity_provider.rb | 243 ++++++++++-- .../activities/changeset_activity_provider.rb | 39 +- app/models/activities/event.rb | 16 + app/models/activities/fetcher.rb | 144 +++++++ .../activities/message_activity_provider.rb | 29 +- .../activities/news_activity_provider.rb | 21 +- .../time_entry_activity_provider.rb | 53 ++- .../wiki_content_activity_provider.rb | 21 +- .../work_package_activity_provider.rb | 41 +- app/models/journal/aggregated_journal.rb | 225 ++--------- .../journal/scopes/aggregated_journal.rb | 359 ++++++++++++++++++ app/models/journal/wiki_content_journal.rb | 1 + app/models/project.rb | 8 +- app/views/activities/index.html.erb | 29 +- app/views/common/feed.atom.builder | 28 +- .../constants/open_project}/activity.rb | 25 +- config/initializers/activity.rb | 3 +- lib/open_project.rb | 1 - lib/open_project/plugins/acts_as_op_engine.rb | 16 + lib/plugins/acts_as_activity_provider/init.rb | 31 -- .../lib/acts_as_activity_provider.rb | 236 ------------ .../acts_as_event/lib/acts_as_event.rb | 1 + lib/redmine/activity/fetcher.rb | 121 ------ lib/redmine/plugin.rb | 6 +- .../app/services/avatars/update_service.rb | 7 +- .../avatars/patches/avatar_helper_patch.rb | 6 +- .../avatars/patches/user_patch.rb | 33 +- .../app/controllers/costlog_controller.rb | 1 - .../cost_object_activity_provider.rb | 18 +- .../costs/lib/open_project/costs/engine.rb | 6 +- .../costs/spec/features/time_entries_spec.rb | 28 +- .../activities/document_activity_provider.rb | 18 +- .../lib/open_project/documents/engine.rb | 6 +- .../activities/meeting_activity_provider.rb | 68 ++-- .../lib/open_project/meeting/engine.rb | 7 +- .../controllers/cost_reports_controller.rb | 3 - .../controllers/activities_controller_spec.rb | 103 +++-- spec/factories/changeset_factory.rb | 2 +- .../activities/fetcher_integration_spec.rb | 264 +++++++++++++ .../work_package_activity_provider_spec.rb | 135 +++++-- .../models/journal/aggregated_journal_spec.rb | 87 ++++- spec/models/repository/git_spec.rb | 4 +- spec/models/repository/subversion_spec.rb | 4 +- .../functional/activities_controller_spec.rb | 147 ------- spec_legacy/unit/activity_spec.rb | 107 ------ 51 files changed, 1627 insertions(+), 1255 deletions(-) create mode 100644 app/models/activities/event.rb create mode 100644 app/models/activities/fetcher.rb create mode 100644 app/models/journal/scopes/aggregated_journal.rb rename {lib/redmine => config/constants/open_project}/activity.rb (76%) delete mode 100644 lib/plugins/acts_as_activity_provider/init.rb delete mode 100644 lib/plugins/acts_as_activity_provider/lib/acts_as_activity_provider.rb delete mode 100644 lib/redmine/activity/fetcher.rb create mode 100644 spec/models/activities/fetcher_integration_spec.rb delete mode 100644 spec_legacy/functional/activities_controller_spec.rb delete mode 100644 spec_legacy/unit/activity_spec.rb diff --git a/app/controllers/activities_controller.rb b/app/controllers/activities_controller.rb index cb95cb8dda..f439983ceb 100644 --- a/app/controllers/activities_controller.rb +++ b/app/controllers/activities_controller.rb @@ -1,4 +1,5 @@ #-- encoding: UTF-8 + #-- copyright # OpenProject is an open source project management software. # Copyright (C) 2012-2020 the OpenProject GmbH @@ -29,46 +30,33 @@ class ActivitiesController < ApplicationController menu_item :activity - before_action :find_optional_project, :verify_activities_module_activated - accept_key_auth :index + before_action :find_optional_project, + :verify_activities_module_activated, + :determine_date_range, + :determine_subprojects, + :determine_author - def index - @days = Setting.activity_days_default.to_i + after_action :set_session - if params[:from] - begin; @date_to = params[:from].to_date + 1.day; rescue; end - end - - @date_to ||= User.current.today + 1.day - @date_from = @date_to - @days - @with_subprojects = params[:with_subprojects].nil? ? Setting.display_subprojects_work_packages? : (params[:with_subprojects] == '1') - @author = (params[:user_id].blank? ? nil : User.active.find(params[:user_id])) - - @activity = Redmine::Activity::Fetcher.new(User.current, project: @project, - with_subprojects: @with_subprojects, - author: @author) + accept_key_auth :index - set_activity_scope + def index + @activity = Activities::Fetcher.new(User.current, + project: @project, + with_subprojects: @with_subprojects, + author: @author, + scope: activity_scope) events = @activity.events(@date_from, @date_to) - censor_events_from_projects_with_disabled_activity!(events) unless @project respond_to do |format| format.html do - @events_by_day = events.group_by { |e| e.event_datetime.in_time_zone(User.current.time_zone).to_date } - render layout: false if request.xhr? + respond_html(events) end format.atom do - title = l(:label_activity) - if @author - title = @author.name - elsif @activity.scope.size == 1 - title = l("label_#{@activity.scope.first.singularize}_plural") - end - render_feed(events, title: "#{@project || Setting.app_title}: #{title}") + respond_atom(events) end end - rescue ActiveRecord::RecordNotFound => e op_handle_warning "Failed to find all resources in activities: #{e.message}" render_404 I18n.t(:error_can_not_find_all_resources) @@ -76,39 +64,61 @@ class ActivitiesController < ApplicationController private - # TODO: this should now be functionally identical to the implementation in application_controller - # double check and remove - def find_optional_project - return true unless params[:project_id] - @project = Project.find(params[:project_id]) - authorize - rescue ActiveRecord::RecordNotFound - render_404 - end - def verify_activities_module_activated render_403 if @project && !@project.module_enabled?('activity') end - # Do not show events, which are associated with projects where activities are disabled. - # In a better world this would be implemented (with better performance) in SQL. - # TODO: make the world a better place. - def censor_events_from_projects_with_disabled_activity!(events) - allowed_project_ids = EnabledModule.where(name: 'activity').map(&:project_id) - events.select! do |event| - event.project_id.nil? || allowed_project_ids.include?(event.project_id) + def determine_date_range + @days = Setting.activity_days_default.to_i + + if params[:from] + begin; @date_to = params[:from].to_date + 1.day; rescue; end + end + + @date_to ||= User.current.today + 1.day + @date_from = @date_to - @days + end + + def determine_subprojects + @with_subprojects = if params[:with_subprojects].nil? + Setting.display_subprojects_work_packages? + else + params[:with_subprojects] == '1' + end + end + + def determine_author + @author = params[:user_id].blank? ? nil : User.active.find(params[:user_id]) + end + + def respond_html(events) + @events_by_day = events.group_by { |e| e.event_datetime.in_time_zone(User.current.time_zone).to_date } + render layout: !request.xhr? + end + + def respond_atom(events) + title = t(:label_activity) + if @author + title = @author.name + elsif @activity.scope.size == 1 + title = t("label_#{@activity.scope.first.singularize}_plural") end + render_feed(events, title: "#{@project || Setting.app_title}: #{title}") end - def set_activity_scope - if params[:apply] - @activity.scope_select { |t| !params["show_#{t}"].nil? } + def activity_scope + if params[:event_types] + params[:event_types] elsif session[:activity] - @activity.scope = session[:activity] + session[:activity] + elsif @author.nil? + :default else - @activity.scope = (@author.nil? ? :default : :all) + :all end + end + def set_session session[:activity] = @activity.scope end end diff --git a/app/controllers/statuses_controller.rb b/app/controllers/statuses_controller.rb index 92fee7141a..dd5d1bad5c 100644 --- a/app/controllers/statuses_controller.rb +++ b/app/controllers/statuses_controller.rb @@ -34,7 +34,6 @@ class StatusesController < ApplicationController before_action :require_admin - verify method: :get, only: :index, render: { nothing: true, status: :method_not_allowed } def index @statuses = Status.page(page_param) .per_page(per_page_param) @@ -42,12 +41,10 @@ class StatusesController < ApplicationController render action: 'index', layout: false if request.xhr? end - verify method: :get, only: :new, render: { nothing: true, status: :method_not_allowed } def new @status = Status.new end - verify method: :post, only: :create, render: { nothing: true, status: :method_not_allowed } def create @status = Status.new(permitted_params.status) if @status.save @@ -58,12 +55,10 @@ class StatusesController < ApplicationController end end - verify method: :get, only: :edit, render: { nothing: true, status: :method_not_allowed } def edit @status = Status.find(params[:id]) end - verify method: :patch, only: :update, render: { nothing: true, status: :method_not_allowed } def update @status = Status.find(params[:id]) if @status.update(permitted_params.status) @@ -74,7 +69,6 @@ class StatusesController < ApplicationController end end - verify method: :delete, only: :destroy, render: { nothing: true, status: :method_not_allowed } def destroy status = Status.find(params[:id]) if status.is_default? @@ -89,8 +83,6 @@ class StatusesController < ApplicationController redirect_to action: 'index' end - verify method: :post, only: :update_work_package_done_ratio, - render: { nothing: true, status: 405 } def update_work_package_done_ratio if Status.update_work_package_done_ratios flash[:notice] = l(:notice_work_package_done_ratios_updated) diff --git a/app/controllers/time_entries/reports_controller.rb b/app/controllers/time_entries/reports_controller.rb index 576b939c2d..5113cacc10 100644 --- a/app/controllers/time_entries/reports_controller.rb +++ b/app/controllers/time_entries/reports_controller.rb @@ -193,7 +193,7 @@ class TimeEntries::ReportsController < ApplicationController if @project.nil? project_context_sql_condition elsif @issue.nil? - @project.project_condition(Setting.display_subprojects_work_packages?) + @project.project_condition(Setting.display_subprojects_work_packages?).to_sql else WorkPackage.self_and_descendants_of_condition(@issue) end diff --git a/app/controllers/timelog_controller.rb b/app/controllers/timelog_controller.rb index d1a3f0da14..cb3cc3fda4 100644 --- a/app/controllers/timelog_controller.rb +++ b/app/controllers/timelog_controller.rb @@ -62,7 +62,7 @@ class TimelogController < ApplicationController if @issue cond << WorkPackage.self_and_descendants_of_condition(@issue) elsif @project - cond << @project.project_condition(Setting.display_subprojects_work_packages?) + cond << @project.project_condition(Setting.display_subprojects_work_packages?).to_sql end retrieve_date_range allow_nil: true diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 43c47d4dec..265eb40696 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -76,7 +76,7 @@ class UsersController < ApplicationController @memberships = @user.memberships .visible(current_user) - events = Redmine::Activity::Fetcher.new(User.current, author: @user).events(nil, nil, limit: 10) + events = Activities::Fetcher.new(User.current, author: @user).events(nil, nil, limit: 10) @events_by_day = events.group_by { |e| e.event_datetime.to_date } unless User.current.admin? @@ -99,7 +99,6 @@ class UsersController < ApplicationController @auth_sources = AuthSource.all end - verify method: :post, only: :create, render: { nothing: true, status: :method_not_allowed } def create @user = User.new(language: Setting.default_language, mail_notification: Setting.default_notification_option) @@ -128,7 +127,6 @@ class UsersController < ApplicationController @membership ||= Member.new end - verify method: :put, only: :update, render: { nothing: true, status: :method_not_allowed } def update @user.attributes = permitted_params.user_update_as_admin(@user.uses_external_authentication?, @user.change_password_allowed?) diff --git a/app/controllers/wiki_controller.rb b/app/controllers/wiki_controller.rb index 75085ca0fc..d5cd5db65d 100644 --- a/app/controllers/wiki_controller.rb +++ b/app/controllers/wiki_controller.rb @@ -55,10 +55,6 @@ class WikiController < ApplicationController destroy] before_action :build_wiki_page_and_content, only: %i[new create] - verify method: :post, only: [:protect], redirect_to: { action: :show } - verify method: :get, only: %i[new new_child], render: { nothing: true, status: :method_not_allowed } - verify method: :post, only: :create, render: { nothing: true, status: :method_not_allowed } - include AttachmentsHelper include PaginationHelper include Redmine::MenuManager::WikiMenuHelper @@ -298,7 +294,6 @@ class WikiController < ApplicationController render_404 unless @annotate end - verify method: :delete, only: [:destroy], redirect_to: { action: :show } # Removes a wiki page and its history # Children can be either set as root pages, removed or reassigned to another parent page def destroy diff --git a/app/models/activities/base_activity_provider.rb b/app/models/activities/base_activity_provider.rb index 31b9b6d4cd..00b41db8ca 100644 --- a/app/models/activities/base_activity_provider.rb +++ b/app/models/activities/base_activity_provider.rb @@ -1,4 +1,5 @@ #-- encoding: UTF-8 + #-- copyright # OpenProject is an open source project management software. # Copyright (C) 2012-2020 the OpenProject GmbH @@ -42,17 +43,64 @@ # See the comments on the methods to get additional information. # ############################################################################### class Activities::BaseActivityProvider - include Redmine::Acts::ActivityProvider include I18n include Redmine::I18n include OpenProject::StaticRouting + class_attribute :activity_provider_options + + # Returns events of type event_type visible by user that occured between from and to + def self.find_events(event_type, user, from, to, options) + raise "#{name} can not provide #{event_type} events." if activity_provider_options[:type] != event_type + + activity_provider_options[:activities] + .map { |activity| new(activity).find_events(user, from, to, options) } + .flatten + end + + def initialize(activity) + self.activity = activity + end + + def self.activity_provider_for(options = {}) + options.assert_valid_keys(:type, :permission, :activities, :aggregated) + + self.activity_provider_options = { + type: name.underscore.pluralize, + activities: [:activity], + aggregated: false, + permission: "view_#{name.underscore.pluralize}".to_sym + }.merge(options) + end + + def find_events(user, from, to, options) + query = if aggregated? + aggregated_event_selection_query(user, from, to, options) + else + event_selection_query(user, from, to, options) + end + + query = apply_order(query) + query = apply_limit(query, options) + query = apply_event_projection(query) + fill_events(query) + end + + def fill_events(events_query) + ActiveRecord::Base.connection.select_all(events_query.to_sql).map do |e| + params = event_params(e) + + Activities::Event.new(**params) if params + end + end + ############################################################################# # Activities may need information not available in the journal table. Thus, # # if you need further information from different tables (e.g., projects # # table) you may extend the query in this method. # ############################################################################# - def extend_event_query(_query, _activity) + def extend_event_query(query) + query end ############################################################################# @@ -61,90 +109,207 @@ class Activities::BaseActivityProvider # You must at least return the column containing the project reference with # # the alias 'project_id'. # ############################################################################# - def event_query_projection(_activity) + def event_query_projection [] end + def event_datetime(event) + event['event_datetime'].is_a?(String) ? DateTime.parse(event['event_datetime']) : event['event_datetime'] + end + + def event_type(_event_data) + activity_provider_options[:type] + end + ############################################################################# # Override this method if the journal table does not contain a reference to # # the 'projects' table. # ############################################################################# - def projects_reference_table(activity) - activity_journals_table(activity) + def projects_reference_table + activity_journals_table end - def filter_for_event_datetime(query, journals_table, typed_journals_table, from, to) - if from - query = query.where(journals_table[:created_at].gteq(from)) - end + def activitied_type + activity_type = self.class.name + + class_name = activity_type.demodulize + class_name.gsub('ActivityProvider', '').constantize + end + + protected - if to - query = query.where(journals_table[:created_at].lteq(to)) + def event_selection_query(user, from, to, options) + query = journals_with_data_query + query = extend_event_query(query) unless aggregated? + query = filter_for_event_datetime(query, from, to) + query = restrict_user(query, options) + restrict_projects(query, user, options) + end + + def apply_event_projection(query) + projection = event_projection + projection << event_query_projection if respond_to?(:event_query_projection) + + query.project(projection) + end + + # When aggregating, we add the query that actually fetches the journals, + # restricted by from, to, user permission, etc. as a CTE. That way, + # that query has only to be executed once inside the aggregated journal query which + # considerably reduces execution time. + def aggregated_event_selection_query(user, from, to, options) + query = aggregated_journal_query + query = add_event_selection_query_as_cte(query, user, from, to, options) + query = join_activity_journals_table(query) + query = extend_event_query(query) + join_with_projects_table(query) + end + + def apply_limit(query, options) + if options[:limit] + query.take(options[:limit]) + else + query end + end + + def filter_for_event_datetime(query, from, to) + query = query.where(journals_table[:created_at].gteq(from)) if from + query = query.where(journals_table[:created_at].lteq(to)) if to query end - def activity_journals_table(_activity) - @activity_journals_table ||= JournalManager.journal_class(activitied_type).arel_table + def apply_order(query) + query.order(journals_table[:id].desc) end - def activitied_type(_activity = nil) - activity_type = self.class.name + def event_params(event_data) + params = { provider: self, + event_description: event_data['event_description'], + author_id: event_data['event_author'].to_i, + journable_id: event_data['journable_id'], + project_id: event_data['project_id'].to_i } - class_name = activity_type.demodulize - class_name.gsub('ActivityProvider', '').constantize + %i[event_name event_title event_type event_description event_datetime event_path event_url].each do |a| + params[a] = send(a, event_data) if self.class.method_defined? a + end + + params + rescue StandardError => e + Rails.logger.error "Failed to deduce event params for #{event_data.inspect}: #{e}" end - def format_event(event, event_data, activity) - [:event_name, :event_title, :event_type, :event_description, :event_datetime, :event_path, :event_url].each do |a| - event[a] = send(a, event_data, activity) if self.class.method_defined? a + def event_projection + [[:id, 'event_id'], + [:created_at, 'event_datetime'], + [:user_id, 'event_author'], + [:notes, 'event_description'], + [:version, 'version'], + [:journable_id, 'journable_id']].map do |column, alias_name| + journals_table[column].as(alias_name) end + end - event + def join_with_projects_table(query) + query.join(projects_table).on(projects_table[:id].eq(projects_reference_table['project_id'])) end - protected + def restrict_user(query, options) + query = query.where(journals_table[:user_id].eq(options[:author].id)) if options[:author] + query + end - def journal_table - @journal_table ||= Journal.arel_table + def restrict_projects(query, user, options) + query = join_with_projects_table(query) + query = restrict_projects_by_selection(options, query) + query = restrict_projects_by_activity_module(query) + restrict_projects_by_permission(query, user) end - def activitied_table - @activitied_table ||= activitied_type.arel_table + def restrict_projects_by_selection(options, query) + if (project = options[:project]) + query = query.where(project.project_condition(options[:with_subprojects])) + end + + query + end + + def restrict_projects_by_activity_module(query) + # Have to use the string based where here as the resulting + # sql would otherwise expect a parameter for the prepared statement. + query.where(projects_table[:id].in(EnabledModule.where("name = 'activity'").select(:project_id).arel)) + end + + def restrict_projects_by_permission(query, user) + perm = activity_provider_options[:permission] + + query.where(projects_table[:id].in(Project.allowed_to(user, perm).select(:id).arel)) + end + + def aggregated_journal_query + # As AggregatedJournal wraps the provided sql statement inside brackets we + # need to provide a fully valid statement and not only the alias string. + Journal::Scopes::AggregatedJournal.fetch(sql: "SELECT * FROM #{aggregated_journals_alias}").arel + end + + def add_event_selection_query_as_cte(query, user, from, to, options) + cte_query = event_selection_query(user, from, to, options).project('journals.*') + cte = Arel::Nodes::As.new(Arel::Table.new(aggregated_journals_alias), cte_query) + + query.with(cte) end - def work_packages_table - @work_packages_table ||= WorkPackage.arel_table + attr_accessor :activity + + def aggregated? + activity_provider_options[:aggregated] + end + + def journals_with_data_query + join_activity_journals_table(journals_table) + .where(journals_table[:journable_type].eq(activitied_type.name)) + end + + def join_activity_journals_table(query) + query + .join(activity_journals_table).on(journals_table[:id].eq(activity_journals_table[:journal_id])) + end + + def journals_table + Journal.arel_table + end + + def activitied_table + @activitied_table ||= activitied_type.arel_table end def projects_table @projects_table ||= Project.arel_table end - def types_table - @types_table = Type.arel_table + def enabled_modules_table + @enabled_modules_table ||= EnabledModule.arel_table end - def statuses_table - @statuses_table = Status.arel_table + def activity_journals_table + @activity_journals_table ||= JournalManager.journal_class(activitied_type).arel_table end - def activity_journal_projection_statement(column, name, activity) - projection_statement(activity_journals_table(activity), column, name) + def activity_journal_projection_statement(column, name) + projection_statement(activity_journals_table, column, name) end def projection_statement(table, column, name) table[column].as(name) end - class UndefinedEventTypeError < StandardError; end - def event_type(_event, _activity) - raise UndefinedEventTypeError.new('Abstract method event_type called') + def event_name(event) + I18n.t(event_type(event).underscore, scope: 'events') end - def event_name(event, activity) - I18n.t(event_type(event, activity).underscore, scope: 'events') + def aggregated_journals_alias + :relevant_journals end def url_helpers diff --git a/app/models/activities/changeset_activity_provider.rb b/app/models/activities/changeset_activity_provider.rb index 76dc19f79b..57471847c0 100644 --- a/app/models/activities/changeset_activity_provider.rb +++ b/app/models/activities/changeset_activity_provider.rb @@ -1,4 +1,5 @@ #-- encoding: UTF-8 + #-- copyright # OpenProject is an open source project management software. # Copyright (C) 2012-2020 the OpenProject GmbH @@ -28,37 +29,37 @@ #++ class Activities::ChangesetActivityProvider < Activities::BaseActivityProvider - acts_as_activity_provider type: 'changesets', - permission: :view_changesets + activity_provider_for type: 'changesets', + permission: :view_changesets - def extend_event_query(query, activity) - query.join(repositories_table).on(activity_journals_table(activity)[:repository_id].eq(repositories_table[:id])) + def extend_event_query(query) + query.join(repositories_table).on(activity_journals_table[:repository_id].eq(repositories_table[:id])) end - def event_query_projection(activity) + def event_query_projection [ - activity_journal_projection_statement(:revision, 'revision', activity), - activity_journal_projection_statement(:comments, 'comments', activity), - activity_journal_projection_statement(:committed_on, 'committed_on', activity), + activity_journal_projection_statement(:revision, 'revision'), + activity_journal_projection_statement(:comments, 'comments'), + activity_journal_projection_statement(:committed_on, 'committed_on'), projection_statement(repositories_table, :project_id, 'project_id'), projection_statement(repositories_table, :type, 'repository_type') ] end - def projects_reference_table(_activity) + def projects_reference_table repositories_table end ## # Override this method if not the journal created_at datetime, but another column # value is the actual relevant time event. (e..g., commit date) - def filter_for_event_datetime(query, journals_table, typed_journals_table, from, to) + def filter_for_event_datetime(query, from, to) if from - query = query.where(typed_journals_table[:committed_on].gteq(from)) + query = query.where(activity_journals_table[:committed_on].gteq(from)) end if to - query = query.where(typed_journals_table[:committed_on].lteq(to)) + query = query.where(activity_journals_table[:committed_on].lteq(to)) end query @@ -66,11 +67,11 @@ class Activities::ChangesetActivityProvider < Activities::BaseActivityProvider protected - def event_type(_event, _activity) + def event_type(_event) 'changeset' end - def event_title(event, _activity) + def event_title(event) revision = format_revision(event) short_comment = split_comment(event['comments']).first @@ -79,20 +80,20 @@ class Activities::ChangesetActivityProvider < Activities::BaseActivityProvider title << (short_comment.blank? ? '' : (': ' + short_comment)) end - def event_description(event, _activity) + def event_description(event) split_comment(event['comments']).last end - def event_datetime(event, _activity) + def event_datetime(event) committed_on = event['committed_on'] - committed_date = committed_on.is_a?(String) ? DateTime.parse(committed_on) : committed_on + committed_on.is_a?(String) ? DateTime.parse(committed_on) : committed_on end - def event_path(event, _activity) + def event_path(event) url_helpers.revisions_project_repository_path(url_helper_parameter(event)) end - def event_url(event, _activity) + def event_url(event) url_helpers.revisions_project_repository_url(url_helper_parameter(event)) end diff --git a/app/models/activities/event.rb b/app/models/activities/event.rb new file mode 100644 index 0000000000..0d050c2c5a --- /dev/null +++ b/app/models/activities/event.rb @@ -0,0 +1,16 @@ +module Activities + Event = Struct.new(:provider, + :event_name, + :event_title, + :event_description, + :author_id, + :event_author, + :event_datetime, + :journable_id, + :project_id, + :project, + :event_type, + :event_path, + :event_url, + keyword_init: true) +end diff --git a/app/models/activities/fetcher.rb b/app/models/activities/fetcher.rb new file mode 100644 index 0000000000..7b0c4e06d4 --- /dev/null +++ b/app/models/activities/fetcher.rb @@ -0,0 +1,144 @@ +#-- encoding: UTF-8 + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2012-2020 the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2017 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# 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. +# +# See docs/COPYRIGHT.rdoc for more details. +#++ + +module Activities + # Class used to retrieve activity events + class Fetcher + attr_reader :user, :project, :scope + + def self.constantized_providers + @constantized_providers ||= Hash.new { |h, k| h[k] = OpenProject::Activity.providers[k].map(&:constantize) } + end + + def initialize(user, options = {}) + options.assert_valid_keys(:project, :with_subprojects, :author, :scope) + @user = user + @project = options[:project] + @options = options + + self.scope = options[:scope] || :all + end + + # Returns an array of available event types + def event_types + @event_types ||= begin + if @project + OpenProject::Activity.available_event_types.select do |o| + @project.self_and_descendants.detect do |_p| + permissions = constantized_providers(o).map do |p| + p.activity_provider_options[:permission] + end.compact + + permissions.all? { |p| @user.allowed_to?(p, @project) } + end + end + else + OpenProject::Activity.available_event_types + end + end + end + + # Returns an array of events for the given date range + # sorted in reverse chronological order + def events(from = nil, to = nil, limit: nil) + events = events_from_providers(from, to, limit) + + eager_load_associations(events) + + sort_by_date(events) + end + + protected + + # Sets the scope + # Argument can be :all, :default or an array of event types + def scope=(scope) + case scope + when :all + @scope = event_types + when :default + default_scope! + else + @scope = scope & event_types + end + end + + # Resets the scope to the default scope + def default_scope! + @scope = OpenProject::Activity.default_event_types + end + + def events_from_providers(from, to, limit) + events = [] + + @scope.each do |event_type| + constantized_providers(event_type).each do |provider| + events += provider.find_events(event_type, @user, from, to, @options.merge(limit: limit)) + end + end + + events + end + + def eager_load_associations(events) + projects = projects_of_event_set(events) + users = users_of_event_set(events) + + events.each do |e| + e.event_author = users[e.author_id]&.first + e.project = projects[e.project_id]&.first + end + end + + def projects_of_event_set(events) + project_ids = events.map(&:project_id).compact.uniq + + if project_ids.any? + Project.find(project_ids).group_by(&:id) + else + {} + end + end + + def users_of_event_set(events) + user_ids = events.map(&:author_id).compact.uniq + + User.where(id: user_ids).group_by(&:id) + end + + def sort_by_date(events) + events.sort { |a, b| b.event_datetime <=> a.event_datetime } + end + + def constantized_providers(event_type) + self.class.constantized_providers[event_type] + end + end +end diff --git a/app/models/activities/message_activity_provider.rb b/app/models/activities/message_activity_provider.rb index 5525864f03..f15c3fd1df 100644 --- a/app/models/activities/message_activity_provider.rb +++ b/app/models/activities/message_activity_provider.rb @@ -1,4 +1,5 @@ #-- encoding: UTF-8 + #-- copyright # OpenProject is an open source project management software. # Copyright (C) 2012-2020 the OpenProject GmbH @@ -28,47 +29,47 @@ #++ class Activities::MessageActivityProvider < Activities::BaseActivityProvider - acts_as_activity_provider type: 'messages', - permission: :view_messages + activity_provider_for type: 'messages', + permission: :view_messages - def extend_event_query(query, activity) - query.join(forums_table).on(activity_journals_table(activity)[:forum_id].eq(forums_table[:id])) + def extend_event_query(query) + query.join(forums_table).on(activity_journals_table[:forum_id].eq(forums_table[:id])) end - def event_query_projection(activity) + def event_query_projection [ - activity_journal_projection_statement(:subject, 'message_subject', activity), - activity_journal_projection_statement(:content, 'message_content', activity), - activity_journal_projection_statement(:parent_id, 'message_parent_id', activity), + activity_journal_projection_statement(:subject, 'message_subject'), + activity_journal_projection_statement(:content, 'message_content'), + activity_journal_projection_statement(:parent_id, 'message_parent_id'), projection_statement(forums_table, :id, 'forum_id'), projection_statement(forums_table, :name, 'forum_name'), projection_statement(forums_table, :project_id, 'project_id') ] end - def projects_reference_table(_activity) + def projects_reference_table forums_table end protected - def event_title(event, _activity) + def event_title(event) "#{event['forum_name']}: #{event['message_subject']}" end - def event_description(event, _activity) + def event_description(event) event['message_content'] end - def event_type(event, _activity) + def event_type(event) event['parent_id'].blank? ? 'message' : 'reply' end - def event_path(event, _activity) + def event_path(event) url_helpers.topic_path(*url_helper_parameter(event)) end - def event_url(event, _activity) + def event_url(event) url_helpers.topic_url(*url_helper_parameter(event)) end diff --git a/app/models/activities/news_activity_provider.rb b/app/models/activities/news_activity_provider.rb index 7b85ca3c7c..2d5bd14e32 100644 --- a/app/models/activities/news_activity_provider.rb +++ b/app/models/activities/news_activity_provider.rb @@ -28,34 +28,31 @@ #++ class Activities::NewsActivityProvider < Activities::BaseActivityProvider - acts_as_activity_provider type: 'news', - permission: :view_news + activity_provider_for type: 'news', + permission: :view_news - def extend_event_query(_query, _activity) - end - - def event_query_projection(activity) + def event_query_projection [ - activity_journal_projection_statement(:title, 'title', activity), - activity_journal_projection_statement(:project_id, 'project_id', activity) + activity_journal_projection_statement(:title, 'title'), + activity_journal_projection_statement(:project_id, 'project_id') ] end protected - def event_title(event, _activity) + def event_title(event) event['title'] end - def event_type(_event, _activity) + def event_type(_event) 'news' end - def event_path(event, _activity) + def event_path(event) url_helpers.news_path(url_helper_parameter(event)) end - def event_url(event, _activity) + def event_url(event) url_helpers.news_url(url_helper_parameter(event)) end diff --git a/app/models/activities/time_entry_activity_provider.rb b/app/models/activities/time_entry_activity_provider.rb index 13fd62c4d1..95d9ed5e53 100644 --- a/app/models/activities/time_entry_activity_provider.rb +++ b/app/models/activities/time_entry_activity_provider.rb @@ -1,4 +1,5 @@ #-- encoding: UTF-8 + #-- copyright # OpenProject is an open source project management software. # Copyright (C) 2012-2020 the OpenProject GmbH @@ -28,21 +29,21 @@ #++ class Activities::TimeEntryActivityProvider < Activities::BaseActivityProvider - acts_as_activity_provider type: 'time_entries', - permission: :view_time_entries + activity_provider_for type: 'time_entries', + permission: :view_time_entries - def extend_event_query(query, activity) - query.join(work_packages_table).on(activity_journals_table(activity)[:work_package_id].eq(work_packages_table[:id])) + def extend_event_query(query) + query.join(work_packages_table).on(activity_journals_table[:work_package_id].eq(work_packages_table[:id])) query.join(types_table).on(work_packages_table[:type_id].eq(types_table[:id])) query.join(statuses_table).on(work_packages_table[:status_id].eq(statuses_table[:id])) end - def event_query_projection(activity) + def event_query_projection [ - activity_journal_projection_statement(:hours, 'time_entry_hours', activity), - activity_journal_projection_statement(:comments, 'time_entry_comments', activity), - activity_journal_projection_statement(:project_id, 'project_id', activity), - activity_journal_projection_statement(:work_package_id, 'work_package_id', activity), + activity_journal_projection_statement(:hours, 'time_entry_hours'), + activity_journal_projection_statement(:comments, 'time_entry_comments'), + activity_journal_projection_statement(:project_id, 'project_id'), + activity_journal_projection_statement(:work_package_id, 'work_package_id'), projection_statement(projects_table, :name, 'project_name'), projection_statement(work_packages_table, :subject, 'work_package_subject'), projection_statement(statuses_table, :name, 'status_name'), @@ -53,12 +54,12 @@ class Activities::TimeEntryActivityProvider < Activities::BaseActivityProvider protected - def event_title(event, _activity) + def event_title(event) time_entry_object_name = event['work_package_id'].blank? ? event['project_name'] : work_package_title(event) "#{l_hours(event['time_entry_hours'])} (#{time_entry_object_name})" end - def event_type(_event, _activity) + def event_type(_event) 'time-entry' end @@ -70,23 +71,35 @@ class Activities::TimeEntryActivityProvider < Activities::BaseActivityProvider event['is_standard']) end - def event_description(event, _activity) + def event_description(event) event['time_entry_description'] end - def event_path(event, _activity) - unless event['work_package_id'].blank? - url_helpers.work_package_time_entries_path(event['work_package_id']) - else + def event_path(event) + if event['work_package_id'].present? url_helpers.project_time_entries_path(event['project_id']) + else + url_helpers.work_package_time_entries_path(event['work_package_id']) end end - def event_url(event, _activity) - unless event['work_package_id'].blank? - url_helpers.work_package_time_entries_url(event['work_package_id']) - else + def event_url(event) + if event['work_package_id'].present? url_helpers.project_time_entries_url(event['project_id']) + else + url_helpers.work_package_time_entries_url(event['work_package_id']) end end + + def types_table + @types_table = Type.arel_table + end + + def statuses_table + @statuses_table = Status.arel_table + end + + def work_packages_table + @work_packages_table ||= WorkPackage.arel_table + end end diff --git a/app/models/activities/wiki_content_activity_provider.rb b/app/models/activities/wiki_content_activity_provider.rb index 8ea32fa8dd..79c3f4d017 100644 --- a/app/models/activities/wiki_content_activity_provider.rb +++ b/app/models/activities/wiki_content_activity_provider.rb @@ -1,4 +1,5 @@ #-- encoding: UTF-8 + #-- copyright # OpenProject is an open source project management software. # Copyright (C) 2012-2020 the OpenProject GmbH @@ -28,40 +29,40 @@ #++ class Activities::WikiContentActivityProvider < Activities::BaseActivityProvider - acts_as_activity_provider type: 'wiki_edits', - permission: :view_wiki_edits + activity_provider_for type: 'wiki_edits', + permission: :view_wiki_edits - def extend_event_query(query, activity) - query.join(wiki_pages_table).on(activity_journals_table(activity)[:page_id].eq(wiki_pages_table[:id])) + def extend_event_query(query) + query.join(wiki_pages_table).on(activity_journals_table[:page_id].eq(wiki_pages_table[:id])) query.join(wikis_table).on(wiki_pages_table[:wiki_id].eq(wikis_table[:id])) end - def event_query_projection(_activity) + def event_query_projection [ projection_statement(wikis_table, :project_id, 'project_id'), projection_statement(wiki_pages_table, :title, 'wiki_title') ] end - def projects_reference_table(_activity) + def projects_reference_table wikis_table end protected - def event_title(event, _activity) + def event_title(event) "#{l(:label_wiki_edit)}: #{event['wiki_title']} (##{event['version']})" end - def event_type(_event, _activity) + def event_type(_event) 'wiki-page' end - def event_path(event, _activity) + def event_path(event) url_helpers.project_wiki_path(*url_helper_parameter(event)) end - def event_url(event, _activity) + def event_url(event) url_helpers.project_wiki_url(*url_helper_parameter(event)) end diff --git a/app/models/activities/work_package_activity_provider.rb b/app/models/activities/work_package_activity_provider.rb index 459c40aef5..d98eaf4613 100644 --- a/app/models/activities/work_package_activity_provider.rb +++ b/app/models/activities/work_package_activity_provider.rb @@ -28,18 +28,19 @@ #++ class Activities::WorkPackageActivityProvider < Activities::BaseActivityProvider - acts_as_activity_provider type: 'work_packages', - permission: :view_work_packages + activity_provider_for type: 'work_packages', + aggregated: true, + permission: :view_work_packages - def extend_event_query(query, activity) - query.join(types_table).on(activity_journals_table(activity)[:type_id].eq(types_table[:id])) - query.join(statuses_table).on(activity_journals_table(activity)[:status_id].eq(statuses_table[:id])) + def extend_event_query(query) + query.join(types_table).on(activity_journals_table[:type_id].eq(types_table[:id])) + query.join(statuses_table).on(activity_journals_table[:status_id].eq(statuses_table[:id])) end - def event_query_projection(activity) + def event_query_projection [ - activity_journal_projection_statement(:subject, 'subject', activity), - activity_journal_projection_statement(:project_id, 'project_id', activity), + activity_journal_projection_statement(:subject, 'subject'), + activity_journal_projection_statement(:project_id, 'project_id'), projection_statement(statuses_table, :name, 'status_name'), projection_statement(statuses_table, :is_closed, 'status_closed'), projection_statement(types_table, :name, 'type_name') @@ -47,13 +48,13 @@ class Activities::WorkPackageActivityProvider < Activities::BaseActivityProvider end def self.work_package_title(id, subject, type_name, status_name, is_standard) - title = "#{(is_standard) ? '' : "#{type_name}"} ##{id}: #{subject}" + title = "#{is_standard ? '' : "#{type_name}"} ##{id}: #{subject}" title << " (#{status_name})" unless status_name.blank? end protected - def event_title(event, _activity) + def event_title(event) self.class.work_package_title(event['journable_id'], event['subject'], event['type_name'], @@ -61,17 +62,17 @@ class Activities::WorkPackageActivityProvider < Activities::BaseActivityProvider event['is_standard']) end - def event_type(event, _activity) + def event_type(event) state = ActiveRecord::Type::Boolean.new.cast(event['status_closed']) ? '-closed' : '-edit' "work_package#{state}" end - def event_path(event, _activity) + def event_path(event) url_helpers.work_package_path(event['journable_id']) end - def event_url(event, _activity) + def event_url(event) url_helpers.work_package_url(event['journable_id'], anchor: notes_anchor(event)) end @@ -81,6 +82,18 @@ class Activities::WorkPackageActivityProvider < Activities::BaseActivityProvider def notes_anchor(event) version = event['version'].to_i - (version > 1) ? "note-#{version - 1}" : '' + version > 1 ? "note-#{version - 1}" : '' + end + + def types_table + @types_table = Type.arel_table + end + + def statuses_table + @statuses_table = Status.arel_table + end + + def work_packages_table + @work_packages_table ||= WorkPackage.arel_table end end diff --git a/app/models/journal/aggregated_journal.rb b/app/models/journal/aggregated_journal.rb index 2e70480116..796363c9be 100644 --- a/app/models/journal/aggregated_journal.rb +++ b/app/models/journal/aggregated_journal.rb @@ -28,13 +28,14 @@ # See docs/COPYRIGHT.rdoc for more details. #++ -# Similar to regular Journals, but under the following circumstances journals are aggregated: +# Similar to regular Journals, but under the following circumstances a set of individual journals is aggregated to +# a single logical journal: # * they are in temporal proximity # * they belong to the same resource # * they were created by the same user (i.e. the same user edited the journable) # * no other user has an own journal on the same object between the aggregated ones -# When a user commented (added a note) twice within a short time, the second comment will -# "open" a new aggregation, since we do not want to merge comments in any way. +# When a user commented (added a note) twice within a short time, the first comment will +# finish the aggregation, since we do not want to merge comments in any way. # The term "aggregation" means the following when applied to our journaling: # * ignore/hide old journal rows (since every journal row contains a full copy of the journaled # object, dropping intermediate rows will just increase the diff of the following journal) @@ -43,14 +44,14 @@ class Journal::AggregatedJournal class << self def with_version(pure_journal) - wp_journals = Journal::AggregatedJournal.aggregated_journals(journable: pure_journal.journable) + wp_journals = aggregated_journals(journable: pure_journal.journable) wp_journals.detect { |journal| journal.version == pure_journal.version } end # Returns the aggregated journal that contains the specified (vanilla/pure) journal. def containing_journal(pure_journal) - raw = Journal::AggregatedJournal.query_aggregated_journals(journable: pure_journal.journable) - .where("#{version_projection} >= ?", pure_journal.version) + raw = Journal::Scopes::AggregatedJournal.fetch(journable: pure_journal.journable) + .where("version >= ?", pure_journal.version) .first raw ? Journal::AggregatedJournal.new(raw) : nil @@ -61,8 +62,8 @@ class Journal::AggregatedJournal # We need to limit the journal aggregation as soon as possible for performance reasons. # Therefore we have to provide the notes_id to the aggregation on top of it being used # in the where clause to pick the desired AggregatedJournal. - raw_journal = query_aggregated_journals(journal_id: notes_id) - .where("#{table_name}.id = ?", notes_id) + raw_journal = Journal::Scopes::AggregatedJournal.fetch + .where(id: notes_id) .first raw_journal ? Journal::AggregatedJournal.new(raw_journal) : nil @@ -73,201 +74,50 @@ class Journal::AggregatedJournal # # The +until_version+ parameter can be used in conjunction with the +journable+ parameter # to see the aggregated journals as if no versions were known after the specified version. - def aggregated_journals(journable: nil, until_version: nil, includes: []) - raw_journals = query_aggregated_journals(journable: journable, until_version: until_version) - predecessors = {} - raw_journals.each do |journal| - journable_key = [journal.journable_type, journal.journable_id] - predecessors[journable_key] = [nil] unless predecessors[journable_key] - predecessors[journable_key] << journal - end - - aggregated_journals = raw_journals.map { |journal| - journable_key = [journal.journable_type, journal.journable_id] - - Journal::AggregatedJournal.new(journal, predecessor: predecessors[journable_key].shift) - } + def aggregated_journals(journable: nil, sql: nil, until_version: nil, includes: []) + raw_journals = Journal::Scopes::AggregatedJournal.fetch(journable: journable, sql: sql, until_version: until_version) + aggregated_journals = map_to_aggregated_journals(raw_journals) preload_associations(journable, aggregated_journals, includes) aggregated_journals end - def query_aggregated_journals(journable: nil, until_version: nil, journal_id: nil) - # Using the roughly aggregated groups from :sql_rough_group we need to merge journals - # where an entry with empty notes follows an entry containing notes, so that the notes - # from the main entry are taken, while the remaining information is taken from the - # more recent entry. We therefore join the rough groups with itself - # _wherever a merge would be valid_. - # Since the results are already pre-merged, this can only happen if Our first entry (master) - # had a comment and its successor (addition) had no comment, but can be merged. - # This alone would, however, leave the addition in the result set, leaving a "no change" - # journal entry back. By an additional self-join towards the predecessor, we can make sure - # that our own row (master) would not already have been merged by its predecessor. If it is - # (that means if we can find a valid predecessor), we drop our current row, because it will - # already be present (in a merged form) in the row of our predecessor. - Journal.from("(#{sql_rough_group(journable, until_version, journal_id)}) #{table_name}") - .joins(Arel.sql("LEFT OUTER JOIN (#{sql_rough_group(journable, until_version, journal_id)}) addition - ON #{sql_on_groups_belong_condition(table_name, 'addition')}")) - .joins(Arel.sql("LEFT OUTER JOIN (#{sql_rough_group(journable, until_version, journal_id)}) predecessor - ON #{sql_on_groups_belong_condition('predecessor', table_name)}")) - .where(Arel.sql('predecessor.id IS NULL')) - .order(Arel.sql("COALESCE(addition.created_at, #{table_name}.created_at) ASC")) - .order(Arel.sql("#{version_projection} ASC")) - .select(Arel.sql("#{table_name}.journable_id, - #{table_name}.journable_type, - #{table_name}.user_id, - #{table_name}.notes, - #{table_name}.id \"notes_id\", - #{table_name}.version \"notes_version\", - #{table_name}.activity_type, - COALESCE(addition.created_at, #{table_name}.created_at) \"created_at\", - COALESCE(addition.id, #{table_name}.id) \"id\", - #{version_projection} \"version\"")) - end - # Returns whether "notification-hiding" should be assumed for the given journal pair. # This leads to an aggregated journal effectively blocking notifications of an earlier journal, # because it "steals" the addition from its predecessor. See the specs section under # "mail suppressing aggregation" (for EnqueueWorkPackageNotificationJob) for more details def hides_notifications?(successor, predecessor) return false unless successor && predecessor - - timeout = Setting.journal_aggregation_time_minutes.to_i.minutes - - if successor.journable_type != predecessor.journable_type || - successor.journable_id != predecessor.journable_id || - successor.user_id != predecessor.user_id || - (successor.created_at - predecessor.created_at) <= timeout - return false - end + return false if belong_to_different_groups?(predecessor, successor) # imaginary state in which the successor never existed # if this makes the predecessor disappear, the successor must have taken journals # from it (that now became part of the predecessor again). - !Journal::AggregatedJournal - .query_aggregated_journals( + !Journal::Scopes::AggregatedJournal + .fetch( journable: successor.journable, - until_version: successor.version - 1) - .where("#{version_projection} = ?", predecessor.version) + until_version: successor.version - 1 + ) + .where(version: predecessor.version) .exists? end - def table_name - Journal.table_name - end - - def version_projection - "COALESCE(addition.version, #{table_name}.version)" - end - private - # Provides a full SQL statement that returns journals that are aggregated on a basic level: - # * a row is dropped as soon as its successor is eligible to be merged with it - # * rows with a comment are never dropped (we _might_ need the comment later) - # Thereby the result already has aggregation performed, but will still have too many rows: - # Changes without notes after changes containing notes (even if both were performed by - # the same user). Those need to be filtered out later. - # To be able to self-join results of this statement, we add an additional column called - # "group_number" to the result. This allows to compare a group resulting from this query with - # its predecessor and successor. - def sql_rough_group(journable, until_version, journal_id) - if until_version && !journable - raise 'need to provide a journable, when specifying a version limit' - elsif journable && journable.id.nil? - raise 'journable has no id' - end - - conditions = additional_conditions(journable, until_version, journal_id) - - "SELECT predecessor.*, #{sql_group_counter} AS group_number - FROM journals predecessor - #{sql_rough_group_join(conditions[:join_conditions])} - #{sql_rough_group_where(conditions[:where_conditions])} - #{sql_rough_group_order}" - end - - def additional_conditions(journable, until_version, journal_id) - where_conditions = '' - join_conditions = '' - - if journable - where_conditions += " AND predecessor.journable_type = '#{journable.class.name}' AND - predecessor.journable_id = #{journable.id}" - - if until_version - where_conditions += " AND predecessor.version <= #{until_version}" - join_conditions += "AND successor.version <= #{until_version}" - end - end - - if journal_id - where_conditions += "AND predecessor.id IN ( - SELECT id_key.id - FROM #{table_name} id_key JOIN #{table_name} journable_key - ON id_key.journable_id = journable_key.journable_id - AND id_key.journable_type = journable_key.journable_type - AND journable_key.id = #{journal_id})" + def map_to_aggregated_journals(raw_journals) + predecessors = {} + raw_journals.each do |journal| + journable_key = [journal.journable_type, journal.journable_id] + predecessors[journable_key] = [nil] unless predecessors[journable_key] + predecessors[journable_key] << journal end - { where_conditions: where_conditions, - join_conditions: join_conditions } - end - - def sql_rough_group_join(additional_conditions) - "LEFT OUTER JOIN #{table_name} successor - ON predecessor.version + 1 = successor.version AND - predecessor.journable_type = successor.journable_type AND - predecessor.journable_id = successor.journable_id - #{additional_conditions}" - end - - def sql_rough_group_where(additional_conditions) - "WHERE (predecessor.user_id != successor.user_id OR - (predecessor.notes != '' AND predecessor.notes IS NOT NULL) OR - #{sql_beyond_aggregation_time?('predecessor', 'successor')} OR - successor.id IS NULL) - #{additional_conditions}" - end - - def sql_rough_group_order - "ORDER BY predecessor.created_at" - end - - # This method returns the appropriate statement to be used inside a SELECT to - # obtain the current group number. - def sql_group_counter - 'row_number() OVER (ORDER BY predecessor.version ASC)' - end - - # Similar to the WHERE statement used in :sql_rough_group. However, this condition will - # match (return true) for all pairs where a merge/aggregation IS possible. - def sql_on_groups_belong_condition(predecessor, successor) - "#{predecessor}.group_number + 1 = #{successor}.group_number AND - (NOT #{sql_beyond_aggregation_time?(predecessor, successor)} AND - #{predecessor}.user_id = #{successor}.user_id AND - #{successor}.journable_type = #{predecessor}.journable_type AND - #{successor}.journable_id = #{predecessor}.journable_id AND - NOT ((#{predecessor}.notes != '' AND #{predecessor}.notes IS NOT NULL) AND - (#{successor}.notes != '' AND #{successor}.notes IS NOT NULL)))" - end + raw_journals.map do |journal| + journable_key = [journal.journable_type, journal.journable_id] - # Returns a SQL condition that will determine whether two entries are too far apart (temporal) - # to be considered for aggregation. This takes the current instance settings for temporal - # proximity into account. - def sql_beyond_aggregation_time?(predecessor, successor) - aggregation_time_seconds = Setting.journal_aggregation_time_minutes.to_i.minutes - if aggregation_time_seconds == 0 - # if aggregation is disabled, we consider everything to be beyond aggregation time - # even if creation dates are exactly equal - return '(true = true)' + Journal::AggregatedJournal.new(journal, predecessor: predecessors[journable_key].shift) end - - difference = "(#{successor}.created_at - #{predecessor}.created_at)" - threshold = "interval '#{aggregation_time_seconds} second'" - - "(#{difference} > #{threshold})" end def preload_associations(journable, aggregated_journals, includes) @@ -311,6 +161,15 @@ class Journal::AggregatedJournal end end end + + def belong_to_different_groups?(predecessor, successor) + timeout = Setting.journal_aggregation_time_minutes.to_i.minutes + + successor.journable_type != predecessor.journable_type || + successor.journable_id != predecessor.journable_id || + successor.user_id != predecessor.user_id || + (successor.created_at - predecessor.created_at) <= timeout + end end include JournalChanges @@ -372,10 +231,10 @@ class Journal::AggregatedJournal def predecessor unless defined? @predecessor - raw_journal = self.class.query_aggregated_journals(journable: journable) - .where("#{self.class.version_projection} < ?", version) + raw_journal = Journal::Scopes::AggregatedJournal.fetch(journable: journable) + .where("version < ?", version) .except(:order) - .order(Arel.sql("#{self.class.version_projection} DESC")) + .order(version: :desc) .first @predecessor = raw_journal ? Journal::AggregatedJournal.new(raw_journal) : nil @@ -386,10 +245,10 @@ class Journal::AggregatedJournal def successor unless defined? @successor - raw_journal = self.class.query_aggregated_journals(journable: journable) - .where("#{self.class.version_projection} > ?", version) + raw_journal = Journal::Scopes::AggregatedJournal.fetch(journable: journable) + .where("version > ?", version) .except(:order) - .order(Arel.sql("#{self.class.version_projection} ASC")) + .order(version: :asc) .first @successor = raw_journal ? Journal::AggregatedJournal.new(raw_journal) : nil diff --git a/app/models/journal/scopes/aggregated_journal.rb b/app/models/journal/scopes/aggregated_journal.rb new file mode 100644 index 0000000000..b7801b2835 --- /dev/null +++ b/app/models/journal/scopes/aggregated_journal.rb @@ -0,0 +1,359 @@ +#-- encoding: UTF-8 + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2012-2020 the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2017 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# 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. +# +# See docs/COPYRIGHT.rdoc for more details. +#++ + +# Scope to fetch all fields necessary to populated AggregatedJournal collections. +# See the AggregatedJournal model class for a description. +module Journal::Scopes + class AggregatedJournal + class << self + def fetch(journable: nil, sql: nil, until_version: nil) + journals_preselection = raw_journals_subselect(journable, sql, until_version) + + # We wrap the sql with a subselect so that outside of this class, + # The fields native to journals (e.g. id, version) can be referenced, without + # having to also use a CASE/COALESCE statement. + Journal + .from(select_sql(journals_preselection)) + end + + private + + # The sql used to query the database for the aggregated journals. + # The query makes use of 4 parts (which are selected from/joined): + # * The minimum journal version that starts an aggregation group. + # * The maximum journal version that ends an aggregation group. + # * Journals with notes that are within the bounds of minimum version/maximum version. + # * Journals with notes that are within the bounds of the before mentioned journal notes and the maximum journal version. + # + # The maximum version are those journals, whose successor: + # * Where created by a different user + # * Where created after the configured aggregation period had expired (always relative to the journal under consideration). + # + # The minimum version then is the maximum version of the group before - 1. + # + # e.g. a group of 10 sequential journals might break into the following groups + # + # Version 10 (User A, 6 minutes after 9) + # Version 9 (User A, 2 minutes after 8) + # Version 8 (User A, 4 minutes after 7) + # Version 7 (User A, 1 minute after 6) + # Version 6 (User A, 3 minutes after 5) + # Version 5 (User A, 1 minute after 4) + # Version 4 (User B, 1 minute after 3) + # Version 3 (User B, 4 minutes after 2) + # Version 2 (User A, 1 minute after 1) + # Version 1 (User A) + # + # would have the following maximum journals if the aggregation period where 5 minutes: + # + # Version 10 (User A, 6 minutes after 9) + # Version 9 (User A, 2 minutes after 8) + # Version 4 (User B, 1 minute after 3) + # Version 2 (User A, 1 minute after 1) + # + # The last journal (one without a successor) of a journable will obviously also always be a maximum journal. + # + # If the aggregation period where to be expanded to 7 minutes, the maximum journals would be slightly different: + # + # Version 10 (User A, 6 minutes after 9) + # Version 4 (User B, 1 minute after 3) + # Version 2 (User A, 1 minute after 1) + # + # As we do not store the aggregated journals, and rather calculate them on reading, the aggregated journals might be tuned + # by a user. + # + # The minimum version in the example with the 5 minute aggregation period would then be calculated from the maximum version: + # + # Version 10 + # Version 5 + # Version 3 + # Version 1 + # + # The first version will always be included. + # + # Without a journal with notes (the user commented on the journable) in between, the maximum journal is returned + # as the representation of every aggregation group. This is possible as the journals (together with their data and their + # customizable_journals/attachable_journals) represent the complete state of the journable at the given time. + # + # e.g. a group of 5 sequential journals without notes, belonging to the same user and created within the configured + # time difference between one journal and its succcessor + # + # Version 9 + # Version 8 + # Version 7 + # Version 6 + # Version 5 + # + # would only return the last journal, Version 9. + # + # In case the group has one journal with notes in it, the last journal is also returned. But as we also want the note + # to be returned, we return the note as if it would belong to the maximum journal version. This explicitly means + # that all journals of the same group that are after the notes journal are also returned. + # + # e.g. a group of 5 sequential journals with only one note, belonging to the same user and created within the configured + # time difference between one journal and its succcessor + # + # Version 9 + # Version 8 + # Version 7 + # Version 6 (note) + # Version 5 + # + # would only return the last journal, Version 9, but would also return the note and the id of the journal the note + # belongs to natively. + # + # But as we do not want to aggregate notes, the behaviour above can no longer work if there is more than one note in the + # same group. In such a case, a group is cut into subsets. The journals returned will then only contain all the changes + # up until a journal with notes. The only exception to this is the last journal note which might also contain changes + # after it up to and including the maximum journal version of the group. + + # e.g. a group of 5 sequential journals with only one note, belonging to the same user and created within the configured + # time difference between one journal and its succcessor + # + # Version 9 + # Version 8 (note) + # Version 7 + # Version 6 (note) + # Version 5 + # + # would return the last journal, Version 9, but with the note of Version 8 and also a reference in the form of + # note_id pointing to Version 8. It would also return Version 6, with its note and a reference in the form of note_id + # this time pointing to the native journal, Version 6. + # + # The journals that are considered for aggregation can also be reduced by providing a subselect. Doing so, one + # can e.g. consider only the journals created after a certain time. + def select_sql(journals_preselection) + <<~SQL + (#{Journal + .from(start_group_journals_select(journals_preselection)) + .joins(end_group_journals_join(journals_preselection)) + .joins(notes_in_group_join(journals_preselection)) + .joins(additional_notes_in_group_join(journals_preselection)) + .select(projection_list).to_sql}) journals + SQL + end + + def user_or_time_group_breaking_journals_subselect(journals_preselection) + <<~SQL + SELECT + predecessor.*, + row_number() OVER (ORDER BY predecessor.journable_type, predecessor.journable_id, predecessor.version ASC) #{group_number_alias} + FROM #{journals_preselection} predecessor + LEFT OUTER JOIN #{journals_preselection} successor + ON predecessor.version + 1 = successor.version + AND predecessor.journable_type = successor.journable_type + AND predecessor.journable_id = successor.journable_id + WHERE (predecessor.user_id != successor.user_id + OR #{beyond_aggregation_time_condition}) + OR successor.id IS NULL + SQL + end + + def notes_journals_subselect(journals_preselection) + <<~SQL + (SELECT + notes_journals.* + FROM #{journals_preselection} notes_journals + WHERE notes_journals.notes != '' AND notes_journals.notes IS NOT NULL) + SQL + end + + def start_group_journals_select(journals_preselection) + "(#{user_or_time_group_breaking_journals_subselect(journals_preselection)}) #{start_group_journals_alias}" + end + + def end_group_journals_join(journals_preselection) + group_journals_join_condition = <<~SQL + #{start_group_journals_alias}.#{group_number_alias} = #{end_group_journals_alias}.#{group_number_alias} - 1 + AND #{start_group_journals_alias}.journable_type = #{end_group_journals_alias}.journable_type + AND #{start_group_journals_alias}.journable_id = #{end_group_journals_alias}.journable_id + SQL + + end_group_journals = <<~SQL + RIGHT OUTER JOIN + (#{user_or_time_group_breaking_journals_subselect(journals_preselection)}) #{end_group_journals_alias} + ON #{group_journals_join_condition} + SQL + + Arel.sql(end_group_journals) + end + + def notes_in_group_join(journals_preselection) + # As we right join on the minimum journal version, the minimum might be empty. We thus have to coalesce in such + # case as <= will not interpret NULL as 0. + # This also works if we do not fetch the whole set of journals starting from the first journal but rather + # start somewhere within the set. This might take place e.g. when fetching only the journals that are + # created after a certain point in time which is done when displaying of the last month in the activity module. + breaking_journals_notes_join_condition = <<~SQL + COALESCE(#{start_group_journals_alias}.version, 0) + 1 <= #{notes_in_group_alias}.version + AND #{end_group_journals_alias}.version >= #{notes_in_group_alias}.version + AND #{end_group_journals_alias}.journable_type = #{notes_in_group_alias}.journable_type + AND #{end_group_journals_alias}.journable_id = #{notes_in_group_alias}.journable_id + SQL + + breaking_journals_notes = <<~SQL + LEFT OUTER JOIN + #{notes_journals_subselect(journals_preselection)} #{notes_in_group_alias} + ON #{breaking_journals_notes_join_condition} + SQL + + Arel.sql(breaking_journals_notes) + end + + def additional_notes_in_group_join(journals_preselection) + successor_journals_notes_join_condition = <<~SQL + #{notes_in_group_alias}.version < successor_notes.version + AND #{end_group_journals_alias}.version >= successor_notes.version + AND #{end_group_journals_alias}.journable_type = successor_notes.journable_type + AND #{end_group_journals_alias}.journable_id = successor_notes.journable_id + SQL + + successor_journals_notes = <<~SQL + LEFT OUTER JOIN + #{notes_journals_subselect(journals_preselection)} successor_notes + ON #{successor_journals_notes_join_condition} + SQL + + Arel.sql(successor_journals_notes) + end + + def projection_list + projections = <<~SQL + #{end_group_journals_alias}.journable_type, + #{end_group_journals_alias}.journable_id, + #{end_group_journals_alias}.user_id, + #{end_group_journals_alias}.activity_type, + #{notes_projection} notes, + #{notes_id_projection} notes_id, + #{notes_in_group_alias}.version notes_version, + #{version_projection} AS version, + #{created_at_projection} created_at, + #{id_projection} id + SQL + + Arel.sql(projections) + end + + def id_projection + <<~SQL + CASE + WHEN successor_notes.version IS NOT NULL THEN #{notes_in_group_alias}.id + ELSE #{end_group_journals_alias}.id END + SQL + end + + def version_projection + <<~SQL + CASE + WHEN successor_notes.version IS NOT NULL THEN #{notes_in_group_alias}.version + ELSE #{end_group_journals_alias}.version END + SQL + end + + def created_at_projection + <<~SQL + CASE + WHEN successor_notes.version IS NOT NULL THEN #{notes_in_group_alias}.created_at + ELSE #{end_group_journals_alias}.created_at END + SQL + end + + def notes_id_projection + <<~SQL + COALESCE(#{notes_in_group_alias}.id, #{end_group_journals_alias}.id) + SQL + end + + def notes_projection + <<~SQL + COALESCE(#{notes_in_group_alias}.notes, '') + SQL + end + + def raw_journals_subselect(journable, sql, until_version) + if sql + raise 'until_version used together with sql' if until_version + + "(#{sql})" + elsif journable + limit = until_version ? "AND journals.version <= #{until_version}" : '' + + <<~SQL + ( + SELECT * from journals + WHERE journals.journable_id = #{journable.id} + AND journals.journable_type = '#{journable.class.name}' + #{limit} + ) + SQL + else + where = until_version ? "WHERE journals.version <= #{until_version}" : '' + + <<~SQL + (SELECT * from journals #{where}) + SQL + end + end + + # Returns a SQL condition that will determine whether two entries are too far apart (temporal) + # to be considered for aggregation. This takes the current instance settings for temporal + # proximity into account. + def beyond_aggregation_time_condition + aggregation_time_seconds = Setting.journal_aggregation_time_minutes.to_i.minutes + if aggregation_time_seconds == 0 + # if aggregation is disabled, we consider everything to be beyond aggregation time + # even if creation dates are exactly equal + return '(true = true)' + end + + difference = "(successor.created_at - predecessor.created_at)" + threshold = "interval '#{aggregation_time_seconds} second'" + + "(#{difference} > #{threshold})" + end + + def start_group_journals_alias + "start_groups_journals" + end + + def end_group_journals_alias + "end_groups_journals" + end + + def group_number_alias + "group_number" + end + + def notes_in_group_alias + "notes_in_group_journals" + end + end + end +end diff --git a/app/models/journal/wiki_content_journal.rb b/app/models/journal/wiki_content_journal.rb index 10416c84fc..79578912f2 100644 --- a/app/models/journal/wiki_content_journal.rb +++ b/app/models/journal/wiki_content_journal.rb @@ -1,4 +1,5 @@ #-- encoding: UTF-8 + #-- copyright # OpenProject is an open source project management software. # Copyright (C) 2012-2020 the OpenProject GmbH diff --git a/app/models/project.rb b/app/models/project.rb index b73cfe147c..4fbb6a29ae 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -250,9 +250,11 @@ class Project < ApplicationRecord # project.project_condition(true) => "(projects.id = 1 OR (projects.lft > 1 AND projects.rgt < 10))" # project.project_condition(false) => "projects.id = 1" def project_condition(with_subprojects) - cond = "#{Project.table_name}.id = #{id}" - cond = "(#{cond} OR (#{Project.table_name}.lft > #{lft} AND #{Project.table_name}.rgt < #{rgt}))" if with_subprojects - cond + projects_table = Project.arel_table + + stmt = projects_table[:id].eq(id) + stmt = stmt.or(projects_table[:lft].gt(lft).and(projects_table[:rgt].lt(rgt))) if with_subprojects + stmt end def types_used_by_work_packages diff --git a/app/views/activities/index.html.erb b/app/views/activities/index.html.erb index 01e227135f..d9dcb2f95a 100644 --- a/app/views/activities/index.html.erb +++ b/app/views/activities/index.html.erb @@ -29,15 +29,15 @@ See docs/COPYRIGHT.rdoc for more details. <%= call_hook :activity_index_head %> -<%= toolbar title: (@author.nil? ? l(:label_activity) : l(:label_user_activity, link_to_user(@author))).html_safe, - subtitle: l(:label_date_from_to, start: format_date(@date_to - @days), end: format_date(@date_to-1)) +<%= toolbar title: (@author.nil? ? t(:label_activity) : l(:label_user_activity, link_to_user(@author))).html_safe, + subtitle: t(:label_date_from_to, start: format_date(@date_to - @days), end: format_date(@date_to-1)) %>
<% @events_by_day.keys.sort.reverse.each do |day| %>

<%= format_activity_day(day) %>