From 62e17c987fb78b3b57fa0a5b3eaefccd614ee07e Mon Sep 17 00:00:00 2001
From: Lauryn Menard <lauryn@zulip.com>
Date: Mon, 29 Apr 2024 15:50:57 +0200
Subject: [PATCH] exceptions: Add IncompatibleParametersError as a
 JsonableError.

Creates an IncompatibleParametersError to be used in cases where
there are two (or more) optional parameters for an endpoint that
are incompatible with each other, e.g. there's a parameter for a
user name and a user ID but only one should be sent in the request
to identify the user.

Documents the error on the /api/rest-error-handling article.

Updates the PATCH users/me/subscriptions/muted_topics endpoint to
use this error when both the stream and stream_id parameters are
passed (note this endpoint is currently deprecated).
---
 zerver/lib/exceptions.py         | 12 ++++++++++++
 zerver/lib/streams.py            |  3 ++-
 zerver/openapi/zulip.yaml        | 26 ++++++++++++++++++++++++++
 zerver/tests/test_user_topics.py |  4 ++--
 4 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/zerver/lib/exceptions.py b/zerver/lib/exceptions.py
index e9e04f2bc1..45d4189522 100644
--- a/zerver/lib/exceptions.py
+++ b/zerver/lib/exceptions.py
@@ -190,6 +190,18 @@ class StreamWithIDDoesNotExistError(JsonableError):
         return _("Channel with ID '{stream_id}' does not exist")
 
 
+class IncompatibleParametersError(JsonableError):
+    data_fields = ["parameters"]
+
+    def __init__(self, parameters: List[str]) -> None:
+        self.parameters = ", ".join(parameters)
+
+    @staticmethod
+    @override
+    def msg_format() -> str:
+        return _("Unsupported parameter combination: {parameters}")
+
+
 class CannotDeactivateLastUserError(JsonableError):
     code = ErrorCode.CANNOT_DEACTIVATE_LAST_USER
     data_fields = ["is_last_owner", "entity"]
diff --git a/zerver/lib/streams.py b/zerver/lib/streams.py
index f6801e406a..6857651105 100644
--- a/zerver/lib/streams.py
+++ b/zerver/lib/streams.py
@@ -7,6 +7,7 @@ from django.utils.translation import gettext as _
 
 from zerver.lib.default_streams import get_default_stream_ids_for_realm
 from zerver.lib.exceptions import (
+    IncompatibleParametersError,
     JsonableError,
     OrganizationAdministratorRequiredError,
     OrganizationOwnerRequiredError,
@@ -326,7 +327,7 @@ def check_for_exactly_one_stream_arg(stream_id: Optional[int], stream: Optional[
         raise JsonableError(error)
 
     if stream_id is not None and stream is not None:
-        raise JsonableError(_("Please supply only one channel parameter: name or ID."))
+        raise IncompatibleParametersError(["stream_id", "stream"])
 
 
 def check_stream_access_for_delete_or_update(
diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml
index a068105857..b5bf537a55 100644
--- a/zerver/openapi/zulip.yaml
+++ b/zerver/openapi/zulip.yaml
@@ -18862,6 +18862,7 @@ paths:
                 oneOf:
                   - $ref: "#/components/schemas/InvalidApiKeyError"
                   - $ref: "#/components/schemas/MissingArgumentError"
+                  - $ref: "#/components/schemas/IncompatibleParametersError"
                   - $ref: "#/components/schemas/UserNotAuthorizedError"
         "401":
           description: |
@@ -21395,6 +21396,31 @@ components:
               "result": "error",
               "var_name": "content",
             }
+    IncompatibleParametersError:
+      allOf:
+        - $ref: "#/components/schemas/CodedErrorBase"
+        - additionalProperties: false
+          description: |
+            ## Incompatible request parameters
+
+            A typical failed JSON response for when two or more, optional
+            parameters are supplied that are incompatible with each other.
+          properties:
+            result: {}
+            msg: {}
+            code: {}
+            parameters:
+              type: string
+              description: |
+                A string containing the parameters, separated by commas,
+                that are incompatible.
+          example:
+            {
+              "code": "BAD_REQUEST",
+              "msg": "Unsupported parameter combination: object_id, object_name",
+              "result": "error",
+              "parameters": "object_id, object_name",
+            }
     UserNotAuthorizedError:
       allOf:
         - $ref: "#/components/schemas/CodedError"
diff --git a/zerver/tests/test_user_topics.py b/zerver/tests/test_user_topics.py
index 08f3144f42..e8a8114f2d 100644
--- a/zerver/tests/test_user_topics.py
+++ b/zerver/tests/test_user_topics.py
@@ -198,7 +198,7 @@ class MutedTopicsTestsDeprecated(ZulipTestCase):
 
         data = {"stream": stream.name, "stream_id": stream.id, "topic": "Verona3", "op": "add"}
         result = self.api_patch(user, url, data)
-        self.assert_json_error(result, "Please supply only one channel parameter: name or ID.")
+        self.assert_json_error(result, "Unsupported parameter combination: stream_id, stream")
 
         data = {"stream_id": stream.id, "topic": "a" * (MAX_TOPIC_NAME_LENGTH + 1), "op": "add"}
         result = self.api_patch(user, url, data)
@@ -238,7 +238,7 @@ class MutedTopicsTestsDeprecated(ZulipTestCase):
 
         data = {"stream": stream.name, "stream_id": stream.id, "topic": "Verona3", "op": "remove"}
         result = self.api_patch(user, url, data)
-        self.assert_json_error(result, "Please supply only one channel parameter: name or ID.")
+        self.assert_json_error(result, "Unsupported parameter combination: stream_id, stream")
 
         data = {"stream_id": stream.id, "topic": "a" * (MAX_TOPIC_NAME_LENGTH + 1), "op": "remove"}
         result = self.api_patch(user, url, data)