From 210d024f9678ad54ed68fa694675f08c31273a7c Mon Sep 17 00:00:00 2001 From: Ivan Kuchin Date: Wed, 16 Oct 2024 14:43:31 +0200 Subject: [PATCH 1/6] don't order absent versions, start date and due date to the end in both directions --- .../queries/work_packages/selects/property_select.rb | 7 ++----- spec/models/query/results_version_integration_spec.rb | 2 +- spec/models/query/sort_criteria_spec.rb | 10 +++++----- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/app/models/queries/work_packages/selects/property_select.rb b/app/models/queries/work_packages/selects/property_select.rb index 7963dfb81348..08426165910e 100644 --- a/app/models/queries/work_packages/selects/property_select.rb +++ b/app/models/queries/work_packages/selects/property_select.rb @@ -97,17 +97,14 @@ def caption association: "version", sortable: "name", default_order: "ASC", - null_handling: "NULLS LAST", groupable: "#{WorkPackage.table_name}.version_id" }, start_date: { - sortable: "#{WorkPackage.table_name}.start_date", - null_handling: "NULLS LAST" + sortable: "#{WorkPackage.table_name}.start_date" }, due_date: { highlightable: true, - sortable: "#{WorkPackage.table_name}.due_date", - null_handling: "NULLS LAST" + sortable: "#{WorkPackage.table_name}.due_date" }, estimated_hours: { sortable: "#{WorkPackage.table_name}.estimated_hours", diff --git a/spec/models/query/results_version_integration_spec.rb b/spec/models/query/results_version_integration_spec.rb index e06e3f5f4b3a..70a434630692 100644 --- a/spec/models/query/results_version_integration_spec.rb +++ b/spec/models/query/results_version_integration_spec.rb @@ -102,7 +102,7 @@ end end 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] } + let(:work_packages_desc) { [no_version_wp, new_version_wp, no_date_version_wp, old_version_wp] } before do login_as(user) diff --git a/spec/models/query/sort_criteria_spec.rb b/spec/models/query/sort_criteria_spec.rb index 04bf3de16177..db61ccc019ad 100644 --- a/spec/models/query/sort_criteria_spec.rb +++ b/spec/models/query/sort_criteria_spec.rb @@ -87,7 +87,7 @@ it "adds the order handling (and the default order by id)" do expect(subject) - .to eq [["work_packages.start_date NULLS LAST"], ["work_packages.id DESC"]] + .to eq [["work_packages.start_date"], ["work_packages.id DESC"]] end end @@ -96,7 +96,7 @@ it "adds the order handling (and the default order by id)" do expect(subject) - .to eq [["work_packages.start_date NULLS LAST"], ["work_packages.id DESC"]] + .to eq [["work_packages.start_date"], ["work_packages.id DESC"]] end end @@ -105,7 +105,7 @@ it "adds the order handling (and the default order by id)" do expect(subject) - .to eq [["work_packages.start_date DESC NULLS LAST"], ["work_packages.id DESC"]] + .to eq [["work_packages.start_date DESC"], ["work_packages.id DESC"]] end end @@ -114,8 +114,8 @@ it "adds the order handling (and the default order by id)" do expect(subject) - .to eq [["name DESC NULLS LAST"], - ["work_packages.start_date NULLS LAST"], + .to eq [["name DESC"], + ["work_packages.start_date"], ["work_packages.id DESC"]] end end From a1d580b59ce004dda3eb6d0d0b94e902a7d01cb7 Mon Sep 17 00:00:00 2001 From: Ivan Kuchin Date: Wed, 16 Oct 2024 14:54:32 +0200 Subject: [PATCH 2/6] remove default_order: asc for work package properties --- app/models/queries/work_packages/selects/property_select.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/models/queries/work_packages/selects/property_select.rb b/app/models/queries/work_packages/selects/property_select.rb index 08426165910e..c73db63aaeff 100644 --- a/app/models/queries/work_packages/selects/property_select.rb +++ b/app/models/queries/work_packages/selects/property_select.rb @@ -53,7 +53,6 @@ def caption }, parent: { association: "ancestors_relations", - default_order: "asc", sortable: false }, status: { @@ -96,7 +95,6 @@ def caption version: { association: "version", sortable: "name", - default_order: "ASC", groupable: "#{WorkPackage.table_name}.version_id" }, start_date: { From 7c8e6d62d276e337a7db4a0f9c980914decb118d Mon Sep 17 00:00:00 2001 From: Ivan Kuchin Date: Mon, 28 Oct 2024 19:31:05 +0100 Subject: [PATCH 3/6] drop NameOrder as it was broken (order without argument), so could not be used --- app/models/queries/versions.rb | 1 - .../queries/versions/orders/name_order.rb | 47 ------------------- 2 files changed, 48 deletions(-) delete mode 100644 app/models/queries/versions/orders/name_order.rb diff --git a/app/models/queries/versions.rb b/app/models/queries/versions.rb index b6af7df75293..6fc03ca3e56d 100644 --- a/app/models/queries/versions.rb +++ b/app/models/queries/versions.rb @@ -30,7 +30,6 @@ module Queries::Versions ::Queries::Register.register(VersionQuery) do filter Filters::SharingFilter - order Orders::NameOrder order Orders::SemverNameOrder end end diff --git a/app/models/queries/versions/orders/name_order.rb b/app/models/queries/versions/orders/name_order.rb deleted file mode 100644 index 362b9340cfb9..000000000000 --- a/app/models/queries/versions/orders/name_order.rb +++ /dev/null @@ -1,47 +0,0 @@ -#-- copyright -# OpenProject is an open source project management software. -# Copyright (C) the OpenProject GmbH -# -# This program is free software; you can redistribute it and/or -# modify it under the terms of the GNU General Public License version 3. -# -# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: -# Copyright (C) 2006-2013 Jean-Philippe Lang -# Copyright (C) 2010-2013 the ChiliProject Team -# -# This program is free software; you can redistribute it and/or -# modify it under the terms of the GNU General Public License -# as published by the Free Software Foundation; either version 2 -# of the License, or (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program; if not, write to the Free Software -# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. -# -# See COPYRIGHT and LICENSE files for more details. -#++ - -class Queries::Versions::Orders::NameOrder < Queries::Orders::Base - self.model = Version - - def self.key - :name - end - - private - - def order - ordered = Version.order(:name) - - if direction == :desc - ordered = ordered.reverse_order - end - - ordered - end -end From eb1699e574dc19c616ca370364c3b0483718d234 Mon Sep 17 00:00:00 2001 From: Ivan Kuchin Date: Tue, 29 Oct 2024 18:23:33 +0100 Subject: [PATCH 4/6] allow ordering versions by id or name, deprecate semver_name --- app/models/queries/versions.rb | 2 +- ...{semver_name_order.rb => default_order.rb} | 18 +-- .../queries/versions/version_query_spec.rb | 150 ++++++++++++++++++ 3 files changed, 159 insertions(+), 11 deletions(-) rename app/models/queries/versions/orders/{semver_name_order.rb => default_order.rb} (80%) create mode 100644 spec/models/queries/versions/version_query_spec.rb diff --git a/app/models/queries/versions.rb b/app/models/queries/versions.rb index 6fc03ca3e56d..6e036b0bae4a 100644 --- a/app/models/queries/versions.rb +++ b/app/models/queries/versions.rb @@ -30,6 +30,6 @@ module Queries::Versions ::Queries::Register.register(VersionQuery) do filter Filters::SharingFilter - order Orders::SemverNameOrder + order Orders::DefaultOrder end end diff --git a/app/models/queries/versions/orders/semver_name_order.rb b/app/models/queries/versions/orders/default_order.rb similarity index 80% rename from app/models/queries/versions/orders/semver_name_order.rb rename to app/models/queries/versions/orders/default_order.rb index 465429fb6bb0..b86f38ef6d52 100644 --- a/app/models/queries/versions/orders/semver_name_order.rb +++ b/app/models/queries/versions/orders/default_order.rb @@ -26,22 +26,20 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class Queries::Versions::Orders::SemverNameOrder < Queries::Orders::Base +class Queries::Versions::Orders::DefaultOrder < Queries::Orders::Base self.model = Version def self.key - :semver_name + /\A(id|name|semver_name)\z/ end - private + def initialize(attribute) + if attribute == :semver_name + OpenProject::Deprecation.warn("Sorting by semver_name is deprecated, name should be used instead") - def order(scope) - ordered = scope.order_by_semver_name - - if direction == :desc - ordered = ordered.reverse_order + super(:name) + else + super end - - ordered end end diff --git a/spec/models/queries/versions/version_query_spec.rb b/spec/models/queries/versions/version_query_spec.rb new file mode 100644 index 000000000000..02b5751d29af --- /dev/null +++ b/spec/models/queries/versions/version_query_spec.rb @@ -0,0 +1,150 @@ +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +require "spec_helper" + +RSpec.describe Queries::Versions::VersionQuery do + def instance = subject + + describe "ordering" do + before do + allow(OpenProject::Deprecation).to receive(:warn) + + instance.order(attribute => direction) if attribute + end + + describe "without it" do + let(:attribute) { nil } + + it "orders by id descending" do + expected = Version.order(id: :desc) + + expect(instance.results.to_sql).to eql expected.to_sql + end + end + + describe "unknown order" do + let(:attribute) { :unknown } + let(:direction) { :asc } + + it "returns a query not returning anything" do + expected = Version.where(Arel::Nodes::Equality.new(1, 0)) + + expect(instance.results.to_sql).to eql expected.to_sql + end + + it { is_expected.not_to be_valid } + end + + describe "by id" do + let(:attribute) { :id } + + describe "ascending" do + let(:direction) { :asc } + + it "is the same as handwriting the query" do + expected = Version.order(id: :asc) + + expect(instance.results.to_sql).to eql expected.to_sql + end + end + + describe "descending" do + let(:direction) { :desc } + + it "is the same as handwriting the query" do + expected = Version.order(id: :desc) + + expect(instance.results.to_sql).to eql expected.to_sql + end + end + end + + describe "by name" do + let(:attribute) { :name } + + describe "ascending" do + let(:direction) { :asc } + + it "is the same as handwriting the query" do + expected = Version.order(name: :asc).order(id: :desc) + + expect(instance.results.to_sql).to eql expected.to_sql + end + end + + describe "descending" do + let(:direction) { :desc } + + it "is the same as handwriting the query" do + expected = Version.order(name: :desc).order(id: :desc) + + expect(instance.results.to_sql).to eql expected.to_sql + end + end + end + + describe "by semver_name" do + let(:attribute) { :semver_name } + + describe "ascending" do + let(:direction) { :asc } + + it "is the same as handwriting the query" do + expected = Version.order(name: :asc).order(id: :desc) + + expect(instance.results.to_sql).to eql expected.to_sql + end + + it "warns about being deprecated" do + instance.results + + expect(OpenProject::Deprecation) + .to have_received(:warn).with("Sorting by semver_name is deprecated, name should be used instead") + end + end + + describe "descending" do + let(:direction) { :desc } + + it "is the same as handwriting the query" do + expected = Version.order(name: :desc).order(id: :desc) + + expect(instance.results.to_sql).to eql expected.to_sql + end + + it "warns about being deprecated" do + instance.results + + expect(OpenProject::Deprecation) + .to have_received(:warn).with("Sorting by semver_name is deprecated, name should be used instead") + end + end + end + end +end From f739e2f8786e93b898df62ac0b51dad4c72b8948 Mon Sep 17 00:00:00 2001 From: Ivan Kuchin Date: Tue, 29 Oct 2024 19:07:15 +0100 Subject: [PATCH 5/6] fix indent in versions api doc --- docs/api/apiv3/paths/versions.yml | 128 +++++++++++++++--------------- 1 file changed, 64 insertions(+), 64 deletions(-) diff --git a/docs/api/apiv3/paths/versions.yml b/docs/api/apiv3/paths/versions.yml index 07f1953b348d..8c509fb6ab5f 100644 --- a/docs/api/apiv3/paths/versions.yml +++ b/docs/api/apiv3/paths/versions.yml @@ -2,18 +2,18 @@ --- get: parameters: - - description: |- - JSON specifying filter conditions. - Accepts the same format as returned by the [queries](https://www.openproject.org/docs/api/endpoints/queries/) endpoint. - Currently supported filters are: + - description: |- + JSON specifying filter conditions. + Accepts the same format as returned by the [queries](https://www.openproject.org/docs/api/endpoints/queries/) endpoint. + Currently supported filters are: - + sharing: filters versions by how they are shared within the server (*none*, *descendants*, *hierarchy*, *tree*, *system*). - example: '[{ "sharing": { "operator": "*", "values": ["system"] }" }]' - in: query - name: filters - required: false - schema: - type: string + + sharing: filters versions by how they are shared within the server (*none*, *descendants*, *hierarchy*, *tree*, *system*). + example: '[{ "sharing": { "operator": "*", "values": ["system"] }" }]' + in: query + name: filters + required: false + schema: + type: string responses: '200': content: @@ -23,57 +23,57 @@ get: value: _embedded: elements: - - _links: - availableInProjects: - href: "/api/v3/versions/11/projects" - definingProject: - href: "/api/v3/projects/12" - self: - href: "/api/v3/versions/11" - _type: Version - description: - format: plain - html: This version has a description - raw: This version has a description - endDate: - id: 11 - name: v3.0 Alpha - startDate: '2014-11-20' - status: Open - - _links: - availableInProjects: - href: "/api/v3/versions/12/projects" - definingProject: - href: "/api/v3/projects/11" - self: - href: "/api/v3/versions/12" - _type: Version - description: - format: plain - html: '' - raw: '' - endDate: - id: 12 - name: v2.0 - startDate: - status: Closed - - _links: - availableInProjects: - href: "/api/v3/versions/13/projects" - definingProject: - href: "/api/v3/projects/13" - self: - href: "/api/v3/versions/10" - _type: Version - description: - format: plain - html: '' - raw: '' - endDate: - id: 10 - name: v1.0 - startDate: - status: Open + - _links: + availableInProjects: + href: "/api/v3/versions/11/projects" + definingProject: + href: "/api/v3/projects/12" + self: + href: "/api/v3/versions/11" + _type: Version + description: + format: plain + html: This version has a description + raw: This version has a description + endDate: + id: 11 + name: v3.0 Alpha + startDate: '2014-11-20' + status: Open + - _links: + availableInProjects: + href: "/api/v3/versions/12/projects" + definingProject: + href: "/api/v3/projects/11" + self: + href: "/api/v3/versions/12" + _type: Version + description: + format: plain + html: '' + raw: '' + endDate: + id: 12 + name: v2.0 + startDate: + status: Closed + - _links: + availableInProjects: + href: "/api/v3/versions/13/projects" + definingProject: + href: "/api/v3/projects/13" + self: + href: "/api/v3/versions/10" + _type: Version + description: + format: plain + html: '' + raw: '' + endDate: + id: 10 + name: v1.0 + startDate: + status: Open _links: self: href: "/api/v3/versions" @@ -85,7 +85,7 @@ get: description: OK headers: {} tags: - - Versions + - Versions description: Returns a collection of versions. The client can choose to filter the versions similar to how work packages are filtered. In addition to the provided filters, the server will reduce the result set to only contain versions, for which @@ -130,7 +130,7 @@ post: * a constraint for a property was violated (`PropertyConstraintViolation`) headers: {} tags: - - Versions + - Versions description: |- Creates a new version applying the attributes provided in the body. Please note that while there is a fixed set of attributes, custom fields can extend a version's attributes and are accepted by the endpoint. From 8b886405b346c5c9ac4eb979566bd9134a622332 Mon Sep 17 00:00:00 2001 From: Ivan Kuchin Date: Tue, 29 Oct 2024 19:10:47 +0100 Subject: [PATCH 6/6] describe versions sortBy parameter --- docs/api/apiv3/paths/versions.yml | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/docs/api/apiv3/paths/versions.yml b/docs/api/apiv3/paths/versions.yml index 8c509fb6ab5f..e69e05a9acc6 100644 --- a/docs/api/apiv3/paths/versions.yml +++ b/docs/api/apiv3/paths/versions.yml @@ -14,6 +14,19 @@ get: required: false schema: type: string + - description: |- + JSON specifying sort criteria. + Accepts the same format as returned by the [queries](https://www.openproject.org/docs/api/endpoints/queries/) endpoint. Currently supported attributes are: + + + id: Sort by the version id + + name: Sort by the version name using numeric collation, comparing sequences of decimal digits by their numeric value + + semver_name: Deprecated, use name instead + example: '[["name", "desc"]]' + in: query + name: sortBy + required: false + schema: + type: string responses: '200': content: