presence: Rewrite the backend data model.

This implements the core of the rewrite described in:

For the backend data model for UserPresence to one that supports much
more efficient queries and is more correct around handling of multiple
clients.  The main loss of functionality is that we no longer track
which Client sent presence data (so we will no longer be able to say
using UserPresence "the user was last online on their desktop 15
minutes ago, but was online with their phone 3 minutes ago").  If we
consider that information important for the occasional investigation
query, we have can construct that answer data via UserActivity
already.  It's not worth making Presence much more expensive/complex
to support it.

For slim_presence clients, this sends the same data format we sent
before, albeit with less complexity involved in constructing it.  Note
that we at present will always send both last_active_time and
last_connected_time; we may revisit that in the future.

This commit doesn't include the finalizing migration, which drops the
UserPresenceOld table.
The way to deploy is to start the backfill migration with the server
down and then start the server *without* the user_presence queue worker,
to let the migration finish without having new data interfering with it.
Once the migration is done, the queue worker can be started, leading to
the presence data catching up to the current state as the queue worker
goes over the queued up events and updating the UserPresence table.

Co-authored-by: Mateusz Mandera <mateusz.mandera@zulip.com>

zerver/actions/message_send.py

@@ -720,27 +720,15 @@ def filter_presence_idle_user_ids(user_ids: Set[int]) -> List[int]:
     # currently idle and should potentially get email notifications
     # (and push notifications with with
     # user_profile.enable_online_push_notifications=False).
-    #
-    # We exclude any presence data from ZulipMobile for the purpose of
-    # triggering these notifications; the mobile app can more
-    # effectively do its own client-side filtering of notification
-    # sounds/etc. for the case that the user is actively doing a PM
-    # conversation in the app.
 
     if not user_ids:
         return []
 
     recent = timezone_now() - datetime.timedelta(seconds=settings.OFFLINE_THRESHOLD_SECS)
-    rows = (
-        UserPresence.objects.filter(
-            user_profile_id__in=user_ids,
-            status=UserPresence.ACTIVE,
-            timestamp__gte=recent,
-        )
-        .exclude(client__name="ZulipMobile")
-        .distinct("user_profile_id")
-        .values("user_profile_id")
-    )
+    rows = UserPresence.objects.filter(
+        user_profile_id__in=user_ids,
+        last_active_time__gte=recent,
+    ).values("user_profile_id")
     active_user_ids = {row["user_profile_id"] for row in rows}
     idle_user_ids = user_ids - active_user_ids
     return sorted(idle_user_ids)

zerver/actions/presence.py

@@ -5,6 +5,7 @@ from django.conf import settings
 from django.db import transaction
 
 from zerver.actions.user_activity import update_user_activity_interval
+from zerver.lib.presence import format_legacy_presence_dict
 from zerver.lib.queue import queue_json_publish
 from zerver.lib.timestamp import datetime_to_timestamp
 from zerver.models import Client, UserPresence, UserProfile, active_user_ids, get_client
@@ -41,7 +42,10 @@ def send_presence_changed(
         # stop sending them at all.
         return
 
-    presence_dict = presence.to_dict()
+    # The mobile app handles these events so we need to use the old format.
+    # The format of the event should also account for the slim_presence
+    # API parameter when this becomes possible in the future.
+    presence_dict = format_legacy_presence_dict(presence)
     event = dict(
         type="presence",
         email=user_profile.email,
@@ -75,39 +79,80 @@ def do_update_user_presence(
 ) -> None:
     client = consolidate_client(client)
 
+    # TODO: While we probably DO want creating an account to
+    # automatically create a first `UserPresence` object with
+    # last_connected_time and last_active_time as the current time,
+    # our presence tests don't understand this, and it'd be perhaps
+    # wrong for some cases of account creation via the API.  So we may
+    # want a "never" value here as the default.
     defaults = dict(
-        timestamp=log_time,
-        status=status,
+        # Given that these are defaults for creation of a UserPresence row
+        # if one doesn't yet exist, the most sensible way to do this
+        # is to set both last_active_time and last_connected_time
+        # to log_time.
+        last_active_time=log_time,
+        last_connected_time=log_time,
         realm_id=user_profile.realm_id,
     )
+    if status == UserPresence.LEGACY_STATUS_IDLE_INT:
+        # If the presence entry for the user is just to be created, and
+        # we want it to be created as idle, then there needs to be an appropriate
+        # offset between last_active_time and last_connected_time, since that's
+        # what the event-sending system calculates the status based on, via
+        # format_legacy_presence_dict.
+        defaults["last_active_time"] = log_time - datetime.timedelta(
+            seconds=settings.PRESENCE_LEGACY_EVENT_OFFSET_FOR_ACTIVITY_SECONDS + 1
+        )
 
     (presence, created) = UserPresence.objects.get_or_create(
         user_profile=user_profile,
-        client=client,
         defaults=defaults,
     )
 
-    stale_status = (log_time - presence.timestamp) > datetime.timedelta(minutes=1, seconds=10)
-    was_idle = presence.status == UserPresence.IDLE
-    became_online = (status == UserPresence.ACTIVE) and (stale_status or was_idle)
+    time_since_last_active = log_time - presence.last_active_time
 
-    # If an object was created, it has already been saved.
-    #
-    # We suppress changes from ACTIVE to IDLE before stale_status is reached;
-    # this protects us from the user having two clients open: one active, the
-    # other idle. Without this check, we would constantly toggle their status
-    # between the two states.
-    if not created and stale_status or was_idle or status == presence.status:
-        # The following block attempts to only update the "status"
-        # field in the event that it actually changed.  This is
-        # important to avoid flushing the UserPresence cache when the
-        # data it would return to a client hasn't actually changed
-        # (see the UserPresence post_save hook for details).
-        presence.timestamp = log_time
-        update_fields = ["timestamp"]
-        if presence.status != status:
-            presence.status = status
-            update_fields.append("status")
+    assert (3 * settings.PRESENCE_PING_INTERVAL_SECS + 20) <= settings.OFFLINE_THRESHOLD_SECS
+    now_online = time_since_last_active > datetime.timedelta(
+        # Here, we decide whether the user is newly online, and we need to consider
+        # sending an immediate presence update via the events system that this user is now online,
+        # rather than waiting for other clients to poll the presence update.
+        # Sending these presence update events adds load to the system, so we only want to do this
+        # if the user has missed a couple regular presence checkins
+        # (so their state is at least 2 * PRESENCE_PING_INTERVAL_SECS + 10 old),
+        # and also is under the risk of being shown by clients as offline before the next regular presence checkin
+        # (so at least `settings.OFFLINE_THRESHOLD_SECS - settings.PRESENCE_PING_INTERVAL_SECS - 10`).
+        # These two values happen to be the same in the default configuration.
+        seconds=settings.OFFLINE_THRESHOLD_SECS
+        - settings.PRESENCE_PING_INTERVAL_SECS
+        - 10
+    )
+    became_online = status == UserPresence.LEGACY_STATUS_ACTIVE_INT and now_online
+
+    update_fields = []
+
+    # This check is to prevent updating `last_connected_time` several
+    # times per minute with multiple connected browser windows.
+    # We also need to be careful not to wrongly "update" the timestamp if we actually already
+    # have newer presence than the reported log_time.
+    if not created and log_time - presence.last_connected_time > datetime.timedelta(
+        seconds=settings.PRESENCE_UPDATE_MIN_FREQ_SECONDS
+    ):
+        presence.last_connected_time = log_time
+        update_fields.append("last_connected_time")
+    if (
+        not created
+        and status == UserPresence.LEGACY_STATUS_ACTIVE_INT
+        and time_since_last_active
+        > datetime.timedelta(seconds=settings.PRESENCE_UPDATE_MIN_FREQ_SECONDS)
+    ):
+        presence.last_active_time = log_time
+        update_fields.append("last_active_time")
+        if log_time > presence.last_connected_time:
+            # Update last_connected_time as well to ensure
+            # last_connected_time >= last_active_time.
+            presence.last_connected_time = log_time
+            update_fields.append("last_connected_time")
+    if len(update_fields) > 0:
         presence.save(update_fields=update_fields)
 
     if force_send_update or (

zerver/actions/user_settings.py

@@ -536,7 +536,7 @@ def do_change_user_setting(
         # setting; not doing so can make it look like the settings
         # change didn't have any effect.
         if setting_value:
-            status = UserPresence.ACTIVE
+            status = UserPresence.LEGACY_STATUS_ACTIVE_INT
             presence_time = timezone_now()
         else:
             # HACK: Remove existing presence data for the current user
@@ -553,7 +553,7 @@ def do_change_user_setting(
             #
             # We add a small additional offset as a fudge factor in
             # case of clock skew.
-            status = UserPresence.IDLE
+            status = UserPresence.LEGACY_STATUS_IDLE_INT
             presence_time = timezone_now() - datetime.timedelta(
                 seconds=settings.OFFLINE_THRESHOLD_SECS + 120
             )

zerver/lib/export.py

@@ -293,7 +293,7 @@ DATE_FIELDS: Dict[TableName, List[Field]] = {
     "zerver_useractivityinterval": ["start", "end"],
     "zerver_useractivity": ["last_visit"],
     "zerver_userhotspot": ["timestamp"],
-    "zerver_userpresence": ["timestamp"],
+    "zerver_userpresence": ["last_active_time", "last_connected_time"],
     "zerver_userprofile": ["date_joined", "last_login", "last_reminder"],
     "zerver_userprofile_mirrordummy": ["date_joined", "last_login", "last_reminder"],
     "zerver_userstatus": ["timestamp"],

zerver/lib/import_realm.py

@@ -1243,7 +1243,6 @@ def do_import_realm(import_dir: Path, subdomain: str, processes: int = 1) -> Rea
 
     fix_datetime_fields(data, "zerver_userpresence")
     re_map_foreign_keys(data, "zerver_userpresence", "user_profile", related_table="user_profile")
-    re_map_foreign_keys(data, "zerver_userpresence", "client", related_table="client")
     re_map_foreign_keys(data, "zerver_userpresence", "realm", related_table="realm")
     update_model_ids(UserPresence, data, "user_presence")
     bulk_import_model(data, UserPresence)

zerver/lib/presence.py

@@ -1,9 +1,9 @@
 import datetime
-import itertools
 import time
 from collections import defaultdict
 from typing import Any, Dict, Mapping, Sequence, Set
 
+from django.conf import settings
 from django.utils.timezone import now as timezone_now
 
 from zerver.lib.timestamp import datetime_to_timestamp
@@ -13,16 +13,6 @@ from zerver.models import PushDeviceToken, Realm, UserPresence, UserProfile, que
 def get_presence_dicts_for_rows(
     all_rows: Sequence[Mapping[str, Any]], mobile_user_ids: Set[int], slim_presence: bool
 ) -> Dict[str, Dict[str, Any]]:
-    # Note that datetime values have sub-second granularity, which is
-    # mostly important for avoiding test flakes, but it's also technically
-    # more precise for real users.
-    # We could technically do this sort with the database, but doing it
-    # here prevents us from having to assume the caller is playing nice.
-    all_rows = sorted(
-        all_rows,
-        key=lambda row: (row["user_profile_id"], row["timestamp"]),
-    )
-
     if slim_presence:
         # Stringify user_id here, since it's gonna be turned
         # into a string anyway by JSON, and it keeps mypy happy.
@@ -34,10 +24,11 @@ def get_presence_dicts_for_rows(
 
     user_statuses: Dict[str, Dict[str, Any]] = {}
 
-    for user_key, presence_rows in itertools.groupby(all_rows, get_user_key):
+    for presence_row in all_rows:
+        user_key = get_user_key(presence_row)
         info = get_user_presence_info(
-            list(presence_rows),
-            mobile_user_ids=mobile_user_ids,
+            presence_row["last_active_time"],
+            presence_row["last_connected_time"],
         )
         user_statuses[user_key] = info
 
@@ -45,59 +36,48 @@ def get_presence_dicts_for_rows(
 
 
 def get_modern_user_presence_info(
-    presence_rows: Sequence[Mapping[str, Any]], mobile_user_ids: Set[int]
+    last_active_time: datetime.datetime, last_connected_time: datetime.datetime
 ) -> Dict[str, Any]:
-    active_timestamp = None
-    for row in reversed(presence_rows):
-        if row["status"] == UserPresence.ACTIVE:
-            active_timestamp = datetime_to_timestamp(row["timestamp"])
-            break
-
-    idle_timestamp = None
-    for row in reversed(presence_rows):
-        if row["status"] == UserPresence.IDLE:
-            idle_timestamp = datetime_to_timestamp(row["timestamp"])
-            break
-
-    # Be stingy about bandwidth, and don't even include
-    # keys for entities that have None values.  JS
-    # code should just do a falsy check here.
+    # TODO: Do further bandwidth optimizations to this structure.
     result = {}
-
-    if active_timestamp is not None:
-        result["active_timestamp"] = active_timestamp
-
-    if idle_timestamp is not None:
-        result["idle_timestamp"] = idle_timestamp
-
+    result["active_timestamp"] = datetime_to_timestamp(last_active_time)
+    result["idle_timestamp"] = datetime_to_timestamp(last_connected_time)
     return result
 
 
 def get_legacy_user_presence_info(
-    presence_rows: Sequence[Mapping[str, Any]], mobile_user_ids: Set[int]
+    last_active_time: datetime.datetime, last_connected_time: datetime.datetime
 ) -> Dict[str, Any]:
-    # The format of data here is for legacy users of our API,
-    # including old versions of the mobile app.
-    info_rows = []
-    for row in presence_rows:
-        client_name = row["client__name"]
-        status = UserPresence.status_to_string(row["status"])
-        dt = row["timestamp"]
-        timestamp = datetime_to_timestamp(dt)
-        push_enabled = row["user_profile__enable_offline_push_notifications"]
-        has_push_devices = row["user_profile_id"] in mobile_user_ids
-        pushable = push_enabled and has_push_devices
+    # Reformats the modern UserPresence data structure so that legacy
+    # API clients can still access presence data.
+    #
+    # We expect this code to remain mostly unchanged until we can delete it.
 
-        info = dict(
-            client=client_name,
-            status=status,
-            timestamp=timestamp,
-            pushable=pushable,
-        )
+    if timezone_now() - last_active_time > datetime.timedelta(minutes=2):
+        dt = last_connected_time
+        status = UserPresence.LEGACY_STATUS_IDLE
+    else:
+        dt = last_active_time
+        status = UserPresence.LEGACY_STATUS_ACTIVE
 
-        info_rows.append(info)
+    client_name = "website"
+    timestamp = datetime_to_timestamp(dt)
 
-    most_recent_info = info_rows[-1]
+    # This field was never used by clients of the legacy API, so we
+    # just set it to a fixed value for API format compatibility.
+    pushable = False
+
+    # Now we put things together in the legacy presence format with
+    # one client + an `aggregated` field.
+    #
+    # TODO: Look at whether we can drop to just the "aggregated" field
+    # if no clients look at the rest.
+    most_recent_info = dict(
+        client=client_name,
+        status=status,
+        timestamp=timestamp,
+        pushable=pushable,
+    )
 
     result = {}
 
@@ -109,22 +89,36 @@ def get_legacy_user_presence_info(
         timestamp=most_recent_info["timestamp"],
     )
 
-    # Build a dictionary of client -> info.  There should
-    # only be one row per client, but to be on the safe side,
-    # we always overwrite with rows that are later in our list.
-    for info in info_rows:
-        result[info["client"]] = info
+    result[client_name] = most_recent_info
 
     return result
 
 
+def format_legacy_presence_dict(presence: UserPresence) -> Dict[str, Any]:
+    """
+    This function assumes it's being called right after the presence object was updated,
+    and is not meant to be used on old presence data.
+    """
+    if (
+        presence.last_active_time
+        + datetime.timedelta(seconds=settings.PRESENCE_LEGACY_EVENT_OFFSET_FOR_ACTIVITY_SECONDS)
+        >= presence.last_connected_time
+    ):
+        status = UserPresence.LEGACY_STATUS_ACTIVE
+        timestamp = datetime_to_timestamp(presence.last_active_time)
+    else:
+        status = UserPresence.LEGACY_STATUS_IDLE
+        timestamp = datetime_to_timestamp(presence.last_connected_time)
+
+    return dict(client="website", status=status, timestamp=timestamp, pushable=False)
+
+
 def get_presence_for_user(
     user_profile_id: int, slim_presence: bool = False
 ) -> Dict[str, Dict[str, Any]]:
     query = UserPresence.objects.filter(user_profile_id=user_profile_id).values(
-        "client__name",
-        "status",
-        "timestamp",
+        "last_active_time",
+        "last_connected_time",
         "user_profile__email",
         "user_profile_id",
         "user_profile__enable_offline_push_notifications",
@@ -145,13 +139,12 @@ def get_presence_dict_by_realm(
     two_weeks_ago = timezone_now() - datetime.timedelta(weeks=2)
     query = UserPresence.objects.filter(
         realm_id=realm_id,
-        timestamp__gte=two_weeks_ago,
+        last_connected_time__gte=two_weeks_ago,
         user_profile__is_active=True,
         user_profile__is_bot=False,
     ).values(
-        "client__name",
-        "status",
-        "timestamp",
+        "last_active_time",
+        "last_connected_time",
         "user_profile__email",
         "user_profile_id",
         "user_profile__enable_offline_push_notifications",

zerver/migrations/0443_userpresence_new_table_schema.py

@@ -0,0 +1,139 @@
+from typing import Any, Callable
+
+import django.db.models.deletion
+import django.utils.timezone
+from django.conf import settings
+from django.db import connection, migrations, models
+from django.db.backends.base.schema import BaseDatabaseSchemaEditor
+from django.db.migrations.state import StateApps
+from psycopg2.sql import SQL, Identifier
+
+
+def rename_indexes_constraints(
+    old_table: str, new_table: str
+) -> Callable[[StateApps, BaseDatabaseSchemaEditor], None]:
+    def inner_migration(apps: StateApps, schema_editor: Any) -> None:
+        with connection.cursor() as cursor:
+            constraints = connection.introspection.get_constraints(cursor, old_table)
+            for old_name, infodict in constraints.items():
+                if infodict["check"]:
+                    suffix = "_check"
+                    is_index = False
+                elif infodict["foreign_key"] is not None:
+                    is_index = False
+                    to_table, to_column = infodict["foreign_key"]
+                    suffix = f"_fk_{to_table}_{to_column}"
+                elif infodict["primary_key"]:
+                    suffix = "_pk"
+                    is_index = True
+                elif infodict["unique"]:
+                    suffix = "_uniq"
+                    is_index = True
+                else:
+                    suffix = "_idx" if len(infodict["columns"]) > 1 else ""
+                    is_index = True
+                new_name = schema_editor._create_index_name(new_table, infodict["columns"], suffix)
+                if is_index:
+                    raw_query = SQL("ALTER INDEX {old_name} RENAME TO {new_name}").format(
+                        old_name=Identifier(old_name), new_name=Identifier(new_name)
+                    )
+                else:
+                    raw_query = SQL(
+                        "ALTER TABLE {old_table} RENAME CONSTRAINT {old_name} TO {new_name}"
+                    ).format(
+                        old_table=Identifier(old_table),
+                        old_name=Identifier(old_name),
+                        new_name=Identifier(new_name),
+                    )
+                cursor.execute(raw_query)
+
+            for infodict in connection.introspection.get_sequences(cursor, old_table):
+                old_name = infodict["name"]
+                column = infodict["column"]
+                new_name = f"{new_table}_{column}_seq"
+
+                raw_query = SQL("ALTER SEQUENCE {old_name} RENAME TO {new_name}").format(
+                    old_name=Identifier(old_name),
+                    new_name=Identifier(new_name),
+                )
+                cursor.execute(raw_query)
+
+            cursor.execute(
+                SQL("ALTER TABLE {old_table} RENAME TO {new_table}").format(
+                    old_table=Identifier(old_table), new_table=Identifier(new_table)
+                )
+            )
+
+    return inner_migration
+
+
+class Migration(migrations.Migration):
+    """
+    First step of migrating to a new UserPresence data model. Creates a new
+    table with the intended fields, into which in the next step
+    data can be ported over from the current UserPresence model.
+    In the last step, the old model will be replaced with the new one.
+    """
+
+    dependencies = [
+        ("zerver", "0442_remove_realmfilter_url_format_string"),
+    ]
+
+    operations = [
+        # Django doesn't rename indexes and constraints when renaming
+        # a table (https://code.djangoproject.com/ticket/23577). This
+        # means that after renaming UserPresence->UserPresenceOld the
+        # UserPresenceOld indexes/constraints retain their old name
+        # causing a conflict when CreateModel tries to create them for
+        # the new UserPresence table.
+        migrations.SeparateDatabaseAndState(
+            database_operations=[
+                migrations.RunPython(
+                    rename_indexes_constraints("zerver_userpresence", "zerver_userpresenceold"),
+                    reverse_code=rename_indexes_constraints(
+                        "zerver_userpresenceold", "zerver_userpresence"
+                    ),
+                )
+            ],
+            state_operations=[
+                migrations.RenameModel(
+                    old_name="UserPresence",
+                    new_name="UserPresenceOld",
+                )
+            ],
+        ),
+        migrations.CreateModel(
+            name="UserPresence",
+            fields=[
+                (
+                    "id",
+                    models.AutoField(
+                        auto_created=True, primary_key=True, serialize=False, verbose_name="ID"
+                    ),
+                ),
+                (
+                    "last_connected_time",
+                    models.DateTimeField(db_index=True, default=django.utils.timezone.now),
+                ),
+                (
+                    "last_active_time",
+                    models.DateTimeField(db_index=True, default=django.utils.timezone.now),
+                ),
+                (
+                    "realm",
+                    models.ForeignKey(
+                        on_delete=django.db.models.deletion.CASCADE, to="zerver.Realm"
+                    ),
+                ),
+                (
+                    "user_profile",
+                    models.OneToOneField(
+                        on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL
+                    ),
+                ),
+            ],
+            options={
+                "index_together": {("realm", "last_active_time"), ("realm", "last_connected_time")},
+            },
+        ),
+    ]

zerver/migrations/0444_userpresence_fill_data.py

@@ -0,0 +1,69 @@
+from django.db import connection, migrations
+from django.db.backends.base.schema import BaseDatabaseSchemaEditor
+from django.db.migrations.state import StateApps
+
+
+def fill_new_columns(apps: StateApps, schema_editor: BaseDatabaseSchemaEditor) -> None:
+    UserPresence = apps.get_model("zerver", "UserPresence")
+
+    # In theory, we'd like to preserve the distinction between the
+    # IDLE and ACTIVE statuses in legacy data.  However, there is no
+    # correct way to do so; the previous data structure only stored
+    # the current IDLE/ACTIVE status of the last update for each
+    # (user, client) pair. There's no way to know whether the last
+    # time the user had the other status with that client was minutes
+    # or months beforehand.
+    #
+    # So the only sane thing we can do with this migration is to treat
+    # the last presence update as having been a PRESENCE_ACTIVE_STATUS
+    # event. This will result in some currently-idle users being
+    # incorrectly recorded as having been active at the last moment
+    # that they were idle before this migration.  This error is
+    # unlikely to be significant in practice, and in any case is an
+    # unavoidable flaw caused by the legacy previous data model.
+    with connection.cursor() as cursor:
+        cursor.execute(
+            "SELECT realm_id, user_profile_id, MAX(timestamp) FROM zerver_userpresenceold WHERE status IN (1, 2) GROUP BY realm_id, user_profile_id"
+        )
+        latest_presence_per_user = cursor.fetchall()
+
+    UserPresence.objects.bulk_create(
+        [
+            UserPresence(
+                user_profile_id=presence_row[1],
+                realm_id=presence_row[0],
+                last_connected_time=presence_row[2],
+                last_active_time=presence_row[2],
+            )
+            for presence_row in latest_presence_per_user
+        ],
+        # Limit the size of individual network requests for very large
+        # servers.
+        batch_size=10000,
+        # If the UserPresence worker has already started, or a user
+        # has changed their invisible status while migrations are
+        # running, then some UserPresence rows may exist. Those will
+        # generally be newer than what we have here, so ignoring
+        # conflicts so we can complete backfilling users who don't
+        # have more current data is the right resolution.
+        ignore_conflicts=True,
+    )
+
+
+def clear_new_columns(apps: StateApps, schema_editor: BaseDatabaseSchemaEditor) -> None:
+    UserPresence = apps.get_model("zerver", "UserPresence")
+    UserPresence.objects.all().delete()
+
+
+class Migration(migrations.Migration):
+    """
+    Ports data from the UserPresence model into the new one.
+    """
+
+    atomic = False
+
+    dependencies = [
+        ("zerver", "0443_userpresence_new_table_schema"),
+    ]
+
+    operations = [migrations.RunPython(fill_new_columns, reverse_code=clear_new_columns)]

zerver/models.py

@@ -4111,82 +4111,44 @@ class UserPresence(models.Model):
       https://zulip.readthedocs.io/en/latest/subsystems/presence.html
     """
 
-    user_profile = models.ForeignKey(UserProfile, on_delete=CASCADE)
+    user_profile = models.OneToOneField(UserProfile, on_delete=CASCADE, unique=True)
+
+    # Realm is just here as denormalization to optimize database
+    # queries to fetch all presence data for a given realm.
     realm = models.ForeignKey(Realm, on_delete=CASCADE)
-    client = models.ForeignKey(Client, on_delete=CASCADE)
 
-    # The time we heard this update from the client.
-    timestamp = models.DateTimeField("presence changed")
+    # The last time the user had a client connected to Zulip,
+    # including idle clients where the user hasn't interacted with the
+    # system recently (and thus might be AFK).
+    last_connected_time = models.DateTimeField(default=timezone_now, db_index=True)
+    # The last time a client connected to Zulip reported that the user
+    # was actually present (E.g. via focusing a browser window or
+    # interacting with a computer running the desktop app)
+    last_active_time = models.DateTimeField(default=timezone_now, db_index=True)
 
-    # The user was actively using this Zulip client as of `timestamp` (i.e.,
-    # they had interacted with the client recently).  When the timestamp is
-    # itself recent, this is the green "active" status in the web app.
-    ACTIVE = 1
-
-    # There had been no user activity (keyboard/mouse/etc.) on this client
-    # recently.  So the client was online at the specified time, but it
-    # could be the user's desktop which they were away from.  Displayed as
-    # orange/idle if the timestamp is current.
-    IDLE = 2
-
-    # Information from the client about the user's recent interaction with
-    # that client, as of `timestamp`.  Possible values above.
-    #
-    # There is no "inactive" status, because that is encoded by the
-    # timestamp being old.
-    status = models.PositiveSmallIntegerField(default=ACTIVE)
+    # The following constants are used in the presence API for
+    # communicating whether a user is active (last_active_time recent)
+    # or idle (last_connected_time recent) or offline (neither
+    # recent).  They're no longer part of the data model.
+    LEGACY_STATUS_ACTIVE = "active"
+    LEGACY_STATUS_IDLE = "idle"
+    LEGACY_STATUS_ACTIVE_INT = 1
+    LEGACY_STATUS_IDLE_INT = 2
 
     class Meta:
-        unique_together = ("user_profile", "client")
         index_together = [
-            ("realm", "timestamp"),
+            ("realm", "last_active_time"),
+            ("realm", "last_connected_time"),
         ]
 
-    @staticmethod
-    def status_to_string(status: int) -> str:
-        if status == UserPresence.ACTIVE:
-            return "active"
-        elif status == UserPresence.IDLE:
-            return "idle"
-        else:  # nocoverage # TODO: Add a presence test to cover this.
-            raise ValueError(f"Unknown status: {status}")
-
-    @staticmethod
-    def to_presence_dict(
-        client_name: str,
-        status: int,
-        dt: datetime.datetime,
-        push_enabled: bool = False,
-        has_push_devices: bool = False,
-    ) -> Dict[str, Any]:
-        presence_val = UserPresence.status_to_string(status)
-
-        timestamp = datetime_to_timestamp(dt)
-        return dict(
-            client=client_name,
-            status=presence_val,
-            timestamp=timestamp,
-            pushable=(push_enabled and has_push_devices),
-        )
-
-    def to_dict(self) -> Dict[str, Any]:
-        return UserPresence.to_presence_dict(
-            self.client.name,
-            self.status,
-            self.timestamp,
-        )
-
     @staticmethod
     def status_from_string(status: str) -> Optional[int]:
         if status == "active":
-            # See https://github.com/python/mypy/issues/2611
-            status_val: Optional[int] = UserPresence.ACTIVE
+            return UserPresence.LEGACY_STATUS_ACTIVE_INT
         elif status == "idle":
-            status_val = UserPresence.IDLE
-        else:
-            status_val = None
+            return UserPresence.LEGACY_STATUS_IDLE_INT
 
-        return status_val
+        return None
 
 
 class UserStatus(AbstractEmoji):

zerver/openapi/curl_param_value_generators.py

@@ -241,7 +241,7 @@ def delete_event_queue() -> Dict[str, object]:
 def get_user_presence() -> Dict[str, object]:
     iago = helpers.example_user("iago")
     client = Client.objects.create(name="curl-test-client-3")
-    update_user_presence(iago, client, timezone_now(), UserPresence.ACTIVE, False)
+    update_user_presence(iago, client, timezone_now(), UserPresence.LEGACY_STATUS_ACTIVE_INT, False)
     return {}
 
 

zerver/tests/test_event_system.py

@@ -1401,7 +1401,7 @@ class TestUserPresenceUpdatesDisabled(ZulipTestCase):
                 self.example_user("cordelia"),
                 get_client("website"),
                 timezone_now(),
-                UserPresence.ACTIVE,
+                UserPresence.LEGACY_STATUS_ACTIVE_INT,
                 force_send_update=True,
             )
 
@@ -1410,6 +1410,6 @@ class TestUserPresenceUpdatesDisabled(ZulipTestCase):
                 self.example_user("hamlet"),
                 get_client("website"),
                 timezone_now(),
-                UserPresence.ACTIVE,
+                UserPresence.LEGACY_STATUS_ACTIVE_INT,
                 force_send_update=False,
             )

zerver/tests/test_events.py

@@ -5,6 +5,7 @@
 # and zerver/lib/data_types.py systems for validating the schemas of
 # events; it also uses the OpenAPI tools to validate our documentation.
 import copy
+import datetime
 import time
 from io import StringIO
 from typing import Any, Callable, Dict, List, Optional, Set
@@ -1071,7 +1072,10 @@ class NormalActionsTest(BaseAction):
     def test_presence_events(self) -> None:
         events = self.verify_action(
             lambda: do_update_user_presence(
-                self.user_profile, get_client("website"), timezone_now(), UserPresence.ACTIVE
+                self.user_profile,
+                get_client("website"),
+                timezone_now(),
+                UserPresence.LEGACY_STATUS_ACTIVE_INT,
             ),
             slim_presence=False,
         )
@@ -1089,7 +1093,7 @@ class NormalActionsTest(BaseAction):
                 self.example_user("cordelia"),
                 get_client("website"),
                 timezone_now(),
-                UserPresence.ACTIVE,
+                UserPresence.LEGACY_STATUS_ACTIVE_INT,
             ),
             slim_presence=True,
         )
@@ -1103,6 +1107,15 @@ class NormalActionsTest(BaseAction):
         )
 
     def test_presence_events_multiple_clients(self) -> None:
+        now = timezone_now()
+        initial_presence = now - datetime.timedelta(days=365)
+        UserPresence.objects.create(
+            user_profile=self.user_profile,
+            realm=self.user_profile.realm,
+            last_active_time=initial_presence,
+            last_connected_time=initial_presence,
+        )
+
         self.api_post(
             self.user_profile,
             "/api/v1/users/me/presence",
@@ -1111,12 +1124,28 @@ class NormalActionsTest(BaseAction):
         )
         self.verify_action(
             lambda: do_update_user_presence(
-                self.user_profile, get_client("website"), timezone_now(), UserPresence.ACTIVE
+                self.user_profile,
+                get_client("website"),
+                timezone_now(),
+                UserPresence.LEGACY_STATUS_ACTIVE_INT,
             )
         )
+        self.verify_action(
+            lambda: do_update_user_presence(
+                self.user_profile,
+                get_client("ZulipAndroid/1.0"),
+                timezone_now(),
+                UserPresence.LEGACY_STATUS_IDLE_INT,
+            ),
+            state_change_expected=False,
+            num_events=0,
+        )
         events = self.verify_action(
             lambda: do_update_user_presence(
-                self.user_profile, get_client("ZulipAndroid/1.0"), timezone_now(), UserPresence.IDLE
+                self.user_profile,
+                get_client("ZulipAndroid/1.0"),
+                timezone_now() + datetime.timedelta(seconds=301),
+                UserPresence.LEGACY_STATUS_ACTIVE_INT,
             )
         )
 
@@ -1124,8 +1153,10 @@ class NormalActionsTest(BaseAction):
             "events[0]",
             events[0],
             has_email=True,
-            presence_key="ZulipAndroid/1.0",
-            status="idle",
+            # We no longer store information about the client and we simply
+            # set the field to 'website' for backwards compatibility.
+            presence_key="website",
+            status="active",
         )
 
     def test_register_events(self) -> None:

zerver/tests/test_import_export.py

@@ -786,7 +786,9 @@ class RealmImportExportTest(ExportFile):
 
         client = get_client("website")
 
-        do_update_user_presence(sample_user, client, timezone_now(), UserPresence.ACTIVE)
+        do_update_user_presence(
+            sample_user, client, timezone_now(), UserPresence.LEGACY_STATUS_ACTIVE_INT
+        )
 
         # send Cordelia to the islands
         do_update_user_status(
@@ -1242,7 +1244,11 @@ class RealmImportExportTest(ExportFile):
         def get_userpresence_timestamp(r: Realm) -> Set[object]:
             # It should be sufficient to compare UserPresence timestamps to verify
             # they got exported/imported correctly.
-            return set(UserPresence.objects.filter(realm=r).values_list("timestamp", flat=True))
+            return set(
+                UserPresence.objects.filter(realm=r).values_list(
+                    "last_active_time", "last_connected_time"
+                )
+            )
 
         @getter
         def get_realm_user_default_values(r: Realm) -> Dict[str, object]:
@@ -1730,14 +1736,13 @@ class SingleUserExportTest(ExportFile):
             self.assertEqual(rec["user_profile"], cordelia.id)
             self.assertEqual(make_datetime(rec["start"]), now)
 
-        do_update_user_presence(cordelia, client, now, UserPresence.ACTIVE)
-        do_update_user_presence(othello, client, now, UserPresence.IDLE)
+        do_update_user_presence(cordelia, client, now, UserPresence.LEGACY_STATUS_ACTIVE_INT)
+        do_update_user_presence(othello, client, now, UserPresence.LEGACY_STATUS_IDLE_INT)
 
         @checker
         def zerver_userpresence(records: List[Record]) -> None:
-            self.assertEqual(records[-1]["status"], UserPresence.ACTIVE)
-            self.assertEqual(records[-1]["client"], client.id)
-            self.assertEqual(make_datetime(records[-1]["timestamp"]), now)
+            self.assertEqual(make_datetime(records[-1]["last_connected_time"]), now)
+            self.assertEqual(make_datetime(records[-1]["last_active_time"]), now)
 
         do_update_user_status(
             user_profile=cordelia,

zerver/tests/test_message_edit_notifications.py

@@ -6,7 +6,7 @@ from django.utils.timezone import now as timezone_now
 from zerver.lib.push_notifications import get_apns_badge_count, get_apns_badge_count_future
 from zerver.lib.test_classes import ZulipTestCase
 from zerver.lib.test_helpers import mock_queue_publish
-from zerver.models import Subscription, UserPresence, get_client
+from zerver.models import Subscription, UserPresence
 from zerver.tornado.event_queue import maybe_enqueue_notifications
 
 
@@ -269,12 +269,12 @@ class EditMessageSideEffectsTest(ZulipTestCase):
 
     def _make_cordelia_present_on_web(self) -> None:
         cordelia = self.example_user("cordelia")
+        now = timezone_now()
         UserPresence.objects.create(
             user_profile_id=cordelia.id,
             realm_id=cordelia.realm_id,
-            status=UserPresence.ACTIVE,
-            client=get_client("web"),
-            timestamp=timezone_now(),
+            last_connected_time=now,
+            last_active_time=now,
         )
 
     def test_stream_push_notify_for_fully_present_user(self) -> None:

zerver/tests/test_messages.py

@@ -10,7 +10,6 @@ from zerver.models import (
     UserPresence,
     UserProfile,
     bulk_get_huddle_user_ids,
-    get_client,
     get_huddle_user_ids,
 )
 
@@ -40,11 +39,11 @@ class MissedMessageTest(ZulipTestCase):
 
         def set_presence(user: UserProfile, client_name: str, ago: int) -> None:
             when = timezone_now() - datetime.timedelta(seconds=ago)
-            UserPresence.objects.create(
+            UserPresence.objects.update_or_create(
                 user_profile_id=user.id,
-                realm_id=user.realm_id,
-                client=get_client(client_name),
-                timestamp=when,
+                defaults=dict(
+                    realm_id=user.realm_id, last_active_time=when, last_connected_time=when
+                ),
             )
 
         hamlet_notifications_data.pm_push_notify = True

zerver/tests/test_presence.py

@@ -3,10 +3,11 @@ from datetime import timedelta
 from typing import Any, Dict
 from unittest import mock
 
+from django.conf import settings
 from django.utils.timezone import now as timezone_now
 
 from zerver.actions.users import do_deactivate_user
-from zerver.lib.presence import get_presence_dict_by_realm
+from zerver.lib.presence import format_legacy_presence_dict, get_presence_dict_by_realm
 from zerver.lib.test_classes import ZulipTestCase
 from zerver.lib.test_helpers import make_client, reset_email_visibility_to_everyone_in_zulip_realm
 from zerver.lib.timestamp import datetime_to_timestamp
@@ -16,6 +17,7 @@ from zerver.models import (
     UserActivityInterval,
     UserPresence,
     UserProfile,
+    get_realm,
 )
 
 
@@ -50,11 +52,13 @@ class UserPresenceModelTests(ZulipTestCase):
         presence_dct = get_presence_dict_by_realm(user_profile.realm_id, slim_presence)
         self.assert_length(presence_dct, 1)
         info = presence_dct[str(user_profile.id)]
-        self.assertEqual(set(info.keys()), {"active_timestamp"})
+        self.assertEqual(set(info.keys()), {"active_timestamp", "idle_timestamp"})
 
         def back_date(num_weeks: int) -> None:
-            user_presence = UserPresence.objects.filter(user_profile=user_profile)[0]
-            user_presence.timestamp = timezone_now() - datetime.timedelta(weeks=num_weeks)
+            user_presence = UserPresence.objects.get(user_profile=user_profile)
+            backdated_timestamp = timezone_now() - datetime.timedelta(weeks=num_weeks)
+            user_presence.last_active_time = backdated_timestamp
+            user_presence.last_connected_time = backdated_timestamp
             user_presence.save()
 
         # Simulate the presence being a week old first.  Nothing should change.
@@ -67,7 +71,9 @@ class UserPresenceModelTests(ZulipTestCase):
         presence_dct = get_presence_dict_by_realm(user_profile.realm_id)
         self.assert_length(presence_dct, 0)
 
-    def test_push_tokens(self) -> None:
+    def test_pushable_always_false(self) -> None:
+        # This field was never used by clients of the legacy API, so we
+        # just want to have it always set to False for API format compatibility.
         UserPresence.objects.all().delete()
 
         user_profile = self.example_user("hamlet")
@@ -93,10 +99,30 @@ class UserPresenceModelTests(ZulipTestCase):
             user=user_profile,
             kind=PushDeviceToken.APNS,
         )
-        self.assertTrue(pushable())
+        self.assertFalse(pushable())
 
 
 class UserPresenceTests(ZulipTestCase):
+    def setUp(self) -> None:
+        """
+        Create some initial, old presence data to make the intended set up
+        simpler for the tests.
+        """
+        realm = get_realm("zulip")
+        now = timezone_now()
+        initial_presence = now - timedelta(days=365)
+        UserPresence.objects.bulk_create(
+            [
+                UserPresence(
+                    user_profile=user_profile,
+                    realm=user_profile.realm,
+                    last_active_time=initial_presence,
+                    last_connected_time=initial_presence,
+                )
+                for user_profile in UserProfile.objects.filter(realm=realm)
+            ]
+        )
+
     def test_invalid_presence(self) -> None:
         user = self.example_user("hamlet")
         self.login_user(user)
@@ -131,8 +157,8 @@ class UserPresenceTests(ZulipTestCase):
         hamlet_info = presences[str(hamlet.id)]
         othello_info = presences[str(othello.id)]
 
-        self.assertEqual(set(hamlet_info.keys()), {"idle_timestamp"})
-        self.assertEqual(set(othello_info.keys()), {"idle_timestamp"})
+        self.assertEqual(set(hamlet_info.keys()), {"idle_timestamp", "active_timestamp"})
+        self.assertEqual(set(othello_info.keys()), {"idle_timestamp", "active_timestamp"})
 
         self.assertGreaterEqual(
             othello_info["idle_timestamp"],
@@ -182,12 +208,12 @@ class UserPresenceTests(ZulipTestCase):
 
         self.assertEqual(
             set(othello_info.keys()),
-            {"active_timestamp"},
+            {"active_timestamp", "idle_timestamp"},
         )
 
         self.assertEqual(
             set(hamlet_info.keys()),
-            {"active_timestamp"},
+            {"active_timestamp", "idle_timestamp"},
         )
 
         self.assertGreaterEqual(
@@ -254,17 +280,31 @@ class UserPresenceTests(ZulipTestCase):
 
         self.login("hamlet")
 
-        self.assertEqual(filter_presence_idle_user_ids({user_profile.id}), [user_profile.id])
-        self.client_post("/json/users/me/presence", {"status": "idle"})
+        # Ensure we're starting with a clean slate.
+        UserPresence.objects.all().delete()
         self.assertEqual(filter_presence_idle_user_ids({user_profile.id}), [user_profile.id])
 
-        # Active presence from the mobile app doesn't count
-        self.client_post(
-            "/json/users/me/presence", {"status": "active"}, HTTP_USER_AGENT="ZulipMobile/1.0"
+        # Create a first presence for the user. It's the first one, so it'll initialize both last_active_time
+        # and last_connected time with the current time. Thus the user will be considered active and won't
+        # get filtered.
+        self.client_post("/json/users/me/presence", {"status": "idle"})
+        self.assertEqual(filter_presence_idle_user_ids({user_profile.id}), [])
+
+        # Make last_active_time be older than OFFLINE_THRESHOLD_SECS. That should
+        # get the user filtered.
+        UserPresence.objects.filter(user_profile=user_profile).update(
+            last_active_time=timezone_now() - timedelta(seconds=settings.OFFLINE_THRESHOLD_SECS + 1)
         )
         self.assertEqual(filter_presence_idle_user_ids({user_profile.id}), [user_profile.id])
 
-        self.client_post("/json/users/me/presence", {"status": "active"})
+        # Sending an idle presence doesn't change anything for filtering.
+        self.client_post("/json/users/me/presence", {"status": "idle"})
+        self.assertEqual(filter_presence_idle_user_ids({user_profile.id}), [user_profile.id])
+
+        # Active presence from the mobile app should count (in the old API it didn't)
+        self.client_post(
+            "/json/users/me/presence", {"status": "active"}, HTTP_USER_AGENT="ZulipMobile/1.0"
+        )
         self.assertEqual(filter_presence_idle_user_ids({user_profile.id}), [])
 
     def test_no_mit(self) -> None:
@@ -397,16 +437,12 @@ class SingleUserPresenceTests(ZulipTestCase):
         self.login("hamlet")
         result = self.client_get("/json/users/othello@zulip.com/presence")
         result_dict = self.assert_json_success(result)
-        self.assertEqual(
-            set(result_dict["presence"].keys()), {"ZulipAndroid", "website", "aggregated"}
-        )
+        self.assertEqual(set(result_dict["presence"].keys()), {"website", "aggregated"})
         self.assertEqual(set(result_dict["presence"]["website"].keys()), {"status", "timestamp"})
 
         result = self.client_get(f"/json/users/{othello.id}/presence")
         result_dict = self.assert_json_success(result)
-        self.assertEqual(
-            set(result_dict["presence"].keys()), {"ZulipAndroid", "website", "aggregated"}
-        )
+        self.assertEqual(set(result_dict["presence"].keys()), {"website", "aggregated"})
         self.assertEqual(set(result_dict["presence"]["website"].keys()), {"status", "timestamp"})
 
     def test_ping_only(self) -> None:
@@ -425,6 +461,11 @@ class UserPresenceAggregationTests(ZulipTestCase):
     ) -> Dict[str, Dict[str, Any]]:
         self.login_user(user)
         timezone_util = "zerver.views.presence.timezone_now"
+        # First create some initial, old presence to avoid the details of the edge case of initial
+        # presence creation messing with the intended setup.
+        with mock.patch(timezone_util, return_value=validate_time - datetime.timedelta(days=365)):
+            self.client_post("/json/users/me/presence", {"status": status})
+
         with mock.patch(timezone_util, return_value=validate_time - datetime.timedelta(seconds=5)):
             self.client_post("/json/users/me/presence", {"status": status})
         with mock.patch(timezone_util, return_value=validate_time - datetime.timedelta(seconds=2)):
@@ -446,8 +487,10 @@ class UserPresenceAggregationTests(ZulipTestCase):
             latest_result_dict["presences"][user.email]["aggregated"],
             {
                 "status": status,
-                "timestamp": datetime_to_timestamp(validate_time - datetime.timedelta(seconds=2)),
-                "client": "ZulipAndroid",
+                "timestamp": datetime_to_timestamp(validate_time - datetime.timedelta(seconds=5)),
+                # We no longer store the client information, but keep the field in these dicts,
+                # with the value 'website' for backwards compatibility.
+                "client": "website",
             },
         )
 
@@ -456,11 +499,12 @@ class UserPresenceAggregationTests(ZulipTestCase):
 
     def test_aggregated_info(self) -> None:
         user = self.example_user("othello")
-        validate_time = timezone_now()
+        offset = datetime.timedelta(seconds=settings.PRESENCE_UPDATE_MIN_FREQ_SECONDS + 1)
+        validate_time = timezone_now() - offset
         self._send_presence_for_aggregated_tests(user, "active", validate_time)
         with mock.patch(
             "zerver.views.presence.timezone_now",
-            return_value=validate_time - datetime.timedelta(seconds=1),
+            return_value=validate_time + offset,
         ):
             result = self.api_post(
                 user,
@@ -473,8 +517,8 @@ class UserPresenceAggregationTests(ZulipTestCase):
             result_dict["presences"][user.email]["aggregated"],
             {
                 "status": "active",
-                "timestamp": datetime_to_timestamp(validate_time - datetime.timedelta(seconds=1)),
-                "client": "ZulipTestDev",
+                "timestamp": datetime_to_timestamp(validate_time + offset),
+                "client": "website",  # This fields is no longer used and is permamenently set to 'website'.
             },
         )
 
@@ -486,7 +530,7 @@ class UserPresenceAggregationTests(ZulipTestCase):
             result_dict["presence"]["aggregated"],
             {
                 "status": "active",
-                "timestamp": datetime_to_timestamp(validate_time - datetime.timedelta(seconds=2)),
+                "timestamp": datetime_to_timestamp(validate_time - datetime.timedelta(seconds=5)),
             },
         )
 
@@ -498,7 +542,7 @@ class UserPresenceAggregationTests(ZulipTestCase):
             result_dict["presence"]["aggregated"],
             {
                 "status": "idle",
-                "timestamp": datetime_to_timestamp(validate_time - datetime.timedelta(seconds=2)),
+                "timestamp": datetime_to_timestamp(validate_time - datetime.timedelta(seconds=5)),
             },
         )
 
@@ -506,22 +550,24 @@ class UserPresenceAggregationTests(ZulipTestCase):
         user = self.example_user("othello")
         self.login_user(user)
         validate_time = timezone_now()
+        self._send_presence_for_aggregated_tests(user, "idle", validate_time)
         with mock.patch(
             "zerver.views.presence.timezone_now",
             return_value=validate_time - datetime.timedelta(seconds=3),
         ):
-            self.api_post(
+            result_dict = self.api_post(
                 user,
                 "/api/v1/users/me/presence",
                 {"status": "active"},
                 HTTP_USER_AGENT="ZulipTestDev/1.0",
-            )
-        result_dict = self._send_presence_for_aggregated_tests(user, "idle", validate_time)
+            ).json()
+
         self.assertDictEqual(
-            result_dict["presence"]["aggregated"],
+            result_dict["presences"][user.email]["aggregated"],
             {
-                "status": "idle",
-                "timestamp": datetime_to_timestamp(validate_time - datetime.timedelta(seconds=2)),
+                "client": "website",
+                "status": "active",
+                "timestamp": datetime_to_timestamp(validate_time - datetime.timedelta(seconds=3)),
             },
         )
 
@@ -529,13 +575,22 @@ class UserPresenceAggregationTests(ZulipTestCase):
         user = self.example_user("othello")
         self.login_user(user)
         validate_time = timezone_now()
-        with self.settings(OFFLINE_THRESHOLD_SECS=1):
-            result_dict = self._send_presence_for_aggregated_tests(user, "idle", validate_time)
+        result_dict = self._send_presence_for_aggregated_tests(user, "idle", validate_time)
+
+        with mock.patch(
+            "zerver.views.presence.timezone_now",
+            return_value=validate_time + datetime.timedelta(settings.OFFLINE_THRESHOLD_SECS + 1),
+        ):
+            # After settings.OFFLINE_THRESHOLD_SECS + 1 this generated, recent presence data
+            # will count as offline.
+            result = self.client_get(f"/json/users/{user.email}/presence")
+        result_dict = self.assert_json_success(result)
+
         self.assertDictEqual(
             result_dict["presence"]["aggregated"],
             {
                 "status": "offline",
-                "timestamp": datetime_to_timestamp(validate_time - datetime.timedelta(seconds=2)),
+                "timestamp": datetime_to_timestamp(validate_time - datetime.timedelta(seconds=5)),
             },
         )
 
@@ -612,3 +667,55 @@ class GetRealmStatusesTest(ZulipTestCase):
         )
         json = self.assert_json_success(result)
         self.assertEqual(set(json["presences"].keys()), {str(hamlet.id)})
+
+
+class FormatLegacyPresenceDictTest(ZulipTestCase):
+    def test_format_legacy_presence_dict(self) -> None:
+        hamlet = self.example_user("hamlet")
+        now = timezone_now()
+        recently = now - timedelta(seconds=50)
+        a_while_ago = now - timedelta(minutes=3)
+        presence = UserPresence(
+            user_profile=hamlet, realm=hamlet.realm, last_active_time=now, last_connected_time=now
+        )
+        self.assertEqual(
+            format_legacy_presence_dict(presence),
+            dict(
+                client="website",
+                status=UserPresence.LEGACY_STATUS_ACTIVE,
+                timestamp=datetime_to_timestamp(now),
+                pushable=False,
+            ),
+        )
+
+        presence = UserPresence(
+            user_profile=hamlet,
+            realm=hamlet.realm,
+            last_active_time=recently,
+            last_connected_time=now,
+        )
+        self.assertEqual(
+            format_legacy_presence_dict(presence),
+            dict(
+                client="website",
+                status=UserPresence.LEGACY_STATUS_ACTIVE,
+                timestamp=datetime_to_timestamp(recently),
+                pushable=False,
+            ),
+        )
+
+        presence = UserPresence(
+            user_profile=hamlet,
+            realm=hamlet.realm,
+            last_active_time=a_while_ago,
+            last_connected_time=now,
+        )
+        self.assertEqual(
+            format_legacy_presence_dict(presence),
+            dict(
+                client="website",
+                status=UserPresence.LEGACY_STATUS_IDLE,
+                timestamp=datetime_to_timestamp(now),
+                pushable=False,
+            ),
+        )

zilencer/management/commands/populate_db.py

@@ -793,17 +793,11 @@ class Command(BaseCommand):
         if not options["test_suite"]:
             # Populate users with some bar data
             for user in user_profiles:
-                status: int = UserPresence.ACTIVE
                 date = timezone_now()
-                client = get_client("website")
-                if user.full_name[0] <= "H":
-                    client = get_client("ZulipAndroid")
                 UserPresence.objects.get_or_create(
                     user_profile=user,
                     realm_id=user.realm_id,
-                    client=client,
-                    timestamp=date,
-                    status=status,
+                    defaults={"last_active_time": date, "last_connected_time": date},
                 )
 
         user_profiles_ids = [user_profile.id for user_profile in user_profiles]

zproject/default_settings.py

@@ -506,6 +506,18 @@ PRESENCE_PING_INTERVAL_SECS = 60
 # disabled.
 USER_LIMIT_FOR_SENDING_PRESENCE_UPDATE_EVENTS = 100
 
+# Controls the how much newer a user presence update needs to be
+# than the currently saved last_active_time or last_connected_time in order for us to
+# update the database state. E.g. If set to 0, we will do
+# a database write each time a client sends a presence update.
+PRESENCE_UPDATE_MIN_FREQ_SECONDS = 55
+
+# Controls the timedelta between last_connected_time and last_active_time
+# within which the user should be considered ACTIVE for the purposes of
+# legacy presence events. That is - when sending a presence update about a user to clients,
+# we will specify ACTIVE status  as long as the timedelta is within this limit and IDLE otherwise.
+PRESENCE_LEGACY_EVENT_OFFSET_FOR_ACTIVITY_SECONDS = 70
+
 # How many days deleted messages data should be kept before being
 # permanently deleted.
 ARCHIVED_DATA_VACUUMING_DELAY_DAYS = 30