From caa7e95f08eca4a35282da61ee58f042da686fc8 Mon Sep 17 00:00:00 2001 From: Ivan Kuchin Date: Fri, 20 Sep 2024 16:08:18 +0200 Subject: [PATCH 1/8] [#57550] Custom field with format version are ordered as strings https://community.openproject.org/work_packages/57550 From 97166ac522baffefd0171de111e6d0b0fff5c589 Mon Sep 17 00:00:00 2001 From: Ivan Kuchin Date: Mon, 23 Sep 2024 13:04:45 +0200 Subject: [PATCH 2/8] change expectations of Version.order_by_semver_name --- .../scopes/order_by_semver_name_spec.rb | 43 ++++++++----------- 1 file changed, 19 insertions(+), 24 deletions(-) diff --git a/spec/models/versions/scopes/order_by_semver_name_spec.rb b/spec/models/versions/scopes/order_by_semver_name_spec.rb index 2e330d0c86a4..f572a0df7f23 100644 --- a/spec/models/versions/scopes/order_by_semver_name_spec.rb +++ b/spec/models/versions/scopes/order_by_semver_name_spec.rb @@ -2,32 +2,27 @@ RSpec.describe Versions::Scopes::OrderBySemverName do let(:project) { create(:project) } - let!(:version1) do - create(:version, name: "aaaaa 1.", project:) - end - let!(:version2) do - create(:version, name: "aaaaa", project:) - end - let!(:version3) do - create(:version, name: "1.10. aaa", project:) - end - let!(:version4) do - create(:version, name: "1.1. zzz", project:, start_date: Date.today, effective_date: Date.today + 1.day) - end - let!(:version5) do - create(:version, name: "1.2. mmm", project:, start_date: Date.today) - end - let!(:version6) do - create(:version, name: "1. xxxx", project:, start_date: Date.today + 5.days) - end - let!(:version7) do - create(:version, name: "1.1. aaa", project:) - end + let(:names) do + [ + "1. xxxx", + "1.1. aaa", + "1.1. zzz", + "1.2. mmm", + "1.10. aaa", + "9", + "10.2", + "10.10.2", + "10.10.10", + "aaaaa", + "aaaaa 1." + ] + end + let!(:versions) { names.map { |name| create(:version, name:, project:) } } - subject { Version.order_by_semver_name } + subject { Version.order_by_semver_name.order(id: :desc).to_a } it "returns the versions in semver order" do - expect(subject.to_a) - .to eql [version6, version7, version4, version5, version3, version2, version1] + expect(subject) + .to eql versions end end From 1dc83446937dcc5ff2a6a149b39445bfe82e9c92 Mon Sep 17 00:00:00 2001 From: Ivan Kuchin Date: Mon, 23 Sep 2024 13:05:10 +0200 Subject: [PATCH 3/8] correct order of versions in version custom field ordering --- .../project_query_custom_field_order_spec.rb | 16 ++++++++-------- .../query/results_cf_sorting_integration_spec.rb | 16 ++++++++-------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/spec/models/queries/projects/project_query_custom_field_order_spec.rb b/spec/models/queries/projects/project_query_custom_field_order_spec.rb index fd13984ed995..5958575f31b1 100644 --- a/spec/models/queries/projects/project_query_custom_field_order_spec.rb +++ b/spec/models/queries/projects/project_query_custom_field_order_spec.rb @@ -316,10 +316,10 @@ def project_without_cf_value let(:projects) do [ - project_with_cf_value(id_by_name.fetch("10.10.10")), - project_with_cf_value(id_by_name.fetch("10.10.2")), - project_with_cf_value(id_by_name.fetch("10.2")), project_with_cf_value(id_by_name.fetch("9")), + project_with_cf_value(id_by_name.fetch("10.2")), + project_with_cf_value(id_by_name.fetch("10.10.2")), + project_with_cf_value(id_by_name.fetch("10.10.10")), project # TODO: should be at index 0 ] end @@ -332,12 +332,12 @@ def project_without_cf_value let(:projects) do [ - project_with_cf_value(*id_by_name.fetch_values("10.10.10")), # 10.10.10 - project_with_cf_value(*id_by_name.fetch_values("9", "10.10.10")), # 10.10.10, 9 - project_with_cf_value(*id_by_name.fetch_values("10.10.10", "9")), # 10.10.10, 9 + project_with_cf_value(*id_by_name.fetch_values("10.10.2", "9")), # 9, 10.10.2 + project_with_cf_value(*id_by_name.fetch_values("10.10.10", "9")), # 9, 10.10.10 + project_with_cf_value(*id_by_name.fetch_values("9", "10.10.10")), # 9, 10.10.10 + project_with_cf_value(*id_by_name.fetch_values("10.2", "10.10.2")), # 10.2, 10.10.2 project_with_cf_value(*id_by_name.fetch_values("10.10.2")), # 10.10.2 - project_with_cf_value(*id_by_name.fetch_values("10.2", "10.10.2")), # 10.10.2, 10.2 - project_with_cf_value(*id_by_name.fetch_values("10.10.2", "9")), # 10.10.2, 9 + project_with_cf_value(*id_by_name.fetch_values("10.10.10")), # 10.10.10 project # TODO: should be at index 0 ] end diff --git a/spec/models/query/results_cf_sorting_integration_spec.rb b/spec/models/query/results_cf_sorting_integration_spec.rb index 50cf643c9a41..74e2f9865b39 100644 --- a/spec/models/query/results_cf_sorting_integration_spec.rb +++ b/spec/models/query/results_cf_sorting_integration_spec.rb @@ -308,10 +308,10 @@ def wp_without_cf_value 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")), + wp_with_cf_value(id_by_name.fetch("10.2")), + wp_with_cf_value(id_by_name.fetch("10.10.2")), + wp_with_cf_value(id_by_name.fetch("10.10.10")), wp_without_cf_value # TODO: should be at index 0 ] end @@ -324,12 +324,12 @@ def wp_without_cf_value let(:work_packages) do [ - wp_with_cf_value(id_by_name.fetch_values("10.10.10")), # 10.10.10 - wp_with_cf_value(id_by_name.fetch_values("9", "10.10.10")), # 10.10.10, 9 - wp_with_cf_value(id_by_name.fetch_values("10.10.10", "9")), # 10.10.10, 9 + wp_with_cf_value(id_by_name.fetch_values("10.10.2", "9")), # 9, 10.10.2 + wp_with_cf_value(id_by_name.fetch_values("10.10.10", "9")), # 9, 10.10.10 + wp_with_cf_value(id_by_name.fetch_values("9", "10.10.10")), # 9, 10.10.10 + wp_with_cf_value(id_by_name.fetch_values("10.2", "10.10.2")), # 10.2, 10.10.2 wp_with_cf_value(id_by_name.fetch_values("10.10.2")), # 10.10.2 - wp_with_cf_value(id_by_name.fetch_values("10.2", "10.10.2")), # 10.10.2, 10.2 - wp_with_cf_value(id_by_name.fetch_values("10.10.2", "9")), # 10.10.2, 9 + wp_with_cf_value(id_by_name.fetch_values("10.10.10")), # 10.10.10 wp_without_cf_value # TODO: should be at index 0 ] end From 8fc8287943e63d63fe9400effe6c0aeb1c11712c Mon Sep 17 00:00:00 2001 From: Ivan Kuchin Date: Mon, 23 Sep 2024 13:06:47 +0200 Subject: [PATCH 4/8] migration to add numeric collation for column name of versions table --- .../20240920152544_set_versions_name_collation.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 db/migrate/20240920152544_set_versions_name_collation.rb diff --git a/db/migrate/20240920152544_set_versions_name_collation.rb b/db/migrate/20240920152544_set_versions_name_collation.rb new file mode 100644 index 000000000000..3a2da158eb9c --- /dev/null +++ b/db/migrate/20240920152544_set_versions_name_collation.rb @@ -0,0 +1,13 @@ +class SetVersionsNameCollation < ActiveRecord::Migration[7.1] + def up + execute <<-SQL.squish + CREATE COLLATION IF NOT EXISTS versions_name (provider = icu, locale = 'en-u-kn-true'); + SQL + + change_column :versions, :name, :string, collation: "versions_name" + end + + def down + change_column :versions, :name, :string + end +end From f0c3288c1e805a78d5c6262886b6900794ddfd66 Mon Sep 17 00:00:00 2001 From: Ivan Kuchin Date: Mon, 23 Sep 2024 13:07:29 +0200 Subject: [PATCH 5/8] simplify Version.order_by_semver_name Also use order instead of reorder --- .../versions/scopes/order_by_semver_name.rb | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/app/models/versions/scopes/order_by_semver_name.rb b/app/models/versions/scopes/order_by_semver_name.rb index 35545c9e8db1..b4affc08d6fa 100644 --- a/app/models/versions/scopes/order_by_semver_name.rb +++ b/app/models/versions/scopes/order_by_semver_name.rb @@ -32,21 +32,7 @@ module OrderBySemverName class_methods do def order_by_semver_name - reorder semver_sql, :name - end - - # Returns an sql for ordering which: - # * Returns a substring from the beginning of the name up until the first alphabetical character e.g. "1.2.3 " - # from "1.2.3 ABC" - # * Replaces all non numerical character groups in that substring by a blank, e.g "1.2.3 " to "1 2 3 " - # * Splits the result into an array of individual number parts, e.g. "{1, 2, 3, ''}" from "1 2 3 " - # * removes all empty array items, e.g. "{1, 2, 3}" from "{1, 2, 3, ''}" - def semver_sql(table_name = Version.table_name) - sql = <<~SQL - array_remove(regexp_split_to_array(regexp_replace(substring(#{table_name}.name from '^[^a-zA-Z]+'), '\\D+', ' ', 'g'), ' '), '')::int[] - SQL - - Arel.sql(sql) + order :name end end end From dd798a0c11bbfa364f5bc399089028eb39d54dfe Mon Sep 17 00:00:00 2001 From: Ivan Kuchin Date: Mon, 23 Sep 2024 14:03:39 +0200 Subject: [PATCH 6/8] fix and simplify sortable of version property of work package --- app/models/queries/work_packages/selects/property_select.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/queries/work_packages/selects/property_select.rb b/app/models/queries/work_packages/selects/property_select.rb index 48635b1c296e..7963dfb81348 100644 --- a/app/models/queries/work_packages/selects/property_select.rb +++ b/app/models/queries/work_packages/selects/property_select.rb @@ -95,7 +95,7 @@ def caption }, version: { association: "version", - sortable: [->(table_name = Version.table_name) { Version.semver_sql(table_name) }, "name"], + sortable: "name", default_order: "ASC", null_handling: "NULLS LAST", groupable: "#{WorkPackage.table_name}.version_id" From ebb3a4eb4bc45baaab6cac6d0e99a88739fd5db7 Mon Sep 17 00:00:00 2001 From: Ivan Kuchin Date: Mon, 23 Sep 2024 14:17:59 +0200 Subject: [PATCH 7/8] fix sort criteria spec --- spec/models/query/sort_criteria_spec.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/spec/models/query/sort_criteria_spec.rb b/spec/models/query/sort_criteria_spec.rb index c1444e2ca065..04bf3de16177 100644 --- a/spec/models/query/sort_criteria_spec.rb +++ b/spec/models/query/sort_criteria_spec.rb @@ -113,12 +113,8 @@ let(:sort_criteria) { [%w[version desc], %w[start_date asc]] } it "adds the order handling (and the default order by id)" do - sort_sql = <<~SQL - array_remove(regexp_split_to_array(regexp_replace(substring(versions.name from '^[^a-zA-Z]+'), '\\D+', ' ', 'g'), ' '), '')::int[] - SQL - expect(subject) - .to eq [["#{sort_sql} DESC NULLS LAST", "name DESC NULLS LAST"], + .to eq [["name DESC NULLS LAST"], ["work_packages.start_date NULLS LAST"], ["work_packages.id DESC"]] end From 3bd5e06f618659d56a35f31066395a365b4e30de Mon Sep 17 00:00:00 2001 From: Ivan Kuchin Date: Mon, 23 Sep 2024 14:18:52 +0200 Subject: [PATCH 8/8] improve a bit results version integration spec --- .../query/results_version_integration_spec.rb | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/spec/models/query/results_version_integration_spec.rb b/spec/models/query/results_version_integration_spec.rb index efb8c8e90b48..e06e3f5f4b3a 100644 --- a/spec/models/query/results_version_integration_spec.rb +++ b/spec/models/query/results_version_integration_spec.rb @@ -42,7 +42,7 @@ let(:old_version) do create(:version, - name: "1. Old version", + name: "4. Old version", project:, start_date: "2019-02-02", effective_date: "2019-02-03") @@ -50,7 +50,7 @@ let(:new_version) do create(:version, - name: "1.2 New version", + name: "10.2 New version", project:, start_date: "2020-02-02", effective_date: "2020-02-03") @@ -58,7 +58,7 @@ let(:no_date_version) do create(:version, - name: "1.1 No date version", + name: "10.1 No date version", project:, start_date: nil, effective_date: nil) @@ -69,13 +69,13 @@ subject: "No version wp", project:) end - let!(:newest_version_wp) do + let!(:new_version_wp) do create(:work_package, subject: "Newest version wp", version: new_version, project:) end - let!(:oldest_version_wp) do + let!(:old_version_wp) do create(:work_package, subject: "Oldest version wp", version: old_version, @@ -101,7 +101,8 @@ q.sort_criteria = sort_criteria end end - let(:work_packages_asc) { [oldest_version_wp, no_date_version_wp, newest_version_wp, no_version_wp] } + let(:work_packages_asc) { [old_version_wp, no_date_version_wp, new_version_wp, no_version_wp] } + let(:work_packages_desc) { [new_version_wp, no_date_version_wp, old_version_wp, no_version_wp] } before do login_as(user) @@ -136,11 +137,8 @@ let(:sort_criteria) { [["version", "desc"]] } it "returns the correctly sorted result" do - # null values are still sorted last - work_packages_order = [newest_version_wp, no_date_version_wp, oldest_version_wp, no_version_wp] - expect(query_results.work_packages.pluck(:id)) - .to match work_packages_order.map(&:id) + .to match work_packages_desc.map(&:id) end end end