From f7fbb5919c85f828692a6056ab0bfde2acdbe266 Mon Sep 17 00:00:00 2001 From: Ivan Kuchin Date: Fri, 27 Sep 2024 15:36:23 +0200 Subject: [PATCH] using instance variables instead of writers for work package selects --- .../selects/custom_field_select.rb | 9 +-- .../selects/relation_of_type_select.rb | 3 +- .../work_packages/selects/relation_select.rb | 2 +- .../selects/relation_to_type_select.rb | 4 +- .../selects/work_package_select.rb | 78 ++++++++----------- .../selects/shared_query_select_specs.rb | 36 +++++---- 6 files changed, 65 insertions(+), 67 deletions(-) diff --git a/app/models/queries/work_packages/selects/custom_field_select.rb b/app/models/queries/work_packages/selects/custom_field_select.rb index c6e2a107d5da..21a0f0c730bb 100644 --- a/app/models/queries/work_packages/selects/custom_field_select.rb +++ b/app/models/queries/work_packages/selects/custom_field_select.rb @@ -69,20 +69,19 @@ def self.instances(context = nil) private def set_name! - self.name = custom_field.column_name.to_sym + @name = custom_field.column_name.to_sym end def set_sortable! - self.sortable = custom_field.order_statements || false + @sortable = custom_field.order_statements || false end def set_groupable! - self.groupable = custom_field.group_by_statement if groupable_custom_field?(custom_field) - self.groupable ||= false + @groupable = groupable_custom_field?(custom_field) ? custom_field.group_by_statement || false : false end def set_summable! - self.summable = if %w(float int).include?(custom_field.field_format) + @summable = if %w(float int).include?(custom_field.field_format) select = summable_select_statement ->(query, grouped) { diff --git a/app/models/queries/work_packages/selects/relation_of_type_select.rb b/app/models/queries/work_packages/selects/relation_of_type_select.rb index eb6809cec69c..46cd6780a501 100644 --- a/app/models/queries/work_packages/selects/relation_of_type_select.rb +++ b/app/models/queries/work_packages/selects/relation_of_type_select.rb @@ -28,7 +28,8 @@ class Queries::WorkPackages::Selects::RelationOfTypeSelect < Queries::WorkPackages::Selects::RelationSelect def initialize(type) - self.type = type + @type = type + super(name) end diff --git a/app/models/queries/work_packages/selects/relation_select.rb b/app/models/queries/work_packages/selects/relation_select.rb index c12f7663eb2e..224b1e58f743 100644 --- a/app/models/queries/work_packages/selects/relation_select.rb +++ b/app/models/queries/work_packages/selects/relation_select.rb @@ -27,7 +27,7 @@ #++ class Queries::WorkPackages::Selects::RelationSelect < Queries::WorkPackages::Selects::WorkPackageSelect - attr_accessor :type + attr_reader :type def self.granted_by_enterprise_token EnterpriseToken.allows_to?(:work_package_query_relation_columns) diff --git a/app/models/queries/work_packages/selects/relation_to_type_select.rb b/app/models/queries/work_packages/selects/relation_to_type_select.rb index cd6ddcd432fc..adf9b685d104 100644 --- a/app/models/queries/work_packages/selects/relation_to_type_select.rb +++ b/app/models/queries/work_packages/selects/relation_to_type_select.rb @@ -31,11 +31,11 @@ def initialize(type) super set_name! type - self.type = type + @type = type end def set_name!(type) - self.name = :"relations_to_type_#{type.id}" + @name = :"relations_to_type_#{type.id}" end def caption diff --git a/app/models/queries/work_packages/selects/work_package_select.rb b/app/models/queries/work_packages/selects/work_package_select.rb index a3ffe42aa3b6..af559df44bd1 100644 --- a/app/models/queries/work_packages/selects/work_package_select.rb +++ b/app/models/queries/work_packages/selects/work_package_select.rb @@ -27,19 +27,13 @@ #++ class Queries::WorkPackages::Selects::WorkPackageSelect - attr_accessor :highlightable, - :name, - :sortable_join, - :summable, - :default_order, - :association - alias_method :highlightable?, :highlightable - - attr_reader :groupable, - :sortable, - :displayable - - attr_writer :null_handling, + attr_reader :highlightable, + :name, + :sortable_join, + :summable, + :default_order, + :association, + :null_handling, :summable_select, :summable_work_packages_select @@ -76,31 +70,25 @@ def null_handling(_asc) @null_handling end - def groupable=(value) - @groupable = name_or_value_or_false(value) + def displayable + @displayable.nil? ? true : @displayable end - def sortable=(value) - @sortable = name_or_value_or_false(value) + def sortable + name_or_value_or_false(@sortable) end - def displayable=(value) - @displayable = value.nil? ? true : value + def groupable + name_or_value_or_false(@groupable) end - def displayable? - displayable - end + def displayable? = !!displayable - # Returns true if the column is sortable, otherwise false - def sortable? - !!sortable - end + def sortable? = !!sortable - # Returns true if the column is groupable, otherwise false - def groupable? - !!groupable - end + def groupable? = !!groupable + + def highlightable? = !!highlightable def summable? summable || @summable_select || @summable_work_packages_select @@ -127,22 +115,24 @@ def value(model) end def initialize(name, options = {}) - self.name = name - - %i(sortable - sortable_join - displayable - groupable - summable - summable_select - summable_work_packages_select - association - null_handling - default_order).each do |attribute| - send(:"#{attribute}=", options[attribute]) + @name = name + + %i[ + sortable + sortable_join + displayable + groupable + summable + summable_select + summable_work_packages_select + association + null_handling + default_order + ].each do |attribute| + instance_variable_set(:"@#{attribute}", options[attribute]) end - self.highlightable = !!options.fetch(:highlightable, false) + @highlightable = !!options.fetch(:highlightable, false) end def caption diff --git a/spec/models/queries/work_packages/selects/shared_query_select_specs.rb b/spec/models/queries/work_packages/selects/shared_query_select_specs.rb index 4fd9758d4c28..645352fd91c6 100644 --- a/spec/models/queries/work_packages/selects/shared_query_select_specs.rb +++ b/spec/models/queries/work_packages/selects/shared_query_select_specs.rb @@ -29,52 +29,60 @@ RSpec.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 + instance.instance_variable_set(:@groupable, true) expect(instance.groupable).to eql(instance.name.to_s) end it "is the value if something truthy is provided" do - instance.groupable = "lorem ipsum" + instance.instance_variable_set(:@groupable, "lorem ipsum") expect(instance.groupable).to eql("lorem ipsum") end it "is false if false is provided" do - instance.groupable = false + instance.instance_variable_set(:@groupable, false) expect(instance.groupable).to be_falsey end - it "is false if nothing is provided" do - instance.groupable = nil + it "is false if nil is provided" do + instance.instance_variable_set(:@groupable, nil) expect(instance.groupable).to be_falsey end + + it "is false if not set" do + expect(instance.groupable).to be_falsey + end end describe "#sortable" do it "is the name if true is provided" do - instance.sortable = true + instance.instance_variable_set(:@sortable, true) expect(instance.sortable).to eql(instance.name.to_s) end it "is the value if something truthy is provided" do - instance.sortable = "lorem ipsum" + instance.instance_variable_set(:@sortable, "lorem ipsum") expect(instance.sortable).to eql("lorem ipsum") end it "is false if false is provided" do - instance.sortable = false + instance.instance_variable_set(:@sortable, false) expect(instance.sortable).to be_falsey end - it "is false if nothing is provided" do - instance.sortable = nil + it "is false if nil is provided" do + instance.instance_variable_set(:@sortable, nil) + + expect(instance.sortable).to be_falsey + end + it "is false if not set" do expect(instance.sortable).to be_falsey end end @@ -85,13 +93,13 @@ end it "is true if told so" do - instance.groupable = true + instance.instance_variable_set(:@groupable, true) 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)" + instance.instance_variable_set(:@groupable, "COALESCE(null, 1)") expect(instance).to be_groupable end @@ -107,13 +115,13 @@ end it "is true if told so" do - instance.sortable = true + instance.instance_variable_set(:@sortable, true) 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)" + instance.instance_variable_set(:@sortable, "COALESCE(null, 1)") expect(instance).to be_sortable end