corporate: Fix string encoding in billing and sponsorship endpoints.

corporate/tests/test_stripe.py

@@ -422,8 +422,6 @@ class StripeTestCase(ZulipTestCase):
         for key in del_args:
             if key in params:
                 del params[key]
-        for key, value in params.items():
-            params[key] = orjson.dumps(value).decode()
         return self.client_post("/json/billing/upgrade", params, **host_args)
 
     # Upgrade without talking to Stripe
@@ -1287,18 +1285,29 @@ class StripeTest(StripeTestCase):
     def test_check_upgrade_parameters(self) -> None:
         # Tests all the error paths except 'not enough licenses'
         def check_error(
-            error_description: str, upgrade_params: Mapping[str, Any], del_args: Sequence[str] = []
+            error_message: str,
+            error_description: str,
+            upgrade_params: Mapping[str, Any],
+            del_args: Sequence[str] = [],
         ) -> None:
             response = self.upgrade(talk_to_stripe=False, del_args=del_args, **upgrade_params)
-            self.assert_json_error_contains(response, "Something went wrong. Please contact")
-            self.assertEqual(orjson.loads(response.content)["error_description"], error_description)
+            self.assert_json_error_contains(response, error_message)
+            if error_description:
+                self.assertEqual(
+                    orjson.loads(response.content)["error_description"], error_description
+                )
 
         hamlet = self.example_user("hamlet")
         self.login_user(hamlet)
-        check_error("unknown billing_modality", {"billing_modality": "invalid"})
-        check_error("unknown schedule", {"schedule": "invalid"})
-        check_error("unknown license_management", {"license_management": "invalid"})
-        check_error("autopay with no card", {}, del_args=["stripe_token"])
+        check_error("Invalid billing_modality", "", {"billing_modality": "invalid"})
+        check_error("Invalid schedule", "", {"schedule": "invalid"})
+        check_error("Invalid license_management", "", {"license_management": "invalid"})
+        check_error(
+            "Something went wrong. Please contact",
+            "autopay with no card",
+            {},
+            del_args=["stripe_token"],
+        )
 
     def test_upgrade_license_counts(self) -> None:
         def check_min_licenses_error(
@@ -1401,11 +1410,9 @@ class StripeTest(StripeTestCase):
         self.login_user(user)
 
         data = {
-            "organization-type": orjson.dumps("Open-source").decode(),
-            "website": orjson.dumps("https://infinispan.org/").decode(),
-            "description": orjson.dumps(
-                "Infinispan is a distributed in-memory key/value data store with optional schema."
-            ).decode(),
+            "organization-type": "Open-source",
+            "website": "https://infinispan.org/",
+            "description": "Infinispan is a distributed in-memory key/value data store with optional schema.",
         }
         response = self.client_post("/json/billing/sponsorship", data)
         self.assert_json_success(response)

corporate/views.py

@@ -46,7 +46,7 @@ from zerver.decorator import (
 from zerver.lib.request import REQ, has_request_variables
 from zerver.lib.response import json_error, json_success
 from zerver.lib.send_email import FromAddress, send_email
-from zerver.lib.validator import check_int, check_string
+from zerver.lib.validator import check_int, check_string_in
 from zerver.models import UserProfile, get_realm
 
 billing_logger = logging.getLogger("corporate.stripe")
@@ -127,14 +127,17 @@ def payment_method_string(stripe_customer: stripe.Customer) -> str:
 def upgrade(
     request: HttpRequest,
     user: UserProfile,
-    billing_modality: str = REQ(json_validator=check_string),
-    schedule: str = REQ(json_validator=check_string),
-    license_management: Optional[str] = REQ(json_validator=check_string, default=None),
+    billing_modality: str = REQ(str_validator=check_string_in(VALID_BILLING_MODALITY_VALUES)),
+    schedule: str = REQ(str_validator=check_string_in(VALID_BILLING_SCHEDULE_VALUES)),
+    signed_seat_count: str = REQ(),
+    salt: str = REQ(),
+    license_management: Optional[str] = REQ(
+        default=None, str_validator=check_string_in(VALID_LICENSE_MANAGEMENT_VALUES)
+    ),
     licenses: Optional[int] = REQ(json_validator=check_int, default=None),
-    stripe_token: Optional[str] = REQ(json_validator=check_string, default=None),
-    signed_seat_count: str = REQ(json_validator=check_string),
-    salt: str = REQ(json_validator=check_string),
+    stripe_token: Optional[str] = REQ(default=None),
 ) -> HttpResponse:
+
     try:
         seat_count = unsign_seat_count(signed_seat_count, salt)
         if billing_modality == "charge_automatically" and license_management == "automatic":
@@ -236,9 +239,9 @@ def initial_upgrade(request: HttpRequest) -> HttpResponse:
 def sponsorship(
     request: HttpRequest,
     user: UserProfile,
-    organization_type: str = REQ("organization-type", json_validator=check_string),
-    website: str = REQ("website", json_validator=check_string),
-    description: str = REQ("description", json_validator=check_string),
+    organization_type: str = REQ("organization-type"),
+    website: str = REQ(),
+    description: str = REQ(),
 ) -> HttpResponse:
     realm = user.realm
 
@@ -388,7 +391,7 @@ def change_plan_status(
 def replace_payment_source(
     request: HttpRequest,
     user: UserProfile,
-    stripe_token: str = REQ("stripe_token", json_validator=check_string),
+    stripe_token: str = REQ(),
 ) -> HttpResponse:
     try:
         do_replace_payment_source(user, stripe_token, pay_invoices=True)

frontend_tests/node_tests/billing.js

@@ -86,11 +86,10 @@ run_test("initialize", (override) => {
     });
 
     create_ajax_request_called = false;
-    function plan_change_ajax(url, form_name, stripe_token, numeric_inputs) {
+    function plan_change_ajax(url, form_name, stripe_token) {
         assert.equal(url, "/json/billing/plan/change");
         assert.equal(form_name, "planchange");
         assert.equal(stripe_token, undefined);
-        assert.deepEqual(numeric_inputs, ["status"]);
         create_ajax_request_called = true;
     }
 

frontend_tests/node_tests/billing_helpers.js

@@ -127,13 +127,13 @@ run_test("create_ajax_request", (override) => {
         assert.equal(url, "/json/billing/upgrade");
 
         assert.equal(Object.keys(data).length, 8);
-        assert.equal(data.stripe_token, '"stripe_token_id"');
-        assert.equal(data.seat_count, '"{{ seat_count }}"');
-        assert.equal(data.signed_seat_count, '"{{ signed_seat_count }}"');
-        assert.equal(data.salt, '"{{ salt }}"');
-        assert.equal(data.billing_modality, '"charge_automatically"');
-        assert.equal(data.schedule, '"monthly"');
-        assert.equal(data.license_management, '"automatic"');
+        assert.equal(data.stripe_token, "stripe_token_id");
+        assert.equal(data.seat_count, "{{ seat_count }}");
+        assert.equal(data.signed_seat_count, "{{ signed_seat_count }}");
+        assert.equal(data.salt, "{{ salt }}");
+        assert.equal(data.billing_modality, "charge_automatically");
+        assert.equal(data.schedule, "monthly");
+        assert.equal(data.license_management, "automatic");
         assert.equal(data.licenses, "");
 
         history.pushState = (state_object, title, path) => {
@@ -174,9 +174,7 @@ run_test("create_ajax_request", (override) => {
         assert.equal(state.free_trial_alert_message_show, 1);
     });
 
-    helpers.create_ajax_request("/json/billing/upgrade", "autopay", {id: "stripe_token_id"}, [
-        "licenses",
-    ]);
+    helpers.create_ajax_request("/json/billing/upgrade", "autopay", {id: "stripe_token_id"});
 });
 
 run_test("format_money", () => {

frontend_tests/node_tests/upgrade.js

@@ -36,30 +36,24 @@ run_test("initialize", (override) => {
     });
 
     let create_ajax_request_form_call_count = 0;
-    helpers.__Rewire__(
-        "create_ajax_request",
-        (url, form_name, stripe_token, numeric_inputs, redirect_to) => {
-            create_ajax_request_form_call_count += 1;
-            if (form_name === "autopay") {
-                assert.equal(url, "/json/billing/upgrade");
-                assert.equal(stripe_token, "stripe_add_card_token");
-                assert.deepEqual(numeric_inputs, ["licenses"]);
-                assert.equal(redirect_to, undefined);
-            } else if (form_name === "invoice") {
-                assert.equal(url, "/json/billing/upgrade");
-                assert.equal(stripe_token, undefined);
-                assert.deepEqual(numeric_inputs, ["licenses"]);
-                assert.equal(redirect_to, undefined);
-            } else if (form_name === "sponsorship") {
-                assert.equal(url, "/json/billing/sponsorship");
-                assert.equal(stripe_token, undefined);
-                assert.equal(numeric_inputs, undefined);
-                assert.equal(redirect_to, "/");
-            } else {
-                throw new Error("Unhandled case");
-            }
-        },
-    );
+    helpers.__Rewire__("create_ajax_request", (url, form_name, stripe_token, redirect_to) => {
+        create_ajax_request_form_call_count += 1;
+        if (form_name === "autopay") {
+            assert.equal(url, "/json/billing/upgrade");
+            assert.equal(stripe_token, "stripe_add_card_token");
+            assert.equal(redirect_to, undefined);
+        } else if (form_name === "invoice") {
+            assert.equal(url, "/json/billing/upgrade");
+            assert.equal(stripe_token, undefined);
+            assert.equal(redirect_to, undefined);
+        } else if (form_name === "sponsorship") {
+            assert.equal(url, "/json/billing/sponsorship");
+            assert.equal(stripe_token, undefined);
+            assert.equal(redirect_to, "/");
+        } else {
+            throw new Error("Unhandled case");
+        }
+    });
 
     const open_func = (config_opts) => {
         assert.equal(config_opts.name, "Zulip");

static/js/billing/billing.js

@@ -30,9 +30,7 @@ export function initialize() {
     });
 
     $("#change-plan-status").on("click", (e) => {
-        helpers.create_ajax_request("/json/billing/plan/change", "planchange", undefined, [
-            "status",
-        ]);
+        helpers.create_ajax_request("/json/billing/plan/change", "planchange");
         e.preventDefault();
     });
 }

static/js/billing/helpers.js

@@ -3,13 +3,7 @@ import $ from "jquery";
 import * as loading from "../loading";
 import {page_params} from "../page_params";
 
-export function create_ajax_request(
-    url,
-    form_name,
-    stripe_token = null,
-    numeric_inputs = [],
-    redirect_to = "/billing",
-) {
+export function create_ajax_request(url, form_name, stripe_token = null, redirect_to = "/billing") {
     const form = $(`#${CSS.escape(form_name)}-form`);
     const form_loading_indicator = `#${CSS.escape(form_name)}_loading_indicator`;
     const form_input_section = `#${CSS.escape(form_name)}-input-section`;
@@ -32,15 +26,11 @@ export function create_ajax_request(
 
     const data = {};
     if (stripe_token) {
-        data.stripe_token = JSON.stringify(stripe_token.id);
+        data.stripe_token = stripe_token.id;
     }
 
     for (const item of form.serializeArray()) {
-        if (numeric_inputs.includes(item.name)) {
-            data[item.name] = item.value;
-        } else {
-            data[item.name] = JSON.stringify(item.value);
-        }
+        data[item.name] = item.value;
     }
 
     $.post({

static/js/billing/upgrade.js

@@ -12,9 +12,7 @@ export const initialize = () => {
         image: "/static/images/logo/zulip-icon-128x128.png",
         locale: "auto",
         token(stripe_token) {
-            helpers.create_ajax_request("/json/billing/upgrade", "autopay", stripe_token, [
-                "licenses",
-            ]);
+            helpers.create_ajax_request("/json/billing/upgrade", "autopay", stripe_token);
         },
     });
 
@@ -43,7 +41,7 @@ export const initialize = () => {
             return;
         }
         e.preventDefault();
-        helpers.create_ajax_request("/json/billing/upgrade", "invoice", undefined, ["licenses"]);
+        helpers.create_ajax_request("/json/billing/upgrade", "invoice");
     });
 
     $("#sponsorship-button").on("click", (e) => {
@@ -51,13 +49,7 @@ export const initialize = () => {
             return;
         }
         e.preventDefault();
-        helpers.create_ajax_request(
-            "/json/billing/sponsorship",
-            "sponsorship",
-            undefined,
-            undefined,
-            "/",
-        );
+        helpers.create_ajax_request("/json/billing/sponsorship", "sponsorship", undefined, "/");
     });
 
     const prices = {};