billing: Use PATCH request for changing plan status.

I think it's much more cleaner to use PATCH request on
/json/billing/plan than using a POST request on
/json/billing/plan/change to update the plan.
This commit is contained in:
Vishnu KS 2020-12-10 22:45:09 +05:30 committed by Tim Abbott
parent 3af0485d84
commit d9baa681b2
7 changed files with 103 additions and 55 deletions

View File

@ -1780,15 +1780,15 @@ class StripeTest(StripeTestCase):
with patch("corporate.lib.stripe.timezone_now", return_value=self.now):
self.local_upgrade(self.seat_count, True, CustomerPlan.ANNUAL, "token")
with self.assertLogs("corporate.stripe", "INFO") as m:
response = self.client_post(
"/json/billing/plan/change", {"status": CustomerPlan.DOWNGRADE_AT_END_OF_CYCLE}
response = self.client_patch(
"/json/billing/plan", {"status": CustomerPlan.DOWNGRADE_AT_END_OF_CYCLE}
)
stripe_customer_id = Customer.objects.get(realm=user.realm).id
new_plan = get_current_plan_by_realm(user.realm)
assert new_plan is not None
expected_log = f"INFO:corporate.stripe:Change plan status: Customer.id: {stripe_customer_id}, CustomerPlan.id: {new_plan.id}, status: {CustomerPlan.DOWNGRADE_AT_END_OF_CYCLE}"
self.assertEqual(m.output[0], expected_log)
self.assert_json_success(response)
self.assert_json_success(response)
# Verify that we still write LicenseLedger rows during the remaining
# part of the cycle
@ -1869,13 +1869,13 @@ class StripeTest(StripeTestCase):
assert new_plan is not None
with self.assertLogs("corporate.stripe", "INFO") as m:
response = self.client_post(
"/json/billing/plan/change",
response = self.client_patch(
"/json/billing/plan",
{"status": CustomerPlan.SWITCH_TO_ANNUAL_AT_END_OF_CYCLE},
)
expected_log = f"INFO:corporate.stripe:Change plan status: Customer.id: {stripe_customer_id}, CustomerPlan.id: {new_plan.id}, status: {CustomerPlan.SWITCH_TO_ANNUAL_AT_END_OF_CYCLE}"
self.assertEqual(m.output[0], expected_log)
self.assert_json_success(response)
self.assert_json_success(response)
monthly_plan.refresh_from_db()
self.assertEqual(monthly_plan.status, CustomerPlan.SWITCH_TO_ANNUAL_AT_END_OF_CYCLE)
with patch("corporate.views.timezone_now", return_value=self.now):
@ -2053,15 +2053,15 @@ class StripeTest(StripeTestCase):
new_plan = get_current_plan_by_realm(user.realm)
assert new_plan is not None
with self.assertLogs("corporate.stripe", "INFO") as m:
response = self.client_post(
"/json/billing/plan/change",
response = self.client_patch(
"/json/billing/plan",
{"status": CustomerPlan.SWITCH_TO_ANNUAL_AT_END_OF_CYCLE},
)
self.assertEqual(
m.output[0],
f"INFO:corporate.stripe:Change plan status: Customer.id: {stripe_customer_id}, CustomerPlan.id: {new_plan.id}, status: {CustomerPlan.SWITCH_TO_ANNUAL_AT_END_OF_CYCLE}",
)
self.assert_json_success(response)
self.assert_json_success(response)
monthly_plan.refresh_from_db()
self.assertEqual(monthly_plan.status, CustomerPlan.SWITCH_TO_ANNUAL_AT_END_OF_CYCLE)
with patch("corporate.views.timezone_now", return_value=self.now):
@ -2152,25 +2152,23 @@ class StripeTest(StripeTestCase):
with patch("corporate.lib.stripe.timezone_now", return_value=self.now):
self.local_upgrade(self.seat_count, True, CustomerPlan.ANNUAL, "token")
with self.assertLogs("corporate.stripe", "INFO") as m:
response = self.client_post(
"/json/billing/plan/change", {"status": CustomerPlan.DOWNGRADE_AT_END_OF_CYCLE}
response = self.client_patch(
"/json/billing/plan", {"status": CustomerPlan.DOWNGRADE_AT_END_OF_CYCLE}
)
stripe_customer_id = Customer.objects.get(realm=user.realm).id
new_plan = get_current_plan_by_realm(user.realm)
assert new_plan is not None
expected_log = f"INFO:corporate.stripe:Change plan status: Customer.id: {stripe_customer_id}, CustomerPlan.id: {new_plan.id}, status: {CustomerPlan.DOWNGRADE_AT_END_OF_CYCLE}"
self.assertEqual(m.output[0], expected_log)
self.assert_json_success(response)
self.assert_json_success(response)
self.assertEqual(
CustomerPlan.objects.first().status, CustomerPlan.DOWNGRADE_AT_END_OF_CYCLE
)
with self.assertLogs("corporate.stripe", "INFO") as m:
response = self.client_post(
"/json/billing/plan/change", {"status": CustomerPlan.ACTIVE}
)
response = self.client_patch("/json/billing/plan", {"status": CustomerPlan.ACTIVE})
expected_log = f"INFO:corporate.stripe:Change plan status: Customer.id: {stripe_customer_id}, CustomerPlan.id: {new_plan.id}, status: {CustomerPlan.ACTIVE}"
self.assertEqual(m.output[0], expected_log)
self.assert_json_success(response)
self.assert_json_success(response)
self.assertEqual(CustomerPlan.objects.first().status, CustomerPlan.ACTIVE)
@patch("stripe.Invoice.create")
@ -2190,8 +2188,8 @@ class StripeTest(StripeTestCase):
stripe_customer_id = Customer.objects.get(realm=user.realm).id
new_plan = get_current_plan_by_realm(user.realm)
assert new_plan is not None
self.client_post(
"/json/billing/plan/change", {"status": CustomerPlan.DOWNGRADE_AT_END_OF_CYCLE}
self.client_patch(
"/json/billing/plan", {"status": CustomerPlan.DOWNGRADE_AT_END_OF_CYCLE}
)
expected_log = f"INFO:corporate.stripe:Change plan status: Customer.id: {stripe_customer_id}, CustomerPlan.id: {new_plan.id}, status: {CustomerPlan.DOWNGRADE_AT_END_OF_CYCLE}"
self.assertEqual(m.output[0], expected_log)
@ -2226,7 +2224,7 @@ class StripeTest(StripeTestCase):
self.assertEqual(last_ledger_entry.licenses_at_next_renewal, 21)
self.login_user(user)
self.client_post("/json/billing/plan/change", {"status": CustomerPlan.ENDED})
self.client_patch("/json/billing/plan", {"status": CustomerPlan.ENDED})
plan.refresh_from_db()
self.assertEqual(get_realm("zulip").plan_type, Realm.LIMITED)
@ -2258,8 +2256,8 @@ class StripeTest(StripeTestCase):
self.login_user(user)
with self.assertLogs("corporate.stripe", "INFO") as m:
self.client_post(
"/json/billing/plan/change", {"status": CustomerPlan.DOWNGRADE_AT_END_OF_CYCLE}
self.client_patch(
"/json/billing/plan", {"status": CustomerPlan.DOWNGRADE_AT_END_OF_CYCLE}
)
stripe_customer_id = Customer.objects.get(realm=user.realm).id
new_plan = get_current_plan_by_realm(user.realm)
@ -2304,13 +2302,22 @@ class StripeTest(StripeTestCase):
self.local_upgrade(self.seat_count, True, CustomerPlan.ANNUAL, "token")
self.login_user(self.example_user("hamlet"))
response = self.client_post(
"/json/billing/plan/change",
response = self.client_patch(
"/json/billing/plan",
{"status": CustomerPlan.NEVER_STARTED},
)
self.assert_json_error_contains(response, "Invalid status")
def test_deactivate_realm(self) -> None:
def test_update_plan_without_any_params(self) -> None:
with patch("corporate.lib.stripe.timezone_now", return_value=self.now):
self.local_upgrade(self.seat_count, True, CustomerPlan.ANNUAL, "token")
self.login_user(self.example_user("hamlet"))
response = self.client_patch("/json/billing/plan", {})
self.assert_json_error_contains(response, "Nothing to change")
@patch("corporate.lib.stripe.billing_logger.info")
def test_deactivate_realm(self, mock_: Mock) -> None:
user = self.example_user("hamlet")
with patch("corporate.lib.stripe.timezone_now", return_value=self.now):
self.local_upgrade(self.seat_count, True, CustomerPlan.ANNUAL, "token")
@ -2478,15 +2485,26 @@ class RequiresBillingAccessTest(ZulipTestCase):
def test_who_cant_access_json_endpoints(self) -> None:
def verify_user_cant_access_endpoint(
username: str, endpoint: str, request_data: Dict[str, str], error_message: str
username: str,
endpoint: str,
method: str,
request_data: Dict[str, str],
error_message: str,
) -> None:
self.login_user(self.example_user(username))
response = self.client_post(endpoint, request_data)
if method == "POST":
response = self.client_post(endpoint, request_data)
elif method == "PATCH":
response = self.client_patch(endpoint, request_data)
else:
raise AssertionError("Invalid method")
self.assert_json_error_contains(response, error_message)
verify_user_cant_access_endpoint(
"polonius",
"/json/billing/upgrade",
"POST",
{
"billing_modality": orjson.dumps("charge_automatically").decode(),
"schedule": orjson.dumps("annual").decode(),
@ -2499,6 +2517,7 @@ class RequiresBillingAccessTest(ZulipTestCase):
verify_user_cant_access_endpoint(
"polonius",
"/json/billing/sponsorship",
"POST",
{
"organization-type": orjson.dumps("event").decode(),
"description": orjson.dumps("event description").decode(),
@ -2512,13 +2531,15 @@ class RequiresBillingAccessTest(ZulipTestCase):
verify_user_cant_access_endpoint(
username,
"/json/billing/sources/change",
"POST",
{"stripe_token": orjson.dumps("token").decode()},
"Must be a billing administrator or an organization owner",
)
verify_user_cant_access_endpoint(
username,
"/json/billing/plan/change",
"/json/billing/plan",
"PATCH",
{"status": orjson.dumps(1).decode()},
"Must be a billing administrator or an organization owner",
)

View File

@ -6,10 +6,10 @@ from django.views.generic import TemplateView
from corporate.views import (
billing_home,
change_plan_status,
initial_upgrade,
replace_payment_source,
sponsorship,
update_plan,
upgrade,
)
from zerver.lib.rest import rest_path
@ -27,7 +27,7 @@ i18n_urlpatterns: Any = [
v1_api_and_json_patterns = [
rest_path("billing/upgrade", POST=upgrade),
rest_path("billing/sponsorship", POST=sponsorship),
rest_path("billing/plan/change", POST=change_plan_status),
rest_path("billing/plan", PATCH=update_plan),
rest_path("billing/sources/change", POST=replace_payment_source),
]

View File

@ -356,10 +356,10 @@ def billing_home(request: HttpRequest) -> HttpResponse:
@require_billing_access
@has_request_variables
def change_plan_status(
def update_plan(
request: HttpRequest,
user: UserProfile,
status: int = REQ(
status: Optional[int] = REQ(
"status",
json_validator=check_int_in(
[
@ -369,27 +369,30 @@ def change_plan_status(
CustomerPlan.ENDED,
]
),
default=None,
),
) -> HttpResponse:
plan = get_current_plan_by_realm(user.realm)
assert plan is not None # for mypy
if status == CustomerPlan.ACTIVE:
assert plan.status == CustomerPlan.DOWNGRADE_AT_END_OF_CYCLE
do_change_plan_status(plan, status)
elif status == CustomerPlan.DOWNGRADE_AT_END_OF_CYCLE:
assert plan.status == CustomerPlan.ACTIVE
downgrade_at_the_end_of_billing_cycle(user.realm)
elif status == CustomerPlan.SWITCH_TO_ANNUAL_AT_END_OF_CYCLE:
assert plan.billing_schedule == CustomerPlan.MONTHLY
assert plan.status == CustomerPlan.ACTIVE
assert plan.fixed_price is None
do_change_plan_status(plan, status)
elif status == CustomerPlan.ENDED:
assert plan.status == CustomerPlan.FREE_TRIAL
downgrade_now_without_creating_additional_invoices(user.realm)
return json_success()
if status is not None:
if status == CustomerPlan.ACTIVE:
assert plan.status == CustomerPlan.DOWNGRADE_AT_END_OF_CYCLE
do_change_plan_status(plan, status)
elif status == CustomerPlan.DOWNGRADE_AT_END_OF_CYCLE:
assert plan.status == CustomerPlan.ACTIVE
downgrade_at_the_end_of_billing_cycle(user.realm)
elif status == CustomerPlan.SWITCH_TO_ANNUAL_AT_END_OF_CYCLE:
assert plan.billing_schedule == CustomerPlan.MONTHLY
assert plan.status == CustomerPlan.ACTIVE
assert plan.fixed_price is None
do_change_plan_status(plan, status)
elif status == CustomerPlan.ENDED:
assert plan.status == CustomerPlan.FREE_TRIAL
downgrade_now_without_creating_additional_invoices(user.realm)
return json_success()
return json_error(_("Nothing to change."))
@require_billing_access

View File

@ -34,10 +34,12 @@ run_test("initialize", (override) => {
});
let create_ajax_request_called = false;
function card_change_ajax(url, form_name, stripe_token) {
function card_change_ajax(url, form_name, stripe_token, redirect_to, method) {
assert.equal(url, "/json/billing/sources/change");
assert.equal(form_name, "cardchange");
assert.equal(stripe_token, "stripe_token");
assert.equal(redirect_to, undefined);
assert.equal(method, undefined);
create_ajax_request_called = true;
}
@ -86,10 +88,12 @@ run_test("initialize", (override) => {
});
create_ajax_request_called = false;
function plan_change_ajax(url, form_name, stripe_token) {
assert.equal(url, "/json/billing/plan/change");
function plan_change_ajax(url, form_name, stripe_token, redirect_to, method) {
assert.equal(url, "/json/billing/plan");
assert.equal(form_name, "planchange");
assert.equal(stripe_token, undefined);
assert.equal(redirect_to, undefined);
assert.equal(method, "PATCH");
create_ajax_request_called = true;
}

View File

@ -114,7 +114,7 @@ run_test("create_ajax_request", (override) => {
$("#autopay-form").serializeArray = () => jquery("#autopay-form").serializeArray();
override($, "post", ({url, data, success, error}) => {
override($, "ajax", ({type, url, data, success, error}) => {
assert.equal(state.form_input_section_hide, 1);
assert.equal(state.form_error_hide, 1);
assert.equal(state.form_loading_show, 1);
@ -124,6 +124,7 @@ run_test("create_ajax_request", (override) => {
assert.equal(state.free_trial_alert_message_show, 0);
assert.equal(state.make_indicator, 1);
assert.equal(type, "PATCH");
assert.equal(url, "/json/billing/upgrade");
assert.equal(Object.keys(data).length, 8);
@ -174,7 +175,13 @@ 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"});
helpers.create_ajax_request(
"/json/billing/upgrade",
"autopay",
{id: "stripe_token_id"},
undefined,
"PATCH",
);
});
run_test("format_money", () => {

View File

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

View File

@ -3,7 +3,13 @@ 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, redirect_to = "/billing") {
export function create_ajax_request(
url,
form_name,
stripe_token = null,
redirect_to = "/billing",
type = "POST",
) {
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`;
@ -33,7 +39,8 @@ export function create_ajax_request(url, form_name, stripe_token = null, redirec
data[item.name] = item.value;
}
$.post({
$.ajax({
type,
url,
data,
success() {