billing/sponsorship: Allow blank organization URLs.

We ran into a bug in production caused by two issues:
- Some users came from orgs that didn't have a website and since
  the URL field was required, they submitted invalid URLs.
- We didn't properly respond to invalid form submissions, which
  led to UnboundLocalError exceptions in another part of the
  code.

This commit solves this by doing the following:
- We now allow blank URLs and have a convenient placeholder text
  label that tells users that they may leave the URL field blank.
- This commit refactors the code such that invalid form submissions
  result in an informative error message about what exactly went
  wrong.
This commit is contained in:
Eeshan Garg 2021-08-10 16:22:01 -02:30 committed by Tim Abbott
parent 6117c3824a
commit a808f02a22
5 changed files with 77 additions and 26 deletions

View File

@ -0,0 +1,18 @@
# Generated by Django 3.2.5 on 2021-08-06 19:18
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
("corporate", "0012_zulipsponsorshiprequest"),
]
operations = [
migrations.AlterField(
model_name="zulipsponsorshiprequest",
name="org_website",
field=models.URLField(blank=True, null=True),
),
]

View File

@ -204,6 +204,6 @@ class ZulipSponsorshipRequest(models.Model):
) )
MAX_ORG_URL_LENGTH: int = 200 MAX_ORG_URL_LENGTH: int = 200
org_website: str = models.URLField(max_length=MAX_ORG_URL_LENGTH) org_website: str = models.URLField(max_length=MAX_ORG_URL_LENGTH, blank=True, null=True)
org_description: str = models.TextField(default="") org_description: str = models.TextField(default="")

View File

@ -1461,6 +1461,32 @@ class StripeTest(StripeTestCase):
orjson.loads(response.content)["error_description"], "uncaught exception during upgrade" orjson.loads(response.content)["error_description"], "uncaught exception during upgrade"
) )
def test_request_sponsorship_form_with_invalid_url(self) -> None:
user = self.example_user("hamlet")
self.login_user(user)
data = {
"organization-type": Realm.ORG_TYPES["opensource"]["id"],
"website": "invalid-url",
"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_error(response, "Enter a valid URL.")
def test_request_sponsorship_form_with_blank_url(self) -> None:
user = self.example_user("hamlet")
self.login_user(user)
data = {
"organization-type": Realm.ORG_TYPES["opensource"]["id"],
"website": "",
"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)
def test_request_sponsorship(self) -> None: def test_request_sponsorship(self) -> None:
user = self.example_user("hamlet") user = self.example_user("hamlet")
self.assertIsNone(get_customer_by_realm(user.realm)) self.assertIsNone(get_customer_by_realm(user.realm))

View File

@ -203,7 +203,7 @@ def initial_upgrade(
class SponsorshipRequestForm(forms.Form): class SponsorshipRequestForm(forms.Form):
website = forms.URLField(max_length=ZulipSponsorshipRequest.MAX_ORG_URL_LENGTH) website = forms.URLField(max_length=ZulipSponsorshipRequest.MAX_ORG_URL_LENGTH, required=False)
organization_type = forms.IntegerField() organization_type = forms.IntegerField()
description = forms.CharField(widget=forms.Textarea) description = forms.CharField(widget=forms.Textarea)
@ -235,8 +235,8 @@ def sponsorship(
post_data.update(organization_type=organization_type) post_data.update(organization_type=organization_type)
form = SponsorshipRequestForm(post_data) form = SponsorshipRequestForm(post_data)
with transaction.atomic(): if form.is_valid():
if form.is_valid(): with transaction.atomic():
sponsorship_request = ZulipSponsorshipRequest( sponsorship_request = ZulipSponsorshipRequest(
realm=realm, realm=realm,
requested_by=user, requested_by=user,
@ -251,27 +251,34 @@ def sponsorship(
realm.org_type = org_type realm.org_type = org_type
realm.save(update_fields=["org_type"]) realm.save(update_fields=["org_type"])
update_sponsorship_status(realm, True, acting_user=user) update_sponsorship_status(realm, True, acting_user=user)
do_make_user_billing_admin(user) do_make_user_billing_admin(user)
org_type_display_name = get_org_type_display_name(org_type) org_type_display_name = get_org_type_display_name(org_type)
context = { context = {
"requested_by": requested_by, "requested_by": requested_by,
"user_role": user_role, "user_role": user_role,
"string_id": realm.string_id, "string_id": realm.string_id,
"support_url": support_url, "support_url": support_url,
"organization_type": org_type_display_name, "organization_type": org_type_display_name,
"website": website, "website": website,
"description": description, "description": description,
} }
send_email( send_email(
"zerver/emails/sponsorship_request", "zerver/emails/sponsorship_request",
to_emails=[FromAddress.SUPPORT], to_emails=[FromAddress.SUPPORT],
from_name="Zulip sponsorship", from_name="Zulip sponsorship",
from_address=FromAddress.tokenized_no_reply_address(), from_address=FromAddress.tokenized_no_reply_address(),
reply_to_email=user.delivery_email, reply_to_email=user.delivery_email,
context=context, context=context,
) )
return json_success() return json_success()
else:
messages = []
for error_list in form.errors.get_json_data().values():
for error in error_list:
messages.append(error["message"])
message = " ".join(messages)
raise BillingError("Form validation error", message=message)

View File

@ -238,7 +238,7 @@
<label> <label>
<h4>Organization website</h4> <h4>Organization website</h4>
</label> </label>
<input name="website" style="width: 100%;" type="text" class="input-large" required/> <input name="website" style="width: 100%;" type="text" class="input-large" placeholder="{{ _('Leave blank if your organization does not have a website.') }}"/>
<label> <label>
<h4>Describe your organization briefly</h4> <h4>Describe your organization briefly</h4>
</label> </label>