Earlier, the 'handle_customer_migration_from_server_to_realms'
function was called during the send analytics step.
It resulted in an error for customers having multiple Zulip servers,
one for testing and the others for not-testing, sharing a
push bouncer registration.
The migration step when run in a test instance caused customers to
have their legacy plan migrated to a test realm, resulting in them
losing their legacy plan.
This commit moves the migration step to run during plan management
login step. This reduces the chances of losing legacy
plan as we expect them to only verify that 8.0 upgrade works and
not bother trying to login to plan management from their test instance.
Adds a support action for updating the minimum licenses on a
customer object once a default discount has also been set.
In the case that the current billing entity has a current active
plan or a scheduled upgrade to a new plan, then the minimum
licenses will not be updated.
Previously, the message string was sent as a success response to
the context, which could have been confusing or ignored when shown
in the support admin view.
- Make `self.write_to_audit_log` support a `background_update:
bool=False` parameter that can be passed when code that might have an
acting user happens to trigger a background update.
- Make `make_end_of_cycle_updates_if_needed` pass that parameter for its
direct audit log writes.
- Audit code that `make_end_of_cycle_updates_if_needed` calls and make
sure those write audit logs this way too.
- Pass the user in the `billing_page` code that had to avoid it as a
workaround:
```
# BUG: This should pass the acting_user; this is just working
# around that make_end_of_cycle_updates_if_needed doesn't do audit
# logging not using the session user properly.
billing_session = RealmBillingSession(user=None, realm=user.realm)
```
This prep commit adds a 'billing_session' field to StripeTest class.
Creates 'client_billing_get', 'client_billing_post', and
'client_billing_patch' helper functions.
This will help in reusing code for RemoteRealm and
RemoteZulipServer end-to-end tests.
This is a general link for logging into the billing system on behalf of
a server, but it's tied to the .contact_email and takes the user
straight to the /deactivate/ page via the next_page mechanism.
Given that most of the use cases for realms-only code path would
really like to upload audit logs too, and the others would likely
produce a better user experience if they upoaded audit logs, we
should just have a single main code path here i.e.
'send_analytics_to_push_bouncer'.
We still only upload usage statistics according to documented
option, and only from the analytics cron job.
The error handling takes place in 'send_analytics_to_push_bouncer'
itself.
I accidentally free trials for both cloud and self hosted
enabled while testing, hence didn't catch it.
This mostly involves fixing `is_free_trial_offer_enabled` to
return the correct value and providing it the correct input.
The way the flow goes now is this:
1. The user initiaties login via "Billing" in the gear menu.
2. That takes them to `/self-hosted-billing/` (possibly with a
`next_page` param if we use that for some gear menu options).
3. The server queries the bouncer to give the user a link with a signed
access token.
4. The user is redirected to that link (on `selfhosting.zulipchat.com`).
Now we have two cases, either the user is logging in for the first time
and already did in the past.
If this is the first time, we have:
5. The user is asked to fill in their email in a form that's shown,
pre-filled with the value provided inside the signed access token.
They POST this to the next endpoint.
6. The next endpoint sends a confirmation email to that address and asks
the user to go check their email.
7. The user clicks the link in their email is taken to the
from_confirmation endpoint.
8. Their initial RemoteBillingUser is created, a new signed link like in
(3) is generated and they're transparently taken back to (4),
where now that they have a RemoteBillingUser, they're handled
just like a user who already logged in before:
If the user already logged in before, they go straight here:
9. "Confirm login" page - they're shown their information (email and
full_name), can update
their full name in the form if they want. They also accept ToS here
if necessary. They POST this form back to
the endpoint and finally have a logged in session.
10. They're redirected to billing (or `next_page`) now that they have
access.
For the last form (with Full Name and ToS consent field), this pretty
shamelessly re-uses and directly renders the
corporate/remote_realm_billing_finalize_login_confirmation.html
template. That's probably good in terms of re-use, but calls for a
clean-up commit that will generalize the name of this template and the
classes/ids in the HTML.
If the RemoteRealmAuditLog has stale data, it means the server
stopped or never uploaded data. We raise MissingDataError in such
cases when a user action led to calculating licenses count from
stale data.
* Reformat "This is a legacy plan" notice on billing page.
* Add a link to the plan name on upgrade page title.
* Tweak discount style on billing page.
* Add line break to server login page title.
* Match server login page title and tab title.
* For free trial, don't show number of licenses for current billing period.
* For free trial scheduled to downgrade, don't show number of
licenses for next billing period.
This commit moves the 'update_license_ledger_if_needed' and its
helper function 'update_license_ledger_for_automanaged_plan'
to the 'BillingSession' abstract class.
This refactoring will help in minimizing duplicate code while
supporting both realm and remote_server customers.
Moves the 'update_license_ledger_for_manual_plan' function
to the 'BillingSession' abstract class.
This refactoring will help in minimizing duplicate code while
supporting both realm and remote_server customers.
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".
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.
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.
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.
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.
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.
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
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.
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.
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.
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.
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 `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.
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>