- Added some tests

- Changed the exercise number pre_save
- Removed unique from assessments oreder
- Assessments are now per course

Author: orronai

lms/lmsdb/models.py

@@ -25,7 +25,9 @@
 from lms.models.errors import AlreadyExists
 from lms.utils import hashing
 from lms.utils.colors import get_hex_color
-from lms.utils.consts import DEFAULT_ACTIVE_COLOR, DEFAULT_COLOR
+from lms.utils.consts import (
+    DEFAULT_ASSESSMENT_BUTTON_ACTIVE_COLOR, DEFAULT_ASSESSMENT_BUTTON_COLOR,
+)
 from lms.utils.log import log
 
 
@@ -370,12 +372,24 @@ def open_for_new_solutions(self) -> bool:
         return datetime.now() < self.due_date and not self.is_archived
 
     @classmethod
-    def get_highest_number(cls):
-        return cls.select(fn.MAX(cls.number)).scalar()
+    def get_highest_number(cls, course: Course):
+        return (
+            cls
+            .select(cls.number)
+            .where(cls.course == course)
+            .order_by(cls.number.desc())
+            .limit(1)
+            .scalar()
+        )
 
     @classmethod
-    def is_number_exists(cls, number: int) -> bool:
-        return cls.select().where(cls.number == number).exists()
+    def is_number_exists(cls, course: Course, number: int) -> bool:
+        return (
+            cls
+            .select()
+            .where(cls.course == course, cls.number == number)
+            .exists()
+        )
 
     @classmethod
     def get_objects(
@@ -424,8 +438,8 @@ def __str__(self):
 def exercise_number_save_handler(model_class, instance, created):
     """Change the exercise number to the highest consecutive number."""
 
-    if model_class.is_number_exists(instance.number):
-        instance.number = model_class.get_highest_number() + 1
+    if model_class.is_number_exists(instance.course, instance.number):
+        instance.number = model_class.get_highest_number(instance.course) + 1
 
 
 class SolutionState(enum.Enum):
@@ -464,11 +478,12 @@ class SolutionAssessment(BaseModel):
     icon = CharField(null=True)
     color = CharField()
     active_color = CharField()
-    order = IntegerField(default=0, index=True, unique=True)
+    order = IntegerField(default=0, index=True)
+    course = ForeignKeyField(Course, backref='assessments')
 
     @classmethod
-    def get_assessments(cls):
-        return cls.select().order_by(cls.order)
+    def get_assessments(cls, course: Course):
+        return cls.select().where(cls.course == course).order_by(cls.order)
 
     def __str__(self):
         return self.name
@@ -481,12 +496,12 @@ def assessment_on_save_handler(_model_class, instance, created):
     try:
         instance.color = get_hex_color(instance.color)
     except ValueError:
-        instance.color = DEFAULT_COLOR
+        instance.color = DEFAULT_ASSESSMENT_BUTTON_COLOR
 
     try:
         instance.active_color = get_hex_color(instance.active_color)
     except ValueError:
-        instance.active_color = DEFAULT_ACTIVE_COLOR
+        instance.active_color = DEFAULT_ASSESSMENT_BUTTON_ACTIVE_COLOR
 
 
 class Solution(BaseModel):
@@ -1121,11 +1136,13 @@ def create_basic_assessments() -> None:
             'color': 'black', 'icon': 'exclamation-triangle', 'order': 4,
         },
     }
-    for name, values in assessments_dict.items():
-        SolutionAssessment.create(
-            name=name, icon=values.get('icon'), color=values.get('color'),
-            active_color='white', order=values.get('order'),
-        )
+    courses = Course.select()
+    for course in courses:
+        for name, values in assessments_dict.items():
+            SolutionAssessment.create(
+                name=name, icon=values.get('icon'), color=values.get('color'),
+                active_color='white', order=values.get('order'), course=course,
+            )
 
 
 def create_basic_course() -> Course:

lms/models/solutions.py

@@ -142,7 +142,8 @@ def get_view_parameters(
             'user_comments':
                 comments._common_comments(user_id=current_user.id),
             'left': Solution.left_in_exercise(solution.exercise),
-            'assessments': SolutionAssessment.get_assessments(),
+            'assessments':
+                SolutionAssessment.get_assessments(solution.exercise.course),
         }
 
     if viewer_is_solver:

lms/templates/user.html

@@ -31,7 +31,7 @@ <h2>{{ _('Exercises Submitted') }}:</h2>
               <th scope="col">{{ _('Submission status') }}</th>
               <th scope="col">{{ _('Submission') }}</th>
               <th scope="col">{{ _('Checker') }}</th>
-              <th scope="col">{{ _('Verbal note') }}</th>
+              <th scope="col">{{ _('Assessment') }}</th>
             </tr>
           </thead>
           <tbody>

lms/templates/view.html

@@ -15,10 +15,18 @@ <h1>{{ _('Exercise view') }} {{ solution['exercise']['id'] }}: {{ solution['exer
         <p class="unchecked-msg {{ direction }}"><strong>{{ _("Your solution hasn't been checked.") }}</strong> {{ _("It's important for us that all exercises will be checked by human eye.") }}</p>
       {% endif %}
     {% else %}
-      <p id="solver"><strong>{{ _('Solver') }}:</strong> <a href="/user/{{ solution['solver']['id'] }}">{{ solution['solver']['fullname'] | e }}</a></p>
+      <p id="solver">
+        <strong>{{ _('Solver') }}: </strong>
+        <a href="/user/{{ solution['solver']['id'] }}">{{ solution['solver']['fullname'] | e }}</a>
+      </p>
     {% endif %}
     {% if solution.assessment and not shared_url %}
-      <p id="assessment"><strong>{{ _('Verbal note') }}:</strong>{% if solution.assessment.icon %} <i class="fa fa-{{ solution.assessment.icon | e }}"></i>{% endif %} {{ solution.assessment.name | e }}</p>
+      <p id="assessment">
+        <strong>{{ _('Assessment') }}: </strong>
+        {% if solution.assessment.icon %}
+        <i class="fa fa-{{ solution.assessment.icon | e }}"> </i>
+        {% endif %}{{ solution.assessment.name | e }}
+      </p>
     {% endif %}
     {% if not shared_url %}
     <nav id="versions" aria-label="{{ _('Navigate in solution versions') }}">
@@ -101,11 +109,16 @@ <h5 class="test-name">
     {% if is_manager and not shared_url %}
     <div id="popular-comments">
       <div id="exercise-assessment" class="{{ direction }}">
-        <h2>{{ _('Verbal note') }}</h2>
+        <h2>{{ _('Assessment') }}</h2>
         <div class="btn-group-vertical" id="solution-assessment" role="group" aria-label="assessments radio toggle button group">
           {% for assessment in assessments %}
-            <input type="radio" class="btn-check" name="assessment" id="btnradio{{ loop.index }}" autocomplete="off" value="{{ assessment.id }}" {%- if solution.assessment.name == assessment.name or loop.first %}checked{% endif -%}>
-            <label class="btn btn-outline" style="--color: {{ assessment.color }}; --active-color: {{ assessment.active_color }};" for="btnradio{{ loop.index }}">{% if assessment.icon %}<i class="fa fa-{{ assessment.icon }}"></i> {% endif %}{{ assessment.name }}</label>
+            <input type="radio" class="btn-check" name="assessment" id="btnradio{{ loop.index }}" autocomplete="off" value="{{ assessment.id }}"{% if solution.assessment.name == assessment.name or loop.first %} checked{% endif %}>
+            <label class="btn btn-outline" style="--color: {{ assessment.color }}; --active-color: {{ assessment.active_color }};" for="btnradio{{ loop.index }}">
+              {% if assessment.icon %}
+                <i class="fa fa-{{ assessment.icon }}"> </i>
+              {% endif %}
+              {{ assessment.name }}
+            </label>
           {% endfor %}
         </div>
       </div>

lms/utils/colors.py

@@ -4,12 +4,12 @@
 from lms.utils.consts import COLORS
 
 
-HEX_COLOR = re.compile(r'(?P<hex>#[a-f0-9]{6}|#[a-f0-9]{3})')
+HEX_COLOR = re.compile(r'#?(?P<hex>[a-f0-9]{6}|[a-f0-9]{3})')
 
 
 def get_hex_color(number: str) -> Optional[str]:
     if color := HEX_COLOR.match(number):
-        return color.groupdict()['hex']
+        return '#' + color.groupdict()['hex']
     elif color := COLORS.get(number):
         return color
     raise ValueError('This is not a valid hex color')

lms/utils/consts.py

@@ -13,5 +13,5 @@
     'teal': '#20c997', 'orange': '#fd7e14', 'pink': '#d63384',
     'purple': '#6f42c1', 'indigo': '#6610f2', 'light': '#f8f9fa',
 }
-DEFAULT_COLOR = '#0d6efd'  # primary
-DEFAULT_ACTIVE_COLOR = '#fff'  # white
+DEFAULT_ASSESSMENT_BUTTON_COLOR = '#0d6efd'  # primary
+DEFAULT_ASSESSMENT_BUTTON_ACTIVE_COLOR = '#fff'  # white

tests/conftest.py

@@ -46,23 +46,6 @@ def populate_roles():
         Role.create(name=role.value)
 
 
-@pytest.fixture(autouse=True, scope='session')
-def populate_assessments():
-    assessments_dict = {
-        'Excellent': {'color': 'green', 'icon': 'star', 'order': 1},
-        'Nice': {'color': 'blue', 'icon': 'check', 'order': 2},
-        'Try again': {'color': 'red', 'icon': 'exclamation', 'order': 3},
-        'Plagiarism': {
-            'color': 'black', 'icon': 'exclamation-triangle', 'order': 4,
-        },
-    }
-    for name, values in assessments_dict.items():
-        SolutionAssessment.create(
-            name=name, icon=values.get('icon'), color=values.get('color'),
-            active_color='white', order=values.get('order'),
-        )
-
-
 @pytest.fixture(autouse=True, scope='function')
 def db(db_in_memory):
     """Rollback all operations between each test-case"""
@@ -352,6 +335,23 @@ def course() -> Course:
     return create_course()
 
 
+@pytest.fixture()
+def _assessments(course: Course) -> None:
+    assessments_dict = {
+        'Excellent': {'color': 'green', 'icon': 'star', 'order': 1},
+        'Nice': {'color': 'blue', 'icon': 'check', 'order': 2},
+        'Try again': {'color': 'red', 'icon': 'exclamation', 'order': 3},
+        'Plagiarism': {
+            'color': 'black', 'icon': 'exclamation-triangle', 'order': 4,
+        },
+    }
+    for name, values in assessments_dict.items():
+        SolutionAssessment.create(
+            name=name, icon=values.get('icon'), color=values.get('color'),
+            active_color='white', order=values.get('order'), course=course,
+        )
+
+
 @pytest.fixture()
 def exercise(course: Course) -> Exercise:
     return create_exercise(course, 1)

tests/test_solutions.py

@@ -8,13 +8,14 @@
 from lms.lmsdb import models
 from lms.lmsdb.models import (
     Comment, Course, Exercise, SharedSolution,
-    Solution, SolutionStatusView, User,
+    Solution, SolutionAssessment, SolutionStatusView, User,
 )
 from lms.lmstests.public.general import tasks as general_tasks
 from lms.lmsweb import routes
 from lms.models import notifications, solutions
 from lms.models.errors import ResourceNotFound
 from lms.models.solutions import get_view_parameters
+from lms.utils.consts import COLORS, DEFAULT_ASSESSMENT_BUTTON_ACTIVE_COLOR, DEFAULT_ASSESSMENT_BUTTON_COLOR
 from tests import conftest
 
 
@@ -591,3 +592,38 @@ def test_done_checking(
             content_type='application/json',
         )
         assert response.status_code == 200
+
+    @staticmethod
+    def test_solution_assessment_color_on_create(course: Course, _assessments):
+        assessment1 = SolutionAssessment.create(
+            name='Ok', color='red', active_color='lmnop',
+            order=5, course=course,
+        )
+        assert assessment1.color == COLORS.get('red')
+        assert (
+            assessment1.active_color == DEFAULT_ASSESSMENT_BUTTON_ACTIVE_COLOR,
+        )
+
+        assessment2 = SolutionAssessment.create(
+            name='Fine', color='invalid', active_color='abcdef',
+            order=6, course=course,
+        )
+        assert assessment2.color == DEFAULT_ASSESSMENT_BUTTON_COLOR
+        assert assessment2.active_color == '#abcdef'
+
+    @staticmethod
+    def test_solution_assessment_on_save(course: Course, _assessments):
+        assessment = SolutionAssessment.get(SolutionAssessment.order == 2)
+        assessment.color = 'secondary'
+        assessment.active_color = 'fff'
+        assessment.save()
+        assert assessment.color == COLORS.get('secondary')
+        assert assessment.active_color == '#fff'
+
+        assessment.color = 'xox'
+        assessment.active_color = 'invalid'
+        assessment.save()
+        assert assessment.color == DEFAULT_ASSESSMENT_BUTTON_COLOR
+        assert (
+            assessment.active_color == DEFAULT_ASSESSMENT_BUTTON_ACTIVE_COLOR,
+        )