Add support for saving comments in global YAML file (#895)

Author: JerrySentry

codecov_auth/commands/owner/interactors/set_yaml_on_owner.py

@@ -1,4 +1,5 @@
 import html
+from typing import Optional
 
 import yaml
 from shared.validation.exceptions import InvalidYamlException
@@ -12,26 +13,27 @@
     ValidationError,
 )
 from codecov.db import sync_to_async
+from codecov_auth.constants import OWNER_YAML_TO_STRING_KEY
 from codecov_auth.helpers import current_user_part_of_org
 from codecov_auth.models import Owner
 
 
 class SetYamlOnOwnerInteractor(BaseInteractor):
-    def validate(self):
+    def validate(self) -> None:
         if not self.current_user.is_authenticated:
             raise Unauthenticated()
 
-    def authorize(self):
+    def authorize(self) -> None:
         if not current_user_part_of_org(self.current_owner, self.owner):
             raise Unauthorized()
 
-    def get_owner(self, username):
+    def get_owner(self, username: str) -> Owner:
         try:
             return Owner.objects.get(username=username, service=self.service)
         except Owner.DoesNotExist:
             raise NotFound()
 
-    def convert_yaml_to_dict(self, yaml_input):
+    def convert_yaml_to_dict(self, yaml_input: str) -> Optional[dict]:
         yaml_safe = html.escape(yaml_input, quote=False)
         try:
             yaml_dict = yaml.safe_load(yaml_safe)
@@ -49,10 +51,12 @@ def convert_yaml_to_dict(self, yaml_input):
             raise ValidationError(message)
 
     @sync_to_async
-    def execute(self, username, yaml_input):
+    def execute(self, username: str, yaml_input: str) -> Owner:
         self.validate()
         self.owner = self.get_owner(username)
         self.authorize()
         self.owner.yaml = self.convert_yaml_to_dict(yaml_input)
+        if self.owner.yaml:
+            self.owner.yaml[OWNER_YAML_TO_STRING_KEY] = yaml_input
         self.owner.save()
         return self.owner

codecov_auth/commands/owner/interactors/tests/test_set_yaml_on_owner.py

@@ -35,6 +35,16 @@
     bot: foo: bar
 """
 
+good_yaml_with_comments = """
+# comment 1
+codecov:  # comment 2
+
+
+  bot: 'codecov'
+# comment 3
+    #comment 4
+"""
+
 
 class SetYamlOnOwnerInteractorTest(TransactionTestCase):
     def setUp(self):
@@ -69,15 +79,25 @@ async def test_user_is_part_of_org_and_yaml_is_good(self):
         )
         # check the interactor returns the right owner
         assert owner_updated.ownerid == self.org.ownerid
-        assert owner_updated.yaml == {"codecov": {"require_ci_to_pass": True}}
+        assert owner_updated.yaml == {
+            "codecov": {
+                "require_ci_to_pass": True,
+            },
+            "to_string": "\n" "codecov:\n" "  require_ci_to_pass: yes\n",
+        }
 
     async def test_user_is_part_of_org_and_yaml_has_quotes(self):
         owner_updated = await self.execute(
             self.current_owner, self.org.username, good_yaml_with_quotes
         )
         # check the interactor returns the right owner
         assert owner_updated.ownerid == self.org.ownerid
-        assert owner_updated.yaml == {"codecov": {"bot": "codecov"}}
+        assert owner_updated.yaml == {
+            "codecov": {
+                "bot": "codecov",
+            },
+            "to_string": "\n" "codecov:\n" "  bot: 'codecov'\n",
+        }
 
     async def test_user_is_part_of_org_and_yaml_is_empty(self):
         owner_updated = await self.execute(self.current_owner, self.org.username, "")
@@ -103,3 +123,23 @@ async def test_yaml_syntax_error(self):
             str(e.value)
             == "Syntax error at line 3, column 13: mapping values are not allowed here"
         )
+
+    async def test_yaml_has_comments(self):
+        owner_updated = await self.execute(
+            self.current_owner, self.org.username, good_yaml_with_comments
+        )
+        # check the interactor returns the right owner
+        assert owner_updated.ownerid == self.org.ownerid
+        assert owner_updated.yaml == {
+            "codecov": {
+                "bot": "codecov",
+            },
+            "to_string": "\n"
+            "# comment 1\n"
+            "codecov:  # comment 2\n"
+            "\n"
+            "\n"
+            "  bot: 'codecov'\n"
+            "# comment 3\n"
+            "    #comment 4\n",
+        }

codecov_auth/constants.py

@@ -3,3 +3,4 @@
 GITLAB_BASE_URL = "https://gitlab.com"
 GRAVATAR_BASE_URL = "https://www.gravatar.com"
 AVATARIO_BASE_URL = "https://avatars.io"
+OWNER_YAML_TO_STRING_KEY = "to_string"

graphql_api/types/owner/owner.py

@@ -1,6 +1,6 @@
 from datetime import datetime
 from hashlib import sha1
-from typing import Iterable, List, Optional
+from typing import Any, Iterable, List, Optional
 
 import shared.rate_limits as rate_limits
 import stripe
@@ -11,6 +11,7 @@
 import services.activation as activation
 import timeseries.helpers as timeseries_helpers
 from codecov.db import sync_to_async
+from codecov_auth.constants import OWNER_YAML_TO_STRING_KEY
 from codecov_auth.helpers import current_user_part_of_org
 from codecov_auth.models import (
     SERVICE_GITHUB,
@@ -26,6 +27,7 @@
 )
 from graphql_api.helpers.ariadne import ariadne_load_local_graphql
 from graphql_api.helpers.connection import (
+    Connection,
     build_connection_graphql,
     queryset_to_connection,
 )
@@ -38,7 +40,7 @@
 from services.profiling import ProfilingSummary
 from services.redis_configuration import get_redis_connection
 from timeseries.helpers import fill_sparse_measurements
-from timeseries.models import Interval, MeasurementSummary
+from timeseries.models import Interval
 from utils.config import get_config
 
 owner = ariadne_load_local_graphql(__file__, "owner.graphql")
@@ -51,12 +53,12 @@
 @convert_kwargs_to_snake_case
 def resolve_repositories(
     owner: Owner,
-    info,
-    filters=None,
-    ordering=RepositoryOrdering.ID,
-    ordering_direction=OrderingDirection.ASC,
-    **kwargs,
-):
+    info: GraphQLResolveInfo,
+    filters: Optional[dict] = None,
+    ordering: Optional[RepositoryOrdering] = RepositoryOrdering.ID,
+    ordering_direction: Optional[OrderingDirection] = OrderingDirection.ASC,
+    **kwargs: Any,
+) -> Connection:
     current_owner = info.context["request"].current_owner
     okta_account_auths: list[int] = info.context["request"].session.get(
         OKTA_SIGNED_IN_ACCOUNTS_SESSION_KEY, []
@@ -81,19 +83,19 @@ def resolve_repositories(
 
 @owner_bindable.field("isCurrentUserPartOfOrg")
 @sync_to_async
-def resolve_is_current_user_part_of_org(owner, info: GraphQLResolveInfo):
+def resolve_is_current_user_part_of_org(owner: Owner, info: GraphQLResolveInfo) -> bool:
     current_owner = info.context["request"].current_owner
     return current_user_part_of_org(current_owner, owner)
 
 
 @owner_bindable.field("yaml")
-def resolve_yaml(owner: Owner, info: GraphQLResolveInfo):
+def resolve_yaml(owner: Owner, info: GraphQLResolveInfo) -> Optional[str]:
     if owner.yaml is None:
-        return
+        return None
     current_owner = info.context["request"].current_owner
     if not current_user_part_of_org(current_owner, owner):
-        return
-    return yaml.dump(owner.yaml)
+        return None
+    return owner.yaml.get(OWNER_YAML_TO_STRING_KEY, yaml.dump(owner.yaml))
 
 
 @owner_bindable.field("plan")
@@ -134,7 +136,9 @@ def resolve_ownerid(owner: Owner, info: GraphQLResolveInfo) -> int:
 
 
 @owner_bindable.field("repository")
-async def resolve_repository(owner: Owner, info, name):
+async def resolve_repository(
+    owner: Owner, info: GraphQLResolveInfo, name: str
+) -> Repository | NotFoundError:
     command = info.context["executor"].get_command("repository")
     okta_authenticated_accounts: list[int] = info.context["request"].session.get(
         OKTA_SIGNED_IN_ACCOUNTS_SESSION_KEY, []
@@ -174,37 +178,43 @@ async def resolve_repository(owner: Owner, info, name):
 
 @owner_bindable.field("numberOfUploads")
 @require_part_of_org
-async def resolve_number_of_uploads(owner: Owner, info, **kwargs):
+async def resolve_number_of_uploads(
+    owner: Owner, info: GraphQLResolveInfo, **kwargs: Any
+) -> int:
     command = info.context["executor"].get_command("owner")
     return await command.get_uploads_number_per_user(owner)
 
 
 @owner_bindable.field("isAdmin")
 @require_part_of_org
-def resolve_is_current_user_an_admin(owner: Owner, info: GraphQLResolveInfo):
+def resolve_is_current_user_an_admin(owner: Owner, info: GraphQLResolveInfo) -> bool:
     current_owner = info.context["request"].current_owner
     command = info.context["executor"].get_command("owner")
     return command.get_is_current_user_an_admin(owner, current_owner)
 
 
 @owner_bindable.field("hashOwnerid")
 @require_part_of_org
-def resolve_hash_ownerid(owner: Owner, info: GraphQLResolveInfo):
+def resolve_hash_ownerid(owner: Owner, info: GraphQLResolveInfo) -> str:
     hash_ownerid = sha1(str(owner.ownerid).encode())
     return hash_ownerid.hexdigest()
 
 
 @owner_bindable.field("orgUploadToken")
 @require_part_of_org
-def resolve_org_upload_token(owner: Owner, info, **kwargs):
+def resolve_org_upload_token(
+    owner: Owner, info: GraphQLResolveInfo, **kwargs: Any
+) -> str:
     command = info.context["executor"].get_command("owner")
     return command.get_org_upload_token(owner)
 
 
 @owner_bindable.field("defaultOrgUsername")
 @sync_to_async
 @require_part_of_org
-def resolve_org_default_org_username(owner: Owner, info, **kwargs) -> int:
+def resolve_org_default_org_username(
+    owner: Owner, info: GraphQLResolveInfo, **kwargs: Any
+) -> Optional[str]:
     return None if owner.default_org is None else owner.default_org.username
 
 
@@ -213,13 +223,13 @@ def resolve_org_default_org_username(owner: Owner, info, **kwargs) -> int:
 @convert_kwargs_to_snake_case
 def resolve_measurements(
     owner: Owner,
-    info,
+    info: GraphQLResolveInfo,
     interval: Interval,
     after: Optional[datetime] = None,
     before: Optional[datetime] = None,
     repos: Optional[List[str]] = None,
     is_public: Optional[bool] = None,
-) -> Iterable[MeasurementSummary]:
+) -> Iterable[dict]:
     current_owner = info.context["request"].current_owner
 
     okta_authenticated_accounts: list[int] = info.context["request"].session.get(
@@ -256,7 +266,7 @@ def resolve_measurements(
 
 @owner_bindable.field("isCurrentUserActivated")
 @sync_to_async
-def resolve_is_current_user_activated(owner: Owner, info: GraphQLResolveInfo):
+def resolve_is_current_user_activated(owner: Owner, info: GraphQLResolveInfo) -> bool:
     current_user = info.context["request"].user
     if not current_user.is_authenticated:
         return False
@@ -303,7 +313,7 @@ def resolve_is_github_rate_limited(
 @convert_kwargs_to_snake_case
 def resolve_owner_invoice(
     owner: Owner,
-    info,
+    info: GraphQLResolveInfo,
     invoice_id: str,
 ) -> stripe.Invoice | None:
     return BillingService(requesting_user=owner).get_invoice(owner, invoice_id)
@@ -371,7 +381,9 @@ def resolve_ai_enabled_repos(
 
 @owner_bindable.field("uploadTokenRequired")
 @require_part_of_org
-def resolve_upload_token_required(owner: Owner, info) -> bool | None:
+def resolve_upload_token_required(
+    owner: Owner, info: GraphQLResolveInfo
+) -> bool | None:
     return owner.upload_token_required_for_public_repos
 
 

services/tests/test_yaml.py

@@ -56,3 +56,15 @@ def test_when_commit_has_yaml_with_owner(self, mock_fetch_yaml):
         """
         config = yaml.final_commit_yaml(self.commit, self.org)
         assert config["codecov"]["require_ci_to_pass"] is False
+
+    @patch("services.yaml.fetch_current_yaml_from_provider_via_reference")
+    def test_when_commit_has_reserved_to_string_key(self, mock_fetch_yaml):
+        mock_fetch_yaml.return_value = """
+        codecov:
+          notify:
+            require_ci_to_pass: no
+        to_string: hello
+        """
+        config = yaml.final_commit_yaml(self.commit, self.org)
+        assert config.get("to_string") is None
+        assert "to_string" not in config.to_dict()