[Concept,26/37] patman: Extend database for review draft tracking and notes

Message ID 20260404213020.372253-27-sjg@u-boot.org
State New
Headers
Series patman: Autolink fixes and AI-assisted patch review |

Commit Message

Simon Glass April 4, 2026, 9:29 p.m. UTC
  From: Simon Glass <sjg@chromium.org>

Add schema migrations v9 and v10 and supporting methods:

 - v9: Add draft_id, status, gmail_msg_id and gmail_thread_id columns
   to the review table for tracking Gmail draft lifecycle
 - v10: Add notes column to ser_ver for storing review-handling notes

New methods:
 - series_set_source() to mark a series as coming from review
 - review_set_draft_id/sent/replied/deleted() for draft lifecycle
 - review_delete_for_version() for force re-review
 - ser_ver_set_notes() and ser_ver_get_all_notes() for the
   handle-reviews skill

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 tools/patman/database.py     | 155 +++++++++++++++++++++++++++++++++--
 tools/patman/test_cseries.py |   2 +-
 2 files changed, 148 insertions(+), 9 deletions(-)
  

Patch

diff --git a/tools/patman/database.py b/tools/patman/database.py
index 7f33137d0b7..8a96030b68f 100644
--- a/tools/patman/database.py
+++ b/tools/patman/database.py
@@ -20,12 +20,20 @@  from u_boot_pylib import tout
 from patman.series import Series
 
 # Schema version (version 0 means there is no database yet)
-LATEST = 8
+LATEST = 10
 
 # Information about a review record
+# Review status values
+REVIEW_NEW = 'new'          # AI review generated, no draft created
+REVIEW_DRAFT = 'draft'      # Gmail draft created
+REVIEW_SENT = 'sent'        # Draft was sent
+REVIEW_DELETED = 'deleted'  # Draft was deleted without sending
+REVIEW_REPLIED = 'replied'  # Author has replied to our review
+
 Review = namedtuple(
     'REVIEW',
-    'idnum,svid,seq,body,approved,timestamp')
+    'idnum,svid,seq,body,approved,timestamp,draft_id,status,'
+    'gmail_msg_id,gmail_thread_id')
 
 # Information about a series/version record
 SerVer = namedtuple(
@@ -243,6 +251,19 @@  class Database:  # pylint:disable=R0904
             'timestamp TEXT, '
             'FOREIGN KEY (svid) REFERENCES ser_ver (id))')
 
+    def _migrate_to_v9(self):
+        """Add draft tracking, status, and Gmail IDs to review table"""
+        self.cur.execute('ALTER TABLE review ADD COLUMN draft_id')
+        self.cur.execute('ALTER TABLE review ADD COLUMN status')
+        self.cur.execute('ALTER TABLE review ADD COLUMN gmail_msg_id')
+        self.cur.execute('ALTER TABLE review ADD COLUMN gmail_thread_id')
+        self.cur.execute("UPDATE review SET status = 'new'")
+
+    def _migrate_to_v10(self):
+        """Add review notes column to ser_ver table"""
+        self.cur.execute('ALTER TABLE ser_ver ADD COLUMN notes')
+
+    # pylint: disable=R0912
     def migrate_to(self, dest_version):
         """Migrate the database to the selected version
 
@@ -277,6 +298,10 @@  class Database:  # pylint:disable=R0904
                 self._migrate_to_v7()
             elif version == 8:
                 self._migrate_to_v8()
+            elif version == 9:
+                self._migrate_to_v9()
+            elif version == 10:
+                self._migrate_to_v10()
 
             # Save the new version if we have a schema_version table
             if version > 1:
@@ -566,6 +591,17 @@  class Database:  # pylint:disable=R0904
             'UPDATE series SET upstream = ? WHERE id = ?',
             (ups, series_idnum))
 
+    def series_set_source(self, series_idnum, source):
+        """Update the source field for a series
+
+        Args:
+            series_idnum (int): ID num of the series
+            source (str): Source value, e.g. 'review'
+        """
+        self.execute(
+            'UPDATE series SET source = ? WHERE id = ?',
+            (source, series_idnum))
+
     def series_get_null_upstream(self):
         """Get a list of series names that have no upstream set
 
@@ -688,6 +724,32 @@  class Database:  # pylint:disable=R0904
         """
         self.execute('UPDATE ser_ver SET desc = ? WHERE id = ?', (desc, svid))
 
+    def ser_ver_set_notes(self, svid, notes):
+        """Store review-handling notes for a series version
+
+        Args:
+            svid (int): ser_ver ID num
+            notes (str): Notes text from review-notes.txt
+        """
+        self.execute(
+            'UPDATE ser_ver SET notes = ? WHERE id = ?', (notes, svid))
+
+    def ser_ver_get_all_notes(self, series_id):
+        """Get review notes from all versions of a series
+
+        Args:
+            series_id (int): Series ID
+
+        Return:
+            list of tuple: (version, notes) for versions that have notes,
+                ordered by version
+        """
+        res = self.execute(
+            'SELECT version, notes FROM ser_ver '
+            'WHERE series_id = ? AND notes IS NOT NULL '
+            'ORDER BY version', (series_id,))
+        return [(v, n) for v, n in res.fetchall() if n]
+
     def ser_ver_add(self, series_idnum, version, link=None, desc=None):
         """Add a new ser_ver record
 
@@ -864,7 +926,7 @@  class Database:  # pylint:disable=R0904
 
     # upstream functions
 
-    # pylint: disable=R0913
+    # pylint: disable=R0913,R0917
     def upstream_add(self, name, url, patchwork_url=None, identity=None,
                      series_to=None, no_maintainers=False, no_tags=False):
         """Add a new upstream record
@@ -1224,7 +1286,7 @@  class Database:  # pylint:disable=R0904
         res = self.execute(query)
         return res.fetchall()
 
-    # pylint: disable=R0913
+    # pylint: disable=R0913,R0917
     def review_add(self, svid, seq, body, approved, timestamp):
         """Add a review record
 
@@ -1239,11 +1301,68 @@  class Database:  # pylint:disable=R0904
             int: ID num of the new review record
         """
         self.execute(
-            'INSERT INTO review (svid, seq, body, approved, timestamp) '
-            'VALUES (?, ?, ?, ?, ?)',
-            (svid, seq, body, 1 if approved else 0, timestamp))
+            'INSERT INTO review (svid, seq, body, approved, timestamp, '
+            'status) VALUES (?, ?, ?, ?, ?, ?)',
+            (svid, seq, body, 1 if approved else 0, timestamp,
+             REVIEW_NEW))
         return self.lastrowid()
 
+    def review_set_draft_id(self, review_id, draft_id):
+        """Set the Gmail draft ID for a review record
+
+        Args:
+            review_id (int): Review record ID
+            draft_id (str or None): Gmail draft ID
+        """
+        status = REVIEW_DRAFT if draft_id else None
+        self.execute(
+            'UPDATE review SET draft_id = ?, status = ? WHERE id = ?',
+            (draft_id, status, review_id))
+
+    def review_set_sent(self, review_id, body, gmail_msg_id=None,
+                        gmail_thread_id=None):
+        """Mark a review as sent and update its body
+
+        Args:
+            review_id (int): Review record ID
+            body (str): Sent email body text
+            gmail_msg_id (str or None): Gmail message ID of sent email
+            gmail_thread_id (str or None): Gmail thread ID
+        """
+        self.execute(
+            'UPDATE review SET body = ?, draft_id = NULL, status = ?, '
+            'gmail_msg_id = ?, gmail_thread_id = ? WHERE id = ?',
+            (body, REVIEW_SENT, gmail_msg_id, gmail_thread_id,
+             review_id))
+
+    def review_set_replied(self, review_id):
+        """Mark a review as having received a reply
+
+        Args:
+            review_id (int): Review record ID
+        """
+        self.execute(
+            'UPDATE review SET status = ? WHERE id = ?',
+            (REVIEW_REPLIED, review_id))
+
+    def review_set_deleted(self, review_id):
+        """Mark a review draft as deleted (not sent)
+
+        Args:
+            review_id (int): Review record ID
+        """
+        self.execute(
+            'UPDATE review SET draft_id = NULL, status = ? '
+            'WHERE id = ?', (REVIEW_DELETED, review_id))
+
+    def review_delete_for_version(self, svid):
+        """Delete all review records for a given series version
+
+        Args:
+            svid (int): ser_ver ID num
+        """
+        self.execute('DELETE FROM review WHERE svid = ?', (svid,))
+
     def review_get_for_version(self, svid):
         """Get review records for a given series version
 
@@ -1254,10 +1373,30 @@  class Database:  # pylint:disable=R0904
             list of Review: Review records ordered by sequence
         """
         res = self.execute(
-            'SELECT id, svid, seq, body, approved, timestamp '
+            'SELECT id, svid, seq, body, approved, timestamp, draft_id, '
+            'status, gmail_msg_id, gmail_thread_id '
             'FROM review WHERE svid = ? ORDER BY seq', (svid,))
         return [Review(*row) for row in res.fetchall()]
 
+    def review_get_by_status(self, status, need_thread=False):
+        """Get review records with a given status
+
+        Args:
+            status (str): Status to filter by (e.g. 'draft', 'sent')
+            need_thread (bool): If True, only return records with a
+                gmail_thread_id
+
+        Return:
+            list of Review: Matching review records
+        """
+        sql = ('SELECT id, svid, seq, body, approved, timestamp, '
+               'draft_id, status, gmail_msg_id, gmail_thread_id '
+               'FROM review WHERE status = ?')
+        if need_thread:
+            sql += ' AND gmail_thread_id IS NOT NULL'
+        res = self.execute(sql, (status,))
+        return [Review(*row) for row in res.fetchall()]
+
     def review_get_previous(self, series_id, version):
         """Get reviews from the previous version of a series
 
diff --git a/tools/patman/test_cseries.py b/tools/patman/test_cseries.py
index 3b78399d907..e207e8bc173 100644
--- a/tools/patman/test_cseries.py
+++ b/tools/patman/test_cseries.py
@@ -3590,7 +3590,7 @@  Date:   .*
             self.assertEqual(f'Update database to v{version}',
                              out.getvalue().strip())
             self.assertEqual(version, db.get_schema_version())
-        self.assertEqual(8, database.LATEST)
+        self.assertEqual(10, database.LATEST)
 
     def test_migrate_future_version(self):
         """Test that a database newer than patman is rejected"""