From fbed95360c7bd9650c5780cf915f2c8fe2bc995b Mon Sep 17 00:00:00 2001 From: mao chao <chaomao@jihulab.com> Date: Tue, 5 Jul 2022 11:14:28 +0800 Subject: [PATCH] Fix pagniator issue in different timezone When we generate cursor info, we should not use below format field_value.strftime('%Y-%m-%d %H:%M:%S.%N %Z') JiHu GitLab set Rails Time.zone = 'Asia/Shanghai', which will generate '2022-07-05 11:17:50.948163000 CST', CST means 'China Standard Time', however when rails try to use this time to build sql, it need convert time to UTC format, which consider 'CST' as 'Central Standard Time', then sql will query data with wrong time. We should use 'field_value.to_s(:inspect)', same as field_value.strftime('%Y-%m-%d %H:%M:%S.%N %z'), "%z" instead of "%Z", wihch generate below format "2022-07-05 16:45:02.933753000 +0800" the specific timezone info will make converting UTC time in sql correctly. Changelog: fixed EE: true --- .../cycle_analytics/consistency_check_service_spec.rb | 4 ++-- lib/gitlab/graphql/pagination/keyset/connection.rb | 2 +- lib/gitlab/pagination/keyset/order.rb | 4 +++- spec/frontend/releases/__snapshots__/util_spec.js.snap | 4 ++-- .../pagination/keyset/connection_generic_keyset_spec.rb | 2 +- .../lib/gitlab/graphql/pagination/keyset/connection_spec.rb | 6 +++--- 6 files changed, 12 insertions(+), 10 deletions(-) diff --git a/ee/spec/services/analytics/cycle_analytics/consistency_check_service_spec.rb b/ee/spec/services/analytics/cycle_analytics/consistency_check_service_spec.rb index 94344cdae10fe..95ff5ff06b31e 100644 --- a/ee/spec/services/analytics/cycle_analytics/consistency_check_service_spec.rb +++ b/ee/spec/services/analytics/cycle_analytics/consistency_check_service_spec.rb @@ -58,8 +58,8 @@ last_processed_event = initial_events[i] expect(response.payload[:cursor]).to eq({ - 'start_event_timestamp' => last_processed_event.start_event_timestamp.strftime('%Y-%m-%d %H:%M:%S.%N %Z'), - 'end_event_timestamp' => last_processed_event.end_event_timestamp.strftime('%Y-%m-%d %H:%M:%S.%N %Z'), + 'start_event_timestamp' => last_processed_event.start_event_timestamp.to_s(:inspect), + 'end_event_timestamp' => last_processed_event.end_event_timestamp.to_s(:inspect), "#{event_model.issuable_id_column}" => last_processed_event[event_model.issuable_id_column].to_s }) diff --git a/lib/gitlab/graphql/pagination/keyset/connection.rb b/lib/gitlab/graphql/pagination/keyset/connection.rb index c284160e53922..3e119a39e6d4f 100644 --- a/lib/gitlab/graphql/pagination/keyset/connection.rb +++ b/lib/gitlab/graphql/pagination/keyset/connection.rb @@ -199,7 +199,7 @@ def encoded_json_from_ordering(node) field_name = field.try(:attribute_name) || field field_value = node[field_name] ordering[field_name] = if field_value.is_a?(Time) - field_value.strftime('%Y-%m-%d %H:%M:%S.%N %Z') + field_value.to_s(:inspect) else field_value.to_s end diff --git a/lib/gitlab/pagination/keyset/order.rb b/lib/gitlab/pagination/keyset/order.rb index eecf74b8d18e4..0d8e4ea6fee2c 100644 --- a/lib/gitlab/pagination/keyset/order.rb +++ b/lib/gitlab/pagination/keyset/order.rb @@ -96,7 +96,9 @@ def cursor_attributes_for_node(node) column_definitions.each_with_object({}.with_indifferent_access) do |column_definition, hash| field_value = node[column_definition.attribute_name] hash[column_definition.attribute_name] = if field_value.is_a?(Time) - field_value.strftime('%Y-%m-%d %H:%M:%S.%N %Z') + # use :inspect formatter to provide specific timezone info + # eg 2022-07-05 21:57:56.041499000 +0800 + field_value.to_s(:inspect) elsif field_value.nil? nil elsif lower_named_function?(column_definition) diff --git a/spec/frontend/releases/__snapshots__/util_spec.js.snap b/spec/frontend/releases/__snapshots__/util_spec.js.snap index 10d250c5ebbc4..0bf0ef1ded495 100644 --- a/spec/frontend/releases/__snapshots__/util_spec.js.snap +++ b/spec/frontend/releases/__snapshots__/util_spec.js.snap @@ -196,10 +196,10 @@ Object { ], "paginationInfo": Object { "__typename": "PageInfo", - "endCursor": "eyJyZWxlYXNlZF9hdCI6IjIwMTgtMTItMTAgMDA6MDA6MDAuMDAwMDAwMDAwIFVUQyIsImlkIjoiMSJ9", + "endCursor": "eyJyZWxlYXNlZF9hdCI6IjIwMTgtMTItMTAgMDA6MDA6MDAuMDAwMDAwMDAwICswMDAwIiwiaWQiOiIxIn0", "hasNextPage": false, "hasPreviousPage": false, - "startCursor": "eyJyZWxlYXNlZF9hdCI6IjIwMTktMDEtMTAgMDA6MDA6MDAuMDAwMDAwMDAwIFVUQyIsImlkIjoiMiJ9", + "startCursor": "eyJyZWxlYXNlZF9hdCI6IjIwMTktMDEtMTAgMDA6MDA6MDAuMDAwMDAwMDAwICswMDAwIiwiaWQiOiIyIn0", }, } `; diff --git a/spec/lib/gitlab/graphql/pagination/keyset/connection_generic_keyset_spec.rb b/spec/lib/gitlab/graphql/pagination/keyset/connection_generic_keyset_spec.rb index 97613edee5e93..8a2b5ae0d3829 100644 --- a/spec/lib/gitlab/graphql/pagination/keyset/connection_generic_keyset_spec.rb +++ b/spec/lib/gitlab/graphql/pagination/keyset/connection_generic_keyset_spec.rb @@ -79,7 +79,7 @@ def decoded_cursor(cursor) let(:nodes) { Project.all.order(Gitlab::Pagination::Keyset::Order.build([column_order_updated_at, column_order_created_at, column_order_id])) } it 'returns the encoded value of the order' do - expect(decoded_cursor(cursor)).to include('updated_at' => project.updated_at.strftime('%Y-%m-%d %H:%M:%S.%N %Z')) + expect(decoded_cursor(cursor)).to include('updated_at' => project.updated_at.to_s(:inspect)) end end end diff --git a/spec/lib/gitlab/graphql/pagination/keyset/connection_spec.rb b/spec/lib/gitlab/graphql/pagination/keyset/connection_spec.rb index 61a79d9054680..6574b3e3131dd 100644 --- a/spec/lib/gitlab/graphql/pagination/keyset/connection_spec.rb +++ b/spec/lib/gitlab/graphql/pagination/keyset/connection_spec.rb @@ -92,7 +92,7 @@ def decoded_cursor(cursor) let(:nodes) { Project.order(:updated_at) } it 'returns the encoded value of the order' do - expect(decoded_cursor(cursor)).to include('updated_at' => project.updated_at.strftime('%Y-%m-%d %H:%M:%S.%N %Z')) + expect(decoded_cursor(cursor)).to include('updated_at' => project.updated_at.to_s(:inspect)) end it 'includes the :id even when not specified in the order' do @@ -104,7 +104,7 @@ def decoded_cursor(cursor) let(:nodes) { Project.order(:updated_at).order(:created_at) } it 'returns the encoded value of the order' do - expect(decoded_cursor(cursor)).to include('updated_at' => project.updated_at.strftime('%Y-%m-%d %H:%M:%S.%N %Z')) + expect(decoded_cursor(cursor)).to include('updated_at' => project.updated_at.to_s(:inspect)) end end @@ -112,7 +112,7 @@ def decoded_cursor(cursor) let(:nodes) { Project.order(Arel.sql('projects.updated_at IS NULL')).order(:updated_at).order(:id) } it 'returns the encoded value of the order' do - expect(decoded_cursor(cursor)).to include('updated_at' => project.updated_at.strftime('%Y-%m-%d %H:%M:%S.%N %Z')) + expect(decoded_cursor(cursor)).to include('updated_at' => project.updated_at.to_s(:inspect)) end end end -- GitLab