The logic for BillingSession.is_sponsored_or_pending would be the
same for all three child classes of BillingSession, so this should
be a method on the BillingSession abstract class.
Creates a process_support_view_request method for BillingSession
to process the various support requests that relate to the billing
system.
Moves approve_realm_sponsorship, update_realm_sponsorship_status,
and attach_discount_to_realm to this new BillingSession method.
Adds a new abstract property to BillingSession to have a string
value, billing_entity_display_name, to use for support messages
sent when these requests are processed.
The "send_invoice" and "charge_automatically" strings used by stripe
for the `collection_method` are referred to both as the "billing
method" and "billing modality" in the billing code.
Because we send this as data to stripe as either `collection_method`
or `billing_modality`, renames any references that are any form of
"billing method".
Analogical to the more complex mechanism implemented for the RemoteRealm
flow in a previous commit in
authenticated_remote_realm_management_endpoint.
As explained in the code comment, this is much easier because:
In this flow, we can only redirect to our local "legacy server flow
login" page. That means that we can do it universally whether the user
has an expired
identity_dict, or just lacks any form of authentication info at all -
there are no security concerns since this is just a local redirect.
Analogical to 1df8e00d7c which implemented
this for the RemoteRealm auth flow.
Except here we don't need to add next_page to the IdentityDict
(LegacyServerIdentityDict in this flow), because the redirect happens
immediately at remote_billing_legacy_server_login upon login - so no
need to have a structure to carry the info through intermediate steps.
Implements a nice redirect flow to give a good UX for users attempting
to access a remote billing page with an expired RemoteRealm session e.g.
/realm/some-uuid/sponsorship - perhaps through their browser
history or just their session expired while they were doing things in
this billing system.
The logic has a few pieces:
1. get_remote_realm_from_session, if the user doesn't have a
identity_dict will raise RemoteBillingAuthenticationError.
2. If the user has an identity_dict, but it's expired, then
get_identity_dict_from_session inside of get_remote_realm_from_session
will raise RemoteBillingIdentityExpiredError.
3. The decorator authenticated_remote_realm_management_endpoint
catches that exception and uses some general logic, described in more
detail in the comments in the code, to figure out the right URL to
redirect them to. Something like:
https://theirserver.example.com/self-hosted-billing/?next_page=...
where the next_page param is determined based on parsing request.path
to see what kind of endpoint they're trying to access.
4. The remote_server_billing_entry endpoint is tweaked to also send
its uri scheme to the bouncer, so that the bouncer can know whether
to do the redirect on http or https.
This default setup will be more realistic, matching the ordinary
conditions for a modern server.
Especially needed as we add bouncer code that will expect to have
RemoteRealm entries for realm_uuid values for which it receives
requests.
This does two important things:
1. Fix return type of get_identity_dict_from_session to correctly be
Optional[Union[RemoteBillingIdentityDict, LegacyServerIdentityDict]].
RemoteBillingIdentityDict is the type in the 8.0+ auth flow,
LegacyServerIdentityDict is the type in old servers flow, where only
the server uuid info is available.
2. The uuid key used in request.session["remote_billing_identities"]
should be explicitly namespaced depending on which flow and type
we're
dealing with - to avoid confusion in case of collisions between a
realm and server that have the same UUID. Such a situation should not
occur naturally and I haven't come up with any actual exploitation
ideas that could utilize this by manipulating your server/realm
uuids, but it's much easier to just not think about such collision
security implications by making them impossible.
Since endpoints using the
`authenticated_remote_realm_management_endpoint` decorator
want to initialize a billing session and if need be remote_realm
is accessible to via the session variable.
We pass `next` parameter with /self-hosted-billing to redirect
users to the intended page after login.
Fixed realm_uuid incorrectly required in remote_realm_upgrade_page.
Updates do_change_plan_to_new_tier in BillingSession to use an
enum for the value returned when checking for a valid change
between two plan tier types. This makes it more explicit that
the implementation for a valid upgrade in plan tier will be
different from a valid downgrade in plan tier.
We've already processed the only URL parameters we intend to support
to determine which RemoteRealm or RemoteZulipServer is involved, so
there should be nothing further to do here.
And it's cleaner to not have to write the downstream code to expect
these unnecessary parameters.
This commit performs a minor update in 'from_name' text for
'support' and 'sponsorship' emails.
Removes capitalization and adds a comment specifying that the
emails are not user-facing.
This commit refactors 'sponsorship_request' view and adds
'BillingSession.get_sponsorship_request' method.
This refactoring will help in minimizing duplicate code
while supporting both realm and remote_Server customers.
This commit refactors 'event_status' view and adds
'BillingSession.get_event_status' method.
This refactoring will help in minimizing duplicate code
while supporting both realm and remote_server customers.
Since Customer already stores the realm it is linked to and
customer is always created to store sponsorship request, we directly
use customer to link to the sponsorship data for the realm.
Moves and generalizes `switch_realm_from_standard_to_plus_plan`
in stripe.py to be a more general function for changing a
CustomerPlan to a new and valid tier, `do_change_plan_to_new_tier`.
Adds a helper function with the previous function name to be used
for the support view and management command for changing a realm
from the Standard plan tier to the Plus plan tier.
Renames CustomerPlan.SWITCH_NOW_FROM_STANDARD_TO_PLUS to be more
generic, CustomerPlan.SWITCH_PLAN_TIER_NOW.
Because the plan tier change is immediate, moves the code to end
the current plan with the old tier and create the new plan with
the new tier from `make_end_of_cycle_updates_if_needed` to instead
be in a separate helper function, `switch_plan_tier`.
This commit moves a major portion of the 'update_plan`
view to a new shared 'BillingSession.do_update_plan' method.
This refactoring will help in minimizing duplicate code
while supporting both realm and remote_server customers.
This makes it possible for a self-hosted realm administrator to
directly access a logged-page on the push notifications bouncer
service, enabling billing, support contacts, and other administrator
for enterprise customers to be managed without manual setup.
This commit updates the 'get_initial_upgrade_context' method
to use 'get_price_per_license' for determining 'annual_price'
and 'monthly_price' based on tier and discount instead of hardcoding.
Also, removed the 'percent_off' page_params as
'get_price_per_license' already performs the price calculation
taking discount into consideration.
If the update / add card session is successful, return user to
manual license management page if user was on it before clicking
the add / update card button.
This commit moves a major portion of the 'initial_upgrade`
view to a new shared 'BillingSession.do_initial_upgrade' method.
This refactoring will help in minimizing duplicate code
while supporting both realm and remote_server customers.
These new models are incomplete and totally untested, but merging this
will provide valuable scaffolding for doing smaller PRs working on
individual gaps, and reveals a clear set of TODOs/refactoring/model
changes needed to support where want to end up.
Co-authored-by: Tim Abbott <tabbott@zulip.com>
This commit moves the main context creation part of the
'billing_home` view to a new shared
'BillingSession.get_billing_page_context' method.
This refactoring will help in minimizing duplicate code
while supporting both realm and remote_server customers.
Moves the 'make_end_of_cycle_updates_if_needed' function to
the 'BillingSession' abstract class.
This refactoring will help in minimizing duplicate code while
supporting both realm and remote_server customers.
Since the function is called from our main daily billing cron job
as well, we have changed 'RealmBillingSession' to accept 'user=None'
(our convention for automated system jobs).
There is a discrepancy between price per license shown on billing
and upgrade page for annual billing frequency due to difference
in methods used to calculate the amount.
To fix it, we use the same method on both the pages.
I didn't use the helpers.format_money directly here since that would
create a visual delay in showing the price per license which will
not be nice considering that will be the only thing on this page
being updated that way.
The stripe fixture used is same as
free_trial_upgrade_by_card--Customer.retrieve.3.json
This commit moves a major portion of the 'upgrade`
view to a new shared 'BillingSession.do_upgrade' method.
This refactoring will help in minimizing duplicate code
while supporting both realm and remote_server customers.
Moves the 'process_initial_upgrade' function to the
'BillingSession' abstract class.
This refactoring will help in minimizing duplicate code while
supporting both realm and remote_server customers.
Updates `process_initial_upgrade` to take a plan_tier parameter,
so that information can be specific to the type of BillingSession.
Note that ideally the plan tier would be passed as metadata to the
stripe.checkout.Session, but in order to do so, we need to be able
to update the generated stripe fixtures for tests. So for now, we
set the plan tier directly in the stripe event handler code.
Instead of hard coding the string for the stripe.InvoiceItem and
stripe.Invoice statement_descriptor, we use the name of the plan
that was just created.
Creates `do_change_plan_type` as a BillingSession abstract method
as an alias for updating the plan_type for a realm (and eventually
remote server).
This removes the last direct reference to a customer's realm in
`process_initial_upgrade`.
Creates `current_count_for_billed_licenses` as an abstract method
in BillingSession class, to get the latest seat count for the realm
(and eventually remote server) connected to the customer.
Moves the 'setup_upgrade_checkout_session_and_payment_intent'
function to the 'BillingSession' abstract class.
This refactoring will help in minimizing duplicate code while
supporting both realm and remote_server customers.
Generated with both parellel=1 and generate-stripe-fixtures params
on test-backend locally.
Note that the previous series of commits need these test fixtures
to pass CI. If those are changed, then this commit should be updated
or dropped and replaced with a commit that updates the fixtures
based on the new changes.
Instead of using the data from the stripe.SetupIntent for processing
an initial upgrade for the free trial session types, use the metadata
from the stripe.checkout.Session that was returned.
The metadata is the same for both the checkout session and the setup
intent.
Updates generate stripe fixtures to use test credit card strings from
Stripe, instead of creating fake stripe.PaymentMethods for the tests
with the test credit card numbers provided by Stripe.
According to stripe's documentation for attaching payment methods to
customers (see https://stripe.com/docs/api/payment_methods/attach),
payment methods should be attached to customers through a SetupIntent
or PaymentIntent, which is what we do as we process new customers
and accounts.
Updates create_stripe_customer so that it is clear that the payment
method should not be added when we directly create a new stripe
customer.
There is a 22 char limit on the stripe.Invoice.statement_descriptor
field. The current value for that field in this error case, "Zulip
Cloud Standard Credit" is 27 chars.
Updates the value for this error case to be "Cloud Standard Credit".
This calculates the largest amount of messages sent within a month for
the last 3 months. The query is targeted for the specific use-case in
this function - for finding the count for a specific server. For
calculating this in bulk for a large number of remote server an
adapted, bulk query will be needed - rather than running this one in a
loop, which would likely be very inefficient.
Instead of randomly generating an integer for the realm string_id
and realm name use a counter to increment the string ids by one.
This way if a particular realm fails the test, it will be easier
to identify which one failed.
Also, removes unused Customer objects that were being returned on
realm creation in the test, as these were not being checked/used
in the test.
In commit a3093fad9, we made Hamlet a billing admin for testing.
Because the approve sponsorship flow sends messages from the
notification bot to both organization owners and billing admins,
we need to update that test to cover both of those user types.
Adds `create_stripe_payment_intent` to the BillingSession abstract
class for the initial upgrade process, which is used only in
`setup_upgrade_checkout_session_and_payment_intent`.
Adds a helper dataclass for the data used to create the stripe
payment intent, StripePaymentIntentData, for the implementation of
more than one child class of BillingSession.
Also adds two abstract helper functions for getting the above stripe
payment intent data as well as updating that data for a stripe
checkout session that is associated with a payment intent:
update_data_for_checkout_session_and_payment_intent,
get_data_for_stripe_payment_intent.
Adds `create_stripe_checkout_session` to BillingSession abstract
class, which is then used in all places where a stripe.checkout.Session
and Session were created: start_retry_payment_intent_session,
start_card_update_stripe_session, and
setup_upgrade_checkout_session_and_payment_intent.
Adds a `billing_session_url` abstract property to the BillingSession
abstract class and for the RealmBillingSession child class sets that
value to `self.realm.uri`.
setup_upgrade_checkout_session_and_payment_intent was not using the
datetime values that were being returned by compute_plan_parameters,
so just get the price per license directly.
Moves `update_billing_method_of_current_plan` to the BillingSession
abstract class.
Adds a helper function for support views for the realm case:
`update_realm_billing_method`.
Moves `update_sponsorship_status` to BillingSession abstract class
as `update_customer_sponsorship_status`.
Updates the support views to have a helper for updating this on a
realm: `update_realm_sponsorship_status`.
Makes `approve_sponshorship` an abstract method in BillingSession
abstract base class and moves the implementation for realms to the
RealmBillingSession child class.
Adds `approve_realm_sponsorship` helper function that's used in
the support view and initiates the billing session.
Creates an enum class, AuditLogEventType, and an abstract method in
BillingSession, get_audit_log_event, so that we have an abstraction
for getting the audit log event type since it might be different for
Customer objects with a realm vs a remote_server.
This moves the logic for `attach_realm_discount`, which is used in
the support view, to be in the BillingSession class.
Updates the function name to be `attach_discount_to_customer` so
that the context is generalized vs realm specific.
Updates RealmBillingSession implementation to account for actions
that are initiated by a support admin user.
Also moves the helper function `get_discount_for_realm` that is
only used in support views to `corporate/lib/support.py`.
So that all child classes of BillingSession generate the same data
structure for customers that are created in Stripe, revise
`get_data_for_stripe_customer` to return a specific dataclass:
StripeCustomerData.
So that `update_or_create_stripe_customer` can work for Customer
objects with either a realm or remote_server, we create an abstract
base class, BillingSession, and implement a child class for the
current implementation of Customer objects with a realm.
Refactoring `update_or_create_stripe_customer` also moves
`create_stripe_customer` and `replace_payment_method` to the
BillingSession class.
In ensure_customer_does_not_have_active_plan, we were already going
through the Customer table to get/check for an active CustomerPlan.
Now we directly get/check for an active CustomerPlan with via the
Customer, which allows for reusing this function for Customer
objects without a Realm set.
Moves two functions in corporate/lib/stripe.py that are used to
get data for the main installation activity analytics page to a
separate file: corporate/lib/analytics.py.
Also, updates these functions for the possibility of realm being
None for a Customer object.
Upgrading stripe to 6.0.0 in e32366638a
breaks our Stripe integration due to API version change making us fail
to finalize creating an invoice and charge the customer.
For the upstream details see:
60ab6ac7d7/CHANGELOG.md (600---2023-08-16)
6.0.0 uses 2023-08-16 Stripe API version unless specified otherwise. We
want to use 2020-08-27.
Setting stripe.api_version in corporate/lib/stripe.py is sufficient for
it to be set everywhere else. This is supported by the fact that we also
only set stripe.api_key in that file.
Fixes two bugs involving organization with
exempt_from_license_number_check enabled:
1. If the organization had e.g. 100 users and upgraded their plan,
specifying 50 licenses, the generated LicenseLedger and thus the
corresponding invoice was still for 100 users.
2. The organization was unable to use the billing/plan endpoint (update
plan endpoint) to make their number of licenses less than the current
number of users.
Organizations with exempt_from_license_number_check are supposed to be
able to declare whatever license number they want, as this attribute
allows having pricing schemes where an organization only pays us for a
subset of their users.
This migration applies under the assumption that extra_data_json has
been populated for all existing and coming audit log entries.
- This removes the manual conversions back and forth for extra_data
throughout the codebase including the orjson.loads(), orjson.dumps(),
and str() calls.
- The custom handler used for converting Decimal is removed since
DjangoJSONEncoder handles that for extra_data.
- We remove None-checks for extra_data because it is now no longer
nullable.
- Meanwhile, we want the bouncer to support processing RealmAuditLog entries for
remote servers before and after the JSONField migration on extra_data.
- Since now extra_data should always be a dict for the newer remote
server, which is now migrated, the test cases are updated to create
RealmAuditLog objects by passing a dict for extra_data before
sending over the analytics data. Note that while JSONField allows for
non-dict values, a proper remote server always passes a dict for
extra_data.
- We still test out the legacy extra_data format because not all
remote servers have migrated to use JSONField extra_data.
This verifies that support for extra_data being a string or None has not
been dropped.
Co-authored-by: Siddharth Asthana <siddharthasthana31@gmail.com>
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
Translators benefit from the extra information in the field names, and
need the reordering freedom that isn’t available with multiple
positional fields.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Note that we use the DjangoJSONEncoder so that we have builtin support
for parsing Decimal and datetime.
During this intermediate state, the migration that creates
extra_data_json field has been run. We prepare for running the backfilling
migration that populates extra_data_json from extra_data.
This change implements double-write, which is important to keep the
state of extra data consistent. For most extra_data usage, this is
handled by the overriden `save` method on `AbstractRealmAuditLog`, where
we either generates extra_data_json using orjson.loads or
ast.literal_eval.
While backfilling ensures that old realm audit log entries have
extra_data_json populated, double-write ensures that any new entries
generated will also have extra_data_json set. So that we can then safely
rename extra_data_json to extra_data while ensuring the non-nullable
invariant.
For completeness, we additionally set RealmAuditLog.NEW_VALUE for
the USER_FULL_NAME_CHANGED event. This cannot be handled with the
overridden `save`.
This addresses: https://github.com/zulip/zulip/pull/23116#discussion_r1040277795
Note that extra_data_json at this point is not used yet. So the test
cases do not need to switch to testing extra_data_json. This is later
done after we rename extra_data_json to extra_data.
Double-write for the remote server audit logs is special, because we only
get the dumped bytes from an external source. Luckily, none of the
payload carries extra_data that is not generated using orjson.dumps for
audit logs of event types in SYNC_BILLING_EVENTS. This can be verified
by looking at:
`git grep -A 6 -E "event_type=.*(USER_CREATED|USER_ACTIVATED|USER_DEACTIVATED|USER_REACTIVATED|USER_ROLE_CHANGED|REALM_DEACTIVATED|REALM_REACTIVATED)"`
Therefore, we just need to populate extra_data_json doing an
orjson.loads call after a None-check.
Co-authored-by: Zixuan James Li <p359101898@gmail.com>
exempt_from_license_number_check was initially added allowing
organizations with it enabled to invite new users above their number of
licenses.
However, an organization with this permission enabled,
cannot upgrade if they weren't on a plan already - because when choosing
Manual license management, you cannot enter a number of licenses lower
than the current seat count. However, an organization like that probably
already has some users that they get free of charge - and thus they need
to be able to enter a lower number of licenses in order to upgrade.
This just replaces the billing/upgrade with the statement that
"Your organization has requested sponsored or discounted hosting.", so
it should include an obvious contact in case the customer wants to amend
something or just bump a request that may have gotten missed.
Previously this was only available on the upgrade page - meaning an
organization that already bought a plan wouldn't be able to request a
sponsorship to get a discount or such, even if qualified.
The Django convention is for __repr__ to include the type and __str__
to omit it. In fact its default __repr__ implementation for models
automatically adds a type prefix to __str__, which has resulted in the
type being duplicated:
>>> UserProfile.objects.first()
<UserProfile: <UserProfile: emailgateway@zulip.com <Realm: zulipinternal 1>>>
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Black 23 enforces some slightly more specific rules about empty line
counts and redundant parenthesis removal, but the result is still
compatible with Black 22.
(This does not actually upgrade our Python environment to Black 23
yet.)
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Updates the message sent by the notification bot when an
organization is approved for full sponsorship on Zulip
Cloud Standard to include a request to list and link to
Zulip on any acknowledgement or sponsorship pages.
When this code was moved from being in zerver in 21a2fd482e, it kept
the `if ZILENCER_ENABLED` blocks. Since ZILENCER and CORPORATE are
generally either both on or both off, the if statement became
mostly-unnecessary.
However, because tests cannot easily remove elements from
INSTALLED_APPS and re-determine URL resolution, we switch to checking
`if CORPORATE_ENABLED` as a guard, and leave these in-place.
The other side effect of this is that with e54ded49c4, most Zulip
deployments started to 404 requests for `/apps` instead of redirecting
them to `https://zulip.com/apps/` since they no longer had any path
configured for `/apps`. Unfortunately, this URL is in widespread use
in the app (e.g. in links from the Welcome Bot), so we should ensure
that it does successfully redirect.
Add the `/apps` path to `zerver`, but only if not CORPORATE_ENABLED,
so the URLs do not overlap.
Adds the count of users with the role of guest to the stats view
`page_params` via a database query. This information is then added
to the summary statistics section of the analytics page after being
formatted by `stats.js`.
Creates Bassanio as a guest user in the database for the analytics
realm.
Fixes#20162.
Updates the organizations listed in the open communities directory
to also include organizations that do not require an invite and do
not restrict email domains for new users to join the organization.
With django-stubs, these explicit copies of Django’s implicit id
fields are no longer needed for type checking. An exception is the
BigAutoField AbstractUserMessage.id, which is left alone.
This reverts commit c08ee904d8 (#15641).
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Previously, we type the model fields with explicit type annotations
manually with the approximate types. This was because the lack of types
for Django.
django-stubs provides more specific types for all these fields that
incompatible with our previous approximate annotations. So now we can
remove the inline type annotations and rely on the types defined in the
stubs. This allows mypy to infer the types of the model fields for us.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
Extends the URL redirect system used for documentation pages to corporate
landing pages. This makes it easier and consistent for contributors who
work on both areas to create new URL redirects when needed.
Disables submit buttons on billing / upgrade page for demo
organizations since they will need to become permanent
organizations before upgrading to Zulip Cloud Standard.
Also creates an alert banner on the same page that links to
the help center article on demo organizations.
Updates sub-headers on demo organizations help center
article to match link text and to follow general convention
of using imperative verb forms in help center subheaders.
Part of #19523.
Co-authored by: Lauryn Menard <lauryn@zulip.com>
Our seat count calculation is different for guest user than normal users
(a number of initial guests are free, and additional marginal guests are
worth 1/5 of a seat) - so these checks we apply when a user is being
invited or signing up need to know whether it's a guest or non-guest
being added.
This is a simple generalization of get_latest_seat_count and is useful
for calculating "what will be the realm's license count if this
number of (guest) users is added?" without duplicating any of the math
logic. Will be used in the next commits.
Our billing FAQ says:
"For an organization with N other users, 5*N guest users are included at
no extra charge. After that, you will be charged at 1/5 of your regular
per-user pricing for each additional guest.".
It wasn't quite intuitive to me that
max(non_guests, math.ceil(guests / 5)) achieves that pricing, so it's
worth mentioning in a comment that it does and that that's why that
formula is used.
`extra_data` as a `TextField` expects a `str`, but we had been passing
`dict` instead. This is a temporary solution before #18391 to fix the
type annotation.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
When being called, the wrapped function is passed `PaymentIntent`
(the `content_object` of `Event`). With that, since `customer` can be
`None`, an assertion is also required.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
This is an unused argument. We removed it so that we don't
need to create a `TypedDict` and unpack it when calling
the test client methods.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
Since `HttpResponse` is an inaccurate representation of the
monkey-patched response object returned by the Django test client, we
replace it with `_MonkeyPatchedWSGIResponse` as `TestHttpResponse`.
This replaces `HttpResponse` in zerver/tests, analytics/tests, coporate/tests,
zerver/lib/test_classes.py, and zerver/lib/test_helpers.py with
`TestHttpResponse`. Several files in zerver/tests are excluded
from this substitution.
This commit is auto-generated by a script, with manual adjustments on certain
files squashed into it.
This is a part of the django-stubs refactorings.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
Adds request as a parameter to json_success as a refactor towards
making `ignored_parameters_unsupported` functionality available
for all API endpoints.
Also, removes any data parameters that are an empty dict or
a dict with the generic success response values.
Given that these values are uuids, it's better to use UUIDField which is
meant for exactly that, rather than an arbitrary CharField.
This requires modifying some tests to use valid uuids.
Now that we pass in the UserProfile.id in the metadata to Stripe's
PaymentIntent objects, we no longer need to retrieve the user via
delivery_email. It makes more sense to just fetch the user by ID
and then get the latest delivery_email directly.
Note that an update to Stripe's fixtures is not necessary here
since a previous commit already modified the metadata passed to
both stripe.Session/PaymentIntent objects.
Previously, our Stripe webhook event handler code retrieved the
user's email from Stripe using the stripe.Customer.email attribute.
This led to situations such that whenever the email that Stripe had
did not correspond to a UserProfile in Zulip, the payment flow
failed since we couldn't find a UserProfile associated with the
given email.
Now, we pass in the UserProfile.id in the metadata to Stripe's
checkout Session object, so that we can fetch the correct email
in future Stripe requests.
After the Stripe migration to the hosted checkout page, we were
missing one fixture for the test for switching from Standard to
Plus. This commit updates the fixtures for that particular test.
This is a follow-up to #19752. The tests in that PR did not verify
that the financial math involved worked properly. This commit
improves the existing tests and adds new fixtures to make sure
that the financial math works as expected.
It is confusing to have the plan type constants not be namespaced
by the thing they represent. We already have a namespacing
convention in place for constants, so we should use it for
Realm.plan_type as well.
compute_plan_parameters assumed that the value of
tier is always CustomerPlan.STANDARD. We change that behavior
to make the function handle CustomerPlan.PLUS as well.
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.
These changes are all independent of each other; I just didn’t feel
like making dozens of commits for them.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
The locally defined NamedTuple was triggering a mypy caching bug
(https://github.com/python/mypy/issues/10913), and we don’t use the
tuple behavior anyway.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
When calling some functions or assigning values to certain attributes,
the arguments/right operand do not match the exact type that the
functions/attributes expect, and thus we fix that by converting types
beforehand.
This fixes a batch of mypy errors of the following format:
'Item "None" of "Optional[Something]" has no attribute "abc"
Since we have already been recklessly using these attritbutes
in the tests, adding assertions beforehand is justified presuming
that they oughtn't to be None.
This upgrades the Stripe API to the most recent version. Going through
the Git history, it looks like our current API version is at 2019-03-14.
The API version should be manually changed in Stripe dashboard at the same
time as the commit is deployed in production.
Backward incompatible changes that are relevant to our codebase between
(2019-03-14, 2020-08-27].
* 2020-08-27 - The `sources` property on Customers is no longer included by
default.
* 2020-03-02 - Nothing applicable
* 2019-12-03 - The `id` field of all invoice line items have changed and are
now prefixed
with `il_`. We only rely on this while we normalize the fixtures.
* 2019-11-05 - Nothing applicable
* 2019-10-17 - The `billing` attribute on invoices, subscriptions, and
subscription schedules is renamed to`collection_method`. The invoice
change is the one that is relevant to us.
* The customer object’s `account_balance` value has been renamed to
`balance`. Only used for the stubs at the moment.
* 2019-10-08 - Nothing applicable
* 2019-09-09 - Nothing applicable
* 2019-08-14 - Nothing applicable
* 2019-05-16 - Nothing applicable
https://stripe.com/docs/upgrades
Also normalize the following IDs in stripe fixtures
* price_[A-Za-z0-9]{24}
* prod_[A-Za-z0-9]{14}
* pi_[A-Za-z0-9]{24}
* il_[A-Za-z0-9]{24}
Previously, we only downgraded and voided small organizations behind
payments only if they had an active plan.
This left us with a bunch of invoices from small realms which used to
have an active plan. It doesn't make much sense for us to get these
realms to pay the invoices so we have decided to just void them. This
commit voids the open invoices of all the small realms without an active
plan and has the last invoice open. Unlike, the realms with an active
plan we don't email them about us voiding the invoice. It's not super
obvious whether Stripe sends an email to the customer when the Invoice
is voided. But they do get the message that the invoice is voided if
they try to pay the invoice through the hosted invoice page.
This avoid some duplicate code as well as improve the readability since
before we were checking for the expected values far away from the
definition of realm. Now we define the expected values right after the
realm definition which improves the code readability.
Also, this get removes the postfixing of realm variable names with numbers.
The postfixing is kind of mess since if we want add any new realm in between
the realms we need to renumber a lot of realm variables.
An additional check for whether customer.stripe_customer_id is
None is added to the function. That check was not really required before
since all the customers with a plan also have a valid value for
stripe_customer_id. So all the calls to stripe.Invoice.list would have
non None value for customer argument.
Even though that is the case, mypy should still have complained about
the possibility of customer.stripe_customer_id being None when passed to
stripe.Invoice.list as customer paramater since mypy don't know that
customers with a plan will always have a non empty value for
stripe_customer_id. Our stripe stubs expect a non empty value for
the customer parameter of stripe.Invoice.list. This is despite the
fact that stripe.Invoice.list can actually be called with customer set
to None. This returns the invoices from the entire organization.
Though, we still decided to ensure that the value of customer should be
non empty since there is no reason for us to ever call this function
with customer set to None. You can just call the function wuthout the
customer argument instead. So this requirement of a non None customer
paramater is useful for catching bugs.
The reason mypy didn't complain was because the type of
Customer.objects.all() is Any and not QuerySet[Customer]. So mypy has no
idea that customer.stripe_customer_id can be theoratically None even
though it was not possible in this [articular case as explained before.
I verified that this was the reason mypy didn't complain by using the
reveal_type function on Customer.objects.all() and the customer object.
After the refactoring it's super to obvious to mypy that the type of the
customer is Customer since it's mentioned in the function defintion. So it
was able to complain about the possibility of customer.stripe_customer_id
being None after the refactoring.
This is a prep commit for the Stripe checkout migration.
The Stripe migration commit adds a lot of new view functions. Keeping
all of the views in one view file makes it super hard for readbability.
So creating a new views folder and splitting the existing view file into
two so that we minimize the changes in the big migration commit.
This makes sure that organizations as tagged with the user-volunteered
organization type in the sponsorship request, in the event that it
differs from what was entered during realm creation.
This makes several changes:
* Fixes a bug where the help text explaining our policies was not displayed.
* No help text was defined for many organization types.
* Copy-edits the help text somewhat.
* Offers all of the organization type options.
* Removes the 100% coverage requirement because it's annoying to test
the e.currentTarget click handler.
We are starting to run into situations where this data could be
quite useful for making future decisions, so it makes to store it
in the database, not just in an email.
This function had a confusing name, which could result in someone
using it unintentionally when they meant do_reactivate_user.
We also add docstrings for both functions.
This should have been in https://github.com/zulip/zulip/pull/18066.
The reason, the tests were not failing inspite of the params being
json encoded was because pretty much all these tests did not test
the functionality of the endpoint. Rather they were testing things
like whether the user has the right to access the endpoint and all.
So the value of the params did not matter.
The only one test which is an exception is test_replace_payment_source.
Even though a json encoded token was passed to an endpoint that
expecteda string, the test continued to work becausethe fixtures were
not updated for the test in that PR, so instead of sending an incorrect
json encoded token to stripe endpoint it was sending the correct string
token. Now that we removed the json.dumps of token, we no longer have to
update the fixtures.
I have run the tests with --generate-stripe-fixtures set to True and all
the tests are passing. Not including the fixture changes since the tests
conntinue to work the same with both the existing and new fixtures.
Billing system uses delivery_email instead of email. We used to make
the email address visible to everyone in tests which means the value
of email and delivery_email is the same. This commit disables that
so that we can distinguish between email and delivery_email in tests.
An organization with at most 5 users that is behind on payments isn't
worth spending time on investigating the situation.
For larger organizations, we likely want somewhat different logic that
at least does not void invoices.
stripe.Invoice.list by default would only get 10 invoices at a
time. So a function like this would be really handy if we have
to go through a lot of invoices.
This also means void_all_open_invoices used to void only the last
10 invoices. The main reason we implemented this function was to
void the invoices generated by realms on free trial so I don't
think there were cases where we had to void realms with more than
10 invoices.
This also fixes a bug in void_all_open_invoices function. If a realm
with a local Customer object but without an associated stripe.Customer
is passed to void_all_open_invoices, then the function will end up
voiding the last 10 invoices created by billing system instead of voiding
no invoices at all. This is because stripe.Invoice.list(customer=None)
return last 10 invoices across all customers.
But this bug won't cauuse any issue in production since
void_all_open_invoices can be only invoked from /support page. And we
show the option to void invoices in support page only if the realm
has a paid plan. And it's not really possible for a realm to have
a paid plan without having an associated stripe_customer_id. Plus I
went through the void events in stripe stream since the PR to add
void invoices was merged and there does not seems to be any suspicious
events.
JsonableError has two major benefits over json_error:
* It can be raised from anywhere in the codebase, rather than
being a return value, which is much more convenient for refactoring,
as one doesn't potentially need to change error handling style when
extracting a bit of view code to a function.
* It is guaranteed to contain the `code` property, which is helpful
for API consistency.
Various stragglers are not updated because JsonableError requires
subclassing in order to specify custom data or HTTP status codes.
django.utils.translation.ugettext is a deprecated alias of
django.utils.translation.gettext as of Django 3.0, and will be removed
in Django 4.0.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This makes it much more clear that this feature does JSON encoding,
which previously was only indicated in the documentation.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This adds the is_user_active with the appropriate code for setting the
value correctly in the future. In the following commit a migration to
backfill the value for existing Subscriptions will be added.
To ensure correct user_profile.is_active handling also in tests, we
replace all direct .is_active mutation with calls to appropriate
functions.
This commit migrates some of the backend tests to use assertLogs(),
instead of mock.patch() as planned in #15331.
Tweaked by tabbott to avoid tautological assertions.
There were some tests that had mock patches for logging, although no
logging was actually happening there. This commit removes such patches
in `corporate/tests/test_stripe.py`, `zerver/tests/test_cache.py`,
`zerver/tests/test_queue_worker.py`,
and `zerver/tests/test_signup.py`.
Lauryn Menard
Aman Agrawal
Mateusz Mandera
Tim Abbott
Prakhar Pratyush
Alex Vandiver
Alya Abbott
Prakhar Pratyush
Anders Kaseorg
Ujjawal Modi
Zixuan James Li
Zixuan Li
Eeshan Garg
Zixuan James Li
Lauryn Menard
Vishnu KS
Abhijeet Prasad Bodas
Siddharth Asthana
shanukun
m-e-l-u-h-a-n