Introduce Address answer type #261

MebinAbraham · 2020-08-27T15:57:00Z

What is the context of this PR?

For context see: #241

Introduced a new answer type to support address answers in runner.
This introduces a new data model for storing address answers:

{
  "answer_id": "address",
  "value": {
    "line1": "7 Evelyn Street",
    "town": "Barry",
    "postcode": "CF63 4EN"
  }
}

How to review

Ensure the feature is as expected.

Note:

Support for lookups will be done in a future card.
Support for piping address answers using placeholder will be done in a future card.

Checklist

New static content marked up for translation
Newly defined schema content included in eq-translations repo

scripts/run_validator.sh

MebinAbraham · 2020-08-27T15:59:53Z

templates/partials/answers/address.html

+    "autosuggest":{
+        "label": {
+            "text": _("Enter address or postcode and select from results")
+        },
+    },


This was added because the DS component requires this at the moment. This bug will be fixed in the DS soon.

app/data_model/answer.py

app/forms/field_handlers/address_handler.py

tests/app/forms/field_handlers/test_address_handler.py

JamesGardiner · 2020-08-28T09:29:29Z

tests/app/test_jinja_filters.py

@@ -17,6 +17,7 @@
    map_list_collector_config,
    RadioConfig,
    CheckboxConfig,
+    get_formatted_address,


These imports should be sorted

Again we need to add a linter for things like this (if possible) if we are to enforce. But I have updated anyway 👍

Thanks - I am in the process of adding a card to integrate isort to our linting and make it compatible with Black

MebinAbraham · 2020-09-01T08:11:59Z

@JamesGardiner All comments should now be addressed.

LJBabbage

Works as expected, a few minor comments

app/forms/field_handlers/address_handler.py

LJBabbage · 2020-09-01T11:26:11Z

app/forms/field_handlers/address_handler.py

+
+        return validate_with
+
+    def get_field(self) -> AddressField:


get_address_field?

get_field because it is overriding the abstract base class method.

LJBabbage · 2020-09-01T11:27:58Z

app/forms/fields/address_field.py

+
+from wtforms import Form, FormField, StringField
+
+logger = logging.getLogger(__name__)


Couldn't see where this is used

Not used explicitly but defined to ensure the logger logs errors with the correct module name. This is a common pattern across runner.

app/forms/fields/address_field.py

app/forms/validators.py

LJBabbage · 2020-09-01T12:02:36Z

app/questionnaire/questionnaire_store_updater.py

@@ -216,13 +216,23 @@ def update_answers(self, form_data, list_item_id=None):
        for answer_id, answer_value in form_data.items():

            if answer_id in answer_ids_for_question:
-                if answer_value not in self.EMPTY_ANSWER_VALUES:
+                answer_value_to_store = (


Not 100% sure what this is doing, looks to me if answer_value is a dict make another dict with all the empties removed. Then check it's not completely empty at a later stage?

This is just filtering out any EMPTY_ANSWER_VALUES when the answer is a dict i.e address since we don't store any answers that match EMPTY_ANSWER_VALUES.

tests/functional/spec/components/address/address.spec.js

…onnaire-runner into address-answer-type

app/forms/validators.py

templates/partials/answers/address.html

ajmaddaford

As discussed, we should be testing the summary output for no address answer (optional address), and I've asked Andy about whether address line 1 should be mandatory for optional addresses (if they fill in at least one address field).

…onnaire-runner into address-answer-type

MebinAbraham · 2020-09-03T13:53:07Z

As discussed, we should be testing the summary output for no address answer (optional address), and I've asked Andy about whether address line 1 should be mandatory for optional addresses (if they fill in at least one address field).

Have updated schema/tests for optional version.

MebinAbraham added 5 commits August 17, 2020 19:46

Spike Address Lookup

94b9f9d

Merge spike

b01c10b

Tidy up

81e6960

Add tests

9796d18

Point to address validator branch

9340169

MebinAbraham mentioned this pull request Aug 27, 2020

Add support for address answers ONSdigital/eq-questionnaire-validator#59

Merged

1 task

Merge branch 'master' into address-answer-type

e67e94b

MebinAbraham commented Aug 27, 2020

View reviewed changes

scripts/run_validator.sh Outdated Show resolved Hide resolved

MebinAbraham commented Aug 27, 2020

View reviewed changes

MebinAbraham added 2 commits August 27, 2020 17:07

Update messages.pot

61f80ea

Revert unnecessary changes

162bb9b

JamesGardiner suggested changes Aug 28, 2020

View reviewed changes

MebinAbraham added 2 commits September 1, 2020 08:57

Address comments

1e0dbbd

Merge master and resolve conflicts

05b8f1a

MebinAbraham and others added 2 commits September 1, 2020 09:54

Fix mypy lint

35ceb4f

Merge branch 'master' into address-answer-type

b3322fe

MebinAbraham requested a review from JamesGardiner September 1, 2020 08:58

JamesGardiner approved these changes Sep 1, 2020

View reviewed changes

LJBabbage suggested changes Sep 1, 2020

View reviewed changes

MebinAbraham added 3 commits September 1, 2020 13:32

Merge remote-tracking branch 'origin/master' into address-answer-type

cf21b93

Address comments

35dd9bd

Merge branch 'address-answer-type' of github.com:ONSdigital/eq-questi…

cfaa712

…onnaire-runner into address-answer-type

MebinAbraham requested a review from LJBabbage September 1, 2020 14:38

LJBabbage reviewed Sep 1, 2020

View reviewed changes

app/forms/validators.py Outdated Show resolved Hide resolved

Fix grammar

b5c92e1

MebinAbraham requested a review from LJBabbage September 2, 2020 07:39

MebinAbraham added 3 commits September 2, 2020 09:47

Remove AddressField and use FormField directly

a8a1590

Remove custom address validator class

58b9388

Merge master and resolve conflicts

67f4efa

MebinAbraham requested a review from ajmaddaford September 2, 2020 09:40

LJBabbage approved these changes Sep 2, 2020

View reviewed changes

ajmaddaford reviewed Sep 3, 2020

View reviewed changes

templates/partials/answers/address.html Outdated Show resolved Hide resolved

MebinAbraham and others added 3 commits September 3, 2020 13:05

Address comment

1168a47

Merge remote-tracking branch 'origin/master' into address-answer-type

4417f38

Revert scripts/run_validator.sh

47f71b1

ajmaddaford reviewed Sep 3, 2020

View reviewed changes

MebinAbraham added 3 commits September 3, 2020 13:52

Add optional address answer

05d9b2e

Merge branch 'address-answer-type' of github.com:ONSdigital/eq-questi…

8d95d89

…onnaire-runner into address-answer-type

Merge master and resolve conflicts

6fd61b4

ajmaddaford approved these changes Sep 3, 2020

View reviewed changes

MebinAbraham merged commit 783f102 into master Sep 3, 2020

MebinAbraham deleted the address-answer-type branch September 3, 2020 15:52

pricem14pc added this to the v3.48.0 milestone Sep 11, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce Address answer type #261

Introduce Address answer type #261

MebinAbraham commented Aug 27, 2020 •

edited

Loading

MebinAbraham Aug 27, 2020

JamesGardiner Aug 28, 2020

MebinAbraham Sep 1, 2020 •

edited

Loading

JamesGardiner Sep 1, 2020

MebinAbraham commented Sep 1, 2020 •

edited

Loading

LJBabbage left a comment

LJBabbage Sep 1, 2020

MebinAbraham Sep 1, 2020

LJBabbage Sep 1, 2020

MebinAbraham Sep 1, 2020

LJBabbage Sep 1, 2020 •

edited

Loading

MebinAbraham Sep 1, 2020

ajmaddaford left a comment

MebinAbraham commented Sep 3, 2020


		from wtforms import Form, FormField, StringField

		logger = logging.getLogger(__name__)

Introduce Address answer type #261

Introduce Address answer type #261

Conversation

MebinAbraham commented Aug 27, 2020 • edited Loading

What is the context of this PR?

How to review

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MebinAbraham Sep 1, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MebinAbraham commented Sep 1, 2020 • edited Loading

LJBabbage left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LJBabbage Sep 1, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ajmaddaford left a comment

Choose a reason for hiding this comment

MebinAbraham commented Sep 3, 2020

MebinAbraham commented Aug 27, 2020 •

edited

Loading

MebinAbraham Sep 1, 2020 •

edited

Loading

MebinAbraham commented Sep 1, 2020 •

edited

Loading

LJBabbage Sep 1, 2020 •

edited

Loading