Disable coloring of comments icon for comments created by user itself

[fosterfarrell9]

• What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

It has been lamented by some users that the new comment icon is also getting colored when a user itself creates a new comment, and this comment is also listed as unread for this user. This PR changes this behaviour: The new comment icon is no longer colored in this case, and the created comment is not listed as unread.

[codecov]

Codecov Report

Merging #515 (8de7dcd) into mampf-next (dacba16) will decrease coverage by 0.02%.
The diff coverage is n/a.

@@              Coverage Diff               @@
##           mampf-next     #515      +/-   ##
==============================================
- Coverage       66.48%   66.47%   -0.02%     
==============================================
  Files             311      311              
  Lines            9417     9417              
==============================================
- Hits             6261     6260       -1     
- Misses           3156     3157       +1     

see 1 file with indirect coverage changes

[Splines]

This introduces the following bug that you can reproduce as follows:

• Login as teacher and clear your comments list ("Clear all" button).
• Login as student and make a comment
• Login as teacher again and see the comments list. Click on the clear button for that specific comment. The comments icon stays yellow and when you visit another page and then go back to the comments list page, you will see that the comment is there again. You can only get rid of it by clicking the "Clear all" button.

[Splines]

With this new behavior, we now get that the new comments icon is not colored anymore when you've just published a comment (which is great I think). Yet, you still see your own comment in the comments list (/main/comments). Do we we want that? I have no strong preference but tend towards not seeing your own comment in the comments list (since we also disabled the indicator color on the comments icon, so users might get confused why the comments icon is not colored even though the list shows a comment).

[fosterfarrell9]

This introduces the following bug that you can reproduce as follows:
Login as teacher and clear your comments list ("Clear all" button).
Login as student and make a comment
Login as teacher again and see the comments list. Click on the clear button for that specific comment. The comments icon stays yellow and when you visit another page and then go back to the comments list page, you will see that the comment is there again. You can only get rid of it by clicking the "Clear all" button.

I made a change that should take care of the problem. The problem was that multiple instances of the reader model belonging to the same user and thread were created, where at most one of such should exist. In order to see that the fix is working, you should remove existing duplicates in your test database. We should discuss whether adding a uniqueness constraint in the reader model on the thread/user columns makes sense in order to have an additional saftey net. In Rails, this could be achieved via

validates :thread, uniqueness: { scope: user }

in readers.rb and adding an index in the database. (Although, at least the Rails validations are skipped in several instances in the code in readers_controller.rb where we make mass-updates using import and update_all).

Yet, you still see your own comment in the comments list (/main/comments).

I cannot reproduce that (was it related to the bug I introduced?) Note that comments are grouped for media. So, if someone else made a new comment on the same medium, it will show in the list. If you can still reproduce it, I would need more info.

[Splines]

Yet, you still see your own comment in the comments list (/main/comments).

I cannot reproduce that (was it related to the bug I introduced?) Note that comments are grouped for media. So, if someone else made a new comment on the same medium, it will show in the list. If you can still reproduce it, I would need more info.

With a fresh database, I cannot reproduce this issue anymore, it was probably related to the old logic when you created new reader instances.

[Splines]

We should discuss whether adding a uniqueness constraint in the reader model on the thread/user columns makes sense in order to have an additional saftey net.

I think this would be really helpful. It could be done in a new PR where we also update the usage of Rails' persistance methods, so that methods with validations are used.

[fosterfarrell9]

I rewrote the code a little since the last review. In particular I adressed the following issue that occured in the last reviewed version:
If two users A and B who have cleared all comments before wrote a comment at around the same time for the same medium (during that time not seeing whether new comments are popping up), the user who posted their comment last , say B, would get a yellow commentsIcon (because the flag was set by A's comment), but when clicking the icon, no comments would appear (because the thread was marked as read by B's post). In the new version, it is checked whether new comments by other users have been posted. If this is the case, the commentsIcon is colored for B after B hits the Submit button, and the thread appears in the new comments list as unread.
Please check if I did not introduce some new issue by this ;-).

[Splines]

Thanks, I've played around with it and it works, code looks good too.

There is still one little (workflow) issue in the following scenario:
Scenario: Users A and B write comments simultaneously on the same medium. A posts the comment first. Then B posts the comment. Since we haven't implemented something like websockets, person B will only be notified of the comment of person A once B has published his/her comment. Then suddenly, the comments icon is colored for person B. Person B clicks on it and finds that the "Latest post" is by him/herself - which is correct. Still, it seems that the icon was only colored because of B's comment, so person B might just dismiss the comment directly and therefore miss the comment of person A.

In my opinion, this is really an edge case and not easily fixable (and "Latest post" is still correct), so we might just totally ignore it.

[fosterfarrell9]

I think we should ignore this edge case here, since formally, everything is correct. (We might add an explanation of this for users in the docs in #311, though).

[Splines]

Remember to "squash and merge".