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: notify_error default=True for repos created after certain date #320

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
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
9 changes: 9 additions & 0 deletions shared/config/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,15 @@ class MissingConfigException(Exception):
},
}

NOTIFY_ERROR_TIME_START = datetime.fromisoformat("2024-09-02 00:00:00.000+00:00")


def add_notify_error_to_config(config: dict[str, Any]):
codecov: dict[str, Any] = config.setdefault("codecov", {})
notify: dict[str, Any] = codecov.setdefault("notify", {})
notify.setdefault("notify_error", True)


default_config = {
"services": {
"minio": {
Expand Down
24 changes: 24 additions & 0 deletions shared/django_apps/core/migrations/0057_repository_created_at.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# Generated by Django 4.2.13 on 2024-08-08 19:49

from django.db import migrations

"""
BEGIN;
--
-- Raw SQL operation
--
ALTER TABLE "repos" ADD COLUMN "created_at" timestamp with time zone DEFAULT null NULL;
COMMIT;
"""


class Migration(migrations.Migration):
dependencies = [
("core", "0056_branch_name_trgm_idx"),
]

operations = [
migrations.RunSQL(
'ALTER TABLE "repos" ADD COLUMN "created_at" timestamp with time zone DEFAULT null NULL;'
Copy link
Contributor

Choose a reason for hiding this comment

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

I don’t think you have to define any DEFAULT if the column is defined as NULLable.

),
]
1 change: 1 addition & 0 deletions shared/django_apps/core/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ class Languages(models.TextChoices):
bundle_analysis_enabled = models.BooleanField(default=False, null=True)
coverage_enabled = models.BooleanField(default=False, null=True)
test_analytics_enabled = models.BooleanField(default=False, null=True)
created_at = models.DateTimeField(null=True, auto_now_add=True)

# tracks field changes being saved
tracker = FieldTracker()
Expand Down
45 changes: 31 additions & 14 deletions shared/yaml/user_yaml.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@

from shared.components import Component
from shared.config import (
NOTIFY_ERROR_TIME_START,
PATCH_CENTRIC_DEFAULT_CONFIG,
PATCH_CENTRIC_DEFAULT_TIME_START,
add_notify_error_to_config,
get_config,
)

Expand All @@ -21,6 +23,11 @@ class OwnerContext(object):
owner_plan: Optional[str] = None


@dataclass
class RepoContext(object):
repo_creation_date: datetime | None = None


class UserYaml(object):
def __init__(self, inner_dict):
self.inner_dict = inner_dict
Expand Down Expand Up @@ -119,11 +126,12 @@ def __str__(self):
def get_final_yaml(
cls,
*,
owner_yaml: Optional[dict[str, Any]] = None,
repo_yaml: Optional[dict[str, Any]] = None,
commit_yaml: Optional[dict[str, Any]] = None,
ownerid: int = None,
owner_context: Optional[OwnerContext] = None,
owner_yaml: dict[str, Any] | None = None,
repo_yaml: dict[str, Any] | None = None,
commit_yaml: dict[str, Any] | None = None,
ownerid: int | None = None,
owner_context: OwnerContext | None = None,
repo_context: RepoContext | None = None,
):
"""Given a owner yaml, repo yaml and user yaml, determines what yaml we need to use

Expand Down Expand Up @@ -159,10 +167,11 @@ def get_final_yaml(
additional_yaml = _get_possible_additional_user_yaml(ownerid)
resulting_yaml = merge_yamls(resulting_yaml, additional_yaml)

if owner_context and owner_context.owner_onboarding_date:
resulting_yaml = _fix_yaml_defaults_based_on_owner_onboarding_date(
resulting_yaml, owner_context.owner_onboarding_date
)
resulting_yaml = _fix_yaml_defaults_based_on_time(
resulting_yaml,
getattr(owner_context, "owner_onboarding_date", None),
getattr(repo_context, "repo_creation_date", None),
)

if owner_yaml is not None:
resulting_yaml = merge_yamls(resulting_yaml, owner_yaml)
Expand All @@ -185,8 +194,10 @@ def _get_possible_additional_user_yaml(ownerid):
return {}


def _fix_yaml_defaults_based_on_owner_onboarding_date(
current_yaml: dict, owner_onboarding_date: datetime
def _fix_yaml_defaults_based_on_time(
current_yaml: dict,
owner_onboarding_date: datetime | None,
repo_creation_date: datetime | None,
) -> dict:
"""Changes the site defaults based on the owner_onboarding_date.
Owners onboarded (Owner.created_at) after PATCH_CENTRIC_DEFAULT_TIME_START use the
Expand All @@ -195,9 +206,15 @@ def _fix_yaml_defaults_based_on_owner_onboarding_date(
Owners can still override the defaults through owner_yaml, repo_yaml and commit_yaml.
"""
res = deepcopy(current_yaml)
if owner_onboarding_date > PATCH_CENTRIC_DEFAULT_TIME_START:
adding = deepcopy(PATCH_CENTRIC_DEFAULT_CONFIG)
res.update(adding)
if owner_onboarding_date:
if owner_onboarding_date > PATCH_CENTRIC_DEFAULT_TIME_START:
adding = deepcopy(PATCH_CENTRIC_DEFAULT_CONFIG)
res.update(adding)

if repo_creation_date:
if repo_creation_date > NOTIFY_ERROR_TIME_START:
add_notify_error_to_config(res)

return res


Expand Down
28 changes: 23 additions & 5 deletions tests/unit/yaml/test_user_yaml.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import datetime
from copy import deepcopy

from freezegun import freeze_time

Expand All @@ -11,7 +12,7 @@
from shared.yaml import UserYaml, merge_yamls
from shared.yaml.user_yaml import (
OwnerContext,
_fix_yaml_defaults_based_on_owner_onboarding_date,
_fix_yaml_defaults_based_on_time,
_get_possible_additional_user_yaml,
)

Expand Down Expand Up @@ -348,21 +349,38 @@ def test_get_final_yaml_nothing(self, mock_configuration):
def test_default_yaml_behavior_change(self):
current_yaml = LEGACY_DEFAULT_SITE_CONFIG
day_timedelta = datetime.timedelta(days=1)
year_timedelta = datetime.timedelta(days=365)
patch_centric_expected_onboarding_date = (
datetime.datetime.now(datetime.timezone.utc) + day_timedelta
)
no_change_expected_onboarding_date = (
datetime.datetime.now(datetime.timezone.utc) - day_timedelta
)
no_change = _fix_yaml_defaults_based_on_owner_onboarding_date(
current_yaml, no_change_expected_onboarding_date
notify_error_expected_onboarding_date = (
datetime.datetime.now(datetime.timezone.utc) + year_timedelta
)
no_change = _fix_yaml_defaults_based_on_time(
current_yaml,
no_change_expected_onboarding_date,
no_change_expected_onboarding_date,
)
assert no_change == current_yaml
patch_centric = _fix_yaml_defaults_based_on_owner_onboarding_date(
current_yaml, patch_centric_expected_onboarding_date
patch_centric = _fix_yaml_defaults_based_on_time(
current_yaml,
patch_centric_expected_onboarding_date,
no_change_expected_onboarding_date,
)
assert patch_centric != current_yaml
assert patch_centric == PATCH_CENTRIC_DEFAULT_CONFIG
patch_centric = _fix_yaml_defaults_based_on_time(
current_yaml,
patch_centric_expected_onboarding_date,
notify_error_expected_onboarding_date,
)
assert patch_centric != current_yaml
notify_error_config = deepcopy(PATCH_CENTRIC_DEFAULT_CONFIG)
notify_error_config["codecov"]["notify"]["notify_error"] = True
assert patch_centric == notify_error_config

def test_get_final_yaml_default_based_on_owner_context(self):
day_timedelta = datetime.timedelta(days=1)
Expand Down
Loading