Remove default value for time zone user preference

This allows us to only display local time on user profiles if a user
has explicitly set their time zone. This commit also allows users to
clear their time zone user preference so local time is not shown on
their profile.

Changelog: changed

app/assets/javascripts/pages/projects/pipeline_schedules/shared/components/timezone_dropdown.js

@@ -17,7 +17,13 @@ export const formatUtcOffset = (offset) => {
   return `${prefix} ${Math.abs(offset / 3600)}`;
 };
 
-export const formatTimezone = (item) => `[UTC ${formatUtcOffset(item.offset)}] ${item.name}`;
+export const formatTimezone = (item) => {
+  if (Object.prototype.hasOwnProperty.call(item, 'offset')) {
+    return `[UTC ${formatUtcOffset(item.offset)}] ${item.name}`;
+  }
+
+  return item.name;
+};
 
 export const findTimezoneByIdentifier = (tzList = [], identifier = null) => {
   if (tzList && tzList.length && identifier && identifier.length) {
@@ -27,19 +33,31 @@ export const findTimezoneByIdentifier = (tzList = [], identifier = null) => {
 };
 
 export default class TimezoneDropdown {
-  constructor({ $dropdownEl, $inputEl, onSelectTimezone, displayFormat } = defaults) {
+  constructor({
+    $dropdownEl,
+    $inputEl,
+    onSelectTimezone,
+    displayFormat,
+    allowEmpty = false,
+  } = defaults) {
     this.$dropdown = $dropdownEl;
     this.$dropdownToggle = this.$dropdown.find('.dropdown-toggle-text');
     this.$input = $inputEl;
-    this.timezoneData = this.$dropdown.data('data');
+    this.timezoneData = this.$dropdown.data('data') || [];
+
+    if (allowEmpty) {
+      this.timezoneData = [
+        {
+          name: this.$dropdownToggle.text(),
+        },
+        ...this.timezoneData,
+      ];
+    }
 
     this.onSelectTimezone = onSelectTimezone;
     this.displayFormat = displayFormat || defaults.displayFormat;
+    this.allowEmpty = allowEmpty;
 
-    this.initialTimezone =
-      findTimezoneByIdentifier(this.timezoneData, this.$input.val()) || defaultTimezone;
-
-    this.initDefaultTimezone();
     this.initDropdown();
   }
 
@@ -52,24 +70,25 @@ export default class TimezoneDropdown {
       search: {
         fields: ['name'],
       },
-      clicked: (cfg) => this.updateInputValue(cfg),
+      clicked: (cfg) => this.handleDropdownChange(cfg),
       text: (item) => formatTimezone(item),
     });
 
-    this.setDropdownToggle(this.displayFormat(this.initialTimezone));
-  }
+    const initialTimezone = findTimezoneByIdentifier(this.timezoneData, this.$input.val());
 
-  initDefaultTimezone() {
-    if (!this.$input.val()) {
-      this.$input.val(defaultTimezone.name);
+    if (initialTimezone !== null) {
+      this.setDropdownValue(initialTimezone);
+    } else if (!this.allowEmpty) {
+      this.setDropdownValue(defaultTimezone);
     }
   }
 
-  setDropdownToggle(dropdownText) {
-    this.$dropdownToggle.text(dropdownText);
+  setDropdownValue(timezone) {
+    this.$dropdownToggle.text(this.displayFormat(timezone));
+    this.$input.val(timezone.name);
   }
 
-  updateInputValue({ selectedObj, e }) {
+  handleDropdownChange({ selectedObj, e }) {
     e.preventDefault();
     this.$input.val(selectedObj.identifier);
     if (this.onSelectTimezone) {

app/assets/javascripts/profile/profile.js

@@ -21,6 +21,7 @@ export default class Profile {
       $inputEl: this.$inputEl,
       $dropdownEl: $('.js-timezone-dropdown'),
       displayFormat: (selectedItem) => formatTimezone(selectedItem),
+      allowEmpty: true,
     });
   }
 

app/helpers/time_zone_helper.rb

@@ -33,6 +33,8 @@ module TimeZoneHelper
   end
 
   def local_time(timezone)
+    return if timezone.blank?
+
     time_zone_instance = ActiveSupport::TimeZone.new(timezone) || Time.zone
     time_zone_instance.now.strftime("%-l:%M %p")
   end

app/models/user_preference.rb

@@ -23,7 +23,6 @@ class UserPreference < ApplicationRecord
   ignore_columns :experience_level, remove_with: '14.10', remove_after: '2021-03-22'
 
   default_value_for :tab_width, value: Gitlab::TabWidth::DEFAULT, allows_nil: false
-  default_value_for :timezone, value: Time.zone.tzinfo.name, allows_nil: false
   default_value_for :time_display_relative, value: true, allows_nil: false
   default_value_for :time_format_in_24h, value: false, allows_nil: false
   default_value_for :render_whitespace_in_code, value: false, allows_nil: false

app/views/profiles/show.html.haml

@@ -80,7 +80,7 @@
       %p= s_("Profiles|Set your local time zone")
     .col-lg-8
       %h5= _("Time zone")
-      = dropdown_tag(_("Select a time zone"), options: { toggle_class: 'gl-button btn js-timezone-dropdown input-lg', title: _("Select a time zone"), filter: true, placeholder: s_("OfSearchInADropdown|Filter"), data: { data: timezone_data } } )
+      = dropdown_tag(_("Select a time zone"), options: { toggle_class: 'gl-button btn js-timezone-dropdown input-lg gl-w-full!', title: _("Select a time zone"), filter: true, placeholder: s_("OfSearchInADropdown|Filter"), data: { data: timezone_data } } )
       %input.hidden{ :type => 'hidden', :id => 'user_timezone', :name => 'user[timezone]', value: @user.timezone }
     .col-lg-12
       %hr

app/views/users/show.html.haml

@@ -84,10 +84,12 @@
                 = sprite_icon('location', css_class: 'fgray')
                 %span{ itemprop: 'addressLocality' }
                   = @user.location
-            = render 'middle_dot_divider', stacking: true do
-              = sprite_icon('clock', css_class: 'fgray')
-              %span
-                = local_time(@user.timezone)
+            - user_local_time = local_time(@user.timezone)
+            - unless user_local_time.nil?
+              = render 'middle_dot_divider', stacking: true, data: { testid: 'user-local-time' } do
+                = sprite_icon('clock', css_class: 'fgray')
+                %span
+                  = user_local_time
             - unless work_information(@user).blank?
               = render 'middle_dot_divider', stacking: true do
                 = sprite_icon('work', css_class: 'fgray')

doc/administration/timezone.md

@@ -48,8 +48,8 @@ gitlab-ctl restart
 > - [Enabled by default](https://gitlab.com/gitlab-org/gitlab/-/issues/29669) in GitLab 13.9.
 > - [Feature flag removed](https://gitlab.com/gitlab-org/gitlab/-/issues/29669) in GitLab 14.1.
 
-A user can set their time zone in their profile. If a user has not set their time zone, it defaults
-to the time zone [configured at the instance level](#changing-your-time-zone). On GitLab.com, the
-default time zone is UTC.
+A user can set their time zone in their profile. In GitLab 14.3 and below if a user has not set their time zone, it defaults
+to the time zone [configured at the instance level](#changing-your-time-zone). On GitLab.com, the default time zone is UTC.
+In GitLab 14.4 and above there is no default time zone for new users. New users will need to explicitly set their time zone before it is shown on their profile.
 
 For more information, see [Set your time zone](../user/profile/index.md#set-your-time-zone).

spec/features/profiles/user_edit_profile_spec.rb

@@ -561,14 +561,11 @@ RSpec.describe 'User edit profile' do
 
         page.find("a", text: "Nuku'alofa").click
 
-        tz = page.find('.user-time-preferences #user_timezone', visible: false)
-
-        expect(tz.value).to eq('Pacific/Tongatapu')
+        expect(page).to have_field(:user_timezone, with: 'Pacific/Tongatapu', type: :hidden)
       end
 
-      it 'timezone defaults to servers default' do
-        timezone_name = Time.zone.tzinfo.name
-        expect(page.find('.user-time-preferences #user_timezone', visible: false).value).to eq(timezone_name)
+      it 'timezone defaults to empty' do
+        expect(page).to have_field(:user_timezone, with: '', type: :hidden)
       end
     end
   end

spec/features/users/show_spec.rb

@@ -81,6 +81,7 @@ RSpec.describe 'User page' do
 
     context 'timezone' do
       let_it_be(:timezone) { 'America/Los_Angeles' }
+      let_it_be(:local_time_selector) { '[data-testid="user-local-time"]' }
 
       before do
         travel_to Time.find_zone(timezone).local(2021, 7, 20, 15, 30, 45)
@@ -92,7 +93,19 @@ RSpec.describe 'User page' do
         it 'shows local time' do
           subject
 
-          expect(page).to have_content('3:30 PM')
+          within local_time_selector do
+            expect(page).to have_content('3:30 PM')
+          end
+        end
+      end
+
+      context 'when timezone is not set' do
+        let_it_be(:user) { create(:user, timezone: nil) }
+
+        it 'does not show local time' do
+          subject
+
+          expect(page).not_to have_selector(local_time_selector)
         end
       end
 
@@ -102,7 +115,9 @@ RSpec.describe 'User page' do
         it 'shows local time using the configured default timezone (UTC in this case)' do
           subject
 
-          expect(page).to have_content('10:30 PM')
+          within local_time_selector do
+            expect(page).to have_content('10:30 PM')
+          end
         end
       end
     end

spec/frontend/pages/projects/pipeline_schedules/shared/components/timezone_dropdown_spec.js

@@ -10,7 +10,17 @@ describe('Timezone Dropdown', () => {
   let $dropdownEl = null;
   let $wrapper = null;
   const tzListSel = '.dropdown-content ul li a.is-active';
-  const tzDropdownToggleText = '.dropdown-toggle-text';
+
+  const initTimezoneDropdown = (options = {}) => {
+    // eslint-disable-next-line no-new
+    new TimezoneDropdown({
+      $inputEl,
+      $dropdownEl,
+      ...options,
+    });
+  };
+
+  const findDropdownToggleText = () => $wrapper.find('.dropdown-toggle-text');
 
   describe('Initialize', () => {
     describe('with dropdown already loaded', () => {
@@ -18,16 +28,13 @@ describe('Timezone Dropdown', () => {
         loadFixtures('pipeline_schedules/edit.html');
         $wrapper = $('.dropdown');
         $inputEl = $('#schedule_cron_timezone');
+        $inputEl.val('');
         $dropdownEl = $('.js-timezone-dropdown');
-
-        // eslint-disable-next-line no-new
-        new TimezoneDropdown({
-          $inputEl,
-          $dropdownEl,
-        });
       });
 
       it('can take an $inputEl in the constructor', () => {
+        initTimezoneDropdown();
+
         const tzStr = '[UTC + 5.5] Sri Jayawardenepura';
         const tzValue = 'Asia/Colombo';
 
@@ -42,6 +49,8 @@ describe('Timezone Dropdown', () => {
       });
 
       it('will format data array of timezones into a list of offsets', () => {
+        initTimezoneDropdown();
+
         const data = $dropdownEl.data('data');
         const formatted = $wrapper.find(tzListSel).text();
 
@@ -50,10 +59,34 @@ describe('Timezone Dropdown', () => {
         });
       });
 
-      it('will default the timezone to UTC', () => {
-        const tz = $inputEl.val();
+      describe('when `allowEmpty` property is `false`', () => {
+        beforeEach(() => {
+          initTimezoneDropdown();
+        });
+
+        it('will default the timezone to UTC', () => {
+          const tz = $inputEl.val();
+
+          expect(tz).toBe('UTC');
+        });
+      });
+
+      describe('when `allowEmpty` property is `true`', () => {
+        beforeEach(() => {
+          initTimezoneDropdown({
+            allowEmpty: true,
+          });
+        });
+
+        it('will default the value of the input to an empty string', () => {
+          expect($inputEl.val()).toBe('');
+        });
 
-        expect(tz).toBe('UTC');
+        it('will add a new dropdown item for selecting an empty value', () => {
+          expect(
+            $(`${tzListSel}:contains('${findDropdownToggleText().html()}')`, $wrapper).length,
+          ).not.toBe(0);
+        });
       });
     });
 
@@ -68,23 +101,15 @@ describe('Timezone Dropdown', () => {
       it('will populate the list of UTC offsets after the dropdown is loaded', () => {
         expect($wrapper.find(tzListSel).length).toEqual(0);
 
-        // eslint-disable-next-line no-new
-        new TimezoneDropdown({
-          $inputEl,
-          $dropdownEl,
-        });
+        initTimezoneDropdown();
 
         expect($wrapper.find(tzListSel).length).toEqual($($dropdownEl).data('data').length);
       });
 
       it('will call a provided handler when a new timezone is selected', () => {
         const onSelectTimezone = jest.fn();
-        // eslint-disable-next-line no-new
-        new TimezoneDropdown({
-          $inputEl,
-          $dropdownEl,
-          onSelectTimezone,
-        });
+
+        initTimezoneDropdown({ onSelectTimezone });
 
         $wrapper.find(tzListSel).first().trigger('click');
 
@@ -94,24 +119,15 @@ describe('Timezone Dropdown', () => {
       it('will correctly set the dropdown label if a timezone identifier is set on the inputEl', () => {
         $inputEl.val('America/St_Johns');
 
-        // eslint-disable-next-line no-new
-        new TimezoneDropdown({
-          $inputEl,
-          $dropdownEl,
-          displayFormat: (selectedItem) => formatTimezone(selectedItem),
-        });
+        initTimezoneDropdown({ displayFormat: (selectedItem) => formatTimezone(selectedItem) });
 
-        expect($wrapper.find(tzDropdownToggleText).html()).toEqual('[UTC - 2.5] Newfoundland');
+        expect(findDropdownToggleText().html()).toEqual('[UTC - 2.5] Newfoundland');
       });
 
       it('will call a provided `displayFormat` handler to format the dropdown value', () => {
         const displayFormat = jest.fn();
-        // eslint-disable-next-line no-new
-        new TimezoneDropdown({
-          $inputEl,
-          $dropdownEl,
-          displayFormat,
-        });
+
+        initTimezoneDropdown({ displayFormat });
 
         $wrapper.find(tzListSel).first().trigger('click');
 
@@ -186,6 +202,14 @@ describe('Timezone Dropdown', () => {
         }),
       ).toEqual('[UTC 0] Accra');
     });
+
+    it('returns name when no offset is provided', () => {
+      expect(
+        formatTimezone({
+          name: 'Foo bar',
+        }),
+      ).toEqual('Foo bar');
+    });
   });
 
   describe('findTimezoneByIdentifier', () => {

spec/helpers/time_zone_helper_spec.rb

@@ -76,6 +76,18 @@ RSpec.describe TimeZoneHelper, :aggregate_failures do
       travel_to Time.find_zone(timezone).local(2021, 7, 20, 15, 30, 45)
     end
 
+    context 'when timezone is `nil`' do
+      it 'returns `nil`' do
+        expect(helper.local_time(nil)).to eq(nil)
+      end
+    end
+
+    context 'when timezone is blank' do
+      it 'returns `nil`' do
+        expect(helper.local_time('')).to eq(nil)
+      end
+    end
+
     context 'when a valid timezone is passed' do
       it 'returns local time' do
         expect(helper.local_time(timezone)).to eq('3:30 PM')

spec/models/user_preference_spec.rb

@@ -80,12 +80,6 @@ RSpec.describe UserPreference do
     end
   end
 
-  describe '#timezone' do
-    it 'returns server time as default' do
-      expect(user_preference.timezone).to eq(Time.zone.tzinfo.name)
-    end
-  end
-
   describe '#tab_width' do
     it 'is set to 8 by default' do
       # Intentionally not using factory here to test the constructor.