Skip to content

Commit

Permalink
Treat intent threads as new based on gate.status. (#4558)
Browse files Browse the repository at this point in the history
  • Loading branch information
jrobbins authored Nov 13, 2024
1 parent 2c22778 commit f9f5571
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 15 deletions.
9 changes: 4 additions & 5 deletions internals/detect_intent.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,10 +169,9 @@ def is_lgtm_allowed(from_addr, feature, gate_info):
return allowed


def detect_new_thread(gate_id: int) -> bool:
"""Return True if there are no previous approval values for this gate."""
existing_votes = Vote.get_votes(gate_id=gate_id)
return not existing_votes
def detect_new_thread(gate: Gate) -> bool:
"""Treat the thread as new if the associated Gate is still PREPARING."""
return gate.state == Gate.PREPARING


def remove_markdown(body):
Expand Down Expand Up @@ -239,7 +238,7 @@ def process_post_data(self, **kwargs):

self.set_intent_thread_url(stage, thread_url, subject)
gate_id = gate.key.integer_id() # In case it was found by gate_type.
is_new_thread = detect_new_thread(gate_id)
is_new_thread = detect_new_thread(gate)
self.create_approvals(
feature, stage, gate, gate_info, from_addr, body, is_new_thread)
self.record_slo(feature, gate_info, from_addr, is_new_thread)
Expand Down
25 changes: 15 additions & 10 deletions internals/detect_intent_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -426,15 +426,20 @@ def test_is_lgtm_allowed__other(self, mock_get_approvers):
self.assertFalse(detect_intent.is_lgtm_allowed(
'[email protected]', self.feature_1, approval_defs.ShipApproval))

def test_detect_new_thread__no_votes(self):
"""New thread is detected when if no votes exist for a given gate."""
def test_detect_new_thread__no_gate_activity(self):
"""New thread is detected when its gate is still PREPARING."""
self.assertTrue(detect_intent.detect_new_thread(
self.gate_1.key.integer_id()))
Gate(id=301, feature_id=1, stage_id=20, gate_type=3,
state=Gate.PREPARING)))

def test_detect_new_thread__existing_votes(self):
"""Existing thread is detected when if votes exist for a given gate."""
def test_detect_new_thread__gate_was_updated(self):
"""Existing thread is detected when its gate has already changed state."""
self.assertFalse(detect_intent.detect_new_thread(
self.gate_2.key.integer_id()))
Gate(id=302, feature_id=1, stage_id=20, gate_type=3,
state=Vote.REVIEW_REQUESTED)))
self.assertFalse(detect_intent.detect_new_thread(
Gate(id=303, feature_id=1, stage_id=20, gate_type=3,
state=Vote.REVIEW_STARTED)))


class IntentEmailHandlerTest(testing_config.CustomTestCase):
Expand All @@ -456,13 +461,13 @@ def setUp(self):
for gate_type in gate_types:
gate = Gate(feature_id=self.feature_id,
stage_id=stage.key.integer_id(), gate_type=gate_type,
state=Vote.NA)
state=Gate.PREPARING)
gate.put()
extra_extension_stage = Stage(feature_id=self.feature_id, stage_type=151)
extra_extension_stage.put()
extra_extension_gate = Gate(feature_id=self.feature_id,
stage_id=extra_extension_stage.key.integer_id(),
gate_type=3, state=Vote.NA)
gate_type=3, state=Gate.PREPARING)
extra_extension_gate.put()
self.stages_dict = stage_helpers.get_feature_stages(self.feature_id)
# The intent thread url already exists for the first extension stage.
Expand All @@ -472,11 +477,11 @@ def setUp(self):
self.stage_1 = self.stages_dict[151][0]
self.gate_1 = Gate(
feature_id=self.feature_id, stage_id=self.stage_1.key.integer_id(),
gate_type=3, state=Vote.NA)
gate_type=3, state=Gate.PREPARING)
self.gate_1.put()
self.gate_with_experiment_type = Gate(
feature_id=self.feature_id, stage_id=self.stage_1.key.integer_id(),
gate_type=2, state=Vote.NA)
gate_type=2, state=Gate.PREPARING)
self.gate_with_experiment_type.put()

self.request_path = '/tasks/detect-intent'
Expand Down

0 comments on commit f9f5571

Please sign in to comment.