Merge branch 'feat/add_timezone_to_git_user' into 'master'

Add timezone attr to Gitlab::Git::User

See merge request gitlab-org/gitlab!64084

Gemfile

@@ -476,7 +476,7 @@ end
 gem 'spamcheck', '~> 0.1.0'
 
 # Gitaly GRPC protocol definitions
-gem 'gitaly', '~> 14.1.0.pre.rc1'
+gem 'gitaly', '~> 14.1.0.pre.rc2'
 
 # KAS GRPC protocol definitions
 gem 'kas-grpc', '~> 0.0.2'

Gemfile.lock

@@ -460,7 +460,7 @@ GEM
       rails (>= 3.2.0)
     git (1.7.0)
       rchardet (~> 1.8)
-    gitaly (14.1.0.pre.rc1)
+    gitaly (14.1.0.pre.rc2)
       grpc (~> 1.0)
     github-markup (1.7.0)
     gitlab (4.16.1)
@@ -1483,7 +1483,7 @@ DEPENDENCIES
   gettext (~> 3.3)
   gettext_i18n_rails (~> 1.8.0)
   gettext_i18n_rails_js (~> 1.3)
-  gitaly (~> 14.1.0.pre.rc1)
+  gitaly (~> 14.1.0.pre.rc2)
   github-markup (~> 1.7.0)
   gitlab-chronic (~> 0.10.5)
   gitlab-dangerfiles (~> 2.1.2)

config/feature_flags/development/add_timezone_to_web_operations.yml

+---
+name: add_timezone_to_web_operations
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/64084
+rollout_issue_url: https://gitlab.com/gitlab-org/gitaly/-/issues/3655
+milestone: '14.1'
+type: development
+group: group::gitaly
+default_enabled: false

lib/gitlab/git/user.rb

@@ -3,10 +3,10 @@
 module Gitlab
   module Git
     class User
-      attr_reader :username, :name, :email, :gl_id
+      attr_reader :username, :name, :email, :gl_id, :timezone
 
       def self.from_gitlab(gitlab_user)
-        new(gitlab_user.username, gitlab_user.name, gitlab_user.commit_email, Gitlab::GlId.gl_id(gitlab_user))
+        new(gitlab_user.username, gitlab_user.name, gitlab_user.commit_email, Gitlab::GlId.gl_id(gitlab_user), gitlab_user.timezone)
       end
 
       def self.from_gitaly(gitaly_user)
@@ -14,23 +14,30 @@ module Gitlab
           gitaly_user.gl_username,
           Gitlab::EncodingHelper.encode!(gitaly_user.name),
           Gitlab::EncodingHelper.encode!(gitaly_user.email),
-          gitaly_user.gl_id
+          gitaly_user.gl_id,
+          gitaly_user.timezone
         )
       end
 
-      def initialize(username, name, email, gl_id)
+      def initialize(username, name, email, gl_id, timezone)
         @username = username
         @name = name
         @email = email
         @gl_id = gl_id
+
+        @timezone = if Feature.enabled?(:add_timezone_to_web_operations)
+                      timezone
+                    else
+                      Time.zone.tzinfo.name
+                    end
       end
 
       def ==(other)
-        [username, name, email, gl_id] == [other.username, other.name, other.email, other.gl_id]
+        [username, name, email, gl_id, timezone] == [other.username, other.name, other.email, other.gl_id, other.timezone]
       end
 
       def to_gitaly
-        Gitaly::User.new(gl_username: username, gl_id: gl_id, name: name.b, email: email.b)
+        Gitaly::User.new(gl_username: username, gl_id: gl_id, name: name.b, email: email.b, timezone: timezone)
       end
     end
   end

spec/lib/gitlab/git/user_spec.rb

@@ -7,15 +7,16 @@ RSpec.describe Gitlab::Git::User do
   let(:name) { 'Jane Doé' }
   let(:email) { 'janedoé@example.com' }
   let(:gl_id) { 'user-123' }
+  let(:timezone) { 'Asia/Shanghai' }
   let(:user) do
-    described_class.new(username, name, email, gl_id)
+    described_class.new(username, name, email, gl_id, timezone)
   end
 
-  subject { described_class.new(username, name, email, gl_id) }
+  subject { described_class.new(username, name, email, gl_id, timezone) }
 
   describe '.from_gitaly' do
     let(:gitaly_user) do
-      Gitaly::User.new(gl_username: username, name: name.b, email: email.b, gl_id: gl_id)
+      Gitaly::User.new(gl_username: username, name: name.b, email: email.b, gl_id: gl_id, timezone: timezone)
     end
 
     subject { described_class.from_gitaly(gitaly_user) }
@@ -25,34 +26,45 @@ RSpec.describe Gitlab::Git::User do
 
   describe '.from_gitlab' do
     context 'when no commit_email has been set' do
-      let(:user) { build(:user, email: 'alice@example.com', commit_email: nil) }
+      let(:user) { build(:user, email: 'alice@example.com', commit_email: nil, timezone: timezone) }
 
       subject { described_class.from_gitlab(user) }
 
-      it { expect(subject).to eq(described_class.new(user.username, user.name, user.email, 'user-')) }
+      it { expect(subject).to eq(described_class.new(user.username, user.name, user.email, 'user-', timezone)) }
     end
 
     context 'when commit_email has been set' do
-      let(:user) { build(:user, email: 'alice@example.com', commit_email: 'bob@example.com') }
+      let(:user) { build(:user, email: 'alice@example.com', commit_email: 'bob@example.com', timezone: timezone) }
 
       subject { described_class.from_gitlab(user) }
 
-      it { expect(subject).to eq(described_class.new(user.username, user.name, user.commit_email, 'user-')) }
+      it { expect(subject).to eq(described_class.new(user.username, user.name, user.commit_email, 'user-', timezone)) }
     end
   end
 
   describe '#==' do
-    def eq_other(username, name, email, gl_id)
-      eq(described_class.new(username, name, email, gl_id))
+    def eq_other(username, name, email, gl_id, timezone)
+      eq(described_class.new(username, name, email, gl_id, timezone))
     end
 
-    it { expect(subject).to eq_other(username, name, email, gl_id) }
+    it { expect(subject).to eq_other(username, name, email, gl_id, timezone) }
 
-    it { expect(subject).not_to eq_other(nil, nil, nil, nil) }
-    it { expect(subject).not_to eq_other(username + 'x', name, email, gl_id) }
-    it { expect(subject).not_to eq_other(username, name + 'x', email, gl_id) }
-    it { expect(subject).not_to eq_other(username, name, email + 'x', gl_id) }
-    it { expect(subject).not_to eq_other(username, name, email, gl_id + 'x') }
+    it { expect(subject).not_to eq_other(nil, nil, nil, nil, timezone) }
+    it { expect(subject).not_to eq_other(username + 'x', name, email, gl_id, timezone) }
+    it { expect(subject).not_to eq_other(username, name + 'x', email, gl_id, timezone) }
+    it { expect(subject).not_to eq_other(username, name, email + 'x', gl_id, timezone) }
+    it { expect(subject).not_to eq_other(username, name, email, gl_id + 'x', timezone) }
+    it { expect(subject).not_to eq_other(username, name, email, gl_id, 'Etc/UTC') }
+
+    context 'when add_timezone_to_web_operations is disabled' do
+      before do
+        stub_feature_flags(add_timezone_to_web_operations: false)
+      end
+
+      it 'ignores timezone arg and sets Etc/UTC by default' do
+        expect(user.timezone).to eq('Etc/UTC')
+      end
+    end
   end
 
   describe '#to_gitaly' do
@@ -69,6 +81,7 @@ RSpec.describe Gitlab::Git::User do
       expect(subject.email).to be_a_binary_string
 
       expect(subject.gl_id).to eq(gl_id)
+      expect(subject.timezone).to eq(timezone)
     end
   end
 end