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/bootstrap.py

@@ -322,6 +322,22 @@ def _assessment_migration() -> bool:
     return True
 
 
+def _linter_email_migration():
+    old_mail_address = 'lms-checks@python.guru'
+    new_mail_address = 'lms-checks@pythonic.guru'
+
+    find_user = models.User.select().where
+    mail_field = models.User.mail_address
+
+    if find_user(mail_field == old_mail_address).exists():
+        user = find_user(mail_field == old_mail_address).get()
+        user.mail_address = new_mail_address
+        user.save()
+        log.info(f'Changed {old_mail_address} to {new_mail_address} in User')
+    else:
+        log.info(f'{new_mail_address} already exists in User')
+
+
 def is_tables_exists(tables: Union[Model, Iterable[Model]]) -> bool:
     if not isinstance(tables, (tuple, list)):
         tables = (tables,)
@@ -351,6 +367,7 @@ def main():
         _uuid_migration()
 
         _add_user_course_constaint()
+        _linter_email_migration()
 
         models.create_basic_roles()
         if models.User.select().count() == 0:

lms/lmsdb/models.py

@@ -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,17 @@ 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),
+            request_user_id=current_user.id,
+            is_manager=current_user.role.is_manager,
+        )
+        return try_or_fail(delete_comment)
 
     if act == "create":
         user = User.get_or_none(User.id == current_user.id)
@@ -560,12 +576,16 @@ def comment():
         return jsonify(
             {
                 "success": "true",
+                "id": comment_.id,
+                "file_id": comment_.file.id,
+                "line_number": comment_.line_number,
                 "text": comment_.comment.text,
-                "is_auto": False,
+                "author_id": user.id,
                 "author_name": user.fullname,
                 "author_role": user.role.id,
-                "id": comment_.id,
-                "line_number": comment_.line_number,
+                "avatar": get_avatar(user.id),
+                "timestamp": comment_.timestamp,
+                "is_auto": False,
             },
         )
 
@@ -821,6 +841,13 @@ 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):
+    # In the meanwhile, support gravatar only.
+    return users.get_gravatar(user_id)
+
+
 @webapp.template_filter("date_humanize")
 def _jinja2_filter_datetime(date):
     try:

lms/models/comments.py

@@ -1,7 +1,6 @@
 from typing import Optional
 
 from flask import request
-from flask_login import current_user  # type: ignore
 from peewee import fn  # type: ignore
 
 from lms.lmsdb.models import (
@@ -52,13 +51,15 @@ def _create_comment(
     )
 
 
-def delete():
-    comment_id = int(request.args.get('commentId'))
+def delete(*, comment_id: int, request_user_id: int, is_manager: bool = False):
+    if not isinstance(comment_id, int):
+        raise NotValidRequest('Invalid comment id.', 400)
+
     comment_ = Comment.get_or_none(Comment.id == comment_id)
-    if (
-        comment_.commenter.id != current_user.id
-        and not current_user.role.is_manager
-    ):
+    if comment_ is None:
+        raise ResourceNotFound('No such comment.', 404)
+
+    if comment_.commenter.id != request_user_id and not is_manager:
         raise ForbiddenPermission(
             "You aren't allowed to access this page.", 403,
         )

lms/models/users.py

@@ -1,13 +1,16 @@
+from functools import cache
+import hashlib
 import re
+from typing import cast
 
-from flask_babel import gettext as _  # type: ignore
+from flask_babel import gettext as _
 from itsdangerous import URLSafeTimedSerializer
 
 from lms.lmsdb.models import Course, User, UserCourse
 from lms.lmsweb import config
 from lms.models.errors import (
-    AlreadyExists, ForbiddenPermission, UnauthorizedError,
-    UnhashedPasswordError,
+    AlreadyExists, ForbiddenPermission, NotValidRequest, ResourceNotFound,
+    UnauthorizedError, UnhashedPasswordError,
 )
 
 
@@ -20,11 +23,22 @@
 )
 
 
+def _to_user_object(user: int | User) -> User:
+    if isinstance(user, int):
+        user = cast(User, User.get_or_none(User.id == user))
+        if user is None:
+            raise ResourceNotFound(_('User not found'), 404)
+
+    if not isinstance(user, User):
+        raise NotValidRequest(_('User is not valid'), 400)
+
+    return user
+
+
 def retrieve_salt(user: User) -> str:
-    password = HASHED_PASSWORD.match(user.password)
-    try:
-        return password.groupdict().get('salt')
-    except AttributeError:  # should never happen
+    if password := HASHED_PASSWORD.match(str(user.password)):
+        return password.groupdict()['salt']
+    else:
         raise UnhashedPasswordError('Password format is invalid.')
 
 
@@ -57,3 +71,11 @@ def join_public_course(course: Course, user: User) -> None:
                 course_name=course.name,
             ), 409,
         )
+
+
+@cache
+def get_gravatar(user: int | User) -> str:
+    user = _to_user_object(user)
+    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'