Skip to content
This repository was archived by the owner on May 5, 2025. It is now read-only.

Commit 40613b5

Browse files
authored
add root_organization property to worker, clean up decoration service, change notification urls for GL (#990)
1 parent 2c7bd18 commit 40613b5

File tree

15 files changed

+323
-82
lines changed

15 files changed

+323
-82
lines changed

database/models/core.py

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import uuid
44
from datetime import datetime
55
from functools import cached_property
6+
from typing import Optional
67

78
from shared.plan.constants import PlanName
89
from sqlalchemy import Column, ForeignKey, Index, UniqueConstraint, types
@@ -105,6 +106,7 @@ class Owner(CodecovBaseModel):
105106
avatar_url = Column(types.Text, server_default=FetchedValue())
106107
updatestamp = Column(types.DateTime, server_default=FetchedValue())
107108
parent_service_id = Column(types.Text, server_default=FetchedValue())
109+
root_parent_service_id = Column(types.Text, nullable=True)
108110
plan_provider = Column(types.Text, server_default=FetchedValue())
109111
trial_start_date = Column(types.DateTime, server_default=FetchedValue())
110112
trial_end_date = Column(types.DateTime, server_default=FetchedValue())
@@ -153,10 +155,43 @@ class Owner(CodecovBaseModel):
153155
)
154156

155157
@property
156-
def slug(self):
158+
def slug(self: "Owner") -> str:
157159
return self.username
158160

159-
def __repr__(self):
161+
@property
162+
def root_organization(self: "Owner") -> Optional["Owner"]:
163+
"""
164+
Find the root organization of Gitlab OwnerOrg, by using the root_parent_service_id
165+
if it exists, otherwise iterating through the parents and cache it in root_parent_service_id
166+
"""
167+
db_session = self.get_db_session()
168+
if self.root_parent_service_id:
169+
return self._get_owner_by_service_id(
170+
db_session, self.root_parent_service_id
171+
)
172+
173+
root = None
174+
if self.service == "gitlab" and self.parent_service_id:
175+
root = self
176+
while root.parent_service_id is not None:
177+
root = self._get_owner_by_service_id(db_session, root.parent_service_id)
178+
self.root_parent_service_id = root.service_id
179+
db_session.commit()
180+
return root
181+
182+
def _get_owner_by_service_id(
183+
self: "Owner", db_session: Session, service_id: str
184+
) -> "Owner":
185+
"""
186+
Helper method to fetch an Owner by service_id.
187+
"""
188+
return (
189+
db_session.query(Owner)
190+
.filter_by(service_id=service_id, service=self.service)
191+
.one()
192+
)
193+
194+
def __repr__(self: "Owner") -> str:
160195
return f"Owner<{self.ownerid}@service<{self.service}>>"
161196

162197

database/tests/factories/core.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import factory
66
from factory import Factory
7+
from shared.plan.constants import PlanName
78

89
from database import enums, models
910
from services.encryption import encryptor
@@ -49,6 +50,7 @@ class Meta:
4950
trial_status = enums.TrialStatus.NOT_STARTED.value
5051
trial_fired_by = None
5152
upload_token_required_for_public_repos = False
53+
plan = PlanName.BASIC_PLAN_NAME.value
5254

5355
oauth_token = factory.LazyAttribute(
5456
lambda o: encrypt_oauth_token(o.unencrypted_oauth_token)

database/tests/unit/test_models.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,48 @@ def test_upload_token_required_for_public_repos(self, dbsession):
191191
assert tokens_not_required_owner.onboarding_completed is True
192192
assert tokens_not_required_owner.upload_token_required_for_public_repos is False
193193

194+
def test_root_organization(self, dbsession):
195+
gitlab_root_group = OwnerFactory.create(
196+
username="root_group",
197+
service="gitlab",
198+
plan="users-pr-inappm",
199+
)
200+
dbsession.add(gitlab_root_group)
201+
gitlab_middle_group = OwnerFactory.create(
202+
username="mid_group",
203+
service="gitlab",
204+
parent_service_id=gitlab_root_group.service_id,
205+
root_parent_service_id=None,
206+
)
207+
dbsession.add(gitlab_middle_group)
208+
gitlab_subgroup = OwnerFactory.create(
209+
username="subgroup",
210+
service="gitlab",
211+
parent_service_id=gitlab_middle_group.service_id,
212+
root_parent_service_id=None,
213+
)
214+
dbsession.add(gitlab_subgroup)
215+
github_org = OwnerFactory.create(
216+
username="gh",
217+
service="github",
218+
)
219+
dbsession.add(github_org)
220+
dbsession.flush()
221+
222+
assert gitlab_root_group.root_organization is None
223+
assert gitlab_root_group.root_parent_service_id is None
224+
225+
assert gitlab_middle_group.root_organization == gitlab_root_group
226+
assert (
227+
gitlab_middle_group.root_parent_service_id == gitlab_root_group.service_id
228+
)
229+
230+
assert gitlab_subgroup.root_organization == gitlab_root_group
231+
assert gitlab_subgroup.root_parent_service_id == gitlab_root_group.service_id
232+
233+
assert github_org.root_organization is None
234+
assert github_org.root_parent_service_id is None
235+
194236

195237
class TestAccountModels(object):
196238
def test_create_account(self, dbsession):

requirements.in

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
https://github.com/codecov/test-results-parser/archive/190bbc8a911099749928e13d5fe57f6027ca1e74.tar.gz#egg=test-results-parser
2-
https://github.com/codecov/shared/archive/de4b37bc5a736317c6e7c93f9c58e9ae07f8c96b.tar.gz#egg=shared
2+
https://github.com/codecov/shared/archive/191837f5e35f5efc410e670aac7e50e0d09e43e1.tar.gz#egg=shared
33
https://github.com/codecov/timestring/archive/d37ceacc5954dff3b5bd2f887936a98a668dda42.tar.gz#egg=timestring
44
asgiref>=3.7.2
55
analytics-python==1.3.0b1

requirements.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,7 @@ sentry-sdk==2.13.0
375375
# shared
376376
setuptools==75.7.0
377377
# via nodeenv
378-
shared @ https://github.com/codecov/shared/archive/de4b37bc5a736317c6e7c93f9c58e9ae07f8c96b.tar.gz#egg=shared
378+
shared @ https://github.com/codecov/shared/archive/191837f5e35f5efc410e670aac7e50e0d09e43e1.tar.gz#egg=shared
379379
# via -r requirements.in
380380
six==1.16.0
381381
# via

services/bundle_analysis/notify/contexts/tests/test_comment_context.py

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
from shared.yaml import UserYaml
66

77
from database.models.core import GITHUB_APP_INSTALLATION_DEFAULT_NAME
8-
from database.tests.factories.core import CommitFactory
8+
from database.tests.factories.core import CommitFactory, PullFactory
99
from services.bundle_analysis.comparison import ComparisonLoader
1010
from services.bundle_analysis.notify.conftest import (
1111
get_commit_pair,
@@ -20,6 +20,7 @@
2020
from services.bundle_analysis.notify.contexts.comment import (
2121
BundleAnalysisPRCommentContextBuilder,
2222
)
23+
from services.repository import EnrichedPull
2324
from services.seats import SeatActivationInfo, ShouldActivateSeat
2425

2526

@@ -305,15 +306,27 @@ def test_build_context(self, dbsession, mocker, mock_storage):
305306
)
306307

307308
def test_initialize_from_context(self, dbsession, mocker):
308-
head_commit, _ = get_commit_pair(dbsession)
309+
head_commit, base_commit = get_commit_pair(dbsession)
309310
user_yaml = UserYaml.from_dict(PATCH_CENTRIC_DEFAULT_CONFIG)
310311
builder = BundleAnalysisPRCommentContextBuilder().initialize(
311312
head_commit, user_yaml, GITHUB_APP_INSTALLATION_DEFAULT_NAME
312313
)
313314
context = builder.get_result()
314315
context.commit_report = MagicMock(name="fake_commit_report")
315316
context.bundle_analysis_report = MagicMock(name="fake_bundle_analysis_report")
316-
context.pull = MagicMock(name="fake_pull")
317+
318+
pull = PullFactory(
319+
repository=base_commit.repository,
320+
head=head_commit.commitid,
321+
base=base_commit.commitid,
322+
compared_to=base_commit.commitid,
323+
)
324+
dbsession.add(pull)
325+
dbsession.commit()
326+
context.pull = EnrichedPull(
327+
database_pull=pull,
328+
provider_pull={},
329+
)
317330

318331
other_builder = BundleAnalysisPRCommentContextBuilder().initialize_from_context(
319332
user_yaml, context
@@ -327,7 +340,9 @@ def test_initialize_from_context(self, dbsession, mocker):
327340
with pytest.raises(ContextNotLoadedError):
328341
other_context.bundle_analysis_comparison
329342

330-
fake_comparison = MagicMock(name="fake_comparison", percentage_delta=10.0)
343+
fake_comparison = MagicMock(
344+
name="fake_comparison", percentage_delta=10.0, total_size_delta=10.0
345+
)
331346
mocker.patch.object(
332347
ComparisonLoader, "get_comparison", return_value=fake_comparison
333348
)

services/bundle_analysis/notify/contexts/tests/test_commit_status_context.py

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
from shared.yaml import UserYaml
77

88
from database.models.core import GITHUB_APP_INSTALLATION_DEFAULT_NAME
9-
from database.tests.factories.core import CommitFactory
9+
from database.tests.factories.core import CommitFactory, PullFactory
1010
from services.bundle_analysis.comparison import ComparisonLoader
1111
from services.bundle_analysis.notify.conftest import (
1212
get_commit_pair,
@@ -23,6 +23,7 @@
2323
CommitStatusNotificationContextBuilder,
2424
)
2525
from services.bundle_analysis.notify.types import NotificationUserConfig
26+
from services.repository import EnrichedPull
2627
from services.seats import SeatActivationInfo, ShouldActivateSeat
2728

2829

@@ -259,15 +260,26 @@ def test_build_context(self, dbsession, mocker, mock_storage):
259260
assert context.commit_status_url is not None
260261

261262
def test_initialize_from_context(self, dbsession, mocker):
262-
head_commit, _ = get_commit_pair(dbsession)
263+
head_commit, base_commit = get_commit_pair(dbsession)
263264
user_yaml = UserYaml.from_dict(PATCH_CENTRIC_DEFAULT_CONFIG)
264265
builder = CommitStatusNotificationContextBuilder().initialize(
265266
head_commit, user_yaml, GITHUB_APP_INSTALLATION_DEFAULT_NAME
266267
)
267268
context = builder.get_result()
268269
context.commit_report = MagicMock(name="fake_commit_report")
269270
context.bundle_analysis_report = MagicMock(name="fake_bundle_analysis_report")
270-
context.pull = MagicMock(name="fake_pull")
271+
pull = PullFactory(
272+
repository=base_commit.repository,
273+
head=head_commit.commitid,
274+
base=base_commit.commitid,
275+
compared_to=base_commit.commitid,
276+
)
277+
dbsession.add(pull)
278+
dbsession.commit()
279+
context.pull = EnrichedPull(
280+
database_pull=pull,
281+
provider_pull={},
282+
)
271283

272284
other_builder = (
273285
CommitStatusNotificationContextBuilder().initialize_from_context(

services/decoration.py

Lines changed: 12 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,9 @@
11
import logging
22
from dataclasses import dataclass
33

4-
from shared.billing import is_pr_billing_plan
54
from shared.config import get_config
65
from shared.plan.service import PlanService
76
from shared.upload.utils import query_monthly_coverage_measurements
8-
from sqlalchemy import func
97

108
from database.enums import Decoration
119
from database.models import Owner
@@ -31,8 +29,8 @@ class DecorationDetails(object):
3129
decoration_type: Decoration
3230
reason: str
3331
should_attempt_author_auto_activation: bool = False
34-
activation_org_ownerid: int = None
35-
activation_author_ownerid: int = None
32+
activation_org_ownerid: int | None = None
33+
activation_author_ownerid: int | None = None
3634

3735

3836
def _is_bot_account(author: Owner) -> bool:
@@ -50,7 +48,7 @@ def determine_uploads_used(plan_service: PlanService) -> int:
5048

5149
def determine_decoration_details(
5250
enriched_pull: EnrichedPull, empty_upload=None
53-
) -> dict:
51+
) -> DecorationDetails:
5452
"""
5553
Determine the decoration details from pull information. We also check if the pull author needs to be activated
5654
@@ -85,7 +83,7 @@ def determine_decoration_details(
8583
)
8684

8785
if db_pull.repository.private is False:
88-
# public repo or repo we arent certain is private should be standard
86+
# public repo or repo we aren't certain is private should be standard
8987
return DecorationDetails(
9088
decoration_type=Decoration.standard, reason="Public repo"
9189
)
@@ -94,19 +92,12 @@ def determine_decoration_details(
9492

9593
db_session = db_pull.get_db_session()
9694

97-
if org.service == "gitlab" and org.parent_service_id:
98-
# need to get root group so we can check plan info
99-
(gl_root_group,) = db_session.query(
100-
func.public.get_gitlab_root_group(org.ownerid)
101-
).first()
95+
# do not access plan directly - only through PlanService
96+
org_plan = PlanService(current_org=org)
97+
# use the org that has the plan - for GL this is the root_org rather than the repository.owner org
98+
org = org_plan.current_org
10299

103-
org = (
104-
db_session.query(Owner)
105-
.filter(Owner.ownerid == gl_root_group.get("ownerid"))
106-
.first()
107-
)
108-
109-
if not is_pr_billing_plan(org.plan):
100+
if not org_plan.is_pr_billing_plan:
110101
return DecorationDetails(
111102
decoration_type=Decoration.standard, reason="Org not on PR plan"
112103
)
@@ -134,13 +125,12 @@ def determine_decoration_details(
134125
reason="PR author not found in database",
135126
)
136127

137-
plan_service = PlanService(current_org=org)
138-
monthly_limit = plan_service.monthly_uploads_limit
128+
monthly_limit = org_plan.monthly_uploads_limit
139129
if monthly_limit is not None:
140-
uploads_used = determine_uploads_used(plan_service=plan_service)
130+
uploads_used = determine_uploads_used(plan_service=org_plan)
141131

142132
if (
143-
uploads_used >= plan_service.monthly_uploads_limit
133+
uploads_used >= org_plan.monthly_uploads_limit
144134
and not requires_license()
145135
):
146136
return DecorationDetails(

services/seats.py

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,7 @@
22
from dataclasses import dataclass
33
from enum import Enum
44

5-
from shared.billing import is_pr_billing_plan
6-
from sqlalchemy import func
5+
from shared.plan.service import PlanService
76
from sqlalchemy.orm import Session
87

98
from database.models import Owner
@@ -58,19 +57,13 @@ def determine_seat_activation(pull: EnrichedPull) -> SeatActivationInfo:
5857
org = db_pull.repository.owner
5958

6059
db_session: Session = db_pull.get_db_session()
61-
if org.service == "gitlab" and org.parent_service_id:
62-
# need to get root group so we can check plan info
63-
(gl_root_group,) = db_session.query(
64-
func.public.get_gitlab_root_group(org.ownerid)
65-
).first()
66-
67-
org = (
68-
db_session.query(Owner)
69-
.filter(Owner.ownerid == gl_root_group.get("ownerid"))
70-
.first()
71-
)
7260

73-
if not is_pr_billing_plan(org.plan):
61+
# do not access plan directly - only through PlanService
62+
org_plan = PlanService(current_org=org)
63+
# use the org that has the plan - for GL this is the root_org rather than the repository.owner org
64+
org = org_plan.current_org
65+
66+
if not org_plan.is_pr_billing_plan:
7467
return SeatActivationInfo(reason="no_pr_billing_plan")
7568

7669
pr_author = (

0 commit comments

Comments
 (0)