diff --git a/lms/lmsdb/models.py b/lms/lmsdb/models.py index 3c08359d..df87b034 100644 --- a/lms/lmsdb/models.py +++ b/lms/lmsdb/models.py @@ -74,6 +74,10 @@ def get_student_role(cls) -> 'Role': def get_staff_role(cls) -> 'Role': return cls.get(Role.name == RoleOptions.STAFF.value) + @classmethod + def get_admin_role(cls) -> 'Role': + return cls.get(Role.name == RoleOptions.ADMINISTRATOR.value) + @classmethod def by_name(cls, name) -> 'Role': if name.startswith('_'): @@ -755,6 +759,7 @@ def _by_file(cls, file_id: int): CommentText.id.alias('comment_id'), CommentText.text, SolutionFile.id.alias('file_id'), User.fullname.alias('author_name'), + User.role.alias('author_role'), ] return ( cls diff --git a/lms/lmsweb/config.py.example b/lms/lmsweb/config.py.example index 5c8973d7..4a55f63d 100644 --- a/lms/lmsweb/config.py.example +++ b/lms/lmsweb/config.py.example @@ -29,6 +29,9 @@ ERRORS_CSV = 'errors.csv' # Shareable option SHAREABLE_SOLUTIONS = True +# Users comments option +USERS_COMMENTS = True + # Babel config LANGUAGES = { 'he': 'Hebrew', diff --git a/lms/lmsweb/views.py b/lms/lmsweb/views.py index 46f89990..f6784a47 100644 --- a/lms/lmsweb/views.py +++ b/lms/lmsweb/views.py @@ -19,7 +19,7 @@ from werkzeug.utils import redirect from lms.lmsdb.models import ( - ALL_MODELS, Comment, CommentText, Exercise, RoleOptions, + ALL_MODELS, Comment, CommentText, Exercise, Role, RoleOptions, SharedSolution, Solution, SolutionFile, User, database, ) from lms.lmsweb import babel, routes, webapp @@ -208,8 +208,8 @@ def _create_comment( return jsonify({ 'success': 'true', 'text': comment_.comment.text, - 'author_name': user.fullname, 'is_auto': False, 'id': comment_.id, - 'line_number': line_number, + 'author_name': user.fullname, 'author_role': user.role.id, + 'is_auto': False, 'id': comment_.id, 'line_number': line_number, }) @@ -274,9 +274,20 @@ def comment(): if act == 'fetch': return jsonify(Comment.by_file(file_id)) + if ( + not webapp.config.get('USERS_COMMENTS', False) + and not current_user.role.is_manager + ): + return fail(403, "You aren't allowed to access this page.") + if act == 'delete': comment_id = int(request.args.get('commentId')) comment_ = Comment.get_or_none(Comment.id == comment_id) + if ( + comment_.commenter.id != current_user.id + and not current_user.role.is_manager + ): + return fail(403, "You aren't allowed to access this page.") if comment_ is not None: comment_.delete_instance() return jsonify({'success': 'true'}) @@ -501,9 +512,17 @@ def _common_comments(exercise_id=None, user_id=None): Most common comments throughout all exercises. Filter by exercise id when specified. """ - query = CommentText.filter(**{ - CommentText.flake8_key.name: None, - }).select(CommentText.id, CommentText.text).join(Comment) + is_moderator_comments = ( + (Comment.commenter.role == Role.get_staff_role().id) + | (Comment.commenter.role == Role.get_admin_role().id), + ) + query = ( + CommentText.select(CommentText.id, CommentText.text) + .join(Comment).join(User).join(Role).where( + CommentText.flake8_key.is_null(True), + is_moderator_comments, + ).switch(Comment) + ) if exercise_id is not None: query = ( diff --git a/lms/static/comments.js b/lms/static/comments.js index ce345809..8bd0f662 100644 --- a/lms/static/comments.js +++ b/lms/static/comments.js @@ -27,9 +27,15 @@ function isUserGrader() { return ['staff', 'administrator'].includes(sessionStorage.getItem('role')); } +function isSolverComment(commentData) { + const authorIsSolver = commentData.author_name === sessionStorage.getItem('solver'); + const allowedComment = sessionStorage.getItem('allowedComment') === 'true'; + return (authorIsSolver && allowedComment); +} + function formatCommentData(commentData) { let changedCommentText = `${commentData.author_name}: ${commentData.text}`; - if (isUserGrader()) { + if (isUserGrader() || isSolverComment(commentData)) { const deleteButton = ``; changedCommentText = `${deleteButton} ${changedCommentText}`; } @@ -139,6 +145,8 @@ window.addEventListener('load', () => { window.solutionId = codeElement.id; window.fileId = codeElement.file; sessionStorage.setItem('role', codeElement.role); + sessionStorage.setItem('solver', codeElement.solver); + sessionStorage.setItem('allowedComment', codeElement.allowedComment); addLineSpansToPre(document.getElementsByTagName('code')); pullComments(window.fileId, treatComments); }); diff --git a/lms/templates/view.html b/lms/templates/view.html index d52373b1..6b777e46 100644 --- a/lms/templates/view.html +++ b/lms/templates/view.html @@ -67,7 +67,7 @@

{{ _('תצוגת תרגיל') }} {{ solution['exercise']['id'] }}: {{ soluti -
+
{{- current_file.code | trim(chars=' ') | e -}}
{% if test_results and not shared_url %} @@ -137,8 +137,10 @@

{{ _('הערות בודק') }}

{% if not shared_url %} -{%- if is_manager %} +{%- if is_manager or config.USERS_COMMENTS %} +{% endif -%} +{%- if is_manager %} {% endif -%} {% endif %} diff --git a/lms/tests/conftest.py b/lms/tests/conftest.py index d58d5e12..2679492b 100644 --- a/lms/tests/conftest.py +++ b/lms/tests/conftest.py @@ -56,6 +56,7 @@ def celery_eager(): @pytest.fixture(autouse=True, scope='session') def webapp_configurations(): webapp.config['SHAREABLE_SOLUTIONS'] = True + webapp.config['USERS_COMMENTS'] = True webapp.secret_key = ''.join( random.choices(string.ascii_letters + string.digits, k=64), ) @@ -65,6 +66,14 @@ def disable_shareable_solutions(): webapp.config['SHAREABLE_SOLUTIONS'] = False +def disable_users_comments(): + webapp.config['USERS_COMMENTS'] = False + + +def enable_users_comments(): + webapp.config['USERS_COMMENTS'] = True + + def get_logged_user(username: str) -> FlaskClient: client = webapp.test_client() client.post( diff --git a/lms/tests/test_solutions.py b/lms/tests/test_solutions.py index 1e730da8..2cb2af8e 100644 --- a/lms/tests/test_solutions.py +++ b/lms/tests/test_solutions.py @@ -188,6 +188,111 @@ def test_start_checking( the_solution = Solution.get_by_id(solution.id) assert the_solution.state == Solution.STATES.IN_CHECKING.name + @staticmethod + def test_user_comments( + exercise: Exercise, + student_user: User, + ): + solution = conftest.create_solution(exercise, student_user) + + client = conftest.get_logged_user(student_user.username) + # Creating a comment + comment_response = client.post('/comments', data=json.dumps(dict( + fileId=solution.files[0].id, act='create', kind='text', + comment='hey', line=1, + )), content_type='application/json') + assert comment_response.status_code == 200 + + # Creating another comment + another_comment_response = client.post( + '/comments', data=json.dumps(dict( + fileId=solution.files[0].id, act='create', kind='text', + comment='noice', line=2, + )), content_type='application/json', + ) + assert another_comment_response.status_code == 200 + + # Removing the second comment + json_response_another_comment = json.loads( + another_comment_response.get_data(as_text=True), + ) + delete_response = client.get('/comments', query_string=dict( + fileId=solution.files[0].id, act='delete', + commentId=json_response_another_comment['id'], + ), content_type='application/json') + assert delete_response.status_code == 200 + + # Disabling users comments option + conftest.disable_users_comments() + + # Trying to remove a comment + json_response_comment = json.loads( + comment_response.get_data(as_text=True), + ) + delete_response = client.get('/comments', query_string=dict( + fileId=solution.files[0].id, act='delete', + commentId=json_response_comment['id'], + ), content_type='application/json') + assert delete_response.status_code == 403 + + # Trying to create a comment + disable_comment_response = client.post( + '/comments', data=json.dumps(dict( + fileId=solution.files[0].id, act='create', kind='text', + comment='well well well', line=2, + )), content_type='application/json', + ) + assert disable_comment_response.status_code == 403 + + @staticmethod + def test_staff_and_user_comments( + exercise: Exercise, + student_user: User, + staff_user: User, + ): + solution = conftest.create_solution(exercise, student_user) + + client = conftest.get_logged_user(staff_user.username) + # Enabling user comments option + conftest.enable_users_comments() + # Creating a comment + comment_response = client.post('/comments', data=json.dumps(dict( + fileId=solution.files[0].id, act='create', kind='text', + comment='try again', line=1, + )), content_type='application/json') + assert comment_response.status_code == 200 + + # Creating another comment + another_comment_response = client.post( + '/comments', data=json.dumps(dict( + fileId=solution.files[0].id, act='create', kind='text', + comment='hey', line=1, + )), content_type='application/json', + ) + assert another_comment_response.status_code == 200 + + # Removing the second comment + json_response_another_comment = json.loads( + another_comment_response.get_data(as_text=True), + ) + delete_response = client.get('/comments', query_string=dict( + fileId=solution.files[0].id, act='delete', + commentId=json_response_another_comment['id'], + ), content_type='application/json') + assert delete_response.status_code == 200 + + conftest.logout_user(client) + client2 = conftest.get_logged_user(student_user.username) + # Trying to remove a comment + json_response_comment = json.loads( + comment_response.get_data(as_text=True), + ) + delete_response = client2.get('/comments', query_string=dict( + fileId=solution.files[0].id, act='delete', + commentId=json_response_comment['id'], + ), content_type='application/json') + assert delete_response.status_code == 403 + @staticmethod def test_share_solution_by_another_user( exercise: Exercise,