We ran into a bug where sometimes updating a file attachment would create a

brand new one instead of replacing the original one. This happens because we

rely on an HTML attribute to set the file attachment history ID used for

uploading the attachment, which would sometimes be undefined. The

attribute would be undefined when clicking directly on the file upload icon,

which would cause the event target of the click to be the icon element instead

of the <a> element which holds the attachment history ID attribute. To fix

this, we grab the attachment history ID from the model instead. An alternative

solution would be to use $(e.currentTarget) instead of $(e.target) to grab

the event target, which will always refer to the element that the event

handler is attached to instead of the actual element that was clicked.
I audited the rest of the codebase for any similar problems and updated

some areas where we caught the problem but used more expensive solutions.

Ran JS unit tests.
Tested updating file attachments, uploading new ones, and deleting.
Hit the case where updating a file attachment would create a new one, saw

  that the value from $(e.target).data('attachment-history-id') was

  undefined but the value from model.get('attachmentHistoryID') was correct.
Tested the expand and collapse buttons for the diffviewer and review

  request page.