settings: Migrate twenty_four_hour_time setting to RealmUserDefault.

This commit removes the existing default_twenty_four_hour_time field in
Realm table which was used to set the twenty_four_hour_time setting of
new user on joining and instead we now use the twenty_four_hour_time
field of RealmUserDefault table for the same.

With some tweaks by tabbott to clarify the documentation.

frontend_tests/node_tests/dispatch.js

@@ -373,9 +373,6 @@ run_test("realm settings", ({override}) => {
     event = event_fixtures.realm__update__disallow_disposable_email_addresses;
     test_realm_boolean(event, "realm_disallow_disposable_email_addresses");
 
-    event = event_fixtures.realm__update__default_twenty_four_hour_time;
-    test_realm_boolean(event, "realm_default_twenty_four_hour_time");
-
     event = event_fixtures.realm__update__email_addresses_visibility;
     dispatch(event);
     assert_same(page_params.realm_email_address_visibility, 3);

frontend_tests/node_tests/lib/events.js

@@ -254,13 +254,6 @@ exports.fixtures = {
         value: "javascript",
     },
 
-    realm__update__default_twenty_four_hour_time: {
-        type: "realm",
-        op: "update",
-        property: "default_twenty_four_hour_time",
-        value: false,
-    },
-
     realm__update__disallow_disposable_email_addresses: {
         type: "realm",
         op: "update",

frontend_tests/node_tests/settings_org.js

@@ -169,7 +169,6 @@ function test_submit_settings_form(override, submit_form) {
         realm_create_stream_by_admins_only: true,
         realm_waiting_period_threshold: 1,
         realm_default_language: '"es"',
-        realm_default_twenty_four_hour_time: false,
         realm_invite_to_stream_policy: settings_config.common_policy_values.by_admins_only.code,
         realm_create_stream_policy: settings_config.common_policy_values.by_members.code,
         realm_invite_to_realm_policy: settings_config.common_policy_values.by_members.code,
@@ -262,24 +261,16 @@ function test_submit_settings_form(override, submit_form) {
     realm_default_language_elem.val("en");
     realm_default_language_elem.attr("id", "id_realm_default_language");
     realm_default_language_elem.data = () => "string";
-    const realm_default_twenty_four_hour_time_elem = $("#id_realm_default_twenty_four_hour_time");
-    realm_default_twenty_four_hour_time_elem.val("true");
-    realm_default_twenty_four_hour_time_elem.attr("id", "id_realm_default_twenty_four_hour_time");
-    realm_default_twenty_four_hour_time_elem.data = () => "boolean";
 
     subsection_elem = $(`#org-${CSS.escape(subsection)}`);
     subsection_elem.closest = () => subsection_elem;
-    subsection_elem.set_find_results(".prop-element", [
-        realm_default_language_elem,
-        realm_default_twenty_four_hour_time_elem,
-    ]);
+    subsection_elem.set_find_results(".prop-element", [realm_default_language_elem]);
 
     submit_form(ev);
     assert.ok(patched);
 
     expected_value = {
         default_language: "en",
-        default_twenty_four_hour_time: "true",
     };
     assert.deepEqual(data, expected_value);
 

static/js/admin.js

@@ -24,7 +24,6 @@ const admin_settings_label = {
     realm_signup_notifications_stream: $t({defaultMessage: "New user notifications:"}),
     realm_inline_image_preview: $t({defaultMessage: "Show previews of uploaded and linked images"}),
     realm_inline_url_embed_preview: $t({defaultMessage: "Show previews of linked websites"}),
-    realm_default_twenty_four_hour_time: $t({defaultMessage: "Time format"}),
     realm_send_welcome_emails: $t({defaultMessage: "Send emails introducing Zulip to new users"}),
     realm_message_content_allowed_in_email_notifications: $t({
         defaultMessage: "Allow message content in message notification emails",
@@ -88,7 +87,6 @@ export function build_page() {
         server_inline_image_preview: page_params.server_inline_image_preview,
         realm_inline_url_embed_preview: page_params.realm_inline_url_embed_preview,
         server_inline_url_embed_preview: page_params.server_inline_url_embed_preview,
-        realm_default_twenty_four_hour_time_values: settings_config.twenty_four_hour_time_values,
         realm_authentication_methods: page_params.realm_authentication_methods,
         realm_user_group_edit_policy: page_params.realm_user_group_edit_policy,
         realm_name_changes_disabled: page_params.realm_name_changes_disabled,
@@ -154,6 +152,7 @@ export function build_page() {
             realm_user_settings_defaults.enable_stream_audible_notifications,
         email_notifications_batching_period_values:
             settings_config.email_notifications_batching_period_values,
+        twenty_four_hour_time_values: settings_config.twenty_four_hour_time_values,
     };
 
     if (options.realm_logo_source !== "D" && options.realm_night_logo_source === "D") {
@@ -182,13 +181,6 @@ export function build_page() {
 
     $("#id_realm_default_language").val(page_params.realm_default_language);
     $("#id_realm_digest_weekday").val(options.realm_digest_weekday);
-
-    // default_twenty_four_hour time is a boolean in the API but a
-    // dropdown, so we need to convert the value to a string for
-    // storage in the browser's DOM.
-    $("#id_realm_default_twenty_four_hour_time").val(
-        JSON.stringify(page_params.realm_default_twenty_four_hour_time),
-    );
 }
 
 export function launch(section) {

static/js/server_events_dispatch.js

@@ -193,7 +193,6 @@ export function dispatch_normal_event(event) {
                 invite_to_stream_policy: noop,
                 default_code_block_language: noop,
                 default_language: noop,
-                default_twenty_four_hour_time: noop,
                 description: noop,
                 digest_emails_enabled: noop,
                 digest_weekday: noop,
@@ -397,6 +396,7 @@ export function dispatch_normal_event(event) {
                 "left_side_userlist",
                 "translate_emoticons",
                 "starred_message_counts",
+                "twenty_four_hour_time",
             ];
 
             const container_elem = $("#realm-user-default-settings");

static/js/settings_display.js

@@ -133,12 +133,12 @@ export function set_up(settings_panel) {
             e.stopPropagation();
             overlays.open_modal(language_modal_elem);
         });
+    }
 
     container.find(".setting_twenty_four_hour_time").on("change", function () {
         const data = {twenty_four_hour_time: this.value};
         change_display_setting(data, settings_panel, ".time-settings-status");
     });
-    }
 
     container.find(".setting_demote_inactive_streams").on("change", function () {
         const data = {demote_inactive_streams: this.value};

static/js/settings_org.js

@@ -179,10 +179,6 @@ function get_property_value(property_name) {
         return "no_restriction";
     }
 
-    if (property_name === "realm_default_twenty_four_hour_time") {
-        return JSON.stringify(page_params[property_name]);
-    }
-
     return page_params[property_name];
 }
 
@@ -866,13 +862,6 @@ export function build_page() {
                 data = {};
                 data.authentication_methods = JSON.stringify(get_auth_method_table_data());
                 break;
-            case "user_defaults": {
-                const realm_default_twenty_four_hour_time = $(
-                    "#id_realm_default_twenty_four_hour_time",
-                ).val();
-                data.default_twenty_four_hour_time = realm_default_twenty_four_hour_time;
-                break;
-            }
         }
         return data;
     }

static/templates/settings/display_settings.hbs

@@ -58,7 +58,6 @@
         </div>
     </div>
 
-    {{#unless for_realm_settings}}
     <div class="time-settings">
         <h3 class="inline-block">{{t "Time settings" }}</h3>
         <div class="alert-notification time-settings-status"></div>
@@ -72,7 +71,6 @@
             </select>
         </div>
     </div>
-    {{/unless}}
 
     <div class="emoji-settings">
         <h3 class="inline-block light">{{t "Emoji settings" }}</h3>

static/templates/settings/organization_settings_admin.hbs

@@ -65,14 +65,6 @@
                         {{/each}}
                     </select>
                 </div>
-                <div class="input-group">
-                    <label for="realm_default_twenty_four_hour_time" class="dropdown-title">{{ admin_settings_label.realm_default_twenty_four_hour_time }}</label>
-                    <select name="realm_default_twenty_four_hour_time" class ="setting-widget prop-element" id="id_realm_default_twenty_four_hour_time" data-setting-widget-type="string">
-                        {{#each realm_default_twenty_four_hour_time_values}}
-                        <option value='{{ this.value }}'>{{ this.description }}</option>
-                        {{/each}}
-                    </select>
-                </div>
             </div>
         </div>
 

templates/zerver/api/changelog.md

@@ -11,6 +11,18 @@ below features are supported.
 
 ## Changes in Zulip 5.0
 
+**Feature level 99**
+
+* [`PATCH /realm/user_settings_defaults`](/api/update-realm-user-settings-defaults),
+  `PATCH /realm`: The `default_twenty_four_hour_time` parameter to
+  `PATCH /realm` has been replaced by the `twenty_four_hour_time` parameter
+  to `PATCH /realm/user_settings_defaults`, to match the new model for
+  user preference defaults settings.
+
+* [`POST /register`](/api/register-queue): Removed `realm_default_twenty_four_hour_time`
+  from the response object. This value is now available in the
+  `twenty_four_hour_time` field of the `realm_user_settings_default` object.
+
 **Feature level 98**
 
 * [`POST /subscribe`](/api/subscribe): Added `is_web_public` parameter

version.py

@@ -33,7 +33,7 @@ DESKTOP_WARNING_VERSION = "5.4.3"
 # Changes should be accompanied by documentation explaining what the
 # new level means in templates/zerver/api/changelog.md, as well as
 # "**Changes**" entries in the endpoint's documentation in `zulip.yaml`.
-API_FEATURE_LEVEL = 98
+API_FEATURE_LEVEL = 99
 
 # Bump the minor PROVISION_VERSION to indicate that folks should provision
 # only when going from an old version of the code to a newer version. Bump

zerver/lib/create_user.py

@@ -29,9 +29,7 @@ def copy_default_settings(
     #
     # Note that this function will do at least one save() on target_profile.
     for settings_name in UserBaseSettings.property_types:
-        if settings_name in ["default_language", "twenty_four_hour_time"] and isinstance(
-            settings_source, RealmUserDefault
-        ):
+        if settings_name == "default_language" and isinstance(settings_source, RealmUserDefault):
             continue
         value = getattr(settings_source, settings_name)
         setattr(target_profile, settings_name, value)
@@ -110,7 +108,6 @@ def create_user_profile(
         enter_sends=enter_sends,
         onboarding_steps=orjson.dumps([]).decode(),
         default_language=realm.default_language,
-        twenty_four_hour_time=realm.default_twenty_four_hour_time,
         delivery_email=email,
         **extra_kwargs,
     )

zerver/lib/event_schema.py

@@ -920,7 +920,7 @@ def check_realm_default_update(
     _check_realm_default_update(var_name, event)
 
     assert prop == event["property"]
-    assert prop not in ["default_language", "twenty_four_hour_time"]
+    assert prop != "default_language"
     assert prop in RealmUserDefault.property_types
 
     prop_type = RealmUserDefault.property_types[prop]

zerver/migrations/0352_migrate_twenty_four_hour_time_to_realmuserdefault.py

@@ -0,0 +1,47 @@
+# Generated by Django 3.2.6 on 2021-09-17 17:08
+
+from django.db import migrations
+from django.db.backends.postgresql.schema import DatabaseSchemaEditor
+from django.db.migrations.state import StateApps
+from django.db.models import F
+
+
+def migrate_twenty_four_hour_time_to_realmuserdefault(
+    apps: StateApps, schema_editor: DatabaseSchemaEditor
+) -> None:
+    RealmUserDefault = apps.get_model("zerver", "RealmUserDefault")
+    realm_user_default_objects = RealmUserDefault.objects.exclude(
+        twenty_four_hour_time=F("realm__default_twenty_four_hour_time")
+    )
+    for realm_user_default in realm_user_default_objects:
+        realm = realm_user_default.realm
+        realm_user_default.twenty_four_hour_time = realm.default_twenty_four_hour_time
+        realm_user_default.save(update_fields=["twenty_four_hour_time"])
+
+
+def reverse_migrate_twenty_four_hour_time_to_realmuserdefault(
+    apps: StateApps, schema_editor: DatabaseSchemaEditor
+) -> None:
+    RealmUserDefault = apps.get_model("zerver", "RealmUserDefault")
+    realm_user_default_objects = RealmUserDefault.objects.exclude(
+        realm__default_twenty_four_hour_time=F("twenty_four_hour_time")
+    )
+    for realm_user_default in realm_user_default_objects:
+        realm = realm_user_default.realm
+        realm.default_twenty_four_hour_time = realm_user_default.twenty_four_hour_time
+        realm.save(update_fields=["default_twenty_four_hour_time"])
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("zerver", "0351_user_topic_visibility_indexes"),
+    ]
+
+    operations = [
+        migrations.RunPython(
+            migrate_twenty_four_hour_time_to_realmuserdefault,
+            reverse_code=reverse_migrate_twenty_four_hour_time_to_realmuserdefault,
+            elidable=True,
+        ),
+    ]

zerver/migrations/0353_remove_realm_default_twenty_four_hour_time.py

@@ -0,0 +1,17 @@
+# Generated by Django 3.2.6 on 2021-09-17 18:25
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("zerver", "0352_migrate_twenty_four_hour_time_to_realmuserdefault"),
+    ]
+
+    operations = [
+        migrations.RemoveField(
+            model_name="realm",
+            name="default_twenty_four_hour_time",
+        ),
+    ]

zerver/models.py

@@ -387,7 +387,6 @@ class Realm(models.Model):
     allow_edit_history: bool = models.BooleanField(default=True)
 
     # Defaults for new users
-    default_twenty_four_hour_time: bool = models.BooleanField(default=False)
     default_language: str = models.CharField(default="en", max_length=MAX_LANGUAGE_ID_LENGTH)
 
     DEFAULT_NOTIFICATION_STREAM_NAME = "general"
@@ -606,7 +605,6 @@ class Realm(models.Model):
         invite_to_stream_policy=int,
         move_messages_between_streams_policy=int,
         default_language=str,
-        default_twenty_four_hour_time=bool,
         description=str,
         digest_emails_enabled=bool,
         disallow_disposable_email_addresses=bool,

zerver/openapi/zulip.yaml

@@ -3894,6 +3894,9 @@ paths:
 
                                 [new-user-defaults]: /help/configure-default-new-user-settings
 
+                                See [PATCH /realm/user_settings_defaults](/api/update-realm-user-settings-defaults)
+                                for details on possible properties.
+
                                 **Changes**: New in Zulip 5.0 (feature level 95).
                               properties:
                                 id:
@@ -7832,6 +7835,17 @@ paths:
           schema:
             type: boolean
           example: true
+        - name: twenty_four_hour_time
+          in: query
+          description: |
+            Whether time should be [displayed in 24-hour notation](/help/change-the-time-format).
+
+            **Changes**: New in Zulip 5.0 (feature level 99).
+            Previously, this default was edited using the
+            `default_twenty_four_hour_time` parameter to the `PATCH /realm` endpoint.
+          schema:
+            type: boolean
+          example: true
       responses:
         "200":
           description: Success
@@ -10373,13 +10387,6 @@ paths:
                           Present if `realm` is present in `fetch_event_types`.
 
                           The default UI language for new users joining this organization.
-                      realm_default_twenty_four_hour_time:
-                        type: boolean
-                        description: |
-                          Present if `realm` is present in `fetch_event_types`.
-
-                          Whether new members of this organization will see times displayed in
-                          24-hour time (true) or 12-hour time (false).
                       realm_description:
                         type: string
                         description: |
@@ -11071,6 +11078,12 @@ paths:
                             type: boolean
                             description: |
                               Whether time should be [displayed in 24-hour notation](/help/change-the-time-format).
+
+                              **Changes**: New in Zulip 5.0 (feature level 99).
+                              This value was previously available as
+                              `realm_default_twenty_four_hour_time` in
+                              the top-level response object (only when `realm` was
+                              present in `fetch_event_types`).
                           dense_mode:
                             type: boolean
                             description: |

zerver/tests/test_events.py

@@ -2265,7 +2265,7 @@ class RealmPropertyActionTest(BaseAction):
 
     def test_change_realm_user_default_setting(self) -> None:
         for prop in RealmUserDefault.property_types:
-            if prop in ["default_language", "twenty_four_hour_time"]:
+            if prop == "default_language":
                 continue
             self.do_set_realm_user_default_setting_test(prop)
 

zerver/tests/test_home.py

@@ -119,7 +119,6 @@ class HomeTest(ZulipTestCase):
         "realm_default_language",
         "realm_default_stream_groups",
         "realm_default_streams",
-        "realm_default_twenty_four_hour_time",
         "realm_description",
         "realm_digest_emails_enabled",
         "realm_digest_weekday",

zerver/tests/test_realm.py

@@ -910,10 +910,10 @@ class RealmAPITest(ZulipTestCase):
         for prop in RealmUserDefault.property_types:
             # enable_marketing_emails setting is not actually used and thus cannot be updated
             # using this endpoint. It is included in notification_setting_types only for avoiding
-            # duplicate code. default_language and twenty_four_hour_time are currently present
-            # in Realm table also and thus are updated using '/realm' endpoint, but those
-            # will be removed in future and the settings in RealmUserDefault table will be used.
-            if prop in ["default_language", "twenty_four_hour_time", "enable_marketing_emails"]:
+            # duplicate code. default_language is currently present in Realm table also and thus
+            # is updated using '/realm' endpoint, but this will be removed in future and the
+            # settings in RealmUserDefault table will be used.
+            if prop in ["default_language", "enable_marketing_emails"]:
                 continue
             self.do_test_realm_default_setting_update_api(prop)
 

zerver/tests/test_signup.py

@@ -44,6 +44,7 @@ from zerver.lib.actions import (
     do_get_user_invites,
     do_invite_users,
     do_set_realm_property,
+    do_set_realm_user_default_setting,
     get_default_streams_for_realm,
 )
 from zerver.lib.email_notifications import enqueue_welcome_emails, followup_day2_email_delay
@@ -89,6 +90,7 @@ from zerver.models import (
     PreregistrationUser,
     Realm,
     RealmAuditLog,
+    RealmUserDefault,
     Recipient,
     ScheduledEmail,
     Stream,
@@ -3714,7 +3716,10 @@ class UserSignUpTest(InviteUserBase):
         email = self.nonreg_email("newguy")
         password = "newpassword"
         realm = get_realm("zulip")
-        do_set_realm_property(realm, "default_twenty_four_hour_time", True, acting_user=None)
+        realm_user_default = RealmUserDefault.objects.get(realm=realm)
+        do_set_realm_user_default_setting(
+            realm_user_default, "twenty_four_hour_time", True, acting_user=None
+        )
 
         result = self.client_post("/accounts/home/", {"email": email})
         self.assertEqual(result.status_code, 302)
@@ -3731,7 +3736,10 @@ class UserSignUpTest(InviteUserBase):
         self.assertEqual(result.status_code, 302)
 
         user_profile = self.nonreg_user("newguy")
-        self.assertEqual(user_profile.twenty_four_hour_time, realm.default_twenty_four_hour_time)
+        realm_user_default = RealmUserDefault.objects.get(realm=realm)
+        self.assertEqual(
+            user_profile.twenty_four_hour_time, realm_user_default.twenty_four_hour_time
+        )
 
     def test_signup_already_active(self) -> None:
         """

zerver/views/realm.py

@@ -118,7 +118,6 @@ def update_realm(
     email_address_visibility: Optional[int] = REQ(
         json_validator=check_int_in(Realm.EMAIL_ADDRESS_VISIBILITY_TYPES), default=None
     ),
-    default_twenty_four_hour_time: Optional[bool] = REQ(json_validator=check_bool, default=None),
     video_chat_provider: Optional[int] = REQ(json_validator=check_int, default=None),
     giphy_rating: Optional[int] = REQ(json_validator=check_int, default=None),
     default_code_block_language: Optional[str] = REQ(default=None),
@@ -350,6 +349,7 @@ def update_realm_user_settings_defaults(
     email_notifications_batching_period_seconds: Optional[int] = REQ(
         json_validator=check_int, default=None
     ),
+    twenty_four_hour_time: Optional[bool] = REQ(json_validator=check_bool, default=None),
 ) -> HttpResponse:
     if notification_sound is not None or email_notifications_batching_period_seconds is not None:
         check_settings_values(notification_sound, email_notifications_batching_period_seconds)