diff --git a/app/models/custom_field/order_statements.rb b/app/models/custom_field/order_statements.rb index c5463cf147c8..e13a19a9f306 100644 --- a/app/models/custom_field/order_statements.rb +++ b/app/models/custom_field/order_statements.rb @@ -38,7 +38,7 @@ def order_statements else [select_custom_option_position] end - when "string", "text", "date", "bool" + when "string", "text", "date", "bool", "link" if multi_value? [select_custom_values_as_group] else diff --git a/spec/factories/custom_field_factory.rb b/spec/factories/custom_field_factory.rb index 4ce3ad459488..4b3d266f1db6 100644 --- a/spec/factories/custom_field_factory.rb +++ b/spec/factories/custom_field_factory.rb @@ -217,8 +217,11 @@ factory :float_wp_custom_field, traits: [:float] factory :date_wp_custom_field, traits: [:date] factory :list_wp_custom_field, traits: [:list] + factory :multi_list_wp_custom_field, traits: [:multi_list] factory :version_wp_custom_field, traits: [:version] + factory :multi_version_wp_custom_field, traits: [:multi_version] factory :user_wp_custom_field, traits: [:user] + factory :multi_user_wp_custom_field, traits: [:multi_user] factory :link_wp_custom_field, traits: [:link] end diff --git a/spec/models/query/results_cf_sorting_integration_spec.rb b/spec/models/query/results_cf_sorting_integration_spec.rb index 28f7e01532db..7957b1b85a9e 100644 --- a/spec/models/query/results_cf_sorting_integration_spec.rb +++ b/spec/models/query/results_cf_sorting_integration_spec.rb @@ -28,15 +28,11 @@ require "spec_helper" -RSpec.describe Query::Results, "Sorting of custom field floats" do +RSpec.describe Query::Results, "Sorting by custom field" do + shared_let(:user) { create(:admin) } + let(:query_results) do - Query::Results.new query - end - let(:user) do - create(:user, - firstname: "user", - lastname: "1", - member_with_permissions: { project => [:view_work_packages] }) + described_class.new query end let(:type) { create(:type_standard, custom_fields: [custom_field]) } @@ -45,22 +41,6 @@ types: [type], work_package_custom_fields: [custom_field]) end - let(:work_package_with_float) do - create(:work_package, - type:, - project:, - custom_values: { custom_field.id => "6.25" }) - end - - let(:work_package_without_float) do - create(:work_package, - type:, - project:) - end - - let(:custom_field) do - create(:float_wp_custom_field, name: "MyFloat") - end let(:query) do build(:query, @@ -74,25 +54,263 @@ before do login_as(user) - work_package_with_float - work_package_without_float end - describe "sorting ASC by float cf" do - let(:sort_criteria) { [[custom_field.column_name, "asc"]] } + def wp_with_cf_value(value) + create(:work_package, type:, project:, custom_values: { custom_field.id => value }) + end + + shared_examples "it sorts" do + let(:work_package_desc) { work_packages.reverse } - it "returns the correctly sorted result" do - expect(query_results.work_packages.pluck(:id)) - .to match [work_package_without_float, work_package_with_float].map(&:id) + before { work_packages } + + context "in ascending order" do + let(:sort_criteria) { [[custom_field.column_name, "asc"], %w[id asc]] } + + it "returns the correctly sorted result" do + expect(query_results.work_packages.map(&:id)) + .to eq work_packages.map(&:id) + end + end + + context "in descending order" do + let(:sort_criteria) { [[custom_field.column_name, "desc"], %w[id asc]] } + + it "returns the correctly sorted result" do + expect(query_results.work_packages.map(&:id)) + .to eq work_package_desc.map(&:id) + end end end - describe "sorting DESC by float cf" do - let(:sort_criteria) { [[custom_field.column_name, "desc"]] } + context "for string format" do + include_examples "it sorts" do + let(:custom_field) { create(:string_wp_custom_field) } + + let(:work_packages) do + [ + create(:work_package, type:, project:), + wp_with_cf_value("16"), + wp_with_cf_value("6.25") + ] + end + end + end + + context "for link format" do + include_examples "it sorts" do + let(:custom_field) { create(:link_wp_custom_field) } + + let(:work_packages) do + [ + create(:work_package, type:, project:), + wp_with_cf_value("https://openproject.org/intro/"), + wp_with_cf_value("https://openproject.org/pricing/") + ] + end + end + end + + context "for int format" do + include_examples "it sorts" do + let(:custom_field) { create(:integer_wp_custom_field) } + + let(:work_packages) do + [ + create(:work_package, type:, project:), + wp_with_cf_value("6"), + wp_with_cf_value("16") + ] + end + end + end + + context "for float format" do + include_examples "it sorts" do + let(:custom_field) { create(:float_wp_custom_field) } + + let(:work_packages) do + [ + create(:work_package, type:, project:), + wp_with_cf_value("6.25"), + wp_with_cf_value("16") + ] + end + end + end + + context "for date format" do + include_examples "it sorts" do + let(:custom_field) { create(:date_wp_custom_field) } + + let(:work_packages) do + [ + create(:work_package, type:, project:), + wp_with_cf_value("2024-01-01"), + wp_with_cf_value("2030-01-01"), + wp_with_cf_value("999-01-01") # TODO: should be at index 1 + ] + end + end + end + + context "for bool format" do + include_examples "it sorts" do + let(:custom_field) { create(:boolean_wp_custom_field) } + + let(:work_packages) do + [ + create(:work_package, type:, project:), + wp_with_cf_value("0"), + wp_with_cf_value("1") + ] + end + end + end + + context "for list format" do + let(:possible_values) { %w[100 3 20] } + let(:id_by_value) { custom_field.possible_values.to_h { [_1.value, _1.id] } } + + context "if not allowing multi select" do + include_examples "it sorts" do + let(:custom_field) { create(:list_wp_custom_field, possible_values:) } + + let(:work_packages) do + [ + # sorting is done by position, and not by value + wp_with_cf_value(id_by_value.fetch("100")), + wp_with_cf_value(id_by_value.fetch("3")), + wp_with_cf_value(id_by_value.fetch("20")), + create(:work_package, type:, project:) # TODO: should be at index 0 + ] + end + end + end + + context "if allowing multi select" do + include_examples "it sorts" do + let(:custom_field) { create(:multi_list_wp_custom_field, possible_values:) } + + let(:work_packages) do + [ + create(:work_package, type:, project:), + # TODO: sorting is done by values sorted by position and joined by `.`, why? + wp_with_cf_value(id_by_value.fetch_values("100")), # => 100 + wp_with_cf_value(id_by_value.fetch_values("20", "100")), # => 100.20 + wp_with_cf_value(id_by_value.fetch_values("3", "100")), # => 100.3 + wp_with_cf_value(id_by_value.fetch_values("100", "3", "20")), # => 100.3.20 + wp_with_cf_value(id_by_value.fetch_values("20")), # => 20 + wp_with_cf_value(id_by_value.fetch_values("3")), # => 3 + wp_with_cf_value(id_by_value.fetch_values("3", "20")) # => 3.20 + ] + end + end + end + end + + context "for user format" do + shared_let(:users) do + [ + create(:user, lastname: "B", firstname: "B", login: "bb1"), + create(:user, lastname: "B", firstname: "B", login: "bb2"), + create(:user, lastname: "B", firstname: "A", login: "ba"), + create(:user, lastname: "A", firstname: "X", login: "ax") + ] + end + shared_let(:id_by_login) { users.to_h { [_1.login, _1.id] } } + + shared_let(:role) { create(:project_role) } + + before do + users.each do |user| + create(:member, project:, principal: user, roles: [role]) + end + end + + context "if not allowing multi select" do + include_examples "it sorts" do + let(:custom_field) { create(:user_wp_custom_field) } + + let(:work_packages) do + [ + wp_with_cf_value(id_by_login.fetch("ax")), + wp_with_cf_value(id_by_login.fetch("ba")), + wp_with_cf_value(id_by_login.fetch("bb1")), + wp_with_cf_value(id_by_login.fetch("bb2")), + create(:work_package, type:, project:) # TODO: should be at index 0 + ] + end + end + end + + context "if allowing multi select" do + include_examples "it sorts" do + let(:custom_field) { create(:multi_user_wp_custom_field) } + + let(:work_packages) do + [ + wp_with_cf_value(id_by_login.fetch_values("ax")), + wp_with_cf_value(id_by_login.fetch_values("ba")), + # TODO: second user is ignored + wp_with_cf_value(id_by_login.fetch_values("bb1", "ba")), + wp_with_cf_value(id_by_login.fetch_values("bb1", "ax")), + create(:work_package, type:, project:) # TODO: should be at index 0 + ] + end + + # TODO: second user is ignored, so order due to falling back on id asc + let(:work_package_desc) { work_packages.values_at(4, 2, 3, 1, 0) } + end + end + end + + context "for version format" do + let(:versions) do + [ + create(:version, project:, sharing: "system", name: "10.10.10"), + create(:version, project:, sharing: "system", name: "10.10.2"), + create(:version, project:, sharing: "system", name: "10.2"), + create(:version, project:, sharing: "system", name: "9") + ] + end + let(:id_by_name) { versions.to_h { [_1.name, _1.id] } } + + context "if not allowing multi select" do + include_examples "it sorts" do + let(:custom_field) { create(:version_wp_custom_field) } + + let(:work_packages) do + [ + wp_with_cf_value(id_by_name.fetch("10.10.10")), + wp_with_cf_value(id_by_name.fetch("10.10.2")), + wp_with_cf_value(id_by_name.fetch("10.2")), + wp_with_cf_value(id_by_name.fetch("9")), + create(:work_package, type:, project:) # TODO: should be at index 0 + ] + end + end + end + + context "if allowing multi select" do + include_examples "it sorts" do + let(:custom_field) { create(:multi_version_wp_custom_field) } + + let(:work_packages) do + [ + wp_with_cf_value(id_by_name.fetch_values("10.10.10")), + wp_with_cf_value(id_by_name.fetch_values("10.10.2")), + # TODO: second version is ignored + wp_with_cf_value(id_by_name.fetch_values("9", "10.10.2")), + wp_with_cf_value(id_by_name.fetch_values("9", "10.10.10")), + create(:work_package, type:, project:) # TODO: should be at index 0 + ] + end - it "returns the correctly sorted result" do - expect(query_results.work_packages.pluck(:id)) - .to match [work_package_with_float, work_package_without_float].map(&:id) + # TODO: second version is ignored, so order due to falling back on id asc + let(:work_package_desc) { work_packages.values_at(4, 2, 3, 1, 0) } + end end end end