diff --git a/app/models/packages/package.rb b/app/models/packages/package.rb
index 0558e1b338d6086f273a85ef231c7fa06b04a3ab..698d7493c8b7cd68ae3993f372b1e5a99a47c8e0 100644
--- a/app/models/packages/package.rb
+++ b/app/models/packages/package.rb
@@ -208,15 +208,23 @@ class Packages::Package < ApplicationRecord
scope :order_by_package_file, -> { joins(:package_files).order('packages_package_files.created_at ASC') }
scope :order_project_path, -> do
- keyset_order = keyset_pagination_order(join_class: Project, column_name: :path, direction: :asc)
-
- joins(:project).reorder(keyset_order)
+ build_keyset_order_on_joined_column(
+ scope: joins(:project),
+ attribute_name: 'project_path',
+ column: Project.arel_table[:path],
+ direction: :asc,
+ nullable: :nulls_last
+ )
end
scope :order_project_path_desc, -> do
- keyset_order = keyset_pagination_order(join_class: Project, column_name: :path, direction: :desc)
-
- joins(:project).reorder(keyset_order)
+ build_keyset_order_on_joined_column(
+ scope: joins(:project),
+ attribute_name: 'project_path',
+ column: Project.arel_table[:path],
+ direction: :desc,
+ nullable: :nulls_first
+ )
end
def self.inheritance_column = 'package_type'
@@ -294,33 +302,6 @@ def self.sort_by_attribute(method)
end
end
- def self.keyset_pagination_order(join_class:, column_name:, direction: :asc)
- join_table = join_class.table_name
- asc_order_expression = join_class.arel_table[column_name].asc.nulls_last
- desc_order_expression = join_class.arel_table[column_name].desc.nulls_first
- order_direction = direction == :asc ? asc_order_expression : desc_order_expression
- reverse_order_direction = direction == :asc ? desc_order_expression : asc_order_expression
- arel_order_classes = ::Gitlab::Pagination::Keyset::ColumnOrderDefinition::AREL_ORDER_CLASSES.invert
-
- ::Gitlab::Pagination::Keyset::Order.build(
- [
- ::Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(
- attribute_name: "#{join_table}_#{column_name}",
- column_expression: join_class.arel_table[column_name],
- order_expression: order_direction,
- reversed_order_expression: reverse_order_direction,
- order_direction: direction,
- distinct: false,
- add_to_projections: true
- ),
- ::Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(
- attribute_name: 'id',
- order_expression: arel_order_classes[direction].new(Packages::Package.arel_table[:id]),
- add_to_projections: true
- )
- ])
- end
-
def versions
project.packages
.preload_pipelines
diff --git a/spec/graphql/resolvers/group_packages_resolver_spec.rb b/spec/graphql/resolvers/group_packages_resolver_spec.rb
index 639d4d93b7966d2ad79b38e1eec8eda7c0754781..897afe088ccc1f337a104d66af989bb3d9f63247 100644
--- a/spec/graphql/resolvers/group_packages_resolver_spec.rb
+++ b/spec/graphql/resolvers/group_packages_resolver_spec.rb
@@ -2,10 +2,9 @@
require 'spec_helper'
-RSpec.describe 'Resolvers::GroupPackagesResolver' do
+RSpec.describe Resolvers::GroupPackagesResolver, feature_category: :package_registry do
include GraphqlHelpers
- let_it_be(:described_class) { Resolvers::GroupPackagesResolver }
let_it_be(:user) { create(:user) }
let_it_be(:group) { create(:group, :public) }
let_it_be(:project) { create(:project, :public, group: group, path: 'a') }
@@ -26,16 +25,16 @@
let_it_be(:package3) { create(:package, project: project) }
let_it_be(:package4) { create(:package, project: project2) }
- context 'filter by package_name' do
+ context 'when sorting desc' do
let(:args) { { sort: 'PROJECT_PATH_DESC' } }
it { is_expected.to eq([package4, package2, package3, package]) }
end
- context 'filter by package_type' do
+ context 'when sorting asc' do
let(:args) { { sort: 'PROJECT_PATH_ASC' } }
- it { is_expected.to eq([package, package3, package2, package4]) }
+ it { is_expected.to eq([package3, package, package4, package2]) }
end
end
end
diff --git a/spec/models/packages/package_spec.rb b/spec/models/packages/package_spec.rb
index 4fdf41f544ac5ea02d2e3809d868afec9a2aee82..c022958642d793497e95e5db1517c5d862bfb351 100644
--- a/spec/models/packages/package_spec.rb
+++ b/spec/models/packages/package_spec.rb
@@ -88,6 +88,8 @@
RSpec.shared_examples 'package sorting by attribute' do |order_by|
subject { described_class.where(id: packages.map(&:id)).sort_by_attribute("#{order_by}_#{sort}").to_a }
+ let(:packages_desc) { packages.reverse }
+
context "sorting by #{order_by}" do
context 'ascending order' do
let(:sort) { 'asc' }
@@ -98,7 +100,7 @@
context 'descending order' do
let(:sort) { 'desc' }
- it { is_expected.to eq packages.reverse }
+ it { is_expected.to eq(packages_desc) }
end
end
end
@@ -123,7 +125,8 @@
let_it_be(:another_project) { create(:project, :public, namespace: group, name: 'project B', path: 'project-b') }
let_it_be(:package4) { create(:npm_package, project: another_project, version: '3.1.0', name: "@#{project.root_namespace.path}/bar") }
- let(:packages) { [package1, package2, package3, package4] }
+ let(:packages) { [package3, package2, package1, package4] }
+ let(:packages_desc) { [package4, package3, package2, package1] }
end
end
@@ -1056,8 +1059,8 @@
end
context 'sorting' do
- let_it_be(:project) { create(:project, name: 'aaa') }
- let_it_be(:project2) { create(:project, name: 'bbb') }
+ let_it_be(:project) { create(:project, path: 'aaa') }
+ let_it_be(:project2) { create(:project, path: 'bbb') }
let_it_be(:package1) { create(:package, project: project) }
let_it_be(:package2) { create(:package, project: project2) }
@@ -1073,20 +1076,13 @@
let_it_be(:package3) { create(:package, project: project2) }
let_it_be(:package4) { create(:package, project: project) }
- shared_examples 'order_project_path scope' do
- it 'orders packages by their projects path asc, then package id asc' do
- expect(described_class.order_project_path).to eq([package1, package4, package2, package3])
- end
+ it 'orders packages by their projects path asc, then package id desc' do
+ expect(described_class.order_project_path).to eq([package4, package1, package3, package2])
end
- shared_examples 'order_project_path_desc scope' do
- it 'orders packages by their projects path desc, then package id desc' do
- expect(described_class.order_project_path_desc).to eq([package3, package2, package4, package1])
- end
+ it 'orders packages by their projects path desc, then package id desc' do
+ expect(described_class.order_project_path_desc).to eq([package3, package2, package4, package1])
end
-
- it_behaves_like 'order_project_path scope'
- it_behaves_like 'order_project_path_desc scope'
end
end
@@ -1104,33 +1100,6 @@
end
end
- describe '.keyset_pagination_order' do
- let(:join_class) { nil }
- let(:column_name) { nil }
- let(:direction) { nil }
-
- subject { described_class.keyset_pagination_order(join_class: join_class, column_name: column_name, direction: direction) }
-
- it { expect { subject }.to raise_error(NoMethodError) }
-
- context 'with valid params' do
- let(:join_class) { Project }
- let(:column_name) { :name }
-
- context 'ascending direction' do
- let(:direction) { :asc }
-
- it { is_expected.to eq('"projects"."name" ASC NULLS LAST, "packages_packages"."id" ASC') }
- end
-
- context 'descending direction' do
- let(:direction) { :desc }
-
- it { is_expected.to eq('"projects"."name" DESC NULLS FIRST, "packages_packages"."id" DESC') }
- end
- end
- end
-
describe '.preload_tags' do
let_it_be(:package) { create(:npm_package) }
let_it_be(:tags) { create_list(:packages_tag, 2, package: package) }
diff --git a/spec/requests/api/group_packages_spec.rb b/spec/requests/api/group_packages_spec.rb
index 0786815c787325b9570e1d8a75caa1b0d54c84a2..2962d6d6140ab6c8f7c02d2192b3067125e31857 100644
--- a/spec/requests/api/group_packages_spec.rb
+++ b/spec/requests/api/group_packages_spec.rb
@@ -57,7 +57,8 @@
let(:another_project) { create(:project, :public, namespace: group, name: 'project B') }
let!(:package4) { create(:npm_package, project: another_project, version: '3.1.0', name: "@#{project.root_namespace.path}/bar") }
- let(:packages) { [package1, package2, package3, package4] }
+ let(:packages) { [package3, package2, package1, package4] }
+ let(:package_ids_desc) { [package4.id, package3.id, package2.id, package1.id] }
end
end
diff --git a/spec/support/shared_examples/services/packages_shared_examples.rb b/spec/support/shared_examples/services/packages_shared_examples.rb
index 7e7d8605d0b26419bf8605210b370c9933dba511..0f590a43e4a70899a9ff9c9a3e5ffc1de219bd74 100644
--- a/spec/support/shared_examples/services/packages_shared_examples.rb
+++ b/spec/support/shared_examples/services/packages_shared_examples.rb
@@ -131,6 +131,8 @@
RSpec.shared_examples 'package sorting' do |order_by|
subject { get api(url), params: { sort: sort, order_by: order_by } }
+ let(:package_ids_desc) { packages.reverse.map(&:id) }
+
context "sorting by #{order_by}" do
context 'ascending order' do
let(:sort) { 'asc' }
@@ -148,7 +150,7 @@
it 'returns the sorted packages' do
subject
- expect(json_response.pluck('id')).to eq(packages.reverse.map(&:id))
+ expect(json_response.pluck('id')).to eq(package_ids_desc)
end
end
end