Re: [PATCH 3 of 7] comment: add draft flag for comments

[Mads Kiilerich]

On 04/14/2015 06:03 PM, Jan Heylen wrote:
# HG changeset patch
# User Jan Heylen <heyl...@gmail.com>
# Date 1427735588 -7200
#      Mon Mar 30 19:13:08 2015 +0200
# Node ID f48bc03627761dd62f7e7094bb0d322834da336c
# Parent  55ba8e68269da8fb0c33c65711a57b4fd9e1b675
comment: add draft flag for comments
diff -r 55ba8e68269d -r f48bc0362776 kallithea/model/comment.py
--- a/kallithea/model/comment.py        Fri Mar 27 16:13:46 2015 +0100
+++ b/kallithea/model/comment.py        Mon Mar 30 19:13:08 2015 +0200
@@ -165,7 +165,7 @@
  
      def create(self, text, repo, user, revision=None, pull_request=None,
                 f_path=None, line_no=None, status_change=None, 
closing_pr=False,
-               send_email=True):
+               send_email=True, draft=False):
          """
          Creates new comment for changeset or pull request.
          If status_change is not None this comment is associated with a
@@ -194,7 +194,7 @@
          comment.text = text
          comment.f_path = f_path
          comment.line_no = line_no
-        comment.draft = False
+        comment.draft = draft
  
          if revision:
              comment.revision = revision
@@ -207,7 +207,7 @@
          Session().add(comment)
          Session().flush()
  
-        if send_email:
+        if (not draft) and send_email:
              (subj, body, recipients, notification_type,
               email_kwargs) = self._get_notification_data(
                                  repo, comment, user,
@@ -250,7 +250,64 @@
  
          return comment
  
-    def get_comments(self, repo_id, revision=None, pull_request=None):
+    def draft_to_comment(self, repo, draft, user, status_change, closing_pr):
+        draft = self.__get_changeset_comment(draft)
+        draft.draft = False
+        (subj, body, recipients, notification_type,
+         email_kwargs) = self._get_notification_data(
+            repo, draft, user,
+            comment_text=draft.text,
+            line_no=draft.line_no,
+            revision=draft.revision,
+            pull_request=draft.pull_request,
+            status_change=status_change,
+            closing_pr=closing_pr)
+
+        mentions = set(self._extract_mentions(body)).difference(recipients)
+
+        return subj, body, recipients, notification_type, email_kwargs, 
mentions
+
+    def commit_drafts(self, repo_id, user_id, revision=None, 
pull_request=None, status_change=None):
+        repo = self._get_repo(repo_id)
+        user = self._get_user(user_id)
+        bodies = []
+        mention_recipients = set()
+
+        drafts = self.get_inline_drafts(repo_id, revision=revision, 
pull_request=pull_request, user_id=user_id)
+        for __,lines in drafts:
+            for draftlist in lines.values():
+                for draft in draftlist:
+                    (subj, body, recipients, notification_type,
+                     email_kwargs, mentions) = self.draft_to_comment(repo, 
draft, user, None, False)
+                    bodies.append(body)
+                    mention_recipients.update(mentions)
+        gen_drafts = self.get_comments(repo_id, revision=revision, 
pull_request=pull_request, user_id=user_id, draft=True)
+        for draft in gen_drafts:
+            (subj, body, recipients, notification_type,
+             email_kwargs, mentions) = self.draft_to_comment(repo, draft, 
user, status_change, False)
+            bodies.append(body)
+            mention_recipients.update(mentions)
+
+        if email_kwargs:
+            email_kwargs['is_mention'] = False
+            # create notification objects, and emails
+            NotificationModel().create(
+                created_by=user_id, subject=subj, body=None, bodies=bodies,
+                recipients=recipients, type_=notification_type,
+                email_kwargs=email_kwargs)
+
+        if mention_recipients:
+            email_kwargs['is_mention'] = True
+            subj = _('[Mention]') + ' ' + subj
+            NotificationModel().create(
+               created_by=user_id, subject=subj, body=None, bodies=bodies,
+               recipients=mention_recipients,
+               type_=notification_type,
+               email_kwargs=email_kwargs
+            )
+        return drafts

There seems to be a lot of duplication of existing code here. Could we 
start with a refactoring that extract methods for writing/updating a 
comment and for sending mails? At first without considering draftness 
and only used with lists of one.

+
+    def get_comments(self, repo_id, revision=None, pull_request=None, 
user_id=None, draft=False):
          """
          Gets main comments based on revision or pull_request_id
  
@@ -270,6 +327,12 @@
              q = q.filter(ChangesetComment.pull_request == pull_request)
          else:
              raise Exception('Please specify revision or pull_request')
+
+        q = q.filter(ChangesetComment.draft == draft)
+
+        if user_id:
+            q = q.filter(ChangesetComment.user_id == user_id)

I guess this touches upon the question: how private are draft comments? 
Does it make sense to ask for draft comments without specifying a user_id?

+
          q = q.order_by(ChangesetComment.created_on)
          return q.all()
  
@@ -278,6 +341,7 @@
              .filter(ChangesetComment.repo_id == repo_id)\
              .filter(ChangesetComment.line_no != None)\
              .filter(ChangesetComment.f_path != None)\
+            .filter(ChangesetComment.draft == False)\
              .order_by(ChangesetComment.comment_id.asc())\
  
          if revision:
@@ -295,3 +359,31 @@
          for co in comments:
              paths[co.f_path][co.line_no].append(co)
          return paths.items()
+
+
+    def get_inline_drafts(self, repo_id, revision=None, pull_request=None, 
user_id=None):

Docstring, please. What is the intention with this function?

+        q = Session().query(ChangesetComment)\
+            .filter(ChangesetComment.repo_id == repo_id)\
+            .filter(ChangesetComment.line_no != None)\
+            .filter(ChangesetComment.f_path != None)\
+            .filter(ChangesetComment.draft == True)\
+            .order_by(ChangesetComment.comment_id.asc())\
+
+        if revision:
+            q = q.filter(ChangesetComment.revision == revision)
+        elif pull_request:
+            pull_request = self.__get_pull_request(pull_request)
+            q = q.filter(ChangesetComment.pull_request == pull_request)
+        else:
+            raise Exception('Please specify revision or pull_request_id')
+
+        if user_id:
+            q = q.filter(ChangesetComment.user_id == user_id)
+
+        comments = q.all()
+
+        paths = defaultdict(lambda: defaultdict(list))
+
+        for co in comments:
+            paths[co.f_path][co.line_no].append(co)
+        return paths.items()

_______________________________________________
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general