Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: count seats for BA notify #654

Merged
merged 2 commits into from
Aug 29, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@
),
)
return activation_success


def schedule_new_user_activated_task(self, org_ownerid, user_ownerid):
celery_app.send_task(

Check warning on line 108 in services/activation.py

View check run for this annotation

Codecov Notifications / codecov/patch

services/activation.py#L108

Added line #L108 was not covered by tests
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(

Check warning on line 114 in services/activation.py

View check run for this annotation

Codecov Notifications / codecov/patch

services/activation.py#L114

Added line #L114 was not covered by tests
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
39 changes: 36 additions & 3 deletions services/bundle_analysis/new_notify/messages/comment.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import logging
from typing import TypedDict
from typing import Literal, TypedDict

import sentry_sdk
from django.template import loader
Expand All @@ -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,26 @@ class BundleCommentTemplateContext(TypedDict):
bundle_rows: list[BundleRow]


class UpgradeCommentTemplateContext(TypedDict):
author_username: str
codecov_instance: Literal["cloud"] | Literal["self_hosted"]
codecov_name: Literal["Codecov"] | Literal["your instance of Codecov"]
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 +68,21 @@ 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),
codecov_instance="self_hosted" if requires_license() else "cloud",
codecov_name="your instance of Codecov"
if requires_license()
else "Codecov",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both of these depend on requires_license(). I would rather define a is_saas: bool field for the template and match on that. In particular, the template already matches on codecov_instance in a bunch of places, but the codecov_name is being resolved in python code instead.

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 codecov_instance == "cloud" %}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 {{codecov_name}} for purposes of overall calculations.
{% if codecov_instance == "cloud" %}
Please don't hesitate to email us at support@codecov.io with any questions.
{% else %}
Please contact your Codecov On-Premises installation administrator with any questions.
{% endif %}
62 changes: 62 additions & 0 deletions services/bundle_analysis/new_notify/messages/tests/test_comment.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from textwrap import dedent
from unittest.mock import AsyncMock, MagicMock

import pytest
Expand All @@ -8,6 +9,7 @@
from shared.yaml import UserYaml

from database.models.core import GITHUB_APP_INSTALLATION_DEFAULT_NAME
from database.tests.factories.core import PullFactory
from services.bundle_analysis.new_notify.conftest import (
get_commit_pair,
get_enriched_pull_setting_up_mocks,
Expand Down Expand Up @@ -300,3 +302,63 @@ async def test_send_message_fail(self, dbsession, mocker):
notification_successful=False,
explanation="TorngitClientError",
)


class TestUpgradeMessage:
def test_build_upgrade_message_cloud(self, dbsession, mocker):
head_commit, _ = get_commit_pair(dbsession)
context = BundleAnalysisPRCommentNotificationContext(
head_commit, GITHUB_APP_INSTALLATION_DEFAULT_NAME
)
context.should_use_upgrade_comment = True
context.pull = MagicMock(
name="fake_pull",
database_pull=PullFactory(),
provider_pull={"author": {"username": "PR_author_username"}},
)
mocker.patch(
"services.bundle_analysis.new_notify.messages.comment.requires_license",
return_value=False,
)
mocker.patch(
"services.bundle_analysis.new_notify.messages.comment.get_members_url",
return_value="http://members_url",
)
strategy = BundleAnalysisCommentMarkdownStrategy()
result = strategy.build_message(context)
assert result == dedent("""\
The author of this PR, PR_author_username, is not an activated member of this organization on Codecov.
Please [activate this user](http://members_url) to display this PR comment.
Bundle data is still being uploaded to Codecov for purposes of overall calculations.

Please don't hesitate to email us at support@codecov.io with any questions.
""")

def test_build_upgrade_message_self_hosted(self, dbsession, mocker):
head_commit, _ = get_commit_pair(dbsession)
context = BundleAnalysisPRCommentNotificationContext(
head_commit, GITHUB_APP_INSTALLATION_DEFAULT_NAME
)
context.should_use_upgrade_comment = True
context.pull = MagicMock(
name="fake_pull",
database_pull=PullFactory(),
provider_pull={"author": {"username": "PR_author_username"}},
)
mocker.patch(
"services.bundle_analysis.new_notify.messages.comment.requires_license",
return_value=True,
)
mocker.patch(
"services.bundle_analysis.new_notify.messages.comment.get_members_url",
return_value="http://members_url",
)
strategy = BundleAnalysisCommentMarkdownStrategy()
result = strategy.build_message(context)
assert result == dedent("""\
The author of this PR, PR_author_username, is not activated in your Codecov Self-Hosted installation.
Please [activate this user](http://members_url) to display this PR comment.
Bundle data is still being uploaded to your instance of Codecov for purposes of overall calculations.

Please contact your Codecov On-Premises installation administrator with any questions.
""")
Loading