linkifier: Support reordering linkifiers.

This adds API support to reorder linkifiers and makes sure that the
returned lists of linkifiers from `GET /events`, `POST /register`, and
`GET /realm/linkifiers` are always sorted with the order that they
should processed when rendering linkifiers.

We set the new `order` field to the ID with the migration. This
preserves the order of the existing linkifiers.

New linkifiers added will always be ordered the last. When reordering,
the `order` field of all linkifiers in the same realm is updated, in
a manner similar to how we implement ordering for
`custom_profile_fields`.

api_docs/changelog.md

@@ -20,6 +20,11 @@ format used by the Zulip server that they are interacting with.
 
 ## Changes in Zulip 8.0
 
+**Feature level 202**
+
+* [`PATCH /realm/linkifiers`](/api/reorder-linkifiers): Added new endpoint
+  to support changing the order in which linkifiers will be processed.
+
 **Feature level 201**
 
 * [`POST /zulip-outgoing-webhook`]: Renamed the notification trigger

api_docs/include/rest-endpoints.md

@@ -92,6 +92,7 @@
 * [Add a linkifier](/api/add-linkifier)
 * [Update a linkifier](/api/update-linkifier)
 * [Remove a linkifier](/api/remove-linkifier)
+* [Reorder linkifiers](/api/reorder-linkifiers)
 * [Add a code playground](/api/add-code-playground)
 * [Remove a code playground](/api/remove-code-playground)
 * [Get all custom emoji](/api/get-custom-emoji)

version.py

@@ -33,7 +33,7 @@ DESKTOP_WARNING_VERSION = "5.9.3"
 # Changes should be accompanied by documentation explaining what the
 # new level means in api_docs/changelog.md, as well as "**Changes**"
 # entries in the endpoint's documentation in `zulip.yaml`.
-API_FEATURE_LEVEL = 201
+API_FEATURE_LEVEL = 202
 
 # 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/actions/realm_linkifiers.py

@@ -2,8 +2,11 @@ from typing import Dict, List, Optional
 
 import orjson
 from django.db import transaction
+from django.db.models import Max
 from django.utils.timezone import now as timezone_now
+from django.utils.translation import gettext as _
 
+from zerver.lib.exceptions import JsonableError
 from zerver.lib.types import LinkifierDict
 from zerver.models import (
     Realm,
@@ -11,6 +14,7 @@ from zerver.models import (
     RealmFilter,
     UserProfile,
     active_user_ids,
+    flush_linkifiers,
     linkifiers_for_realm,
 )
 from zerver.tornado.django_api import send_event_on_commit
@@ -35,7 +39,15 @@ def do_add_linkifier(
 ) -> int:
     pattern = pattern.strip()
     url_template = url_template.strip()
+    # This makes sure that the new linkifier is always ordered the last modulo
+    # the rare race condition.
+    max_order = RealmFilter.objects.aggregate(Max("order"))["order__max"]
+    if max_order is None:
         linkifier = RealmFilter(realm=realm, pattern=pattern, url_template=url_template)
+    else:
+        linkifier = RealmFilter(
+            realm=realm, pattern=pattern, url_template=url_template, order=max_order + 1
+        )
     linkifier.full_clean()
     linkifier.save()
 
@@ -131,3 +143,54 @@ def do_update_linkifier(
     )
 
     notify_linkifiers(realm, realm_linkifiers)
+
+
+@transaction.atomic(durable=True)
+def check_reorder_linkifiers(
+    realm: Realm, ordered_linkifier_ids: List[int], *, acting_user: Optional[UserProfile]
+) -> None:
+    """ordered_linkifier_ids should contain ids of all existing linkifiers.
+    In the rare situation when any of the linkifier gets deleted that more ids
+    are passed, the checks below are sufficient to detect inconsistencies most of
+    the time."""
+    # Repeated IDs in the user request would collapse into the same key when
+    # constructing the set.
+    linkifier_id_set = set(ordered_linkifier_ids)
+    if len(linkifier_id_set) < len(ordered_linkifier_ids):
+        raise JsonableError(_("The ordered list must not contain duplicated linkifiers"))
+
+    linkifiers = RealmFilter.objects.filter(realm=realm)
+    if {linkifier.id for linkifier in linkifiers} != linkifier_id_set:
+        raise JsonableError(
+            _("The ordered list must enumerate all existing linkifiers exactly once")
+        )
+
+    # After the validation, we are sure that there is nothing to do. Return
+    # early to avoid flushing the cache and populating the audit logs.
+    if len(linkifiers) == 0:
+        return
+
+    id_to_new_order = {
+        linkifier_id: order for order, linkifier_id in enumerate(ordered_linkifier_ids)
+    }
+
+    for linkifier in linkifiers:
+        assert linkifier.id in id_to_new_order
+        linkifier.order = id_to_new_order[linkifier.id]
+    RealmFilter.objects.bulk_update(linkifiers, fields=["order"])
+    flush_linkifiers(instance=linkifiers[0])
+
+    # This roundtrip re-fetches the linkifiers sorted in the new order.
+    realm_linkifiers = linkifiers_for_realm(realm.id)
+    RealmAuditLog.objects.create(
+        realm=realm,
+        acting_user=acting_user,
+        event_type=RealmAuditLog.REALM_LINKIFIERS_REORDERED,
+        event_time=timezone_now(),
+        extra_data=orjson.dumps(
+            {
+                "realm_linkifiers": realm_linkifiers,
+            }
+        ).decode(),
+    )
+    notify_linkifiers(realm, realm_linkifiers)

zerver/migrations/0466_realmfilter_order.py

@@ -0,0 +1,28 @@
+# Generated by Django 4.2.3 on 2023-08-09 18:48
+
+from django.db import migrations, models
+from django.db.backends.base.schema import BaseDatabaseSchemaEditor
+from django.db.migrations.state import StateApps
+from django.db.models import F
+
+
+def migrate_set_order_value(apps: StateApps, schema_editor: BaseDatabaseSchemaEditor) -> None:
+    RealmFilter = apps.get_model("zerver", "RealmFilter")
+    RealmFilter.objects.all().update(order=F("id"))
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("zerver", "0465_backfill_scheduledmessagenotificationemail_trigger"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="realmfilter",
+            name="order",
+            field=models.IntegerField(default=0),
+        ),
+        migrations.RunPython(
+            migrate_set_order_value, reverse_code=migrations.RunPython.noop, elidable=True
+        ),
+    ]

zerver/models.py

@@ -1246,6 +1246,9 @@ class RealmFilter(models.Model):
     realm = models.ForeignKey(Realm, on_delete=CASCADE)
     pattern = models.TextField()
     url_template = models.TextField(validators=[url_template_validator])
+    # Linkifiers are applied in a message/topic in order; the processing order
+    # is important when there are overlapping patterns.
+    order = models.IntegerField(default=0)
 
     class Meta:
         unique_together = ("realm", "pattern")
@@ -1309,7 +1312,7 @@ def linkifiers_for_realm(realm_id: int) -> List[LinkifierDict]:
             url_template=linkifier.url_template,
             id=linkifier.id,
         )
-        for linkifier in RealmFilter.objects.filter(realm_id=realm_id).order_by("id")
+        for linkifier in RealmFilter.objects.filter(realm_id=realm_id).order_by("order")
     ]
 
 
@@ -4499,6 +4502,7 @@ class AbstractRealmAuditLog(models.Model):
     REALM_LINKIFIER_REMOVED = 225
     REALM_EMOJI_ADDED = 226
     REALM_EMOJI_REMOVED = 227
+    REALM_LINKIFIERS_REORDERED = 228
 
     SUBSCRIPTION_CREATED = 301
     SUBSCRIPTION_ACTIVATED = 302

zerver/openapi/python_examples.py

@@ -392,6 +392,18 @@ def get_realm_linkifiers(client: Client) -> None:
     validate_against_openapi_schema(result, "/realm/linkifiers", "get", "200")
 
 
+@openapi_test_function("/realm/linkifiers:patch")
+def reorder_realm_linkifiers(client: Client) -> None:
+    # {code_example|start}
+    # Reorder the linkifiers in the user's organization.
+    order = [4, 3, 2, 1]
+    request = {"ordered_linkifier_ids": json.dumps(order)}
+
+    result = client.call_endpoint(url="/realm/linkifiers", method="PATCH", request=request)
+    # {code_example|end}
+    validate_against_openapi_schema(result, "/realm/linkifiers", "patch", "200")
+
+
 @openapi_test_function("/realm/profile_fields:get")
 def get_realm_profile_fields(client: Client) -> None:
     # {code_example|start}
@@ -1629,6 +1641,7 @@ def test_server_organizations(client: Client) -> None:
     update_realm_filter(client)
     add_realm_playground(client)
     get_server_settings(client)
+    reorder_realm_linkifiers(client)
     remove_realm_filter(client)
     remove_realm_playground(client)
     get_realm_emoji(client)

zerver/openapi/zulip.yaml

@@ -3205,8 +3205,16 @@ paths:
                                 realm_linkifiers:
                                   type: array
                                   description: |
-                                    Array of dictionaries where each dictionary contains details about
-                                    a single realm linkifier.
+                                    An ordered array of dictionaries where each
+                                    dictionary contains details about a single
+                                    realm linkifier.
+
+                                    The order of the array reflects the order
+                                    that each linkifier should be processed when
+                                    linkifying messages and topics. By default,
+                                    new linkifiers are ordered last. This order
+                                    can be modified with [`PATCH
+                                    /realm/linkifiers`](/api/reorder-linkifiers).
                                   items:
                                     type: object
                                     additionalProperties: false
@@ -10606,7 +10614,14 @@ paths:
                       linkifiers:
                         type: array
                         description: |
-                          An array of objects, where each object describes a linkifier.
+                          An ordered array of objects, where each object
+                          describes a linkifier.
+
+                          The order of the array reflects the order that each
+                          linkifier should be processed when linkifying messages
+                          and topics. By default, new linkifiers are ordered
+                          last. This order can be modified with [`PATCH
+                          /realm/linkifiers`](/api/reorder-linkifiers).
                         items:
                           type: object
                           additionalProperties: false
@@ -10641,6 +10656,35 @@ paths:
                           ],
                         "result": "success",
                       }
+    patch:
+      operationId: reorder-linkifiers
+      summary: Reorder all linkifiers
+      tags: ["server_and_organizations"]
+      description: |
+        Change the order that the regular expression patterns in the organization's
+        [linkifiers](/help/add-a-custom-linkifier) are matched in messages and topics.
+        Useful when defining linkifiers with overlapping patterns.
+
+        **Changes**: New in Zulip 8.0 (feature level 202). Before this feature level,
+        linkifiers were always processed in order by ID, which meant users would
+        need to delete and recreate them to reorder the list of linkifiers.
+      parameters:
+        - name: ordered_linkifier_ids
+          in: query
+          description: |
+            A list of the IDs of all the linkifiers defined in this
+            organization, in the desired new order.
+          content:
+            application/json:
+              schema:
+                type: array
+                items:
+                  type: integer
+              example: [3, 2, 1, 5]
+          required: true
+      responses:
+        "200":
+          $ref: "#/components/responses/SimpleSuccess"
   /realm/filters:
     post:
       operationId: add-linkifier
@@ -11382,9 +11426,15 @@ paths:
                         description: |
                           Present if `realm_linkifiers` is present in `fetch_event_types`.
 
-                          Array of objects where each object describes a single
+                          An ordered array of objects where each object describes a single
                           [linkifier](/help/add-a-custom-linkifier).
 
+                          The order of the array reflects the order that each
+                          linkifier should be processed when linkifying messages
+                          and topics. By default, new linkifiers are ordered
+                          last. This order can be modified with [`PATCH
+                          /realm/linkifiers`](/api/reorder-linkifiers).
+
                           Clients will receive an empty array unless the event queue is
                           registered with the client capability `{"linkifier_url_template": true}`.
                           See [`client_capabilities`](/api/register-queue#parameter-client_capabilities)

zerver/tests/test_events.py

@@ -62,6 +62,7 @@ from zerver.actions.realm_domains import (
 from zerver.actions.realm_emoji import check_add_realm_emoji, do_remove_realm_emoji
 from zerver.actions.realm_icon import do_change_icon_source
 from zerver.actions.realm_linkifiers import (
+    check_reorder_linkifiers,
     do_add_linkifier,
     do_remove_linkifier,
     do_update_linkifier,
@@ -2179,6 +2180,17 @@ class NormalActionsTest(BaseAction):
         )
         check_realm_linkifiers("events[0]", events[0])
 
+        linkifier_ids = list(
+            RealmFilter.objects.all().values_list("id", flat=True).order_by("order")
+        )
+        events = self.verify_action(
+            lambda: check_reorder_linkifiers(
+                self.user_profile.realm, [linkifier_ids[-1], *linkifier_ids[:-1]], acting_user=None
+            ),
+            num_events=1,
+        )
+        check_realm_linkifiers("events[0]", events[0])
+
         events = self.verify_action(
             lambda: do_remove_linkifier(self.user_profile.realm, regex, acting_user=None),
             num_events=1,

zerver/tests/test_markdown.py

@@ -1398,21 +1398,25 @@ class MarkdownTest(ZulipTestCase):
                     realm=realm,
                     pattern="http",
                     url_template="http://example.com/",
+                    order=1,
                 ),
                 RealmFilter(
                     realm=realm,
                     pattern="b#(?P<id>[a-z]+)",
                     url_template="http://example.com/b/{id}",
+                    order=2,
                 ),
                 RealmFilter(
                     realm=realm,
                     pattern="a#(?P<aid>[a-z]+) b#(?P<bid>[a-z]+)",
                     url_template="http://example.com/a/{aid}/b/{bid}",
+                    order=3,
                 ),
                 RealmFilter(
                     realm=realm,
                     pattern="a#(?P<id>[a-z]+)",
                     url_template="http://example.com/a/{id}",
+                    order=4,
                 ),
             ],
             [
@@ -1454,20 +1458,22 @@ class MarkdownTest(ZulipTestCase):
     def test_linkifier_precedence(self) -> None:
         realm = self.example_user("hamlet").realm
         RealmFilter.objects.filter(realm=realm).delete()
-        # The insertion order should not affect the fact that the linkifiers are ordered by id.
-        # Note that we might later switch to a different field to order the linkifiers.
-        sequence = (10, 3, 11, 2, 4, 5, 6)
-        for cur_precedence in sequence:
+        # The insertion order should not affect the fact that the linkifiers are
+        # ordered by the `order` field.
+        order_values = (10, 3, 11, 2, 4, 5, 6)
+        order_to_id = {}
+        for cur_order in order_values:
             linkifier = RealmFilter(
-                id=cur_precedence,
                 realm=realm,
-                pattern=f"abc{cur_precedence}",
+                pattern=f"abc{cur_order}",
                 url_template="http://foo.com",
+                order=cur_order,
             )
             linkifier.save()
+            order_to_id[cur_order] = linkifier.id
         linkifiers = linkifiers_for_realm(realm.id)
-        for index, cur_precedence in enumerate(sorted(sequence)):
-            self.assertEqual(linkifiers[index]["id"], cur_precedence)
+        for index, cur_order in enumerate(sorted(order_values)):
+            self.assertEqual(linkifiers[index]["id"], order_to_id[cur_order])
 
     def test_realm_patterns_negative(self) -> None:
         realm = get_realm("zulip")

zerver/tests/test_realm_linkifiers.py

@@ -1,9 +1,12 @@
 import re
+from typing import List
 
+import orjson
 from django.core.exceptions import ValidationError
 
 from zerver.lib.test_classes import ZulipTestCase
-from zerver.models import RealmFilter, url_template_validator
+from zerver.lib.utils import assert_is_not_none
+from zerver.models import RealmAuditLog, RealmFilter, url_template_validator
 
 
 class RealmFilterTest(ZulipTestCase):
@@ -265,3 +268,118 @@ class RealmFilterTest(ZulipTestCase):
         for url in invalid_urls:
             with self.assertRaises(ValidationError):
                 url_template_validator(url)
+
+    def test_reorder_linkifiers(self) -> None:
+        iago = self.example_user("iago")
+        self.login("iago")
+
+        def assert_linkifier_audit_logs(expected_id_order: List[int]) -> None:
+            """Check if the audit log created orders the linkifiers correctly"""
+            extra_data = orjson.loads(
+                assert_is_not_none(
+                    RealmAuditLog.objects.filter(
+                        acting_user=iago,
+                        event_type=RealmAuditLog.REALM_LINKIFIERS_REORDERED,
+                    )
+                    .latest("event_time")
+                    .extra_data
+                )
+            )
+            audit_logged_ids = [
+                linkifier_dict["id"] for linkifier_dict in extra_data["realm_linkifiers"]
+            ]
+            self.assertListEqual(expected_id_order, audit_logged_ids)
+
+        def assert_linkifier_order(expected_id_order: List[int]) -> None:
+            """Verify that the realm audit log created matches the expected ordering"""
+            result = self.client_get("/json/realm/linkifiers")
+            actual_id_order = [
+                linkifier["id"] for linkifier in self.assert_json_success(result)["linkifiers"]
+            ]
+            self.assertListEqual(expected_id_order, actual_id_order)
+
+        def reorder_verify_succeed(expected_id_order: List[int]) -> None:
+            """Send a reorder request and verify that it succeeds"""
+            result = self.client_patch(
+                "/json/realm/linkifiers",
+                {"ordered_linkifier_ids": orjson.dumps(expected_id_order).decode()},
+            )
+            self.assert_json_success(result)
+
+        reorder_verify_succeed([])
+        self.assertEqual(
+            RealmAuditLog.objects.filter(
+                realm=iago.realm, event_type=RealmAuditLog.REALM_LINKIFIERS_REORDERED
+            ).count(),
+            0,
+        )
+
+        linkifiers = [
+            {
+                "pattern": "1#(?P<id>[123])",
+                "url_template": "https://filter.com/foo/{id}",
+            },
+            {
+                "pattern": "2#(?P<id>[123])",
+                "url_template": "https://filter.com/bar/{id}",
+            },
+            {
+                "pattern": "3#(?P<id>[123])",
+                "url_template": "https://filter.com/baz/{id}",
+            },
+        ]
+        original_id_order = []
+        for linkifier in linkifiers:
+            result = self.client_post("/json/realm/filters", linkifier)
+            original_id_order.append(self.assert_json_success(result)["id"])
+        assert_linkifier_order(original_id_order)
+        self.assertListEqual([0, 1, 2], list(RealmFilter.objects.values_list("order", flat=True)))
+
+        # The creation order orders the linkifiers by default.
+        # When the order values are the same, fallback to order by ID.
+        RealmFilter.objects.all().update(order=0)
+        assert_linkifier_order(original_id_order)
+
+        # This should successfully reorder the linkifiers.
+        new_order = [original_id_order[2], original_id_order[1], original_id_order[0]]
+        reorder_verify_succeed(new_order)
+        assert_linkifier_audit_logs(new_order)
+        assert_linkifier_order(new_order)
+
+        # After reordering, newly created linkifier is ordered at the last, and
+        # the other linkifiers are unchanged.
+        result = self.client_post(
+            "/json/realm/filters", {"pattern": "3#123", "url_template": "https://example.com"}
+        )
+        new_linkifier_id = self.assert_json_success(result)["id"]
+        new_order = [*new_order, new_linkifier_id]
+        assert_linkifier_order(new_order)
+
+        # Deleting a linkifier should preserve the order.
+        deleted_linkifier_id = new_order[2]
+        result = self.client_delete(f"/json/realm/filters/{deleted_linkifier_id}")
+        self.assert_json_success(result)
+        new_order = [*new_order[:2], new_linkifier_id]
+        assert_linkifier_order(new_order)
+
+        # Extra non-existent ids are ignored.
+        new_order = [new_order[2], new_order[0], new_order[1]]
+        result = self.client_patch(
+            "/json/realm/linkifiers", {"ordered_linkifier_ids": [deleted_linkifier_id, *new_order]}
+        )
+        self.assert_json_error(
+            result, "The ordered list must enumerate all existing linkifiers exactly once"
+        )
+
+        # Duplicated IDs are not allowed.
+        new_order = [*new_order, new_order[0]]
+        result = self.client_patch("/json/realm/linkifiers", {"ordered_linkifier_ids": new_order})
+        self.assert_json_error(result, "The ordered list must not contain duplicated linkifiers")
+
+        # Incomplete lists of linkifiers are not allowed.
+        result = self.client_patch(
+            "/json/realm/linkifiers", {"ordered_linkifier_ids": new_order[:2]}
+        )
+        self.assert_json_error(
+            result, "The ordered list must enumerate all existing linkifiers exactly once"
+        )

zerver/views/realm_linkifiers.py

@@ -1,8 +1,11 @@
+from typing import List
+
 from django.core.exceptions import ValidationError
 from django.http import HttpRequest, HttpResponse
 from django.utils.translation import gettext as _
 
 from zerver.actions.realm_linkifiers import (
+    check_reorder_linkifiers,
     do_add_linkifier,
     do_remove_linkifier,
     do_update_linkifier,
@@ -11,6 +14,7 @@ from zerver.decorator import require_realm_admin
 from zerver.lib.exceptions import JsonableError, ValidationFailureError
 from zerver.lib.request import REQ, has_request_variables
 from zerver.lib.response import json_success
+from zerver.lib.validator import check_int, check_list
 from zerver.models import RealmFilter, UserProfile, linkifiers_for_realm
 
 
@@ -73,3 +77,14 @@ def update_linkifier(
         raise JsonableError(_("Linkifier not found."))
     except ValidationError as e:
         raise ValidationFailureError(e)
+
+
+@require_realm_admin
+@has_request_variables
+def reorder_linkifiers(
+    request: HttpRequest,
+    user_profile: UserProfile,
+    ordered_linkifier_ids: List[int] = REQ(json_validator=check_list(check_int)),
+) -> HttpResponse:
+    check_reorder_linkifiers(user_profile.realm, ordered_linkifier_ids, acting_user=user_profile)
+    return json_success(request)

zproject/urls.py

@@ -114,6 +114,7 @@ from zerver.views.realm_linkifiers import (
     create_linkifier,
     delete_linkifier,
     list_linkifiers,
+    reorder_linkifiers,
     update_linkifier,
 )
 from zerver.views.realm_logo import delete_logo_backend, get_logo_backend, upload_logo
@@ -270,7 +271,7 @@ v1_api_and_json_patterns = [
     # realm/logo -> zerver.views.realm_logo
     rest_path("realm/logo", POST=upload_logo, DELETE=delete_logo_backend, GET=get_logo_backend),
     # realm/filters and realm/linkifiers -> zerver.views.realm_linkifiers
-    rest_path("realm/linkifiers", GET=list_linkifiers),
+    rest_path("realm/linkifiers", GET=list_linkifiers, PATCH=reorder_linkifiers),
     rest_path("realm/filters", POST=create_linkifier),
     rest_path("realm/filters/<int:filter_id>", DELETE=delete_linkifier, PATCH=update_linkifier),
     # realm/playgrounds -> zerver.views.realm_playgrounds