fixes error where progress is not being saved in the database

Miguel Barão
1 parent d8e3946a
Showing 6 changed files with 151 additions and 85 deletions Show diff stats
BUGS.md
aprendizations/learnapp.py
aprendizations/questions.py
aprendizations/serve.py
aprendizations/student.py
aprendizations/tools.py
 # BUGS
+- ir para inicio da pagina quando le nova pergunta.
 - nao esta a seguir o max_tries definido no ficheiro de dependencias.
 - devia mostrar timeout para o aluno saber a razao.
 - permitir configuracao para escolher entre static files locais ou remotos
@@ -31,6 +32,8 @@
 # FIXED
+- CRITICAL nao esta a guardar o progresso na base de dados.
+- mesma ref no mesmo ficheiro não é detectado.
 - enter nas respostas mostra json
 - apos clicar no botao responder, inactivar o input (importante quando o tempo de correcção é grande)
 - double click submits twice.
@@ -7,7 +7,7 @@ from datetime import datetime
 import logging
 from random import random
 from os import path
-from typing import Any, Dict, Iterable, List, Optional, Tuple
+from typing import Any, Dict, Iterable, List, Optional, Tuple, Set, DefaultDict
 # third party libraries
 import bcrypt
@@ -88,9 +88,9 @@ class LearnApp(object):
         self.courses = config['courses']
         logger.info(f'Courses:  {", ".join(self.courses.keys())}')
         for c, d in self.courses.items():
+            d.setdefault('title', '')  # course title undefined
             for goal in d['goals']:
                 if goal not in self.deps.nodes():
-                    # logger.error(f'Goal "{goal}" of "{c}"" not in the graph')
                     raise LearnException(f'Goal "{goal}" from course "{c}" '
                                          ' does not exist')
@@ -229,9 +229,8 @@ class LearnApp(object):
     # ------------------------------------------------------------------------
     async def check_answer(self, uid: str, answer) -> Question:
         student = self.online[uid]['state']
-        topic = student.get_current_topic()
-
-        q = await student.check_answer(answer)
+        await student.check_answer(answer)
+        q = student.get_current_question()
         logger.info(f'User "{uid}" got {q["grade"]:.2} in "{q["ref"]}"')
@@ -243,14 +242,24 @@ class LearnApp(object):
                 starttime=str(q['start_time']),
                 finishtime=str(q['finish_time']),
                 student_id=uid,
-                topic_id=topic))
-            logger.debug(f'db insert answer of {q["ref"]}')
+                topic_id=student.get_current_topic()))
+
+        return q
+
+    # ------------------------------------------------------------------------
+    # get the question to show (current or new one)
+    # if no more questions, save/update level in database
+    # ------------------------------------------------------------------------
+    async def get_question(self, uid: str) -> Optional[Question]:
+        student = self.online[uid]['state']
+        q: Optional[Question] = await student.get_question()
-        # save topic if finished
+        # save topic to database if finished
         if student.topic_has_finished():
-            logger.info(f'User "{uid}" finished "{topic}"')
+            topic: str = student.get_previous_topic()
             level: float = student.get_topic_level(topic)
             date: str = str(student.get_topic_date(topic))
+            logger.info(f'User "{uid}" finished "{topic}" (level={level:.2})')
             with self.db_session() as s:
                 a = s.query(StudentTopic) \
@@ -276,12 +285,6 @@ class LearnApp(object):
         return q
     # ------------------------------------------------------------------------
-    # get the question to show (current or new one)
-    # ------------------------------------------------------------------------
-    async def get_question(self, uid: str) -> Optional[Question]:
-        return await self.online[uid]['state'].get_question()
-
-    # ------------------------------------------------------------------------
     # Start course
     # ------------------------------------------------------------------------
     def start_course(self, uid: str, course: str) -> None:
@@ -409,14 +412,28 @@ class LearnApp(object):
             fullpath: str = path.join(topicpath, t['file'])
             logger.debug(f'  Loading {fullpath}')
-            questions: List[QDict] = load_yaml(fullpath, default=[])
+            # questions: List[QDict] = load_yaml(fullpath, default=[])
+            try:
+                questions: List[QDict] = load_yaml(fullpath)
+            except Exception:
+                raise LearnException(f'Failed to load "{fullpath}"')
+
+            if not isinstance(questions, list):
+                msg = f'File "{fullpath}" must be a list of questions'
+                raise LearnException(msg)
             # update refs to include topic as prefix.
             # refs are required to be unique only within the file.
             # undefined are set to topic:n, where n is the question number
             # within the file
+            localrefs: Set[str] = set()  # refs in current file
             for i, q in enumerate(questions):
                 qref = q.get('ref', str(i))  # ref or number
+                if qref in localrefs:
+                    msg = f'Duplicate ref "{qref}" in "{topicpath}"'
+                    raise LearnException(msg)
+                localrefs.add(qref)
+
                 q['ref'] = f'{tref}:{qref}'
                 q['path'] = topicpath
                 q.setdefault('append_wrong', t['append_wrong'])
@@ -439,7 +456,7 @@ class LearnApp(object):
     # ------------------------------------------------------------------------
     def get_login_counter(self, uid: str) -> int:
-        return self.online[uid]['counter']
+        return int(self.online[uid]['counter'])
     # ------------------------------------------------------------------------
     def get_student_name(self, uid: str) -> str:
@@ -474,7 +491,7 @@ class LearnApp(object):
         return self.online[uid]['state'].get_current_course_title()
     # ------------------------------------------------------------------------
-    def get_student_course_id(self, uid: str) -> str:
+    def get_student_course_id(self, uid: str) -> Optional[str]:
         return self.online[uid]['state'].get_current_course_id()
     # ------------------------------------------------------------------------
@@ -520,7 +537,7 @@ class LearnApp(object):
         # compute topic progress
         now = datetime.now()
         goals = self.courses[course_id]['goals']
-        prog = defaultdict(int)
+        prog: DefaultDict[str, float] = defaultdict(int)
         for uid, topic, level, date in student_topics:
             if topic in goals:
 # python standard library
 import asyncio
+from datetime import datetime
 import random
 import re
 from os import path
@@ -44,6 +45,10 @@ class Question(dict):
             'files': {},
             }))
+    def set_answer(self, ans) -> None:
+        self['answer'] = ans
+        self['finish_time'] = datetime.now()
+
     def correct(self) -> None:
         self['comments'] = ''
         self['grade'] = 0.0
@@ -74,7 +79,14 @@ class QuestionRadio(Question):
     def __init__(self, q: QDict) -> None:
         super().__init__(q)
-        n = len(self['options'])
+        try:
+            n = len(self['options'])
+        except KeyError:
+            msg = f'Missing `options` in radio question. See {self["path"]}'
+            raise QuestionException(msg)
+        except TypeError:
+            msg = f'`options` must be a list. See {self["path"]}'
+            raise QuestionException(msg)
         self.set_defaults(QDict({
             'text': '',
@@ -165,7 +177,7 @@ class QuestionRadio(Question):
             #       x_aver can be exactly 1.0 if all options are right
             if self['discount'] and x_aver != 1.0:
                 x = (x - x_aver) / (1.0 - x_aver)
-            self['grade'] = x
+            self['grade'] = float(x)
 # ============================================================================
@@ -185,7 +197,14 @@ class QuestionCheckbox(Question):
     def __init__(self, q: QDict) -> None:
         super().__init__(q)
-        n = len(self['options'])
+        try:
+            n = len(self['options'])
+        except KeyError:
+            msg = f'Missing `options` in radio question. See {self["path"]}'
+            raise QuestionException(msg)
+        except TypeError:
+            msg = f'`options` must be a list. See {self["path"]}'
+            raise QuestionException(msg)
         # set defaults if missing
         self.set_defaults(QDict({
@@ -293,13 +293,18 @@ class QuestionHandler(BaseHandler):
         'alert':            'question-information.html',
     }
-    # --- get question to render
+    # ------------------------------------------------------------------------
+    # GET
+    # gets question to render. If there are no more questions in the topic
+    # shows an animated trophy
+    # ------------------------------------------------------------------------
     @tornado.web.authenticated
     async def get(self):
         logger.debug('[QuestionHandler]')
         user = self.current_user
         q = await self.learn.get_question(user)
+        # show current question
         if q is not None:
             qhtml = self.render_string(self.templates[q['type']],
                                        question=q, md=md_to_html)
@@ -313,6 +318,7 @@ class QuestionHandler(BaseHandler):
                     }
                 }
+        # show animated trophy
         else:
             finished = self.render_string('finished_topic.html')
             response = {
@@ -324,7 +330,11 @@ class QuestionHandler(BaseHandler):
         self.write(response)
-    # --- post answer, returns what to do next: shake, new_question, finished
+    # ------------------------------------------------------------------------
+    # POST
+    # corrects answer and returns status: right, wrong, try_again
+    # does not move to next question.
+    # ------------------------------------------------------------------------
     @tornado.web.authenticated
     async def post(self) -> None:
         user = self.current_user
@@ -364,6 +374,7 @@ class QuestionHandler(BaseHandler):
         # --- built response to return
         response = {'method': q['status'], 'params': {}}
+
         if q['status'] == 'right':   # get next question in the topic
             comments_html = self.render_string(
                 'comments-right.html', comments=q['comments'], md=md_to_html)
@@ -17,12 +17,21 @@ logger = logging.getLogger(__name__)
 # ----------------------------------------------------------------------------
-# kowledge state of a student
-# Contains:
+# kowledge state of a student:
+#   uid - string with userid, e.g. '12345'
 #   state - dict of unlocked topics and their levels
-#   deps  - access to dependency graph shared between students
-#   topic_sequence - list with the recommended topic sequence
-#   current_topic - nameref of the current topic
+#           {'topic1': {'level': 0.5, 'date': datetime}, ...}
+#   topic_sequence - recommended topic sequence ['topic1', 'topic2', ...]
+#   questions - [Question, ...] for the current topic
+#   current_course - string or None
+#   current_topic - string or None
+#   current_question - Question or None
+#
+#
+# also has access to shared data between students:
+#   courses - dictionary {course: [topic1, ...]}
+#   deps  - dependency graph as a networkx digraph
+#   factory - dictionary {ref: QFactory}
 # ----------------------------------------------------------------------------
 class StudentState(object):
     # =======================================================================
@@ -50,6 +59,7 @@ class StudentState(object):
             self.current_course: Optional[str] = None
             self.topic_sequence: List[str] = []
             self.current_topic: Optional[str] = None
+            # self.previous_topic: Optional[str] = None
         else:
             logger.debug(f'starting course {course}')
             self.current_course = course
@@ -58,7 +68,7 @@ class StudentState(object):
     # ------------------------------------------------------------------------
     # Start a new topic.
-    #    questions: list of generated questions to do in the topic
+    #    questions: list of generated questions to do in the given topic
     #    current_question: the current question to be presented
     # ------------------------------------------------------------------------
     async def start_topic(self, topic: str) -> None:
@@ -74,9 +84,10 @@ class StudentState(object):
             logger.debug(f'is locked "{topic}"')
             return
+        self.previous_topic: Optional[str] = None
+
         # choose k questions
         self.current_topic = topic
-        # self.current_question = None
         self.correct_answers = 0
         self.wrong_answers = 0
         t = self.deps.nodes[topic]
@@ -96,30 +107,17 @@ class StudentState(object):
         self.next_question()
     # ------------------------------------------------------------------------
-    # The topic has finished and there are no more questions.
-    # The topic level is updated in state and unlocks are performed.
-    # The current topic is unchanged.
-    # ------------------------------------------------------------------------
-    def finish_topic(self) -> None:
-        logger.debug(f'finished {self.current_topic}')
-
-        self.state[self.current_topic] = {
-            'date': datetime.now(),
-            'level': self.correct_answers / (self.correct_answers +
-                                             self.wrong_answers)
-            }
-        self.current_topic = None
-        self.current_question = None
-        self.unlock_topics()
-
-    # ------------------------------------------------------------------------
     # corrects current question
+    # updates keys: answer, grade, finish_time, status, tries
     # ------------------------------------------------------------------------
-    async def check_answer(self, answer) -> Tuple[Question, str]:
+    async def check_answer(self, answer) -> None:
         q = self.current_question
-        q['answer'] = answer
-        q['finish_time'] = datetime.now()
-        await q.correct_async()
+        if q is None:
+            logger.error('check_answer called but current_question is None!')
+            return None
+
+        q.set_answer(answer)
+        await q.correct_async()  # updates q['grade']
         if q['grade'] > 0.999:
             self.correct_answers += 1
@@ -134,13 +132,17 @@ class StudentState(object):
                 q['status'] = 'wrong'
         logger.debug(f'ref = {q["ref"]},  status = {q["status"]}')
-        return q
     # ------------------------------------------------------------------------
-    # get question to show, current or next
+    # gets next question to show if the status is 'right' or 'wrong',
+    # otherwise just returns the current question
     # ------------------------------------------------------------------------
     async def get_question(self) -> Optional[Question]:
         q = self.current_question
+        if q is None:
+            logger.error('get_question called but current_question is None!')
+            return None
+
         logger.debug(f'{q["ref"]} status = {q["status"]}')
         if q['status'] == 'right':
@@ -151,10 +153,6 @@ class StudentState(object):
                 new_question = await self.factory[q['ref']].gen_async()
                 self.questions.append(new_question)
             self.next_question()
-        # elif q['status'] == 'new':
-        #     pass
-        # elif q['status'] == 'try_again':
-        #     pass
         return self.current_question
@@ -172,7 +170,25 @@ class StudentState(object):
         q['start_time'] = datetime.now()
         q['tries'] = q.get('max_tries', t['max_tries'])
         q['status'] = 'new'
-        self.current_question = q
+        self.current_question: Optional[Question] = q
+
+    # ------------------------------------------------------------------------
+    # The topic has finished and there are no more questions.
+    # The topic level is updated in state and unlocks are performed.
+    # The current topic is unchanged.
+    # ------------------------------------------------------------------------
+    def finish_topic(self) -> None:
+        logger.debug(f'finished "{self.current_topic}"')
+
+        self.state[self.current_topic] = {
+            'date': datetime.now(),
+            'level': self.correct_answers / (self.correct_answers +
+                                             self.wrong_answers)
+            }
+        self.previous_topic = self.current_topic
+        self.current_topic = None
+        self.current_question = None
+        self.unlock_topics()
     # ------------------------------------------------------------------------
     # Update proficiency level of the topics using a forgetting factor
@@ -211,7 +227,7 @@ class StudentState(object):
     # ========================================================================
     def topic_has_finished(self) -> bool:
-        return self.current_topic is None
+        return self.current_topic is None and self.previous_topic is not None
     # ------------------------------------------------------------------------
     # compute recommended sequence of topics  ['a', 'b', ...]
@@ -238,8 +254,12 @@ class StudentState(object):
         return self.current_topic
     # ------------------------------------------------------------------------
-    def get_current_course_title(self) -> Optional[str]:
-        return self.courses[self.current_course]['title']
+    def get_previous_topic(self) -> Optional[str]:
+        return self.previous_topic
+
+    # ------------------------------------------------------------------------
+    def get_current_course_title(self) -> str:
+        return str(self.courses[self.current_course]['title'])
     # ------------------------------------------------------------------------
     def get_current_course_id(self) -> Optional[str]:
@@ -269,7 +289,7 @@ class StudentState(object):
     # ------------------------------------------------------------------------
     def get_topic_level(self, topic: str) -> float:
-        return self.state[topic]['level']
+        return float(self.state[topic]['level'])
     # ------------------------------------------------------------------------
     def get_topic_date(self, topic: str):
@@ -111,14 +111,13 @@ class HighlightRenderer(mistune.Renderer):
     def table(self, header, body):
         return ('<table class="table table-sm"><thead class="thead-light">'
-               f'{header}</thead><tbody>{body}</tbody></table>')
+                f'{header}</thead><tbody>{body}</tbody></table>')
     def image(self, src, title, alt):
         alt = mistune.escape(alt, quote=True)
         title = mistune.escape(title or '', quote=True)
         return (f'<img src="/file/{src}" alt="{alt}" title="{title}"'
-                 'class="img-fluid">')
-                # class="img-fluid mx-auto d-block"
+                f'class="img-fluid">')  # class="img-fluid mx-auto d-block"
     # Pass math through unaltered - mathjax does the rendering in the browser
     def block_math(self, text):
@@ -150,25 +149,22 @@ def load_yaml(filename: str, default: Any = None) -&gt; Any:
     filename = path.expanduser(filename)
     try:
         f = open(filename, 'r', encoding='utf-8')
-    except FileNotFoundError:
-        logger.error(f'Cannot open "{filename}": not found')
-    except PermissionError:
-        logger.error(f'Cannot open "{filename}": no permission')
-    except OSError:
-        logger.error(f'Cannot open file "{filename}"')
-    else:
-        with f:
-            try:
-                default = yaml.safe_load(f)
-            except yaml.YAMLError as e:
-                if hasattr(e, 'problem_mark'):
-                    mark = e.problem_mark
-                    logger.error(f'File "{filename}" near line {mark.line+1}, '
-                                 f'column {mark.column+1}')
-                else:
-                    logger.error(f'File "{filename}"')
-    finally:
-        return default
+    except Exception as e:
+        logger.error(e)
+        if default is not None:
+            return default
+        else:
+            raise
+
+    with f:
+        try:
+            return yaml.safe_load(f)
+        except yaml.YAMLError as e:
+            logger.error(str(e).replace('\n', ' '))
+            if default is not None:
+                return default
+            else:
+                raise
 # ---------------------------------------------------------------------------