TheDude
/
openproject
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

always use list cf subquery on list custom field filtering

Browse Source 
The check in place before would check if any custom value, when casted to int would not be greater than zero. This also applies to nil -> 0. This then lead to the non list cf subquery being used when filtering. As a lot of custom values have nil for their value attribute, filtering for list custom fields was only possible in very rare cases.
pull/8721/head
ulferts 4 years ago committed by Oliver Günther
parent d74ba0134c
commit 296d19163c
2 changed files with 65 additions and 68 deletions
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Show Stats Download Patch File Download Diff File
  1. 16
      modules/reporting/app/models/cost_query/custom_field_mixin.rb
  2. 117
      modules/reporting/spec/features/custom_fields_spec.rb

16
modules/reporting/app/models/cost_query/custom_field_mixin.rb
Unescape View File

@ -98,7 +98,7 @@ module CostQuery::CustomFieldMixin
@class_name = class_name
dont_inherit :group_fields
db_field table_name
if field.list? && all_values_int?(field)
if field.list?
join_table list_join_table(field)
else
join_table default_join_table(field)
@ -107,15 +107,6 @@ module CostQuery::CustomFieldMixin
self
end
##
# HACK: CustomValues of lists MAY have non-integer values when the list
# contained invalid values.
def all_values_int?(field)
field.custom_values.pluck(:value).all? { |val| val.to_i > 0 }
rescue StandardError
false
end
def list_join_table(field)
cast_as = SQL_TYPES[field.field_format]
cf_name = "custom_field#{field.id}"
@ -123,6 +114,9 @@ module CostQuery::CustomFieldMixin
custom_values_table = CustomValue.table_name
custom_options_table = CustomOption.table_name
# CustomValues of lists MAY have non-integer values when the list contained invalid values.
# Because of this, we do not cast the cv.value but rather the co.id
<<-SQL
-- BEGIN Custom Field Join: #{cf_name}
LEFT OUTER JOIN (
@ -133,7 +127,7 @@ module CostQuery::CustomFieldMixin
cv.customized_id
FROM #{custom_values_table} cv
INNER JOIN #{custom_options_table} co
ON cv.custom_field_id = co.custom_field_id AND CAST(cv.value AS decimal(60,3)) = co.id
ON cv.custom_field_id = co.custom_field_id AND cv.value = co.id::VARCHAR
) AS #{cf_name}
ON #{cf_name}.customized_type = 'WorkPackage'

117
modules/reporting/spec/features/custom_fields_spec.rb
Unescape View File

@ -34,27 +34,28 @@ describe 'Custom fields reporting', type: :feature, js: true do
let(:user) { FactoryBot.create :admin }
let(:work_package) {
let(:work_package) do
FactoryBot.create :work_package,
project: project,
custom_values: initial_custom_values
}
project: project,
type: type,
custom_values: initial_custom_values
end
let!(:time_entry1) {
let!(:time_entry1) do
FactoryBot.create :time_entry,
user: user,
work_package: work_package,
project: project,
hours: 10
}
user: user,
work_package: work_package,
project: project,
hours: 10
end
let!(:time_entry2) {
let!(:time_entry2) do
FactoryBot.create :time_entry,
user: user,
work_package: work_package,
project: project,
hours: 2.50
}
user: user,
work_package: work_package,
project: project,
hours: 2.50
end
def custom_value_for(cf, str)
@ -63,19 +64,25 @@ describe 'Custom fields reporting', type: :feature, js: true do
context 'with multi value cf' do
let!(:custom_field) do
FactoryBot.create(
:list_wp_custom_field,
name: "List CF",
multi_value: true,
types: [type],
projects: [project],
possible_values: ['First option', 'Second option']
)
FactoryBot.create(:list_wp_custom_field,
name: "List CF",
multi_value: true,
types: [type],
projects: [project],
possible_values: ['First option', 'Second option'])
end
let(:initial_custom_values) { { custom_field.id => custom_value_for(custom_field, 'First option') } }
let(:cf_id) { "custom_field#{custom_field.id}" }
# Have a second work package in the test that will have no values
# as this caused problems with casting the nil value of the custom value to 0.
let!(:work_package2) do
FactoryBot.create :work_package,
project: project,
type: type
end
before do
login_as(user)
visit '/cost_reports'
@ -85,31 +92,31 @@ describe 'Custom fields reporting', type: :feature, js: true do
expect(page).to have_selector('#add_filter_select option', text: 'List CF')
select 'List CF', from: 'add_filter_select'
# Adds filter to page
# Adds filter to page, filtering out the time entries on the work package
expect(page).to have_selector("label##{cf_id}")
custom_field_selector = "##{cf_id}_arg_1_val"
select = find(custom_field_selector)
expect(select).to have_selector('option', text: 'First option')
expect(select).to have_selector('option', text: 'Second option')
select.find('option', text: 'First option').select_option
select.find('option', text: 'Second option').select_option
find('#query-icon-apply-button').click
# Expect row of work package
# Expect empty result table
within('#result-table') do
expect(page).to have_selector('.top.result', text: '12.50 hours')
expect(page).to have_no_selector('.top.result', text: '12.50 hours')
end
expect(page).to have_selector('.generic-table--no-results-title')
# Update filter to other value
# Update filter to value the work package has
select = find(custom_field_selector)
select.find('option', text: 'Second option').select_option
select.find('option', text: 'First option').select_option
find('#query-icon-apply-button').click
# Expect empty result table
# Expect row of work package
within('#result-table') do
expect(page).to have_no_selector('.top.result', text: '12.50 hours')
expect(page).to have_selector('.top.result', text: '12.50 hours')
end
expect(page).to have_selector('.generic-table--no-results-title')
end
it 'groups by the multi CF (Regression #26050)' do
@ -141,29 +148,27 @@ describe 'Custom fields reporting', type: :feature, js: true do
context 'with additional WP with invalid value' do
let!(:custom_field_2) do
FactoryBot.create(
:list_wp_custom_field,
name: "Invalid List CF",
multi_value: true,
types: [type],
projects: [project],
possible_values: ['A', 'B']
)
FactoryBot.create(:list_wp_custom_field,
name: "Invalid List CF",
multi_value: true,
types: [type],
projects: [project],
possible_values: %w[A B])
end
let!(:work_package2) {
let!(:work_package2) do
FactoryBot.create :work_package,
project: project,
custom_values: { custom_field_2.id => custom_value_for(custom_field_2, 'A')}
}
project: project,
custom_values: { custom_field_2.id => custom_value_for(custom_field_2, 'A')}
end
let!(:time_entry1) {
let!(:time_entry1) do
FactoryBot.create :time_entry,
user: user,
work_package: work_package2,
project: project,
hours: 10
}
user: user,
work_package: work_package2,
project: project,
hours: 10
end
before do
CustomValue.find_by(customized_id: work_package2.id).update_columns(value: 'invalid')
@ -196,12 +201,10 @@ describe 'Custom fields reporting', type: :feature, js: true do
context 'with text CF' do
let(:custom_field) do
FactoryBot.create(
:text_wp_custom_field,
name: 'Text CF',
types: [type],
projects: [project]
)
FactoryBot.create(:text_wp_custom_field,
name: 'Text CF',
types: [type],
projects: [project])
end
let(:initial_custom_values) { { custom_field.id => 'foo' } }

Write Preview
Loading…
Cancel
Save