Merge pull request 'Tickets: don't adopt a Closed ticket Tinker couldn't reopen' (#34) from tinker-create-action into main

monitor.py

@@ -393,6 +393,8 @@ class TicketClient:
         self.comment_url = cfg.get('comment_url') or (
             urljoin(self.url, 'api/ticket_comment_api.php') if self.url else '')
         self.comments_denied = False
+        # Titles whose create matched a Closed ticket Tinker couldn't reopen: {title: (ticket_id, when)}
+        self._closed_matches: Dict[str, tuple] = {}
 
     def comment(self, ticket_id: str, text: str) -> bool:
         """Add a Markdown comment to a ticket. Never raises; False means try again later."""
@@ -425,10 +427,23 @@ class TicketClient:
             logger.error(f'Ticket comment failed for #{ticket_id}: {e}')
         return False
 
+    # How long to wait before asking again after a create matched an unreopenable Closed ticket.
+    # Each ask adds a "recurred" comment to that ticket, so once a poll would pile them up.
+    CLOSED_MATCH_RETRY_S = 24 * 3600
+
     def create(self, title: str, description: str, priority: str = '2') -> Optional[str]:
+        """Open a ticket. Returns its ID, or None if there is no ticket to track.
+
+        Tinker deduplicates: the response's `action` says what happened (new, deduplicated,
+        updated, reopened, recurrence_noted). recurrence_noted means it matched a Closed ticket
+        it could not reopen unattended; that ticket isn't tracking this alert, so it isn't used.
+        """
         if not self.api_key or not self.url:
             logger.warning('Ticket API not configured: skipping ticket creation')
             return None
+        seen = self._closed_matches.get(title)
+        if seen and time.time() - seen[1] < self.CLOSED_MATCH_RETRY_S:
+            return None
         try:
             resp = requests.post(
                 self.url,
@@ -450,7 +465,16 @@ class TicketClient:
                 if not tid:
                     logger.warning(f'Ticket API success but no ticket_id in response: {data}')
                     return None
-                logger.info(f'Created ticket #{tid}: {title}')
+                action = data.get('action') or 'created'
+                if action == 'recurrence_noted':
+                    self._closed_matches[title] = (tid, time.time())
+                    logger.warning(
+                        f'Ticket API matched Closed ticket #{tid}, which it could not reopen; not using it '
+                        f'for this alert. Reopen #{tid} in Tinker to track it there. Retrying in 24 h: {title}'
+                    )
+                    return None
+                self._closed_matches.pop(title, None)
+                logger.info(f'Ticket #{tid} ({action}): {title}')
                 return tid
             if data.get('existing_ticket_id'):
                 logger.info(f'Duplicate suppressed by API: existing #{data["existing_ticket_id"]}')

tests/test_monitor.py

@@ -748,3 +748,38 @@ class TestTicketClientComment:
         with patch.object(monitor.requests, 'post', side_effect=OSError('down')):
             assert c.comment('42', 'hi') is False
         assert not c.comments_denied
+
+
+class TestTicketClientCreate:
+    def _client(self):
+        return monitor.TicketClient({'url': 'http://t.test/create_ticket_api.php', 'api_key': 'k'})
+
+    def _post(self, body):
+        resp = MagicMock(status_code=200)
+        resp.json.return_value = body
+        return patch.object(monitor.requests, 'post', return_value=resp)
+
+    def test_new_and_deduplicated_tickets_are_used(self):
+        c = self._client()
+        with self._post({'success': True, 'ticket_id': '5', 'action': 'deduplicated'}):
+            assert c.create('t', 'd') == '5'
+        with self._post({'success': True, 'ticket_id': '6'}):
+            assert c.create('u', 'd') == '6'
+
+    def test_recurrence_on_a_closed_ticket_is_not_used_and_not_retried_each_poll(self):
+        c = self._client()
+        with self._post({'success': True, 'ticket_id': '993', 'action': 'recurrence_noted'}) as post:
+            assert c.create('t', 'd') is None
+            assert c.create('t', 'd') is None
+        assert post.call_count == 1  # the second poll doesn't add another comment to the closed ticket
+        with self._post({'success': True, 'ticket_id': '7', 'action': 'none'}) as post:
+            assert c.create('other', 'd') == '7'  # other alerts are unaffected
+
+    def test_retry_after_a_day(self):
+        c = self._client()
+        with self._post({'success': True, 'ticket_id': '993', 'action': 'recurrence_noted'}):
+            c.create('t', 'd')
+        c._closed_matches['t'] = ('993', time.time() - c.CLOSED_MATCH_RETRY_S - 1)
+        with self._post({'success': True, 'ticket_id': '8', 'action': 'none'}):
+            assert c.create('t', 'd') == '8'
+        assert 't' not in c._closed_matches