Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions backend/src/accounts/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -742,8 +742,6 @@ def save_model(self, request, obj, form, change):
with transaction.atomic():
if obj.billing_plan == BillingPlanType.PREMIUM:
max_users = obj.get_paid_users_count()
elif obj.billing_plan == BillingPlanType.FREEMIUM:
max_users = settings.DEFAULT_MAX_USERS

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Admin hardcodes free plan limit

Low Severity

Non-premium admin saves now always set max_users to a literal 1000 instead of settings.DEFAULT_MAX_USERS. That duplicates the new default and can drift if the setting changes again.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit baaa949. Configure here.

else:
max_users = 1000
obj = service.partial_update(
Expand Down
12 changes: 4 additions & 8 deletions backend/src/accounts/services/account.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,6 @@ def _create_tenant(
tenant_name: str,
) -> Account:

billing_enabled = (
settings.PROJECT_CONF['BILLING'] and master_account.billing_sync
)
account = Account(
is_verified=True,
name='Company name',
Expand All @@ -77,7 +74,7 @@ def _create_tenant(
):
account.billing_plan = BillingPlanType.FREEMIUM
elif master_account.billing_plan == BillingPlanType.UNLIMITED:
if billing_enabled:
if master_account.billing_sync:
# Need buy
account.billing_plan = None
else:
Expand All @@ -97,7 +94,7 @@ def _create_instance(
name: Optional[str] = None,
tenant_name: Optional[str] = None,
master_account: Optional[Account] = None,
billing_sync: bool = True,
billing_sync: bool = settings.PROJECT_CONF['BILLING'],
**kwargs,
) -> Account:

Expand All @@ -107,13 +104,12 @@ def _create_instance(
tenant_name=tenant_name,
)
else:
billing_enabled = settings.PROJECT_CONF['BILLING'] and billing_sync
self.instance = Account(
is_verified=is_verified,
name=name or 'Company name',
billing_sync=billing_sync,
)
if not billing_enabled:
if not billing_sync:
self.instance.billing_plan = BillingPlanType.FREEMIUM
self.instance.billing_sync = False
self.instance.save()
Expand Down Expand Up @@ -286,7 +282,7 @@ def partial_update(
],
)
self._update_tenants()
if settings.PROJECT_CONF['BILLING'] and self.instance.billing_sync:
if self.instance.billing_sync:
self._update_stripe_account(**update_kwargs)
self._identify_users()
self.group(user=self.user, account=self.instance)
Expand Down
3 changes: 1 addition & 2 deletions backend/src/accounts/services/convert_account.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
from typing import Optional

from django.conf import settings
from django.contrib.auth import get_user_model

from src.accounts.enums import (
Expand Down Expand Up @@ -59,7 +58,7 @@ def _standard_to_tenant(self):
)
master_account = self.instance.master_account
billing_enabled = (
settings.PROJECT_CONF['BILLING'] and master_account.billing_sync
master_account.billing_sync
)
update_kwargs = {
'logo_lg': master_account.logo_lg,
Expand Down
3 changes: 1 addition & 2 deletions backend/src/accounts/services/user_invite.py
Original file line number Diff line number Diff line change
Expand Up @@ -446,8 +446,7 @@ def accept(
)
account_service.update_users_counts()
if (
settings.PROJECT_CONF['BILLING']
and user.account.billing_sync
user.account.billing_sync
and user.account.billing_plan == BillingPlanType.PREMIUM
):
increase_plan_users.delay(
Expand Down
7 changes: 2 additions & 5 deletions backend/src/accounts/services/user_transfer.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
from django.conf import settings
from django.contrib.auth import get_user_model
from django.db import transaction
from rest_framework_simplejwt.exceptions import TokenError
Expand Down Expand Up @@ -80,8 +79,7 @@ def _get_valid_token(self, token_str: str) -> TransferToken:
def _after_transfer_actions(self):

if (
settings.PROJECT_CONF['BILLING']
and self.user.account.billing_sync
self.user.account.billing_sync
and self.user.account.billing_plan == BillingPlanType.PREMIUM
):
increase_plan_users.delay(
Expand Down Expand Up @@ -134,8 +132,7 @@ def _deactivate_prev_user(self):
)
self._delete_prev_user_pending_invites()
if (
settings.PROJECT_CONF['BILLING']
and self.prev_user.account.billing_sync
self.prev_user.account.billing_sync
and self.prev_user.is_account_owner
and self.prev_user.account.billing_plan in
BillingPlanType.PAYMENT_PLANS
Expand Down
144 changes: 8 additions & 136 deletions backend/src/accounts/tests/services/test_account.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,6 @@ def test_create_tenant__master_account_on_premium__inherit_plan(
):

# arrange
settings_mock = mocker.patch(
'src.accounts.services.account.settings',
)
settings_mock.PROJECT_CONF = {'BILLING': True}
trial_start = timezone.now() - timedelta(days=30)
trial_end = timezone.now() - timedelta(days=23)
trial_ended = True
Expand Down Expand Up @@ -90,10 +86,6 @@ def test_create_tenant__master_account_on_fractionalcoo__set_plan_freemium(
):

# arrange
settings_mock = mocker.patch(
'src.accounts.services.account.settings',
)
settings_mock.PROJECT_CONF = {'BILLING': True}
trial_start = timezone.now() - timedelta(days=30)
trial_end = timezone.now() - timedelta(days=23)
trial_ended = True
Expand Down Expand Up @@ -142,10 +134,6 @@ def test_create_tenant__master_account_on_freemium__set_plan_freemium(
):

# arrange
settings_mock = mocker.patch(
'src.accounts.services.account.settings',
)
settings_mock.PROJECT_CONF = {'BILLING': True}
master_account = create_test_account(
logo_lg='https://some.logo/image1.png',
logo_sm='https://some.logo/image2.png',
Expand Down Expand Up @@ -189,10 +177,6 @@ def test_create_tenant__master_account_on_freemium__set_plan_freemium(
def test_create_tenant__master_account_on_unlimited__set_plan_is_null(mocker):

# arrange
settings_mock = mocker.patch(
'src.accounts.services.account.settings',
)
settings_mock.PROJECT_CONF = {'BILLING': True}
trial_start = timezone.now() - timedelta(days=30)
trial_end = timezone.now() - timedelta(days=23)
trial_ended = True
Expand Down Expand Up @@ -235,16 +219,7 @@ def test_create_tenant__master_account_on_unlimited__set_plan_is_null(mocker):
assert tenant_account.trial_end is None
assert tenant_account.trial_ended is False


def test_create_tenant__master_account_on_unlimited_bs_disabled__inherit_plan(
mocker,
):

# arrange
settings_mock = mocker.patch(
'src.accounts.services.account.settings',
)
settings_mock.PROJECT_CONF = {'BILLING': False}
trial_ended = True
master_account = create_test_account(
logo_lg='https://some.logo/image1.png',
Expand Down Expand Up @@ -273,15 +248,15 @@ def test_create_tenant__master_account_on_unlimited_bs_disabled__inherit_plan(
assert tenant_account.lease_level == LeaseLevel.TENANT
assert tenant_account.logo_lg == master_account.logo_lg
assert tenant_account.logo_sm == master_account.logo_sm
assert tenant_account.max_users == master_account.max_users
assert tenant_account.billing_sync == master_account.billing_sync
assert tenant_account.max_users == 1000
assert tenant_account.billing_sync is True

assert tenant_account.billing_plan == master_account.billing_plan
assert tenant_account.billing_period == master_account.billing_period
assert tenant_account.plan_expiration == master_account.plan_expiration
assert tenant_account.trial_start == master_account.trial_start
assert tenant_account.trial_end == master_account.trial_end
assert tenant_account.trial_ended == trial_ended
assert tenant_account.billing_plan is None
assert tenant_account.billing_period is None
assert tenant_account.plan_expiration is None
assert tenant_account.trial_start is None
assert tenant_account.trial_end is None
assert tenant_account.trial_ended is False
Comment thread
pneumojoseph marked this conversation as resolved.


def test_create_instance__tenant__ok(mocker):
Expand Down Expand Up @@ -318,10 +293,6 @@ def test_create_instance__ok(mocker):
'src.accounts.services.account.AccountService.'
'_create_tenant',
)
settings_mock = mocker.patch(
'src.accounts.services.account.settings',
)
settings_mock.PROJECT_CONF = {'BILLING': True}
company_name = 'Name'
service = AccountService()

Expand Down Expand Up @@ -377,10 +348,6 @@ def test_create_instance__disable_billing_sync__set_freemium_plan(mocker):
'src.accounts.services.account.AccountService.'
'_create_tenant',
)
settings_mock = mocker.patch(
'src.accounts.services.account.settings',
)
settings_mock.PROJECT_CONF = {'BILLING': True}
company_name = 'Company'
service = AccountService()

Expand All @@ -405,41 +372,6 @@ def test_create_instance__disable_billing_sync__set_freemium_plan(mocker):
create_tenant_mock.assert_not_called()


def test_create_instance__disable_global_billing__set_freemium_plan(mocker):

# arrange
create_tenant_mock = mocker.patch(
'src.accounts.services.account.AccountService.'
'_create_tenant',
)
settings_mock = mocker.patch(
'src.accounts.services.account.settings',
)
settings_mock.PROJECT_CONF = {'BILLING': False}
company_name = 'Company'
service = AccountService()

# act
account = service._create_instance(
is_verified=False,
name=company_name,
billing_sync=True,
)

# assert
assert account.id
assert account.billing_sync is False
assert account.name == company_name
assert account.is_verified is False
assert account.billing_plan == BillingPlanType.FREEMIUM
assert account.billing_period is None
assert account.plan_expiration is None
assert account.trial_start is None
assert account.trial_end is None
assert account.trial_ended is False
create_tenant_mock.assert_not_called()


def test_create_related__not_utm__ok():

# arrange
Expand Down Expand Up @@ -803,10 +735,6 @@ def test_partial_update__ok(mocker):
instance=account,
user=user,
)
settings_mock = mocker.patch(
'src.accounts.services.account.settings',
)
settings_mock.PROJECT_CONF = {'BILLING': True}
group_mock = mocker.patch(
'src.analysis.mixins.BaseIdentifyMixin.group',
)
Expand Down Expand Up @@ -900,62 +828,6 @@ def test_partial_update__disabled_billing_sync__ok(mocker):
instance=account,
user=user,
)
settings_mock = mocker.patch(
'src.accounts.services.account.settings',
)
settings_mock.PROJECT_CONF = {'BILLING': True}
group_mock = mocker.patch(
'src.analysis.mixins.BaseIdentifyMixin.group',
)
update_stripe_account_mock = mocker.patch(
'src.accounts.services.account.'
'AccountService._update_stripe_account',
)
update_tenants_mock = mocker.patch(
'src.accounts.services.account.'
'AccountService._update_tenants',
)
identify_users_mock = mocker.patch(
'src.accounts.services.account.'
'AccountService._identify_users',
)

# act
result = service.partial_update(
name=name,
logo_lg=logo_lg,
logo_sm=logo_sm,
force_save=True,
)

# assert
account.refresh_from_db()
assert result.id == account.id
assert account.logo_lg == logo_lg
assert account.logo_sm == logo_sm
assert account.name == name
update_tenants_mock.assert_called_once()
update_stripe_account_mock.assert_not_called()
identify_users_mock.assert_called_once()
group_mock.assert_called_once_with(user=user, account=account)


def test_partial_update__disabled_billing__ok(mocker):

# arrange
account = create_test_account(lease_level=LeaseLevel.STANDARD)
user = create_test_user(account=account)
logo_lg = 'http://site.com/logo_lg.img'
logo_sm = 'http://site.com/logo_sm.img'
name = 'New account name'
service = AccountService(
instance=account,
user=user,
)
settings_mock = mocker.patch(
'src.accounts.services.account.settings',
)
settings_mock.PROJECT_CONF = {'BILLING': False}
group_mock = mocker.patch(
'src.analysis.mixins.BaseIdentifyMixin.group',
)
Expand Down
Loading