[WIP] Store Questions in Module rather than individual records

guidedmodules/_MQS_notes.txt

@@ -0,0 +1,133 @@
+MQS
+===
+
+Migrate Questions from being individual records to bein array in Module.
+
+Benefits:
+
+- Easier authoring by either just changing one Module record, or copying one Module record and bumping version.
+
+Done
+----
+
+- File app_loading.py now retains the questions inside module.spec. When app is loaded, the question array still exists in Module.
+
+- Add guidedmodules.models.Module.questions_set to return questions from Module.spec.
+
+- Add guidedmodules.models.get_question_by_id to return question by id from Module.spec.
+
+
+In Progress
+-----------
+
+
+
+CONTINUE HERE > - Have views.show_question retrieve the current answer if exists
+    answer = taskq.get_current_answer()
+
+    When Q does 'show_question', q value is passed in and q is type ModuleQuestion. But we are migrating away from ModuleQuestion.
+    The module_question_id must be passed instead to get the question from Module.spec.question by matching on the question id.
+    BUT, I don't understand how a whole question instance can be passed in a get URL? BECAUSE of the @task_view decorator, dammit!
+    We need @task_view to not return ModuleQuestion: see 
+        `question = get_object_or_404(ModuleQuestion, module=task.module, key=question_key)`
+
+    to recover the current question answers the code calls `models.get_all_current_answer_records`
+
+        questions = {question.id: question for question in
+                     ModuleQuestion.objects.select_related('module').filter(
+                         id__in=history.values_list('taskanswer__question_id', flat=True))}
+
+- Have guidedmodules.views.get_next_question get next question from Module.spec.question array instead of list of Module.question.
+    - The next question is determined as part of saving the answer.
+
+- Add get_all_current_answer_records2(tasks) to get all TaskAnswers efficiently
+
+- Editing a question saves in Module.spec.question.
+
+
+TO DO
+-----
+
+- (general) Use Module.questions_set everywhere instead of relationship Module.questions to get questions.
+
+- Editing question edits Module.spec.question
+
+- What type of object should module.get_question_by_id return? dictionary object?
+
+- Deleting question (optionally) bumps app version and adds new module record (to protect old version).
+
+- Adding/Deleting modules bumps app version
+
+- Make sure TaskAnswers are related/can be found by data in Module.spec.question instead of by Django ORM relationship to ModuleQuestion.
+
+- Migrate Guidedmodules.models.ModuleQuestion back into guidedmodules.models.Module. Need to make sure any questionaires created witin application are now stored in Module.spec.question.
+
+- Have models.Module.serialize Module.spec.question.
+    Revise this section within code block:
+    ```
+    spec["questions"] = []
+        for i, q in enumerate(self.questions.order_by('definition_order')):
+            if i == 0 and q.key == "_introduction":
+                spec["introduction"] = {"format": "markdown", "template": q.spec["prompt"]}
+                continue
+    ...
+    ```
+
+- Revise Module.questions_dependencies
+    Use Module.spec.questions
+
+- Delete guidedmodules.models.ModuleQuestion model.
+
+- Remove instrumentation?
+
+
+
+Areas
+-----
+app_loading - storing questions, they are already in module.spec
+show_question
+update question from authoring tool
+walk questions
+impute questions
+serialize
+serialize to download
+
+TaskAnswer overhaul bc relationship would be on module.spec['question'].get_by_id
+
+key vs id might be related to reassigning the name of a module in app.yaml
+
+adding `key` and `definition_order` into Module.spec.questions
+
+when starting module and creating tasks and task questions - views.new_task
+
+taskanswer point back to question
+
+I want to start using module.questions_set as replacement for module.questions
+
+self.module.questions.get(key=question) needs to be self.module.questions_set.get_by_key(key=question) or get_by_id
+
+Lines that need to change:
+--------------------------
+guidedmodules.views:  q = task.module.questions.get(id=request.POST.get("question"))
+to change that file, I need to also send the module_question_id or question_key in the question form
+
+
+    def redirect_to():
+
+        # ==============================================
+        # TEMPORARY TODO: FIX
+        # TEMPORARY WHILE MOVE AWAY FROM ModuleQuestions
+        return "/projects"
+        # ==============================================
+
+        next_q = get_next_question(q, task)
+        if next_q:
+            # Redirect to the next question.
+            return task.get_absolute_url_to_question(next_q) + f"?back_url={back_url}&previous=nquestion"
+        # Redirect to the module finished page because there are no more questions to answer.
+        return task.get_absolute_url() + f"/finished?back_url={back_url}&previous=nquestion"
+
+
+
+questions are already in Module.spec in module.spec['questions']; could leave them there.
+But remeber to create as OrderedDict

guidedmodules/admin.py

@@ -3,6 +3,7 @@
 from django.utils.html import escape as escape_html
 from django_json_widget.widgets import JSONEditorWidget
 from jsonfield import JSONField
+from django.db import models
 
 
 from .models import \
@@ -261,6 +262,14 @@ class ModuleAdmin(admin.ModelAdmin):
     }
     def app_(self, obj): return "{} [{}]".format(obj.app.appname, obj.app.id) if obj.app else "(not in an app)"
 
+# class ModuleQuestionSetAdmin(admin.ModelAdmin):
+#     list_display = ('id', 'module')
+#     raw_id_fields = ('module',)
+#     readonly_fields = ("id",)
+#     formfield_overrides = {
+#         models.JSONField: {'widget': JSONEditorWidget},
+#     }
+
 class ModuleQuestionAdmin(admin.ModelAdmin):
     list_display = ('id', 'key', 'module')
     raw_id_fields = ('module', 'answer_type_module')
@@ -293,11 +302,12 @@ def organization_and_project(self, obj):
         return obj.project.organization_and_title()
 
 class TaskAnswerAdmin(admin.ModelAdmin):
-    list_display = ('id', 'question', 'task', '_project', 'created')
+    list_display = ('id', 'question', 'module_question_id', 'task', '_project', 'created')
     raw_id_fields = ('task',)
     readonly_fields = ('id', 'task', 'question')
-    search_fields = ('task__project__organization__name', 'task__module__key')
+    search_fields = ('task__project__organization__name', 'task__module__key', 'module_question_id')
     fieldsets = [
+        (None, {"fields": ('module_question_id',)}),
         (None, {"fields": ('task', 'question')}),
         (None, {"fields": ('notes',)}),
         (None, {"fields": ('extra',)}),
@@ -334,6 +344,7 @@ class InstrumentationEventAdmin(admin.ModelAdmin):
 admin.site.register(AppVersion, AppVersionAdmin)
 admin.site.register(AppInput, AppInputAdmin)
 admin.site.register(Module, ModuleAdmin)
+# admin.site.register(ModuleQuestionSet, ModuleQuestionSetAdmin)
 admin.site.register(ModuleQuestion, ModuleQuestionAdmin)
 admin.site.register(ModuleAsset, ModuleAssetAdmin)
 admin.site.register(Task, TaskAdmin)

guidedmodules/app_loading.py

@@ -226,8 +226,8 @@ def create_module(app, appinst, spec):
 
 def remove_questions(spec):
     spec = OrderedDict(spec) # clone
-    if "questions" in spec:
-        del spec["questions"]
+    # if "questions" in spec:
+    #     del spec["questions"]
     return spec
 
 
@@ -246,6 +246,7 @@ def update_module(m, spec, log_status):
     # Update its questions.
     qs = set()
     for i, question in enumerate(spec.get("questions", [])):
+        # print(f"adding question {i}, {question}")
         qs.add(update_question(m, i, question, log_status))
 
     # Delete removed questions (only happens if the Module is
@@ -303,11 +304,13 @@ def is_module_changed(m, source, spec, module_id_map=None):
     #   any string => Incompatible change - a new database record is needed.
 
     # If all other metadata is the same, then there are no changes.
-    if \
-            json.dumps(m.spec, sort_keys=True) == json.dumps(remove_questions(spec), sort_keys=True) \
-        and json.dumps([q.spec for q in m.get_questions()], sort_keys=True) \
-            == json.dumps([q for q in spec.get("questions", [])], sort_keys=True):
+    if json.dumps(m.spec, sort_keys=True) == json.dumps(spec, sort_keys=True):
         return None
+    # if \
+    #         json.dumps(m.spec, sort_keys=True) == json.dumps(remove_questions(spec), sort_keys=True) \
+    #     and json.dumps([q.spec for q in m.get_questions()], sort_keys=True) \
+    #         == json.dumps([q for q in spec.get("questions", [])], sort_keys=True):
+    #     return None
 
     # Now we're just checking if the change is compatible or not with
     # the existing database record.

guidedmodules/migrations/0061_taskanswer_module_question_id.py

@@ -0,0 +1,28 @@
+# Generated by Django 3.2.5 on 2021-12-19 14:30
+
+from django.db import migrations, models
+
+def forwards_func(apps, schema_editor):
+    # Update set module_question_id for existing TaskAnswer
+    # module_question_id is not a Django id, but is question yaml identifier string (e.g., key)
+    ta_model = apps.get_model("guidedmodules", "TaskAnswer")
+    db_alias = schema_editor.connection.alias
+    taskanswers = ta_model.objects.all()
+    for taskanswer in taskanswers:
+        taskanswer.module_question_id = taskanswer.question.spec['id']
+    ta_model.objects.bulk_update(taskanswers, ['module_question_id'], 100)
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('guidedmodules', '0060_alter_moduleasset_updated'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='taskanswer',
+            name='module_question_id',
+            field=models.CharField(blank=True, help_text='ID of the question', max_length=200, null=True),
+        ),
+        migrations.RunPython(forwards_func, migrations.RunPython.noop),
+    ]

guidedmodules/migrations/0062_alter_taskanswer_question.py

@@ -0,0 +1,19 @@
+# Generated by Django 3.2.5 on 2021-12-19 16:47
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('guidedmodules', '0061_taskanswer_module_question_id'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='taskanswer',
+            name='question',
+            field=models.ForeignKey(help_text="The question (within the Task's Module) that this TaskAnswer is answering.", null=True, on_delete=django.db.models.deletion.PROTECT, to='guidedmodules.modulequestion'),
+        ),
+    ]

guidedmodules/models.py

@@ -15,6 +15,7 @@
 from collections import OrderedDict
 import uuid
 
+from django.db import models
 from api.base.models import BaseModel
 from siteapp.enums.assets import AssetTypeEnum
 from guidedmodules.enums.inputs import InputTypeEnum
@@ -368,6 +369,20 @@ def spec_yaml(self):
         import rtyaml
         return rtyaml.dump(self.spec)
 
+    @property
+    def questions_set(self):
+        """Return list of questions from Module spec"""
+
+        questions_set = self.spec.get("questions", [])
+        # Add in keys and definition order until we do this work on load
+        [q.update({"key": q["id"], "definition_order": i}) for i, q in enumerate(questions_set,1)]
+        return questions_set
+
+    def get_question_by_id(self, q_id):
+        """Return question object by id of question"""
+
+        return next((q for q in self.questions_set if q['id'] == q_id), None)
+
     def get_questions(self):
         # Return the ModuleQuestions in definition order.
         return list(self.questions.order_by('definition_order'))
@@ -744,6 +759,31 @@ def get_static_asset_image_data_url(self, asset_path, max_image_size):
 
     # ANSWERS
 
+    @staticmethod
+    def get_all_current_answer_records2(tasks):
+        # Efficiently get the current answer to every question of each of the tasks.
+        #
+        # Since we track the history of answers to each question, we need to get the most
+        # recent answer for each question. It's fastest if we pre-load the complete history
+        # of every question rather than making a separate database call for each question
+        # to find its most recent answer. See TaskAnswer.get_current_answer().
+        #
+        # Return a generator that yields tuples of (Task, Module.spec['question']['question']?, TaskAnswerHistory).
+        # Among tuples for a particular Task, the tuples are in order of the questions
+
+        ## TODO Migrating away from ModuleQuestions
+        # Should be able to replace most of the below with the following revised code
+        # for task in tasks:
+        #   module = task.module
+        #   for question in module.spec.['questions']:
+        #       # Get the latest TaskAnswerHistory for the question, if there is one
+        #       # [by n+1 filter]: TaskAnswerHistory.objects.filter(task and question_key)
+                # [by sorting through history]
+                # next(tah for tah blah, blah if tah.value=x)
+                # get answer
+                # yield (task, question, answer)
+                return (1, 1, 1)
+
     @staticmethod
     def get_all_current_answer_records(tasks):
         # Efficiently get the current answer to every question of each of the tasks.
@@ -766,15 +806,21 @@ def get_all_current_answer_records(tasks):
         tasks_ = {task.id: task for task in
                   Task.objects.select_related('module', 'project').filter(
                       id__in=history.values_list("taskanswer__task_id", flat=True))}
+
+        # TODO: To migrate away from ModuleQuestion, we need to use module.spec.question
+        # or module.get_questionby_id
+        # I think we can completely redo this process because we don't need to re-assemble questions
+        # from rows in database
+        # Below gets question for immediate module
         questions = {question.id: question for question in
                      ModuleQuestion.objects.select_related('module').filter(
                          id__in=history.values_list('taskanswer__question_id', flat=True))}
+        print(50, "=============", questions)
 
         for ansh in history:
             current_answers.setdefault(
                 (tasks_.get(ansh.taskanswer.task_id), questions.get(ansh.taskanswer.question_id)), ansh)
-
-        # Batch load all of the ModuleQuestions.
+        # Batch load all of the ModuleQuestions for any question that has a module as an answer?
         questions = ModuleQuestion.objects.prefetch_related('answer_type_module__questions').select_related('module') \
             .filter(module__in={task.module for task in tasks}) \
             .order_by("definition_order")
@@ -1730,9 +1776,9 @@ def import_json_update(self, data, deserializer):
 class TaskAnswer(BaseModel):
     task = models.ForeignKey(Task, on_delete=models.CASCADE, related_name="answers",
                              help_text="The Task that this TaskAnswer is a part of.")
-    question = models.ForeignKey(ModuleQuestion, on_delete=models.PROTECT,
+    question = models.ForeignKey(ModuleQuestion, null=True, on_delete=models.PROTECT,
                                  help_text="The question (within the Task's Module) that this TaskAnswer is answering.")
-
+    module_question_id = models.CharField(max_length=200, unique=False, null=True, blank=True, help_text="ID of the question")
     notes = models.TextField(blank=True, help_text="Notes entered by editors working on this question.")
     extra = JSONField(blank=True, help_text="Additional information stored with this object.")
 
@@ -1751,6 +1797,11 @@ def get_absolute_url(self):
         from urllib.parse import quote
         return self.task.get_absolute_url_to_question(self.question)
 
+    def get_module_question(self):
+        """Get question object from module.spec["questions"]"""
+        question_obj = self.task.module.get_question_by_id(self.module_question_id)
+        return question_obj
+
     def get_current_answer(self):
         # The current answer is the one with the highest primary key.
         return self.answer_history \
@@ -1910,6 +1961,7 @@ def are_files_same():
 
         # Kick the Task and TaskAnswer's updated field and let the Task know that
         # its answers have changed.
+        print(10,"=========10 inside save_answer====")
         self.save(update_fields=[])
         self.task.on_answer_changed()