Skip to content

Commit

Permalink
feat: don't send autoactivate pr comment if owner autoactivate is off (
Browse files Browse the repository at this point in the history
  • Loading branch information
giovanni-guidini authored Dec 13, 2024
1 parent a1dbbc3 commit bdc738f
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 1 deletion.
2 changes: 2 additions & 0 deletions services/notification/notifiers/comment/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
ComparisonHasPull,
HasEnoughBuilds,
HasEnoughRequiredChanges,
NoAutoActivateMessageIfAutoActivateIsOff,
NotifyCondition,
PullHeadMatchesComparisonHead,
PullRequestInProvider,
Expand All @@ -47,6 +48,7 @@ class CommentNotifier(MessageMixin, AbstractBaseNotifier):
PullHeadMatchesComparisonHead,
HasEnoughBuilds,
HasEnoughRequiredChanges,
NoAutoActivateMessageIfAutoActivateIsOff,
]

def store_results(self, comparison: ComparisonProxy, result: NotificationResult):
Expand Down
29 changes: 29 additions & 0 deletions services/notification/notifiers/comment/conditions.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,14 @@ class NotifyCondition(ABC):

failure_explanation: str

@staticmethod
@abstractmethod
def check_condition(
notifier: AbstractBaseNotifier, comparison: ComparisonProxy
) -> bool:
return True

@staticmethod
def on_failure_side_effect(
notifier: AbstractBaseNotifier, comparison: ComparisonProxy
) -> NotificationResult:
Expand All @@ -42,6 +44,7 @@ def on_failure_side_effect(
class ComparisonHasPull(NotifyCondition):
failure_explanation = "no_pull_request"

@staticmethod
def check_condition(
notifier: AbstractBaseNotifier, comparison: ComparisonProxy
) -> bool:
Expand All @@ -51,6 +54,7 @@ def check_condition(
class PullRequestInProvider(NotifyCondition):
failure_explanation = "pull_request_not_in_provider"

@staticmethod
def check_condition(
notifier: AbstractBaseNotifier, comparison: ComparisonProxy
) -> bool:
Expand All @@ -63,6 +67,7 @@ def check_condition(
class PullRequestOpen(NotifyCondition):
failure_explanation = "pull_request_closed"

@staticmethod
def check_condition(
notifier: AbstractBaseNotifier, comparison: ComparisonProxy
) -> bool:
Expand All @@ -72,6 +77,7 @@ def check_condition(
class PullHeadMatchesComparisonHead(NotifyCondition):
failure_explanation = "pull_head_does_not_match"

@staticmethod
def check_condition(
notifier: AbstractBaseNotifier, comparison: ComparisonProxy
) -> bool:
Expand All @@ -81,6 +87,7 @@ def check_condition(
class HasEnoughBuilds(NotifyCondition):
failure_explanation = "not_enough_builds"

@staticmethod
def check_condition(
notifier: AbstractBaseNotifier, comparison: ComparisonProxy
) -> bool:
Expand All @@ -92,23 +99,27 @@ def check_condition(
class HasEnoughRequiredChanges(NotifyCondition):
failure_explanation = "changes_required"

@staticmethod
def _check_unexpected_changes(comparison: ComparisonProxy) -> bool:
"""Returns a bool that indicates wether there are unexpected changes"""
return bool(comparison.get_changes())

@staticmethod
def _check_coverage_change(comparison: ComparisonProxy) -> bool:
"""Returns a bool that indicates wether there is any change in coverage"""
diff = comparison.get_diff()
res = comparison.head.report.calculate_diff(diff)
return res is not None and res["general"].lines > 0

@staticmethod
def _check_any_change(comparison: ComparisonProxy) -> bool:
unexpected_changes = HasEnoughRequiredChanges._check_unexpected_changes(
comparison
)
coverage_changes = HasEnoughRequiredChanges._check_coverage_change(comparison)
return unexpected_changes or coverage_changes

@staticmethod
def _check_coverage_drop(comparison: ComparisonProxy) -> bool:
no_head_coverage = comparison.head.report.totals.coverage is None
no_base_report = comparison.project_coverage_base.report is None
Expand Down Expand Up @@ -137,6 +148,7 @@ def _check_coverage_drop(comparison: ComparisonProxy) -> bool:
# Need to take the project threshold into consideration
return diff < 0 and abs(diff) >= (threshold + Decimal(0.01))

@staticmethod
def _check_uncovered_patch(comparison: ComparisonProxy) -> bool:
diff = comparison.get_diff(use_original_base=True)
totals = comparison.head.report.apply_diff(diff)
Expand All @@ -150,6 +162,7 @@ def _check_uncovered_patch(comparison: ComparisonProxy) -> bool:
0.01
)

@staticmethod
def check_condition_OR_group(
condition_group: CoverageCommentRequiredChangesORGroup,
comparison: ComparisonProxy,
Expand All @@ -174,6 +187,7 @@ def check_condition_OR_group(
final_result |= cache_results[individual_condition]
return final_result

@staticmethod
def check_condition(
notifier: AbstractBaseNotifier, comparison: ComparisonProxy
) -> bool:
Expand All @@ -195,3 +209,18 @@ def check_condition(
HasEnoughRequiredChanges.check_condition_OR_group(or_group, comparison)
for or_group in required_changes
)


class NoAutoActivateMessageIfAutoActivateIsOff(NotifyCondition):
failure_explanation = "auto_activate_message_but_auto_activate_is_off"

@staticmethod
def check_condition(
notifier: AbstractBaseNotifier, comparison: ComparisonProxy
) -> bool:
owner = notifier.repository.owner
# Return False ONLY if (owner.plan_auto_activate is False) and should_use_upgrade_message
# Checking if owner.plan_auto_activate is False so None will pass (tests)
return (owner.plan_auto_activate != False) or (
not notifier.should_use_upgrade_decoration()
)
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,14 @@
)
from shared.yaml import UserYaml

from database.enums import Decoration
from database.models.core import Repository
from services.comparison import ComparisonProxy
from services.notification.notifiers.comment import CommentNotifier
from services.notification.notifiers.comment.conditions import HasEnoughRequiredChanges
from services.notification.notifiers.comment.conditions import (
HasEnoughRequiredChanges,
NoAutoActivateMessageIfAutoActivateIsOff,
)


def _get_notifier(
Expand Down Expand Up @@ -253,3 +257,46 @@ def test_coverage_drop_with_different_project_configs(
None,
)
assert HasEnoughRequiredChanges.check_condition(notifier, comparison) == expected


@pytest.mark.parametrize(
"decoration_type, plan_auto_activate, expected",
[
pytest.param(
Decoration.upgrade, False, False, id="upgrade_no_auto_activate__dont_send"
),
pytest.param(Decoration.upgrade, True, True, id="upgrade_auto_activate__send"),
pytest.param(
Decoration.upload_limit,
False,
True,
id="other_decoration_no_auto_activate__send",
),
pytest.param(
Decoration.upload_limit,
True,
True,
id="other_decoration_auto_activate__send",
),
],
)
def test_no_auto_activate_message_if_auto_activate_is_off(
sample_comparison_no_change,
mock_repo_provider,
decoration_type,
plan_auto_activate,
expected,
):
notifier = _get_notifier(
sample_comparison_no_change.head.commit.repository,
[CoverageCommentRequiredChanges.any_change.value],
mock_repo_provider,
)
notifier.decoration_type = decoration_type
notifier.repository.owner.plan_auto_activate = plan_auto_activate
assert (
NoAutoActivateMessageIfAutoActivateIsOff.check_condition(
notifier, sample_comparison_no_change
)
== expected
)

0 comments on commit bdc738f

Please sign in to comment.