Questionnaire choice labels

[ian-ross]

Proposed changes in this pull request

• Fix Adding New Location - Drop Down Pulling from Name instead of Label #478.
• Bump django-jsonattrs version to a version supporting labels for choice fields. This automatically makes dropdowns in attribute forms and attribute detail views use choice labels if they exist.
• Add question option ordering (to keep same order in dropdowns as in XLSForms).
• Migrations:
  • Schema: add choice_labels column for django-jsonattrs Attribute model (django-jsonattrs/jsonattrs/migrations/0002_attribute_choice_labels.py).
  • Data: copy over attribute choice labels from QuestionOption models to new choice_labels field django-jsonattr Attribute instances (cadasta-platform/cadasta/questionnaires/migrations/0006_add_choice_labels.py).
  • Schema: add index field to QuestionOption model, initially with null=True and a default value to populate existing instances (cadasta-platform/cadasta/questionnaires/migrations/0007_add_question_option_index_field.py).
  • Data: set appropriate index values for all existing QuestionOption instances (cadasta-platform/cadasta/questionnaires/migrations/0008_populate_question_option_index_field.py).
  • Schema: change QuestionOption index field to have null=False (cadasta-platform/cadasta/questionnaires/migrations/0009_set_question_option_index_field_properties.py).

The approach taken for setting up the index field in the QuestionOption model is the recommended approach for adding non-NULL fields to existing Django models (add the field with null=True in one migration, fix up the data values in a second migration, then set null=False in a third migration).

When should this PR be merged

I'd be very happy if this could have a "dogpile" review. In particular, I'd like @oliverroick, @bjohare and @linzjax to have a look at it to see if they can spot any possible problems with the migrations. I'd also like to do some testing on staging with a mirror of the current production database.

Risks

All the usual dangers associated with database migrations...

Follow up actions

None.

[oliverroick]

I’ve tested the migrations against an existing project and 0006_add_choice_labels breaks:

  Rendering model states... DONE
  Applying jsonattrs.0002_attribute_choice_labels... OK
  Applying questionnaires.0006_add_choice_labels...Traceback (most recent call last):
  File "/opt/cadasta/env/lib/python3.5/site-packages/django/db/backends/utils.py", line 64, in execute
    return self.cursor.execute(sql, params)
psycopg2.ProgrammingError: column questionnaires_questionoption.index does not exist
LINE 1: ...."name", "questionnaires_questionoption"."label", "questionn...
                                                             ^


The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "./manage.py", line 10, in <module>
    execute_from_command_line(sys.argv)
  File "/opt/cadasta/env/lib/python3.5/site-packages/django/core/management/__init__.py", line 350, in execute_from_command_line
    utility.execute()
  File "/opt/cadasta/env/lib/python3.5/site-packages/django/core/management/__init__.py", line 342, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/opt/cadasta/env/lib/python3.5/site-packages/django/core/management/base.py", line 348, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/opt/cadasta/env/lib/python3.5/site-packages/django/core/management/base.py", line 399, in execute
    output = self.handle(*args, **options)
  File "/opt/cadasta/env/lib/python3.5/site-packages/django/core/management/commands/migrate.py", line 200, in handle
    executor.migrate(targets, plan, fake=fake, fake_initial=fake_initial)
  File "/opt/cadasta/env/lib/python3.5/site-packages/django/db/migrations/executor.py", line 92, in migrate
    self._migrate_all_forwards(plan, full_plan, fake=fake, fake_initial=fake_initial)
  File "/opt/cadasta/env/lib/python3.5/site-packages/django/db/migrations/executor.py", line 121, in _migrate_all_forwards
    state = self.apply_migration(state, migration, fake=fake, fake_initial=fake_initial)
  File "/opt/cadasta/env/lib/python3.5/site-packages/django/db/migrations/executor.py", line 198, in apply_migration
    state = migration.apply(state, schema_editor)
  File "/opt/cadasta/env/lib/python3.5/site-packages/django/db/migrations/migration.py", line 123, in apply
    operation.database_forwards(self.app_label, schema_editor, old_state, project_state)
  File "/opt/cadasta/env/lib/python3.5/site-packages/django/db/migrations/operations/special.py", line 183, in database_forwards
    self.code(from_state.apps, schema_editor)
  File "/vagrant/cadasta/questionnaires/migrations/0006_add_choice_labels.py", line 57, in add_attribute_choice_labels
    opts = {o.name: o.label for o in q.options.all()}
  File "/opt/cadasta/env/lib/python3.5/site-packages/django/db/models/query.py", line 258, in __iter__
    self._fetch_all()
  File "/opt/cadasta/env/lib/python3.5/site-packages/django/db/models/query.py", line 1074, in _fetch_all
    self._result_cache = list(self.iterator())
  File "/opt/cadasta/env/lib/python3.5/site-packages/django/db/models/query.py", line 52, in __iter__
    results = compiler.execute_sql()
  File "/opt/cadasta/env/lib/python3.5/site-packages/django/db/models/sql/compiler.py", line 852, in execute_sql
    cursor.execute(sql, params)
  File "/opt/cadasta/env/lib/python3.5/site-packages/django/db/backends/utils.py", line 79, in execute
    return super(CursorDebugWrapper, self).execute(sql, params)
  File "/opt/cadasta/env/lib/python3.5/site-packages/django/db/backends/utils.py", line 64, in execute
    return self.cursor.execute(sql, params)
  File "/opt/cadasta/env/lib/python3.5/site-packages/django/db/utils.py", line 95, in __exit__
    six.reraise(dj_exc_type, dj_exc_value, traceback)
  File "/opt/cadasta/env/lib/python3.5/site-packages/django/utils/six.py", line 685, in reraise
    raise value.with_traceback(tb)
  File "/opt/cadasta/env/lib/python3.5/site-packages/django/db/backends/utils.py", line 64, in execute
    return self.cursor.execute(sql, params)
django.db.utils.ProgrammingError: column questionnaires_questionoption.index does not exist
LINE 1: ...."name", "questionnaires_questionoption"."label", "questionn...

I think this is because the migration imports the current Questionnaire model; therefore QuestionOption requires the field index, which is only added in a later migration.

Instead, Question should be imported as Questionnaire = apps.get_model("questionnaires", "Questionnaire"). More info on historical models and migrations here.

[ian-ross]

You're right. I'll fix that!

[ian-ross]

I've changed this, but I want to have a bit of a test on staging first before it's merged. I'll let you know when I've done that.

[linzjax]

So it's working for adding locations, but not for tenure or parties:

[ian-ross]

Huh. Let me look at that!

[ian-ross]

OK, I know what this is. I'll fix it somehow.