- 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

Author: orronai

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)

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)

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,

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

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;

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>

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)
 
 

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