Merge request interface becomes confused when emailed reply includes a patch

[Steve Beattie]

Please see https://code.launchpad.net/~pfsmorigo/ubuntu-cve-tracker/+git/ubuntu-cve-tracker/+merge/448821 for an example of this.

The launchpad merge request interface becomes confused when an emailed reply to the merge request includes a patch as an attachment. In the example request above, Emi Torino's comments were made against the preview diff. However, when I sent a response that included an attached patch that I felt should be included as part of the merge, the interface correctly identified that it was a patch/diff and tried to treat it as such, but then adjusted Emi's inline comments to make them appear that she was commenting on the emailed patch. The emailed reply was not in response to her comments, but to the initial merge request email (though the response was sent after the inline comments were made, and the associated email for them was received).

[Ines Almeida (ines-almeida)]

Interesting, thanks for reporting.

My current understanding is that when we render the patch file, we render it similarly to the diff section. This includes adding items with id=`diff-line-<line number>`, which conflicts with the ids of the real diff items.

When we then try to render the inline-comments, we then get the item with id `diff-line-<line>` but that returns the patch item instead of the actual diff item. See `lib/lp/code/javascript/branchmergeproposal.inlinecomments.js` for where inline-comments a rendered: `Y.one('#diff-line-' + comment.line_number).insert(comments_tr, 'after');`

To fix this, we would need to rework how the patch files are rendered so they don't use the same item IDs as the real diff.
See `lib/lp/code/templates/branchmergeproposal-diff.pt`.