Add message when upload count differs (#500)

Co-authored-by: michelletran-codecov <[email protected]>

services/comparison/__init__.py

@@ -302,14 +302,15 @@ def get_reports_uploaded_count_per_flag(self) -> List[ReportUploadedCount]:
         head_report = self.comparison.head.report
         ops = [(base_report, "base_count"), (head_report, "head_count")]
         for curr_report, curr_counter in ops:
-            for session in curr_report.sessions:
+            for session in curr_report.sessions.values():
                 # We ignore carryforward sessions
                 # Because not all commits would upload all flags (potentially)
                 # But they are still carried forward
                 if session.session_type != SessionType.carriedforward:
-                    if session.flags == []:
-                        session.flags = [""]
-                    for flag in session.flags:
+                    # It's possible that an upload is done without flags.
+                    # In this case we still want to count it in the count, but there's no flag associated to it
+                    session_flags = session.flags or [""]
+                    for flag in session_flags:
                         dict_value = per_flag_dict.get(flag)
                         if dict_value is None:
                             dict_value = ReportUploadedCount(

services/comparison/tests/unit/test_reports_uploaded_count_diff.py

@@ -12,24 +12,26 @@
     "head_sessions, base_sessions, expected_count, expected_diff",
     [
         (
-            [
-                Session(
+            {
+                0: Session(
                     flags=["unit", "local"], session_type=SessionType.carriedforward
                 ),
-                Session(flags=["integration"], session_type=SessionType.uploaded),
-                Session(flags=["unit"], session_type=SessionType.uploaded),
-                Session(flags=["unit"], session_type=SessionType.uploaded),
-                Session(flags=["integration"], session_type=SessionType.uploaded),
-                Session(flags=[], session_type=SessionType.uploaded),
-            ],
-            [
-                Session(
+                1: Session(flags=["integration"], session_type=SessionType.uploaded),
+                2: Session(flags=["unit"], session_type=SessionType.uploaded),
+                3: Session(flags=["unit"], session_type=SessionType.uploaded),
+                4: Session(flags=["integration"], session_type=SessionType.uploaded),
+                5: Session(flags=[], session_type=SessionType.uploaded),
+            },
+            {
+                0: Session(
                     flags=["unit", "local"], session_type=SessionType.carriedforward
                 ),
-                Session(flags=["integration"], session_type=SessionType.carriedforward),
-                Session(flags=["unit"], session_type=SessionType.uploaded),
-                Session(flags=["unit"], session_type=SessionType.uploaded),
-            ],
+                1: Session(
+                    flags=["integration"], session_type=SessionType.carriedforward
+                ),
+                2: Session(flags=["unit"], session_type=SessionType.uploaded),
+                3: Session(flags=["unit"], session_type=SessionType.uploaded),
+            },
             [
                 ReportUploadedCount(flag="unit", base_count=2, head_count=2),
                 ReportUploadedCount(flag="integration", base_count=0, head_count=2),
@@ -38,23 +40,23 @@
             [],
         ),
         (
-            [
-                Session(
+            {
+                0: Session(
                     flags=["unit", "local"], session_type=SessionType.carriedforward
                 ),
-                Session(flags=["integration"], session_type=SessionType.uploaded),
-                Session(flags=["unit"], session_type=SessionType.uploaded),
-                Session(flags=["unit"], session_type=SessionType.uploaded),
-                Session(flags=["integration"], session_type=SessionType.uploaded),
-                Session(flags=[""], session_type=SessionType.uploaded),
-            ],
-            [
-                Session(flags=["unit", "local"], session_type=SessionType.uploaded),
-                Session(flags=["integration"], session_type=SessionType.uploaded),
-                Session(flags=["unit"], session_type=SessionType.uploaded),
-                Session(flags=["unit"], session_type=SessionType.uploaded),
-                Session(flags=["obscure_flag"], session_type=SessionType.uploaded),
-            ],
+                1: Session(flags=["integration"], session_type=SessionType.uploaded),
+                2: Session(flags=["unit"], session_type=SessionType.uploaded),
+                3: Session(flags=["unit"], session_type=SessionType.uploaded),
+                4: Session(flags=["integration"], session_type=SessionType.uploaded),
+                5: Session(flags=[""], session_type=SessionType.uploaded),
+            },
+            {
+                0: Session(flags=["unit", "local"], session_type=SessionType.uploaded),
+                1: Session(flags=["integration"], session_type=SessionType.uploaded),
+                2: Session(flags=["unit"], session_type=SessionType.uploaded),
+                3: Session(flags=["unit"], session_type=SessionType.uploaded),
+                4: Session(flags=["obscure_flag"], session_type=SessionType.uploaded),
+            },
             [
                 ReportUploadedCount(flag="unit", base_count=3, head_count=2),
                 ReportUploadedCount(flag="local", base_count=1, head_count=0),
@@ -67,8 +69,21 @@
                 ReportUploadedCount(flag="integration", base_count=1, head_count=2),
             ],
         ),
+        (
+            {0: Session(flags=[], session_type=SessionType.uploaded)},
+            {
+                0: Session(flags=[], session_type=SessionType.uploaded),
+                1: Session(flags=[], session_type=SessionType.uploaded),
+            },
+            [ReportUploadedCount(flag="", base_count=2, head_count=1)],
+            [ReportUploadedCount(flag="", base_count=2, head_count=1)],
+        ),
+    ],
+    ids=[
+        "flag_counts_no_diff",
+        "flag_count_yes_diff",
+        "diff_from_session_with_no_flags",
     ],
-    ids=["flag_counts_no_diff", "flag_count_yes_diff"],
 )
 def test_get_reports_uploaded_count_per_flag(
     head_sessions, base_sessions, expected_count, expected_diff

services/notification/notifiers/mixins/message/__init__.py

@@ -1,11 +1,10 @@
 import logging
-from typing import Callable, List
+from typing import List
 
 from shared.reports.resources import Report, ReportTotals
 from shared.validation.helpers import LayoutStructure
 
 from database.models.core import Owner
-from helpers.environment import is_enterprise
 from helpers.metrics import metrics
 from services.billing import BillingPlan
 from services.comparison import ComparisonProxy
@@ -93,6 +92,8 @@ async def create_message(
         # note: since we're using append, calling write("") will add a newline to the message
 
         upper_section_names = self.get_upper_section_names(settings)
+        # We write the header and then the messages_to_user section
+        upper_section_names.append("messages_to_user")
         for upper_section_name in upper_section_names:
             section_writer_class = get_section_class_from_layout_name(
                 upper_section_name
@@ -114,10 +115,6 @@ async def create_message(
         if base_report is None:
             base_report = Report()
 
-        # Announcement section specific for the GitHub App Login changes
-        # This might be removed eventually
-        await self._possibly_write_gh_app_login_announcement(comparison, write)
-
         if head_report:
             if is_compact_message:
                 write(
@@ -181,21 +178,6 @@ async def create_message(
 
         return [m for m in message if m is not None]
 
-    async def _possibly_write_gh_app_login_announcement(
-        self, comparison: ComparisonProxy, write: Callable
-    ) -> None:
-        repo = comparison.head.commit.repository
-        owner = repo.owner
-        if (
-            owner.service == "github"
-            and not owner.integration_id
-            and owner.github_app_installations == []
-            and not is_enterprise()
-        ):
-            message_to_display = "Your organization needs to install the [Codecov GitHub app](https://github.com/apps/codecov/installations/select_target) to enable full functionality."
-            write(f":exclamation: {message_to_display}")
-            write("")
-
     def _team_plan_notification(
         self,
         comparison: ComparisonProxy,
@@ -230,15 +212,17 @@ def _team_plan_notification(
     async def write_section_to_msg(
         self, comparison, changes, diff, links, write, section_writer, behind_by=None
     ):
+        wrote_something: bool = False
         with metrics.timer(
             f"worker.services.notifications.notifiers.comment.section.{section_writer.name}"
         ):
             for line in await section_writer.write_section(
                 comparison, diff, changes, links, behind_by=behind_by
             ):
+                wrote_something |= line != None
                 write(line)
-
-        write("")
+        if wrote_something:
+            write("")
 
     def get_middle_layout_section_names(self, settings):
         sections = map(lambda l: l.strip(), (settings["layout"] or "").split(","))

services/notification/notifiers/mixins/message/helpers.py

@@ -3,13 +3,53 @@
 from typing import List, Sequence
 
 from shared.reports.resources import ReportTotals
+from shared.yaml.user_yaml import UserYaml
 
+from services.comparison import ComparisonProxy
 from services.comparison.changes import Change
+from services.yaml import read_yaml_field
 from services.yaml.reader import get_minimum_precision, round_number
 
 zero_change_regex = re.compile("0.0+%?")
 
 
+def has_project_status(yaml: UserYaml) -> bool:
+    project_status_details = read_yaml_field(
+        yaml, ("coverage", "status", "project"), False
+    )
+    if type(project_status_details) == bool:
+        return project_status_details
+    # If it's not a bool, it has to be a dict
+    if type(project_status_details) == dict:
+        if "enabled" in project_status_details:
+            return project_status_details["enabled"]
+        return True
+    # The config is not according to what we expect
+    # So it's an invalid status definition
+    return False
+
+
+def is_coverage_drop_significant(comparison: ComparisonProxy) -> bool:
+    head_coverage = (
+        comparison.head.report.totals.coverage if comparison.has_head_report() else None
+    )
+    base_coverage = (
+        comparison.project_coverage_base.report.totals.coverage
+        if comparison.has_project_coverage_base_report()
+        else None
+    )
+    if head_coverage is None or base_coverage is None:
+        # No change to be significant
+        return False
+    diff = Decimal(head_coverage) - Decimal(base_coverage)
+    change_is_positive = diff >= 0
+    # head_coverage is the percent of the project covered in HEAD
+    # base_coverage is the percent of the project covered in BASE
+    # So "change_is_significant" is checking if the diff between them is more than 5%
+    change_is_significant = abs(diff) >= Decimal(5)
+    return not change_is_positive and change_is_significant
+
+
 def make_metrics(before, after, relative, show_complexity, yaml, pull_url=None):
     coverage_good = None
     icon = " |"
@@ -48,18 +88,20 @@ def make_metrics(before, after, relative, show_complexity, yaml, pull_url=None):
             relative=format_number_to_str(
                 yaml, relative.coverage if relative else 0, style="{0}%", if_null="\xf8"
             ),
-            impact=format_number_to_str(
-                yaml,
-                coverage_change,
-                style="{0}%",
-                if_zero="\xf8",
-                if_null="\u2205",
-                plus=True,
-            )
-            if before
-            else "?"
-            if before is None
-            else "\xf8",
+            impact=(
+                format_number_to_str(
+                    yaml,
+                    coverage_change,
+                    style="{0}%",
+                    if_zero="\xf8",
+                    if_null="\u2205",
+                    plus=True,
+                )
+                if before
+                else "?"
+                if before is None
+                else "\xf8"
+            ),
         )
 
         if show_complexity:
@@ -108,9 +150,11 @@ def make_metrics(before, after, relative, show_complexity, yaml, pull_url=None):
             icon = (
                 " :arrow_up: |"
                 if coverage_good
-                else " :arrow_down: |"
-                if coverage_good is False and coverage_change != 0
-                else " |"
+                else (
+                    " :arrow_down: |"
+                    if coverage_good is False and coverage_change != 0
+                    else " |"
+                )
             )
 
     return "".join(("|", coverage, complexity, icon))