Merge branch '37385-respect-commit-timezones-from-gitaly' into 'master'

Respect the timezone reported from Gitaly for commits

Closes #37385

See merge request gitlab-org/gitlab!21066

changelogs/unreleased/37385-respect-commit-timezones-from-gitaly.yml

+---
+title: Respect the timezone reported from Gitaly
+merge_request: 21066
+author:
+type: fixed

lib/gitlab/git/commit.rb

@@ -370,15 +370,26 @@ module Gitlab
         # subject from the message to make it clearer when there's one
         # available but not the other.
         @message = message_from_gitaly_body
-        @authored_date = Time.at(commit.author.date.seconds).utc
+        @authored_date = init_date_from_gitaly(commit.author)
         @author_name = commit.author.name.dup
         @author_email = commit.author.email.dup
-        @committed_date = Time.at(commit.committer.date.seconds).utc
+
+        @committed_date = init_date_from_gitaly(commit.committer)
         @committer_name = commit.committer.name.dup
         @committer_email = commit.committer.email.dup
         @parent_ids = Array(commit.parent_ids)
       end
 
+      # Gitaly provides a UNIX timestamp in author.date.seconds, and a timezone
+      # offset in author.timezone. If the latter isn't present, assume UTC.
+      def init_date_from_gitaly(author)
+        if author.timezone.present?
+          Time.strptime("#{author.date.seconds} #{author.timezone}", '%s %z')
+        else
+          Time.at(author.date.seconds).utc
+        end
+      end
+
       def serialize_keys
         SERIALIZE_KEYS
       end

spec/lib/gitlab/git/commit_spec.rb

@@ -17,13 +17,13 @@ describe Gitlab::Git::Commit, :seed_helper do
       @committer = {
         email: 'mike@smith.com',
         name: "Mike Smith",
-        time: Time.now
+        time: Time.new(2000, 1, 1, 0, 0, 0, "+08:00")
       }
 
       @author = {
         email: 'john@smith.com',
         name: "John Smith",
-        time: Time.now
+        time: Time.new(2000, 1, 1, 0, 0, 0, "-08:00")
       }
 
       @parents = [rugged_repo.head.target]
@@ -48,7 +48,7 @@ describe Gitlab::Git::Commit, :seed_helper do
     it { expect(@commit.id).to eq(@raw_commit.oid) }
     it { expect(@commit.sha).to eq(@raw_commit.oid) }
     it { expect(@commit.safe_message).to eq(@raw_commit.message) }
-    it { expect(@commit.created_at).to eq(@raw_commit.author[:time]) }
+    it { expect(@commit.created_at).to eq(@raw_commit.committer[:time]) }
     it { expect(@commit.date).to eq(@raw_commit.committer[:time]) }
     it { expect(@commit.author_email).to eq(@author[:email]) }
     it { expect(@commit.author_name).to eq(@author[:name]) }
@@ -79,13 +79,27 @@ describe Gitlab::Git::Commit, :seed_helper do
     it { expect(commit.id).to eq(id) }
     it { expect(commit.sha).to eq(id) }
     it { expect(commit.safe_message).to eq(body) }
-    it { expect(commit.created_at).to eq(Time.at(committer.date.seconds)) }
+    it { expect(commit.created_at).to eq(Time.at(committer.date.seconds).utc) }
     it { expect(commit.author_email).to eq(author.email) }
     it { expect(commit.author_name).to eq(author.name) }
     it { expect(commit.committer_name).to eq(committer.name) }
     it { expect(commit.committer_email).to eq(committer.email) }
     it { expect(commit.parent_ids).to eq(gitaly_commit.parent_ids) }
 
+    context 'non-UTC dates' do
+      let(:seconds) { Time.now.to_i }
+
+      it 'sets timezones correctly' do
+        gitaly_commit.author.date.seconds = seconds
+        gitaly_commit.author.timezone = '-0800'
+        gitaly_commit.committer.date.seconds = seconds
+        gitaly_commit.committer.timezone = '+0800'
+
+        expect(commit.authored_date).to eq(Time.at(seconds, in: '-08:00'))
+        expect(commit.committed_date).to eq(Time.at(seconds, in: '+08:00'))
+      end
+    end
+
     context 'body_size != body.size' do
       let(:body) { (+"").force_encoding('ASCII-8BIT') }
 

spec/models/commit_spec.rb

@@ -359,7 +359,7 @@ eos
 
     it { expect(data).to be_a(Hash) }
     it { expect(data[:message]).to include('adds bar folder and branch-test text file to check Repository merged_to_root_ref method') }
-    it { expect(data[:timestamp]).to eq('2016-09-27T14:37:46Z') }
+    it { expect(data[:timestamp]).to eq('2016-09-27T14:37:46+00:00') }
     it { expect(data[:added]).to contain_exactly("bar/branch-test.txt") }
     it { expect(data[:modified]).to eq([]) }
     it { expect(data[:removed]).to eq([]) }

spec/requests/api/files_spec.rb

@@ -315,11 +315,11 @@ describe API::Files do
         expect(range['commit']['message'])
           .to eq("Files, encoding and much more\n\nSigned-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>\n")
 
-        expect(range['commit']['authored_date']).to eq('2014-02-27T08:14:56.000Z')
+        expect(range['commit']['authored_date']).to eq('2014-02-27T10:14:56.000+02:00')
         expect(range['commit']['author_name']).to eq('Dmitriy Zaporozhets')
         expect(range['commit']['author_email']).to eq('dmitriy.zaporozhets@gmail.com')
 
-        expect(range['commit']['committed_date']).to eq('2014-02-27T08:14:56.000Z')
+        expect(range['commit']['committed_date']).to eq('2014-02-27T10:14:56.000+02:00')
         expect(range['commit']['committer_name']).to eq('Dmitriy Zaporozhets')
         expect(range['commit']['committer_email']).to eq('dmitriy.zaporozhets@gmail.com')
       end