feat: Better comments section

devops/build.sh

@@ -6,7 +6,7 @@ MAIN_FOLDER="${SCRIPT_FOLDER}/.."
 
 echo "Using sudo to remove the old erlang cookie"
 ERLANG_COOKIE_FILE="${SCRIPT_FOLDER}/rabbitmq.cookie"
-sudo rm -f "$ERLANG_COOKIE_FILE"
+sudo rm -rf "$ERLANG_COOKIE_FILE"
 
 echo "Running build on folder ${MAIN_FOLDER}"
 ( cd "${MAIN_FOLDER}" && docker build -t lms . )

devops/lms.yml

@@ -132,5 +132,5 @@ volumes:
 
 networks:
   lms:
-    external:
-      name: lms
+    external: true
+    name: lms

lms/lmsdb/models.py

@@ -193,8 +193,8 @@ def has_course(self, course_id: int) -> bool:
     @classmethod
     def get_system_user(cls) -> 'User':
         instance, _ = cls.get_or_create(**{
-            cls.mail_address.name: 'linter-checks@python.guru',
-            User.username.name: 'linter-checks@python.guru',
+            cls.mail_address.name: 'linter-checks@pythonic.guru',
+            User.username.name: 'linter-checks@pythonic.guru',
         }, defaults={
             User.fullname.name: 'Checker guru',
             User.role.name: Role.get_staff_role(),
@@ -1096,9 +1096,13 @@ def create_comment(
     @classmethod
     def _by_file(cls, file_id: int):
         fields = (
-            cls.id, cls.line_number, cls.is_auto,
-            CommentText.id.alias('comment_id'), CommentText.text,
+            cls.id,
+            cls.line_number,
+            cls.is_auto,
+            cls.timestamp,
+            CommentText.text,
             SolutionFile.id.alias('file_id'),
+            User.id.alias('author_id'),
             User.fullname.alias('author_name'),
             User.role.alias('author_role'),
         )

lms/lmsweb/views.py

@@ -1,3 +1,4 @@
+from functools import partial
 from typing import Any, Callable, Optional
 
 import arrow  # type: ignore
@@ -522,9 +523,11 @@ def note(user_id: int):
 @webapp.route("/comments", methods=["GET", "POST"])
 @login_required
 def comment():
-    act = request.args.get("act") or request.json.get("act")
+    act = request.args.get("act")
+    if act is None and request.json and request.json.get("act"):
+        act = request.json.get("act")
 
-    if request.method == "POST":
+    if request.method == "POST" and request.json:
         file_id = int(request.json.get("fileId", 0))
     else:  # it's a GET
         file_id = int(request.args.get("fileId", 0))
@@ -538,7 +541,10 @@ def comment():
         return fail(403, "You aren't allowed to access this page.")
 
     if act == "fetch":
-        return jsonify(Comment.by_file(file_id))
+        fetched_comments = Comment.by_file(file_id)
+        for c in fetched_comments:
+            c['avatar'] = get_avatar(c['author_id'])
+        return jsonify(fetched_comments)
 
     if (
         not webapp.config.get("USERS_COMMENTS", False)
@@ -547,7 +553,12 @@ def comment():
         return fail(403, "You aren't allowed to access this page.")
 
     if act == "delete":
-        return try_or_fail(comments.delete)
+        comment_id = request.args.get('commentId')
+        if not comment_id:
+            return fail(400, "No comment id was given.")
+
+        delete_comment = partial(comments.delete, comment_id=int(comment_id))
+        return try_or_fail(delete_comment)
 
     if act == "create":
         user = User.get_or_none(User.id == current_user.id)
@@ -821,6 +832,18 @@ def common_comments(exercise_id=None):
     return jsonify(comments._common_comments(exercise_id=exercise_id))
 
 
+@webapp.route("/user/<int:user_id>/avatar")
+@login_required
+def get_avatar(user_id: int):
+    user = User.get_or_none(User.id == user_id)
+    if user is None:
+        return fail(404, "There is no such user.")
+
+    # In the meanwhile, support gravatar only.
+    gravatar = users.get_gravatar(user)
+    return gravatar
+
+
 @webapp.template_filter("date_humanize")
 def _jinja2_filter_datetime(date):
     try:

lms/models/comments.py

@@ -52,9 +52,14 @@ def _create_comment(
     )
 
 
-def delete():
-    comment_id = int(request.args.get('commentId'))
+def delete(comment_id: int):
+    if not isinstance(comment_id, int):
+        raise NotValidRequest('Invalid comment id.', 400)
+
     comment_ = Comment.get_or_none(Comment.id == comment_id)
+    if comment_ is None:
+        raise ResourceNotFound('No such comment.', 404)
+
     if (
         comment_.commenter.id != current_user.id
         and not current_user.role.is_manager

lms/models/users.py

@@ -1,3 +1,4 @@
+import hashlib
 import re
 
 from flask_babel import gettext as _  # type: ignore
@@ -57,3 +58,12 @@ def join_public_course(course: Course, user: User) -> None:
                 course_name=course.name,
             ), 409,
         )
+
+
+def get_gravatar(user: User) -> str:
+    if not isinstance(user, User):
+        raise ValueError('User is None')
+
+    user_email = str(user.mail_address).strip().lower()
+    gravatar_hash = hashlib.sha256(user_email.encode('utf-8')).hexdigest()
+    return f'https://www.gravatar.com/avatar/{gravatar_hash}?d=404'

lms/static/comments.js

@@ -29,49 +29,64 @@ function isSolverComment(commentData) {
   return (authorIsSolver && allowedComment);
 }
 
-function formatCommentData(commentData) {
-  const commentText = DOMPurify.sanitize(marked.parse(commentData.text));
-  let changedCommentText = `<span class="comment-author">${commentData.author_name}:</span> ${commentText}`;
-  if (window.isUserGrader() || isSolverComment(commentData)) {
-    const deleteButton = `<i class="fa fa-trash grader-delete" aria-hidden="true" data-commentid="${commentData.id}" onclick="deleteComment(${window.fileId}, ${commentData.id});"></i>`;
-    changedCommentText = `${deleteButton} ${changedCommentText}`;
-  }
-  return changedCommentText;
+function createCommentLine(commentData) {
+  const commentLineElement = document.createElement('comment-line');
+  const commentContent = DOMPurify.sanitize(marked.parse(commentData.text));
+
+  commentAttributes = {
+    'data-comment-id': commentData.id,
+    'data-file-id': commentData.file_id,
+    'data-line': commentData.line_number,
+    'data-author-role': commentData.author_role,
+    'avatar': commentData.avatar,
+    'name': commentData.author_name,
+    'date': commentData.timestamp,
+    'editor': window.isUserGrader() || isSolverComment(commentData),
+  }
+
+  for (const [key, value] of Object.entries(commentAttributes)) {
+    commentLineElement.setAttribute(key, value);
+  }
+
+  commentLineElement.innerHTML = `<span class="comment-text">${commentContent}</span>`;
+
+  return commentLineElement;
 }
 
-function addCommentToLine(line, commentData) {
-  const commentElement = document.querySelector(`.line[data-line="${line}"]`);
-  const formattedComment = formatCommentData(commentData);
-  const commentText = `<span class="comment" data-line="${line}" data-commentid="${commentData.id}" data-author-role="${commentData.author_role}">${formattedComment}</span>`;
-  let existingPopover = bootstrap.Popover.getInstance(commentElement);
-  if (existingPopover !== null) {
-    const existingContent = `${existingPopover._config.content} <hr>`;
-    existingPopover._config.content = existingContent + commentText;
-  } else {
-    existingPopover = new bootstrap.Popover(commentElement, {
-      html: true,
-      title: `שורה ${line}`,
-      content: commentText,
-      sanitize: false,
-      boundary: 'viewport',
-      placement: 'auto',
-    });
+function getCommentsContainer(line) {
+  let commentsContainer = document.querySelector(`.comments-container[data-line="${line}"]`);
+  if (commentsContainer !== null) {
+    return commentsContainer;
+  }
 
-    commentElement.addEventListener('shown.bs.popover', function () {
-      Prism.highlightAllUnder(existingPopover.tip);
-    })
+  const lineContainer = document.querySelector(`.line-container[data-line="${line}"]`);
+  commentsContainer = document.createElement('div');
+  commentsContainer.classList.add('comments-container');
+  commentsContainer.setAttribute('data-line', line);
+
+  if (document.documentElement?.dir === 'rtl') {
+    commentsContainer.classList.add('rtl');
   }
 
-  commentElement.dataset.comment = 'true';
-  if ((commentData.is_auto) && (commentElement.dataset.marked !== 'true')) {
-    markLine(commentElement, FLAKE_COMMENTED_LINE_COLOR);
-  } else {
-    const lineColor = window.getLineColorByRole(commentData.author_role);
-    markLine(commentElement, lineColor, true);
-    commentElement.dataset.marked = true;
+  lineContainer.insertAdjacentElement('afterend', commentsContainer);
+  return commentsContainer;
+}
+
+function addCommentToLine(line, commentData) {
+  const commentedLine = document.querySelector(`.line-container[data-line="${line}"]`);
+  if (commentedLine === null) {
+    console.error(`No line found for comment: ${commentData.id}`);
+    return;
   }
 
-  return existingPopover;
+  const commentsContainer = getCommentsContainer(line);
+  const commentLine = createCommentLine(commentData);
+  commentsContainer.appendChild(commentLine);
+  Prism.highlightAllUnder(commentLine);
+
+  commentedLine.dataset.comment = 'true';
+
+  return commentLine;
 }
 
 function getLineColorByRole(authorRole) {
@@ -132,19 +147,107 @@ function addLineSpansToPre(items) {
   const openSpans = [];
   Array.from(items).forEach((item) => {
     const code = item.innerHTML.trim().split('\n');
+    const digits = code.length.toString().length;
     item.innerHTML = code.map(
       (line, i) => {
         let lineContent = openSpans.join('') + line;
         updateOpenedSpans(openSpans, line);
         lineContent += '</span>'.repeat(openSpans.length);
-        const wrappedLine = `<span data-line="${i + 1}" class="line">${lineContent}</span>`;
+        const wrappedLine = `<div class="line-container" data-line="${i + 1}"><span class="line-number" style="width: ${digits}em">${i + 1}</span> <span data-line="${i + 1}" class="line">${lineContent}</span></div>`;
         return wrappedLine;
       },
     ).join('\n');
   });
   window.dispatchEvent(new Event('lines-numbered'));
 }
 
+class LineComment extends HTMLElement {
+  static observedAttributes = [
+    'data-line', 'avatar', 'name', 'date', 'editor', 'data-comment-id', 'data-file-id',
+  ];
+
+  constructor() {
+    super();
+    this.attachShadow({ mode: 'open' });
+    const template = document.getElementById('comment-template').content.cloneNode(true);
+    this.shadowRoot.appendChild(template);
+  }
+
+  connectedCallback() {
+    this.#trackEditButton();
+    this.#trackDeleteButton();
+    this.updateComponent();
+  }
+
+  #trackEditButton() {
+    const editButton = this.shadowRoot.querySelector('.edit-btn');
+    const commentId = this.getAttribute('data-comment-id');
+    editButton.addEventListener('click', () => {
+      window.location.href = `/comments/${commentId}/edit`;
+    });
+  }
+
+  #trackDeleteButton() {
+    const deleteButton = this.shadowRoot.querySelector('.delete-btn');
+
+    const fileId = this.getAttribute('data-file-id');
+    const commentId = this.getAttribute('data-comment-id');
+
+    deleteButton.addEventListener('click', () => {
+      deleteComment(fileId, commentId);
+    });
+  }
+
+  attributeChangedCallback(_, oldValue, newValue) {
+    if (oldValue !== newValue) {
+      this.updateComponent();
+    }
+  }
+
+  updateComponent() {
+    const img = this.shadowRoot.querySelector('.commenter-image');
+    const name = this.shadowRoot.querySelector('.commenter-name');
+    const dateElement = this.shadowRoot.querySelector('.comment-date');
+    const editDeleteBtns = this.shadowRoot.querySelector('.edit-delete-btns');
+
+    img.src = this.getAttribute('avatar') || '/static/avatar.jpg';
+    img.alt = `${this.getAttribute('name')}'s profile picture`;
+
+    name.textContent = this.getAttribute('name');
+
+    const dateString = this.getAttribute('date');
+    dateElement.textContent = this.formatDate(dateString);
+    dateElement.setAttribute('datetime', this.createDatetime(dateString));
+
+    editDeleteBtns.style.display = 'none';
+    if (this.getAttribute('editor') === 'true') {
+      editDeleteBtns.style.display = 'flex';
+    }
+  }
+
+  formatDate(dateString) {
+    if (!dateString) return '';
+    const lang = document.documentElement.lang;
+    const date = new Date(dateString);
+    const options = {
+      year: 'numeric',
+      month: 'numeric',
+      day: 'numeric',
+      hour: '2-digit',
+      minute: '2-digit',
+    }
+    return new Intl.DateTimeFormat(lang, options).format(date);
+  }
+
+  createDatetime(dateString) {
+    const date = new Date(dateString);
+    let year = date.getFullYear();
+    let month = String(date.getMonth() + 1).padStart(2, '0'); // JS months are 0-based
+    let day = String(date.getDate()).padStart(2, '0');
+    return `${year}-${month}-${day}`;
+  }
+}
+
 function configureMarkdownParser() {
   marked.use({
     renderer: {
@@ -156,7 +259,7 @@ function configureMarkdownParser() {
   });
 }
 
-window.markLink = markLine;
+window.markLine = markLine;
 window.hoverLine = hoverLine;
 window.addCommentToLine = addCommentToLine;
 window.getLineColorByRole = getLineColorByRole;
@@ -167,7 +270,9 @@ window.addEventListener('load', () => {
   window.exerciseId = codeElementData.exercise;
   sessionStorage.setItem('role', codeElementData.role);
   sessionStorage.setItem('solver', codeElementData.solver);
+  sessionStorage.setItem('solverId', codeElementData.solverId);
   sessionStorage.setItem('allowedComment', codeElementData.allowedComment);
+  customElements.define('comment-line', LineComment);
   configureMarkdownParser();
   addLineSpansToPre(document.getElementsByTagName('code'));
   pullComments(window.fileId, treatComments);