@@ -222,6 +222,17 @@ Tables
- ``url``: URL to the merge request
- ``created_at``: Timestamp when the MR was created
+**comment**
+ Tracks MR comments that have been processed by the review agent.
+
+ - ``id``: Primary key
+ - ``mr_iid``: GitLab merge request IID
+ - ``comment_id``: GitLab comment/note ID
+ - ``processed_at``: Timestamp when the comment was processed
+
+ This table prevents the same comment from being addressed multiple times
+ when running ``review`` or ``poll`` commands.
+
Configuration
-------------
@@ -11,6 +11,7 @@ To adjust the schema, increment LATEST, create a _migrate_to_v<x>() function
and add code in migrate_to() to call it.
"""
+from datetime import datetime
import os
import sqlite3
@@ -18,7 +19,7 @@ from u_boot_pylib import tools
from u_boot_pylib import tout
# Schema version (version 0 means there is no database yet)
-LATEST = 2
+LATEST = 3
# Default database filename
DB_FNAME = '.pickman.db'
@@ -129,6 +130,17 @@ class Database: # pylint: disable=too-many-public-methods
'created_at TEXT, '
'FOREIGN KEY (source_id) REFERENCES source(id))')
+ def _create_v3(self):
+ """Migrate database to v3 schema - add comment table"""
+ # Table for tracking processed MR comments
+ self.cur.execute(
+ 'CREATE TABLE comment ('
+ 'id INTEGER PRIMARY KEY AUTOINCREMENT, '
+ 'mr_iid INTEGER, '
+ 'comment_id INTEGER, '
+ 'processed_at TEXT, '
+ 'UNIQUE(mr_iid, comment_id))')
+
def migrate_to(self, dest_version):
"""Migrate the database to the selected version
@@ -151,6 +163,8 @@ class Database: # pylint: disable=too-many-public-methods
self._create_v1()
elif version == 2:
self._create_v2()
+ elif version == 3:
+ self._create_v3()
self.cur.execute('DELETE FROM schema_version')
self.cur.execute(
@@ -413,3 +427,46 @@ class Database: # pylint: disable=too-many-public-methods
"""
self.execute(
'UPDATE mergereq SET status = ? WHERE mr_id = ?', (status, mr_id))
+
+ # comment functions
+
+ def comment_is_processed(self, mr_iid, comment_id):
+ """Check if a comment has been processed
+
+ Args:
+ mr_iid (int): Merge request IID
+ comment_id (int): Comment ID
+
+ Return:
+ bool: True if already processed
+ """
+ res = self.execute(
+ 'SELECT id FROM comment WHERE mr_iid = ? AND comment_id = ?',
+ (mr_iid, comment_id))
+ return res.fetchone() is not None
+
+ def comment_mark_processed(self, mr_iid, comment_id):
+ """Mark a comment as processed
+
+ Args:
+ mr_iid (int): Merge request IID
+ comment_id (int): Comment ID
+ """
+ self.execute(
+ 'INSERT OR IGNORE INTO comment '
+ '(mr_iid, comment_id, processed_at) VALUES (?, ?, ?)',
+ (mr_iid, comment_id, datetime.now().isoformat()))
+
+ def comment_get_processed(self, mr_iid):
+ """Get all processed comment IDs for an MR
+
+ Args:
+ mr_iid (int): Merge request IID
+
+ Return:
+ list: List of comment IDs
+ """
+ res = self.execute(
+ 'SELECT comment_id FROM comment WHERE mr_iid = ?',
+ (mr_iid,))
+ return [row[0] for row in res.fetchall()]
@@ -721,6 +721,88 @@ class TestDatabaseCommitMergereq(unittest.TestCase):
dbs.close()
+class TestDatabaseComment(unittest.TestCase):
+ """Tests for Database comment functions."""
+
+ def setUp(self):
+ """Set up test fixtures."""
+ fd, self.db_path = tempfile.mkstemp(suffix='.db')
+ os.close(fd)
+ os.unlink(self.db_path)
+
+ def tearDown(self):
+ """Clean up test fixtures."""
+ if os.path.exists(self.db_path):
+ os.unlink(self.db_path)
+ database.Database.instances.clear()
+
+ def test_comment_mark_and_check_processed(self):
+ """Test marking and checking processed comments"""
+ with terminal.capture():
+ dbs = database.Database(self.db_path)
+ dbs.start()
+
+ # Comment should not be processed initially
+ self.assertFalse(dbs.comment_is_processed(123, 456))
+
+ # Mark as processed
+ dbs.comment_mark_processed(123, 456)
+ dbs.commit()
+
+ # Now should be processed
+ self.assertTrue(dbs.comment_is_processed(123, 456))
+
+ # Different comment should not be processed
+ self.assertFalse(dbs.comment_is_processed(123, 789))
+ self.assertFalse(dbs.comment_is_processed(999, 456))
+
+ dbs.close()
+
+ def test_comment_get_processed(self):
+ """Test getting all processed comments for an MR"""
+ with terminal.capture():
+ dbs = database.Database(self.db_path)
+ dbs.start()
+
+ # Mark several comments as processed
+ dbs.comment_mark_processed(100, 1)
+ dbs.comment_mark_processed(100, 2)
+ dbs.comment_mark_processed(100, 3)
+ dbs.comment_mark_processed(200, 10) # Different MR
+ dbs.commit()
+
+ # Get processed for MR 100
+ processed = dbs.comment_get_processed(100)
+ self.assertEqual(len(processed), 3)
+ self.assertIn(1, processed)
+ self.assertIn(2, processed)
+ self.assertIn(3, processed)
+ self.assertNotIn(10, processed)
+
+ # Get processed for MR 200
+ processed = dbs.comment_get_processed(200)
+ self.assertEqual(len(processed), 1)
+ self.assertIn(10, processed)
+
+ dbs.close()
+
+ def test_comment_mark_processed_idempotent(self):
+ """Test that marking same comment twice doesn't fail"""
+ with terminal.capture():
+ dbs = database.Database(self.db_path)
+ dbs.start()
+
+ # Mark same comment twice (should not raise)
+ dbs.comment_mark_processed(123, 456)
+ dbs.comment_mark_processed(123, 456)
+ dbs.commit()
+
+ # Should still be processed
+ self.assertTrue(dbs.comment_is_processed(123, 456))
+
+ dbs.close()
+
+
class TestListSources(unittest.TestCase):
"""Tests for list-sources command."""