diff --git a/bugsink/registry.py b/bugsink/registry.py index acb395d..a953fc1 100644 --- a/bugsink/registry.py +++ b/bugsink/registry.py @@ -3,12 +3,11 @@ from datetime import datetime, timezone from projects.models import Project from events.models import Event +from issues.models import Issue from .period_counter import PeriodCounter from .volume_based_condition import VolumeBasedCondition -from issues.models import Issue - _registry = None UNMUTE_PURPOSE = "unmute" @@ -18,24 +17,7 @@ def create_unmute_issue_handler(issue_id): def unmute(): # we might just push this into a [class]method of Issue issue = Issue.objects.get(id=issue_id) - - if issue.is_muted: - # we check on is_muted explicitly: it may be so that multiple unmute conditions happens simultaneously (and - # not just in "funny configurations"). i.e. a single event could push you past more than 3 events per day or - # 100 events per year. We don't want 2 "unmuted" alerts being sent in that case. - - issue.is_muted = False - - issue.unmute_on_volume_based_conditions = "[]" - issue.save() - - # We keep the pc_registry and the value of issue.unmute_on_volume_based_conditions in-sync to avoid going - # mad (in general). A specific case that I can think of off the top of my head that goes wrong if you - # wouldn't do this, even given the fact that we check on is_muted in the above: you might re-mute but with - # different unmute conditions, and in that case you don't want your old outdated conditions triggering - # anything. (Side note: I'm not sure how I feel about reaching out to the global registry here; the - # alternative would be to pass this along.) - get_pc_registry().by_issue[issue_id].remove_event_listener(UNMUTE_PURPOSE) + issue.unmute() return unmute @@ -107,6 +89,12 @@ def get_pc_registry(): return _registry +def reset_pc_registry(): + # needed for tests + global _registry + _registry = None + + # some TODOs: # # * quotas (per project, per issue) diff --git a/issues/models.py b/issues/models.py index 97571c9..a7e575e 100644 --- a/issues/models.py +++ b/issues/models.py @@ -70,6 +70,27 @@ class Issue(models.Model): def occurs_in_last_release(self): return False # TODO actually implement (and then: implement in a performant manner) + def unmute(self): + from bugsink.registry import get_pc_registry, UNMUTE_PURPOSE # avoid circular import + + if self.is_muted: + # we check on is_muted explicitly: it may be so that multiple unmute conditions happens simultaneously (and + # not just in "funny configurations"). i.e. a single event could push you past more than 3 events per day or + # 100 events per year. We don't want 2 "unmuted" alerts being sent in that case. + + self.is_muted = False + + self.unmute_on_volume_based_conditions = "[]" + self.save() + + # We keep the pc_registry and the value of self.unmute_on_volume_based_conditions in-sync to avoid going + # mad (in general). A specific case that I can think of off the top of my head that goes wrong if you + # wouldn't do this, even given the fact that we check on is_muted in the above: you might re-mute but with + # different unmute conditions, and in that case you don't want your old outdated conditions triggering + # anything. (Side note: I'm not sure how I feel about reaching out to the global registry here; the + # alternative would be to pass this along.) + get_pc_registry().by_issue[self.id].remove_event_listener(UNMUTE_PURPOSE) + class IssueResolver(object): """basically: a namespace""" diff --git a/issues/tests.py b/issues/tests.py index 9596a6c..eb2ed5d 100644 --- a/issues/tests.py +++ b/issues/tests.py @@ -1,8 +1,11 @@ from unittest import TestCase from django.test import TestCase as DjangoTestCase +from datetime import datetime, timezone from projects.models import Project from releases.models import create_release_if_needed +from bugsink.registry import reset_pc_registry, get_pc_registry +from bugsink.period_counter import TL_DAY from .models import Issue, IssueResolver from .regressions import is_regression, is_regression_2, issue_is_regression @@ -258,3 +261,28 @@ the wild ("b") and resolve it ("c"). Then, if we were to see it again in "a", as would be seen as a regression when in reality it was never solved in "a", and its marking-as-such should probably have seen as an undo rather than anything else. """ + + +class UnmuteTestCase(TestCase): + + def setUp(self): + reset_pc_registry() + + def tearDown(self): + reset_pc_registry() + + def test_unmute_simple_case(self): + issue = Issue.objects.create( + unmute_on_volume_based_conditions='[{"period": "day", "nr_of_periods": 1, "volume": 1}]', + is_muted=True, + ) + + # because we create our objects before getting the lazy registry, event-listeners will be correctly set by + # registry.load_from_scratch() + registry = get_pc_registry() + + registry.by_issue[issue.id].inc(datetime.now(timezone.utc)) + + self.assertFalse(Issue.objects.get(id=issue.id).is_muted) + self.assertEquals("[]", Issue.objects.get(id=issue.id).unmute_on_volume_based_conditions) + self.assertEquals({}, registry.by_issue[issue.id].event_listeners[TL_DAY])