Correctly use the me_value replacements in the CF filter base

pull/6097/head
Oliver Günther 7 years ago
parent 941ed90b6a
commit b70a891b47
No known key found for this signature in database
GPG Key ID: 88872239EB414F99
  1. 3
      app/models/queries/filters/base.rb
  2. 66
      app/models/queries/filters/shared/custom_field_filter.rb
  3. 36
      app/models/queries/filters/shared/custom_fields/base.rb
  4. 5
      app/models/queries/filters/shared/custom_fields/user.rb
  5. 27
      app/models/queries/work_packages/filter/me_value_filter_mixin.rb
  6. 32
      app/models/queries/work_packages/filter/principal_base_filter.rb
  7. 198
      spec/features/timelines/view_custom_fields_spec.rb
  8. 181
      spec/features/work_packages/table/queries/me_filter_spec.rb

@ -140,6 +140,9 @@ class Queries::Filters::Base
@values || []
end
# Values may contain an internal representation for some filters
alias :values_replaced :values
def values=(values)
@values = Array(values).map(&:to_s)
end

@ -30,70 +30,12 @@
module Queries::Filters::Shared::CustomFieldFilter
def self.included(base)
base.include(InstanceMethods)
base.extend(ClassMethods)
base.class_eval do
attr_accessor :custom_field
validate :custom_field_valid
class_attribute :custom_field_context
end
end
module InstanceMethods
def error_messages
messages = errors
.full_messages
.join(" #{I18n.t('support.array.sentence_connector')} ")
human_name + I18n.t(default: ' %<message>s', message: messages)
end
private
def type_strategy
@type_strategy ||= (strategies[type] || strategies[:inexistent]).new(self)
end
def custom_field_valid
if custom_field.nil?
errors.add(:base, I18n.t('activerecord.errors.models.query.filters.custom_fields.inexistent'))
elsif invalid_custom_field_for_context?
errors.add(:base, I18n.t('activerecord.errors.models.query.filters.custom_fields.invalid'))
end
end
def validate_inclusion_of_operator
super if custom_field
end
def invalid_custom_field_for_context?
project && invalid_custom_field_for_project? ||
!project && invalid_custom_field_globally?
end
def invalid_custom_field_globally?
!self.class.custom_fields(project)
.exists?(custom_field.id)
end
def invalid_custom_field_for_project?
!self.class.custom_fields(project)
.map(&:id).include? custom_field.id
end
def strategies
strategies = Queries::Filters::STRATEGIES.dup
strategies[:list_optional] = Queries::Filters::Strategies::CfListOptional
strategies[:integer] = Queries::Filters::Strategies::CfInteger
# knowing that only bool have list type
strategies[:list] = Queries::Filters::Strategies::BooleanList
strategies
end
end
module ClassMethods
def key
/cf_(\d+)/
@ -121,13 +63,11 @@ module Queries::Filters::Shared::CustomFieldFilter
##
# Find the given custom field by its accessor, should it exist.
def find_by_accessor(name)
match = name.match /(custom_field_|cf_)(\d+)/
match = name.match /cf_(\d+)/
if match.present? && match[2].to_i > 0
custom_field_context.custom_field_class.find_by(id: match[2])
if match.present? && match[1].to_i > 0
custom_field_context.custom_field_class.find_by(id: match[1])
end
nil
end
##

@ -34,6 +34,8 @@ module Queries::Filters::Shared
attr_reader :custom_field
attr_reader :custom_field_context
validate :custom_field_valid
def initialize(custom_field, custom_field_context, options = {})
name = :"cf_#{custom_field.id}"
@ -104,15 +106,47 @@ module Queries::Filters::Shared
(SELECT #{model_db_table}.id
FROM #{model_db_table}
#{custom_field_context.where_subselect_joins(custom_field)}
WHERE #{operator_strategy.sql_for_field(values, cv_db_table, 'value')})
WHERE #{operator_strategy.sql_for_field(values_replaced, cv_db_table, 'value')})
SQL
end
def error_messages
messages = errors
.full_messages
.join(" #{I18n.t('support.array.sentence_connector')} ")
human_name + I18n.t(default: ' %<message>s', message: messages)
end
protected
def type_strategy_class
strategies[type] || strategies[:inexistent]
end
def custom_field_valid
if invalid_custom_field_for_context?
errors.add(:base, I18n.t('activerecord.errors.models.query.filters.custom_fields.invalid'))
end
end
def invalid_custom_field_for_context?
if project
invalid_custom_field_for_project?
else
invalid_custom_field_globally?
end
end
def invalid_custom_field_globally?
!self.custom_field_context.custom_fields(project)
.exists?(custom_field.id)
end
def invalid_custom_field_for_project?
!self.custom_field_context.custom_fields(project)
.map(&:id).include? custom_field.id
end
end
end
end

@ -39,6 +39,11 @@ module Queries::Filters::Shared
# from the Me mixin, which will override the ListOptional value_objects definition.
include ::Queries::WorkPackages::Filter::MeValueFilterMixin
def allowed_values
@allowed_values ||= begin
me_allowed_value + super
end
end
end
end
end

@ -45,6 +45,31 @@ module Queries::WorkPackages::Filter::MeValueFilterMixin
Principal.where(id: prepared_values)
end
##
# Returns a hash of the value objects with the me value referenced
# in order for the representer to identify it.
def value_objects_hash
objects = super
# Replace me value identifier
if has_me_value?
search = User.current.id
objects.each do |value_object|
if value_object[:id] == search
value_object[:id] = me_value
value_object[:name] = me_label
break
end
end
end
objects
end
def values
super
end
##
# Return the values object with the me value
# mapped to the current user.
@ -77,7 +102,7 @@ module Queries::WorkPackages::Filter::MeValueFilterMixin
def me_allowed_value
values = []
if User.current.logged?
values << [me_abel, me_value]
values << [me_label, me_value]
end
values
end

@ -37,24 +37,6 @@ class Queries::WorkPackages::Filter::PrincipalBaseFilter <
User.current.logged? || allowed_values.any?
end
def value_objects_hash
objects = super
# Replace me value identifier
if has_me_value?
search = User.current.id
objects.each do |value_object|
if value_object[:id] == search
value_object[:id] = me_value
value_object[:name] = me_label
break
end
end
end
objects
end
def ar_object_filter?
true
end
@ -72,18 +54,4 @@ class Queries::WorkPackages::Filter::PrincipalBaseFilter <
def principal_loader
@principal_loader ||= ::Queries::WorkPackages::Filter::PrincipalLoader.new(project)
end
def values_replaced
vals = values.clone
if vals.delete(me_value)
if User.current.logged?
vals.push(User.current.id.to_s)
else
vals.push('0')
end
end
vals
end
end

@ -1,198 +0,0 @@
#-- copyright
# OpenProject is a project management system.
# Copyright (C) 2012-2017 the OpenProject Foundation (OPF)
#
# 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 doc/COPYRIGHT.rdoc for more details.
#++
require 'spec_helper'
describe Timeline, 'view custom fields', type: :feature, js: true do
let(:role) { FactoryGirl.create(:role, permissions: permissions) }
let(:permissions) do
[:view_work_packages,
:view_timelines,
:edit_timelines,
:edit_work_packages]
end
let(:project) { FactoryGirl.create(:project, name: "Lil'ol'project") }
let(:user) { FactoryGirl.create(:user, member_in_project: project, member_through_role: role) }
let(:other_user) do
FactoryGirl.create(:user,
firstname: 'Other',
lastname: 'User',
member_in_project: project,
member_through_role: role)
end
let(:type) { project.types.first }
let(:bool_cf) do
field = FactoryGirl.create(:bool_wp_custom_field,
name: 'A_bool',
is_filter: true,
is_for_all: true)
type.custom_fields << field
field
end
let(:list_cf) do
field = FactoryGirl.create(:list_wp_custom_field,
name: 'A_list',
is_filter: true,
is_for_all: true,
possible_values: ['A_list_value',
'B_list_value',
'C_list_value'])
type.custom_fields << field
field
end
let(:bool_cf_local) do
field = FactoryGirl.create(:bool_wp_custom_field,
name: 'A_local_bool',
is_filter: true,
is_for_all: false)
type.custom_fields << field
project.work_package_custom_fields << field
field
end
let(:user_cf_local) do
field = FactoryGirl.create(:user_wp_custom_field,
name: 'A_user',
is_filter: true,
is_for_all: false)
type.custom_fields << field
project.work_package_custom_fields << field
field
end
let(:work_package1) do
wp = FactoryGirl.build(:work_package,
subject: "Lil'ol'wp",
assigned_to: other_user,
type: type,
project: project)
wp.custom_field_values = { bool_cf.id => true,
bool_cf_local.id => false,
user_cf_local.id => user,
list_cf.id => list_cf.possible_values.first.id }
wp.save!
wp
end
let(:timeline) do
FactoryGirl.create(:timeline, project: project)
end
before do
work_package1
login_as(user)
end
include_context 'ui-select helpers'
it 'displays custom values' do
visit edit_project_timeline_path(project_id: project, id: timeline)
select = page.find('#s2id_timeline_options_columns_')
ui_select(select, list_cf.name)
ui_select(select, 'Assignee')
ui_select(select, bool_cf_local.name)
click_button 'Save'
within '#timeline' do
expect(page).to have_content(work_package1.subject)
expect(page).to have_selector('th:nth-of-type(2)',
text: list_cf.name)
expect(page).to have_selector('td:nth-of-type(2)',
text: list_cf.possible_values.first.value)
expect(page).to have_selector('th:nth-of-type(3)',
text: 'Assignee')
expect(page).to have_selector('td:nth-of-type(3)',
text: work_package1.assigned_to.name)
expect(page).to have_selector('th:nth-of-type(4)',
text: bool_cf_local.name)
expect(page).to have_selector('td:nth-of-type(4)',
text: 'No')
expect(page).to have_no_selector('td', text: 'Yes')
expect(page).to have_no_selector('td', text: user.name)
end
visit edit_project_timeline_path(project_id: project, id: timeline)
select = page.find('#s2id_timeline_options_columns_')
ui_select_clear(select)
ui_select(select, bool_cf.name)
ui_select(select, user_cf_local.name)
click_button 'Save'
within '#timeline' do
expect(page).to have_content(work_package1.subject)
expect(page).to have_selector('th:nth-of-type(2)',
text: bool_cf.name)
expect(page).to have_selector('td:nth-of-type(2)',
text: 'Yes')
expect(page).to have_selector('th:nth-of-type(3)',
text: user_cf_local.name)
expect(page).to have_selector('td:nth-of-type(3)',
text: user.name)
expect(page).to have_no_selector('td', text: 'No')
expect(page).to have_no_selector('td', text: list_cf.possible_values.first.value)
expect(page).to have_no_selector('td', text: work_package1.assigned_to.name)
end
# if the custom value has been deactivated in the project
# the value is no longer displayed
project.work_package_custom_fields = []
visit project_timeline_path(project_id: project, id: timeline)
within '#timeline' do
expect(page).to have_content(work_package1.subject)
expect(page).to have_selector('th:nth-of-type(2)',
text: bool_cf.name)
expect(page).to have_selector('td:nth-of-type(2)',
text: 'Yes')
expect(page).to have_no_selector('td', text: user.name)
expect(page).to have_no_selector('td', text: 'No')
expect(page).to have_no_selector('td', text: list_cf.possible_values.first.value)
expect(page).to have_no_selector('td', text: work_package1.assigned_to.name)
end
end
end

@ -33,9 +33,6 @@ describe 'filter me value', js: true do
let(:role) { FactoryGirl.create :existing_role, permissions: [:view_work_packages] }
let(:admin) { FactoryGirl.create :admin }
let(:user) { FactoryGirl.create :user }
let(:wp_admin) { FactoryGirl.create :work_package, project: project, assigned_to: admin }
let(:wp_user) { FactoryGirl.create :work_package, project: project, assigned_to: user }
let(:wp_table) { ::Pages::WorkPackagesTable.new(project) }
let(:filters) { ::Components::WorkPackages::Filters.new }
@ -45,63 +42,161 @@ describe 'filter me value', js: true do
project.add_member! user, role
end
context 'as anonymous', with_settings: { login_required?: false } do
let(:assignee_query) do
query = FactoryGirl.create(:query,
name: 'Assignee Query',
project: project,
user: user)
describe 'assignee' do
let(:wp_admin) { FactoryGirl.create :work_package, project: project, assigned_to: admin }
let(:wp_user) { FactoryGirl.create :work_package, project: project, assigned_to: user }
context 'as anonymous', with_settings: { login_required?: false } do
let(:assignee_query) do
query = FactoryGirl.create(:query,
name: 'Assignee Query',
project: project,
user: user)
query.add_filter('assigned_to_id', '=', ['me'])
query.save!(validate: false)
query.add_filter('assigned_to_id', '=', ['me'])
query.save!(validate: false)
query
end
query
it 'shows an error visiting a query with a me value' do
wp_table.visit_query assignee_query
wp_table.expect_notification(type: :error,
message: I18n.t('js.work_packages.faulty_query.description'))
end
end
context 'logged in' do
before do
wp_admin
wp_user
login_as(admin)
end
it 'shows the one work package filtering for myself' do
wp_table.visit!
wp_table.expect_work_package_listed(wp_admin, wp_user)
# Add and save query with me filter
filters.open
filters.remove_filter 'status'
filters.add_filter_by('Assignee', 'is', 'me')
wp_table.expect_work_package_not_listed(wp_user)
wp_table.expect_work_package_listed(wp_admin)
wp_table.save_as('Me query')
loading_indicator_saveguard
# Expect correct while saving
wp_table.expect_title 'Me query'
query = Query.last
expect(query.filters.first.values).to eq ['me']
filters.expect_filter_by('Assignee', 'is', 'me')
it 'shows an error visiting a query with a me value' do
wp_table.visit_query assignee_query
wp_table.expect_notification(type: :error,
message: I18n.t('js.work_packages.faulty_query.description'))
# Revisit query
wp_table.visit_query query
wp_table.expect_work_package_not_listed(wp_user)
wp_table.expect_work_package_listed(wp_admin)
filters.open
filters.expect_filter_by('Assignee', 'is', 'me')
end
end
end
context 'logged in' do
before do
wp_admin
wp_user
describe 'custom_field of type user' do
let(:custom_field) {
FactoryGirl.create(
:work_package_custom_field,
name: 'CF user',
field_format: 'user',
is_required: false
)
}
let(:type_task) { FactoryGirl.create(:type_task, custom_fields: [custom_field]) }
let(:project) {
FactoryGirl.create(:project,
types: [type_task],
work_package_custom_fields: [custom_field])
}
let(:cf_accessor) { "cf_#{custom_field.id}" }
let(:cf_accessor_frontend) { "customField#{custom_field.id}" }
let(:wp_admin) do
FactoryGirl.create :work_package,
type: type_task,
project: project,
custom_field_values: { custom_field.id => admin.id }
end
let(:wp_user) do
FactoryGirl.create :work_package,
type: type_task,
project: project,
custom_field_values: { custom_field.id => user.id }
end
context 'as anonymous', with_settings: { login_required?: false } do
let(:assignee_query) do
query = FactoryGirl.create(:query,
name: 'CF user Query',
project: project,
user: user)
login_as(admin)
query.add_filter(cf_accessor, '=', ['me'])
query.save!(validate: false)
query
end
it 'shows an error visiting a query with a me value' do
wp_table.visit_query assignee_query
wp_table.expect_notification(type: :error,
message: I18n.t('js.work_packages.faulty_query.description'))
end
end
it 'shows the one work package filtering for myself' do
wp_table.visit!
wp_table.expect_work_package_listed(wp_admin, wp_user)
context 'logged in' do
before do
wp_admin
wp_user
login_as(admin)
end
it 'shows the one work package filtering for myself' do
wp_table.visit!
wp_table.expect_work_package_listed(wp_admin, wp_user)
# Add and save query with me filter
filters.open
filters.remove_filter 'status'
filters.add_filter_by('Assignee', 'is', 'me')
# Add and save query with me filter
filters.open
filters.remove_filter 'status'
filters.add_filter_by('CF user', 'is', 'me', cf_accessor_frontend)
wp_table.expect_work_package_not_listed(wp_user)
wp_table.expect_work_package_listed(wp_admin)
wp_table.expect_work_package_not_listed(wp_user)
wp_table.expect_work_package_listed(wp_admin)
wp_table.save_as('Me query')
loading_indicator_saveguard
wp_table.save_as('Me query')
loading_indicator_saveguard
# Expect correct while saving
wp_table.expect_title 'Me query'
query = Query.last
expect(query.filters.first.values).to eq ['me']
filters.expect_filter_by('Assignee', 'is', 'me')
# Expect correct while saving
wp_table.expect_title 'Me query'
query = Query.last
expect(query.filters.first.values).to eq ['me']
filters.expect_filter_by('CF user', 'is', 'me', cf_accessor_frontend)
# Revisit query
wp_table.visit_query query
wp_table.expect_work_package_not_listed(wp_user)
wp_table.expect_work_package_listed(wp_admin)
# Revisit query
wp_table.visit_query query
wp_table.expect_work_package_not_listed(wp_user)
wp_table.expect_work_package_listed(wp_admin)
filters.open
filters.expect_filter_by('Assignee', 'is', 'me')
filters.open
filters.expect_filter_by('CF user', 'is', 'me', cf_accessor_frontend)
end
end
end
end

Loading…
Cancel
Save