[PATCH] backout "pullrequests: don't add automatic 'status change' message - it will be added in template"

[Thomas De Schampheleire]

On Wed, Jun 17, 2015 at 7:40 AM, Jan Heylen <heyleke at gmail.com> wrote:
> What I also wanted to say is that the original change is most probably
> not the issue, it just triggers an underlying issue...
>
> What I observed:
> our kallithea repo:
> * the issue is still seen on the tip of the default branch
> * the issue is not seen on the stable branch (as this commit is not on
> the stable branch)
> inside kallithea:
> * the issue is only seen on commits that are in a (freshly created) pull request
> * the issue is only seen when those commits don't have any other
> comments given yet
>
> Btw, that is why I still believe we need to backout the breaking
> change, any commit that is in a fresh created pull request without any
> comments on it, cannot be accessed.
>
> My 5 cents in solving it:
> * It is not the first time I was thinking this: a for loop over a an
> object that is a NoneType should not go wrong in this way. I think
> this is wrong in the mako runtime. I had the same issue in the email
> templates. But I could be wrong? A simple workaround though is to
> place an extra %if statement to check for NoneType...
> * The actual question is, how is c.comments a noneType in this case,
> and not when there is no review request? That could answer the above
> question, I am wrong and c.comments is created because a PR is open,
> but 0 comments are returned, so, the first entry on c.comments stays a
> NoneType, that would be an issue in the code that creates c.comments.
> But still, the for loop should not loop over an empty array?

I don't think the array is empty, but rather it contains an element
that is None.

%for co in c.comments:
   <div id="comment-tr-${co.comment_id}">
   ...

c.comments has at least one element that is None, placed in co. Later,
co.comment_id resolves to None.comment_id, causing the error you see.

So I don't think that mako is wrong (but I haven't verified this
theoretical analysis).
The question is: why does 'co.comments' contain a None element?

/Thomas