From 1204de35cb423a6a6c115f11778fe9f5bbc566a4 Mon Sep 17 00:00:00 2001 From: Or Ronai <orronai@gmail.com> Date: Tue, 5 Oct 2021 15:55:53 +0300 Subject: [PATCH 1/6] feat: Add exercises tags - Added tags per course - Added for the exercises.html template the tags - Added the tables of the tags - Added a test --- lms/lmsdb/models.py | 56 ++++++++++++++++++++++++++++++++++-- lms/lmsweb/views.py | 21 ++++++++++---- lms/models/solutions.py | 16 ++++++++++- lms/static/my.css | 18 ++++++++++++ lms/templates/exercises.html | 7 ++++- lms/templates/navbar.html | 2 +- tests/conftest.py | 12 ++++++-- tests/test_exercises.py | 25 ++++++++++++++++ 8 files changed, 143 insertions(+), 14 deletions(-) diff --git a/lms/lmsdb/models.py b/lms/lmsdb/models.py index cf3c8bc1..853c10f1 100644 --- a/lms/lmsdb/models.py +++ b/lms/lmsdb/models.py @@ -346,6 +346,18 @@ def on_notification_saved( instance.delete_instance() +class ExerciseTagText(BaseModel): + text = TextField(unique=True) + + @classmethod + def create_tag(cls, text: str) -> 'ExerciseTagText': + instance, _ = cls.get_or_create(**{cls.text.name: html.escape(text)}) + return instance + + def __str__(self): + return self.text + + class Exercise(BaseModel): subject = CharField() date = DateTimeField() @@ -378,7 +390,7 @@ def is_number_exists(cls, number: int) -> bool: @classmethod def get_objects( cls, user_id: int, fetch_archived: bool = False, - from_all_courses: bool = False, + from_all_courses: bool = False, exercise_tag: Optional[str] = None, ): user = User.get(User.id == user_id) exercises = ( @@ -394,6 +406,13 @@ def get_objects( exercises = exercises.where( UserCourse.course == user.last_course_viewed, ) + if exercise_tag: + exercises = ( + exercises + .join(ExerciseTag) + .join(ExerciseTagText) + .where(ExerciseTagText.text == exercise_tag) + ) if not fetch_archived: exercises = exercises.where(cls.is_archived == False) # NOQA: E712 return exercises @@ -408,6 +427,7 @@ def as_dict(self) -> Dict[str, Any]: 'exercise_number': self.number, 'course_id': self.course.id, 'course_name': self.course.name, + 'tags': ExerciseTag.by_exercise(self), } @staticmethod @@ -426,6 +446,36 @@ def exercise_number_save_handler(model_class, instance, created): instance.number = model_class.get_highest_number() + 1 +class ExerciseTag(BaseModel): + exercise = ForeignKeyField(Exercise) + tag = ForeignKeyField(ExerciseTagText) + date = DateTimeField(default=datetime.now) + + @classmethod + def by_exercise( + cls, exercise: Exercise, + ) -> Union[Iterable['ExerciseTag'], 'ExerciseTag']: + return ( + cls + .select() + .where(cls.exercise == exercise) + .order_by(cls.date) + ) + + @classmethod + def is_course_tag_exists(cls, course: Course, tag_name: str): + return ( + cls + .select() + .join(ExerciseTagText) + .where(ExerciseTagText.text == tag_name) + .switch() + .join(Exercise) + .where(Exercise.course == course) + .exists() + ) + + class SolutionState(enum.Enum): CREATED = 'Created' IN_CHECKING = 'In checking' @@ -561,11 +611,11 @@ def test_results(self) -> Iterable[dict]: @classmethod def of_user( cls, user_id: int, with_archived: bool = False, - from_all_courses: bool = False, + from_all_courses: bool = False, exercise_tag: Optional[str] = None, ) -> Iterable[Dict[str, Any]]: db_exercises = Exercise.get_objects( user_id=user_id, fetch_archived=with_archived, - from_all_courses=from_all_courses, + from_all_courses=from_all_courses, exercise_tag=exercise_tag, ) exercises = Exercise.as_dicts(db_exercises) solutions = ( diff --git a/lms/lmsweb/views.py b/lms/lmsweb/views.py index cc5c9689..31fc0fe9 100644 --- a/lms/lmsweb/views.py +++ b/lms/lmsweb/views.py @@ -15,8 +15,9 @@ from werkzeug.utils import redirect from lms.lmsdb.models import ( - ALL_MODELS, Comment, Course, Note, Role, RoleOptions, SharedSolution, - Solution, SolutionFile, User, UserCourse, database, + ALL_MODELS, Comment, Course, ExerciseTag, ExerciseTagText, Note, Role, + RoleOptions, SharedSolution, Solution, SolutionFile, User, UserCourse, + database, ) from lms.lmsweb import babel, limiter, routes, webapp from lms.lmsweb.admin import ( @@ -343,16 +344,26 @@ def change_last_course_viewed(course_id: int): @webapp.route('/exercises') +@webapp.route('/exercises/<tag_name>') @login_required -def exercises_page(): +def exercises_page(tag_name: Optional[str] = None): fetch_archived = bool(request.args.get('archived')) - exercises = Solution.of_user(current_user.id, fetch_archived) + try: + solutions.check_tag_name(tag_name) + except LmsError as e: + error_message, status_code = e.args + return fail(status_code, error_message) + + exercises = Solution.of_user( + current_user.id, fetch_archived, exercise_tag=tag_name, + ) is_manager = current_user.role.is_manager return render_template( 'exercises.html', exercises=exercises, is_manager=is_manager, fetch_archived=fetch_archived, + tag_name=tag_name, ) @@ -479,7 +490,7 @@ def comment(): @webapp.route('/send/<int:course_id>/<int:_exercise_number>') @login_required -def send(course_id: int, _exercise_number: Optional[int]): +def send(course_id: int, _exercise_number: Optional[int] = None): if not UserCourse.is_user_registered(current_user.id, course_id): return fail(403, "You aren't allowed to watch this page.") return render_template('upload.html', course_id=course_id) diff --git a/lms/models/solutions.py b/lms/models/solutions.py index d61d5e15..de803b1b 100644 --- a/lms/models/solutions.py +++ b/lms/models/solutions.py @@ -8,7 +8,9 @@ from playhouse.shortcuts import model_to_dict # type: ignore from lms.extractors.base import File -from lms.lmsdb.models import SharedSolution, Solution, SolutionFile, User +from lms.lmsdb.models import ( + ExerciseTag, ExerciseTagText, SharedSolution, Solution, SolutionFile, User, +) from lms.lmstests.public.general import tasks as general_tasks from lms.lmstests.public.identical_tests import tasks as identical_tests_tasks from lms.lmsweb import config, routes @@ -205,3 +207,15 @@ def get_files_tree(files: Iterable[SolutionFile]) -> List[Dict[str, Any]]: for file in file_details: del file['fullpath'] return file_details + + +def check_tag_name(tag_name: Optional[str]) -> None: + if ( + tag_name is not None and not ExerciseTag.is_course_tag_exists( + current_user.last_course_viewed, tag_name, + ) + ): + raise ResourceNotFound( + f'No such tag {tag_name} for course ' + f'{current_user.last_course_viewed.name}.', 404, + ) diff --git a/lms/static/my.css b/lms/static/my.css index a637d906..d76d66a5 100644 --- a/lms/static/my.css +++ b/lms/static/my.css @@ -177,6 +177,24 @@ a { line-height: 3rem; } +.exercise-tags { + align-items: center; + display: flex; + font-size: 1em; + height: 3rem; + justify-content: center; + text-align: center; + white-space: normal; +} + +#exercise-tag-link { + color: #919191; +} + +#exercise-tag-link:hover { + color: #646464; +} + .exercise-send { display: flex; flex-direction: row; diff --git a/lms/templates/exercises.html b/lms/templates/exercises.html index 0b08335f..8a0bf292 100644 --- a/lms/templates/exercises.html +++ b/lms/templates/exercises.html @@ -5,7 +5,7 @@ <div id="exercises-page" class="page {{ direction }}"> <div id="exercises-header"> <div id="main-title"> - <h1 id="exercises-head">{{ _('Exercises') }}</h1> + <h1 id="exercises-head">{{ _('Exercises') }}{% if tag_name %} - #{{ tag_name }}{% endif %}</h1> </div> </div> <div id="exercises"> @@ -14,6 +14,11 @@ <h1 id="exercises-head">{{ _('Exercises') }}</h1> <div class="right-side {{ direction }}-language"> <div class="exercise-number me-3">{{ exercise['exercise_number'] }}</div> <div class="exercise-name"><div class="ex-title">{{ exercise['exercise_name'] | e }}</div></div> + <div class="exercise-tags ms-1"> + {% for tag in exercise.tags %} + <a class="ms-1" id="exercise-tag-link" href="{{ url_for('exercises_page', tag_name=tag.tag) }}">#{{ tag.tag }}</a> + {% endfor %} + </div> </div> <div class="left-side"> <div class="comments-count"> diff --git a/lms/templates/navbar.html b/lms/templates/navbar.html index d2e49864..d027c660 100644 --- a/lms/templates/navbar.html +++ b/lms/templates/navbar.html @@ -66,7 +66,7 @@ </a> </li> {% endif -%} - {%- if not exercises or fetch_archived %} + {%- if not exercises or fetch_archived or tag_name %} <li class="nav-item"> <a href="/exercises" class="nav-link"> <i class="fa fa-book" aria-hidden="true"></i> diff --git a/tests/conftest.py b/tests/conftest.py index 2b73bb3f..5b16217d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -15,8 +15,9 @@ import pytest from lms.lmsdb.models import ( - ALL_MODELS, Comment, CommentText, Course, Exercise, Note, Notification, - Role, RoleOptions, SharedSolution, Solution, User, UserCourse, + ALL_MODELS, Comment, CommentText, Course, Exercise, ExerciseTag, + ExerciseTagText, Note, Notification, Role, RoleOptions, SharedSolution, + Solution, User, UserCourse, ) from lms.extractors.base import File from lms.lmstests.public import celery_app as public_app @@ -308,6 +309,11 @@ def create_exercise( ) +def create_exercise_tag(tag_text: str, exercise: Exercise): + new_tag_id = ExerciseTagText.create_tag(text=tag_text).id + return ExerciseTag.create(exercise=exercise, tag=new_tag_id) + + def create_shared_solution(solution: Solution) -> SharedSolution: return SharedSolution.create_new(solution=solution) @@ -315,7 +321,7 @@ def create_shared_solution(solution: Solution) -> SharedSolution: def create_note( creator: User, user: User, - note_text: CommentText, + note_text: str, privacy: int, ): new_note_id = CommentText.create_comment(text=note_text).id diff --git a/tests/test_exercises.py b/tests/test_exercises.py index dc655d48..4658955b 100644 --- a/tests/test_exercises.py +++ b/tests/test_exercises.py @@ -52,3 +52,28 @@ def test_courses_exercises( template, _ = captured_templates[-1] assert template.name == 'exercises.html' assert len(list(Exercise.get_objects(student_user.id))) == 2 + + @staticmethod + def test_exercise_tags( + student_user: User, course: Course, exercise: Exercise, + ): + client = conftest.get_logged_user(username=student_user.username) + conftest.create_usercourse(student_user, course) + client.get(f'course/{course.id}') + conftest.create_exercise_tag('tag1', exercise) + tag_response = client.get('/exercises/tag1') + assert tag_response.status_code == 200 + + course2 = conftest.create_course(index=1) + exercise2 = conftest.create_exercise(course2, 2) + conftest.create_usercourse(student_user, course2) + conftest.create_exercise_tag('tag2', exercise2) + bad_tag_response = client.get('/exercises/tag2') + assert bad_tag_response.status_code == 404 + + client.get(f'course/{course2.id}') + success_tag_response = client.get('/exercises/tag2') + assert success_tag_response.status_code == 200 + + another_bad_tag_response = client.get('/exercises/wrongtag') + assert another_bad_tag_response.status_code == 404 From 865a4f44d828bd25df4833955d74f9923ab9576f Mon Sep 17 00:00:00 2001 From: Or Ronai <orronai@gmail.com> Date: Tue, 5 Oct 2021 16:00:55 +0300 Subject: [PATCH 2/6] removed unused imports --- lms/lmsweb/views.py | 5 ++--- lms/models/solutions.py | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/lms/lmsweb/views.py b/lms/lmsweb/views.py index 31fc0fe9..bea24060 100644 --- a/lms/lmsweb/views.py +++ b/lms/lmsweb/views.py @@ -15,9 +15,8 @@ from werkzeug.utils import redirect from lms.lmsdb.models import ( - ALL_MODELS, Comment, Course, ExerciseTag, ExerciseTagText, Note, Role, - RoleOptions, SharedSolution, Solution, SolutionFile, User, UserCourse, - database, + ALL_MODELS, Comment, Course, Note, Role, RoleOptions, SharedSolution, + Solution, SolutionFile, User, UserCourse, database, ) from lms.lmsweb import babel, limiter, routes, webapp from lms.lmsweb.admin import ( diff --git a/lms/models/solutions.py b/lms/models/solutions.py index de803b1b..f9b57e05 100644 --- a/lms/models/solutions.py +++ b/lms/models/solutions.py @@ -9,7 +9,7 @@ from lms.extractors.base import File from lms.lmsdb.models import ( - ExerciseTag, ExerciseTagText, SharedSolution, Solution, SolutionFile, User, + ExerciseTag, SharedSolution, Solution, SolutionFile, User, ) from lms.lmstests.public.general import tasks as general_tasks from lms.lmstests.public.identical_tests import tasks as identical_tests_tasks From 23d0479d438686a48dee2becce16ff2b4cf9694a Mon Sep 17 00:00:00 2001 From: Or Ronai <orronai@gmail.com> Date: Wed, 6 Oct 2021 11:52:53 +0300 Subject: [PATCH 3/6] - Changed tags color - Added a bridge of tags in models folder - Added course and date_created column to the tags table - Added a constraint to the tags table --- lms/lmsdb/models.py | 55 +++++++++++++++++++++++------------- lms/lmsweb/views.py | 17 +++++++++-- lms/models/solutions.py | 12 +++----- lms/models/tags.py | 33 ++++++++++++++++++++++ lms/static/my.css | 8 ------ lms/templates/exercises.html | 4 +-- tests/conftest.py | 6 ++-- tests/test_exercises.py | 15 ++++++++-- 8 files changed, 106 insertions(+), 44 deletions(-) create mode 100644 lms/models/tags.py diff --git a/lms/lmsdb/models.py b/lms/lmsdb/models.py index 853c10f1..f84d3ef9 100644 --- a/lms/lmsdb/models.py +++ b/lms/lmsdb/models.py @@ -346,12 +346,21 @@ def on_notification_saved( instance.delete_instance() -class ExerciseTagText(BaseModel): - text = TextField(unique=True) +class Tag(BaseModel): + text = TextField() + course = ForeignKeyField(Course) + date_created = DateTimeField(default=datetime.now) + + class Meta: + indexes = ( + (('text', 'course_id'), True), + ) @classmethod - def create_tag(cls, text: str) -> 'ExerciseTagText': - instance, _ = cls.get_or_create(**{cls.text.name: html.escape(text)}) + def create_tag(cls, text: str, course: Course) -> 'Tag': + instance, _ = cls.get_or_create( + **{cls.text.name: html.escape(text), cls.course.name: course}, + ) return instance def __str__(self): @@ -388,12 +397,8 @@ def is_number_exists(cls, number: int) -> bool: return cls.select().where(cls.number == number).exists() @classmethod - def get_objects( - cls, user_id: int, fetch_archived: bool = False, - from_all_courses: bool = False, exercise_tag: Optional[str] = None, - ): - user = User.get(User.id == user_id) - exercises = ( + def by_user(cls, user_id: int): + return ( cls .select() .join(Course) @@ -402,17 +407,20 @@ def get_objects( .switch() .order_by(UserCourse.date, Exercise.number, Exercise.order) ) + + @classmethod + def get_objects( + cls, user_id: int, fetch_archived: bool = False, + from_all_courses: bool = False, exercise_tag: Optional[str] = None, + ): + user = User.get(User.id == user_id) + exercises = cls.by_user(user_id) if not from_all_courses: exercises = exercises.where( UserCourse.course == user.last_course_viewed, ) if exercise_tag: - exercises = ( - exercises - .join(ExerciseTag) - .join(ExerciseTagText) - .where(ExerciseTagText.text == exercise_tag) - ) + exercises = Exercise.by_tags(exercises, exercise_tag) if not fetch_archived: exercises = exercises.where(cls.is_archived == False) # NOQA: E712 return exercises @@ -434,6 +442,15 @@ def as_dict(self) -> Dict[str, Any]: def as_dicts(exercises: Iterable['Exercise']) -> ExercisesDictById: return {exercise.id: exercise.as_dict() for exercise in exercises} + @staticmethod + def by_tags(exercises: Iterable['Exercise'], exercise_tag: str): + return ( + exercises + .join(ExerciseTag) + .join(Tag) + .where(Tag.text == exercise_tag) + ) + def __str__(self): return self.subject @@ -448,7 +465,7 @@ def exercise_number_save_handler(model_class, instance, created): class ExerciseTag(BaseModel): exercise = ForeignKeyField(Exercise) - tag = ForeignKeyField(ExerciseTagText) + tag = ForeignKeyField(Tag) date = DateTimeField(default=datetime.now) @classmethod @@ -467,8 +484,8 @@ def is_course_tag_exists(cls, course: Course, tag_name: str): return ( cls .select() - .join(ExerciseTagText) - .where(ExerciseTagText.text == tag_name) + .join(Tag) + .where(Tag.text == tag_name) .switch() .join(Exercise) .where(Exercise.course == course) diff --git a/lms/lmsweb/views.py b/lms/lmsweb/views.py index bea24060..47c49485 100644 --- a/lms/lmsweb/views.py +++ b/lms/lmsweb/views.py @@ -343,12 +343,25 @@ def change_last_course_viewed(course_id: int): @webapp.route('/exercises') +@login_required +def exercises_page(): + fetch_archived = bool(request.args.get('archived')) + exercises = Solution.of_user(current_user.id, fetch_archived) + is_manager = current_user.role.is_manager + return render_template( + 'exercises.html', + exercises=exercises, + is_manager=is_manager, + fetch_archived=fetch_archived, + ) + + @webapp.route('/exercises/<tag_name>') @login_required -def exercises_page(tag_name: Optional[str] = None): +def exercises_tag_page(tag_name: str): fetch_archived = bool(request.args.get('archived')) try: - solutions.check_tag_name(tag_name) + solutions.check_tag_name(tag_name, current_user.last_course_viewed) except LmsError as e: error_message, status_code = e.args return fail(status_code, error_message) diff --git a/lms/models/solutions.py b/lms/models/solutions.py index f9b57e05..b7c5584e 100644 --- a/lms/models/solutions.py +++ b/lms/models/solutions.py @@ -9,12 +9,12 @@ from lms.extractors.base import File from lms.lmsdb.models import ( - ExerciseTag, SharedSolution, Solution, SolutionFile, User, + Course, SharedSolution, Solution, SolutionFile, User, ) from lms.lmstests.public.general import tasks as general_tasks from lms.lmstests.public.identical_tests import tasks as identical_tests_tasks from lms.lmsweb import config, routes -from lms.models import comments, notifications +from lms.models import comments, notifications, tags from lms.models.errors import ForbiddenPermission, ResourceNotFound from lms.utils.files import ALLOWED_IMAGES_EXTENSIONS @@ -209,12 +209,8 @@ def get_files_tree(files: Iterable[SolutionFile]) -> List[Dict[str, Any]]: return file_details -def check_tag_name(tag_name: Optional[str]) -> None: - if ( - tag_name is not None and not ExerciseTag.is_course_tag_exists( - current_user.last_course_viewed, tag_name, - ) - ): +def check_tag_name(tag_name: str, course: Course) -> None: + if not tags.get_exercises_of(course, tag_name): raise ResourceNotFound( f'No such tag {tag_name} for course ' f'{current_user.last_course_viewed.name}.', 404, diff --git a/lms/models/tags.py b/lms/models/tags.py new file mode 100644 index 00000000..bfe6f78e --- /dev/null +++ b/lms/models/tags.py @@ -0,0 +1,33 @@ +from typing import Iterable, Optional, Union + +from lms.lmsdb.models import Course, Exercise, ExerciseTag, Tag + + +def get_exercises_of( + course: Course, tag_name: str, +) -> Union[Iterable['ExerciseTag'], 'ExerciseTag']: + return ( + ExerciseTag + .select(ExerciseTag.exercise) + .join(Tag) + .where(Tag.text == tag_name, Tag.course == course) + ) + + +def of_exercise( + exercise_id: Optional[int] = None, course: Optional[int] = None, + number: Optional[int] = None, +) -> Optional[Union[Iterable['ExerciseTag'], 'ExerciseTag']]: + if exercise_id is not None: + return ExerciseTag.select().where(ExerciseTag.exercise == id) + elif course is not None: + tags = ( + ExerciseTag + .select() + .join(Exercise) + .where(Exercise.course == course) + ) + if number is not None: + tags = tags.where(Exercise.number == number) + return tags + return None diff --git a/lms/static/my.css b/lms/static/my.css index d76d66a5..8ee3dc8c 100644 --- a/lms/static/my.css +++ b/lms/static/my.css @@ -187,14 +187,6 @@ a { white-space: normal; } -#exercise-tag-link { - color: #919191; -} - -#exercise-tag-link:hover { - color: #646464; -} - .exercise-send { display: flex; flex-direction: row; diff --git a/lms/templates/exercises.html b/lms/templates/exercises.html index 8a0bf292..cff7a9f8 100644 --- a/lms/templates/exercises.html +++ b/lms/templates/exercises.html @@ -5,7 +5,7 @@ <div id="exercises-page" class="page {{ direction }}"> <div id="exercises-header"> <div id="main-title"> - <h1 id="exercises-head">{{ _('Exercises') }}{% if tag_name %} - #{{ tag_name }}{% endif %}</h1> + <h1 id="exercises-head">{{ _('Exercises') }}{% if tag_name %} - #{{ tag_name | e }}{% endif %}</h1> </div> </div> <div id="exercises"> @@ -16,7 +16,7 @@ <h1 id="exercises-head">{{ _('Exercises') }}{% if tag_name %} - #{{ tag_name }}{ <div class="exercise-name"><div class="ex-title">{{ exercise['exercise_name'] | e }}</div></div> <div class="exercise-tags ms-1"> {% for tag in exercise.tags %} - <a class="ms-1" id="exercise-tag-link" href="{{ url_for('exercises_page', tag_name=tag.tag) }}">#{{ tag.tag }}</a> + <a class="ms-1" id="exercise-tag-link" href="{{ url_for('exercises_tag_page', tag_name=tag.tag) }}">#{{ tag.tag | e }}</a> {% endfor %} </div> </div> diff --git a/tests/conftest.py b/tests/conftest.py index 5b16217d..7797d36f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -16,7 +16,7 @@ from lms.lmsdb.models import ( ALL_MODELS, Comment, CommentText, Course, Exercise, ExerciseTag, - ExerciseTagText, Note, Notification, Role, RoleOptions, SharedSolution, + Tag, Note, Notification, Role, RoleOptions, SharedSolution, Solution, User, UserCourse, ) from lms.extractors.base import File @@ -309,8 +309,8 @@ def create_exercise( ) -def create_exercise_tag(tag_text: str, exercise: Exercise): - new_tag_id = ExerciseTagText.create_tag(text=tag_text).id +def create_exercise_tag(tag_text: str, course: Course, exercise: Exercise): + new_tag_id = Tag.create_tag(text=tag_text, course=course).id return ExerciseTag.create(exercise=exercise, tag=new_tag_id) diff --git a/tests/test_exercises.py b/tests/test_exercises.py index 4658955b..20334546 100644 --- a/tests/test_exercises.py +++ b/tests/test_exercises.py @@ -1,6 +1,7 @@ import datetime from lms.lmsdb.models import Course, Exercise, User +from lms.models import tags from tests import conftest @@ -60,14 +61,14 @@ def test_exercise_tags( client = conftest.get_logged_user(username=student_user.username) conftest.create_usercourse(student_user, course) client.get(f'course/{course.id}') - conftest.create_exercise_tag('tag1', exercise) + conftest.create_exercise_tag('tag1', course, exercise) tag_response = client.get('/exercises/tag1') assert tag_response.status_code == 200 course2 = conftest.create_course(index=1) exercise2 = conftest.create_exercise(course2, 2) conftest.create_usercourse(student_user, course2) - conftest.create_exercise_tag('tag2', exercise2) + conftest.create_exercise_tag('tag2', course2, exercise2) bad_tag_response = client.get('/exercises/tag2') assert bad_tag_response.status_code == 404 @@ -77,3 +78,13 @@ def test_exercise_tags( another_bad_tag_response = client.get('/exercises/wrongtag') assert another_bad_tag_response.status_code == 404 + + @staticmethod + def test_course_tags(course: Course, exercise: Exercise): + course2 = conftest.create_course(index=1) + conftest.create_exercise_tag('tag1', course, exercise) + conftest.create_exercise_tag('tag2', course, exercise) + exercise2 = conftest.create_exercise(course, 2) + conftest.create_exercise_tag('tag1', course, exercise2) + assert len(tags.of_exercise(course=course.id)) == 3 + assert len(tags.of_exercise(course=course2.id)) == 0 From 4c74eb21f167a00f6622d68e10ccdc3277ec05c9 Mon Sep 17 00:00:00 2001 From: Or Ronai <orronai@gmail.com> Date: Wed, 6 Oct 2021 11:55:41 +0300 Subject: [PATCH 4/6] Removed unused method --- lms/lmsdb/models.py | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/lms/lmsdb/models.py b/lms/lmsdb/models.py index f84d3ef9..b8e77615 100644 --- a/lms/lmsdb/models.py +++ b/lms/lmsdb/models.py @@ -479,19 +479,6 @@ def by_exercise( .order_by(cls.date) ) - @classmethod - def is_course_tag_exists(cls, course: Course, tag_name: str): - return ( - cls - .select() - .join(Tag) - .where(Tag.text == tag_name) - .switch() - .join(Exercise) - .where(Exercise.course == course) - .exists() - ) - class SolutionState(enum.Enum): CREATED = 'Created' From 200be26e12ac1fc660b7bcc71a87ed53a9742836 Mon Sep 17 00:00:00 2001 From: Or Ronai <orronai@gmail.com> Date: Fri, 8 Oct 2021 16:12:54 +0300 Subject: [PATCH 5/6] Changed into on the /exercises route --- lms/lmsweb/views.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lms/lmsweb/views.py b/lms/lmsweb/views.py index 27cf3836..c41f4f78 100644 --- a/lms/lmsweb/views.py +++ b/lms/lmsweb/views.py @@ -357,20 +357,20 @@ def exercises_page(): ) -@webapp.route('/exercises/<tag_name>') +@webapp.route('/exercises/<tagname>') @login_required -def exercises_tag_page(tag_name: str): +def exercises_tag_page(tagname: str): fetch_archived = bool(request.args.get('archived')) try: solutions.is_tag_name_exists( - tag_name, current_user.last_course_viewed.id, + tagname, current_user.last_course_viewed.id, ) except LmsError as e: error_message, status_code = e.args return fail(status_code, error_message) exercises = Solution.of_user( - current_user.id, fetch_archived, exercise_tag=tag_name, + current_user.id, fetch_archived, exercise_tag=tagname, ) is_manager = current_user.role.is_manager return render_template( @@ -378,7 +378,7 @@ def exercises_tag_page(tag_name: str): exercises=exercises, is_manager=is_manager, fetch_archived=fetch_archived, - tag_name=tag_name, + tag_name=tagname, ) From 07c743c27c86d611e9fbc348d7f9a5a981643248 Mon Sep 17 00:00:00 2001 From: Or Ronai <orronai@gmail.com> Date: Fri, 8 Oct 2021 16:18:38 +0300 Subject: [PATCH 6/6] Fixed route attribute --- lms/templates/exercises.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/templates/exercises.html b/lms/templates/exercises.html index cff7a9f8..28ef8e0d 100644 --- a/lms/templates/exercises.html +++ b/lms/templates/exercises.html @@ -16,7 +16,7 @@ <h1 id="exercises-head">{{ _('Exercises') }}{% if tag_name %} - #{{ tag_name | e <div class="exercise-name"><div class="ex-title">{{ exercise['exercise_name'] | e }}</div></div> <div class="exercise-tags ms-1"> {% for tag in exercise.tags %} - <a class="ms-1" id="exercise-tag-link" href="{{ url_for('exercises_tag_page', tag_name=tag.tag) }}">#{{ tag.tag | e }}</a> + <a class="ms-1" id="exercise-tag-link" href="{{ url_for('exercises_tag_page', tagname=tag.tag) }}">#{{ tag.tag | e }}</a> {% endfor %} </div> </div>