From 8428399941124d43b5b761479932750cf416e8ee Mon Sep 17 00:00:00 2001 From: Or Ronai Date: Sat, 25 Sep 2021 12:32:12 +0300 Subject: [PATCH 1/5] feat: Status of last time viewed solution - Added two columns of status and datetime - Added migration --- lms/lmsdb/bootstrap.py | 18 ++++++++++++++++++ lms/lmsdb/models.py | 32 ++++++++++++++++++++++++++++++++ lms/lmsweb/views.py | 3 +++ 3 files changed, 53 insertions(+) diff --git a/lms/lmsdb/bootstrap.py b/lms/lmsdb/bootstrap.py index 0bc54d0c..ff516537 100644 --- a/lms/lmsdb/bootstrap.py +++ b/lms/lmsdb/bootstrap.py @@ -231,12 +231,29 @@ def _add_api_keys_to_users_table(table: Model, _column: Field) -> None: user.save() +def _add_last_status_view_to_solution_table( + table: Model, _column: Field, +) -> None: + with db_config.database.transaction(): + for solution in table: + solution.last_status_view = models.SolutionStatusView.UPLOADED.name + solution.save() + + def _api_keys_migration() -> bool: User = models.User _add_not_null_column(User, User.api_key, _add_api_keys_to_users_table) return True +def _last_status_view_migration() -> bool: + Solution = models.Solution + _add_not_null_column( + Solution, Solution.last_status_view, _add_last_status_view_to_solution_table, + ) + _migrate_column_in_table_if_needed(Solution, Solution.last_time_view) + + def main(): with models.database.connection_context(): models.database.create_tables(models.ALL_MODELS, safe=True) @@ -247,6 +264,7 @@ def main(): models.create_demo_users() _api_keys_migration() + _last_status_view_migration() text_fixer.fix_texts() import_tests.load_tests_from_path('/app_dir/notebooks-tests') diff --git a/lms/lmsdb/models.py b/lms/lmsdb/models.py index 8277e1a5..d2bddc70 100644 --- a/lms/lmsdb/models.py +++ b/lms/lmsdb/models.py @@ -348,8 +348,20 @@ def to_choices(cls: enum.EnumMeta) -> Tuple[Tuple[str, str], ...]: return tuple((choice.name, choice.value) for choice in choices) +class SolutionStatusView(enum.Enum): + UPLOADED = 'Uploaded' + NOT_CHECKED = 'Not checked' + CHECKED = 'Checked' + + @classmethod + def to_choices(cls: enum.EnumMeta) -> Tuple[Tuple[str, str], ...]: + choices = cast(Iterable[enum.Enum], tuple(cls)) + return tuple((choice.name, choice.value) for choice in choices) + + class Solution(BaseModel): STATES = SolutionState + STATUS_VIEW = SolutionStatusView MAX_CHECK_TIME_SECONDS = 60 * 10 exercise = ForeignKeyField(Exercise, backref='solutions') @@ -365,6 +377,12 @@ class Solution(BaseModel): ) submission_timestamp = DateTimeField(index=True) hashed = TextField() + last_status_view = CharField( + choices=STATUS_VIEW.to_choices(), + default=STATUS_VIEW.UPLOADED.name, + index=True, + ) + last_time_view = DateTimeField(default=datetime.now, null=True, index=True) @property def solution_files( @@ -406,6 +424,20 @@ def is_duplicate( return last_submission_hash == hash_ + def view_solution(self) -> None: + self.last_time_view = datetime.now() + if ( + self.last_status_view != self.STATUS_VIEW.NOT_CHECKED.name + and self.state == self.STATES.CREATED.name + ): + self.last_status_view = self.STATUS_VIEW.NOT_CHECKED.name + elif ( + self.last_status_view != self.STATUS_VIEW.CHECKED.name + and self.state == self.STATES.DONE.name + ): + self.last_status_view = self.STATUS_VIEW.CHECKED.name + self.save() + def start_checking(self) -> bool: return self.set_state(Solution.STATES.IN_CHECKING) diff --git a/lms/lmsweb/views.py b/lms/lmsweb/views.py index eecc397d..6281d9d0 100644 --- a/lms/lmsweb/views.py +++ b/lms/lmsweb/views.py @@ -496,6 +496,9 @@ def view( error_message, status_code = e.args return fail(status_code, error_message) + if viewer_is_solver: + solution.view_solution() + return render_template('view.html', **view_params) From 817cdd30b5c03944a921a12b596251610563d6ad Mon Sep 17 00:00:00 2001 From: Or Ronai Date: Sat, 25 Sep 2021 14:01:04 +0300 Subject: [PATCH 2/5] Added migration and a test --- lms/lmsdb/bootstrap.py | 18 +++++------------- tests/test_solutions.py | 24 +++++++++++++++++++++++- 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/lms/lmsdb/bootstrap.py b/lms/lmsdb/bootstrap.py index ff516537..8d3c6b2c 100644 --- a/lms/lmsdb/bootstrap.py +++ b/lms/lmsdb/bootstrap.py @@ -231,15 +231,6 @@ def _add_api_keys_to_users_table(table: Model, _column: Field) -> None: user.save() -def _add_last_status_view_to_solution_table( - table: Model, _column: Field, -) -> None: - with db_config.database.transaction(): - for solution in table: - solution.last_status_view = models.SolutionStatusView.UPLOADED.name - solution.save() - - def _api_keys_migration() -> bool: User = models.User _add_not_null_column(User, User.api_key, _add_api_keys_to_users_table) @@ -248,14 +239,16 @@ def _api_keys_migration() -> bool: def _last_status_view_migration() -> bool: Solution = models.Solution - _add_not_null_column( - Solution, Solution.last_status_view, _add_last_status_view_to_solution_table, - ) + _migrate_column_in_table_if_needed(Solution, Solution.last_status_view) _migrate_column_in_table_if_needed(Solution, Solution.last_time_view) + return True def main(): with models.database.connection_context(): + if models.database.table_exists(models.Solution.__name__.lower()): + _last_status_view_migration() + models.database.create_tables(models.ALL_MODELS, safe=True) if models.Role.select().count() == 0: @@ -264,7 +257,6 @@ def main(): models.create_demo_users() _api_keys_migration() - _last_status_view_migration() text_fixer.fix_texts() import_tests.load_tests_from_path('/app_dir/notebooks-tests') diff --git a/tests/test_solutions.py b/tests/test_solutions.py index 67b8c2e5..68cf80f1 100644 --- a/tests/test_solutions.py +++ b/tests/test_solutions.py @@ -5,7 +5,9 @@ from flask import json import pytest -from lms.lmsdb.models import Comment, Exercise, SharedSolution, Solution, User +from lms.lmsdb.models import ( + Comment, Exercise, SharedSolution, Solution, SolutionStatusView, User, +) from lms.lmstests.public.general import tasks as general_tasks from lms.lmsweb import routes from lms.models import notifications, solutions @@ -499,3 +501,23 @@ def test_manager_can_see_solutions( staff_client = conftest.get_logged_user(staff_user.username) view_response = staff_client.get(f'{routes.SOLUTIONS}/{solution.id}') assert view_response.status_code == 200 + + @staticmethod + def test_last_view_status( + solution: Solution, + student_user: User, + staff_user: User, + ): + client = conftest.get_logged_user(student_user.username) + assert solution.last_status_view == SolutionStatusView.UPLOADED.name + + client.get(f'/view/{solution.id}') + solution = Solution.get_by_id(solution.id) + assert solution.last_status_view == SolutionStatusView.NOT_CHECKED.name + + solutions.mark_as_checked(solution.id, staff_user.id) + solution = Solution.get_by_id(solution.id) + assert solution.last_status_view == SolutionStatusView.NOT_CHECKED.name + client.get(f'/view/{solution.id}') + solution = Solution.get_by_id(solution.id) + assert solution.last_status_view == SolutionStatusView.CHECKED.name From d24d82a1cd581e51f6249acc0d75c285f5e2cc8e Mon Sep 17 00:00:00 2001 From: Or Ronai Date: Sat, 25 Sep 2021 14:03:36 +0300 Subject: [PATCH 3/5] fixed a condition --- lms/lmsdb/bootstrap.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lms/lmsdb/bootstrap.py b/lms/lmsdb/bootstrap.py index 8d3c6b2c..3e0fdde5 100644 --- a/lms/lmsdb/bootstrap.py +++ b/lms/lmsdb/bootstrap.py @@ -218,9 +218,7 @@ def _drop_constraint_if_needed(table: Type[Model], column_name: str) -> bool: def has_column_named(table: Model, column_name: str) -> bool: db = db_config.database columns = {col.name for col in db.get_columns(table.__name__.lower())} - if column_name not in columns: - return False - return True + return column_name in columns def _add_api_keys_to_users_table(table: Model, _column: Field) -> None: From 8595941984053b06ecc13472b7c94ae1d39f5414 Mon Sep 17 00:00:00 2001 From: Or Ronai Date: Sat, 25 Sep 2021 14:17:35 +0300 Subject: [PATCH 4/5] Added a test --- tests/test_solutions.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/test_solutions.py b/tests/test_solutions.py index 68cf80f1..9b04f883 100644 --- a/tests/test_solutions.py +++ b/tests/test_solutions.py @@ -521,3 +521,16 @@ def test_last_view_status( client.get(f'/view/{solution.id}') solution = Solution.get_by_id(solution.id) assert solution.last_status_view == SolutionStatusView.CHECKED.name + + @staticmethod + def test_invalid_file_solution( + solution: Solution, + student_user: User, + ): + client = conftest.get_logged_user(student_user.username) + successful_response = client.get(f'/view/{solution.id}') + assert successful_response.status_code == 200 + + fail_response = client.get(f'/view/{solution.id}/12345') + assert fail_response.status_code == 404 + From 032dc7ecf6215da8c17e6e3476b8984770fa9341 Mon Sep 17 00:00:00 2001 From: Or Ronai Date: Sun, 26 Sep 2021 16:01:14 +0300 Subject: [PATCH 5/5] Cleaner view of login page --- lms/static/my.css | 5 +++++ lms/templates/login.html | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/lms/static/my.css b/lms/static/my.css index 886ed538..f3acc0aa 100644 --- a/lms/static/my.css +++ b/lms/static/my.css @@ -81,6 +81,11 @@ a { color: #860606; } +#forgot-my-password-link { + display: block; + margin: 0.5rem; +} + .page { margin: 3rem 0; } diff --git a/lms/templates/login.html b/lms/templates/login.html index f76945de..20dd92a6 100644 --- a/lms/templates/login.html +++ b/lms/templates/login.html @@ -33,8 +33,8 @@

{{ _('התחברות') }}

- {{ _('שכחת את הסיסמה?') }} + {{ _('שכחת את הסיסמה?') }} {% if config.REGISTRATION_OPEN %}
{{ _('הירשם') }}