Questions in database

[albertas-jn]

Implements questions in a database.
Don't forget to backup your database before testing!

Closes #679

[BeritJanssen]

These questions should be randomized.

[BeritJanssen]

I haven't had the chance to test this yet, but here's my two cents:

• in the interest of not bloating the experiment app even further, I'd be strongly in favour of moving the question logic (including the questions directory within experiment) to its own app, which we may creatively call question. I know it's a rather painful change to request (sorry), but I think it's less painful to refactor now than later.
• there is no unit test for the createquestions command, or for any of the admin views
• right now (since I've only had a superficial look really) I cannot fully understand why questionseries_admin and the views it calls are needed, i.e., why this is not possible in vanilla Django admin. Docstrings would be appreciated, also docstrings explaining what the different question models (Question / QuestioInSeries / QuestionInSeries / QuestionGroup) are for

[BeritJanssen]

I don't quite understand the distinction of this model vs. QuestionSeries. Why do both have a ManyToManyField to Question? And why does QuestionInSeries have a ForeignKey to QuestionSeries on top of that? It seems to me as if we're now defining relationships multiple ways.

[albertas-jn]

I was hoping that people test it first, so that code is much clearer, because I think the interface is quite intuitive. QuestionGroup is just a collection of questions for convenience to easier add to the QuestionSeries, which belongs to Experiment and describes the question sequence in experiment.

[BeritJanssen]

Docstrings would still be a good idea.

[BeritJanssen]

This should be a OneToOneField, I think.

[albertas-jn]

OneToOneField to a Question? The same Question can participate in multiple experiments, so this would require making multiple copies of the same Question in the database.

[BeritJanssen]

You're absolutely right, of course!

[BeritJanssen]

While the naming of the Experiment, ExperimentCollection and ExperimentGroup is probably just as hard to follow for outsiders, the principles behind this are (in my mind) rather similar, so I think we'd benefit from following the same naming conventions, as much as possible.

[albertas-jn]

When I started working on this, there was still ExperimentSeries, so I tried to follow that convention. To me, "Series" is clearer because it implies ordering. But of course it can be renamed. What would you suggest?

[BeritJanssen]

If you want to call this QuestionSeries, that's fine. ExperimentCollection will be renamed anyway, so it's not necessary to keep consistency there. But perhaps Question -> GroupedQuestion -> QuestionGroup -> QuestionSeries?

[Evert-R]

I tried running this branch but run into these migration problems:

Operations to perform:
server-1 | Apply all migrations: admin, auth, contenttypes, experiment, image, participant, result, section, session, sessions, theme
server-1 | Running migrations:
server-1 | Applying experiment.0034_add_question_model... OK
server-1 | Traceback (most recent call last):
server-1 | File "manage.py", line 24, in
server-1 | main()
server-1 | File "manage.py", line 20, in main
server-1 | execute_from_command_line(sys.argv)
server-1 | File "/usr/local/lib/python3.8/site-packages/django/core/management/init.py", line 419, in execute_from_command_line
server-1 | utility.execute()
server-1 | File "/usr/local/lib/python3.8/site-packages/django/core/management/init.py", line 413, in execute
server-1 | self.fetch_command(subcommand).run_from_argv(self.argv)
server-1 | File "/usr/local/lib/python3.8/site-packages/django/core/management/base.py", line 354, in run_from_argv
server-1 | self.execute(*args, **cmd_options)
server-1 | File "/usr/local/lib/python3.8/site-packages/django/core/management/base.py", line 398, in execute
server-1 | output = self.handle(*args, **options)
server-1 | File "/usr/local/lib/python3.8/site-packages/django/core/management/base.py", line 89, in wrapped
server-1 | res = handle_func(*args, **kwargs)
server-1 | File "/usr/local/lib/python3.8/site-packages/django/core/management/commands/migrate.py", line 244, in handle
server-1 | post_migrate_state = executor.migrate(
server-1 | File "/usr/local/lib/python3.8/site-packages/django/db/migrations/executor.py", line 117, in migrate
server-1 | state = self._migrate_all_forwards(state, plan, full_plan, fake=fake, fake_initial=fake_initial)
server-1 | File "/usr/local/lib/python3.8/site-packages/django/db/migrations/executor.py", line 147, in _migrate_all_forwards
server-1 | state = self.apply_migration(state, migration, fake=fake, fake_initial=fake_initial)
server-1 | File "/usr/local/lib/python3.8/site-packages/django/db/migrations/executor.py", line 227, in apply_migration
server-1 | state = migration.apply(state, schema_editor)
server-1 | File "/usr/local/lib/python3.8/site-packages/django/db/migrations/migration.py", line 126, in apply
server-1 | operation.database_forwards(self.app_label, schema_editor, old_state, project_state)
server-1 | File "/usr/local/lib/python3.8/site-packages/django/db/migrations/operations/special.py", line 190, in database_forwards
server-1 | self.code(from_state.apps, schema_editor)
server-1 | File "/server/experiment/migrations/0035_add_question_model_data.py", line 12, in add_default_question_series
server-1 | experiment.add_default_question_series()
server-1 | File "/server/experiment/models.py", line 279, in add_default_question_series
server-1 | for i,question_series in enumerate(EXPERIMENT_RULESself.rules.question_series):
server-1 | AttributeError: 'CongoSameDiff' object has no attribute 'question_series'
server-1 | Applying experiment.0035_add_question_model_data...
server-1 exited with code 1

[albertas-jn]

@Evert-R Fixed, please try again

[albertas-jn]

The commit messages of the changes made since the last review speak for themselves.

I would just like to comment on the changes in backend/experiment/migrations/0017_experiment_add_questions_field.py, one of the older migrations. It is the migration that introduced the question field in Experiment. This field is not relevant anymore. The problem was that it called a function for a default value of question keys, get_default_question_keys(). This function is obsolete now, but I could not remove it, because the migration file depended on it. To be able to remove the function, I had to edit the old migration file and set default question keys to empty list. Therefore such seemingly odd change in the old migration file.

[BeritJanssen]

Is there a situation where you would want to run the bootstrap command and not the createquestions command, or vice versa? If the answer is "no", it would be nicer to not have to call this extra command from the docker startup, and rather include this utility in bootstrap.py.

[BeritJanssen]

Nice! The createquestions command looks as if it might be included in the bootstrap command, but perhaps there's arguments against it? Does retrieval of a single question from the question series work now, or is this left for later?