Flesh out Subscription model methods

Added `select`, `update`, and `delete` methods to `Subscription` to handle all
CRUD operations.

Changed the signatures on `from_attachment`, `from_changeset`, and
`from_comment` to let the `user`, `role`, and `notify` values be overridden.

Added `for_attachment` and `for_changeset` methods to shortcut queries for
those resource types.

Added `attachment_deleted`, `attachment_reparented`, and `changeset_modified`
event handlers.

Author: schwuk

code_comments/subscription.py

@@ -26,20 +26,77 @@ def __init__(self, env, data=None):
             self.__dict__ = data
         self.env = env
 
+    @classmethod
+    def select(cls, env, args={}):
+        select = 'SELECT * FROM code_comments_subscriptions'
+        if len(args) > 0:
+            select += ' WHERE '
+            criteria = []
+            for key, value in args.iteritems():
+                template = '{0}={1}'
+                if isinstance(value, str):
+                    template = '{0}=\'{1}\''
+                criteria.append(template.format(key, value))
+            select += ' AND '.join(criteria)
+        cursor = env.get_read_db().cursor()
+        cursor.execute(select)
+        for row in cursor:
+            yield cls._from_row(env, row)
+
     def insert(self, db=None):
         """
-        Insert a new subscription.
+        Insert a new subscription. Returns bool to indicate success.
+        """
+        if self.id > 0:
+            # Already has an id, don't insert
+            return False
+        else:
+            @self.env.with_transaction()
+            def do_insert(db):
+                cursor = db.cursor()
+                insert = ("INSERT INTO code_comments_subscriptions "
+                          "(user, role, type, path, repos, rev, notify) "
+                          "VALUES (%s, %s, %s, %s, %s, %s, %s)")
+                values = (self.user, self.role, self.type, self.path,
+                          self.repos, self.rev, self.notify)
+                cursor.execute(insert, values)
+                self.id = db.get_last_id(cursor, 'code_comments_subscriptions')
+                return True
+
+    def update(self, db=None):
+        """
+        Update an existing subscription. Returns bool to indicate success.
+        """
+        if self.id == 0:
+            # Doesn't have a valid id, don't update
+            return False
+        else:
+            @self.env.with_transaction()
+            def do_update(db):
+                cursor = db.cursor()
+                update = ("UPDATE code_comments_subscriptions SET "
+                          "user=%s, role=%s, type=%s, path=%s, repos=%s, "
+                          "rev=%s, notify=%s WHERE id=%s")
+                values = (self.user, self.role, self.type, self.path,
+                          self.repos, self.rev, self.notify, self.id)
+                try:
+                    cursor.execute(update, values)
+                except db.IntegrityError:
+                    self.env.log.warning("Subscription update failed.")
+                    return False
+                return True
+
+    def delete(self, db=None):
+        """
+        Delete an existing subscription.
         """
-        @self.env.with_transaction(db)
-        def do_insert(db):
-            cursor = db.cursor()
-            insert = ("INSERT INTO code_comments_subscriptions "
-                      "(user, role, type, path, repos, rev, notify) "
-                      "VALUES (%s, %s, %s, %s, %s, %s, %s)")
-            values = (self.user, self.role, self.type, self.path, self.repos,
-                      self.rev, self.notify)
-            cursor.execute(insert, values)
-            self.id = db.get_last_id(cursor, 'code_comments_subscriptions')
+        if self.id > 0:
+            @self.env.with_transaction()
+            def do_delete(db):
+                cursor = db.cursor()
+                delete = ("DELETE FROM code_comments_subscriptions WHERE "
+                          "id=%s")
+                cursor.execute(delete, (self.id,))
 
     @classmethod
     def _from_row(cls, env, row):
@@ -48,7 +105,7 @@ def _from_row(cls, env, row):
         """
         try:
             subscription = cls(env)
-            subscription.id = row[0]
+            subscription.id = int(row[0])
             subscription.user = row[1]
             subscription.role = row[2]
             subscription.type = row[3]
@@ -87,7 +144,8 @@ def _from_dict(cls, env, dict_):
             return subscription
 
     @classmethod
-    def from_attachment(cls, env, attachment):
+    def from_attachment(cls, env, attachment, user=None, role='author',
+                        notify='always'):
         """
         Creates a subscription from an Attachment object.
         """
@@ -96,42 +154,44 @@ def from_attachment(cls, env, attachment):
                                       attachment.filename)
 
         sub = {
-            'user': attachment.author,
-            'role': 'author',
+            'user': user or attachment.author,
+            'role': role,
             'type': 'attachment',
             'path': _path,
             'repos': '',
             'rev': '',
-            'notify': 'always',
+            'notify': notify,
         }
         return cls._from_dict(env, sub)
 
     @classmethod
-    def from_changeset(cls, env, changeset):
+    def from_changeset(cls, env, changeset, user=None, role='author',
+                       notify='always'):
         """
         Creates a subscription from a Changeset object.
         """
         sub = {
-            'user': changeset.author,
-            'role': 'author',
+            'user': user or changeset.author,
+            'role': role,
             'type': 'changeset',
             'path': '',
             'repos': changeset.repos.reponame,
             'rev': changeset.rev,
-            'notify': 'always',
+            'notify': notify,
         }
         return cls._from_dict(env, sub)
 
     @classmethod
-    def from_comment(cls, env, comment):
+    def from_comment(cls, env, comment, user=None, role='commenter',
+                     notify='always'):
         """
         Creates a subscription from a Comment object.
         """
         sub = {
-            'user': comment.author,
-            'role': 'commenter',
+            'user': user or comment.author,
+            'role': user,
             'type': comment.type,
-            'notify': 'always'
+            'notify': notify,
         }
 
         # Munge attachments
@@ -160,6 +220,34 @@ def from_comment(cls, env, comment):
 
         return cls._from_dict(env, sub)
 
+    @classmethod
+    def for_attachment(cls, env, attachment, path=None):
+        """
+        Returns all subscriptions for an attachment. The path can be
+        overridden.
+        """
+        path_template = "/{0}/{1}/{2}"
+        _path = path or path_template.format(attachment.parent_realm,
+                                             attachment.parent_id,
+                                             attachment.filename)
+        args = {
+            'type': 'attachment',
+            'path': _path,
+        }
+        return cls.select(env, args)
+
+    @classmethod
+    def for_changeset(cls, env, changeset):
+        """
+        Returns all subscriptions for an changeset.
+        """
+        args = {
+            'type': 'changeset',
+            'repos': changeset.repos.reponame,
+            'rev': changeset.rev,
+        }
+        return cls.select(env, args)
+
 
 class SubscriptionAdmin(Component):
     """
@@ -217,11 +305,47 @@ class SubscriptionListeners(Component):
     def attachment_added(self, attachment):
         Subscription.from_attachment(self.env, attachment)
 
+    def attachment_deleted(self, attachment):
+        for subscription in Subscription.for_attachment(self.env, attachment):
+            subscription.delete()
+
+    def attachment_reparented(self, attachment, old_parent_realm,
+                              old_parent_id):
+        path_template = "/{0}/{1}/{2}"
+        old_path = path_template.format(old_parent_realm,
+                                        old_parent_id,
+                                        attachment.filename)
+        new_path = path_template.format(attachment.parent_realm,
+                                        attachment.parent_id,
+                                        attachment.filename)
+
+        for subscription in Subscription.for_attachment(self.env, attachment,
+                                                        old_path):
+            subscription.path = new_path
+            subscription.update()
+
     # IRepositoryChangeListener methods
 
     def changeset_added(self, repos, changeset):
         Subscription.from_changeset(self.env, changeset)
 
+    def changeset_modified(self, repos, changeset, old_changeset):
+        # Handle existing subscriptions
+        if changeset.author != old_changeset.author:
+            for subscription in Subscription.for_changeset(self.env,
+                                                           changeset):
+                if (subscription.user == old_changeset.author and
+                        subscription.role == 'author'):
+                    subscription.role = 'subscriber'
+
+                if subscription.user == changeset.author:
+                    subscription.role = 'author'
+
+                subscription.update()
+
+        # Create a new author subscription
+        Subscription.from_changeset(self.env, changeset)
+
     # ICodeCommentChangeListener methods
 
     def comment_created(self, comment):

todo.md

@@ -24,17 +24,17 @@ Need to figure out how to handle blanket subscriptions.
 
 - [x] model
  - [x] create
- - [ ] update
- - [ ] delete
- - [ ] query
+ - [x] update
+ - [x] delete
+ - [x] query
 - [x] db
 - [ ] api
 
 ## Listeners
 
 - [x] comments_created - create a subscription for the comment author
 - [x] attachment_added - create a subscription for the attachment author
-- [ ] attachment_deleted - remove all subscriptions for the attachment (should we also remove all comments?)
-- [ ] attachment_reparented - update all subscriptions for the attachment
+- [x] attachment_deleted - remove all subscriptions for the attachment (should we also remove all comments?)
+- [x] attachment_reparented - update all subscriptions for the attachment
 - [x] changeset_added - create a subscription for the changeset author
-- [ ] changeset_modified - update the subscription (author?)
+- [x] changeset_modified - update the subscription (author?)