[PATCH 12 of 14] controllers: align pullrequests.comment with changeset.comment

[Mads Kiilerich]

On 11/24/2018 03:07 PM, Thomas De Schampheleire wrote:
>>>>> @@ -672,26 +676,30 @@ class PullrequestsController(BaseRepoCon
>>>>>             comment = create_comment(
>>>>>                 text,
>>>>>                 status,
>>>>> +            revision=revision,
>>>>>                 pull_request_id=pull_request_id,
>>>>>                 f_path=f_path,
>>>>>                 line_no=line_no,
>>>>>                 closing_pr=close_pr,
>>>>>             )
>>>>>
>>>>> -        action_logger(request.authuser,
>>>>> -                      'user_commented_pull_request:%s' % pull_request_id,
>>>>> -                      c.db_repo, request.ip_addr)
>>>>> -
>>>>>             if status:
>>>>>                 ChangesetStatusModel().set_status(
>>>>>                     c.db_repo.repo_id,
>>>>>                     status,
>>>>>                     request.authuser.user_id,
>>>>>                     comment,
>>>>> -                pull_request=pull_request_id
>>>>> +                revision=revision,
>>>>> +                pull_request=pull_request_id,
>>>>>                 )
>>>>>
>>>>> -        if close_pr:
>>>>> +        if pull_request:
>>>>> +            action = 'user_closed_pull_request:%s' % pull_request_id
>>>> 'user_commented_pull_request' ... so apparently no coverage of this
>>>> logging ... but no big deal ...
>>> Here I'm not sure what you mean: this log string is effectively for
>>> closing the PR so the text seems correct?
>> The check for close_pr comes next and has its own action logger?
> I really don't get you.
> Here is the final code as it is in the changeset controller, basically
> a move from what was in pullrequest after aligning:
>
>      if pull_request:
>          action = 'user_commented_pull_request:%s' % pull_request_id

In the patch above, this was 'user_closed_pull_request' - I fixed it to 
this while landing it.
So it seems like we agree and can move on ;-)

>      else:
>          action = 'user_commented_revision:%s' % revision
>      action_logger(request.authuser, action, c.db_repo, request.ip_addr)
>
>      if pull_request and close_pr:
>          PullRequestModel().close_pull_request(pull_request_id)
>          action_logger(request.authuser,
>                        'user_closed_pull_request:%s' % pull_request_id,
>                        c.db_repo, request.ip_addr)
>
> There are two action logger calls. First is for the commenting part,
> and differentiates revision and pull request.
> Second action_logger call is only there for closing of pull requests,
> and uses the 'user_closed_pull_request' string.
>
> What is not correct here?
>
> Thanks,
> Thomas