From 71d5470100cccb56dc4c317b1e2bc8474273c094 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Fri, 21 Jul 2023 16:20:31 +0200 Subject: [PATCH 01/14] fix(graphql): implement real cursor pagination --- Gemfile.lock | 2 +- app/graphql/api/v2/stored_query.rb | 28 +- app/graphql/connections/cursor_connection.rb | 116 ++++ .../deleted_dossiers_connection.rb | 13 + .../connections/dossiers_connection.rb | 10 +- .../pending_deleted_dossiers_connection.rb | 36 ++ app/graphql/schema.graphql | 17 +- app/graphql/types/demarche_type.rb | 28 +- .../groupe_instructeur_with_dossiers_type.rb | 28 +- app/models/dossier.rb | 4 +- .../graphql_controller_stored_queries_spec.rb | 527 ++++++++++++++++-- 11 files changed, 722 insertions(+), 87 deletions(-) create mode 100644 app/graphql/connections/cursor_connection.rb create mode 100644 app/graphql/connections/deleted_dossiers_connection.rb create mode 100644 app/graphql/connections/pending_deleted_dossiers_connection.rb diff --git a/Gemfile.lock b/Gemfile.lock index 8ad9fa508..20b6309aa 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -278,7 +278,7 @@ GEM i18n (>= 0.7) multi_json request_store (>= 1.0) - graphql (2.0.15) + graphql (2.0.24) graphql-batch (0.5.1) graphql (>= 1.10, < 3) promise.rb (~> 0.7.2) diff --git a/app/graphql/api/v2/stored_query.rb b/app/graphql/api/v2/stored_query.rb index 5650db3fb..00662d48e 100644 --- a/app/graphql/api/v2/stored_query.rb +++ b/app/graphql/api/v2/stored_query.rb @@ -18,17 +18,21 @@ class API::V2::StoredQuery $state: DossierState $order: Order $first: Int + $last: Int + $before: String $after: String $archived: Boolean $revision: ID $createdSince: ISO8601DateTime $updatedSince: ISO8601DateTime - $pendingDeletedOrder: Order $pendingDeletedFirst: Int + $pendingDeletedLast: Int + $pendingDeletedBefore: String $pendingDeletedAfter: String $pendingDeletedSince: ISO8601DateTime - $deletedOrder: Order $deletedFirst: Int + $deletedLast: Int + $deletedBefore: String $deletedAfter: String $deletedSince: ISO8601DateTime $includeGroupeInstructeurs: Boolean = false @@ -67,6 +71,8 @@ class API::V2::StoredQuery state: $state order: $order first: $first + last: $last + before: $before after: $after archived: $archived createdSince: $createdSince @@ -81,8 +87,9 @@ class API::V2::StoredQuery } } pendingDeletedDossiers( - order: $pendingDeletedOrder first: $pendingDeletedFirst + last: $pendingDeletedLast + before: $pendingDeletedBefore after: $pendingDeletedAfter deletedSince: $pendingDeletedSince ) @include(if: $includePendingDeletedDossiers) { @@ -94,8 +101,9 @@ class API::V2::StoredQuery } } deletedDossiers( - order: $deletedOrder first: $deletedFirst + last: $deletedLast + before: $deletedBefore after: $deletedAfter deletedSince: $deletedSince ) @include(if: $includeDeletedDossiers) { @@ -114,6 +122,8 @@ class API::V2::StoredQuery $state: DossierState $order: Order $first: Int + $last: Int + $before: String $after: String $archived: Boolean $revision: ID @@ -121,10 +131,14 @@ class API::V2::StoredQuery $updatedSince: ISO8601DateTime $pendingDeletedOrder: Order $pendingDeletedFirst: Int + $pendingDeletedLast: Int + $pendingDeletedBefore: String $pendingDeletedAfter: String $pendingDeletedSince: ISO8601DateTime $deletedOrder: Order $deletedFirst: Int + $deletedLast: Int + $deletedBefore: String $deletedAfter: String $deletedSince: ISO8601DateTime $includeDossiers: Boolean = false @@ -151,6 +165,8 @@ class API::V2::StoredQuery state: $state order: $order first: $first + last: $last + before: $before after: $after archived: $archived createdSince: $createdSince @@ -167,6 +183,8 @@ class API::V2::StoredQuery pendingDeletedDossiers( order: $pendingDeletedOrder first: $pendingDeletedFirst + last: $pendingDeletedLast + before: $pendingDeletedBefore after: $pendingDeletedAfter deletedSince: $pendingDeletedSince ) @include(if: $includePendingDeletedDossiers) { @@ -180,6 +198,8 @@ class API::V2::StoredQuery deletedDossiers( order: $deletedOrder first: $deletedFirst + last: $deletedLast + before: $deletedBefore after: $deletedAfter deletedSince: $deletedSince ) @include(if: $includeDeletedDossiers) { diff --git a/app/graphql/connections/cursor_connection.rb b/app/graphql/connections/cursor_connection.rb new file mode 100644 index 000000000..3f2b53994 --- /dev/null +++ b/app/graphql/connections/cursor_connection.rb @@ -0,0 +1,116 @@ +module Connections + class CursorConnection < GraphQL::Pagination::Connection + def initialize(items, deprecated_order: nil, **kwargs) + super(items, **kwargs) + @deprecated_order = deprecated_order + end + + def nodes + load_nodes + end + + def has_previous_page + load_nodes + @has_previous_page + end + + def has_next_page + load_nodes + @has_next_page + end + + def cursor_for(item) + cursor_from_column(item, order_column) + end + + private + + def load_nodes + @nodes ||= begin + page_info = compute_page_info(before:, after:, first:, last:) + nodes = resolve_nodes(**page_info.slice(:before, :after, :limit, :inverted)) + result_size = nodes.size + @has_previous_page = page_info[:has_previous_page].(result_size) + @has_next_page = page_info[:has_next_page].(result_size) + + trimmed_nodes = nodes.first(page_info[:expected_size]) + trimmed_nodes.reverse! if page_info[:inverted] + trimmed_nodes + end + end + + def timestamp_and_id_from_cursor(cursor) + timestamp, id = decode(cursor).split(';') + [Time.zone.parse(timestamp), id.to_i] + end + + def cursor_from_column(item, column) + encode([item.read_attribute(column).utc.strftime("%Y-%m-%dT%H:%M:%S.%NZ"), item.id].join(';')) + end + + def order_column + :updated_at + end + + def order_table + raise StandardError, 'Not implemented' + end + + def resolve_nodes(before:, after:, limit:, inverted:) + order = inverted ? :desc : :asc + nodes = items.order(order_column => order, id: order) + nodes = nodes.limit(limit) + + if before.present? + timestamp, id = timestamp_and_id_from_cursor(before) + nodes.where("(#{order_table}.#{order_column}, #{order_table}.id) < (?, ?)", timestamp, id) + elsif after.present? + timestamp, id = timestamp_and_id_from_cursor(after) + nodes.where("(#{order_table}.#{order_column}, #{order_table}.id) > (?, ?)", timestamp, id) + else + nodes + end + end + + def compute_page_info(before: nil, after: nil, first: nil, last: nil) + if first.present? && last.present? + raise GraphQL::ExecutionError.new('Arguments "first" and "last" are exclusive', extensions: { code: :bad_request }) + end + + if before.present? && after.present? + raise GraphQL::ExecutionError.new('Arguments "before" and "after" are exclusive', extensions: { code: :bad_request }) + end + + if first.present? && first < 0 + raise GraphQL::ExecutionError.new('Argument "first" must be a non-negative integer', extensions: { code: :bad_request }) + end + + if last.present? && last < 0 + raise GraphQL::ExecutionError.new('Argument "last" must be a non-negative integer', extensions: { code: :bad_request }) + end + + if @deprecated_order == :desc + if last.present? + first = [last, max_page_size].min + last = nil + else + last = [first || default_page_size, max_page_size].min + first = nil + end + end + + limit = [first || last || default_page_size, max_page_size].min + 1 + inverted = last.present? || before.present? + + { + before:, + after:, + limit:, + inverted:, + expected_size: limit - 1, + has_previous_page: -> (result_size) { after.present? || (result_size >= limit && inverted) }, + has_next_page: -> (result_size) { before.present? || (result_size >= limit && !inverted) } + } + end + end +end diff --git a/app/graphql/connections/deleted_dossiers_connection.rb b/app/graphql/connections/deleted_dossiers_connection.rb new file mode 100644 index 000000000..2416b78c5 --- /dev/null +++ b/app/graphql/connections/deleted_dossiers_connection.rb @@ -0,0 +1,13 @@ +module Connections + class DeletedDossiersConnection < CursorConnection + private + + def order_column + :deleted_at + end + + def order_table + :deleted_dossiers + end + end +end diff --git a/app/graphql/connections/dossiers_connection.rb b/app/graphql/connections/dossiers_connection.rb index 34567936b..1f1db6436 100644 --- a/app/graphql/connections/dossiers_connection.rb +++ b/app/graphql/connections/dossiers_connection.rb @@ -1,5 +1,5 @@ module Connections - class DossiersConnection < GraphQL::Pagination::ActiveRecordRelationConnection + class DossiersConnection < CursorConnection def initialize(items, lookahead: nil, **kwargs) super(items, **kwargs) @lookahead = lookahead @@ -15,6 +15,14 @@ module Connections private + def order_column + arguments[:updated_since].present? ? :updated_at : :depose_at + end + + def order_table + :dossiers + end + # We check if the query selects champs form dossier. If it's the case we preload the dossier. def preload? @lookahead.selection(:nodes).selects?(:champs) || @lookahead.selection(:nodes).selects?(:annotations) diff --git a/app/graphql/connections/pending_deleted_dossiers_connection.rb b/app/graphql/connections/pending_deleted_dossiers_connection.rb new file mode 100644 index 000000000..2fbf63b09 --- /dev/null +++ b/app/graphql/connections/pending_deleted_dossiers_connection.rb @@ -0,0 +1,36 @@ +module Connections + class PendingDeletedDossiersConnection < CursorConnection + def cursor_for(item) + if item.en_construction? + cursor_from_column(item, :hidden_by_user_at) + else + cursor_from_column(item, :hidden_by_administration_at) + end + end + + private + + def resolve_nodes(before:, after:, limit:, inverted:) + order = inverted ? :desc : :asc + + dossiers_table = Dossier.arel_table + case_statement = dossiers_table[:state] + .when(:en_construction) + .then(dossiers_table[:hidden_by_user_at]) + .else(dossiers_table[:hidden_by_administration_at]) + + nodes = items.order(case_statement.public_send(order)).order(dossiers_table[:id].public_send(order)) + nodes = nodes.limit(limit) + + if before.present? + timestamp, id = timestamp_and_id_from_cursor(before) + nodes.where("(#{case_statement.to_sql}, dossiers.id) < (?, ?)", timestamp, id) + elsif after.present? + timestamp, id = timestamp_and_id_from_cursor(after) + nodes.where("(#{case_statement.to_sql}, dossiers.id) > (?, ?)", timestamp, id) + else + nodes + end + end + end +end diff --git a/app/graphql/schema.graphql b/app/graphql/schema.graphql index 0d0ea7a2f..404457cd6 100644 --- a/app/graphql/schema.graphql +++ b/app/graphql/schema.graphql @@ -1,3 +1,8 @@ +""" +Requires that exactly one field must be supplied and that field must not be `null`. +""" +directive @oneOf on INPUT_OBJECT + type Address { """ code INSEE de la commune @@ -895,7 +900,7 @@ type Demarche { """ L’ordre des dossiers supprimés. """ - order: Order = ASC + order: Order = ASC @deprecated(reason: "Utilisez l’argument `last` à la place.") ): DeletedDossierConnection! """ @@ -950,7 +955,7 @@ type Demarche { """ L’ordre des dossiers. """ - order: Order = ASC + order: Order = ASC @deprecated(reason: "Utilisez l’argument `last` à la place.") """ Seulement les dossiers pour la révision donnée. @@ -1008,7 +1013,7 @@ type Demarche { """ L’ordre des dossiers en attente de suppression. """ - order: Order = ASC + order: Order = ASC @deprecated(reason: "Utilisez l’argument `last` à la place.") ): DeletedDossierConnection! publishedRevision: Revision revisions: [Revision!]! @@ -2580,7 +2585,7 @@ type GroupeInstructeurWithDossiers { """ L’ordre des dossiers supprimés. """ - order: Order = ASC + order: Order = ASC @deprecated(reason: "Utilisez l’argument `last` à la place.") ): DeletedDossierConnection! """ @@ -2630,7 +2635,7 @@ type GroupeInstructeurWithDossiers { """ L’ordre des dossiers. """ - order: Order = ASC + order: Order = ASC @deprecated(reason: "Utilisez l’argument `last` à la place.") """ Seulement les dossiers pour la révision donnée. @@ -2692,7 +2697,7 @@ type GroupeInstructeurWithDossiers { """ L’ordre des dossiers en attente de suppression. """ - order: Order = ASC + order: Order = ASC @deprecated(reason: "Utilisez l’argument `last` à la place.") ): DeletedDossierConnection! } diff --git a/app/graphql/types/demarche_type.rb b/app/graphql/types/demarche_type.rb index 5f0eafe3f..319da410d 100644 --- a/app/graphql/types/demarche_type.rb +++ b/app/graphql/types/demarche_type.rb @@ -35,7 +35,7 @@ module Types field :service, Types::ServiceType, null: true field :dossiers, Types::DossierType.connection_type, "Liste de tous les dossiers d’une démarche.", null: false, extras: [:lookahead] do - argument :order, Types::Order, default_value: :asc, required: false, description: "L’ordre des dossiers." + argument :order, Types::Order, default_value: :asc, required: false, description: "L’ordre des dossiers.", deprecation_reason: 'Utilisez l’argument `last` à la place.' argument :created_since, GraphQL::Types::ISO8601DateTime, required: false, description: "Dossiers déposés depuis la date." argument :updated_since, GraphQL::Types::ISO8601DateTime, required: false, description: "Dossiers mis à jour depuis la date." argument :state, Types::DossierType::DossierState, required: false, description: "Dossiers avec statut." @@ -46,11 +46,11 @@ module Types end field :deleted_dossiers, Types::DeletedDossierType.connection_type, "Liste de tous les dossiers supprimés d’une démarche.", null: false do - argument :order, Types::Order, default_value: :asc, required: false, description: "L’ordre des dossiers supprimés." + argument :order, Types::Order, default_value: :asc, required: false, description: "L’ordre des dossiers supprimés.", deprecation_reason: 'Utilisez l’argument `last` à la place.' argument :deleted_since, GraphQL::Types::ISO8601DateTime, required: false, description: "Dossiers supprimés depuis la date." end field :pending_deleted_dossiers, Types::DeletedDossierType.connection_type, "Liste de tous les dossiers en attente de suppression définitive d’une démarche.", null: false do - argument :order, Types::Order, default_value: :asc, required: false, description: "L’ordre des dossiers en attente de suppression." + argument :order, Types::Order, default_value: :asc, required: false, description: "L’ordre des dossiers en attente de suppression.", deprecation_reason: 'Utilisez l’argument `last` à la place.' argument :deleted_since, GraphQL::Types::ISO8601DateTime, required: false, description: "Dossiers en attente de suppression depuis la date." end @@ -111,20 +111,18 @@ module Types end if updated_since.present? - dossiers = dossiers.updated_since(updated_since).order_by_updated_at(order) + dossiers = dossiers.updated_since(updated_since) else if created_since.present? dossiers = dossiers.created_since(created_since) end - - dossiers = dossiers.order_by_created_at(order) end # We wrap dossiers in a custom connection alongsite the lookahead for the query. # The custom connection is responsible for preloading paginated dossiers. # https://graphql-ruby.org/pagination/custom_connections.html#using-a-custom-connection # https://graphql-ruby.org/queries/lookahead.html - Connections::DossiersConnection.new(dossiers, lookahead: lookahead) + Connections::DossiersConnection.new(dossiers, lookahead: lookahead, deprecated_order: order) end def deleted_dossiers(deleted_since: nil, order:) @@ -134,7 +132,7 @@ module Types dossiers = dossiers.deleted_since(deleted_since) end - dossiers.order(deleted_at: order) + Connections::DeletedDossiersConnection.new(dossiers, deprecated_order: order) end def pending_deleted_dossiers(deleted_since: nil, order:) @@ -144,19 +142,7 @@ module Types dossiers = dossiers.hidden_since(deleted_since) end - dossiers_table = Dossier.arel_table - case_statement = dossiers_table[:state] - .when(:en_construction) - .then(dossiers_table[:hidden_by_user_at]) - .else(dossiers_table[:hidden_by_administration_at]) - - dossiers = dossiers.order(case_statement) - - if order == :desc - dossiers = dossiers.reverse_order - end - - dossiers + Connections::PendingDeletedDossiersConnection.new(dossiers, deprecated_order: order) end def champ_descriptors diff --git a/app/graphql/types/groupe_instructeur_with_dossiers_type.rb b/app/graphql/types/groupe_instructeur_with_dossiers_type.rb index 91d0865cb..32999241a 100644 --- a/app/graphql/types/groupe_instructeur_with_dossiers_type.rb +++ b/app/graphql/types/groupe_instructeur_with_dossiers_type.rb @@ -3,7 +3,7 @@ module Types description "Un groupe instructeur avec ses dossiers" field :dossiers, Types::DossierType.connection_type, "Liste de tous les dossiers d’un groupe instructeur.", null: false, extras: [:lookahead] do - argument :order, Types::Order, default_value: :asc, required: false, description: "L’ordre des dossiers." + argument :order, Types::Order, default_value: :asc, required: false, description: "L’ordre des dossiers.", deprecation_reason: 'Utilisez l’argument `last` à la place.' argument :created_since, GraphQL::Types::ISO8601DateTime, required: false, description: "Dossiers déposés depuis la date." argument :updated_since, GraphQL::Types::ISO8601DateTime, required: false, description: "Dossiers mis à jour depuis la date." argument :state, Types::DossierType::DossierState, required: false, description: "Dossiers avec statut." @@ -14,12 +14,12 @@ module Types end field :deleted_dossiers, Types::DeletedDossierType.connection_type, "Liste de tous les dossiers supprimés d’un groupe instructeur.", null: false do - argument :order, Types::Order, default_value: :asc, required: false, description: "L’ordre des dossiers supprimés." + argument :order, Types::Order, default_value: :asc, required: false, description: "L’ordre des dossiers supprimés.", deprecation_reason: 'Utilisez l’argument `last` à la place.' argument :deleted_since, GraphQL::Types::ISO8601DateTime, required: false, description: "Dossiers supprimés depuis la date." end field :pending_deleted_dossiers, Types::DeletedDossierType.connection_type, "Liste de tous les dossiers en attente de suppression définitive d’un groupe instructeur.", null: false do - argument :order, Types::Order, default_value: :asc, required: false, description: "L’ordre des dossiers en attente de suppression." + argument :order, Types::Order, default_value: :asc, required: false, description: "L’ordre des dossiers en attente de suppression.", deprecation_reason: 'Utilisez l’argument `last` à la place.' argument :deleted_since, GraphQL::Types::ISO8601DateTime, required: false, description: "Dossiers en attente de suppression depuis la date." end @@ -50,20 +50,18 @@ module Types end if updated_since.present? - dossiers = dossiers.updated_since(updated_since).order_by_updated_at(order) + dossiers = dossiers.updated_since(updated_since) else if created_since.present? dossiers = dossiers.created_since(created_since) end - - dossiers = dossiers.order_by_created_at(order) end # We wrap dossiers in a custom connection alongsite the lookahead for the query. # The custom connection is responsible for preloading paginated dossiers. # https://graphql-ruby.org/pagination/custom_connections.html#using-a-custom-connection # https://graphql-ruby.org/queries/lookahead.html - Connections::DossiersConnection.new(dossiers, lookahead: lookahead) + Connections::DossiersConnection.new(dossiers, lookahead: lookahead, deprecated_order: order) end def deleted_dossiers(deleted_since: nil, order:) @@ -73,7 +71,7 @@ module Types dossiers = dossiers.deleted_since(deleted_since) end - dossiers.order(deleted_at: order) + Connections::DeletedDossiersConnection.new(dossiers, deprecated_order: order) end def pending_deleted_dossiers(deleted_since: nil, order:) @@ -83,19 +81,7 @@ module Types dossiers = dossiers.hidden_since(deleted_since) end - dossiers_table = Dossier.arel_table - case_statement = dossiers_table[:state] - .when(:en_construction) - .then(dossiers_table[:hidden_by_user_at]) - .else(dossiers_table[:hidden_by_administration_at]) - - dossiers = dossiers.order(case_statement) - - if order == :desc - dossiers = dossiers.reverse_order - end - - dossiers + Connections::PendingDeletedDossiersConnection.new(dossiers, deprecated_order: order) end end end diff --git a/app/models/dossier.rb b/app/models/dossier.rb index 17931f171..7b022faf4 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -229,8 +229,8 @@ class Dossier < ApplicationRecord scope :for_procedure_preview, -> { where(for_procedure_preview: true) } scope :for_editing_fork, -> { where.not(editing_fork_origin_id: nil) } scope :for_groupe_instructeur, -> (groupe_instructeurs) { where(groupe_instructeur: groupe_instructeurs) } - scope :order_by_updated_at, -> (order = :desc) { order(updated_at: order) } - scope :order_by_created_at, -> (order = :asc) { order(depose_at: order, created_at: order, id: order) } + scope :order_by_updated_at, -> (order = :desc) { order(updated_at: order, id: order) } + scope :order_by_created_at, -> (order = :asc) { order(depose_at: order, id: order) } scope :updated_since, -> (since) { where('dossiers.updated_at >= ?', since) } scope :created_since, -> (since) { where('dossiers.depose_at >= ?', since) } scope :hidden_by_user_since, -> (since) { where('dossiers.hidden_by_user_at IS NOT NULL AND dossiers.hidden_by_user_at >= ?', since) } diff --git a/spec/controllers/api/v2/graphql_controller_stored_queries_spec.rb b/spec/controllers/api/v2/graphql_controller_stored_queries_spec.rb index 8da03933a..1e9776844 100644 --- a/spec/controllers/api/v2/graphql_controller_stored_queries_spec.rb +++ b/spec/controllers/api/v2/graphql_controller_stored_queries_spec.rb @@ -6,10 +6,13 @@ describe API::V2::GraphqlController do let(:legacy_token) { APIToken.send(:unpack, token)[:plain_token] } let(:procedure) { create(:procedure, :published, :for_individual, :with_service, administrateurs: [admin], types_de_champ_public:) } let(:types_de_champ_public) { [] } - let(:dossier) { create(:dossier, :en_construction, :with_individual, procedure: procedure) } - let(:dossier1) { create(:dossier, :en_construction, :with_individual, procedure: procedure, en_construction_at: 1.day.ago) } - let(:dossier2) { create(:dossier, :en_construction, :with_individual, :archived, procedure: procedure, en_construction_at: 3.days.ago) } - let(:dossier_accepte) { create(:dossier, :accepte, :with_individual, procedure: procedure) } + let(:dossier) { create(:dossier, :en_construction, :with_individual, procedure:, depose_at: 4.days.ago) } + let(:dossier1) { create(:dossier, :en_construction, :with_individual, procedure:, en_construction_at: 1.day.ago, depose_at: 3.days.ago) } + let(:dossier2) { create(:dossier, :en_construction, :with_individual, :archived, procedure:, en_construction_at: 3.days.ago, depose_at: 2.days.ago) } + let(:dossier3) { create(:dossier, :accepte, :with_individual, procedure:, depose_at: 1.day.ago) } + let(:dossier_accepte) { create(:dossier, :accepte, :with_individual, procedure:) } + let(:dossier_accepte1) { create(:dossier, :accepte, :with_individual, procedure:) } + let(:dossier_accepte2) { create(:dossier, :accepte, :with_individual, procedure:) } let(:dossiers) { [dossier] } let(:instructeur) { create(:instructeur, followed_dossiers: dossiers) } let(:authorization_header) { ActionController::HttpAuthentication::Token.encode_credentials(token) } @@ -45,7 +48,7 @@ describe API::V2::GraphqlController do end describe 'ds-query-v2' do - let(:dossier) { create(:dossier, :en_construction, :with_individual, procedure: procedure) } + let(:dossier) { create(:dossier, :en_construction, :with_individual, procedure:, depose_at: 4.days.ago) } let(:query_id) { 'ds-query-v2' } context 'not found operation id' do @@ -145,16 +148,6 @@ describe API::V2::GraphqlController do } end - context 'include Dossiers' do - let(:variables) { { demarcheNumber: procedure.id, includeDossiers: true } } - - it { - expect(gql_errors).to be_nil - expect(gql_data[:demarche][:id]).to eq(procedure.to_typed_id) - expect(gql_data[:demarche][:dossiers][:nodes].size).to eq(1) - } - end - context 'include Revision' do let(:variables) { { demarcheNumber: procedure.id, includeRevision: true } } @@ -165,41 +158,513 @@ describe API::V2::GraphqlController do } end - context 'include deleted Dossiers' do - let(:variables) { { demarcheNumber: procedure.id, includeDeletedDossiers: true, deletedSince: 2.weeks.ago.iso8601 } } - let(:deleted_dossier) { DeletedDossier.create_from_dossier(dossier_accepte, DeletedDossier.reasons.fetch(:user_request)) } + context 'include Dossiers' do + def cursor_for(item, column) + cursor = [item.read_attribute(column).utc.strftime("%Y-%m-%dT%H:%M:%S.%NZ"), item.id].join(';') + API::V2::Schema.cursor_encoder.encode(cursor, nonce: true) + end - before { deleted_dossier } + let(:order_column) { :depose_at } + let(:variables) { { demarcheNumber: procedure.id, includeDossiers: true } } + let(:start_cursor) { cursor_for(dossier, order_column) } + let(:end_cursor) { cursor_for(dossier3, order_column) } + + before { dossier1; dossier2; dossier3 } + + context 'depose_at' do + it { + expect(gql_errors).to be_nil + expect(gql_data[:demarche][:id]).to eq(procedure.to_typed_id) + expect(gql_data[:demarche][:dossiers][:nodes].size).to eq(4) + expect(gql_data[:demarche][:dossiers][:pageInfo][:hasNextPage]).to be_falsey + expect(gql_data[:demarche][:dossiers][:pageInfo][:hasPreviousPage]).to be_falsey + expect(gql_data[:demarche][:dossiers][:pageInfo][:startCursor]).to eq(start_cursor) + expect(gql_data[:demarche][:dossiers][:pageInfo][:endCursor]).to eq(end_cursor) + } + + context 'first' do + let(:variables) { { demarcheNumber: procedure.id, includeDossiers: true, first: 2 } } + let(:end_cursor) { cursor_for(dossier1, order_column) } + + it { + expect(gql_errors).to be_nil + expect(gql_data[:demarche][:dossiers][:nodes].size).to eq(2) + expect(gql_data[:demarche][:dossiers][:pageInfo][:hasNextPage]).to be_truthy + expect(gql_data[:demarche][:dossiers][:pageInfo][:hasPreviousPage]).to be_falsey + expect(gql_data[:demarche][:dossiers][:pageInfo][:startCursor]).to eq(start_cursor) + expect(gql_data[:demarche][:dossiers][:pageInfo][:endCursor]).to eq(end_cursor) + } + + context 'with deprecated order' do + let(:variables) { { demarcheNumber: procedure.id, includeDossiers: true, first: 2, order: 'DESC' } } + let(:start_cursor) { cursor_for(dossier2, order_column) } + let(:end_cursor) { cursor_for(dossier3, order_column) } + + it { + expect(gql_errors).to be_nil + expect(gql_data[:demarche][:dossiers][:nodes].size).to eq(2) + expect(gql_data[:demarche][:dossiers][:pageInfo][:hasNextPage]).to be_falsey + expect(gql_data[:demarche][:dossiers][:pageInfo][:hasPreviousPage]).to be_truthy + expect(gql_data[:demarche][:dossiers][:pageInfo][:startCursor]).to eq(start_cursor) + expect(gql_data[:demarche][:dossiers][:pageInfo][:endCursor]).to eq(end_cursor) + } + end + + context 'after' do + let(:variables) { { demarcheNumber: procedure.id, includeDossiers: true, first: 2, after: current_cursor } } + let(:current_cursor) { cursor_for(dossier1, order_column) } + let(:start_cursor) { cursor_for(dossier2, order_column) } + let(:end_cursor) { cursor_for(dossier3, order_column) } + + it { + expect(gql_errors).to be_nil + expect(gql_data[:demarche][:dossiers][:nodes].size).to eq(2) + expect(gql_data[:demarche][:dossiers][:pageInfo][:hasNextPage]).to be_falsey + expect(gql_data[:demarche][:dossiers][:pageInfo][:hasPreviousPage]).to be_truthy + expect(gql_data[:demarche][:dossiers][:pageInfo][:startCursor]).to eq(start_cursor) + expect(gql_data[:demarche][:dossiers][:pageInfo][:endCursor]).to eq(end_cursor) + } + + context 'with deleted' do + let(:variables) { { demarcheNumber: procedure.id, includeDossiers: true } } + + before { dossier.hide_and_keep_track!(dossier.user, DeletedDossier.reasons.fetch(:user_request)) } + + it { + expect(gql_errors).to be_nil + expect(gql_data[:demarche][:dossiers][:nodes].size).to eq(3) + } + + context 'second page not changed' do + let(:variables) { { demarcheNumber: procedure.id, includeDossiers: true, first: 2, after: current_cursor } } + + it { + expect(gql_errors).to be_nil + expect(gql_data[:demarche][:dossiers][:nodes].size).to eq(2) + expect(gql_data[:demarche][:dossiers][:pageInfo][:hasNextPage]).to be_falsey + expect(gql_data[:demarche][:dossiers][:pageInfo][:hasPreviousPage]).to be_truthy + expect(gql_data[:demarche][:dossiers][:pageInfo][:startCursor]).to eq(start_cursor) + expect(gql_data[:demarche][:dossiers][:pageInfo][:endCursor]).to eq(end_cursor) + } + end + end + end + + context 'before' do + let(:variables) { { demarcheNumber: procedure.id, includeDossiers: true, first: 2, before: current_cursor } } + let(:current_cursor) { cursor_for(dossier2, order_column) } + let(:start_cursor) { cursor_for(dossier, order_column) } + let(:end_cursor) { cursor_for(dossier1, order_column) } + + it { + expect(gql_errors).to be_nil + expect(gql_data[:demarche][:dossiers][:nodes].size).to eq(2) + expect(gql_data[:demarche][:dossiers][:pageInfo][:hasNextPage]).to be_truthy + expect(gql_data[:demarche][:dossiers][:pageInfo][:hasPreviousPage]).to be_falsey + expect(gql_data[:demarche][:dossiers][:pageInfo][:startCursor]).to eq(start_cursor) + expect(gql_data[:demarche][:dossiers][:pageInfo][:endCursor]).to eq(end_cursor) + } + end + end + + context 'last' do + let(:variables) { { demarcheNumber: procedure.id, includeDossiers: true, last: 2 } } + let(:start_cursor) { cursor_for(dossier2, order_column) } + let(:end_cursor) { cursor_for(dossier3, order_column) } + + it { + expect(gql_errors).to be_nil + expect(gql_data[:demarche][:dossiers][:nodes].size).to eq(2) + expect(gql_data[:demarche][:dossiers][:pageInfo][:hasNextPage]).to be_falsey + expect(gql_data[:demarche][:dossiers][:pageInfo][:hasPreviousPage]).to be_truthy + expect(gql_data[:demarche][:dossiers][:pageInfo][:startCursor]).to eq(start_cursor) + expect(gql_data[:demarche][:dossiers][:pageInfo][:endCursor]).to eq(end_cursor) + } + + context 'before' do + let(:variables) { { demarcheNumber: procedure.id, includeDossiers: true, last: 2, before: current_cursor } } + let(:current_cursor) { cursor_for(dossier2, order_column) } + let(:start_cursor) { cursor_for(dossier, order_column) } + let(:end_cursor) { cursor_for(dossier1, order_column) } + + it { + expect(gql_errors).to be_nil + expect(gql_data[:demarche][:dossiers][:nodes].size).to eq(2) + expect(gql_data[:demarche][:dossiers][:pageInfo][:hasNextPage]).to be_truthy + expect(gql_data[:demarche][:dossiers][:pageInfo][:hasPreviousPage]).to be_falsey + expect(gql_data[:demarche][:dossiers][:pageInfo][:startCursor]).to eq(start_cursor) + expect(gql_data[:demarche][:dossiers][:pageInfo][:endCursor]).to eq(end_cursor) + } + end + + context 'after' do + let(:variables) { { demarcheNumber: procedure.id, includeDossiers: true, last: 2, after: current_cursor } } + let(:current_cursor) { cursor_for(dossier1, order_column) } + let(:start_cursor) { cursor_for(dossier2, order_column) } + let(:end_cursor) { cursor_for(dossier3, order_column) } + + it { + expect(gql_errors).to be_nil + expect(gql_data[:demarche][:dossiers][:nodes].size).to eq(2) + expect(gql_data[:demarche][:dossiers][:pageInfo][:hasNextPage]).to be_falsey + expect(gql_data[:demarche][:dossiers][:pageInfo][:hasPreviousPage]).to be_truthy + expect(gql_data[:demarche][:dossiers][:pageInfo][:startCursor]).to eq(start_cursor) + expect(gql_data[:demarche][:dossiers][:pageInfo][:endCursor]).to eq(end_cursor) + } + end + end + end + + context 'updated_at' do + let(:order_column) { :updated_at } + + context 'first' do + let(:variables) { { demarcheNumber: procedure.id, includeDossiers: true, first: 2, updatedSince: 10.days.ago.iso8601 } } + let(:end_cursor) { cursor_for(dossier1, order_column) } + + it { + expect(gql_errors).to be_nil + expect(gql_data[:demarche][:dossiers][:nodes].size).to eq(2) + expect(gql_data[:demarche][:dossiers][:pageInfo][:hasNextPage]).to be_truthy + expect(gql_data[:demarche][:dossiers][:pageInfo][:hasPreviousPage]).to be_falsey + expect(gql_data[:demarche][:dossiers][:pageInfo][:startCursor]).to eq(start_cursor) + expect(gql_data[:demarche][:dossiers][:pageInfo][:endCursor]).to eq(end_cursor) + } + + context 'after' do + let(:variables) { { demarcheNumber: procedure.id, includeDossiers: true, first: 2, after: current_cursor, updatedSince: 10.days.ago.iso8601 } } + let(:current_cursor) { cursor_for(dossier1, order_column) } + let(:start_cursor) { cursor_for(dossier2, order_column) } + let(:end_cursor) { cursor_for(dossier3, order_column) } + + it { + expect(gql_errors).to be_nil + expect(gql_data[:demarche][:dossiers][:nodes].size).to eq(2) + expect(gql_data[:demarche][:dossiers][:pageInfo][:hasNextPage]).to be_falsey + expect(gql_data[:demarche][:dossiers][:pageInfo][:hasPreviousPage]).to be_truthy + expect(gql_data[:demarche][:dossiers][:pageInfo][:startCursor]).to eq(start_cursor) + expect(gql_data[:demarche][:dossiers][:pageInfo][:endCursor]).to eq(end_cursor) + } + end + + context 'before' do + let(:variables) { { demarcheNumber: procedure.id, includeDossiers: true, first: 2, before: current_cursor, updatedSince: 10.days.ago.iso8601 } } + let(:current_cursor) { cursor_for(dossier2, order_column) } + let(:start_cursor) { cursor_for(dossier, order_column) } + let(:end_cursor) { cursor_for(dossier1, order_column) } + + it { + expect(gql_errors).to be_nil + expect(gql_data[:demarche][:dossiers][:nodes].size).to eq(2) + expect(gql_data[:demarche][:dossiers][:pageInfo][:hasNextPage]).to be_truthy + expect(gql_data[:demarche][:dossiers][:pageInfo][:hasPreviousPage]).to be_falsey + expect(gql_data[:demarche][:dossiers][:pageInfo][:startCursor]).to eq(start_cursor) + expect(gql_data[:demarche][:dossiers][:pageInfo][:endCursor]).to eq(end_cursor) + } + end + end + + context 'last' do + let(:variables) { { demarcheNumber: procedure.id, includeDossiers: true, last: 2, updatedSince: 10.days.ago.iso8601 } } + let(:start_cursor) { cursor_for(dossier2, order_column) } + let(:end_cursor) { cursor_for(dossier3, order_column) } + + it { + expect(gql_errors).to be_nil + expect(gql_data[:demarche][:dossiers][:nodes].size).to eq(2) + expect(gql_data[:demarche][:dossiers][:pageInfo][:hasNextPage]).to be_falsey + expect(gql_data[:demarche][:dossiers][:pageInfo][:hasPreviousPage]).to be_truthy + expect(gql_data[:demarche][:dossiers][:pageInfo][:startCursor]).to eq(start_cursor) + expect(gql_data[:demarche][:dossiers][:pageInfo][:endCursor]).to eq(end_cursor) + } + + context 'before' do + let(:variables) { { demarcheNumber: procedure.id, includeDossiers: true, last: 2, before: current_cursor, updatedSince: 10.days.ago.iso8601 } } + let(:current_cursor) { cursor_for(dossier2, order_column) } + let(:start_cursor) { cursor_for(dossier, order_column) } + let(:end_cursor) { cursor_for(dossier1, order_column) } + + it { + expect(gql_errors).to be_nil + expect(gql_data[:demarche][:dossiers][:nodes].size).to eq(2) + expect(gql_data[:demarche][:dossiers][:pageInfo][:hasNextPage]).to be_truthy + expect(gql_data[:demarche][:dossiers][:pageInfo][:hasPreviousPage]).to be_falsey + expect(gql_data[:demarche][:dossiers][:pageInfo][:startCursor]).to eq(start_cursor) + expect(gql_data[:demarche][:dossiers][:pageInfo][:endCursor]).to eq(end_cursor) + } + end + + context 'after' do + let(:variables) { { demarcheNumber: procedure.id, includeDossiers: true, last: 2, after: current_cursor, updatedSince: 10.days.ago.iso8601 } } + let(:current_cursor) { cursor_for(dossier1, order_column) } + let(:start_cursor) { cursor_for(dossier2, order_column) } + let(:end_cursor) { cursor_for(dossier3, order_column) } + + it { + expect(gql_errors).to be_nil + expect(gql_data[:demarche][:dossiers][:nodes].size).to eq(2) + expect(gql_data[:demarche][:dossiers][:pageInfo][:hasNextPage]).to be_falsey + expect(gql_data[:demarche][:dossiers][:pageInfo][:hasPreviousPage]).to be_truthy + expect(gql_data[:demarche][:dossiers][:pageInfo][:startCursor]).to eq(start_cursor) + expect(gql_data[:demarche][:dossiers][:pageInfo][:endCursor]).to eq(end_cursor) + } + end + end + end + end + + context 'include deleted Dossiers' do + def cursor_for(item) + cursor = [item.deleted_at.utc.strftime("%Y-%m-%dT%H:%M:%S.%NZ"), item.id].join(';') + API::V2::Schema.cursor_encoder.encode(cursor, nonce: true) + end + + let(:variables) { { demarcheNumber: procedure.id, includeDeletedDossiers: true, deletedSince: 2.weeks.ago.iso8601 } } + let(:deleted_dossier) { DeletedDossier.create_from_dossier(dossier_accepte, DeletedDossier.reasons.fetch(:user_request)).tap { _1.update(deleted_at: 4.days.ago) } } + let(:deleted_dossier1) { DeletedDossier.create_from_dossier(dossier_accepte1, DeletedDossier.reasons.fetch(:user_request)).tap { _1.update(deleted_at: 3.days.ago) } } + let(:deleted_dossier2) { DeletedDossier.create_from_dossier(dossier_accepte2, DeletedDossier.reasons.fetch(:user_request)).tap { _1.update(deleted_at: 2.days.ago) } } + let(:deleted_dossier3) { DeletedDossier.create_from_dossier(dossier3, DeletedDossier.reasons.fetch(:user_request)).tap { _1.update(deleted_at: 1.day.ago) } } + + let(:start_cursor) { cursor_for(deleted_dossier) } + let(:end_cursor) { cursor_for(deleted_dossier3) } + + before { deleted_dossier; deleted_dossier1; deleted_dossier2; deleted_dossier3 } it { expect(gql_errors).to be_nil expect(gql_data[:demarche][:id]).to eq(procedure.to_typed_id) - expect(gql_data[:demarche][:deletedDossiers][:nodes].size).to eq(1) + expect(gql_data[:demarche][:deletedDossiers][:nodes].size).to eq(4) expect(gql_data[:demarche][:deletedDossiers][:nodes].first[:id]).to eq(deleted_dossier.to_typed_id) expect(gql_data[:demarche][:deletedDossiers][:nodes].first[:dateSupression]).to eq(deleted_dossier.deleted_at.iso8601) } + + context 'first' do + let(:variables) { { demarcheNumber: procedure.id, includeDeletedDossiers: true, deletedFirst: 2 } } + let(:end_cursor) { cursor_for(deleted_dossier1) } + + it { + expect(gql_errors).to be_nil + expect(gql_data[:demarche][:deletedDossiers][:nodes].size).to eq(2) + expect(gql_data[:demarche][:deletedDossiers][:pageInfo][:hasNextPage]).to be_truthy + expect(gql_data[:demarche][:deletedDossiers][:pageInfo][:hasPreviousPage]).to be_falsey + expect(gql_data[:demarche][:deletedDossiers][:pageInfo][:startCursor]).to eq(start_cursor) + expect(gql_data[:demarche][:deletedDossiers][:pageInfo][:endCursor]).to eq(end_cursor) + } + + context 'after' do + let(:variables) { { demarcheNumber: procedure.id, includeDeletedDossiers: true, deletedFirst: 2, deletedAfter: current_cursor } } + let(:current_cursor) { cursor_for(deleted_dossier1) } + let(:start_cursor) { cursor_for(deleted_dossier2) } + let(:end_cursor) { cursor_for(deleted_dossier3) } + + it { + expect(gql_errors).to be_nil + expect(gql_data[:demarche][:deletedDossiers][:nodes].size).to eq(2) + expect(gql_data[:demarche][:deletedDossiers][:pageInfo][:hasNextPage]).to be_falsey + expect(gql_data[:demarche][:deletedDossiers][:pageInfo][:hasPreviousPage]).to be_truthy + expect(gql_data[:demarche][:deletedDossiers][:pageInfo][:startCursor]).to eq(start_cursor) + expect(gql_data[:demarche][:deletedDossiers][:pageInfo][:endCursor]).to eq(end_cursor) + } + end + + context 'before' do + let(:variables) { { demarcheNumber: procedure.id, includeDeletedDossiers: true, deletedFirst: 2, deletedBefore: current_cursor } } + let(:current_cursor) { cursor_for(deleted_dossier2) } + let(:start_cursor) { cursor_for(deleted_dossier) } + let(:end_cursor) { cursor_for(deleted_dossier1) } + + it { + expect(gql_errors).to be_nil + expect(gql_data[:demarche][:deletedDossiers][:nodes].size).to eq(2) + expect(gql_data[:demarche][:deletedDossiers][:pageInfo][:hasNextPage]).to be_truthy + expect(gql_data[:demarche][:deletedDossiers][:pageInfo][:hasPreviousPage]).to be_falsey + expect(gql_data[:demarche][:deletedDossiers][:pageInfo][:startCursor]).to eq(start_cursor) + expect(gql_data[:demarche][:deletedDossiers][:pageInfo][:endCursor]).to eq(end_cursor) + } + end + end + + context 'last' do + let(:variables) { { demarcheNumber: procedure.id, includeDeletedDossiers: true, deletedLast: 2 } } + let(:start_cursor) { cursor_for(deleted_dossier2) } + let(:end_cursor) { cursor_for(deleted_dossier3) } + + it { + expect(gql_errors).to be_nil + expect(gql_data[:demarche][:deletedDossiers][:nodes].size).to eq(2) + expect(gql_data[:demarche][:deletedDossiers][:pageInfo][:hasNextPage]).to be_falsey + expect(gql_data[:demarche][:deletedDossiers][:pageInfo][:hasPreviousPage]).to be_truthy + expect(gql_data[:demarche][:deletedDossiers][:pageInfo][:startCursor]).to eq(start_cursor) + expect(gql_data[:demarche][:deletedDossiers][:pageInfo][:endCursor]).to eq(end_cursor) + } + + context 'before' do + let(:variables) { { demarcheNumber: procedure.id, includeDeletedDossiers: true, deletedLast: 2, deletedBefore: current_cursor } } + let(:current_cursor) { cursor_for(deleted_dossier2) } + let(:start_cursor) { cursor_for(deleted_dossier) } + let(:end_cursor) { cursor_for(deleted_dossier1) } + + it { + expect(gql_errors).to be_nil + expect(gql_data[:demarche][:deletedDossiers][:nodes].size).to eq(2) + expect(gql_data[:demarche][:deletedDossiers][:pageInfo][:hasNextPage]).to be_truthy + expect(gql_data[:demarche][:deletedDossiers][:pageInfo][:hasPreviousPage]).to be_falsey + expect(gql_data[:demarche][:deletedDossiers][:pageInfo][:startCursor]).to eq(start_cursor) + expect(gql_data[:demarche][:deletedDossiers][:pageInfo][:endCursor]).to eq(end_cursor) + } + end + + context 'after' do + let(:variables) { { demarcheNumber: procedure.id, includeDeletedDossiers: true, deletedLast: 2, deletedAfter: current_cursor } } + let(:current_cursor) { cursor_for(deleted_dossier1) } + let(:start_cursor) { cursor_for(deleted_dossier2) } + let(:end_cursor) { cursor_for(deleted_dossier3) } + + it { + expect(gql_errors).to be_nil + expect(gql_data[:demarche][:deletedDossiers][:nodes].size).to eq(2) + expect(gql_data[:demarche][:deletedDossiers][:pageInfo][:hasNextPage]).to be_falsey + expect(gql_data[:demarche][:deletedDossiers][:pageInfo][:hasPreviousPage]).to be_truthy + expect(gql_data[:demarche][:deletedDossiers][:pageInfo][:startCursor]).to eq(start_cursor) + expect(gql_data[:demarche][:deletedDossiers][:pageInfo][:endCursor]).to eq(end_cursor) + } + end + end end context 'include pending deleted Dossiers' do + def cursor_for(item) + cursor = [(item.en_construction? ? item.hidden_by_user_at : item.hidden_by_administration_at).utc.strftime("%Y-%m-%dT%H:%M:%S.%NZ"), item.id].join(';') + API::V2::Schema.cursor_encoder.encode(cursor, nonce: true) + end + let(:variables) { { demarcheNumber: procedure.id, includePendingDeletedDossiers: true, pendingDeletedSince: 2.weeks.ago.iso8601 } } - before { + let(:pending_deleted_dossier) do dossier.hide_and_keep_track!(dossier.user, DeletedDossier.reasons.fetch(:user_request)) - Timecop.travel(3.hours.ago) { - dossier_accepte.hide_and_keep_track!(instructeur, DeletedDossier.reasons.fetch(:instructeur_request)) - } - } + dossier.tap { _1.update(hidden_by_user_at: 4.days.ago) } + end + let(:pending_deleted_dossier1) do + dossier_accepte.hide_and_keep_track!(instructeur, DeletedDossier.reasons.fetch(:instructeur_request)) + dossier_accepte.tap { _1.update(hidden_by_administration_at: 3.days.ago) } + end + let(:pending_deleted_dossier2) do + dossier1.hide_and_keep_track!(dossier.user, DeletedDossier.reasons.fetch(:user_request)) + dossier1.tap { _1.update(hidden_by_user_at: 2.days.ago) } + end + let(:pending_deleted_dossier3) do + dossier_accepte1.hide_and_keep_track!(instructeur, DeletedDossier.reasons.fetch(:instructeur_request)) + dossier_accepte1.tap { _1.update(hidden_by_administration_at: 1.day.ago) } + end + + let(:start_cursor) { cursor_for(pending_deleted_dossier) } + let(:end_cursor) { cursor_for(pending_deleted_dossier3) } + + before { pending_deleted_dossier; pending_deleted_dossier1; pending_deleted_dossier2; pending_deleted_dossier3 } it { expect(gql_errors).to be_nil expect(gql_data[:demarche][:id]).to eq(procedure.to_typed_id) - expect(gql_data[:demarche][:pendingDeletedDossiers][:nodes].size).to eq(2) - expect(gql_data[:demarche][:pendingDeletedDossiers][:nodes].first[:id]).to eq(GraphQL::Schema::UniqueWithinType.encode('DeletedDossier', dossier_accepte.id)) - expect(gql_data[:demarche][:pendingDeletedDossiers][:nodes].second[:id]).to eq(GraphQL::Schema::UniqueWithinType.encode('DeletedDossier', dossier.id)) - expect(gql_data[:demarche][:pendingDeletedDossiers][:nodes].first[:dateSupression]).to eq(dossier_accepte.hidden_by_administration_at.iso8601) - expect(gql_data[:demarche][:pendingDeletedDossiers][:nodes].second[:dateSupression]).to eq(dossier.hidden_by_user_at.iso8601) - expect(gql_data[:demarche][:pendingDeletedDossiers][:nodes].first[:dateSupression] < gql_data[:demarche][:pendingDeletedDossiers][:nodes].second[:dateSupression]) + expect(gql_data[:demarche][:pendingDeletedDossiers][:nodes].size).to eq(4) + expect(gql_data[:demarche][:pendingDeletedDossiers][:nodes].first[:id]).to eq(GraphQL::Schema::UniqueWithinType.encode('DeletedDossier', dossier.id)) + expect(gql_data[:demarche][:pendingDeletedDossiers][:nodes].second[:id]).to eq(GraphQL::Schema::UniqueWithinType.encode('DeletedDossier', dossier_accepte.id)) + expect(gql_data[:demarche][:pendingDeletedDossiers][:nodes].first[:dateSupression]).to eq(pending_deleted_dossier.hidden_by_user_at.iso8601) + expect(gql_data[:demarche][:pendingDeletedDossiers][:nodes].second[:dateSupression]).to eq(pending_deleted_dossier1.hidden_by_administration_at.iso8601) + expect(gql_data[:demarche][:pendingDeletedDossiers][:nodes].first[:dateSupression] < gql_data[:demarche][:pendingDeletedDossiers][:nodes].second[:dateSupression]).to be_truthy } + + context 'first' do + let(:variables) { { demarcheNumber: procedure.id, includePendingDeletedDossiers: true, pendingDeletedFirst: 2 } } + let(:end_cursor) { cursor_for(pending_deleted_dossier1) } + + it { + expect(gql_errors).to be_nil + expect(gql_data[:demarche][:pendingDeletedDossiers][:nodes].size).to eq(2) + expect(gql_data[:demarche][:pendingDeletedDossiers][:pageInfo][:hasNextPage]).to be_truthy + expect(gql_data[:demarche][:pendingDeletedDossiers][:pageInfo][:hasPreviousPage]).to be_falsey + expect(gql_data[:demarche][:pendingDeletedDossiers][:pageInfo][:startCursor]).to eq(start_cursor) + expect(gql_data[:demarche][:pendingDeletedDossiers][:pageInfo][:endCursor]).to eq(end_cursor) + } + + context 'after' do + let(:variables) { { demarcheNumber: procedure.id, includePendingDeletedDossiers: true, pendingDeletedFirst: 2, pendingDeletedAfter: current_cursor } } + let(:current_cursor) { cursor_for(pending_deleted_dossier1) } + let(:start_cursor) { cursor_for(pending_deleted_dossier2) } + let(:end_cursor) { cursor_for(pending_deleted_dossier3) } + + it { + expect(gql_errors).to be_nil + expect(gql_data[:demarche][:pendingDeletedDossiers][:nodes].size).to eq(2) + expect(gql_data[:demarche][:pendingDeletedDossiers][:pageInfo][:hasNextPage]).to be_falsey + expect(gql_data[:demarche][:pendingDeletedDossiers][:pageInfo][:hasPreviousPage]).to be_truthy + expect(gql_data[:demarche][:pendingDeletedDossiers][:pageInfo][:startCursor]).to eq(start_cursor) + expect(gql_data[:demarche][:pendingDeletedDossiers][:pageInfo][:endCursor]).to eq(end_cursor) + } + end + + context 'before' do + let(:variables) { { demarcheNumber: procedure.id, includePendingDeletedDossiers: true, pendingDeletedFirst: 2, pendingDeletedBefore: current_cursor } } + let(:current_cursor) { cursor_for(pending_deleted_dossier2) } + let(:start_cursor) { cursor_for(pending_deleted_dossier) } + let(:end_cursor) { cursor_for(pending_deleted_dossier1) } + + it { + expect(gql_errors).to be_nil + expect(gql_data[:demarche][:pendingDeletedDossiers][:nodes].size).to eq(2) + expect(gql_data[:demarche][:pendingDeletedDossiers][:pageInfo][:hasNextPage]).to be_truthy + expect(gql_data[:demarche][:pendingDeletedDossiers][:pageInfo][:hasPreviousPage]).to be_falsey + expect(gql_data[:demarche][:pendingDeletedDossiers][:pageInfo][:startCursor]).to eq(start_cursor) + expect(gql_data[:demarche][:pendingDeletedDossiers][:pageInfo][:endCursor]).to eq(end_cursor) + } + end + end + + context 'last' do + let(:variables) { { demarcheNumber: procedure.id, includePendingDeletedDossiers: true, pendingDeletedLast: 2 } } + let(:start_cursor) { cursor_for(pending_deleted_dossier2) } + let(:end_cursor) { cursor_for(pending_deleted_dossier3) } + + it { + expect(gql_errors).to be_nil + expect(gql_data[:demarche][:pendingDeletedDossiers][:nodes].size).to eq(2) + expect(gql_data[:demarche][:pendingDeletedDossiers][:pageInfo][:hasNextPage]).to be_falsey + expect(gql_data[:demarche][:pendingDeletedDossiers][:pageInfo][:hasPreviousPage]).to be_truthy + expect(gql_data[:demarche][:pendingDeletedDossiers][:pageInfo][:startCursor]).to eq(start_cursor) + expect(gql_data[:demarche][:pendingDeletedDossiers][:pageInfo][:endCursor]).to eq(end_cursor) + } + + context 'before' do + let(:variables) { { demarcheNumber: procedure.id, includePendingDeletedDossiers: true, pendingDeletedLast: 2, pendingDeletedBefore: current_cursor } } + let(:current_cursor) { cursor_for(pending_deleted_dossier2) } + let(:start_cursor) { cursor_for(pending_deleted_dossier) } + let(:end_cursor) { cursor_for(pending_deleted_dossier1) } + + it { + expect(gql_errors).to be_nil + expect(gql_data[:demarche][:pendingDeletedDossiers][:nodes].size).to eq(2) + expect(gql_data[:demarche][:pendingDeletedDossiers][:pageInfo][:hasNextPage]).to be_truthy + expect(gql_data[:demarche][:pendingDeletedDossiers][:pageInfo][:hasPreviousPage]).to be_falsey + expect(gql_data[:demarche][:pendingDeletedDossiers][:pageInfo][:startCursor]).to eq(start_cursor) + expect(gql_data[:demarche][:pendingDeletedDossiers][:pageInfo][:endCursor]).to eq(end_cursor) + } + end + + context 'after' do + let(:variables) { { demarcheNumber: procedure.id, includePendingDeletedDossiers: true, pendingDeletedLast: 2, pendingDeletedAfter: current_cursor } } + let(:current_cursor) { cursor_for(pending_deleted_dossier1) } + let(:start_cursor) { cursor_for(pending_deleted_dossier2) } + let(:end_cursor) { cursor_for(pending_deleted_dossier3) } + + it { + expect(gql_errors).to be_nil + expect(gql_data[:demarche][:pendingDeletedDossiers][:nodes].size).to eq(2) + expect(gql_data[:demarche][:pendingDeletedDossiers][:pageInfo][:hasNextPage]).to be_falsey + expect(gql_data[:demarche][:pendingDeletedDossiers][:pageInfo][:hasPreviousPage]).to be_truthy + expect(gql_data[:demarche][:pendingDeletedDossiers][:pageInfo][:startCursor]).to eq(start_cursor) + expect(gql_data[:demarche][:pendingDeletedDossiers][:pageInfo][:endCursor]).to eq(end_cursor) + } + end + end end end From 82322e8874655aa9fd9193875c7d5498f40a2be2 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Mon, 28 Aug 2023 12:16:24 +0200 Subject: [PATCH 02/14] =?UTF-8?q?chore(breakman):=20ignore=20injection=20w?= =?UTF-8?q?arnings=20=E2=80=93=20table=20and=20column=20names=20com=20from?= =?UTF-8?q?=20our=20code=20not=20user=20input?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- config/brakeman.ignore | 89 ++++++++++++++++++++++++------------------ 1 file changed, 52 insertions(+), 37 deletions(-) diff --git a/config/brakeman.ignore b/config/brakeman.ignore index 52d48e3db..12bece0c1 100644 --- a/config/brakeman.ignore +++ b/config/brakeman.ignore @@ -15,7 +15,7 @@ "type": "controller", "class": "Users::DossiersController", "method": "merci", - "line": 232, + "line": 291, "file": "app/controllers/users/dossiers_controller.rb", "rendered": { "name": "users/dossiers/merci", @@ -39,6 +39,9 @@ }, "user_input": "current_user.dossiers.includes(:procedure)", "confidence": "Weak", + "cwe_id": [ + 79 + ], "note": "" }, { @@ -70,26 +73,55 @@ }, "user_input": "FranceConnectInformation.find_by(:merge_token => merge_token_params).email_france_connect", "confidence": "Weak", + "cwe_id": [ + 79 + ], "note": "explicitely sanitized even if we are using html_safe" }, { - "warning_type": "Redirect", - "warning_code": 18, - "fingerprint": "8a1ccc92988486094b2c89e586902a3b6fcbd43910d6363dce14b9981ca8ddeb", - "check_name": "Redirect", - "message": "Possible unprotected redirect", - "file": "app/controllers/instructeurs/procedures_controller.rb", - "line": 175, - "link": "https://brakemanscanner.org/docs/warning_types/redirect/", - "code": "redirect_to(Export.find_or_create_export(export_format, current_instructeur.groupe_instructeurs.where(:procedure => procedure), :force => force_export?, **export_options).file.service_url)", + "warning_type": "SQL Injection", + "warning_code": 0, + "fingerprint": "737aa4f7931ece068cce98d7cc66057a1ec81b9be43e469c3569ff1be91bbf09", + "check_name": "SQL", + "message": "Possible SQL injection", + "file": "app/graphql/connections/cursor_connection.rb", + "line": 66, + "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", + "code": "items.order(order_column => ((:desc or :asc)), :id => ((:desc or :asc))).limit(limit).where(\"(#{order_table}.#{order_column}, #{order_table}.id) < (?, ?)\", timestamp, id)", "render_path": null, "location": { "type": "method", - "class": "Instructeurs::ProceduresController", - "method": "download_export" + "class": "Connections::CursorConnection", + "method": "resolve_nodes" }, - "user_input": "Export.find_or_create_export(export_format, current_instructeur.groupe_instructeurs.where(:procedure => procedure), :force => force_export?, **export_options).file.service_url", - "confidence": "High", + "user_input": "order_table", + "confidence": "Weak", + "cwe_id": [ + 89 + ], + "note": "" + }, + { + "warning_type": "SQL Injection", + "warning_code": 0, + "fingerprint": "a94939cb1e551341f443c6414634816e335bbfb03f0836ebd8b3ad8564d7f343", + "check_name": "SQL", + "message": "Possible SQL injection", + "file": "app/graphql/connections/cursor_connection.rb", + "line": 69, + "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", + "code": "items.order(order_column => ((:desc or :asc)), :id => ((:desc or :asc))).limit(limit).where(\"(#{order_table}.#{order_column}, #{order_table}.id) > (?, ?)\", timestamp, id)", + "render_path": null, + "location": { + "type": "method", + "class": "Connections::CursorConnection", + "method": "resolve_nodes" + }, + "user_input": "order_table", + "confidence": "Weak", + "cwe_id": [ + 89 + ], "note": "" }, { @@ -99,7 +131,7 @@ "check_name": "SQL", "message": "Possible SQL injection", "file": "app/models/concerns/dossier_filtering_concern.rb", - "line": 30, + "line": 32, "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", "code": "where(\"#{values.count} OR #{\"(#{ProcedurePresentation.sanitized_column(table, column)} ILIKE ?)\"}\", *values.map do\n \"%#{value}%\"\n end)", "render_path": null, @@ -110,29 +142,12 @@ }, "user_input": "values.count", "confidence": "Medium", + "cwe_id": [ + 89 + ], "note": "The table and column are escaped, which should make this safe" - }, - { - "warning_type": "Redirect", - "warning_code": 18, - "fingerprint": "e2220b7cda7df5d02de77e7c3ce137653126e0d8e91ce445676b63ec4c94bbcb", - "check_name": "Redirect", - "message": "Possible unprotected redirect", - "file": "app/controllers/administrateurs/exports_controller.rb", - "line": 18, - "link": "https://brakemanscanner.org/docs/warning_types/redirect/", - "code": "redirect_to(Export.find_or_create_export(export_format, all_groupe_instructeurs, :force => force_export?, **export_options).file.service_url)", - "render_path": null, - "location": { - "type": "method", - "class": "Administrateurs::ExportsController", - "method": "download" - }, - "user_input": "Export.find_or_create_export(export_format, all_groupe_instructeurs, :force => force_export?, **export_options).file.service_url", - "confidence": "High", - "note": "" } ], - "updated": "2022-11-15 23:04:53 +0100", - "brakeman_version": "5.2.2" + "updated": "2023-08-28 12:16:04 +0200", + "brakeman_version": "5.4.1" } From 0dd68ab73d1a9e79353c5da8f2eba4f2314753c1 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 17 Oct 2023 16:34:08 +0200 Subject: [PATCH 03/14] add comments --- app/graphql/connections/cursor_connection.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/graphql/connections/cursor_connection.rb b/app/graphql/connections/cursor_connection.rb index 3f2b53994..5235e7786 100644 --- a/app/graphql/connections/cursor_connection.rb +++ b/app/graphql/connections/cursor_connection.rb @@ -72,6 +72,9 @@ module Connections end end + # before and after are a serialized version of (timestamp, id) + # first is a number (n) and mean take n element in order ascendant + # last : n element in order descendant def compute_page_info(before: nil, after: nil, first: nil, last: nil) if first.present? && last.present? raise GraphQL::ExecutionError.new('Arguments "first" and "last" are exclusive', extensions: { code: :bad_request }) From 311c9215cd4481b6c86ed4225a7148ea6f3813d7 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 17 Oct 2023 16:35:24 +0200 Subject: [PATCH 04/14] extract ensure_valid_params --- app/graphql/connections/cursor_connection.rb | 35 +++++++++++--------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/app/graphql/connections/cursor_connection.rb b/app/graphql/connections/cursor_connection.rb index 5235e7786..8d07b5534 100644 --- a/app/graphql/connections/cursor_connection.rb +++ b/app/graphql/connections/cursor_connection.rb @@ -27,6 +27,7 @@ module Connections def load_nodes @nodes ||= begin + ensure_valid_params page_info = compute_page_info(before:, after:, first:, last:) nodes = resolve_nodes(**page_info.slice(:before, :after, :limit, :inverted)) result_size = nodes.size @@ -39,6 +40,24 @@ module Connections end end + def ensure_valid_params + if first.present? && last.present? + raise GraphQL::ExecutionError.new('Arguments "first" and "last" are exclusive', extensions: { code: :bad_request }) + end + + if before.present? && after.present? + raise GraphQL::ExecutionError.new('Arguments "before" and "after" are exclusive', extensions: { code: :bad_request }) + end + + if first.present? && first < 0 + raise GraphQL::ExecutionError.new('Argument "first" must be a non-negative integer', extensions: { code: :bad_request }) + end + + if last.present? && last < 0 + raise GraphQL::ExecutionError.new('Argument "last" must be a non-negative integer', extensions: { code: :bad_request }) + end + end + def timestamp_and_id_from_cursor(cursor) timestamp, id = decode(cursor).split(';') [Time.zone.parse(timestamp), id.to_i] @@ -76,22 +95,6 @@ module Connections # first is a number (n) and mean take n element in order ascendant # last : n element in order descendant def compute_page_info(before: nil, after: nil, first: nil, last: nil) - if first.present? && last.present? - raise GraphQL::ExecutionError.new('Arguments "first" and "last" are exclusive', extensions: { code: :bad_request }) - end - - if before.present? && after.present? - raise GraphQL::ExecutionError.new('Arguments "before" and "after" are exclusive', extensions: { code: :bad_request }) - end - - if first.present? && first < 0 - raise GraphQL::ExecutionError.new('Argument "first" must be a non-negative integer', extensions: { code: :bad_request }) - end - - if last.present? && last < 0 - raise GraphQL::ExecutionError.new('Argument "last" must be a non-negative integer', extensions: { code: :bad_request }) - end - if @deprecated_order == :desc if last.present? first = [last, max_page_size].min From c5899f2c4625cfe66503fae95ff4774d7e1989ff Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 17 Oct 2023 17:29:43 +0200 Subject: [PATCH 05/14] extract limit and expected_size --- app/graphql/connections/cursor_connection.rb | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/app/graphql/connections/cursor_connection.rb b/app/graphql/connections/cursor_connection.rb index 8d07b5534..a388d2e74 100644 --- a/app/graphql/connections/cursor_connection.rb +++ b/app/graphql/connections/cursor_connection.rb @@ -28,13 +28,18 @@ module Connections def load_nodes @nodes ||= begin ensure_valid_params - page_info = compute_page_info(before:, after:, first:, last:) - nodes = resolve_nodes(**page_info.slice(:before, :after, :limit, :inverted)) + + limit = compute_limit(first:, last:) + expected_size = limit - 1 + + page_info = compute_page_info(limit:, before:, after:, first:, last:) + nodes = resolve_nodes(limit:, **page_info.slice(:before, :after, :inverted)) + result_size = nodes.size @has_previous_page = page_info[:has_previous_page].(result_size) @has_next_page = page_info[:has_next_page].(result_size) - trimmed_nodes = nodes.first(page_info[:expected_size]) + trimmed_nodes = nodes.first(expected_size) trimmed_nodes.reverse! if page_info[:inverted] trimmed_nodes end @@ -58,6 +63,10 @@ module Connections end end + def compute_limit(first: nil, last: nil) + [first || last || default_page_size, max_page_size].min + 1 + end + def timestamp_and_id_from_cursor(cursor) timestamp, id = decode(cursor).split(';') [Time.zone.parse(timestamp), id.to_i] @@ -94,7 +103,7 @@ module Connections # before and after are a serialized version of (timestamp, id) # first is a number (n) and mean take n element in order ascendant # last : n element in order descendant - def compute_page_info(before: nil, after: nil, first: nil, last: nil) + def compute_page_info(limit:, before: nil, after: nil, first: nil, last: nil) if @deprecated_order == :desc if last.present? first = [last, max_page_size].min @@ -105,15 +114,12 @@ module Connections end end - limit = [first || last || default_page_size, max_page_size].min + 1 inverted = last.present? || before.present? { before:, after:, - limit:, inverted:, - expected_size: limit - 1, has_previous_page: -> (result_size) { after.present? || (result_size >= limit && inverted) }, has_next_page: -> (result_size) { before.present? || (result_size >= limit && !inverted) } } From dbe67aaf185c6283d7c041b1fac5ec3004c0bb11 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 17 Oct 2023 17:36:00 +0200 Subject: [PATCH 06/14] default_page <= max_page_size --- app/graphql/connections/cursor_connection.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/graphql/connections/cursor_connection.rb b/app/graphql/connections/cursor_connection.rb index a388d2e74..2d4a24283 100644 --- a/app/graphql/connections/cursor_connection.rb +++ b/app/graphql/connections/cursor_connection.rb @@ -64,7 +64,7 @@ module Connections end def compute_limit(first: nil, last: nil) - [first || last || default_page_size, max_page_size].min + 1 + [first || last || default_page_size].min + 1 end def timestamp_and_id_from_cursor(cursor) @@ -109,7 +109,7 @@ module Connections first = [last, max_page_size].min last = nil else - last = [first || default_page_size, max_page_size].min + last = [first || default_page_size].min first = nil end end From 1bb4d3cf174b73f575cee5b830447a6a1a38a181 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 17 Oct 2023 17:39:28 +0200 Subject: [PATCH 07/14] remove useless assignment --- app/graphql/connections/cursor_connection.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/graphql/connections/cursor_connection.rb b/app/graphql/connections/cursor_connection.rb index 2d4a24283..f71a11bf3 100644 --- a/app/graphql/connections/cursor_connection.rb +++ b/app/graphql/connections/cursor_connection.rb @@ -106,11 +106,9 @@ module Connections def compute_page_info(limit:, before: nil, after: nil, first: nil, last: nil) if @deprecated_order == :desc if last.present? - first = [last, max_page_size].min last = nil else last = [first || default_page_size].min - first = nil end end From ce5cd63ed2045b02c0a4987d4babce651c8c6164 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 17 Oct 2023 21:06:30 +0200 Subject: [PATCH 08/14] comment --- app/graphql/connections/cursor_connection.rb | 26 ++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/app/graphql/connections/cursor_connection.rb b/app/graphql/connections/cursor_connection.rb index f71a11bf3..6177bff2e 100644 --- a/app/graphql/connections/cursor_connection.rb +++ b/app/graphql/connections/cursor_connection.rb @@ -25,6 +25,32 @@ module Connections private + # [d1, d2, d3, d4, d5, d6] + # + # first: 2 + # -> d1, d2 + # first: 2, after: d2 + # -> d3, d4 + # first: 2, before: d3 + # -> d1, d2 + # + # last: 2 + # -> d5, d6 + # last: 2, before: d5 + # -> d3, d4 + # last: 2, after: d4 + # -> d5, d6 + # + # si after ou before present, last ou first donne juste limit + # + # order: + # order, ne sert rien si after ou before + # + # first: 2, order: desc => last: 2 + # -> d5, d6 + # + # last: 2, order: desc => first 2 + # -> d1, d2 def load_nodes @nodes ||= begin ensure_valid_params From 17aab9c891fbd7b968cd2ef1f3f8df275e747ed3 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 17 Oct 2023 23:24:01 +0200 Subject: [PATCH 09/14] add limit_and_inverted method --- app/graphql/connections/cursor_connection.rb | 11 ++++ .../connections/cursor_connection_spec.rb | 60 +++++++++++++++++++ 2 files changed, 71 insertions(+) create mode 100644 spec/graphql/connections/cursor_connection_spec.rb diff --git a/app/graphql/connections/cursor_connection.rb b/app/graphql/connections/cursor_connection.rb index 6177bff2e..34db4fb56 100644 --- a/app/graphql/connections/cursor_connection.rb +++ b/app/graphql/connections/cursor_connection.rb @@ -51,6 +51,17 @@ module Connections # # last: 2, order: desc => first 2 # -> d1, d2 + def limit_and_inverted(first: nil, last: nil, after: nil, before: nil, order: nil) + limit = [first, last, max_page_size].compact.min + 1 + inverted = last.present? || before.present? + + if order == :desc && after.nil? && before.nil? + inverted = !inverted + end + + [limit, inverted] + end + def load_nodes @nodes ||= begin ensure_valid_params diff --git a/spec/graphql/connections/cursor_connection_spec.rb b/spec/graphql/connections/cursor_connection_spec.rb new file mode 100644 index 000000000..57ef64b89 --- /dev/null +++ b/spec/graphql/connections/cursor_connection_spec.rb @@ -0,0 +1,60 @@ +RSpec.describe Connections::CursorConnection do + describe '.limit_and_inverted' do + let(:max_page_size) { 100 } + + subject do + cursor = Connections::CursorConnection.new(Dossier) + allow(cursor).to receive(:max_page_size).and_return(max_page_size) + limit, inverted = cursor.send(:limit_and_inverted, **args) + { limit:, inverted: } + end + + context 'without explicit args' do + let(:args) { { } } + + it { is_expected.to eq(limit: max_page_size + 1, inverted: false) } + end + + context 'when asked for 2 first elements' do + let(:args) { { first: 2 } } + + it { is_expected.to eq(limit: 3, inverted: false) } + end + + context 'when asked for 2 first elements, in order desc' do + let(:args) { { first: 2, order: :desc} } + + it { is_expected.to eq(limit: 3, inverted: true) } + end + + context 'when exceeding the max_page_size' do + let(:args) { { first: max_page_size + 1 } } + + it { is_expected.to eq(limit: max_page_size + 1, inverted: false) } + end + + context 'when asked for 2 last elements' do + let(:args) { { last: 2 } } + + it { is_expected.to eq(limit: 3, inverted: true) } + end + + context 'when asked for 2 last elements, in order desc' do + let(:args) { { last: 2, order: :desc} } + + it { is_expected.to eq(limit: 3, inverted: false) } + end + + context '' do + let(:args) { { after: :after, first: 2 } } + + it { is_expected.to eq(limit: 3, inverted: false) } + end + + context '' do + let(:args) { { before: :before, first: 2 } } + + it { is_expected.to eq(limit: 3, inverted: true) } + end + end +end From 137c19cce845745ebfd8479ebf4e5829bfb81650 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 17 Oct 2023 23:37:01 +0200 Subject: [PATCH 10/14] add previous_page? --- app/graphql/connections/cursor_connection.rb | 4 +++ .../connections/cursor_connection_spec.rb | 26 +++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/app/graphql/connections/cursor_connection.rb b/app/graphql/connections/cursor_connection.rb index 34db4fb56..dc9699530 100644 --- a/app/graphql/connections/cursor_connection.rb +++ b/app/graphql/connections/cursor_connection.rb @@ -62,6 +62,10 @@ module Connections [limit, inverted] end + def previous_page?(after, result_size, limit, inverted) + after.present? || (result_size == limit && inverted) + end + def load_nodes @nodes ||= begin ensure_valid_params diff --git a/spec/graphql/connections/cursor_connection_spec.rb b/spec/graphql/connections/cursor_connection_spec.rb index 57ef64b89..2410e7401 100644 --- a/spec/graphql/connections/cursor_connection_spec.rb +++ b/spec/graphql/connections/cursor_connection_spec.rb @@ -57,4 +57,30 @@ RSpec.describe Connections::CursorConnection do it { is_expected.to eq(limit: 3, inverted: true) } end end + + describe '.previous_page?' do + let(:after) { nil } + let(:result_size) { nil } + let(:limit) { nil } + let(:inverted) { false } + + subject do + cursor = Connections::CursorConnection.new(Dossier) + cursor.send(:previous_page?, after, result_size, limit, inverted) + end + + context 'when after is present' do + let(:after) { :after } + + it { is_expected.to be true } + end + + context 'when inverted and result_size == limit' do + let(:inverted) { true } + let(:result_size) { 3 } + let(:limit) { 3 } + + it { is_expected.to be true } + end + end end From b37f2c68254e34af6f5a46eda23c54a293337612 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 17 Oct 2023 23:42:20 +0200 Subject: [PATCH 11/14] add next_page? --- app/graphql/connections/cursor_connection.rb | 4 +++ .../connections/cursor_connection_spec.rb | 27 +++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/app/graphql/connections/cursor_connection.rb b/app/graphql/connections/cursor_connection.rb index dc9699530..0a3556231 100644 --- a/app/graphql/connections/cursor_connection.rb +++ b/app/graphql/connections/cursor_connection.rb @@ -66,6 +66,10 @@ module Connections after.present? || (result_size == limit && inverted) end + def next_page?(before, result_size, limit, inverted) + before.present? || (result_size == limit && !inverted) + end + def load_nodes @nodes ||= begin ensure_valid_params diff --git a/spec/graphql/connections/cursor_connection_spec.rb b/spec/graphql/connections/cursor_connection_spec.rb index 2410e7401..925266eab 100644 --- a/spec/graphql/connections/cursor_connection_spec.rb +++ b/spec/graphql/connections/cursor_connection_spec.rb @@ -83,4 +83,31 @@ RSpec.describe Connections::CursorConnection do it { is_expected.to be true } end end + + describe '.next_page?' do + let(:before) { nil } + let(:result_size) { nil } + let(:limit) { nil } + let(:inverted) { false } + + subject do + cursor = Connections::CursorConnection.new(Dossier) + cursor.send(:next_page?, before, result_size, limit, inverted) + end + + context 'when before is present' do + let(:before) { :before } + + it { is_expected.to be true } + end + + context 'when not inverted and result_size == limit' do + let(:inverted) { false } + let(:result_size) { 3 } + let(:limit) { 3 } + + it { is_expected.to be true } + end + end + end From 6a02e670aae62e4fd410ce66c905418d25d03689 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 17 Oct 2023 23:48:28 +0200 Subject: [PATCH 12/14] use new methods --- app/graphql/connections/cursor_connection.rb | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/app/graphql/connections/cursor_connection.rb b/app/graphql/connections/cursor_connection.rb index 0a3556231..1c232777d 100644 --- a/app/graphql/connections/cursor_connection.rb +++ b/app/graphql/connections/cursor_connection.rb @@ -74,18 +74,17 @@ module Connections @nodes ||= begin ensure_valid_params - limit = compute_limit(first:, last:) + limit, inverted = limit_and_inverted(first:, last:, after:, before:, order: @deprecated_order) expected_size = limit - 1 - page_info = compute_page_info(limit:, before:, after:, first:, last:) - nodes = resolve_nodes(limit:, **page_info.slice(:before, :after, :inverted)) + nodes = resolve_nodes(limit:, before:, after:, inverted:) result_size = nodes.size - @has_previous_page = page_info[:has_previous_page].(result_size) - @has_next_page = page_info[:has_next_page].(result_size) + @has_previous_page = previous_page?(after, result_size, limit, inverted) + @has_next_page = next_page?(before, result_size, limit, inverted) trimmed_nodes = nodes.first(expected_size) - trimmed_nodes.reverse! if page_info[:inverted] + trimmed_nodes.reverse! if inverted trimmed_nodes end end From 071167da96021db991aa625c49ff61b0765a5796 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 17 Oct 2023 23:52:22 +0200 Subject: [PATCH 13/14] remove methods --- app/graphql/connections/cursor_connection.rb | 27 -------------------- 1 file changed, 27 deletions(-) diff --git a/app/graphql/connections/cursor_connection.rb b/app/graphql/connections/cursor_connection.rb index 1c232777d..e9471a6f3 100644 --- a/app/graphql/connections/cursor_connection.rb +++ b/app/graphql/connections/cursor_connection.rb @@ -107,10 +107,6 @@ module Connections end end - def compute_limit(first: nil, last: nil) - [first || last || default_page_size].min + 1 - end - def timestamp_and_id_from_cursor(cursor) timestamp, id = decode(cursor).split(';') [Time.zone.parse(timestamp), id.to_i] @@ -143,28 +139,5 @@ module Connections nodes end end - - # before and after are a serialized version of (timestamp, id) - # first is a number (n) and mean take n element in order ascendant - # last : n element in order descendant - def compute_page_info(limit:, before: nil, after: nil, first: nil, last: nil) - if @deprecated_order == :desc - if last.present? - last = nil - else - last = [first || default_page_size].min - end - end - - inverted = last.present? || before.present? - - { - before:, - after:, - inverted:, - has_previous_page: -> (result_size) { after.present? || (result_size >= limit && inverted) }, - has_next_page: -> (result_size) { before.present? || (result_size >= limit && !inverted) } - } - end end end From a43176b2d6b4d36a7369f5f7ce4c3d30f6bf907c Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Wed, 18 Oct 2023 13:02:59 +0200 Subject: [PATCH 14/14] fix lint --- spec/graphql/connections/cursor_connection_spec.rb | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/spec/graphql/connections/cursor_connection_spec.rb b/spec/graphql/connections/cursor_connection_spec.rb index 925266eab..6952eb8fc 100644 --- a/spec/graphql/connections/cursor_connection_spec.rb +++ b/spec/graphql/connections/cursor_connection_spec.rb @@ -10,7 +10,7 @@ RSpec.describe Connections::CursorConnection do end context 'without explicit args' do - let(:args) { { } } + let(:args) { {} } it { is_expected.to eq(limit: max_page_size + 1, inverted: false) } end @@ -22,7 +22,7 @@ RSpec.describe Connections::CursorConnection do end context 'when asked for 2 first elements, in order desc' do - let(:args) { { first: 2, order: :desc} } + let(:args) { { first: 2, order: :desc } } it { is_expected.to eq(limit: 3, inverted: true) } end @@ -40,11 +40,11 @@ RSpec.describe Connections::CursorConnection do end context 'when asked for 2 last elements, in order desc' do - let(:args) { { last: 2, order: :desc} } + let(:args) { { last: 2, order: :desc } } it { is_expected.to eq(limit: 3, inverted: false) } end - + context '' do let(:args) { { after: :after, first: 2 } } @@ -109,5 +109,4 @@ RSpec.describe Connections::CursorConnection do it { is_expected.to be true } end end - end