[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 <heyleke at 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()