Merge branch 'main' into export-plans-and-tiers-to-csv

Author: RulaKhaled

.github/workflows/ci.yml

@@ -36,22 +36,13 @@ jobs:
     uses: codecov/gha-workflows/.github/workflows/[email protected]
     secrets: inherit
 
-  # ats:
-  #   name: ATS
-  #   needs: [build]
-  #   if: ${{ !github.event.pull_request.head.repo.fork && github.repository_owner == 'codecov' }}
-  #   uses: codecov/gha-workflows/.github/workflows/[email protected]
-  #   secrets: inherit
-  #   with:
-  #     repo: ${{ vars.CODECOV_IMAGE_V2 || 'codecov/self-hosted-api' }}
-  #     codecov_cli_upload_args: '--plugin pycoverage --plugin compress-pycoverage --flag smart-labels'
-  #     app_container_name: api
   test:
     name: Test
     needs: [build]
-    uses: codecov/gha-workflows/.github/workflows/[email protected].27
+    uses: codecov/gha-workflows/.github/workflows/run-tests-split[email protected].30
     secrets: inherit
     with:
+      run_integration: false
       repo: ${{ vars.CODECOV_IMAGE_V2 || 'codecov/self-hosted-api' }}
 
   build-self-hosted:

.test_durations

@@ -34,10 +34,18 @@ test:
 	COVERAGE_CORE=sysmon python -m pytest --cov=./ --junitxml=junit.xml -o junit_family=legacy
 
 test.unit:
-	COVERAGE_CORE=sysmon python -m pytest --cov=./ -m "not integration" --cov-report=xml:unit.coverage.xml --junitxml=unit.junit.xml -o junit_family=legacy
+	@if [ -n "$(GROUP)" ]; then \
+		COVERAGE_CORE=sysmon python -m pytest --splits ${SPLIT} --group $(GROUP) --cov=./ -m "not integration" --cov-report=xml:unit.$(GROUP).coverage.xml --junitxml=unit.$(GROUP).junit.xml -o junit_family=legacy; \
+	else \
+		COVERAGE_CORE=sysmon python -m pytest --cov=./ -m "not integration" --cov-report=xml:unit.coverage.xml --junitxml=unit.junit.xml -o junit_family=legacy; \
+	fi
 
 test.integration:
-	COVERAGE_CORE=sysmon python -m pytest --cov=./ -m "integration" --cov-report=xml:integration.coverage.xml --junitxml=integration.junit.xml -o junit_family=legacy
+	@if [ -n "$(GROUP)" ]; then \
+		COVERAGE_CORE=sysmon python -m pytest --splits ${SPLIT} --group $(GROUP) --cov=./ -m "integration" --cov-report=xml:integration.$(GROUP).coverage.xml --junitxml=integration.$(GROUP).junit.xml -o junit_family=legacy; \
+	else \
+		COVERAGE_CORE=sysmon python -m pytest --cov=./ -m "integration" --cov-report=xml:integration.coverage.xml --junitxml=integration.junit.xml -o junit_family=legacy; \
+	fi
 
 lint:
 	make lint.install
@@ -197,55 +205,30 @@ test_env.install_cli:
 
 test_env.container_prepare:
 	apt-get -y install git build-essential netcat-traditional
-	make test_env.install_cli
-	git config --global --add safe.directory /app
+	git config --global --add safe.directory /app || true
 
 test_env.container_check_db:
 	while ! nc -vz postgres 5432; do sleep 1; echo "waiting for postgres"; done
 	while ! nc -vz timescale 5432; do sleep 1; echo "waiting for timescale"; done
 
 test_env.run_unit:
-	docker-compose exec api make test.unit
+	@if [ -n "$(GROUP)" ]; then \
+		docker-compose exec api make test.unit SPLIT=${SPLIT} GROUP=${GROUP}; \
+	else \
+		docker-compose exec api make test.unit; \
+	fi
 
 test_env.run_integration:
-	#docker-compose exec api make test.integration
+	# @if [ -n "$(GROUP)" ]; then \
+	# 	docker-compose exec api make test.integration SPLIT=${SPLIT} GROUP=${GROUP}; \
+	# else \
+	# 	docker-compose exec api make test.integration; \
+	# fi
 	echo "Skipping. No Tests"
 
 test_env.check-for-migration-conflicts:
 	docker-compose exec api python manage.py check_for_migration_conflicts
 
-test_env.upload:
-	docker-compose exec api make test_env.container_upload CODECOV_UPLOAD_TOKEN=${CODECOV_UPLOAD_TOKEN} CODECOV_URL=${CODECOV_URL}
-	docker-compose exec api make test_env.container_upload_test_results CODECOV_UPLOAD_TOKEN=${CODECOV_UPLOAD_TOKEN} CODECOV_URL=${CODECOV_URL}
-
-test_env.container_upload:
-	codecovcli -u ${CODECOV_URL} upload-process --flag unit-latest-uploader --flag unit  \
-	--coverage-files-search-exclude-folder=graphql_api/types/** \
-	--coverage-files-search-exclude-folder=api/internal/tests/unit/views/cassetes/**
-
-test_env.container_upload_test_results:
-	codecovcli -u ${CODECOV_URL} do-upload --report-type "test_results" \
-	--files-search-exclude-folder=graphql_api/types/** \
-	--files-search-exclude-folder=api/internal/tests/unit/views/cassetes/** || true
-
-test_env.static_analysis:
-	docker-compose exec api make test_env.container_static_analysis CODECOV_STATIC_TOKEN=${CODECOV_STATIC_TOKEN}
-
-test_env.label_analysis:
-	docker-compose exec api make test_env.container_label_analysis CODECOV_STATIC_TOKEN=${CODECOV_STATIC_TOKEN}
-
-test_env.ats:
-	docker-compose exec api make test_env.container_ats CODECOV_UPLOAD_TOKEN=${CODECOV_UPLOAD_TOKEN}
-
-test_env.container_static_analysis:
-	codecovcli -u ${CODECOV_URL} static-analysis --token=${CODECOV_STATIC_TOKEN}
-
-test_env.container_label_analysis:
-	codecovcli -u ${CODECOV_URL} label-analysis --base-sha=${merge_sha} --token=${CODECOV_STATIC_TOKEN}
-
-test_env.container_ats:
-	codecovcli -u ${CODECOV_URL} --codecov-yml-path=codecov_cli.yml upload-process --plugin pycoverage --plugin compress-pycoverage --flag smart-labels --fail-on-error
-
 test_env:
 	make test_env.up
 	make test_env.prepare

Makefile

@@ -1 +1 @@
-25.2.3
+25.2.7

VERSION

@@ -3,7 +3,6 @@
 from typing import Any, Dict
 
 from dateutil.relativedelta import relativedelta
-from django.conf import settings
 from rest_framework import serializers
 from rest_framework.exceptions import PermissionDenied
 from shared.plan.constants import (
@@ -132,7 +131,7 @@ def validate_value(self, value: str) -> str:
 
         plan_service = PlanService(current_org=current_org)
         plan_values = [
-            plan["value"] for plan in plan_service.available_plans(current_owner)
+            plan.name for plan in plan_service.available_plans(current_owner)
         ]
         if value not in plan_values:
             raise serializers.ValidationError(
@@ -217,11 +216,7 @@ class StripeScheduledPhaseSerializer(serializers.Serializer):
 
     def get_plan(self, phase: Dict[str, Any]) -> str:
         plan_id = phase["items"][0]["plan"]
-        stripe_plan_dict = settings.STRIPE_PLAN_IDS
-        plan_name = list(stripe_plan_dict.keys())[
-            list(stripe_plan_dict.values()).index(plan_id)
-        ]
-        marketing_plan_name = Plan.objects.get(name=plan_name).marketing_name
+        marketing_plan_name = Plan.objects.get(stripe_id=plan_id).marketing_name
         return marketing_plan_name
 
     def get_quantity(self, phase: Dict[str, Any]) -> int:
@@ -344,11 +339,13 @@ def update(self, instance: Owner, validated_data: Dict[str, Any]) -> object:
                 instance, desired_plan
             )
 
-            sentry_plans = Plan.objects.filter(
-                tier__tier_name=TierName.SENTRY.value, is_active=True
-            ).values_list("name", flat=True)
+            plan = (
+                Plan.objects.select_related("tier")
+                .filter(name=desired_plan["value"])
+                .first()
+            )
 
-            if desired_plan["value"] in sentry_plans:
+            if plan and plan.tier.tier_name == TierName.SENTRY.value:
                 current_owner = self.context["view"].request.current_owner
                 send_sentry_webhook(current_owner, instance)
 

api/internal/owner/serializers.py

@@ -7,6 +7,7 @@
 from rest_framework.exceptions import PermissionDenied, ValidationError
 from rest_framework.response import Response
 from shared.django_apps.codecov_auth.models import Owner
+from shared.plan.constants import DEFAULT_FREE_PLAN
 
 from api.shared.mixins import OwnerPropertyMixin
 from api.shared.owner.mixins import OwnerViewSetMixin, UserViewSetMixin
@@ -45,8 +46,13 @@ def retrieve(self, *args, **kwargs):
         return res
 
     @stripe_safe
-    def update(self, *args, **kwargs):
-        return super().update(*args, **kwargs)
+    def update(self, request, *args, **kwargs):
+        # Temporary fix. Remove once Gazebo uses the new free plan
+        plan_value = request.data.get("plan", {}).get("value")
+        if plan_value == "users-basic":
+            request.data["plan"]["value"] = DEFAULT_FREE_PLAN
+
+        return super().update(request, *args, **kwargs)
 
     def destroy(self, request, *args, **kwargs):
         if self.owner.ownerid != request.current_owner.ownerid:

api/internal/owner/views.py

@@ -14,7 +14,7 @@
     OwnerFactory,
     UserFactory,
 )
-from shared.plan.constants import PlanName, TrialStatus
+from shared.plan.constants import DEFAULT_FREE_PLAN, PlanName, TrialStatus
 from stripe import StripeError
 
 from api.internal.tests.test_utils import GetAdminProviderAdapter
@@ -185,7 +185,7 @@ def test_retrieve_account_gets_account_fields(self):
             "inactive_user_count": 1,
             "plan": {
                 "marketing_name": "Developer",
-                "value": PlanName.BASIC_PLAN_NAME.value,
+                "value": DEFAULT_FREE_PLAN,
                 "billing_rate": None,
                 "base_unit_price": 0,
                 "benefits": [
@@ -234,7 +234,7 @@ def test_retrieve_account_gets_account_fields_when_there_are_scheduled_details(
         schedule_params = {
             "id": 123,
             "start_date": 123689126736,
-            "stripe_plan_id": "plan_H6P3KZXwmAbqPS",
+            "stripe_plan_id": "plan_pro_yearly",
             "quantity": 6,
         }
         phases = [
@@ -270,7 +270,7 @@ def test_retrieve_account_gets_account_fields_when_there_are_scheduled_details(
             "plan_provider": owner.plan_provider,
             "plan": {
                 "marketing_name": "Developer",
-                "value": PlanName.BASIC_PLAN_NAME.value,
+                "value": DEFAULT_FREE_PLAN,
                 "billing_rate": None,
                 "base_unit_price": 0,
                 "benefits": [
@@ -330,7 +330,7 @@ def test_retrieve_account_returns_last_phase_when_more_than_one_scheduled_phases
         schedule_params = {
             "id": 123,
             "start_date": 123689126736,
-            "stripe_plan_id": "plan_H6P3KZXwmAbqPS",
+            "stripe_plan_id": "plan_pro_yearly",
             "quantity": 6,
         }
         phases = [
@@ -367,7 +367,7 @@ def test_retrieve_account_returns_last_phase_when_more_than_one_scheduled_phases
             "inactive_user_count": 1,
             "plan": {
                 "marketing_name": "Developer",
-                "value": PlanName.BASIC_PLAN_NAME.value,
+                "value": DEFAULT_FREE_PLAN,
                 "billing_rate": None,
                 "base_unit_price": 0,
                 "benefits": [
@@ -441,7 +441,7 @@ def test_retrieve_account_gets_none_for_schedule_details_when_schedule_is_nonexi
             "inactive_user_count": 1,
             "plan": {
                 "marketing_name": "Developer",
-                "value": PlanName.BASIC_PLAN_NAME.value,
+                "value": DEFAULT_FREE_PLAN,
                 "billing_rate": None,
                 "base_unit_price": 0,
                 "benefits": [
@@ -509,13 +509,13 @@ def test_retrieve_account_gets_account_students(self):
         }
 
     def test_account_with_free_user_plan(self):
-        self.current_owner.plan = PlanName.BASIC_PLAN_NAME.value
+        self.current_owner.plan = DEFAULT_FREE_PLAN
         self.current_owner.save()
         response = self._retrieve()
         assert response.status_code == status.HTTP_200_OK
         assert response.data["plan"] == {
             "marketing_name": "Developer",
-            "value": PlanName.BASIC_PLAN_NAME.value,
+            "value": DEFAULT_FREE_PLAN,
             "billing_rate": None,
             "base_unit_price": 0,
             "benefits": [
@@ -711,7 +711,7 @@ def test_update_can_set_plan_auto_activate_on_org_with_account(self):
         assert self.current_owner.plan_auto_activate is False
         assert response.data["plan_auto_activate"] is False
 
-    def test_update_can_set_plan_to_users_basic(self):
+    def test_update_can_set_plan_to_users_developer_should_set_to_developer(self):
         self.current_owner.plan = PlanName.CODECOV_PRO_YEARLY.value
         self.current_owner.save()
 
@@ -720,14 +720,14 @@ def test_update_can_set_plan_to_users_basic(self):
                 "service": self.current_owner.service,
                 "owner_username": self.current_owner.username,
             },
-            data={"plan": {"value": PlanName.BASIC_PLAN_NAME.value}},
+            data={"plan": {"value": DEFAULT_FREE_PLAN}},
         )
 
         assert response.status_code == status.HTTP_200_OK
 
         self.current_owner.refresh_from_db()
 
-        assert self.current_owner.plan == PlanName.BASIC_PLAN_NAME.value
+        assert self.current_owner.plan == DEFAULT_FREE_PLAN
         assert self.current_owner.plan_activated_users is None
         assert self.current_owner.plan_user_count == 1
         assert response.data["plan_auto_activate"] is True
@@ -984,7 +984,7 @@ def test_update_must_fail_if_quantity_and_plan_are_equal_to_the_owners_current_o
         )
 
     def test_update_team_plan_must_fail_if_too_many_activated_users_during_trial(self):
-        self.current_owner.plan = PlanName.BASIC_PLAN_NAME.value
+        self.current_owner.plan = DEFAULT_FREE_PLAN
         self.current_owner.plan_user_count = 1
         self.current_owner.trial_status = TrialStatus.ONGOING.value
         self.current_owner.plan_activated_users = list(range(11))
@@ -1005,13 +1005,7 @@ def test_update_team_plan_must_fail_if_too_many_activated_users_during_trial(sel
             )
 
             assert response.status_code == status.HTTP_400_BAD_REQUEST
-            assert response.json() == {
-                "plan": {
-                    "value": [
-                        f"Invalid value for plan: {desired_plan['value']}; must be one of ['users-basic', 'users-pr-inappm', 'users-pr-inappy']"
-                    ]
-                }
-            }
+            assert "Invalid value for plan:" in response.json()["plan"]["value"][0]
 
     def test_update_team_plan_must_fail_if_currently_team_plan_add_too_many_users(self):
         self.current_owner.plan = PlanName.TEAM_MONTHLY.value
@@ -1602,7 +1596,7 @@ def test_update_sentry_plan_non_sentry_user(
         assert res.json() == {
             "plan": {
                 "value": [
-                    "Invalid value for plan: users-sentrym; must be one of ['users-basic', 'users-pr-inappm', 'users-pr-inappy', 'users-teamm', 'users-teamy']"
+                    f"Invalid value for plan: users-sentrym; must be one of ['users-pr-inappm', 'users-pr-inappy', 'users-teamm', 'users-teamy', '{DEFAULT_FREE_PLAN}']"
                 ]
             }
         }

api/internal/tests/views/test_account_viewset.py

@@ -10,7 +10,7 @@
     PullFactory,
     RepositoryFactory,
 )
-from shared.plan.constants import PlanName, TierName
+from shared.plan.constants import DEFAULT_FREE_PLAN, TierName
 
 from core.models import Pull
 from utils.test_utils import APIClient
@@ -20,7 +20,7 @@ class UserViewSetTests(APITestCase):
     def setUp(self):
         non_org_active_user = OwnerFactory()
         tier = TierFactory(tier_name=TierName.BASIC.value)
-        plan = PlanFactory(name=PlanName.BASIC_PLAN_NAME.value, tier=tier)
+        plan = PlanFactory(name=DEFAULT_FREE_PLAN, tier=tier)
         self.current_owner = OwnerFactory(
             plan=plan.name,
             plan_user_count=5,

api/internal/tests/views/test_user_viewset.py

@@ -16,3 +16,7 @@ class Meta:
             "state",
         )
         fields = read_only_fields + ("user_provided_base_sha",)
+
+
+class PullIdSerializer(serializers.Serializer):
+    pullid = serializers.IntegerField()

api/public/v1/serializers.py

@@ -133,3 +133,10 @@ def test_post_pull_user_provided_base(self, pulls_sync_mock):
         )
         self.assertEqual(response.status_code, 405)
         assert not pulls_sync_mock.called
+
+    def test_get_pull_no_pullid_provided(self):
+        self.client.credentials(HTTP_AUTHORIZATION="Token " + self.repo.upload_token)
+        response = self.client.get("/api/github/codecov/testRepoName/pulls/abc")
+        self.assertEqual(response.status_code, 400)
+        content = json.loads(response.content.decode())
+        self.assertEqual(content["pullid"], ["A valid integer is required."])