Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug/57550 custom field with format version are ordered as strings #16776

Open
wants to merge 8 commits into
base: dev
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
16 changes: 1 addition & 15 deletions app/models/versions/scopes/order_by_semver_name.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 13 additions & 0 deletions db/migrate/20240920152544_set_versions_name_collation.rb
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
16 changes: 8 additions & 8 deletions spec/models/query/results_cf_sorting_integration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
18 changes: 8 additions & 10 deletions spec/models/query/results_version_integration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,23 +42,23 @@

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")
end

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")
end

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)
Expand All @@ -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,
Expand All @@ -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)
Expand Down Expand Up @@ -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
6 changes: 1 addition & 5 deletions spec/models/query/sort_criteria_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
43 changes: 19 additions & 24 deletions spec/models/versions/scopes/order_by_semver_name_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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