From fc687df65dff522c24667c938c46029e07011e12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Wed, 8 Sep 2021 14:53:26 +0200 Subject: [PATCH 1/7] Add group_by capabilities to the base query --- app/models/queries/base_query.rb | 54 ++++++++++- .../{ => filters}/not_existing_filter.rb | 0 .../queries/group_bys/available_group_bys.rb | 51 +++++++++++ app/models/queries/group_bys/base.rb | 72 +++++++++++++++ .../group_bys/not_existing_group_by.rb | 47 ++++++++++ app/models/queries/notifications.rb | 5 ++ .../group_bys/group_by_reason.rb} | 53 +---------- app/models/queries/orders/base.rb | 90 +++++++++++++++++++ app/models/queries/register.rb | 12 ++- 9 files changed, 332 insertions(+), 52 deletions(-) rename app/models/queries/{ => filters}/not_existing_filter.rb (100%) create mode 100644 app/models/queries/group_bys/available_group_bys.rb create mode 100644 app/models/queries/group_bys/base.rb create mode 100644 app/models/queries/group_bys/not_existing_group_by.rb rename app/models/queries/{base_order.rb => notifications/group_bys/group_by_reason.rb} (58%) create mode 100644 app/models/queries/orders/base.rb diff --git a/app/models/queries/base_query.rb b/app/models/queries/base_query.rb index 0650c03f8e..b2707892c1 100644 --- a/app/models/queries/base_query.rb +++ b/app/models/queries/base_query.rb @@ -38,17 +38,21 @@ class Queries::BaseQuery end attr_accessor :filters, :orders + attr_reader :group_by include Queries::AvailableFilters include Queries::AvailableOrders + include Queries::GroupBys::AvailableGroupBys include ActiveModel::Validations validate :filters_valid, - :sortation_valid + :sortation_valid, + :group_by_valid def initialize(user: nil) @filters = [] @orders = [] + @group_by = nil @user = user end @@ -60,6 +64,15 @@ class Queries::BaseQuery end end + def groups + return nil if group_by.nil? + return empty_scope unless valid? + + apply_group_by(apply_filters(default_scope)) + .pluck(group_by.name, Arel.sql('COUNT(*)')) + .to_h + end + def where(attribute, operator, values) filter = filter_for(attribute) filter.operator = operator @@ -81,6 +94,12 @@ class Queries::BaseQuery self end + def group(attribute) + self.group_by = group_by_for(attribute) + + self + end + def default_scope self.class.model.all end @@ -100,6 +119,7 @@ class Queries::BaseQuery protected attr_accessor :user + attr_writer :group_by def filters_valid filters.each do |filter| @@ -117,6 +137,12 @@ class Queries::BaseQuery end end + def group_by_valid + return if group_by.nil? || group_by.valid? + + add_error(:group_by, group_by.name, group_by) + end + def add_error(local_attribute, attribute_name, object) messages = object .errors @@ -145,7 +171,7 @@ class Queries::BaseQuery end def apply_orders(scope) - orders.each do |order| + build_orders.each do |order| scope = scope.merge(order.scope) end @@ -156,6 +182,30 @@ class Queries::BaseQuery already_ordered_by_id?(scope) ? scope : scope.order(id: :desc) end + def apply_group_by(scope) + return scope if group_by.nil? + + scope + .merge(group_by.scope) + .order(group_by.name) + end + + def build_orders + return orders if group_by.nil? || has_group_by_order? + + [group_by_order] + orders + end + + def has_group_by_order? + !!group_by && orders.detect { |order| order.class.key == group_by.order_key } + end + + def group_by_order + order_for(group_by.order_key).tap do |order| + order.direction = :asc + end + end + def already_ordered_by_id?(scope) scope.order_values.any? do |order| order.respond_to?(:value) && order.value.respond_to?(:relation) && diff --git a/app/models/queries/not_existing_filter.rb b/app/models/queries/filters/not_existing_filter.rb similarity index 100% rename from app/models/queries/not_existing_filter.rb rename to app/models/queries/filters/not_existing_filter.rb diff --git a/app/models/queries/group_bys/available_group_bys.rb b/app/models/queries/group_bys/available_group_bys.rb new file mode 100644 index 0000000000..b1a0184f0f --- /dev/null +++ b/app/models/queries/group_bys/available_group_bys.rb @@ -0,0 +1,51 @@ +#-- encoding: UTF-8 + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2012-2021 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-2013 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 COPYRIGHT and LICENSE files for more details. +#++ + +module Queries + module GroupBys + module AvailableGroupBys + def group_by_for(key) + (find_registered_group_by(key) || ::Queries::GroupBys::NotExistingGroupBy).new(key) + end + + private + + def find_registered_group_by(key) + group_by_register.detect do |s| + s.key === key.to_sym + end + end + + def group_by_register + ::Queries::Register.group_bys[self.class] + end + end + end +end diff --git a/app/models/queries/group_bys/base.rb b/app/models/queries/group_bys/base.rb new file mode 100644 index 0000000000..6022150f9d --- /dev/null +++ b/app/models/queries/group_bys/base.rb @@ -0,0 +1,72 @@ +#-- encoding: UTF-8 + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2012-2021 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-2013 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 COPYRIGHT and LICENSE files for more details. +#++ + +module Queries + module GroupBys + class Base + include ActiveModel::Validations + + def self.i18n_scope + :activerecord + end + + class_attribute :model + attr_accessor :attribute + + def initialize(attribute) + self.attribute = attribute + end + + def self.key + raise NotImplementedError + end + + def scope + group_by + end + + def name + attribute + end + + # Default to the same key for order + # as the one for group + def order_key + self.class.key + end + + protected + + def group_by + model.group(name) + end + end + end +end \ No newline at end of file diff --git a/app/models/queries/group_bys/not_existing_group_by.rb b/app/models/queries/group_bys/not_existing_group_by.rb new file mode 100644 index 0000000000..b1b5ca9ff2 --- /dev/null +++ b/app/models/queries/group_bys/not_existing_group_by.rb @@ -0,0 +1,47 @@ +#-- encoding: UTF-8 + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2012-2021 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-2013 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 COPYRIGHT and LICENSE files for more details. +#++ + +module Queries + module GroupBys + class NotExistingGroupBy < Base + validate :always_false + + def self.key + :inexistent + end + + private + + def always_false + errors.add :base, I18n.t(:'activerecord.errors.messages.does_not_exist') + end + end + end +end \ No newline at end of file diff --git a/app/models/queries/notifications.rb b/app/models/queries/notifications.rb index 5517c55f0f..39bbd27525 100644 --- a/app/models/queries/notifications.rb +++ b/app/models/queries/notifications.rb @@ -43,4 +43,9 @@ module Queries::Notifications Queries::Register.order Queries::Notifications::NotificationQuery, order end + + [Queries::Notifications::GroupBys::GroupByReason].each do |group| + Queries::Register.group_by Queries::Notifications::NotificationQuery, + group + end end diff --git a/app/models/queries/base_order.rb b/app/models/queries/notifications/group_bys/group_by_reason.rb similarity index 58% rename from app/models/queries/base_order.rb rename to app/models/queries/notifications/group_bys/group_by_reason.rb index f28cae7677..343038db3a 100644 --- a/app/models/queries/base_order.rb +++ b/app/models/queries/notifications/group_bys/group_by_reason.rb @@ -28,59 +28,14 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class Queries::BaseOrder - include ActiveModel::Validations - - VALID_DIRECTIONS = %i(asc desc).freeze - - def self.i18n_scope - :activerecord - end - - validates :direction, inclusion: { in: VALID_DIRECTIONS } - - class_attribute :model - attr_accessor :direction, - :attribute - - def initialize(attribute) - self.attribute = attribute - end +class Queries::Notifications::GroupBys::GroupByReason < Queries::GroupBys::Base + self.model = Notification def self.key - raise NotImplementedError - end - - def scope - scope = order - scope = scope.joins(joins) if joins - scope = scope.left_outer_joins(left_outer_joins) if left_outer_joins - scope + :reason end def name - attribute - end - - private - - def order - model.order(name => direction) - end - - def joins - nil - end - - def left_outer_joins - nil - end - - def with_raise_on_invalid - if VALID_DIRECTIONS.include?(direction) - yield - else - raise ArgumentError, "Only one of #{VALID_DIRECTIONS} allowed. #{direction} is provided." - end + :reason_ian end end diff --git a/app/models/queries/orders/base.rb b/app/models/queries/orders/base.rb new file mode 100644 index 0000000000..20c7d07810 --- /dev/null +++ b/app/models/queries/orders/base.rb @@ -0,0 +1,90 @@ +#-- encoding: UTF-8 + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2012-2021 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-2013 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 COPYRIGHT and LICENSE files for more details. +#++ + +module Queries + module Orders + class Base + include ActiveModel::Validations + + VALID_DIRECTIONS = %i(asc desc).freeze + + def self.i18n_scope + :activerecord + end + + validates :direction, inclusion: { in: VALID_DIRECTIONS } + + class_attribute :model + attr_accessor :direction, + :attribute + + def initialize(attribute) + self.attribute = attribute + end + + def self.key + raise NotImplementedError + end + + def scope + scope = order + scope = scope.joins(joins) if joins + scope = scope.left_outer_joins(left_outer_joins) if left_outer_joins + scope + end + + def name + attribute + end + + private + + def order + model.order(name => direction) + end + + def joins + nil + end + + def left_outer_joins + nil + end + + def with_raise_on_invalid + if VALID_DIRECTIONS.include?(direction) + yield + else + raise ArgumentError, "Only one of #{VALID_DIRECTIONS} allowed. #{direction} is provided." + end + end + end + end +end diff --git a/app/models/queries/register.rb b/app/models/queries/register.rb index 9178c2f4d5..6ee9a6a2d2 100644 --- a/app/models/queries/register.rb +++ b/app/models/queries/register.rb @@ -46,6 +46,14 @@ module Queries::Register @orders[query] << order end + def group_by(query, group_by) + @group_bys ||= Hash.new do |hash, group_key| + hash[group_key] = [] + end + + @group_bys[query] << group_by + end + def column(query, column) @columns ||= Hash.new do |hash, column_key| hash[column_key] = [] @@ -60,6 +68,8 @@ module Queries::Register attr_accessor :filters, :orders, - :columns + :columns, + :group_bys + end end From 3b455b812962a4f955a195b626154eaf7eca07ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 13 Sep 2021 09:39:58 +0200 Subject: [PATCH 2/7] Move base filter and order into their namespaces --- app/models/queries/available_filters.rb | 127 ----------------- app/models/queries/base_query.rb | 4 +- .../queries/capabilities/orders/id_order.rb | 2 +- .../queries/filters/available_filters.rb | 132 ++++++++++++++++++ .../queries/filters/not_existing_filter.rb | 83 +++++------ app/models/queries/filters/serializable.rb | 2 +- app/models/queries/group_bys/base.rb | 2 +- .../queries/groups/orders/default_order.rb | 2 +- .../orders/group_order.rb | 2 +- .../orders/name_order.rb | 2 +- .../queries/members/orders/default_order.rb | 2 +- .../queries/members/orders/email_order.rb | 2 +- .../queries/members/orders/name_order.rb | 2 +- .../queries/members/orders/status_order.rb | 2 +- .../queries/news/orders/default_order.rb | 2 +- .../notifications/orders/default_order.rb | 2 +- .../notifications/orders/read_ian_order.rb | 2 +- .../notifications/orders/reason_order.rb | 2 +- .../queries/{ => orders}/available_orders.rb | 28 ++-- .../{ => orders}/not_existing_order.rb | 20 +-- .../placeholder_users/orders/default_order.rb | 2 +- .../queries/principals/orders/name_order.rb | 2 +- .../projects/orders/custom_field_order.rb | 2 +- .../queries/projects/orders/default_order.rb | 2 +- .../orders/latest_activity_at_order.rb | 2 +- .../queries/projects/orders/name_order.rb | 2 +- .../projects/orders/project_status_order.rb | 2 +- .../orders/required_disk_space_order.rb | 2 +- .../queries/relations/orders/default_order.rb | 2 +- .../queries/users/orders/default_order.rb | 2 +- .../queries/versions/orders/name_order.rb | 2 +- .../versions/orders/semver_name_order.rb | 2 +- .../work_packages/filter_serializer.rb | 4 +- app/models/query.rb | 2 +- .../time_entries/orders/default_order.rb | 2 +- .../queries/documents/orders/default_order.rb | 2 +- .../{ => filters}/available_filters_spec.rb | 14 +- 37 files changed, 244 insertions(+), 226 deletions(-) delete mode 100644 app/models/queries/available_filters.rb create mode 100644 app/models/queries/filters/available_filters.rb rename app/models/queries/{ => orders}/available_orders.rb (75%) rename app/models/queries/{ => orders}/not_existing_order.rb (81%) rename spec/models/queries/{ => filters}/available_filters_spec.rb (92%) diff --git a/app/models/queries/available_filters.rb b/app/models/queries/available_filters.rb deleted file mode 100644 index 36bbf69a9d..0000000000 --- a/app/models/queries/available_filters.rb +++ /dev/null @@ -1,127 +0,0 @@ -#-- encoding: UTF-8 - -#-- copyright -# OpenProject is an open source project management software. -# Copyright (C) 2012-2021 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-2013 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 COPYRIGHT and LICENSE files for more details. -#++ - -require_dependency 'queries/filters' - -module Queries::AvailableFilters - def self.included(base) - base.extend(ClassMethods) - end - - module ClassMethods - def registered_filters - Queries::Register.filters[self] - end - - def find_registered_filter(key) - registered_filters.detect do |f| - f.key === key.to_sym - end - end - end - - def available_filters - uninitialized = registered_filters - already_initialized_filters - - uninitialized.each do |filter| - initialize_filter(filter) - end - - initialized_filters.select(&:available?) - end - - def filter_for(key, no_memoization = false) - filter = get_initialized_filter(key, no_memoization) - - raise ::Queries::Filters::MissingError if filter.nil? - - filter - rescue ::Queries::Filters::InvalidError => e - Rails.logger.error "Failed to register filter for #{key}: #{e} \n" \ - "Falling back to non-existing filter." - non_existing_filter(key) - rescue ::Queries::Filters::MissingError => e - Rails.logger.error "Failed to find filter for #{key}: #{e} \n" \ - "Falling back to non-existing filter." - non_existing_filter(key) - end - - private - - def non_existing_filter(key) - Queries::NotExistingFilter.create!(name: key) - end - - def get_initialized_filter(key, no_memoization) - filter = find_registered_filter(key) - - return unless filter - - if no_memoization - filter.create!(name: key) - else - initialize_filter(filter) - - find_initialized_filter(key) - end - end - - def initialize_filter(filter) - return if already_initialized_filters.include?(filter) - - already_initialized_filters << filter - - new_filters = filter.all_for(context) - - initialized_filters.push(*Array(new_filters)) - end - - def find_registered_filter(key) - self.class.find_registered_filter(key) - end - - def find_initialized_filter(key) - initialized_filters.detect do |f| - f.name == key.to_sym - end - end - - def already_initialized_filters - @already_initialized_filters ||= [] - end - - def initialized_filters - @initialized_filters ||= [] - end - - def registered_filters - self.class.registered_filters - end -end diff --git a/app/models/queries/base_query.rb b/app/models/queries/base_query.rb index b2707892c1..cab98823f2 100644 --- a/app/models/queries/base_query.rb +++ b/app/models/queries/base_query.rb @@ -40,8 +40,8 @@ class Queries::BaseQuery attr_accessor :filters, :orders attr_reader :group_by - include Queries::AvailableFilters - include Queries::AvailableOrders + include Queries::Filters::AvailableFilters + include Queries::Orders::AvailableOrders include Queries::GroupBys::AvailableGroupBys include ActiveModel::Validations diff --git a/app/models/queries/capabilities/orders/id_order.rb b/app/models/queries/capabilities/orders/id_order.rb index ff3ca9bc86..f987496696 100644 --- a/app/models/queries/capabilities/orders/id_order.rb +++ b/app/models/queries/capabilities/orders/id_order.rb @@ -28,7 +28,7 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class Queries::Capabilities::Orders::IdOrder < Queries::BaseOrder +class Queries::Capabilities::Orders::IdOrder < Queries::Orders::Base self.model = Capability def self.key diff --git a/app/models/queries/filters/available_filters.rb b/app/models/queries/filters/available_filters.rb new file mode 100644 index 0000000000..9c022ebbef --- /dev/null +++ b/app/models/queries/filters/available_filters.rb @@ -0,0 +1,132 @@ +#-- encoding: UTF-8 + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2012-2021 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-2013 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 COPYRIGHT and LICENSE files for more details. +#++ + +require_dependency 'queries/filters' + +module Queries + module Filters + module AvailableFilters + def self.included(base) + base.extend(ClassMethods) + end + + module ClassMethods + def registered_filters + ::Queries::Register.filters[self] + end + + def find_registered_filter(key) + registered_filters.detect do |f| + f.key === key.to_sym + end + end + end + + def available_filters + uninitialized = registered_filters - already_initialized_filters + + uninitialized.each do |filter| + initialize_filter(filter) + end + + initialized_filters.select(&:available?) + end + + def filter_for(key, no_memoization = false) + filter = get_initialized_filter(key, no_memoization) + + raise ::Queries::Filters::MissingError if filter.nil? + + filter + rescue ::Queries::Filters::InvalidError => e + Rails.logger.error "Failed to register filter for #{key}: #{e} \n" \ + "Falling back to non-existing filter." + non_existing_filter(key) + rescue ::Queries::Filters::MissingError => e + Rails.logger.error "Failed to find filter for #{key}: #{e} \n" \ + "Falling back to non-existing filter." + non_existing_filter(key) + end + + private + + def non_existing_filter(key) + ::Queries::Filters::NotExistingFilter.create!(name: key) + end + + def get_initialized_filter(key, no_memoization) + filter = find_registered_filter(key) + + return unless filter + + if no_memoization + filter.create!(name: key) + else + initialize_filter(filter) + + find_initialized_filter(key) + end + end + + def initialize_filter(filter) + return if already_initialized_filters.include?(filter) + + already_initialized_filters << filter + + new_filters = filter.all_for(context) + + initialized_filters.push(*Array(new_filters)) + end + + def find_registered_filter(key) + self.class.find_registered_filter(key) + end + + def find_initialized_filter(key) + initialized_filters.detect do |f| + f.name == key.to_sym + end + end + + def already_initialized_filters + @already_initialized_filters ||= [] + end + + def initialized_filters + @initialized_filters ||= [] + end + + def registered_filters + self.class.registered_filters + end + end + end +end + diff --git a/app/models/queries/filters/not_existing_filter.rb b/app/models/queries/filters/not_existing_filter.rb index 8c6eb85b64..ddd7da6d67 100644 --- a/app/models/queries/filters/not_existing_filter.rb +++ b/app/models/queries/filters/not_existing_filter.rb @@ -28,54 +28,59 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class Queries::NotExistingFilter < Queries::Filters::Base - def available? - false - end +module Queries + module Filters + class NotExistingFilter < Base + def available? + false + end - def type - :inexistent - end + def type + :inexistent + end - def self.key - :not_existent - end + def self.key + :not_existent + end - def human_name - name.to_s.blank? ? type : name.to_s - end + def human_name + name.to_s.blank? ? type : name.to_s + end - validate :always_false + validate :always_false - def always_false - errors.add :base, I18n.t(:'activerecord.errors.messages.does_not_exist') - end + def always_false + errors.add :base, I18n.t(:'activerecord.errors.messages.does_not_exist') + end - # deactivating superclass validation - def validate_inclusion_of_operator; end + # deactivating superclass validation + def validate_inclusion_of_operator; end - def to_hash - { - non_existent_filter: { - operator: operator, - values: values - } - } - end + def to_hash + { + non_existent_filter: { + operator: operator, + values: values + } + } + end - def scope - # TODO: remove switch once the WP query is a - # subclass of Queries::Base - model = if context.respond_to?(:model) - context.model - else - WorkPackage - end + def scope + # TODO: remove switch once the WP query is a + # subclass of Queries::Base + model = if context.respond_to?(:model) + context.model + else + WorkPackage + end - model.unscoped - end + model.unscoped + end + + def attributes_hash + nil + end + end - def attributes_hash - nil end end diff --git a/app/models/queries/filters/serializable.rb b/app/models/queries/filters/serializable.rb index 70b4f5aae8..a5334c7161 100644 --- a/app/models/queries/filters/serializable.rb +++ b/app/models/queries/filters/serializable.rb @@ -41,7 +41,7 @@ module Queries create!(name, filter_hash[field]) rescue ::Queries::Filters::InvalidError Rails.logger.error "Failed to constantize field filter #{field} from hash." - ::Queries::NotExistingFilter.create!(field) + ::Queries::Filters::NotExistingFilter.create!(field) end end end diff --git a/app/models/queries/group_bys/base.rb b/app/models/queries/group_bys/base.rb index 6022150f9d..6863b7af82 100644 --- a/app/models/queries/group_bys/base.rb +++ b/app/models/queries/group_bys/base.rb @@ -69,4 +69,4 @@ module Queries end end end -end \ No newline at end of file +end diff --git a/app/models/queries/groups/orders/default_order.rb b/app/models/queries/groups/orders/default_order.rb index f2008a1682..60df2020b2 100644 --- a/app/models/queries/groups/orders/default_order.rb +++ b/app/models/queries/groups/orders/default_order.rb @@ -28,7 +28,7 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class Queries::Groups::Orders::DefaultOrder < Queries::BaseOrder +class Queries::Groups::Orders::DefaultOrder < Queries::Orders::Base self.model = Group def self.key diff --git a/app/models/queries/individual_principals/orders/group_order.rb b/app/models/queries/individual_principals/orders/group_order.rb index 9ba36a56cb..47caa1bb6e 100644 --- a/app/models/queries/individual_principals/orders/group_order.rb +++ b/app/models/queries/individual_principals/orders/group_order.rb @@ -27,7 +27,7 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class Queries::IndividualPrincipals::Orders::GroupOrder < Queries::BaseOrder +class Queries::IndividualPrincipals::Orders::GroupOrder < Queries::Orders::Base self.model = Principal def self.key diff --git a/app/models/queries/individual_principals/orders/name_order.rb b/app/models/queries/individual_principals/orders/name_order.rb index 319fdababf..0dedc615e7 100644 --- a/app/models/queries/individual_principals/orders/name_order.rb +++ b/app/models/queries/individual_principals/orders/name_order.rb @@ -28,7 +28,7 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class Queries::IndividualPrincipals::Orders::NameOrder < Queries::BaseOrder +class Queries::IndividualPrincipals::Orders::NameOrder < Queries::Orders::Base self.model = Principal def self.key diff --git a/app/models/queries/members/orders/default_order.rb b/app/models/queries/members/orders/default_order.rb index 70e3b18d4f..622056d77a 100644 --- a/app/models/queries/members/orders/default_order.rb +++ b/app/models/queries/members/orders/default_order.rb @@ -28,7 +28,7 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class Queries::Members::Orders::DefaultOrder < Queries::BaseOrder +class Queries::Members::Orders::DefaultOrder < Queries::Orders::Base self.model = Member def self.key diff --git a/app/models/queries/members/orders/email_order.rb b/app/models/queries/members/orders/email_order.rb index b9e4553b86..c967dc5493 100644 --- a/app/models/queries/members/orders/email_order.rb +++ b/app/models/queries/members/orders/email_order.rb @@ -28,7 +28,7 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class Queries::Members::Orders::EmailOrder < Queries::BaseOrder +class Queries::Members::Orders::EmailOrder < Queries::Orders::Base self.model = Member def self.key diff --git a/app/models/queries/members/orders/name_order.rb b/app/models/queries/members/orders/name_order.rb index 83d5a56be7..e2b81495d2 100644 --- a/app/models/queries/members/orders/name_order.rb +++ b/app/models/queries/members/orders/name_order.rb @@ -28,7 +28,7 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class Queries::Members::Orders::NameOrder < Queries::BaseOrder +class Queries::Members::Orders::NameOrder < Queries::Orders::Base self.model = Member def self.key diff --git a/app/models/queries/members/orders/status_order.rb b/app/models/queries/members/orders/status_order.rb index ad1d3aef60..9127fb3d17 100644 --- a/app/models/queries/members/orders/status_order.rb +++ b/app/models/queries/members/orders/status_order.rb @@ -28,7 +28,7 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class Queries::Members::Orders::StatusOrder < Queries::BaseOrder +class Queries::Members::Orders::StatusOrder < Queries::Orders::Base self.model = Member def self.key diff --git a/app/models/queries/news/orders/default_order.rb b/app/models/queries/news/orders/default_order.rb index 240436c9a3..ad86230139 100644 --- a/app/models/queries/news/orders/default_order.rb +++ b/app/models/queries/news/orders/default_order.rb @@ -28,7 +28,7 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class Queries::News::Orders::DefaultOrder < Queries::BaseOrder +class Queries::News::Orders::DefaultOrder < Queries::Orders::Base self.model = News def self.key diff --git a/app/models/queries/notifications/orders/default_order.rb b/app/models/queries/notifications/orders/default_order.rb index 151c22895d..06bff276e7 100644 --- a/app/models/queries/notifications/orders/default_order.rb +++ b/app/models/queries/notifications/orders/default_order.rb @@ -28,7 +28,7 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class Queries::Notifications::Orders::DefaultOrder < Queries::BaseOrder +class Queries::Notifications::Orders::DefaultOrder < Queries::Orders::Base self.model = Notification def self.key diff --git a/app/models/queries/notifications/orders/read_ian_order.rb b/app/models/queries/notifications/orders/read_ian_order.rb index b679355325..042fd7dd63 100644 --- a/app/models/queries/notifications/orders/read_ian_order.rb +++ b/app/models/queries/notifications/orders/read_ian_order.rb @@ -28,7 +28,7 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class Queries::Notifications::Orders::ReadIanOrder < Queries::BaseOrder +class Queries::Notifications::Orders::ReadIanOrder < Queries::Orders::Base self.model = Notification def self.key diff --git a/app/models/queries/notifications/orders/reason_order.rb b/app/models/queries/notifications/orders/reason_order.rb index a70bceb50c..68afe3cc49 100644 --- a/app/models/queries/notifications/orders/reason_order.rb +++ b/app/models/queries/notifications/orders/reason_order.rb @@ -28,7 +28,7 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class Queries::Notifications::Orders::ReasonOrder < Queries::BaseOrder +class Queries::Notifications::Orders::ReasonOrder < Queries::Orders::Base self.model = Notification def self.key diff --git a/app/models/queries/available_orders.rb b/app/models/queries/orders/available_orders.rb similarity index 75% rename from app/models/queries/available_orders.rb rename to app/models/queries/orders/available_orders.rb index 08ff45254e..60750c9c11 100644 --- a/app/models/queries/available_orders.rb +++ b/app/models/queries/orders/available_orders.rb @@ -28,20 +28,24 @@ # See COPYRIGHT and LICENSE files for more details. #++ -module Queries::AvailableOrders - def order_for(key) - (find_registered_order(key) || Queries::NotExistingOrder).new(key) - end +module Queries + module Orders + module AvailableOrders + def order_for(key) + (find_registered_order(key) || ::Queries::Orders::NotExistingOrder).new(key) + end - private + private - def find_registered_order(key) - orders_register.detect do |s| - s.key === key.to_sym - end - end + def find_registered_order(key) + orders_register.detect do |s| + s.key === key.to_sym + end + end - def orders_register - Queries::Register.orders[self.class] + def orders_register + ::Queries::Register.orders[self.class] + end + end end end diff --git a/app/models/queries/not_existing_order.rb b/app/models/queries/orders/not_existing_order.rb similarity index 81% rename from app/models/queries/not_existing_order.rb rename to app/models/queries/orders/not_existing_order.rb index dad4b6f6ec..482a996dbf 100644 --- a/app/models/queries/not_existing_order.rb +++ b/app/models/queries/orders/not_existing_order.rb @@ -28,16 +28,20 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class Queries::NotExistingOrder < Queries::BaseOrder - validate :always_false +module Queries + module Orders + class NotExistingOrder < Base + validate :always_false - def self.key - :inexistent - end + def self.key + :inexistent + end - private + private - def always_false - errors.add :base, I18n.t(:'activerecord.errors.messages.does_not_exist') + def always_false + errors.add :base, I18n.t(:'activerecord.errors.messages.does_not_exist') + end + end end end diff --git a/app/models/queries/placeholder_users/orders/default_order.rb b/app/models/queries/placeholder_users/orders/default_order.rb index f880ecaa7c..2f97bb316c 100644 --- a/app/models/queries/placeholder_users/orders/default_order.rb +++ b/app/models/queries/placeholder_users/orders/default_order.rb @@ -28,7 +28,7 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class Queries::PlaceholderUsers::Orders::DefaultOrder < Queries::BaseOrder +class Queries::PlaceholderUsers::Orders::DefaultOrder < Queries::Orders::Base self.model = PlaceholderUser def self.key diff --git a/app/models/queries/principals/orders/name_order.rb b/app/models/queries/principals/orders/name_order.rb index 2d5a9668d5..2d8311cffb 100644 --- a/app/models/queries/principals/orders/name_order.rb +++ b/app/models/queries/principals/orders/name_order.rb @@ -28,7 +28,7 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class Queries::Principals::Orders::NameOrder < Queries::BaseOrder +class Queries::Principals::Orders::NameOrder < Queries::Orders::Base self.model = Principal def self.key diff --git a/app/models/queries/projects/orders/custom_field_order.rb b/app/models/queries/projects/orders/custom_field_order.rb index 23dd81aca2..1360e3c829 100644 --- a/app/models/queries/projects/orders/custom_field_order.rb +++ b/app/models/queries/projects/orders/custom_field_order.rb @@ -28,7 +28,7 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class Queries::Projects::Orders::CustomFieldOrder < Queries::BaseOrder +class Queries::Projects::Orders::CustomFieldOrder < Queries::Orders::Base self.model = Project.all validates :custom_field, presence: { message: I18n.t(:'activerecord.errors.messages.does_not_exist') } diff --git a/app/models/queries/projects/orders/default_order.rb b/app/models/queries/projects/orders/default_order.rb index d7b11f4374..db1bd86570 100644 --- a/app/models/queries/projects/orders/default_order.rb +++ b/app/models/queries/projects/orders/default_order.rb @@ -28,7 +28,7 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class Queries::Projects::Orders::DefaultOrder < Queries::BaseOrder +class Queries::Projects::Orders::DefaultOrder < Queries::Orders::Base self.model = Project def self.key diff --git a/app/models/queries/projects/orders/latest_activity_at_order.rb b/app/models/queries/projects/orders/latest_activity_at_order.rb index bdb1923bfc..84a6098914 100644 --- a/app/models/queries/projects/orders/latest_activity_at_order.rb +++ b/app/models/queries/projects/orders/latest_activity_at_order.rb @@ -28,7 +28,7 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class Queries::Projects::Orders::LatestActivityAtOrder < Queries::BaseOrder +class Queries::Projects::Orders::LatestActivityAtOrder < Queries::Orders::Base self.model = Project def self.key diff --git a/app/models/queries/projects/orders/name_order.rb b/app/models/queries/projects/orders/name_order.rb index 39c72d14f5..d2690358e2 100644 --- a/app/models/queries/projects/orders/name_order.rb +++ b/app/models/queries/projects/orders/name_order.rb @@ -28,7 +28,7 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class Queries::Projects::Orders::NameOrder < Queries::BaseOrder +class Queries::Projects::Orders::NameOrder < Queries::Orders::Base self.model = Project def self.key diff --git a/app/models/queries/projects/orders/project_status_order.rb b/app/models/queries/projects/orders/project_status_order.rb index 573bf88ca1..a79c9513a6 100644 --- a/app/models/queries/projects/orders/project_status_order.rb +++ b/app/models/queries/projects/orders/project_status_order.rb @@ -28,7 +28,7 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class Queries::Projects::Orders::ProjectStatusOrder < Queries::BaseOrder +class Queries::Projects::Orders::ProjectStatusOrder < Queries::Orders::Base self.model = Project def self.key diff --git a/app/models/queries/projects/orders/required_disk_space_order.rb b/app/models/queries/projects/orders/required_disk_space_order.rb index f897d1344f..c7bf66d645 100644 --- a/app/models/queries/projects/orders/required_disk_space_order.rb +++ b/app/models/queries/projects/orders/required_disk_space_order.rb @@ -28,7 +28,7 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class Queries::Projects::Orders::RequiredDiskSpaceOrder < Queries::BaseOrder +class Queries::Projects::Orders::RequiredDiskSpaceOrder < Queries::Orders::Base self.model = Project def self.key diff --git a/app/models/queries/relations/orders/default_order.rb b/app/models/queries/relations/orders/default_order.rb index b1c2f16aed..612e5912cf 100644 --- a/app/models/queries/relations/orders/default_order.rb +++ b/app/models/queries/relations/orders/default_order.rb @@ -31,7 +31,7 @@ module Queries module Relations module Orders - class DefaultOrder < ::Queries::BaseOrder + class DefaultOrder < ::Queries::Orders::Base self.model = Relation def self.key diff --git a/app/models/queries/users/orders/default_order.rb b/app/models/queries/users/orders/default_order.rb index a796d78f25..5a79ec9907 100644 --- a/app/models/queries/users/orders/default_order.rb +++ b/app/models/queries/users/orders/default_order.rb @@ -28,7 +28,7 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class Queries::Users::Orders::DefaultOrder < Queries::BaseOrder +class Queries::Users::Orders::DefaultOrder < Queries::Orders::Base self.model = User def self.key diff --git a/app/models/queries/versions/orders/name_order.rb b/app/models/queries/versions/orders/name_order.rb index 8c59f4b00e..54dfef5f83 100644 --- a/app/models/queries/versions/orders/name_order.rb +++ b/app/models/queries/versions/orders/name_order.rb @@ -28,7 +28,7 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class Queries::Versions::Orders::NameOrder < Queries::BaseOrder +class Queries::Versions::Orders::NameOrder < Queries::Orders::Base self.model = Version def self.key diff --git a/app/models/queries/versions/orders/semver_name_order.rb b/app/models/queries/versions/orders/semver_name_order.rb index a737091e16..63b014553b 100644 --- a/app/models/queries/versions/orders/semver_name_order.rb +++ b/app/models/queries/versions/orders/semver_name_order.rb @@ -28,7 +28,7 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class Queries::Versions::Orders::SemverNameOrder < Queries::BaseOrder +class Queries::Versions::Orders::SemverNameOrder < Queries::Orders::Base self.model = Version def self.key diff --git a/app/models/queries/work_packages/filter_serializer.rb b/app/models/queries/work_packages/filter_serializer.rb index fb85e134ff..5fb9e7b23d 100644 --- a/app/models/queries/work_packages/filter_serializer.rb +++ b/app/models/queries/work_packages/filter_serializer.rb @@ -29,8 +29,8 @@ #++ module Queries::WorkPackages::FilterSerializer - extend Queries::AvailableFilters - extend Queries::AvailableFilters::ClassMethods + extend Queries::Filters::AvailableFilters + extend Queries::Filters::AvailableFilters::ClassMethods def self.load(serialized_filter_hash) return [] if serialized_filter_hash.nil? diff --git a/app/models/query.rb b/app/models/query.rb index d10eeb9fcf..750af1c5a3 100644 --- a/app/models/query.rb +++ b/app/models/query.rb @@ -32,7 +32,7 @@ class Query < ApplicationRecord include Timelines include Highlighting include ManualSorting - include Queries::AvailableFilters + include Queries::Filters::AvailableFilters belongs_to :project belongs_to :user diff --git a/modules/costs/app/models/queries/time_entries/orders/default_order.rb b/modules/costs/app/models/queries/time_entries/orders/default_order.rb index b0745e7049..1b35890c85 100644 --- a/modules/costs/app/models/queries/time_entries/orders/default_order.rb +++ b/modules/costs/app/models/queries/time_entries/orders/default_order.rb @@ -28,7 +28,7 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class Queries::TimeEntries::Orders::DefaultOrder < Queries::BaseOrder +class Queries::TimeEntries::Orders::DefaultOrder < Queries::Orders::Base self.model = TimeEntry def self.key diff --git a/modules/documents/app/models/queries/documents/orders/default_order.rb b/modules/documents/app/models/queries/documents/orders/default_order.rb index 2ab8e901b2..b248189643 100644 --- a/modules/documents/app/models/queries/documents/orders/default_order.rb +++ b/modules/documents/app/models/queries/documents/orders/default_order.rb @@ -28,7 +28,7 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class Queries::Documents::Orders::DefaultOrder < Queries::BaseOrder +class Queries::Documents::Orders::DefaultOrder < Queries::Orders::Base self.model = Document def self.key diff --git a/spec/models/queries/available_filters_spec.rb b/spec/models/queries/filters/available_filters_spec.rb similarity index 92% rename from spec/models/queries/available_filters_spec.rb rename to spec/models/queries/filters/available_filters_spec.rb index 5f0fa33cf5..a48a56a631 100644 --- a/spec/models/queries/available_filters_spec.rb +++ b/spec/models/queries/filters/available_filters_spec.rb @@ -28,7 +28,7 @@ require 'spec_helper' -describe Queries::AvailableFilters, type: :model do +describe Queries::Filters::AvailableFilters, type: :model do let(:context) { FactoryBot.build_stubbed(:project) } let(:register) { Queries::FilterRegister } @@ -39,7 +39,7 @@ describe Queries::AvailableFilters, type: :model do self.context = context end - include Queries::AvailableFilters + include Queries::Filters::AvailableFilters end let(:includer) do @@ -165,7 +165,7 @@ describe Queries::AvailableFilters, type: :model do end it 'returns the NotExistingFilter if the name is not matched' do - expect(includer.filter_for(:not_a_filter_name)).to be_a Queries::NotExistingFilter + expect(includer.filter_for(:not_a_filter_name)).to be_a Queries::Filters::NotExistingFilter end end @@ -174,7 +174,7 @@ describe Queries::AvailableFilters, type: :model do let(:filter_3_available) { true } it 'returns the NotExistingFilter if the name is not matched' do - expect(includer.filter_for(:not_a_filter_name)).to be_a Queries::NotExistingFilter + expect(includer.filter_for(:not_a_filter_name)).to be_a Queries::Filters::NotExistingFilter end it 'returns an instance of the matching filter if not caring for availablility' do @@ -190,11 +190,11 @@ describe Queries::AvailableFilters, type: :model do end it 'returns the NotExistingFilter if the key is not matched' do - expect(includer.filter_for(:f_i1)).to be_a Queries::NotExistingFilter + expect(includer.filter_for(:f_i1)).to be_a Queries::Filters::NotExistingFilter end it 'returns the NotExistingFilter if the key is matched but the name is not' do - expect(includer.filter_for(:f_2)).to be_a Queries::NotExistingFilter + expect(includer.filter_for(:f_2)).to be_a Queries::Filters::NotExistingFilter end end @@ -202,7 +202,7 @@ describe Queries::AvailableFilters, type: :model do let(:filter_2_available) { false } it 'returns the NotExistingFilter' do - expect(includer.filter_for(:f_i)).to be_a Queries::NotExistingFilter + expect(includer.filter_for(:f_i)).to be_a Queries::Filters::NotExistingFilter end end end From 4e2dc171db370abf5d132dc728351aecb18045d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 13 Sep 2021 13:15:25 +0200 Subject: [PATCH 3/7] Extract group functionality into index endpoint --- .../notifications/orders/reason_order.rb | 4 + ...k_package_collection_from_query_service.rb | 2 +- app/services/params_to_query_service.rb | 9 +++ lib/api/decorators/aggregation_group.rb | 34 +-------- lib/api/decorators/collection.rb | 9 ++- .../decorators/offset_paginated_collection.rb | 4 +- lib/api/v3/utilities/endpoints/index.rb | 9 +++ .../work_package_aggregation_group.rb | 73 +++++++++++++++++++ .../work_package_collection_representer.rb | 7 +- 9 files changed, 111 insertions(+), 40 deletions(-) create mode 100644 lib/api/v3/work_packages/work_package_aggregation_group.rb diff --git a/app/models/queries/notifications/orders/reason_order.rb b/app/models/queries/notifications/orders/reason_order.rb index 68afe3cc49..90f508539c 100644 --- a/app/models/queries/notifications/orders/reason_order.rb +++ b/app/models/queries/notifications/orders/reason_order.rb @@ -34,4 +34,8 @@ class Queries::Notifications::Orders::ReasonOrder < Queries::Orders::Base def self.key :reason end + + def name + :reason_ian + end end diff --git a/app/services/api/v3/work_package_collection_from_query_service.rb b/app/services/api/v3/work_package_collection_from_query_service.rb index f88d9d7b8e..4a09c77254 100644 --- a/app/services/api/v3/work_package_collection_from_query_service.rb +++ b/app/services/api/v3/work_package_collection_from_query_service.rb @@ -104,7 +104,7 @@ module API sums = generate_group_sums results.work_package_count_by_group.map do |group, count| - ::API::Decorators::AggregationGroup.new(group, count, query: query, sums: sums[group], current_user: current_user) + ::API::V3::WorkPackages::WorkPackageAggregationGroup.new(group, count, query: query, sums: sums[group], current_user: current_user) end end diff --git a/app/services/params_to_query_service.rb b/app/services/params_to_query_service.rb index 8caf5ad8ff..b8e479a4ac 100644 --- a/app/services/params_to_query_service.rb +++ b/app/services/params_to_query_service.rb @@ -40,6 +40,7 @@ class ParamsToQueryService query = apply_filters(query, params) apply_order(query, params) + apply_group_by(query, params) end private @@ -74,6 +75,14 @@ class ParamsToQueryService query.order(hash_sort) end + def apply_group_by(query, params) + return query unless params[:groupBy] + + group_by = convert_attribute(params[:groupBy]) + + query.group(group_by) + end + # Expected format looks like: # [ # { diff --git a/lib/api/decorators/aggregation_group.rb b/lib/api/decorators/aggregation_group.rb index d5f6c881b0..d431487063 100644 --- a/lib/api/decorators/aggregation_group.rb +++ b/lib/api/decorators/aggregation_group.rb @@ -31,9 +31,8 @@ module API module Decorators class AggregationGroup < Single - def initialize(group_key, count, query:, current_user:, sums: nil) + def initialize(group_key, count, query:, current_user:) @count = count - @sums = sums @query = query if group_key.is_a?(Array) @@ -55,15 +54,6 @@ module API end end - link :groupBy do - converted_name = convert_attribute(query.group_by_column.name) - - { - href: api_v3_paths.query_group_by(converted_name), - title: query.group_by_column.caption - } - end - property :value, exec_context: :decorator, render_nil: true @@ -73,23 +63,11 @@ module API getter: ->(*) { count }, render_nil: true - property :sums, - exec_context: :decorator, - getter: ->(*) { - ::API::V3::WorkPackages::WorkPackageSumsRepresenter.create(sums, current_user) if sums - }, - render_nil: false - - def has_sums? - sums.present? - end - def model_required? false end - attr_reader :sums, - :count, + attr_reader :count, :query ## @@ -104,17 +82,13 @@ module API } end - if group_key.empty? - nil - else + if group_key.present? group_key.map(&:name).sort.join(", ") end end def value - if query.group_by_column.name == :done_ratio - "#{represented}%" - elsif represented == true || represented == false + if represented == true || represented == false represented else represented ? represented.to_s : nil diff --git a/lib/api/decorators/collection.rb b/lib/api/decorators/collection.rb index f62d50d76a..1ca2c12c8e 100644 --- a/lib/api/decorators/collection.rb +++ b/lib/api/decorators/collection.rb @@ -33,8 +33,9 @@ module API class Collection < ::API::Decorators::Single include API::Utilities::UrlHelper - def initialize(models, total, self_link:, current_user:) + def initialize(models, total, self_link:, current_user:, groups: nil) @total = total + @groups = groups @self_link = self_link super(models, current_user: current_user) @@ -69,6 +70,10 @@ module API property :total, getter: ->(*) { @total }, exec_context: :decorator property :count, getter: ->(*) { count } + property :groups, + exec_context: :decorator, + render_nil: false + collection :elements, getter: ->(*) { represented.map do |model| @@ -81,6 +86,8 @@ module API def _type 'Collection' end + + attr_reader :groups end end end diff --git a/lib/api/decorators/offset_paginated_collection.rb b/lib/api/decorators/offset_paginated_collection.rb index c496918463..d4e5069224 100644 --- a/lib/api/decorators/offset_paginated_collection.rb +++ b/lib/api/decorators/offset_paginated_collection.rb @@ -37,7 +37,7 @@ module API relation.base_class.per_page end - def initialize(models, self_link:, current_user:, query: {}, page: nil, per_page: nil) + def initialize(models, self_link:, current_user:, query: {}, page: nil, per_page: nil, groups: nil) @self_link_base = self_link @query = query @page = page.to_i > 0 ? page.to_i : 1 @@ -46,7 +46,7 @@ module API full_self_link = make_page_link(page: @page, page_size: @per_page) paged = paged_models(models) - super(paged, models.count, self_link: full_self_link, current_user: current_user) + super(paged, models.count, self_link: full_self_link, current_user: current_user, groups: groups) end link :jumpTo do diff --git a/lib/api/v3/utilities/endpoints/index.rb b/lib/api/v3/utilities/endpoints/index.rb index 7e1c816d32..b3b3ef1b37 100644 --- a/lib/api/v3/utilities/endpoints/index.rb +++ b/lib/api/v3/utilities/endpoints/index.rb @@ -88,6 +88,7 @@ module API query: resulting_params, page: resulting_params[:offset], per_page: resulting_params[:pageSize], + groups: calculate_groups(query), current_user: User.current) end @@ -105,6 +106,14 @@ module API end end + def calculate_groups(query) + return unless query.group_by + + query.groups.map do |group, count| + ::API::Decorators::AggregationGroup.new(group, count, query: query, current_user: User.current) + end + end + def calculate_default_params(query) ::API::Decorators::QueryParamsRepresenter .new(query) diff --git a/lib/api/v3/work_packages/work_package_aggregation_group.rb b/lib/api/v3/work_packages/work_package_aggregation_group.rb new file mode 100644 index 0000000000..0da70bea56 --- /dev/null +++ b/lib/api/v3/work_packages/work_package_aggregation_group.rb @@ -0,0 +1,73 @@ +#-- encoding: UTF-8 + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2012-2021 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-2013 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 COPYRIGHT and LICENSE files for more details. +#++ + +module API + module V3 + module WorkPackages + class WorkPackageAggregationGroup < ::API::Decorators::AggregationGroup + def initialize(group_key, count, query:, current_user:, sums: nil) + @sums = sums + + super(group_key, count, query: query, current_user: current_user) + end + + property :sums, + exec_context: :decorator, + getter: ->(*) { + ::API::V3::WorkPackages::WorkPackageSumsRepresenter.create(sums, current_user) if sums + }, + render_nil: false + + link :groupBy do + converted_name = convert_attribute(query.group_by_column.name) + + { + href: api_v3_paths.query_group_by(converted_name), + title: query.group_by_column.caption + } + end + + def has_sums? + sums.present? + end + + attr_reader :sums + + def value + if query.group_by_column.name == :done_ratio + "#{represented}%" + else + super + end + end + end + end + end +end diff --git a/lib/api/v3/work_packages/work_package_collection_representer.rb b/lib/api/v3/work_packages/work_package_collection_representer.rb index 05c5ef489b..01c1e353ca 100644 --- a/lib/api/v3/work_packages/work_package_collection_representer.rb +++ b/lib/api/v3/work_packages/work_package_collection_representer.rb @@ -40,7 +40,6 @@ module API per_page: nil, embed_schemas: false) @project = project - @groups = groups @total_sums = total_sums @embed_schemas = embed_schemas @@ -49,6 +48,7 @@ module API query: query, page: page, per_page: per_page, + groups: groups, current_user: current_user) # In order to optimize performance we @@ -145,10 +145,6 @@ module API embedded: true, render_nil: false - property :groups, - exec_context: :decorator, - render_nil: false - property :total_sums, exec_context: :decorator, getter: ->(*) { @@ -281,7 +277,6 @@ module API end attr_reader :project, - :groups, :total_sums, :embed_schemas end From 5d39c59d352d1981b6eab9855f2e27753f083052 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 13 Sep 2021 14:19:05 +0200 Subject: [PATCH 4/7] Add spec for group_by notification sql output --- app/models/queries/base_query.rb | 15 ++++-- .../notifications/notification_query_spec.rb | 51 ++++++++++++++++++- 2 files changed, 59 insertions(+), 7 deletions(-) diff --git a/app/models/queries/base_query.rb b/app/models/queries/base_query.rb index cab98823f2..de7e658bf0 100644 --- a/app/models/queries/base_query.rb +++ b/app/models/queries/base_query.rb @@ -69,6 +69,11 @@ class Queries::BaseQuery return empty_scope unless valid? apply_group_by(apply_filters(default_scope)) + .select(group_by.name, Arel.sql('COUNT(*)')) + end + + def group_values + groups .pluck(group_by.name, Arel.sql('COUNT(*)')) .to_h end @@ -145,11 +150,11 @@ class Queries::BaseQuery def add_error(local_attribute, attribute_name, object) messages = object - .errors - .messages - .values - .flatten - .join(" #{I18n.t('support.array.sentence_connector')} ") + .errors + .messages + .values + .flatten + .join(" #{I18n.t('support.array.sentence_connector')} ") errors.add local_attribute, errors.full_message(attribute_name, messages) end diff --git a/spec/models/queries/notifications/notification_query_spec.rb b/spec/models/queries/notifications/notification_query_spec.rb index 68d7cb95f5..93c70e1ae9 100644 --- a/spec/models/queries/notifications/notification_query_spec.rb +++ b/spec/models/queries/notifications/notification_query_spec.rb @@ -114,7 +114,11 @@ describe Queries::Notifications::NotificationQuery, type: :model do describe '#results' do it 'is the same as handwriting the query' do - expected = "SELECT \"notifications\".* FROM \"notifications\" WHERE \"notifications\".\"recipient_id\" = #{recipient.id} ORDER BY \"notifications\".\"read_ian\" DESC, \"notifications\".\"id\" DESC" + expected = <<~SQL.squish + SELECT "notifications".* FROM "notifications" + WHERE "notifications"."recipient_id" = #{recipient.id} + ORDER BY "notifications"."read_ian" DESC, "notifications"."id" DESC + SQL expect(instance.results.to_sql).to eql expected end @@ -128,7 +132,11 @@ describe Queries::Notifications::NotificationQuery, type: :model do describe '#results' do it 'is the same as handwriting the query' do - expected = "SELECT \"notifications\".* FROM \"notifications\" WHERE \"notifications\".\"recipient_id\" = #{recipient.id} ORDER BY \"reason\" DESC, \"notifications\".\"id\" DESC" + expected = <<~SQL.squish + SELECT "notifications".* FROM "notifications" + WHERE "notifications"."recipient_id" = #{recipient.id} + ORDER BY "notifications"."reason_ian" DESC, "notifications"."id" DESC + SQL expect(instance.results.to_sql).to eql expected end @@ -154,4 +162,43 @@ describe Queries::Notifications::NotificationQuery, type: :model do end end end + + context 'with a reason group_by' do + before do + instance.group(:reason) + end + + describe '#results' do + it 'is the same as handwriting the query' do + expected = <<~SQL.squish + SELECT "notifications"."reason_ian", COUNT(*) FROM "notifications" + WHERE "notifications"."recipient_id" = #{recipient.id} + GROUP BY "notifications"."reason_ian" + ORDER BY "notifications"."reason_ian" ASC + SQL + + expect(instance.groups.to_sql).to eql expected + end + end + end + + context 'with a non existing group_by' do + before do + instance.group(:does_not_exist) + end + + describe '#results' do + it 'returns a query not returning anything' do + expected = Notification.where(Arel::Nodes::Equality.new(1, 0)) + + expect(instance.results.to_sql).to eql expected.to_sql + end + end + + describe 'valid?' do + it 'is false' do + expect(instance).to be_invalid + end + end + end end From e9ac1665931a4589751f3ba1285bed99cf2eec5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 13 Sep 2021 14:39:28 +0200 Subject: [PATCH 5/7] Add notification api specs for grouping --- lib/api/v3/utilities/endpoints/index.rb | 2 +- lib/api/v3/utilities/path_helper.rb | 3 +- ...otification_collection_representer_spec.rb | 84 +++++++++++++++++++ .../v3/notifications/index_resource_spec.rb | 26 ++++++ 4 files changed, 113 insertions(+), 2 deletions(-) create mode 100644 spec/lib/api/v3/notifications/notification_collection_representer_spec.rb diff --git a/lib/api/v3/utilities/endpoints/index.rb b/lib/api/v3/utilities/endpoints/index.rb index b3b3ef1b37..2b441ba723 100644 --- a/lib/api/v3/utilities/endpoints/index.rb +++ b/lib/api/v3/utilities/endpoints/index.rb @@ -109,7 +109,7 @@ module API def calculate_groups(query) return unless query.group_by - query.groups.map do |group, count| + query.group_values.map do |group, count| ::API::Decorators::AggregationGroup.new(group, count, query: query, current_user: User.current) end end diff --git a/lib/api/v3/utilities/path_helper.rb b/lib/api/v3/utilities/path_helper.rb index afa84b49df..74e9bb7c15 100644 --- a/lib/api/v3/utilities/path_helper.rb +++ b/lib/api/v3/utilities/path_helper.rb @@ -490,10 +490,11 @@ module API "#{project(project_id)}/work_packages" end - def self.path_for(path, filters: nil, sort_by: nil, page_size: nil) + def self.path_for(path, filters: nil, sort_by: nil, group_by: nil, page_size: nil) query_params = { filters: filters&.to_json, sortBy: sort_by&.to_json, + groupBy: group_by, pageSize: page_size }.reject { |_, v| v.blank? } diff --git a/spec/lib/api/v3/notifications/notification_collection_representer_spec.rb b/spec/lib/api/v3/notifications/notification_collection_representer_spec.rb new file mode 100644 index 0000000000..10035792b8 --- /dev/null +++ b/spec/lib/api/v3/notifications/notification_collection_representer_spec.rb @@ -0,0 +1,84 @@ +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2012-2021 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-2013 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 COPYRIGHT and LICENSE files for more details. +#++ + +require 'spec_helper' + +describe ::API::V3::Notifications::NotificationCollectionRepresenter do + let(:self_base_link) { '/api/v3/notifications' } + let(:user) { FactoryBot.build_stubbed :user } + let(:notifications) do + FactoryBot.build_stubbed_list(:notification, + 3).tap do |items| + allow(items) + .to receive(:per_page) + .with(page_size) + .and_return(items) + + allow(items) + .to receive(:page) + .with(page) + .and_return(items) + end + end + let(:current_user) { FactoryBot.build_stubbed(:user) } + let(:representer) do + described_class.new(notifications, + self_link: self_base_link, + per_page: page_size, + page: page, + groups: groups, + current_user: current_user) + end + let(:total) { 3 } + let(:page) { 1 } + let(:page_size) { 2 } + let(:actual_count) { 3 } + let(:collection_inner_type) { 'Notification' } + let(:groups) { nil } + + include API::V3::Utilities::PathHelper + + describe 'generation' do + subject(:collection) { representer.to_json } + + it_behaves_like 'offset-paginated APIv3 collection', 3, 'notifications', 'Notification' + + context 'when passing groups' do + let(:groups) do + [ + { value: 'mentioned', count: 34 }, + { value: 'involved', count: 5} + ] + end + + it 'renders the groups object as json' do + expect(subject).to be_json_eql(groups.to_json).at_path('groups') + end + end + end +end diff --git a/spec/requests/api/v3/notifications/index_resource_spec.rb b/spec/requests/api/v3/notifications/index_resource_spec.rb index 4717634fc5..3113379c3e 100644 --- a/spec/requests/api/v3/notifications/index_resource_spec.rb +++ b/spec/requests/api/v3/notifications/index_resource_spec.rb @@ -129,6 +129,32 @@ describe ::API::V3::Notifications::NotificationsAPI, end end end + + context 'with a reason groupBy' do + let(:involved_notification) { FactoryBot.create :notification, recipient: recipient, reason_ian: :involved } + + let(:notifications) { [notification1, notification2, involved_notification] } + + let(:send_request) do + get api_v3_paths.path_for :notifications, group_by: :reason + end + + let(:groups) { parsed_response['groups'] } + + context 'with the filter being set to false' do + it_behaves_like 'API V3 collection response', 3, 3, 'Notification' + + it 'contains the reason groups', :aggregate_failures do + expect(groups).to be_a Array + expect(groups.count).to eq 2 + + keyed = groups.index_by { |el| el['value'] } + expect(keyed.keys).to contain_exactly 'mentioned', 'involved' + expect(keyed['mentioned']['count']).to eq 2 + expect(keyed['involved']['count']).to eq 1 + end + end + end end describe 'admin user' do From e5dec2dc31d8bc7d40151187d9f60c110e4232c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 13 Sep 2021 15:20:06 +0200 Subject: [PATCH 6/7] Add notification group_by for projects with transformed keys --- app/models/queries/base_query.rb | 15 ++++- .../queries/filters/available_filters.rb | 7 +- .../queries/filters/not_existing_filter.rb | 3 +- app/models/queries/group_bys/base.rb | 16 ++++- .../group_bys/not_existing_group_by.rb | 2 +- app/models/queries/notifications.rb | 4 +- .../group_bys/group_by_project.rb | 45 +++++++++++++ .../notifications/orders/project_order.rb | 50 ++++++++++++++ app/models/queries/register.rb | 1 - .../work_packages/filter_serializer.rb | 2 +- ...k_package_collection_from_query_service.rb | 4 +- .../v3/utilities/resource_link_generator.rb | 2 + ...otification_collection_representer_spec.rb | 2 +- .../queries/filters/available_filters_spec.rb | 60 ++++++++--------- .../notifications/notification_query_spec.rb | 19 ++++++ .../v3/notifications/index_resource_spec.rb | 66 +++++++++++++++---- ...kage_collection_from_query_service_spec.rb | 13 ++-- 17 files changed, 244 insertions(+), 67 deletions(-) create mode 100644 app/models/queries/notifications/group_bys/group_by_project.rb create mode 100644 app/models/queries/notifications/orders/project_order.rb diff --git a/app/models/queries/base_query.rb b/app/models/queries/base_query.rb index de7e658bf0..7e1401ca82 100644 --- a/app/models/queries/base_query.rb +++ b/app/models/queries/base_query.rb @@ -73,9 +73,8 @@ class Queries::BaseQuery end def group_values - groups - .pluck(group_by.name, Arel.sql('COUNT(*)')) - .to_h + groups_hash = groups.pluck(group_by.name, Arel.sql('COUNT(*)')).to_h + instantiate_group_keys groups_hash end def where(attribute, operator, values) @@ -211,6 +210,16 @@ class Queries::BaseQuery end end + def instantiate_group_keys(groups) + return groups unless group_by&.association_class + + ar_keys = group_by.association_class.where(id: groups.keys.compact) + + groups.transform_keys do |key| + ar_keys.detect { |ar_key| ar_key.id == key } || "#{key} #{I18n.t(:label_not_found)}" + end + end + def already_ordered_by_id?(scope) scope.order_values.any? do |order| order.respond_to?(:value) && order.value.respond_to?(:relation) && diff --git a/app/models/queries/filters/available_filters.rb b/app/models/queries/filters/available_filters.rb index 9c022ebbef..675a97876e 100644 --- a/app/models/queries/filters/available_filters.rb +++ b/app/models/queries/filters/available_filters.rb @@ -59,7 +59,7 @@ module Queries initialized_filters.select(&:available?) end - def filter_for(key, no_memoization = false) + def filter_for(key, no_memoization: false) filter = get_initialized_filter(key, no_memoization) raise ::Queries::Filters::MissingError if filter.nil? @@ -67,11 +67,11 @@ module Queries filter rescue ::Queries::Filters::InvalidError => e Rails.logger.error "Failed to register filter for #{key}: #{e} \n" \ - "Falling back to non-existing filter." + "Falling back to non-existing filter." non_existing_filter(key) rescue ::Queries::Filters::MissingError => e Rails.logger.error "Failed to find filter for #{key}: #{e} \n" \ - "Falling back to non-existing filter." + "Falling back to non-existing filter." non_existing_filter(key) end @@ -129,4 +129,3 @@ module Queries end end end - diff --git a/app/models/queries/filters/not_existing_filter.rb b/app/models/queries/filters/not_existing_filter.rb index ddd7da6d67..db2f3653ec 100644 --- a/app/models/queries/filters/not_existing_filter.rb +++ b/app/models/queries/filters/not_existing_filter.rb @@ -44,7 +44,7 @@ module Queries end def human_name - name.to_s.blank? ? type : name.to_s + name.to_s.presence || type end validate :always_false @@ -81,6 +81,5 @@ module Queries nil end end - end end diff --git a/app/models/queries/group_bys/base.rb b/app/models/queries/group_bys/base.rb index 6863b7af82..89b79e6537 100644 --- a/app/models/queries/group_bys/base.rb +++ b/app/models/queries/group_bys/base.rb @@ -48,14 +48,24 @@ module Queries raise NotImplementedError end + def association_class + nil + end + def scope - group_by + scope = model + scope = model.joins(joins) if joins + group_by scope end def name attribute end + def joins + nil + end + # Default to the same key for order # as the one for group def order_key @@ -64,8 +74,8 @@ module Queries protected - def group_by - model.group(name) + def group_by(scope) + scope.group(name) end end end diff --git a/app/models/queries/group_bys/not_existing_group_by.rb b/app/models/queries/group_bys/not_existing_group_by.rb index b1b5ca9ff2..86d2b4c7ff 100644 --- a/app/models/queries/group_bys/not_existing_group_by.rb +++ b/app/models/queries/group_bys/not_existing_group_by.rb @@ -44,4 +44,4 @@ module Queries end end end -end \ No newline at end of file +end diff --git a/app/models/queries/notifications.rb b/app/models/queries/notifications.rb index 39bbd27525..0338150875 100644 --- a/app/models/queries/notifications.rb +++ b/app/models/queries/notifications.rb @@ -39,12 +39,14 @@ module Queries::Notifications [Queries::Notifications::Orders::DefaultOrder, Queries::Notifications::Orders::ReasonOrder, + Queries::Notifications::Orders::ProjectOrder, Queries::Notifications::Orders::ReadIanOrder].each do |order| Queries::Register.order Queries::Notifications::NotificationQuery, order end - [Queries::Notifications::GroupBys::GroupByReason].each do |group| + [Queries::Notifications::GroupBys::GroupByReason, + Queries::Notifications::GroupBys::GroupByProject].each do |group| Queries::Register.group_by Queries::Notifications::NotificationQuery, group end diff --git a/app/models/queries/notifications/group_bys/group_by_project.rb b/app/models/queries/notifications/group_bys/group_by_project.rb new file mode 100644 index 0000000000..ed3f2a4e2d --- /dev/null +++ b/app/models/queries/notifications/group_bys/group_by_project.rb @@ -0,0 +1,45 @@ +#-- encoding: UTF-8 + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2012-2021 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-2013 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 COPYRIGHT and LICENSE files for more details. +#++ + +class Queries::Notifications::GroupBys::GroupByProject < Queries::GroupBys::Base + self.model = Notification + + def self.key + :project + end + + def name + :project_id + end + + def association_class + Project + end +end diff --git a/app/models/queries/notifications/orders/project_order.rb b/app/models/queries/notifications/orders/project_order.rb new file mode 100644 index 0000000000..27712c4109 --- /dev/null +++ b/app/models/queries/notifications/orders/project_order.rb @@ -0,0 +1,50 @@ +#-- encoding: UTF-8 + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2012-2021 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-2013 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 COPYRIGHT and LICENSE files for more details. +#++ + +class Queries::Notifications::Orders::ProjectOrder < Queries::Orders::Base + self.model = Notification + + def self.key + :project + end + + def joins + :project + end + + protected + + def order + order_string = "projects.name" + order_string += " DESC" if direction == :desc + + model.order(order_string) + end +end diff --git a/app/models/queries/register.rb b/app/models/queries/register.rb index 6ee9a6a2d2..bd4d3c6839 100644 --- a/app/models/queries/register.rb +++ b/app/models/queries/register.rb @@ -70,6 +70,5 @@ module Queries::Register :orders, :columns, :group_bys - end end diff --git a/app/models/queries/work_packages/filter_serializer.rb b/app/models/queries/work_packages/filter_serializer.rb index 5fb9e7b23d..52396fbdbf 100644 --- a/app/models/queries/work_packages/filter_serializer.rb +++ b/app/models/queries/work_packages/filter_serializer.rb @@ -41,7 +41,7 @@ module Queries::WorkPackages::FilterSerializer (YAML.load(yaml) || {}).each_with_object([]) do |(field, options), array| options = options.with_indifferent_access - filter = filter_for(field, true) + filter = filter_for(field, no_memoization: true) filter.operator = options['operator'] filter.values = options['values'] array << filter diff --git a/app/services/api/v3/work_package_collection_from_query_service.rb b/app/services/api/v3/work_package_collection_from_query_service.rb index 4a09c77254..ded1386b8f 100644 --- a/app/services/api/v3/work_package_collection_from_query_service.rb +++ b/app/services/api/v3/work_package_collection_from_query_service.rb @@ -104,7 +104,9 @@ module API sums = generate_group_sums results.work_package_count_by_group.map do |group, count| - ::API::V3::WorkPackages::WorkPackageAggregationGroup.new(group, count, query: query, sums: sums[group], current_user: current_user) + ::API::V3::WorkPackages::WorkPackageAggregationGroup.new( + group, count, query: query, sums: sums[group], current_user: current_user + ) end end diff --git a/lib/api/v3/utilities/resource_link_generator.rb b/lib/api/v3/utilities/resource_link_generator.rb index 2700c283dd..1207888d95 100644 --- a/lib/api/v3/utilities/resource_link_generator.rb +++ b/lib/api/v3/utilities/resource_link_generator.rb @@ -51,6 +51,8 @@ module API # since not all things are equally named between APIv3 and the rails code, # we need to convert some names manually case record + when Project + :project when IssuePriority :priority when AnonymousUser, DeletedUser, SystemUser diff --git a/spec/lib/api/v3/notifications/notification_collection_representer_spec.rb b/spec/lib/api/v3/notifications/notification_collection_representer_spec.rb index 10035792b8..a80401670d 100644 --- a/spec/lib/api/v3/notifications/notification_collection_representer_spec.rb +++ b/spec/lib/api/v3/notifications/notification_collection_representer_spec.rb @@ -72,7 +72,7 @@ describe ::API::V3::Notifications::NotificationCollectionRepresenter do let(:groups) do [ { value: 'mentioned', count: 34 }, - { value: 'involved', count: 5} + { value: 'involved', count: 5 } ] end diff --git a/spec/models/queries/filters/available_filters_spec.rb b/spec/models/queries/filters/available_filters_spec.rb index a48a56a631..a086cd1c28 100644 --- a/spec/models/queries/filters/available_filters_spec.rb +++ b/spec/models/queries/filters/available_filters_spec.rb @@ -53,24 +53,24 @@ describe Queries::Filters::AvailableFilters, type: :model do end describe '#filter_for' do - let(:filter_1_available) { true } - let(:filter_2_available) { true } - let(:filter_1_key) { :filter_1 } - let(:filter_2_key) { /f_\d+/ } - let(:filter_1_name) { :filter_1 } - let(:filter_2_name) { :f_1 } - let(:registered_filters) { [filter_1, filter_2] } + let(:filter1_available) { true } + let(:filter2_available) { true } + let(:filter1_key) { :filter1 } + let(:filter2_key) { /f_\d+/ } + let(:filter1_name) { :filter1 } + let(:filter2_name) { :f1 } + let(:registered_filters) { [filter1, filter_2] } - let(:filter_1_instance) do - instance = double("filter_1_instance") + let(:filter1_instance) do + instance = double("filter1_instance") # rubocop:disable Rspec/VerifiedDoubles allow(instance) .to receive(:available?) - .and_return(:filter_1_available) + .and_return(:filter1_available) allow(instance) .to receive(:name) - .and_return(:filter_1) + .and_return(filter1_name) allow(instance) .to receive(:name=) @@ -78,35 +78,35 @@ describe Queries::Filters::AvailableFilters, type: :model do instance end - let(:filter_1) do - filter = double('filter_1') + let(:filter1) do + filter = double('filter1') # rubocop:disable Rspec/VerifiedDoubles allow(filter) .to receive(:key) - .and_return(:filter_1) + .and_return(filter1_key) allow(filter) .to receive(:create!) - .and_return(filter_1_instance) + .and_return(filter1_instance) allow(filter) .to receive(:all_for) .with(context) - .and_return(filter_1_instance) + .and_return(filter1_instance) filter end let(:filter_2_instance) do - instance = double("filter_2_instance") + instance = double("filter_2_instance") # rubocop:disable Rspec/VerifiedDoubles allow(instance) .to receive(:available?) - .and_return(:filter_2_available) + .and_return(filter2_available) allow(instance) .to receive(:name) - .and_return(:f_1) + .and_return(:f1) allow(instance) .to receive(:name=) @@ -115,11 +115,11 @@ describe Queries::Filters::AvailableFilters, type: :model do end let(:filter_2) do - filter = double('filter_2') + filter = double('filter_2') # rubocop:disable Rspec/VerifiedDoubles allow(filter) .to receive(:key) - .and_return(/f_\d+/) + .and_return(/f\d+/) allow(filter) .to receive(:all_for) @@ -131,7 +131,7 @@ describe Queries::Filters::AvailableFilters, type: :model do context 'for a filter identified by a symbol' do let(:filter_3_available) { true } - let(:registered_filters) { [filter_3, filter_1, filter_2] } + let(:registered_filters) { [filter_3, filter1, filter_2] } # As we use regexp to find the filters # we have to ensure that a filter identified a substring symbol @@ -161,7 +161,7 @@ describe Queries::Filters::AvailableFilters, type: :model do let(:filter_3_available) { false } it 'returns an instance of the matching filter' do - expect(includer.filter_for(:filter_1)).to eql filter_1_instance + expect(includer.filter_for(:filter1)).to eql filter1_instance end it 'returns the NotExistingFilter if the name is not matched' do @@ -170,7 +170,7 @@ describe Queries::Filters::AvailableFilters, type: :model do end context 'if not available' do - let(:filter_1_available) { false } + let(:filter1_available) { false } let(:filter_3_available) { true } it 'returns the NotExistingFilter if the name is not matched' do @@ -178,7 +178,7 @@ describe Queries::Filters::AvailableFilters, type: :model do end it 'returns an instance of the matching filter if not caring for availablility' do - expect(includer.filter_for(:filter_1, true)).to eql filter_1_instance + expect(includer.filter_for(:filter1, no_memoization: true)).to eql filter1_instance end end end @@ -186,23 +186,23 @@ describe Queries::Filters::AvailableFilters, type: :model do context 'for a filter identified by a regexp' do context 'if available' do it 'returns an instance of the matching filter' do - expect(includer.filter_for(:f_1)).to eql filter_2_instance + expect(includer.filter_for(:f1)).to eql filter_2_instance end it 'returns the NotExistingFilter if the key is not matched' do - expect(includer.filter_for(:f_i1)).to be_a Queries::Filters::NotExistingFilter + expect(includer.filter_for(:fi1)).to be_a Queries::Filters::NotExistingFilter end it 'returns the NotExistingFilter if the key is matched but the name is not' do - expect(includer.filter_for(:f_2)).to be_a Queries::Filters::NotExistingFilter + expect(includer.filter_for(:f2)).to be_a Queries::Filters::NotExistingFilter end end context 'is false if unavailable' do - let(:filter_2_available) { false } + let(:filter2_available) { false } it 'returns the NotExistingFilter' do - expect(includer.filter_for(:f_i)).to be_a Queries::Filters::NotExistingFilter + expect(includer.filter_for(:fi)).to be_a Queries::Filters::NotExistingFilter end end end diff --git a/spec/models/queries/notifications/notification_query_spec.rb b/spec/models/queries/notifications/notification_query_spec.rb index 93c70e1ae9..5321cf1c3d 100644 --- a/spec/models/queries/notifications/notification_query_spec.rb +++ b/spec/models/queries/notifications/notification_query_spec.rb @@ -182,6 +182,25 @@ describe Queries::Notifications::NotificationQuery, type: :model do end end + context 'with a project group_by' do + before do + instance.group(:project) + end + + describe '#results' do + it 'is the same as handwriting the query' do + expected = <<~SQL.squish + SELECT "notifications"."project_id", COUNT(*) FROM "notifications" + WHERE "notifications"."recipient_id" = #{recipient.id} + GROUP BY "notifications"."project_id" + ORDER BY "notifications"."project_id" ASC + SQL + + expect(instance.groups.to_sql).to eql expected + end + end + end + context 'with a non existing group_by' do before do instance.group(:does_not_exist) diff --git a/spec/requests/api/v3/notifications/index_resource_spec.rb b/spec/requests/api/v3/notifications/index_resource_spec.rb index 3113379c3e..cd15b71cb4 100644 --- a/spec/requests/api/v3/notifications/index_resource_spec.rb +++ b/spec/requests/api/v3/notifications/index_resource_spec.rb @@ -43,8 +43,18 @@ describe ::API::V3::Notifications::NotificationsAPI, member_in_project: work_package.project, member_with_permissions: %i[view_work_packages] end - shared_let(:notification1) { FactoryBot.create :notification, recipient: recipient, resource: work_package } - shared_let(:notification2) { FactoryBot.create :notification, recipient: recipient, resource: work_package } + shared_let(:notification1) do + FactoryBot.create :notification, + recipient: recipient, + resource: work_package, + project: work_package.project + end + shared_let(:notification2) do + FactoryBot.create :notification, + recipient: recipient, + resource: work_package, + project: work_package.project + end let(:notifications) { [notification1, notification2] } @@ -141,18 +151,50 @@ describe ::API::V3::Notifications::NotificationsAPI, let(:groups) { parsed_response['groups'] } - context 'with the filter being set to false' do - it_behaves_like 'API V3 collection response', 3, 3, 'Notification' + it_behaves_like 'API V3 collection response', 3, 3, 'Notification' - it 'contains the reason groups', :aggregate_failures do - expect(groups).to be_a Array - expect(groups.count).to eq 2 + it 'contains the reason groups', :aggregate_failures do + expect(groups).to be_a Array + expect(groups.count).to eq 2 - keyed = groups.index_by { |el| el['value'] } - expect(keyed.keys).to contain_exactly 'mentioned', 'involved' - expect(keyed['mentioned']['count']).to eq 2 - expect(keyed['involved']['count']).to eq 1 - end + keyed = groups.index_by { |el| el['value'] } + expect(keyed.keys).to contain_exactly 'mentioned', 'involved' + expect(keyed['mentioned']['count']).to eq 2 + expect(keyed['involved']['count']).to eq 1 + end + end + + context 'with a project groupBy' do + let(:work_package2) { FactoryBot.create :work_package } + let(:other_project_notification) do + FactoryBot.create :notification, + resource: work_package2, + project: work_package2.project, + recipient: recipient, + reason_ian: :involved + end + + let(:notifications) { [notification1, notification2, other_project_notification] } + + let(:send_request) do + get api_v3_paths.path_for :notifications, group_by: :project + end + + let(:groups) { parsed_response['groups'] } + + it_behaves_like 'API V3 collection response', 3, 3, 'Notification' + + it 'contains the project groups', :aggregate_failures do + expect(groups).to be_a Array + expect(groups.count).to eq 2 + + keyed = groups.index_by { |el| el['value'] } + expect(keyed.keys).to contain_exactly work_package2.project.name, work_package.project.name + expect(keyed[work_package.project.name]['count']).to eq 2 + expect(keyed[work_package2.project.name]['count']).to eq 1 + + expect(keyed.dig(work_package.project.name, '_links', 'valueLink')[0]['href']) + .to eq "/api/v3/projects/#{work_package.project.id}" end end end diff --git a/spec/services/api/v3/work_package_collection_from_query_service_spec.rb b/spec/services/api/v3/work_package_collection_from_query_service_spec.rb index 03c37feacc..d4d228f331 100644 --- a/spec/services/api/v3/work_package_collection_from_query_service_spec.rb +++ b/spec/services/api/v3/work_package_collection_from_query_service_spec.rb @@ -159,14 +159,9 @@ describe ::API::V3::WorkPackageCollectionFromQueryService, describe '#call' do subject { instance.call(params) } - it 'is successful' do - is_expected - .to be_success - end - before do stub_const('::API::V3::WorkPackages::WorkPackageCollectionRepresenter', mock_wp_representer) - stub_const('::API::Decorators::AggregationGroup', mock_aggregation_representer) + stub_const('::API::V3::WorkPackages::WorkPackageAggregationGroup', mock_aggregation_representer) allow(::API::V3::UpdateQueryFromV3ParamsService) .to receive(:new) @@ -174,11 +169,15 @@ describe ::API::V3::WorkPackageCollectionFromQueryService, .and_return(mock_update_query_service) end + it 'is successful' do + expect(subject).to be_success + end + context 'result' do subject { instance.call(params).result } it 'is a WorkPackageCollectionRepresenter' do - is_expected + expect(subject) .to be_a(::API::V3::WorkPackages::WorkPackageCollectionRepresenter) end From 650a78fac537602bbd72ea1fea68bfa0d9e292b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 13 Sep 2021 21:23:08 +0200 Subject: [PATCH 7/7] Add groupBy API description --- docs/api/apiv3/paths/notifications.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/docs/api/apiv3/paths/notifications.yml b/docs/api/apiv3/paths/notifications.yml index 15b85d6f04..18c393e3a4 100644 --- a/docs/api/apiv3/paths/notifications.yml +++ b/docs/api/apiv3/paths/notifications.yml @@ -32,6 +32,18 @@ get: required: false schema: type: string + - description: |- + string specifying group_by criteria. + + + reason: Group by notification reason + + + project: Sort by associated project + example: 'reason' + in: query + name: groupBy + required: false + schema: + type: string - description: |- JSON specifying filter conditions. Accepts the same format as returned by the [queries](https://www.openproject.org/docs/api/endpoints/queries/) endpoint. Currently supported filters are: