Skip to content

Commit 68b2fc7

Browse files
authored
update has_seats_left on plan service to work properly with self-hosted and DEC (#748)
1 parent 163d8cb commit 68b2fc7

File tree

4 files changed

+88
-17
lines changed

4 files changed

+88
-17
lines changed

graphql_api/tests/test_plan.py

+48-3
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ def test_plan_user_count_for_enterprise_org(self, mocked_license):
113113
is_valid=True,
114114
message=None,
115115
url="https://codeov.mysite.com",
116-
number_allowed_users=50,
116+
number_allowed_users=5,
117117
number_allowed_repos=10,
118118
expires=datetime.strptime("2020-05-09 00:00:00", "%Y-%m-%d %H:%M:%S"),
119119
is_trial=False,
@@ -131,6 +131,21 @@ def test_plan_user_count_for_enterprise_org(self, mocked_license):
131131
plan_user_count=1,
132132
plan_activated_users=[],
133133
)
134+
for i in range(4):
135+
new_owner = OwnerFactory()
136+
enterprise_org.plan_activated_users.append(new_owner.ownerid)
137+
enterprise_org.save()
138+
139+
other_org_in_enterprise = OwnerFactory(
140+
service="github",
141+
plan=PlanName.CODECOV_PRO_YEARLY.value,
142+
plan_user_count=1,
143+
plan_activated_users=[],
144+
)
145+
for i in range(4):
146+
new_owner = OwnerFactory()
147+
other_org_in_enterprise.plan_activated_users.append(new_owner.ownerid)
148+
other_org_in_enterprise.save()
134149

135150
query = """{
136151
owner(username: "%s") {
@@ -142,5 +157,35 @@ def test_plan_user_count_for_enterprise_org(self, mocked_license):
142157
}
143158
""" % (enterprise_org.username)
144159
data = self.gql_request(query, owner=enterprise_org)
145-
assert data["owner"]["plan"]["planUserCount"] == 50
146-
assert data["owner"]["plan"]["hasSeatsLeft"] == True
160+
assert data["owner"]["plan"]["planUserCount"] == 5
161+
assert data["owner"]["plan"]["hasSeatsLeft"] == False
162+
163+
@patch("services.self_hosted.get_current_license")
164+
def test_plan_user_count_for_enterprise_org_invaild_license(self, mocked_license):
165+
mock_enterprise_license = LicenseInformation(
166+
is_valid=False,
167+
)
168+
mocked_license.return_value = mock_enterprise_license
169+
mock_config_helper(
170+
self.mocker, configs={"setup.enterprise_license": mock_enterprise_license}
171+
)
172+
173+
enterprise_org = OwnerFactory(
174+
username="random-plan-user",
175+
service="github",
176+
plan=PlanName.CODECOV_PRO_YEARLY.value,
177+
plan_user_count=1,
178+
plan_activated_users=[],
179+
)
180+
query = """{
181+
owner(username: "%s") {
182+
plan {
183+
planUserCount
184+
hasSeatsLeft
185+
}
186+
}
187+
}
188+
""" % (enterprise_org.username)
189+
data = self.gql_request(query, owner=enterprise_org)
190+
assert data["owner"]["plan"]["planUserCount"] == 0
191+
assert data["owner"]["plan"]["hasSeatsLeft"] == False

plan/service.py

+3-1
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
TrialStatus,
2222
)
2323
from services import sentry
24-
from services.self_hosted import license_seats
24+
from services.self_hosted import enterprise_has_seats_left, license_seats
2525
from utils.config import get_config
2626

2727
log = logging.getLogger(__name__)
@@ -267,6 +267,8 @@ def has_trial_dates(self) -> bool:
267267

268268
@property
269269
def has_seats_left(self) -> bool:
270+
if get_config("setup", "enterprise_license"):
271+
return enterprise_has_seats_left()
270272
return (
271273
self.plan_activated_users is None
272274
or len(self.plan_activated_users) < self.plan_user_count

services/self_hosted.py

+33-11
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
from django.db.models import F, Func, Q, QuerySet
88
from shared.license import get_current_license
99

10+
from codecov.db import sync_to_async
1011
from codecov_auth.models import Owner
1112
from services import ServiceException
1213
from utils.config import get_config
@@ -44,16 +45,16 @@ def admin_owners() -> QuerySet:
4445

4546
def is_admin_owner(owner: Optional[Owner]) -> bool:
4647
"""
47-
Returns true iff the given owner is an admin.
48+
Returns true if the given owner is an admin.
4849
"""
4950
return owner is not None and admin_owners().filter(pk=owner.pk).exists()
5051

5152

52-
def activated_owners() -> QuerySet:
53-
"""
54-
Returns all owners that are activated in ANY org's `plan_activated_users`
55-
across the entire instance.
56-
"""
53+
async def activated_owners_async() -> QuerySet:
54+
return await sync_to_async(activated_owner_query)()
55+
56+
57+
def activated_owner_query() -> QuerySet:
5758
owner_ids = (
5859
Owner.objects.annotate(
5960
plan_activated_owner_ids=Func(
@@ -64,13 +65,20 @@ def activated_owners() -> QuerySet:
6465
.values_list("plan_activated_owner_ids", flat=True)
6566
.distinct()
6667
)
67-
6868
return Owner.objects.filter(pk__in=owner_ids)
6969

7070

71+
def activated_owners() -> QuerySet:
72+
"""
73+
Returns all owners that are activated in ANY org's `plan_activated_users`
74+
across the entire instance.
75+
"""
76+
return activated_owner_query()
77+
78+
7179
def is_activated_owner(owner: Owner) -> bool:
7280
"""
73-
Returns true iff the given owner is activated in this instance.
81+
Returns true if the given owner is activated in this instance.
7482
"""
7583
return activated_owners().filter(pk=owner.pk).exists()
7684

@@ -79,13 +87,27 @@ def license_seats() -> int:
7987
"""
8088
Max number of seats allowed by the current license.
8189
"""
82-
license = get_current_license()
83-
return license.number_allowed_users or 0
90+
enterprise_license = get_current_license()
91+
if not enterprise_license.is_valid:
92+
return 0
93+
return enterprise_license.number_allowed_users or 0
94+
95+
96+
async def enterprise_has_seats_left() -> bool:
97+
"""
98+
The activated_owner_query is heavy, so check the license first, only proceed if they have a valid license.
99+
"""
100+
license_seat_count = license_seats()
101+
if license_seat_count == 0:
102+
return False
103+
owners = await activated_owners_async()
104+
count = await sync_to_async(owners.count)()
105+
return count < license_seat_count
84106

85107

86108
def can_activate_owner(owner: Owner) -> bool:
87109
"""
88-
Returns true iff there are available seats left for activation.
110+
Returns true if there are available seats left for activation.
89111
"""
90112
if is_activated_owner(owner):
91113
# user is already activated in at least 1 org

services/tests/test_self_hosted.py

+4-2
Original file line numberDiff line numberDiff line change
@@ -87,12 +87,14 @@ def test_is_activated_owner(self, activated_owners):
8787

8888
@patch("services.self_hosted.get_current_license")
8989
def test_license_seats(self, get_current_license):
90-
get_current_license.return_value = LicenseInformation(number_allowed_users=123)
90+
get_current_license.return_value = LicenseInformation(
91+
number_allowed_users=123, is_valid=True
92+
)
9193
assert license_seats() == 123
9294

9395
@patch("services.self_hosted.get_current_license")
9496
def test_license_seats_not_specified(self, get_current_license):
95-
get_current_license.return_value = LicenseInformation()
97+
get_current_license.return_value = LicenseInformation(is_valid=True)
9698
assert license_seats() == 0
9799

98100
@patch("services.self_hosted.activated_owners")

0 commit comments

Comments
 (0)