Skip to content

Commit

Permalink
Merge branch 'main' into 170-add-events-to-app
Browse files Browse the repository at this point in the history
  • Loading branch information
adrian-codecov authored Aug 29, 2024
2 parents 9f8fd49 + 1a56eea commit 88b6ffc
Show file tree
Hide file tree
Showing 25 changed files with 348 additions and 307 deletions.
5 changes: 3 additions & 2 deletions requirements.in
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
https://github.com/codecov/opentelem-python/archive/refs/tags/v0.0.4a1.tar.gz#egg=codecovopentelem
https://github.com/codecov/shared/archive/f9de90a4f65332893b0fc6ddda0d3d0c228d13e9.tar.gz#egg=shared
https://github.com/codecov/shared/archive/a18a584a99fc1a6b9ae7fd0e1873e72b6499bf23.tar.gz#egg=shared
https://github.com/codecov/test-results-parser/archive/1507de2241601d678e514c08b38426e48bb6d47d.tar.gz#egg=test-results-parser
https://github.com/codecov/timestring/archive/d37ceacc5954dff3b5bd2f887936a98a668dda42.tar.gz#egg=timestring
asgiref>=3.7.2
Expand Down Expand Up @@ -39,7 +39,8 @@ redis>=4.4.4
regex
requests>=2.32.0
respx
sentry-sdk>=1.40.0
sentry-sdk>=2.13.0
sentry-sdk[celery]
SQLAlchemy
SQLAlchemy-Utils
statsd
Expand Down
5 changes: 3 additions & 2 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ celery==5.3.6
# via
# -r requirements.in
# pytest-celery
# sentry-sdk
cerberus==1.3.4
# via shared
certifi==2024.7.4
Expand Down Expand Up @@ -352,11 +353,11 @@ rsa==4.7.2
# via google-auth
s3transfer==0.10.1
# via boto3
sentry-sdk==1.40.0
sentry-sdk[celery]==2.13.0
# via
# -r requirements.in
# shared
shared @ https://github.com/codecov/shared/archive/f9de90a4f65332893b0fc6ddda0d3d0c228d13e9.tar.gz
shared @ https://github.com/codecov/shared/archive/a18a584a99fc1a6b9ae7fd0e1873e72b6499bf23.tar.gz
# via -r requirements.in
six==1.16.0
# via
Expand Down
22 changes: 22 additions & 0 deletions services/activation.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
import logging

from shared.celery_config import (
activate_account_user_task_name,
new_user_activated_task_name,
)
from sqlalchemy import func
from sqlalchemy.sql import text

from app import celery_app
from services.license import (
calculate_reason_for_not_being_valid,
get_current_license,
Expand Down Expand Up @@ -97,3 +102,20 @@ def activate_user(db_session, org_ownerid: int, user_ownerid: int) -> bool:
),
)
return activation_success


def schedule_new_user_activated_task(self, org_ownerid, user_ownerid):
celery_app.send_task(
new_user_activated_task_name,
args=None,
kwargs=dict(org_ownerid=org_ownerid, user_ownerid=user_ownerid),
)
# Activate the account user if it exists.
celery_app.send_task(
activate_account_user_task_name,
args=None,
kwargs=dict(
user_ownerid=user_ownerid,
org_ownerid=org_ownerid,
),
)
44 changes: 37 additions & 7 deletions services/bundle_analysis/new_notify/contexts/comment.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import logging
from typing import Self

import sentry_sdk
from asgiref.sync import async_to_sync
Expand All @@ -8,6 +9,7 @@
from shared.yaml import UserYaml

from database.models.core import Commit
from services.activation import activate_user, schedule_new_user_activated_task
from services.bundle_analysis.comparison import ComparisonLoader
from services.bundle_analysis.exceptions import (
MissingBaseCommit,
Expand All @@ -29,6 +31,7 @@
EnrichedPull,
fetch_and_update_pull_request_information_from_commit,
)
from services.seats import ShouldActivateSeat, determine_seat_activation

log = logging.getLogger(__name__)

Expand All @@ -42,6 +45,7 @@ class BundleAnalysisPRCommentNotificationContext(BaseBundleAnalysisNotificationC
bundle_analysis_comparison: BundleAnalysisComparison = NotificationContextField[
BundleAnalysisComparison
]()
should_use_upgrade_comment: bool


class BundleAnalysisPRCommentContextBuilder(NotificationContextBuilder):
Expand All @@ -64,7 +68,7 @@ def initialize(
return self

@sentry_sdk.trace
async def load_enriched_pull(self) -> "BundleAnalysisPRCommentContextBuilder":
async def load_enriched_pull(self) -> Self:
"""Loads the EnrichedPull into the NotificationContext.
EnrichedPull includes updated info from the git provider and info saved in the database.
Raises:
Expand All @@ -87,7 +91,7 @@ async def load_enriched_pull(self) -> "BundleAnalysisPRCommentContextBuilder":
return self

@sentry_sdk.trace
def load_bundle_comparison(self) -> "BundleAnalysisPRCommentContextBuilder":
def load_bundle_comparison(self) -> Self:
"""Loads the BundleAnalysisComparison into the NotificationContext.
BundleAnalysisComparison is the diff between 2 BundleAnalysisReports,
respectively the one for the pull's base and one for the pull's head.
Expand All @@ -112,7 +116,7 @@ def load_bundle_comparison(self) -> "BundleAnalysisPRCommentContextBuilder":
"load_bundle_comparison", detail=exp.__class__.__name__
)

def evaluate_has_enough_changes(self) -> "BundleAnalysisPRCommentContextBuilder":
def evaluate_has_enough_changes(self) -> Self:
"""Evaluates if the NotificationContext includes enough changes to send the notification.
Configuration is done via UserYaml.
If a comment was previously made for this PR the required changes are bypassed so that we
Expand Down Expand Up @@ -155,12 +159,38 @@ def evaluate_has_enough_changes(self) -> "BundleAnalysisPRCommentContextBuilder"
raise NotificationContextBuildError("evaluate_has_enough_changes")
return self

def build_context(self) -> "BundleAnalysisPRCommentContextBuilder":
@sentry_sdk.trace
def evaluate_should_use_upgrade_message(self) -> Self:
activate_seat_info = determine_seat_activation(self._notification_context.pull)
match activate_seat_info.should_activate_seat:
case ShouldActivateSeat.AUTO_ACTIVATE:
successful_activation = activate_user(
db_session=self._notification_context.commit.get_db_session(),
org_ownerid=activate_seat_info.owner_id,
user_ownerid=activate_seat_info.author_id,
)
if successful_activation:
schedule_new_user_activated_task(
activate_seat_info.owner_id,
activate_seat_info.author_id,
)
self._notification_context.should_use_upgrade_comment = False
else:
self._notification_context.should_use_upgrade_comment = True
case ShouldActivateSeat.MANUAL_ACTIVATE:
self._notification_context.should_use_upgrade_comment = True
case ShouldActivateSeat.NO_ACTIVATE:
self._notification_context.should_use_upgrade_comment = False
return self

def build_context(self) -> Self:
super().build_context()
async_to_sync(self.load_enriched_pull)()
self.load_bundle_comparison()
self.evaluate_has_enough_changes()
return self
return (
self.load_bundle_comparison()
.evaluate_has_enough_changes()
.evaluate_should_use_upgrade_message()
)

def get_result(self) -> BundleAnalysisPRCommentNotificationContext:
return self._notification_context
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
from services.bundle_analysis.new_notify.contexts.comment import (
BundleAnalysisPRCommentContextBuilder,
)
from services.seats import SeatActivationInfo, ShouldActivateSeat


class TestBundleAnalysisPRCommentNotificationContext:
Expand Down Expand Up @@ -222,6 +223,49 @@ def test_evaluate_changes_comment_exists(self, dbsession):
result = builder.evaluate_has_enough_changes()
assert result == builder

@pytest.mark.parametrize(
"activation_result, auto_activate_succeeds, expected",
[
(ShouldActivateSeat.AUTO_ACTIVATE, True, False),
(ShouldActivateSeat.AUTO_ACTIVATE, False, True),
(ShouldActivateSeat.MANUAL_ACTIVATE, False, True),
(ShouldActivateSeat.NO_ACTIVATE, False, False),
],
)
def test_evaluate_should_use_upgrade_message(
self, activation_result, dbsession, auto_activate_succeeds, expected, mocker
):
activation_result = SeatActivationInfo(
should_activate_seat=activation_result,
owner_id=1,
author_id=10,
reason="mocked",
)
mocker.patch(
"services.bundle_analysis.new_notify.contexts.comment.determine_seat_activation",
return_value=activation_result,
)
mocker.patch(
"services.bundle_analysis.new_notify.contexts.comment.activate_user",
return_value=auto_activate_succeeds,
)
mocker.patch(
"services.bundle_analysis.new_notify.contexts.comment.schedule_new_user_activated_task",
return_value=auto_activate_succeeds,
)
head_commit, _ = get_commit_pair(dbsession)
user_yaml = UserYaml.from_dict({})
builder = BundleAnalysisPRCommentContextBuilder().initialize(
head_commit, user_yaml, GITHUB_APP_INSTALLATION_DEFAULT_NAME
)
mock_pull = MagicMock(
name="fake_pull",
database_pull=MagicMock(bundle_analysis_commentid=12345, id=12),
)
builder._notification_context.pull = mock_pull
builder.evaluate_should_use_upgrade_message()
assert builder._notification_context.should_use_upgrade_comment == expected

def test_build_context(self, dbsession, mocker, mock_storage):
head_commit, base_commit = get_commit_pair(dbsession)
repository = head_commit.repository
Expand Down
14 changes: 6 additions & 8 deletions services/bundle_analysis/new_notify/helpers.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import numbers
from typing import Literal
from typing import Iterable, Literal

from shared.bundle_analysis import (
BundleAnalysisComparison,
Expand Down Expand Up @@ -80,15 +80,13 @@ def bytes_readable(bytes: int) -> str:


def to_BundleThreshold(value: int | float | BundleThreshold) -> BundleThreshold:
# Currently the yaml validator returns the raw values, not the BundleThreshold object
# Because the changes are not forwards compatible.
# https://github.com/codecov/engineering-team/issues/2087 is to fix that
# and then this function can be removed too
if isinstance(value, BundleThreshold):
return value
if isinstance(value, Iterable) and value[0] in ["absolute", "percentage"]:
return BundleThreshold(*value)
if isinstance(value, numbers.Integral):
return BundleThreshold("absolute", value)
return BundleThreshold("percentage", value)
elif isinstance(value, numbers.Number):
return BundleThreshold("percentage", value)
raise TypeError(f"Can't parse {value} into BundleThreshold")


def is_bundle_change_within_bundle_threshold(
Expand Down
33 changes: 31 additions & 2 deletions services/bundle_analysis/new_notify/messages/comment.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@
get_github_app_used,
)
from services.bundle_analysis.new_notify.messages import MessageStrategyInterface
from services.license import requires_license
from services.notification.notifiers.base import NotificationResult
from services.urls import get_bundle_analysis_pull_url
from services.urls import get_bundle_analysis_pull_url, get_members_url

log = logging.getLogger(__name__)

Expand All @@ -37,9 +38,25 @@ class BundleCommentTemplateContext(TypedDict):
bundle_rows: list[BundleRow]


class UpgradeCommentTemplateContext(TypedDict):
author_username: str
is_saas: bool
activation_link: str


class BundleAnalysisCommentMarkdownStrategy(MessageStrategyInterface):
def build_message(
self, context: BundleAnalysisPRCommentNotificationContext
) -> str | bytes:
if context.should_use_upgrade_comment:
return self.build_upgrade_message(context)
else:
return self.build_default_message(context)

@sentry_sdk.trace
def build_message(self, context: BundleAnalysisPRCommentNotificationContext) -> str:
def build_default_message(
self, context: BundleAnalysisPRCommentNotificationContext
) -> str:
template = loader.get_template("bundle_analysis_notify/bundle_comment.md")
total_size_delta = context.bundle_analysis_comparison.total_size_delta
context = BundleCommentTemplateContext(
Expand All @@ -50,6 +67,18 @@ def build_message(self, context: BundleAnalysisPRCommentNotificationContext) ->
)
return template.render(context)

@sentry_sdk.trace
def build_upgrade_message(
self, context: BundleAnalysisPRCommentNotificationContext
) -> str:
template = loader.get_template("bundle_analysis_notify/upgrade_comment.md")
context = UpgradeCommentTemplateContext(
activation_link=get_members_url(context.pull.database_pull),
is_saas=not requires_license(),
author_username=context.pull.provider_pull["author"].get("username"),
)
return template.render(context)

@sentry_sdk.trace
async def send_message(
self, context: BundleAnalysisPRCommentNotificationContext, message: str
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
The author of this PR, {{author_username}}, {% if is_saas %}is not an activated member of this organization on Codecov.{% else %}is not activated in your Codecov Self-Hosted installation.{% endif %}
Please [activate this user]({{activation_link}}) to display this PR comment.
Bundle data is still being uploaded to {{ is_saas|yesno:"Codecov,your instance of Codecov"}} for purposes of overall calculations.
{% if is_saas %}
Please don't hesitate to email us at [email protected] with any questions.
{% else %}
Please contact your Codecov On-Premises installation administrator with any questions.
{% endif %}
Loading

0 comments on commit 88b6ffc

Please sign in to comment.