comments: record and prevent duplicates

[groovecoder]

Combines https://github.com/mdn/discord/pull/168 and https://github.com/mdn/discord/compare/master...record-comments-97?expand=1

Testing

1. Push this branch up to your test webhook
2. Open a pull request in the repo that uses the test webhook
3. Wait about 30 seconds, refresh, and note the number of comments
4. Close the pull request, reopen it, wait a few seconds, refresh, and note the number of comments

The number of comments should not change between step 3 and step 4. No lines should be double-commented.

Todo

• [ ] Add tests
• [ ] Add in-source comments

[groovecoder]

@openjck - I'm not sure what you mean by "in-source" comments?

[groovecoder]

Did this work to add comments on your PR? I can't get it to post comments to my discord-test repo PR from my discord instance.

I did check the database, and it's storing comments, but the line number is -1 - maybe there's something wrong with the line number logic here?

groovecord2::DATABASE=> select * from "Comments";
 id |           repo           | pr |    filename    | line |          feature          |         createdAt          |         updatedAt
----+--------------------------+----+----------------+------+---------------------------+----------------------------+----------------------------
  1 | groovecoder/discord-test |  4 | transition.css |   -1 | CSS 2.1 selectors         | 2016-01-21 19:44:06.719+00 | 2016-01-21 19:44:06.719+00
  2 | groovecoder/discord-test |  4 | transition.css |   -1 | CSS3 Multiple backgrounds | 2016-01-21 19:44:06.754+00 | 2016-01-21 19:44:06.754+00
  3 | groovecoder/discord-test |  4 | transition.css |   -1 | CSS3 Multiple backgrounds | 2016-01-21 19:44:06.76+00  | 2016-01-21 19:44:06.76+00
  4 | groovecoder/discord-test |  4 | transition.css |   -1 | CSS3 Multiple backgrounds | 2016-01-21 19:44:06.762+00 | 2016-01-21 19:44:06.762+00
(4 rows)

[openjck]

I think Discord reports the wrong line number in very short stylesheets. I'll open a bug about that. In the meantime, do the correct numbers show up when using a bigger file like this one?

[groovecoder]

Looks like it works with the bigger file. I filed https://github.com/mdn/discord/issues/170 for the 1-line bug. I'm going to remove my comment test from this branch and merge it. Can follow up with the tests in a follow-up branch.

[openjck]

Did we also want to move the INSERT to the worker before merging?

[openjck]

If you wanted to remove the test, I can follow up with a commit that moves the INSERT.

[groovecoder]

The branch has now dropped the commit with the test. If you want to add a commit that moves the INSERT go ahead. I've got commits with the tests on my local branch.

[Faeranne]

I find it very intriguing that the same code can both pass and fail at the same time.

[groovecoder]

Updated the code to pass jshint. Yeah I'm not sure why the pr and push builds are different. We'll see how this goes ...

[openjck]

Why was this merged and not #172?

[groovecoder]

I thought this was just the rebased version that combined our commits together? Did I merge the wrong one? :cry:

[openjck]

Yeah, #172 was the follow-up that removed the failing tests. I guess I forgot to close this one in the process.