From ac27dba49346fc38c5c3aa5f09ee2cfe3364a9a5 Mon Sep 17 00:00:00 2001 From: Christophe Bliard Date: Tue, 17 Jan 2023 12:03:18 +0100 Subject: [PATCH] refactor: Extract CustomField#column_name for "cf_#{id}" --- app/cells/projects/table_cell.rb | 4 +- app/models/custom_field.rb | 4 ++ .../filters/shared/custom_field_filter.rb | 22 +++----- .../filters/shared/custom_fields/base.rb | 2 +- .../columns/custom_field_column.rb | 2 +- .../work_packages/filter/search_filter.rb | 2 +- app/services/custom_fields/create_service.rb | 6 +-- .../spec/lib/custom_field_xls_export_spec.rb | 4 +- .../project/exporter/xls_integration_spec.rb | 2 +- .../exporter/xls_integration_spec.rb | 2 +- .../multi_value_custom_field_spec.rb | 4 +- spec/features/projects/projects_index_spec.rb | 22 ++++---- .../projects/projects_portfolio_spec.rb | 4 +- .../new/attributes_from_filter_spec.rb | 2 +- .../work_packages/table/switch_types_spec.rb | 2 +- .../query_filter_instance_representer_spec.rb | 2 +- spec/models/custom_field_spec.rb | 8 +++ .../projects/exporter/csv_integration_spec.rb | 2 +- .../filters/custom_field_filter_spec.rb | 46 ++++++++-------- .../columns/custom_field_column_spec.rb | 11 ++-- .../columns/shared_query_column_specs.rb | 24 +++++---- .../contains_text_custom_field_filter_spec.rb | 2 +- .../custom_fields/custom_field_filter_spec.rb | 53 +++++++++---------- .../results_cf_sorting_integration_spec.rb | 4 +- .../query/results_sort_intergration_spec.rb | 6 +-- spec/models/query/results_spec.rb | 16 +++--- .../query/results_sums_integration_spec.rb | 4 +- spec/models/query_spec.rb | 6 +-- .../custom_fields/create_service_spec.rb | 4 +- 29 files changed, 136 insertions(+), 136 deletions(-) diff --git a/app/cells/projects/table_cell.rb b/app/cells/projects/table_cell.rb index 822af6e142..573cd7134c 100644 --- a/app/cells/projects/table_cell.rb +++ b/app/cells/projects/table_cell.rb @@ -88,7 +88,7 @@ module Projects def custom_field_columns project_custom_fields.values.map do |custom_field| - [:"cf_#{custom_field.id}", { caption: custom_field.name, custom_field: true }] + [custom_field.column_name.to_sym, { caption: custom_field.name, custom_field: true }] end end @@ -102,7 +102,7 @@ module Projects end fields - .index_by { |cf| :"cf_#{cf.id}" } + .index_by { |cf| cf.column_name.to_sym } end end end diff --git a/app/models/custom_field.rb b/app/models/custom_field.rb index 9e72d5833a..54cd5d98d9 100644 --- a/app/models/custom_field.rb +++ b/app/models/custom_field.rb @@ -244,6 +244,10 @@ class CustomField < ApplicationRecord :"#{attribute_name}=" end + def column_name + "cf_#{id}" + end + def type_name nil end diff --git a/app/models/queries/filters/shared/custom_field_filter.rb b/app/models/queries/filters/shared/custom_field_filter.rb index 826c133524..6fb695cc77 100644 --- a/app/models/queries/filters/shared/custom_field_filter.rb +++ b/app/models/queries/filters/shared/custom_field_filter.rb @@ -39,23 +39,13 @@ module Queries::Filters::Shared::CustomFieldFilter /cf_(\d+)/ end - ## - # TODO this differs from CustomField#attribute_name for reasons I don't see, - # however this name will be persisted in queries so we can't just map one to the other. - def custom_field_accessor(custom_field) - "cf_#{custom_field.id}" - end - def all_for(context = nil) - custom_field_context.custom_fields(context).map do |cf| - cf_accessor = custom_field_accessor(cf) - begin - create!(name: cf_accessor, custom_field: cf, context:) - rescue ::Queries::Filters::InvalidError - Rails.logger.error "Failed to map custom field filter for #{cf_accessor} (CF##{cf.id}." - nil - end - end.compact + custom_field_context.custom_fields(context).filter_map do |cf| + create!(name: cf.column_name, custom_field: cf, context:) + rescue ::Queries::Filters::InvalidError + Rails.logger.error "Failed to map custom field filter for #{cf.column_name} (CF##{cf.id})." + nil + end end ## diff --git a/app/models/queries/filters/shared/custom_fields/base.rb b/app/models/queries/filters/shared/custom_fields/base.rb index 07eb4f9e59..c32704ba91 100644 --- a/app/models/queries/filters/shared/custom_fields/base.rb +++ b/app/models/queries/filters/shared/custom_fields/base.rb @@ -34,7 +34,7 @@ module Queries::Filters::Shared attr_reader :custom_field, :custom_field_context def initialize(custom_field:, custom_field_context:, **options) - name = :"cf_#{custom_field.id}" + name = custom_field.column_name.to_sym @custom_field = custom_field @custom_field_context = custom_field_context diff --git a/app/models/queries/work_packages/columns/custom_field_column.rb b/app/models/queries/work_packages/columns/custom_field_column.rb index b1328c97bd..f8c44a781f 100644 --- a/app/models/queries/work_packages/columns/custom_field_column.rb +++ b/app/models/queries/work_packages/columns/custom_field_column.rb @@ -69,7 +69,7 @@ class Queries::WorkPackages::Columns::CustomFieldColumn < Queries::WorkPackages: private def set_name! - self.name = "cf_#{custom_field.id}".to_sym + self.name = custom_field.column_name.to_sym end def set_sortable! diff --git a/app/models/queries/work_packages/filter/search_filter.rb b/app/models/queries/work_packages/filter/search_filter.rb index 57007c38a9..cc7287cf09 100644 --- a/app/models/queries/work_packages/filter/search_filter.rb +++ b/app/models/queries/work_packages/filter/search_filter.rb @@ -91,7 +91,7 @@ class Queries::WorkPackages::Filter::SearchFilter < custom_fields.map do |custom_field| Queries::WorkPackages::Filter::FilterConfiguration.new( Queries::WorkPackages::Filter::CustomFieldFilter, - "cf_#{custom_field.id}", + custom_field.column_name, CONTAINS_OPERATOR ) end diff --git a/app/services/custom_fields/create_service.rb b/app/services/custom_fields/create_service.rb index 060f09ad4f..d81a50d7f9 100644 --- a/app/services/custom_fields/create_service.rb +++ b/app/services/custom_fields/create_service.rb @@ -55,7 +55,7 @@ module CustomFields cf = call.result if cf.is_a?(ProjectCustomField) - add_cf_to_visible_columns(cf.id) + add_cf_to_visible_columns(cf) end call @@ -63,8 +63,8 @@ module CustomFields private - def add_cf_to_visible_columns(id) - Setting.enabled_projects_columns = (Setting.enabled_projects_columns + ["cf_#{id}"]).uniq + def add_cf_to_visible_columns(custom_field) + Setting.enabled_projects_columns = (Setting.enabled_projects_columns + [custom_field.column_name]).uniq end end end diff --git a/modules/xls_export/spec/lib/custom_field_xls_export_spec.rb b/modules/xls_export/spec/lib/custom_field_xls_export_spec.rb index 8736b4d07e..f903182dc8 100644 --- a/modules/xls_export/spec/lib/custom_field_xls_export_spec.rb +++ b/modules/xls_export/spec/lib/custom_field_xls_export_spec.rb @@ -40,7 +40,7 @@ describe "WorkPackageXlsExport Custom Fields" do let!(:query) do query = build(:query, user: current_user, project:) - query.column_names = ['subject', "cf_#{custom_field.id}"] + query.column_names = ['subject', custom_field.column_name] query.sort_criteria = [%w[id asc]] query.save! @@ -67,7 +67,7 @@ describe "WorkPackageXlsExport Custom Fields" do end it 'produces the valid XLS result' do - expect(query.columns.map(&:name)).to eq [:subject, :"cf_#{custom_field.id}"] + expect(query.columns.map(&:name)).to eq [:subject, custom_field.column_name.to_sym] expect(sheet.rows.first.take(2)).to eq ['Subject', 'Ingredients'] # Subjects diff --git a/modules/xls_export/spec/models/xls_export/project/exporter/xls_integration_spec.rb b/modules/xls_export/spec/models/xls_export/project/exporter/xls_integration_spec.rb index fa23f5b082..2af4289a1d 100644 --- a/modules/xls_export/spec/models/xls_export/project/exporter/xls_integration_spec.rb +++ b/modules/xls_export/spec/models/xls_export/project/exporter/xls_integration_spec.rb @@ -46,7 +46,7 @@ describe XlsExport::Project::Exporter::XLS do describe 'custom field columns selected' do before do - Setting.enabled_projects_columns += custom_fields.map { |cf| "cf_#{cf.id}" } + Setting.enabled_projects_columns += custom_fields.map(&:column_name) end context 'when ee enabled', with_ee: %i[custom_fields_in_projects_list] do diff --git a/modules/xls_export/spec/models/xls_export/work_package/exporter/xls_integration_spec.rb b/modules/xls_export/spec/models/xls_export/work_package/exporter/xls_integration_spec.rb index eb09e8820c..cb1578c880 100644 --- a/modules/xls_export/spec/models/xls_export/work_package/exporter/xls_integration_spec.rb +++ b/modules/xls_export/spec/models/xls_export/work_package/exporter/xls_integration_spec.rb @@ -195,7 +195,7 @@ describe XlsExport::WorkPackage::Exporter::XLS do wps[3].save! wps end - let(:column_names) { ['subject', 'status', 'estimated_hours', "cf_#{custom_field.id}"] } + let(:column_names) { ['subject', 'status', 'estimated_hours', custom_field.column_name] } before do allow(Setting) diff --git a/spec/features/custom_fields/multi_value_custom_field_spec.rb b/spec/features/custom_fields/multi_value_custom_field_spec.rb index f654ecbaff..56b6af36d9 100644 --- a/spec/features/custom_fields/multi_value_custom_field_spec.rb +++ b/spec/features/custom_fields/multi_value_custom_field_spec.rb @@ -190,10 +190,10 @@ describe "multi select custom values", js: true do let(:wp2_field) { table_edit_field(work_package2) } let!(:query) do query = build(:query, user:, project:) - query.column_names = ['id', 'type', 'subject', "cf_#{custom_field.id}"] + query.column_names = ['id', 'type', 'subject', custom_field.column_name] query.filters.clear query.timeline_visible = false - query.sort_criteria = [["cf_#{custom_field.id}", 'asc']] + query.sort_criteria = [[custom_field.column_name, 'asc']] query.save! query diff --git a/spec/features/projects/projects_index_spec.rb b/spec/features/projects/projects_index_spec.rb index aa6fce0ce8..679eb74b44 100644 --- a/spec/features/projects/projects_index_spec.rb +++ b/spec/features/projects/projects_index_spec.rb @@ -209,7 +209,7 @@ describe 'Projects index page', end it 'CF columns and filters are visible when added to settings' do - Setting.enabled_projects_columns += ["cf_#{custom_field.id}", "cf_#{invisible_custom_field.id}"] + Setting.enabled_projects_columns += [custom_field.column_name, invisible_custom_field.column_name] load_and_open_filters admin # CF's column is present: @@ -669,7 +669,7 @@ describe 'Projects index page', SeleniumHubWaiter.wait remove_filter('latest_activity_at') - projects_page.set_filter("cf_#{list_custom_field.id}", + projects_page.set_filter(list_custom_field.column_name, list_custom_field.name, 'is', [list_custom_field.possible_values[2].value]) @@ -680,7 +680,7 @@ describe 'Projects index page', expect(page).not_to have_text(project_created_on_fixed_date.name) # switching to multiselect keeps the current selection - cf_filter = page.find("li[filter-name='cf_#{list_custom_field.id}']") + cf_filter = page.find("li[filter-name='#{list_custom_field.column_name}']") within(cf_filter) do # Initial filter is a 'single select' expect(cf_filter.find(:select, 'value')).not_to be_multiple @@ -696,7 +696,7 @@ describe 'Projects index page', click_on 'Apply' - cf_filter = page.find("li[filter-name='cf_#{list_custom_field.id}']") + cf_filter = page.find("li[filter-name='#{list_custom_field.column_name}']") within(cf_filter) do # Query has two values for that filter, so it should show a 'multi select'. expect(cf_filter.find(:select, 'value')).to be_multiple @@ -718,7 +718,7 @@ describe 'Projects index page', click_on 'Apply' - cf_filter = page.find("li[filter-name='cf_#{list_custom_field.id}']") + cf_filter = page.find("li[filter-name='#{list_custom_field.column_name}']") within(cf_filter) do # Query has one value for that filter, so it should show a 'single select'. expect(cf_filter.find(:select, 'value')).not_to be_multiple @@ -726,9 +726,9 @@ describe 'Projects index page', # CF date filter work (at least for one operator) SeleniumHubWaiter.wait - remove_filter("cf_#{list_custom_field.id}") + remove_filter(list_custom_field.column_name) - projects_page.set_filter("cf_#{date_custom_field.id}", + projects_page.set_filter(date_custom_field.column_name, date_custom_field.name, 'on', ['2011-11-11']) @@ -914,7 +914,7 @@ describe 'Projects index page', end it 'allows to alter the order in which projects are displayed' do - Setting.enabled_projects_columns += ["cf_#{integer_custom_field.id}"] + Setting.enabled_projects_columns += [integer_custom_field.column_name] # initially, ordered by name asc on each hierarchical level expect_projects_in_order(development_project, @@ -991,8 +991,8 @@ describe 'Projects index page', allow_enterprise_edition allow(Setting) - .to receive(:enabled_projects_columns) # << "cf_#{list_custom_field.id}" - .and_return ["cf_#{list_custom_field.id}"] + .to receive(:enabled_projects_columns) + .and_return [list_custom_field.column_name] login_as(admin) visit projects_path @@ -1004,7 +1004,7 @@ describe 'Projects index page', .where(value: %w[A B]) .reorder(:id) .pluck(:value) - expect(page).to have_selector(".cf_#{list_custom_field.id}.format-list", text: expected_sort.join(", ")) + expect(page).to have_selector(".#{list_custom_field.column_name}.format-list", text: expected_sort.join(", ")) end end end diff --git a/spec/features/projects/projects_portfolio_spec.rb b/spec/features/projects/projects_portfolio_spec.rb index ff1e6b42e3..9d17486d2b 100644 --- a/spec/features/projects/projects_portfolio_spec.rb +++ b/spec/features/projects/projects_portfolio_spec.rb @@ -96,10 +96,10 @@ describe 'Projects index page', # Check the status and custom field only find('input[value="project_status"]').check - find(%(input[value="cf_#{string_cf.id}"])).check + find(%(input[value="#{string_cf.column_name}"])).check expect(page).to have_selector('input[value="project_status"]:checked') - expect(page).to have_selector(%(input[value="cf_#{string_cf.id}"]:checked)) + expect(page).to have_selector(%(input[value="#{string_cf.column_name}"]:checked)) # Edit the project gantt query scroll_to_and_click(find('button', text: 'Edit query')) diff --git a/spec/features/work_packages/new/attributes_from_filter_spec.rb b/spec/features/work_packages/new/attributes_from_filter_spec.rb index 6622a56c92..5cf1abfe60 100644 --- a/spec/features/work_packages/new/attributes_from_filter_spec.rb +++ b/spec/features/work_packages/new/attributes_from_filter_spec.rb @@ -88,7 +88,7 @@ RSpec.describe 'Work package create uses attributes from filters', js: true, sel let(:filters) do [['type_id', '=', [type_task.id]], - ["cf_#{custom_field.id}", '=', [custom_field.custom_options.detect { |o| o.value == 'A' }.id]]] + [custom_field.column_name, '=', [custom_field.custom_options.detect { |o| o.value == 'A' }.id]]] end it 'allows to save with a single value (Regression test #27833)' do diff --git a/spec/features/work_packages/table/switch_types_spec.rb b/spec/features/work_packages/table/switch_types_spec.rb index bf50fc0f8e..9674a8ed63 100644 --- a/spec/features/work_packages/table/switch_types_spec.rb +++ b/spec/features/work_packages/table/switch_types_spec.rb @@ -42,7 +42,7 @@ describe 'Switching types in work package table', js: true do let(:query) do query = build(:query, user:, project:) - query.column_names = ['id', 'subject', 'type', "cf_#{cf_text.id}"] + query.column_names = ['id', 'subject', 'type', cf_text.column_name] query.save! query diff --git a/spec/lib/api/v3/queries/filters/query_filter_instance_representer_spec.rb b/spec/lib/api/v3/queries/filters/query_filter_instance_representer_spec.rb index ade325fdc3..70b3f4e2d5 100644 --- a/spec/lib/api/v3/queries/filters/query_filter_instance_representer_spec.rb +++ b/spec/lib/api/v3/queries/filters/query_filter_instance_representer_spec.rb @@ -207,7 +207,7 @@ describe API::V3::Queries::Filters::QueryFilterInstanceRepresenter do context 'with a bool custom field filter' do let(:bool_cf) { create(:bool_wp_custom_field) } let(:filter) do - Queries::WorkPackages::Filter::CustomFieldFilter.create!(name: "cf_#{bool_cf.id}", operator:, values:) + Queries::WorkPackages::Filter::CustomFieldFilter.create!(name: bool_cf.column_name, operator:, values:) end context "with 't' as filter value" do diff --git a/spec/models/custom_field_spec.rb b/spec/models/custom_field_spec.rb index e980fbb7f1..a962b6845a 100644 --- a/spec/models/custom_field_spec.rb +++ b/spec/models/custom_field_spec.rb @@ -207,6 +207,14 @@ describe CustomField do it { is_expected.to eq(:"custom_field_#{field.id}=") } end + describe '#column_name' do + let(:field) { build_stubbed(:custom_field) } + + subject { field.column_name } + + it { is_expected.to eq("cf_#{field.id}") } + end + describe '#possible_values_options' do let(:project) { build_stubbed(:project) } let(:user1) { build_stubbed(:user) } diff --git a/spec/models/projects/exporter/csv_integration_spec.rb b/spec/models/projects/exporter/csv_integration_spec.rb index 4e02d3dfef..b164258e40 100644 --- a/spec/models/projects/exporter/csv_integration_spec.rb +++ b/spec/models/projects/exporter/csv_integration_spec.rb @@ -62,7 +62,7 @@ describe Projects::Exports::CSV, 'integration' do describe 'custom field columns selected' do before do - Setting.enabled_projects_columns += custom_fields.map { |cf| "cf_#{cf.id}" } + Setting.enabled_projects_columns += custom_fields.map(&:column_name) end context 'when ee enabled', with_ee: %i[custom_fields_in_projects_list] do diff --git a/spec/models/queries/projects/filters/custom_field_filter_spec.rb b/spec/models/queries/projects/filters/custom_field_filter_spec.rb index 73c8ad8653..bf59ff08b2 100644 --- a/spec/models/queries/projects/filters/custom_field_filter_spec.rb +++ b/spec/models/queries/projects/filters/custom_field_filter_spec.rb @@ -50,7 +50,7 @@ describe Queries::Projects::Filters::CustomFieldFilter do date_project_custom_field, string_project_custom_field] end - let(:cf_accessor) { "cf_#{custom_field.id}" } + let(:cf_accessor) { custom_field.column_name } let(:instance) do described_class.create!(name: cf_accessor, operator: '=', context: query) end @@ -128,7 +128,7 @@ describe Queries::Projects::Filters::CustomFieldFilter do all_custom_fields.each do |cf| name = "cf_#{cf.id}" filter = described_class.create!(name:) - expect(filter.name).to eql(:"cf_#{cf.id}") + expect(filter.name).to eql(cf.column_name.to_sym) expect(filter.order).to be(20) end end @@ -136,7 +136,7 @@ describe Queries::Projects::Filters::CustomFieldFilter do describe '#type' do context 'integer' do - let(:cf_accessor) { "cf_#{int_project_custom_field.id}" } + let(:cf_accessor) { int_project_custom_field.column_name } it 'is integer for an integer' do expect(instance.type) @@ -145,7 +145,7 @@ describe Queries::Projects::Filters::CustomFieldFilter do end context 'float' do - let(:cf_accessor) { "cf_#{float_project_custom_field.id}" } + let(:cf_accessor) { float_project_custom_field.column_name } it 'is integer for a float' do expect(instance.type) @@ -154,7 +154,7 @@ describe Queries::Projects::Filters::CustomFieldFilter do end context 'text' do - let(:cf_accessor) { "cf_#{text_project_custom_field.id}" } + let(:cf_accessor) { text_project_custom_field.column_name } it 'is text for a text' do expect(instance.type) @@ -163,7 +163,7 @@ describe Queries::Projects::Filters::CustomFieldFilter do end context 'list optional' do - let(:cf_accessor) { "cf_#{list_project_custom_field.id}" } + let(:cf_accessor) { list_project_custom_field.column_name } it 'is list_optional for a list' do expect(instance.type) @@ -172,7 +172,7 @@ describe Queries::Projects::Filters::CustomFieldFilter do end context 'user' do - let(:cf_accessor) { "cf_#{user_project_custom_field.id}" } + let(:cf_accessor) { user_project_custom_field.column_name } it 'is list_optional for a user' do expect(instance.type) @@ -181,7 +181,7 @@ describe Queries::Projects::Filters::CustomFieldFilter do end context 'version' do - let(:cf_accessor) { "cf_#{version_project_custom_field.id}" } + let(:cf_accessor) { version_project_custom_field.column_name } it 'is list_optional for a version' do expect(instance.type) @@ -190,7 +190,7 @@ describe Queries::Projects::Filters::CustomFieldFilter do end context 'version' do - let(:cf_accessor) { "cf_#{date_project_custom_field.id}" } + let(:cf_accessor) { date_project_custom_field.column_name } it 'is date for a date' do expect(instance.type) @@ -199,7 +199,7 @@ describe Queries::Projects::Filters::CustomFieldFilter do end context 'bool' do - let(:cf_accessor) { "cf_#{bool_project_custom_field.id}" } + let(:cf_accessor) { bool_project_custom_field.column_name } it 'is list for a bool' do expect(instance.type) @@ -208,7 +208,7 @@ describe Queries::Projects::Filters::CustomFieldFilter do end context 'string' do - let(:cf_accessor) { "cf_#{string_project_custom_field.id}" } + let(:cf_accessor) { string_project_custom_field.column_name } it 'is string for a string' do expect(instance.type) @@ -226,7 +226,7 @@ describe Queries::Projects::Filters::CustomFieldFilter do describe '#allowed_values' do context 'integer' do - let(:cf_accessor) { "cf_#{int_project_custom_field.id}" } + let(:cf_accessor) { int_project_custom_field.column_name } it 'is nil for an integer' do expect(instance.allowed_values) @@ -235,7 +235,7 @@ describe Queries::Projects::Filters::CustomFieldFilter do end context 'float' do - let(:cf_accessor) { "cf_#{float_project_custom_field.id}" } + let(:cf_accessor) { float_project_custom_field.column_name } it 'is integer for a float' do expect(instance.allowed_values) @@ -244,7 +244,7 @@ describe Queries::Projects::Filters::CustomFieldFilter do end context 'text' do - let(:cf_accessor) { "cf_#{text_project_custom_field.id}" } + let(:cf_accessor) { text_project_custom_field.column_name } it 'is text for a text' do expect(instance.allowed_values) @@ -253,7 +253,7 @@ describe Queries::Projects::Filters::CustomFieldFilter do end context 'list' do - let(:cf_accessor) { "cf_#{list_project_custom_field.id}" } + let(:cf_accessor) { list_project_custom_field.column_name } it 'is list_optional for a list' do expect(instance.allowed_values) @@ -262,7 +262,7 @@ describe Queries::Projects::Filters::CustomFieldFilter do end context 'user' do - let(:cf_accessor) { "cf_#{user_project_custom_field.id}" } + let(:cf_accessor) { user_project_custom_field.column_name } it 'is list_optional for a user' do bogus_return_value = ['user1', 'user2'] @@ -276,7 +276,7 @@ describe Queries::Projects::Filters::CustomFieldFilter do end context 'version' do - let(:cf_accessor) { "cf_#{version_project_custom_field.id}" } + let(:cf_accessor) { version_project_custom_field.column_name } it 'is list_optional for a version' do bogus_return_value = ['version1', 'version2'] @@ -290,7 +290,7 @@ describe Queries::Projects::Filters::CustomFieldFilter do end context 'date' do - let(:cf_accessor) { "cf_#{date_project_custom_field.id}" } + let(:cf_accessor) { date_project_custom_field.column_name } it 'is nil for a date' do expect(instance.allowed_values) @@ -299,7 +299,7 @@ describe Queries::Projects::Filters::CustomFieldFilter do end context 'bool' do - let(:cf_accessor) { "cf_#{bool_project_custom_field.id}" } + let(:cf_accessor) { bool_project_custom_field.column_name } it 'is list for a bool' do expect(instance.allowed_values) @@ -309,7 +309,7 @@ describe Queries::Projects::Filters::CustomFieldFilter do end context 'string' do - let(:cf_accessor) { "cf_#{string_project_custom_field.id}" } + let(:cf_accessor) { string_project_custom_field.column_name } it 'is nil for a string' do expect(instance.allowed_values) @@ -341,12 +341,12 @@ describe Queries::Projects::Filters::CustomFieldFilter do text_project_custom_field, date_project_custom_field, string_project_custom_field].each do |cf| - expect(filters.detect { |filter| filter.name == :"cf_#{cf.id}" }).not_to be_nil + expect(filters.detect { |filter| filter.name == cf.column_name.to_sym }).not_to be_nil end - expect(filters.detect { |filter| filter.name == :"cf_#{version_project_custom_field.id}" }) + expect(filters.detect { |filter| filter.name == version_project_custom_field.column_name.to_sym }) .to be_nil - expect(filters.detect { |filter| filter.name == :"cf_#{user_project_custom_field.id}" }) + expect(filters.detect { |filter| filter.name == user_project_custom_field.column_name.to_sym }) .to be_nil end end diff --git a/spec/models/queries/work_packages/columns/custom_field_column_spec.rb b/spec/models/queries/work_packages/columns/custom_field_column_spec.rb index 23d3f6e2a8..4f330b514b 100644 --- a/spec/models/queries/work_packages/columns/custom_field_column_spec.rb +++ b/spec/models/queries/work_packages/columns/custom_field_column_spec.rb @@ -31,15 +31,10 @@ require_relative 'shared_query_column_specs' describe Queries::WorkPackages::Columns::CustomFieldColumn do let(:project) { build_stubbed(:project) } - let(:custom_field) do - double('CustomField', - field_format: 'string', - id: 5, - order_statements: nil) - end + let(:custom_field) { build_stubbed(:string_wp_custom_field) } let(:instance) { described_class.new(custom_field) } - it_behaves_like 'query column' + it_behaves_like 'query column', sortable_by_default: true describe 'instances' do let(:text_custom_field) do @@ -86,7 +81,7 @@ describe Queries::WorkPackages::Columns::CustomFieldColumn do end describe '#value' do - let(:mock) { double(WorkPackage) } + let(:mock) { instance_double(WorkPackage) } it 'delegates to formatted_custom_value_for' do expect(mock).to receive(:formatted_custom_value_for).with(custom_field.id) diff --git a/spec/models/queries/work_packages/columns/shared_query_column_specs.rb b/spec/models/queries/work_packages/columns/shared_query_column_specs.rb index 4a2a7f5a98..e36238c052 100644 --- a/spec/models/queries/work_packages/columns/shared_query_column_specs.rb +++ b/spec/models/queries/work_packages/columns/shared_query_column_specs.rb @@ -26,7 +26,7 @@ # See COPYRIGHT and LICENSE files for more details. #++ -shared_examples_for 'query column' do +shared_examples_for 'query column' do |sortable_by_default: false| describe '#groupable' do it 'is the name if true is provided' do instance.groupable = true @@ -34,7 +34,7 @@ shared_examples_for 'query column' do expect(instance.groupable).to eql(instance.name.to_s) end - it 'is the value if something trueish is provided' do + it 'is the value if something truthy is provided' do instance.groupable = 'lorem ipsum' expect(instance.groupable).to eql('lorem ipsum') @@ -60,7 +60,7 @@ shared_examples_for 'query column' do expect(instance.sortable).to eql(instance.name.to_s) end - it 'is the value if something trueish is provided' do + it 'is the value if something truthy is provided' do instance.sortable = 'lorem ipsum' expect(instance.sortable).to eql('lorem ipsum') @@ -81,37 +81,41 @@ shared_examples_for 'query column' do describe '#groupable?' do it 'is false by default' do - expect(instance.groupable?).to be_falsey + expect(instance).not_to be_groupable end it 'is true if told so' do instance.groupable = true - expect(instance.groupable?).to be_truthy + expect(instance).to be_groupable end it 'is true if a value is provided (e.g. for specifying sql code)' do instance.groupable = 'COALESCE(null, 1)' - expect(instance.groupable?).to be_truthy + expect(instance).to be_groupable end end describe '#sortable?' do - it 'is false by default' do - expect(instance.sortable?).to be_falsey + it "is #{sortable_by_default} by default" do + if sortable_by_default + expect(instance).to be_sortable + else + expect(instance).not_to be_sortable + end end it 'is true if told so' do instance.sortable = true - expect(instance.sortable?).to be_truthy + expect(instance).to be_sortable end it 'is true if a value is provided (e.g. for specifying sql code)' do instance.sortable = 'COALESCE(null, 1)' - expect(instance.sortable?).to be_truthy + expect(instance).to be_sortable end end end diff --git a/spec/models/queries/work_packages/filter/custom_fields/contains_text_custom_field_filter_spec.rb b/spec/models/queries/work_packages/filter/custom_fields/contains_text_custom_field_filter_spec.rb index be88d55929..e40e1b1de2 100644 --- a/spec/models/queries/work_packages/filter/custom_fields/contains_text_custom_field_filter_spec.rb +++ b/spec/models/queries/work_packages/filter/custom_fields/contains_text_custom_field_filter_spec.rb @@ -30,7 +30,7 @@ require 'spec_helper' describe Queries::WorkPackages::Filter::CustomFieldFilter, 'with contains filter (Regression test #28348)' do - let(:cf_accessor) { "cf_#{custom_field.id}" } + let(:cf_accessor) { custom_field.column_name } let(:query) { build_stubbed(:query, project:) } let(:instance) do described_class.create!(name: cf_accessor, operator:, values: %w(foo), context: query) diff --git a/spec/models/queries/work_packages/filter/custom_fields/custom_field_filter_spec.rb b/spec/models/queries/work_packages/filter/custom_fields/custom_field_filter_spec.rb index da8ebd78f7..e7b844f267 100644 --- a/spec/models/queries/work_packages/filter/custom_fields/custom_field_filter_spec.rb +++ b/spec/models/queries/work_packages/filter/custom_fields/custom_field_filter_spec.rb @@ -51,7 +51,7 @@ describe Queries::WorkPackages::Filter::CustomFieldFilter do string_wp_custom_field] end let(:query) { build_stubbed(:query, project:) } - let(:cf_accessor) { "cf_#{custom_field.id}" } + let(:cf_accessor) { custom_field.column_name } let(:instance) do described_class.create!(name: cf_accessor, operator: '=', context: query) end @@ -127,16 +127,15 @@ describe Queries::WorkPackages::Filter::CustomFieldFilter do describe 'instance attributes' do it 'are valid' do all_custom_fields.each do |cf| - name = "cf_#{cf.id}" - filter = described_class.create!(name:) - expect(filter.name).to eql(:"cf_#{cf.id}") + filter = described_class.create!(name: cf.column_name) + expect(filter.name).to eql(cf.column_name.to_sym) end end end describe '#type' do context 'integer' do - let(:cf_accessor) { "cf_#{int_wp_custom_field.id}" } + let(:cf_accessor) { int_wp_custom_field.column_name } it 'is integer for an integer' do expect(instance.type) @@ -145,7 +144,7 @@ describe Queries::WorkPackages::Filter::CustomFieldFilter do end context 'float' do - let(:cf_accessor) { "cf_#{float_wp_custom_field.id}" } + let(:cf_accessor) { float_wp_custom_field.column_name } it 'is integer for a float' do expect(instance.type) @@ -154,7 +153,7 @@ describe Queries::WorkPackages::Filter::CustomFieldFilter do end context 'text' do - let(:cf_accessor) { "cf_#{text_wp_custom_field.id}" } + let(:cf_accessor) { text_wp_custom_field.column_name } it 'is text for a text' do expect(instance.type) @@ -163,7 +162,7 @@ describe Queries::WorkPackages::Filter::CustomFieldFilter do end context 'list optional' do - let(:cf_accessor) { "cf_#{list_wp_custom_field.id}" } + let(:cf_accessor) { list_wp_custom_field.column_name } it 'is list_optional for a list' do expect(instance.type) @@ -172,7 +171,7 @@ describe Queries::WorkPackages::Filter::CustomFieldFilter do end context 'user' do - let(:cf_accessor) { "cf_#{user_wp_custom_field.id}" } + let(:cf_accessor) { user_wp_custom_field.column_name } it 'is list_optional for a user' do expect(instance.type) @@ -181,7 +180,7 @@ describe Queries::WorkPackages::Filter::CustomFieldFilter do end context 'version' do - let(:cf_accessor) { "cf_#{version_wp_custom_field.id}" } + let(:cf_accessor) { version_wp_custom_field.column_name } it 'is list_optional for a version' do expect(instance.type) @@ -190,7 +189,7 @@ describe Queries::WorkPackages::Filter::CustomFieldFilter do end context 'version' do - let(:cf_accessor) { "cf_#{date_wp_custom_field.id}" } + let(:cf_accessor) { date_wp_custom_field.column_name } it 'is date for a date' do expect(instance.type) @@ -199,7 +198,7 @@ describe Queries::WorkPackages::Filter::CustomFieldFilter do end context 'bool' do - let(:cf_accessor) { "cf_#{bool_wp_custom_field.id}" } + let(:cf_accessor) { bool_wp_custom_field.column_name } it 'is list for a bool' do expect(instance.type) @@ -208,7 +207,7 @@ describe Queries::WorkPackages::Filter::CustomFieldFilter do end context 'string' do - let(:cf_accessor) { "cf_#{string_wp_custom_field.id}" } + let(:cf_accessor) { string_wp_custom_field.column_name } it 'is string for a string' do expect(instance.type) @@ -226,7 +225,7 @@ describe Queries::WorkPackages::Filter::CustomFieldFilter do describe '#allowed_values' do context 'integer' do - let(:cf_accessor) { "cf_#{int_wp_custom_field.id}" } + let(:cf_accessor) { int_wp_custom_field.column_name } it 'is nil for an integer' do expect(instance.allowed_values) @@ -235,7 +234,7 @@ describe Queries::WorkPackages::Filter::CustomFieldFilter do end context 'float' do - let(:cf_accessor) { "cf_#{float_wp_custom_field.id}" } + let(:cf_accessor) { float_wp_custom_field.column_name } it 'is integer for a float' do expect(instance.allowed_values) @@ -244,7 +243,7 @@ describe Queries::WorkPackages::Filter::CustomFieldFilter do end context 'text' do - let(:cf_accessor) { "cf_#{text_wp_custom_field.id}" } + let(:cf_accessor) { text_wp_custom_field.column_name } it 'is text for a text' do expect(instance.allowed_values) @@ -253,7 +252,7 @@ describe Queries::WorkPackages::Filter::CustomFieldFilter do end context 'list' do - let(:cf_accessor) { "cf_#{list_wp_custom_field.id}" } + let(:cf_accessor) { list_wp_custom_field.column_name } it 'is list_optional for a list' do expect(instance.allowed_values) @@ -262,7 +261,7 @@ describe Queries::WorkPackages::Filter::CustomFieldFilter do end context 'user' do - let(:cf_accessor) { "cf_#{user_wp_custom_field.id}" } + let(:cf_accessor) { user_wp_custom_field.column_name } it 'is list_optional for a user' do bogus_return_value = ['user1', 'user2'] @@ -278,7 +277,7 @@ describe Queries::WorkPackages::Filter::CustomFieldFilter do end context 'version' do - let(:cf_accessor) { "cf_#{version_wp_custom_field.id}" } + let(:cf_accessor) { version_wp_custom_field.column_name } it 'is list_optional for a version' do bogus_return_value = ['version1', 'version2'] @@ -294,7 +293,7 @@ describe Queries::WorkPackages::Filter::CustomFieldFilter do end context 'date' do - let(:cf_accessor) { "cf_#{date_wp_custom_field.id}" } + let(:cf_accessor) { date_wp_custom_field.column_name } it 'is nil for a date' do expect(instance.allowed_values) @@ -303,7 +302,7 @@ describe Queries::WorkPackages::Filter::CustomFieldFilter do end context 'bool' do - let(:cf_accessor) { "cf_#{bool_wp_custom_field.id}" } + let(:cf_accessor) { bool_wp_custom_field.column_name } it 'is list for a bool' do expect(instance.allowed_values) @@ -313,7 +312,7 @@ describe Queries::WorkPackages::Filter::CustomFieldFilter do end context 'string' do - let(:cf_accessor) { "cf_#{string_wp_custom_field.id}" } + let(:cf_accessor) { string_wp_custom_field.column_name } it 'is nil for a string' do expect(instance.allowed_values) @@ -347,7 +346,7 @@ describe Queries::WorkPackages::Filter::CustomFieldFilter do filters = described_class.all_for(query) all_custom_fields.each do |cf| - expect(filters.detect { |filter| filter.name == :"cf_#{cf.id}" }).not_to be_nil + expect(filters.detect { |filter| filter.name == cf.column_name.to_sym }).not_to be_nil end end @@ -355,7 +354,7 @@ describe Queries::WorkPackages::Filter::CustomFieldFilter do filters = described_class.all_for(query) all_custom_fields.each do |cf| - expect(filters.detect { |filter| filter.name == :"cf_#{cf.id}" }.context.project) + expect(filters.detect { |filter| filter.name == cf.column_name.to_sym }.context.project) .to eq project end end @@ -386,12 +385,12 @@ describe Queries::WorkPackages::Filter::CustomFieldFilter do text_wp_custom_field, date_wp_custom_field, string_wp_custom_field].each do |cf| - expect(filters.detect { |filter| filter.name == :"cf_#{cf.id}" }).not_to be_nil + expect(filters.detect { |filter| filter.name == cf.column_name.to_sym }).not_to be_nil end - expect(filters.detect { |filter| filter.name == :"cf_#{version_wp_custom_field.id}" }) + expect(filters.detect { |filter| filter.name == version_wp_custom_field.column_name.to_sym }) .to be_nil - expect(filters.detect { |filter| filter.name == :"cf_#{user_wp_custom_field.id}" }) + expect(filters.detect { |filter| filter.name == user_wp_custom_field.column_name.to_sym }) .to be_nil end end diff --git a/spec/models/query/results_cf_sorting_integration_spec.rb b/spec/models/query/results_cf_sorting_integration_spec.rb index a74e22a8ea..fd7d3487be 100644 --- a/spec/models/query/results_cf_sorting_integration_spec.rb +++ b/spec/models/query/results_cf_sorting_integration_spec.rb @@ -80,7 +80,7 @@ describe Query::Results, 'Sorting of custom field floats', with_mail: false do end describe 'sorting ASC by float cf' do - let(:sort_criteria) { [["cf_#{custom_field.id}", 'asc']] } + let(:sort_criteria) { [[custom_field.column_name, 'asc']] } it 'returns the correctly sorted result' do expect(query_results.work_packages.pluck(:id)) @@ -89,7 +89,7 @@ describe Query::Results, 'Sorting of custom field floats', with_mail: false do end describe 'sorting DESC by float cf' do - let(:sort_criteria) { [["cf_#{custom_field.id}", 'desc']] } + let(:sort_criteria) { [[custom_field.column_name, 'desc']] } it 'returns the correctly sorted result' do expect(query_results.work_packages.pluck(:id)) diff --git a/spec/models/query/results_sort_intergration_spec.rb b/spec/models/query/results_sort_intergration_spec.rb index 7b52c0d24e..d0f9691490 100644 --- a/spec/models/query/results_sort_intergration_spec.rb +++ b/spec/models/query/results_sort_intergration_spec.rb @@ -424,7 +424,7 @@ describe Query::Results, 'sorting and grouping', with_mail: false do end context 'when ascending' do - let(:sort_by) { [["cf_#{string_cf.id}", 'asc']] } + let(:sort_by) { [[string_cf.column_name, 'asc']] } it 'sorts case insensitive' do expect(query_results.work_packages) @@ -483,7 +483,7 @@ describe Query::Results, 'sorting and grouping', with_mail: false do end context 'when ascending' do - let(:sort_by) { [["cf_#{int_cf.id}", 'asc']] } + let(:sort_by) { [[int_cf.column_name, 'asc']] } it 'sorts case insensitive' do expect(query_results.work_packages) @@ -492,7 +492,7 @@ describe Query::Results, 'sorting and grouping', with_mail: false do end context 'when descending' do - let(:sort_by) { [["cf_#{int_cf.id}", 'desc']] } + let(:sort_by) { [[int_cf.column_name, 'desc']] } it 'sorts case insensitive' do expect(query_results.work_packages) diff --git a/spec/models/query/results_spec.rb b/spec/models/query/results_spec.rb index 1a0b49ec66..610c887efc 100644 --- a/spec/models/query/results_spec.rb +++ b/spec/models/query/results_spec.rb @@ -191,7 +191,7 @@ describe Query::Results, with_mail: false do let(:last_value) do custom_field.custom_options.last end - let(:group_by) { "cf_#{custom_field.id}" } + let(:group_by) { custom_field.column_name } before do login_as(user1) @@ -221,7 +221,7 @@ describe Query::Results, with_mail: false do create(:int_wp_custom_field, is_for_all: true, is_filter: true) end - let(:group_by) { "cf_#{custom_field.id}" } + let(:group_by) { custom_field.column_name } before do login_as(user1) @@ -245,7 +245,7 @@ describe Query::Results, with_mail: false do create(:user_wp_custom_field, is_for_all: true, is_filter: true) end - let(:group_by) { "cf_#{custom_field.id}" } + let(:group_by) { custom_field.column_name } before do login_as(user1) @@ -264,7 +264,7 @@ describe Query::Results, with_mail: false do create(:bool_wp_custom_field, is_for_all: true, is_filter: true) end - let(:group_by) { "cf_#{custom_field.id}" } + let(:group_by) { custom_field.column_name } before do login_as(user1) @@ -288,7 +288,7 @@ describe Query::Results, with_mail: false do create(:date_wp_custom_field, is_for_all: true, is_filter: true) end - let(:group_by) { "cf_#{custom_field.id}" } + let(:group_by) { custom_field.column_name } before do login_as(user1) @@ -388,7 +388,7 @@ describe Query::Results, with_mail: false do context 'when grouping by assignees' do before do - query.column_names = [:assigned_to, :"cf_#{custom_field.id}"] + query.column_names = [:assigned_to, custom_field.column_name.to_sym] query.group_by = 'assigned_to' end @@ -407,7 +407,7 @@ describe Query::Results, with_mail: false do let(:group_by) { 'responsible' } before do - query.column_names = [:responsible, :"cf_#{custom_field.id}"] + query.column_names = [:responsible, custom_field.column_name.to_sym] end it 'returns all work packages of project 2' do @@ -513,7 +513,7 @@ describe Query::Results, with_mail: false do activate_cf - query.add_filter(:"cf_#{bool_cf.id}", '=', [filter_value]) + query.add_filter(bool_cf.column_name.to_sym, '=', [filter_value]) end shared_examples_for 'is empty' do diff --git a/spec/models/query/results_sums_integration_spec.rb b/spec/models/query/results_sums_integration_spec.rb index d28ecbbde1..0b80a140e0 100644 --- a/spec/models/query/results_sums_integration_spec.rb +++ b/spec/models/query/results_sums_integration_spec.rb @@ -36,8 +36,8 @@ describe Query::Results, 'sums' do end end let(:estimated_hours_column) { query.displayable_columns.detect { |c| c.name.to_s == 'estimated_hours' } } - let(:int_cf_column) { query.displayable_columns.detect { |c| c.name.to_s == "cf_#{int_cf.id}" } } - let(:float_cf_column) { query.displayable_columns.detect { |c| c.name.to_s == "cf_#{float_cf.id}" } } + let(:int_cf_column) { query.displayable_columns.detect { |c| c.name.to_s == int_cf.column_name } } + let(:float_cf_column) { query.displayable_columns.detect { |c| c.name.to_s == float_cf.column_name } } let(:material_costs_column) { query.displayable_columns.detect { |c| c.name.to_s == "material_costs" } } let(:labor_costs_column) { query.displayable_columns.detect { |c| c.name.to_s == "labor_costs" } } let(:overall_costs_column) { query.displayable_columns.detect { |c| c.name.to_s == "overall_costs" } } diff --git a/spec/models/query_spec.rb b/spec/models/query_spec.rb index 1cd9890344..8e7322f1ee 100644 --- a/spec/models/query_spec.rb +++ b/spec/models/query_spec.rb @@ -403,7 +403,7 @@ describe Query do parent done_ratio priority responsible spent_hours start_date status subject type updated_at version) + - [:"cf_#{custom_field.id}"] + + [custom_field.column_name.to_sym] + [:"relations_to_type_#{type.id}"] + %i(relations_of_type_relation1 relations_of_type_relation2) @@ -420,7 +420,7 @@ describe Query do parent done_ratio priority responsible spent_hours start_date status subject type updated_at version) + - [:"cf_#{custom_field.id}"] + [custom_field.column_name.to_sym] unexpected_columns = [:"relations_to_type_#{type.id}"] + %i(relations_of_type_relation1 relations_of_type_relation2) @@ -465,7 +465,7 @@ describe Query do end before do - query.add_filter('cf_' + custom_field.id.to_s, '=', ['']) + query.add_filter(custom_field.column_name, '=', ['']) end it 'has the name of the custom field in the error message' do diff --git a/spec/services/custom_fields/create_service_spec.rb b/spec/services/custom_fields/create_service_spec.rb index eeebbd5f36..0c83ec2640 100644 --- a/spec/services/custom_fields/create_service_spec.rb +++ b/spec/services/custom_fields/create_service_spec.rb @@ -32,11 +32,11 @@ require 'services/base_services/behaves_like_create_service' describe CustomFields::CreateService, type: :model do it_behaves_like 'BaseServices create service' do context 'when creating a project cf' do - let(:model_instance) { build_stubbed :project_custom_field } + let(:model_instance) { build_stubbed(:project_custom_field) } it 'modifies the settings on successful call' do subject - expect(Setting.enabled_projects_columns).to include "cf_#{model_instance.id}" + expect(Setting.enabled_projects_columns).to include(model_instance.column_name) end end end