Commit 06facbf4 authored by Harsh Chouraria's avatar Harsh Chouraria

Format timestamp data in pipeline charts

This is a follow-up fix to an issue introduced by the changes
in https://gitlab.com/gitlab-org/gitlab/-/merge_requests/47069

Without formatting the timestamp datatypes returned by the
database, the lookup of each day can fail if GitLab is
configured to run with a Rails time_zone set to anything
non-UTC.

This happens due to the use of Date/Time objects for hash
lookups, which are influenced by the global Rails
configuration and produce a timezoned object that does not
equate with the same time in UTC.

Tests added here fail without the fix, displaying an all zero
data for the chart.

Fixes issue https://gitlab.com/gitlab-org/gitlab/-/issues/296451
parent 5b225881
---
title: Fix empty pipeline analytics charts when time_zone is non-UTC
merge_request:
author:
type: fixed
...@@ -31,9 +31,10 @@ module Gitlab ...@@ -31,9 +31,10 @@ module Gitlab
current = @from current = @from
while current <= @to while current <= @to
@labels << current.strftime(@format) label = current.strftime(@format)
@total << (totals_count[current] || 0) @labels << label
@success << (success_count[current] || 0) @total << (totals_count[label] || 0)
@success << (success_count[label] || 0)
current += interval_step current += interval_step
end end
...@@ -45,6 +46,7 @@ module Gitlab ...@@ -45,6 +46,7 @@ module Gitlab
query query
.group("date_trunc('#{interval}', #{::Ci::Pipeline.table_name}.created_at)") .group("date_trunc('#{interval}', #{::Ci::Pipeline.table_name}.created_at)")
.count(:created_at) .count(:created_at)
.transform_keys { |date| date.strftime(@format) }
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
......
...@@ -47,6 +47,10 @@ RSpec.describe Gitlab::Ci::Charts do ...@@ -47,6 +47,10 @@ RSpec.describe Gitlab::Ci::Charts do
subject { chart.to } subject { chart.to }
before do
create(:ci_empty_pipeline, project: project, duration: 120)
end
it 'includes the whole current day' do it 'includes the whole current day' do
is_expected.to eq(Date.today.end_of_day) is_expected.to eq(Date.today.end_of_day)
end end
...@@ -58,6 +62,37 @@ RSpec.describe Gitlab::Ci::Charts do ...@@ -58,6 +62,37 @@ RSpec.describe Gitlab::Ci::Charts do
it 'uses %d %B as labels format' do it 'uses %d %B as labels format' do
expect(chart.labels).to include(chart.from.strftime('%d %B')) expect(chart.labels).to include(chart.from.strftime('%d %B'))
end end
it 'returns count of pipelines run each day in the current week' do
expect(chart.total).to contain_exactly(0, 0, 0, 0, 0, 0, 0, 1)
end
end
context 'weekchart_non_utc' do
today = Date.today
end_of_today = Time.use_zone(Time.find_zone('Asia/Dubai')) { today.end_of_day }
let(:project) { create(:project) }
let(:chart) do
allow(Date).to receive(:today).and_return(today)
allow(today).to receive(:end_of_day).and_return(end_of_today)
Gitlab::Ci::Charts::WeekChart.new(project)
end
subject { chart.total }
before do
create(:ci_empty_pipeline, project: project, duration: 120)
end
it 'uses a non-utc time zone for range times' do
expect(chart.to.zone).to eq(end_of_today.zone)
expect(chart.from.zone).to eq(end_of_today.zone)
end
it 'returns count of pipelines run each day in the current week' do
is_expected.to contain_exactly(0, 0, 0, 0, 0, 0, 0, 1)
end
end end
context 'pipeline_times' do context 'pipeline_times' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment