diff --git a/database/models/core.py b/database/models/core.py index 5e2e85e59..3fdfa2c89 100644 --- a/database/models/core.py +++ b/database/models/core.py @@ -3,6 +3,7 @@ import uuid from datetime import datetime from functools import cached_property +from typing import Optional from shared.plan.constants import PlanName from sqlalchemy import Column, ForeignKey, Index, UniqueConstraint, types @@ -105,6 +106,7 @@ class Owner(CodecovBaseModel): avatar_url = Column(types.Text, server_default=FetchedValue()) updatestamp = Column(types.DateTime, server_default=FetchedValue()) parent_service_id = Column(types.Text, server_default=FetchedValue()) + root_parent_service_id = Column(types.Text, nullable=True) plan_provider = Column(types.Text, server_default=FetchedValue()) trial_start_date = Column(types.DateTime, server_default=FetchedValue()) trial_end_date = Column(types.DateTime, server_default=FetchedValue()) @@ -153,10 +155,43 @@ class Owner(CodecovBaseModel): ) @property - def slug(self): + def slug(self: "Owner") -> str: return self.username - def __repr__(self): + @property + def root_organization(self: "Owner") -> Optional["Owner"]: + """ + Find the root organization of Gitlab OwnerOrg, by using the root_parent_service_id + if it exists, otherwise iterating through the parents and cache it in root_parent_service_id + """ + db_session = self.get_db_session() + if self.root_parent_service_id: + return self._get_owner_by_service_id( + db_session, self.root_parent_service_id + ) + + root = None + if self.service == "gitlab" and self.parent_service_id: + root = self + while root.parent_service_id is not None: + root = self._get_owner_by_service_id(db_session, root.parent_service_id) + self.root_parent_service_id = root.service_id + db_session.commit() + return root + + def _get_owner_by_service_id( + self: "Owner", db_session: Session, service_id: str + ) -> "Owner": + """ + Helper method to fetch an Owner by service_id. + """ + return ( + db_session.query(Owner) + .filter_by(service_id=service_id, service=self.service) + .one() + ) + + def __repr__(self: "Owner") -> str: return f"Owner<{self.ownerid}@service<{self.service}>>" diff --git a/database/tests/factories/core.py b/database/tests/factories/core.py index d62ecae2f..38905a3dd 100644 --- a/database/tests/factories/core.py +++ b/database/tests/factories/core.py @@ -4,6 +4,7 @@ import factory from factory import Factory +from shared.plan.constants import PlanName from database import enums, models from services.encryption import encryptor @@ -49,6 +50,7 @@ class Meta: trial_status = enums.TrialStatus.NOT_STARTED.value trial_fired_by = None upload_token_required_for_public_repos = False + plan = PlanName.BASIC_PLAN_NAME.value oauth_token = factory.LazyAttribute( lambda o: encrypt_oauth_token(o.unencrypted_oauth_token) diff --git a/database/tests/unit/test_models.py b/database/tests/unit/test_models.py index 60404b881..ce2bea1f3 100644 --- a/database/tests/unit/test_models.py +++ b/database/tests/unit/test_models.py @@ -191,6 +191,48 @@ def test_upload_token_required_for_public_repos(self, dbsession): assert tokens_not_required_owner.onboarding_completed is True assert tokens_not_required_owner.upload_token_required_for_public_repos is False + def test_root_organization(self, dbsession): + gitlab_root_group = OwnerFactory.create( + username="root_group", + service="gitlab", + plan="users-pr-inappm", + ) + dbsession.add(gitlab_root_group) + gitlab_middle_group = OwnerFactory.create( + username="mid_group", + service="gitlab", + parent_service_id=gitlab_root_group.service_id, + root_parent_service_id=None, + ) + dbsession.add(gitlab_middle_group) + gitlab_subgroup = OwnerFactory.create( + username="subgroup", + service="gitlab", + parent_service_id=gitlab_middle_group.service_id, + root_parent_service_id=None, + ) + dbsession.add(gitlab_subgroup) + github_org = OwnerFactory.create( + username="gh", + service="github", + ) + dbsession.add(github_org) + dbsession.flush() + + assert gitlab_root_group.root_organization is None + assert gitlab_root_group.root_parent_service_id is None + + assert gitlab_middle_group.root_organization == gitlab_root_group + assert ( + gitlab_middle_group.root_parent_service_id == gitlab_root_group.service_id + ) + + assert gitlab_subgroup.root_organization == gitlab_root_group + assert gitlab_subgroup.root_parent_service_id == gitlab_root_group.service_id + + assert github_org.root_organization is None + assert github_org.root_parent_service_id is None + class TestAccountModels(object): def test_create_account(self, dbsession): diff --git a/requirements.in b/requirements.in index fce39b083..9f737e5fd 100644 --- a/requirements.in +++ b/requirements.in @@ -1,5 +1,5 @@ https://github.com/codecov/test-results-parser/archive/190bbc8a911099749928e13d5fe57f6027ca1e74.tar.gz#egg=test-results-parser -https://github.com/codecov/shared/archive/de4b37bc5a736317c6e7c93f9c58e9ae07f8c96b.tar.gz#egg=shared +https://github.com/codecov/shared/archive/191837f5e35f5efc410e670aac7e50e0d09e43e1.tar.gz#egg=shared https://github.com/codecov/timestring/archive/d37ceacc5954dff3b5bd2f887936a98a668dda42.tar.gz#egg=timestring asgiref>=3.7.2 analytics-python==1.3.0b1 diff --git a/requirements.txt b/requirements.txt index 87a7cc6ab..34ecf93ad 100644 --- a/requirements.txt +++ b/requirements.txt @@ -375,7 +375,7 @@ sentry-sdk==2.13.0 # shared setuptools==75.7.0 # via nodeenv -shared @ https://github.com/codecov/shared/archive/de4b37bc5a736317c6e7c93f9c58e9ae07f8c96b.tar.gz#egg=shared +shared @ https://github.com/codecov/shared/archive/191837f5e35f5efc410e670aac7e50e0d09e43e1.tar.gz#egg=shared # via -r requirements.in six==1.16.0 # via diff --git a/services/bundle_analysis/notify/contexts/tests/test_comment_context.py b/services/bundle_analysis/notify/contexts/tests/test_comment_context.py index 8faea7cb4..eb77b2415 100644 --- a/services/bundle_analysis/notify/contexts/tests/test_comment_context.py +++ b/services/bundle_analysis/notify/contexts/tests/test_comment_context.py @@ -5,7 +5,7 @@ from shared.yaml import UserYaml from database.models.core import GITHUB_APP_INSTALLATION_DEFAULT_NAME -from database.tests.factories.core import CommitFactory +from database.tests.factories.core import CommitFactory, PullFactory from services.bundle_analysis.comparison import ComparisonLoader from services.bundle_analysis.notify.conftest import ( get_commit_pair, @@ -20,6 +20,7 @@ from services.bundle_analysis.notify.contexts.comment import ( BundleAnalysisPRCommentContextBuilder, ) +from services.repository import EnrichedPull from services.seats import SeatActivationInfo, ShouldActivateSeat @@ -305,7 +306,7 @@ def test_build_context(self, dbsession, mocker, mock_storage): ) def test_initialize_from_context(self, dbsession, mocker): - head_commit, _ = get_commit_pair(dbsession) + head_commit, base_commit = get_commit_pair(dbsession) user_yaml = UserYaml.from_dict(PATCH_CENTRIC_DEFAULT_CONFIG) builder = BundleAnalysisPRCommentContextBuilder().initialize( head_commit, user_yaml, GITHUB_APP_INSTALLATION_DEFAULT_NAME @@ -313,7 +314,19 @@ def test_initialize_from_context(self, dbsession, mocker): context = builder.get_result() context.commit_report = MagicMock(name="fake_commit_report") context.bundle_analysis_report = MagicMock(name="fake_bundle_analysis_report") - context.pull = MagicMock(name="fake_pull") + + pull = PullFactory( + repository=base_commit.repository, + head=head_commit.commitid, + base=base_commit.commitid, + compared_to=base_commit.commitid, + ) + dbsession.add(pull) + dbsession.commit() + context.pull = EnrichedPull( + database_pull=pull, + provider_pull={}, + ) other_builder = BundleAnalysisPRCommentContextBuilder().initialize_from_context( user_yaml, context @@ -327,7 +340,9 @@ def test_initialize_from_context(self, dbsession, mocker): with pytest.raises(ContextNotLoadedError): other_context.bundle_analysis_comparison - fake_comparison = MagicMock(name="fake_comparison", percentage_delta=10.0) + fake_comparison = MagicMock( + name="fake_comparison", percentage_delta=10.0, total_size_delta=10.0 + ) mocker.patch.object( ComparisonLoader, "get_comparison", return_value=fake_comparison ) diff --git a/services/bundle_analysis/notify/contexts/tests/test_commit_status_context.py b/services/bundle_analysis/notify/contexts/tests/test_commit_status_context.py index 37edee934..afb5be30c 100644 --- a/services/bundle_analysis/notify/contexts/tests/test_commit_status_context.py +++ b/services/bundle_analysis/notify/contexts/tests/test_commit_status_context.py @@ -6,7 +6,7 @@ from shared.yaml import UserYaml from database.models.core import GITHUB_APP_INSTALLATION_DEFAULT_NAME -from database.tests.factories.core import CommitFactory +from database.tests.factories.core import CommitFactory, PullFactory from services.bundle_analysis.comparison import ComparisonLoader from services.bundle_analysis.notify.conftest import ( get_commit_pair, @@ -23,6 +23,7 @@ CommitStatusNotificationContextBuilder, ) from services.bundle_analysis.notify.types import NotificationUserConfig +from services.repository import EnrichedPull from services.seats import SeatActivationInfo, ShouldActivateSeat @@ -259,7 +260,7 @@ def test_build_context(self, dbsession, mocker, mock_storage): assert context.commit_status_url is not None def test_initialize_from_context(self, dbsession, mocker): - head_commit, _ = get_commit_pair(dbsession) + head_commit, base_commit = get_commit_pair(dbsession) user_yaml = UserYaml.from_dict(PATCH_CENTRIC_DEFAULT_CONFIG) builder = CommitStatusNotificationContextBuilder().initialize( head_commit, user_yaml, GITHUB_APP_INSTALLATION_DEFAULT_NAME @@ -267,7 +268,18 @@ def test_initialize_from_context(self, dbsession, mocker): context = builder.get_result() context.commit_report = MagicMock(name="fake_commit_report") context.bundle_analysis_report = MagicMock(name="fake_bundle_analysis_report") - context.pull = MagicMock(name="fake_pull") + pull = PullFactory( + repository=base_commit.repository, + head=head_commit.commitid, + base=base_commit.commitid, + compared_to=base_commit.commitid, + ) + dbsession.add(pull) + dbsession.commit() + context.pull = EnrichedPull( + database_pull=pull, + provider_pull={}, + ) other_builder = ( CommitStatusNotificationContextBuilder().initialize_from_context( diff --git a/services/decoration.py b/services/decoration.py index e2df645aa..9fe6bc0d6 100644 --- a/services/decoration.py +++ b/services/decoration.py @@ -1,11 +1,9 @@ import logging from dataclasses import dataclass -from shared.billing import is_pr_billing_plan from shared.config import get_config from shared.plan.service import PlanService from shared.upload.utils import query_monthly_coverage_measurements -from sqlalchemy import func from database.enums import Decoration from database.models import Owner @@ -31,8 +29,8 @@ class DecorationDetails(object): decoration_type: Decoration reason: str should_attempt_author_auto_activation: bool = False - activation_org_ownerid: int = None - activation_author_ownerid: int = None + activation_org_ownerid: int | None = None + activation_author_ownerid: int | None = None def _is_bot_account(author: Owner) -> bool: @@ -50,7 +48,7 @@ def determine_uploads_used(plan_service: PlanService) -> int: def determine_decoration_details( enriched_pull: EnrichedPull, empty_upload=None -) -> dict: +) -> DecorationDetails: """ Determine the decoration details from pull information. We also check if the pull author needs to be activated @@ -85,7 +83,7 @@ def determine_decoration_details( ) if db_pull.repository.private is False: - # public repo or repo we arent certain is private should be standard + # public repo or repo we aren't certain is private should be standard return DecorationDetails( decoration_type=Decoration.standard, reason="Public repo" ) @@ -94,19 +92,12 @@ def determine_decoration_details( db_session = db_pull.get_db_session() - if org.service == "gitlab" and org.parent_service_id: - # need to get root group so we can check plan info - (gl_root_group,) = db_session.query( - func.public.get_gitlab_root_group(org.ownerid) - ).first() + # do not access plan directly - only through PlanService + org_plan = PlanService(current_org=org) + # use the org that has the plan - for GL this is the root_org rather than the repository.owner org + org = org_plan.current_org - org = ( - db_session.query(Owner) - .filter(Owner.ownerid == gl_root_group.get("ownerid")) - .first() - ) - - if not is_pr_billing_plan(org.plan): + if not org_plan.is_pr_billing_plan: return DecorationDetails( decoration_type=Decoration.standard, reason="Org not on PR plan" ) @@ -134,13 +125,12 @@ def determine_decoration_details( reason="PR author not found in database", ) - plan_service = PlanService(current_org=org) - monthly_limit = plan_service.monthly_uploads_limit + monthly_limit = org_plan.monthly_uploads_limit if monthly_limit is not None: - uploads_used = determine_uploads_used(plan_service=plan_service) + uploads_used = determine_uploads_used(plan_service=org_plan) if ( - uploads_used >= plan_service.monthly_uploads_limit + uploads_used >= org_plan.monthly_uploads_limit and not requires_license() ): return DecorationDetails( diff --git a/services/seats.py b/services/seats.py index ce4b753ee..3c2612a6c 100644 --- a/services/seats.py +++ b/services/seats.py @@ -2,8 +2,7 @@ from dataclasses import dataclass from enum import Enum -from shared.billing import is_pr_billing_plan -from sqlalchemy import func +from shared.plan.service import PlanService from sqlalchemy.orm import Session from database.models import Owner @@ -58,19 +57,13 @@ def determine_seat_activation(pull: EnrichedPull) -> SeatActivationInfo: org = db_pull.repository.owner db_session: Session = db_pull.get_db_session() - if org.service == "gitlab" and org.parent_service_id: - # need to get root group so we can check plan info - (gl_root_group,) = db_session.query( - func.public.get_gitlab_root_group(org.ownerid) - ).first() - - org = ( - db_session.query(Owner) - .filter(Owner.ownerid == gl_root_group.get("ownerid")) - .first() - ) - if not is_pr_billing_plan(org.plan): + # do not access plan directly - only through PlanService + org_plan = PlanService(current_org=org) + # use the org that has the plan - for GL this is the root_org rather than the repository.owner org + org = org_plan.current_org + + if not org_plan.is_pr_billing_plan: return SeatActivationInfo(reason="no_pr_billing_plan") pr_author = ( diff --git a/services/tests/test_billing.py b/services/tests/test_billing.py index 1dd6051a7..f4fe367f5 100644 --- a/services/tests/test_billing.py +++ b/services/tests/test_billing.py @@ -1,15 +1,21 @@ from django.test import override_settings -from shared.billing import BillingPlan, is_pr_billing_plan +from shared.plan.constants import PlanName +from shared.plan.service import PlanService from database.tests.factories import OwnerFactory class TestBillingServiceTestCase(object): + """ + BillingService is deprecated - use PlanService instead. + """ + def test_pr_author_plan_check(self, request, dbsession, with_sql_functions): owner = OwnerFactory.create(service="github", plan="users-pr-inappm") dbsession.add(owner) dbsession.flush() - assert is_pr_billing_plan(owner.plan) + plan = PlanService(owner) + assert plan.is_pr_billing_plan @override_settings(IS_ENTERPRISE=True) def test_pr_author_enterprise_plan_check( @@ -25,16 +31,20 @@ def test_pr_author_enterprise_plan_check( "https://codecov.mysite.com" ) - assert is_pr_billing_plan(owner.plan) + plan = PlanService(owner) + + assert plan.is_pr_billing_plan def test_plan_not_pr_author(self, request, dbsession, with_sql_functions): owner = OwnerFactory.create( - service="github", plan=BillingPlan.users_monthly.value + service="github", plan=PlanName.CODECOV_PRO_MONTHLY_LEGACY.value ) dbsession.add(owner) dbsession.flush() - assert not is_pr_billing_plan(owner.plan) + plan = PlanService(owner) + + assert not plan.is_pr_billing_plan @override_settings(IS_ENTERPRISE=True) def test_pr_author_enterprise_plan_check_non_pr_plan( @@ -49,5 +59,6 @@ def test_pr_author_enterprise_plan_check_non_pr_plan( mock_configuration.params["setup"]["codecov_dashboard_url"] = ( "https://codeov.mysite.com" ) + plan = PlanService(owner) - assert not is_pr_billing_plan(owner.plan) + assert not plan.is_pr_billing_plan diff --git a/services/tests/test_decoration.py b/services/tests/test_decoration.py index 9a1bc6eae..daef67d9e 100644 --- a/services/tests/test_decoration.py +++ b/services/tests/test_decoration.py @@ -97,6 +97,8 @@ def gitlab_root_group(dbsession): unencrypted_oauth_token="testtlxuu2kfef3km1fbecdlmnb2nvpikvmoadi3", plan="users-pr-inappm", plan_activated_users=[], + plan_auto_activate=False, + plan_user_count=3, ) dbsession.add(root_group) dbsession.flush() @@ -104,13 +106,31 @@ def gitlab_root_group(dbsession): @pytest.fixture -def gitlab_enriched_pull_subgroup(dbsession, gitlab_root_group): +def gitlab_middle_group(dbsession, gitlab_root_group): + mid_group = OwnerFactory.create( + username="mid_group", + service="gitlab", + unencrypted_oauth_token="testtlxuu2kfef3km1fbecdlmnb2nvpikvmoadi4", + plan="users-pr-inappy", + plan_activated_users=[], + parent_service_id=gitlab_root_group.service_id, + plan_auto_activate=True, + ) + dbsession.add(mid_group) + dbsession.flush() + return mid_group + + +@pytest.fixture +def gitlab_enriched_pull_subgroup(dbsession, gitlab_middle_group): subgroup = OwnerFactory.create( username="subgroup", service="gitlab", unencrypted_oauth_token="testtlxuu2kfef3km1fbecdlmnb2nvpikvmoadi3", plan=None, - parent_service_id=gitlab_root_group.service_id, + parent_service_id=gitlab_middle_group.service_id, + plan_activated_users=[], + plan_auto_activate=True, ) dbsession.add(subgroup) dbsession.flush() @@ -779,9 +799,10 @@ def test_get_decoration_type_pr_author_manual_activation_required_gitlab_subgrou gitlab_enriched_pull_subgroup, with_sql_functions, ): - gitlab_root_group.plan_user_count = 3 - gitlab_root_group.plan_activated_users = [] gitlab_root_group.plan_auto_activate = False + # setting on child group should not matter, uses setting from root + child_group = gitlab_enriched_pull_subgroup.database_pull.repository.owner + child_group.plan_auto_activate = True pr_author = OwnerFactory.create( username=gitlab_enriched_pull_subgroup.provider_pull["author"]["username"], @@ -797,13 +818,23 @@ def test_get_decoration_type_pr_author_manual_activation_required_gitlab_subgrou assert decoration_details.decoration_type == Decoration.upgrade assert decoration_details.reason == "User must be manually activated" assert decoration_details.should_attempt_author_auto_activation is False + assert decoration_details.activation_org_ownerid is None + assert decoration_details.activation_author_ownerid is None + + # allow auto-activate on root + gitlab_root_group.plan_auto_activate = True + # setting on child group should not matter, uses setting from root + child_group.plan_auto_activate = False + decoration_details = determine_decoration_details(gitlab_enriched_pull_subgroup) + dbsession.commit() + assert decoration_details.decoration_type == Decoration.upgrade + assert decoration_details.reason == "User must be activated" + assert decoration_details.should_attempt_author_auto_activation is True + assert decoration_details.activation_org_ownerid == gitlab_root_group.ownerid + assert decoration_details.activation_author_ownerid == pr_author.ownerid + # activation hasn't happened yet assert pr_author.ownerid not in gitlab_root_group.plan_activated_users - # shouldn't be in subgroup plan_activated_users either - assert ( - pr_author.ownerid - not in gitlab_enriched_pull_subgroup.database_pull.repository.owner.plan_activated_users - ) def test_get_decoration_type_pr_author_already_active_subgroup( self, @@ -820,7 +851,6 @@ def test_get_decoration_type_pr_author_already_active_subgroup( ) dbsession.add(pr_author) dbsession.flush() - gitlab_root_group.plan_user_count = 3 gitlab_root_group.plan_activated_users = [pr_author.ownerid] gitlab_root_group.plan_auto_activate = False dbsession.flush() @@ -831,6 +861,8 @@ def test_get_decoration_type_pr_author_already_active_subgroup( assert decoration_details.decoration_type == Decoration.standard assert decoration_details.reason == "User is currently activated" assert decoration_details.should_attempt_author_auto_activation is False + assert decoration_details.activation_org_ownerid is None + assert decoration_details.activation_author_ownerid is None def test_get_decoration_type_should_attempt_pr_author_auto_activation( self, @@ -860,7 +892,7 @@ def test_get_decoration_type_should_attempt_pr_author_auto_activation( assert decoration_details.should_attempt_author_auto_activation is True assert decoration_details.activation_org_ownerid == gitlab_root_group.ownerid assert decoration_details.activation_author_ownerid == pr_author.ownerid - # activation hasnt happened yet + # activation hasn't happened yet assert pr_author.ownerid not in gitlab_root_group.plan_activated_users def test_get_decoration_type_owner_activated_users_null( @@ -923,3 +955,47 @@ def test_uploads_used_with_expired_trial(self, mocker, dbsession): uploads_used = determine_uploads_used(plan_service=plan_service) assert uploads_used == 0 + + def test_author_is_activated_on_subgroup_not_root( + self, dbsession, gitlab_root_group, gitlab_enriched_pull_subgroup + ): + pr_author = OwnerFactory.create( + username=gitlab_enriched_pull_subgroup.provider_pull["author"]["username"], + service="gitlab", + service_id=gitlab_enriched_pull_subgroup.provider_pull["author"]["id"], + ) + dbsession.add(pr_author) + dbsession.flush() + + # user is activated on subgroup but not root group and root group does not auto activate + gitlab_root_group.plan_auto_activate = False + child_group = gitlab_enriched_pull_subgroup.database_pull.repository.owner + child_group.plan_auto_activate = False + child_group.plan_activated_users = [pr_author.ownerid] + dbsession.flush() + + decoration_details = determine_decoration_details(gitlab_enriched_pull_subgroup) + dbsession.commit() + + assert decoration_details.decoration_type == Decoration.upgrade + assert decoration_details.reason == "User must be manually activated" + assert decoration_details.should_attempt_author_auto_activation is False + assert decoration_details.activation_org_ownerid is None + assert decoration_details.activation_author_ownerid is None + + assert pr_author.ownerid not in gitlab_root_group.plan_activated_users + assert ( + pr_author.ownerid + in gitlab_enriched_pull_subgroup.database_pull.repository.owner.plan_activated_users + ) + + # allow auto-activate on root for user to get non-blocking decoration + gitlab_root_group.plan_auto_activate = True + decoration_details = determine_decoration_details(gitlab_enriched_pull_subgroup) + dbsession.commit() + + assert decoration_details.decoration_type == Decoration.upgrade + assert decoration_details.reason == "User must be activated" + assert decoration_details.should_attempt_author_auto_activation is True + assert decoration_details.activation_org_ownerid == gitlab_root_group.ownerid + assert decoration_details.activation_author_ownerid == pr_author.ownerid diff --git a/services/tests/test_urls.py b/services/tests/test_urls.py index 144d1ea16..54ab2f1e9 100644 --- a/services/tests/test_urls.py +++ b/services/tests/test_urls.py @@ -1,4 +1,5 @@ -from services.urls import append_tracking_params_to_urls +from database.tests.factories import OwnerFactory, PullFactory, RepositoryFactory +from services.urls import append_tracking_params_to_urls, get_members_url, get_plan_url def test_append_tracking_params_to_urls(): @@ -29,3 +30,60 @@ def test_append_tracking_params_to_urls(): ] assert result == expected_result + + +class TestURLs(object): + def test_gitlab_url_username_swap(self, dbsession): + base_for_member_url = "https://app.codecov.io/members/" + base_for_plan_url = "https://app.codecov.io/plan/" + + github_org = OwnerFactory.create( + service="github", + username="gh", + ) + dbsession.add(github_org) + r = RepositoryFactory.create(owner=github_org) + dbsession.add(r) + gh_pull = PullFactory.create(repository=r) + dbsession.add(gh_pull) + dbsession.flush() + member_url = get_members_url(gh_pull) + assert member_url == base_for_member_url + "gh/gh" + + gitlab_root_org = OwnerFactory.create(service="gitlab", username="gl_root") + dbsession.add(gitlab_root_org) + r = RepositoryFactory.create(owner=gitlab_root_org) + dbsession.add(r) + gl_root_pull = PullFactory.create(repository=r) + dbsession.add(gl_root_pull) + dbsession.flush() + plan_url = get_plan_url(gl_root_pull) + assert plan_url == base_for_plan_url + "gl/gl_root" + + gitlab_mid_org = OwnerFactory.create( + service="gitlab", + username="gl_mid", + parent_service_id=gitlab_root_org.service_id, + ) + dbsession.add(gitlab_mid_org) + r = RepositoryFactory.create(owner=gitlab_mid_org) + dbsession.add(r) + gl_mid_pull = PullFactory.create(repository=r) + dbsession.add(gl_mid_pull) + dbsession.flush() + member_url = get_members_url(gl_mid_pull) + assert member_url == base_for_member_url + "gl/gl_root" + + gitlab_sub_org = OwnerFactory.create( + service="gitlab", + username="gl_child", + parent_service_id=gitlab_mid_org.service_id, + ) + dbsession.add(gitlab_sub_org) + r = RepositoryFactory.create(owner=gitlab_sub_org) + dbsession.add(r) + gl_child_pull = PullFactory.create(repository=r) + dbsession.add(gl_child_pull) + dbsession.flush() + plan_url = get_plan_url(gl_child_pull) + assert plan_url == base_for_plan_url + "gl/gl_root" diff --git a/services/urls.py b/services/urls.py index 0122d209b..ad7a625d2 100644 --- a/services/urls.py +++ b/services/urls.py @@ -6,6 +6,7 @@ from urllib.parse import parse_qs, quote_plus, urlencode, urlparse, urlunparse from shared.config import get_config +from shared.django_apps.codecov_auth.models import Service from database.models import Commit, Pull, Repository from services.license import requires_license @@ -57,6 +58,16 @@ def get_dashboard_base_url() -> str: return configured_dashboard_url or "https://app.codecov.io" +def _get_username_for_url(repository: Repository) -> str: + username = repository.owner.username + if repository.owner.service == Service.GITLAB.value: + # if GL, direct url to root org not subgroup + root_org = repository.owner.root_organization + if root_org is not None: + username = root_org.username + return username + + def get_commit_url(commit: Commit) -> str: return SiteUrls.commit_url.get_url( base_url=get_dashboard_base_url(), @@ -162,11 +173,12 @@ def get_org_account_url(pull: Pull) -> str: def get_members_url(pull: Pull) -> str: repository = pull.repository + username = _get_username_for_url(repository=repository) if not requires_license(): return SiteUrls.members_url.get_url( dashboard_base_url=get_dashboard_base_url(), service_short=services_short_dict.get(repository.service), - username=repository.owner.username, + username=username, ) else: return SiteUrls.members_url_self_hosted.get_url( @@ -178,10 +190,11 @@ def get_members_url(pull: Pull) -> str: def get_plan_url(pull: Pull) -> str: repository = pull.repository + username = _get_username_for_url(repository=repository) return SiteUrls.plan_url.get_url( dashboard_base_url=get_dashboard_base_url(), service_short=services_short_dict.get(repository.service), - username=repository.owner.username, + username=username, ) diff --git a/tasks/new_user_activated.py b/tasks/new_user_activated.py index e294d7ae8..03b0f6f63 100644 --- a/tasks/new_user_activated.py +++ b/tasks/new_user_activated.py @@ -2,9 +2,8 @@ from datetime import datetime, timedelta from typing import Iterator -from shared.billing import is_pr_billing_plan from shared.celery_config import new_user_activated_task_name, notify_task_name -from sqlalchemy import func +from shared.plan.service import PlanService from app import celery_app from database.enums import Decoration @@ -85,20 +84,10 @@ def is_org_on_pr_plan(self, db_session, ownerid: int) -> bool: log.info("Org not found", extra=dict(org_ownerid=ownerid)) return False - if owner.service == "gitlab" and owner.parent_service_id: - # need to get root group so we can check plan info - (gl_root_group,) = db_session.query( - func.public.get_gitlab_root_group(ownerid) - ).first() + # do not access plan directly - only through PlanService + plan = PlanService(current_org=owner) - root_group = ( - db_session.query(Owner) - .filter(Owner.ownerid == gl_root_group.get("ownerid")) - .first() - ) - return is_pr_billing_plan(root_group.plan) - - return is_pr_billing_plan(owner.plan) + return plan.is_pr_billing_plan @metrics.timer("worker.task.new_user_activated.get_pulls_authored_by_user") def get_pulls_authored_by_user( diff --git a/tasks/tests/unit/test_ta_finisher_task.py b/tasks/tests/unit/test_ta_finisher_task.py index 22c0243ef..7aa251e5e 100644 --- a/tasks/tests/unit/test_ta_finisher_task.py +++ b/tasks/tests/unit/test_ta_finisher_task.py @@ -15,6 +15,7 @@ RepositoryFactory, UploadFactory, ) +from services.seats import SeatActivationInfo from services.urls import services_short_dict from tasks.ta_finisher import TAFinisherTask from tasks.ta_processor import TAProcessorTask @@ -154,6 +155,10 @@ def test_test_analytics(dbsession, mocker, mock_storage, celery_app, snapshot): "tasks.ta_finisher.fetch_and_update_pull_request_information_from_commit", return_value=mock_pull_request_information, ) + mocker.patch( + "tasks.ta_finisher.determine_seat_activation", + return_value=SeatActivationInfo(reason="public_repo"), + ) result = TAProcessorTask().run_impl( dbsession,