Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Request IR by Post from Hub #238

Merged
merged 18 commits into from
Aug 20, 2020
Merged

Request IR by Post from Hub #238

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
d9af537
Add response from hub
iwootten Aug 10, 2020
4656daa
Reinsert accidental condition removal
iwootten Aug 14, 2020
bff1c34
Update against feedback
iwootten Aug 17, 2020
f0e3a87
Merge branch 'master' into request-hub-post-response
iwootten Aug 17, 2020
9e7bdab
Remove unneccessary condition
iwootten Aug 17, 2020
0365cb4
Fix previous routing
iwootten Aug 17, 2020
84b86ad
Make tests more explicit
iwootten Aug 17, 2020
dd5e376
Merge branch 'master' into request-hub-post-response
iwootten Aug 18, 2020
fae5c27
Update templates/hub.html
iwootten Aug 19, 2020
860a4d6
Prevent redirect by moving into previous stage
iwootten Aug 19, 2020
bf038c8
Use journey arg over return_to
iwootten Aug 19, 2020
693ee5e
Merge branch 'master' into request-hub-post-response
iwootten Aug 19, 2020
83921be
Merge branch 'master' into request-hub-post-response
iwootten Aug 19, 2020
da5ad48
Add non_primary property to list store
iwootten Aug 20, 2020
62853c6
Don't call .keys()
iwootten Aug 20, 2020
67846eb
Update app/data_model/list_store.py
iwootten Aug 20, 2020
f94cf74
Update to use new property
iwootten Aug 20, 2020
8187657
Flip fallback previous url
iwootten Aug 20, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions app/questionnaire/questionnaire_schema.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,9 @@ def get_driving_question_for_list(cls, section, list_name):
):
return block

def get_individual_response_list(self):
return self.json.get("individual_response", {}).get("for_list")

def get_title_for_section(self, section_id):
return self._sections_by_id.get(section_id).get("title")

Expand Down
60 changes: 42 additions & 18 deletions app/routes/individual_response.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
IndividualResponseHowHandler,
IndividualResponsePostAddressConfirmHandler,
IndividualResponseChangeHandler,
IndividualResponseWhoHandler,
)

logger = get_logger()
Expand Down Expand Up @@ -53,6 +54,7 @@ def before_individual_response_request():
def request_individual_response(schema, questionnaire_store):
language_code = get_session_store().session_data.language_code
list_item_id = request.args.get("list_item_id")

individual_response_handler = IndividualResponseHandler(
block_definition=None,
schema=schema,
Expand All @@ -63,10 +65,10 @@ def request_individual_response(schema, questionnaire_store):
list_item_id=list_item_id,
)

if request.method == "GET":
return individual_response_handler.handle_get()
if request.method == "POST":
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't immediate see a reason for it to change to POST, I am sure there must be. I only raise it as in other routes we usually do GET first

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default fallback option should really always be the call to the get. The only other place I can see this is 3 cases in questionnaire.py, where the logic on the get condition is confusing because it involves a call to the post form. They should probably be changed too.

return individual_response_handler.handle_post()

return redirect(url_for(".get_individual_response_how", list_item_id=list_item_id))
return individual_response_handler.handle_get()


@individual_response_blueprint.route("/<list_item_id>/how", methods=["GET", "POST"])
Expand All @@ -75,6 +77,7 @@ def request_individual_response(schema, questionnaire_store):
@with_schema
def get_individual_response_how(schema, questionnaire_store, list_item_id):
language_code = get_session_store().session_data.language_code

individual_response_handler = IndividualResponseHowHandler(
schema=schema,
questionnaire_store=questionnaire_store,
Expand All @@ -84,10 +87,10 @@ def get_individual_response_how(schema, questionnaire_store, list_item_id):
list_item_id=list_item_id,
)

if request.method == "GET" or not individual_response_handler.form.validate():
return individual_response_handler.handle_get()
if request.method == "POST" and individual_response_handler.form.validate():
return individual_response_handler.handle_post()

return individual_response_handler.handle_post()
return individual_response_handler.handle_get()


@individual_response_blueprint.route("/<list_item_id>/change", methods=["GET", "POST"])
Expand All @@ -105,10 +108,10 @@ def get_individual_response_change(schema, questionnaire_store, list_item_id):
list_item_id=list_item_id,
)

if request.method == "GET" or not individual_response_handler.form.validate():
return individual_response_handler.handle_get()
if request.method == "POST" and individual_response_handler.form.validate():
return individual_response_handler.handle_post()

return individual_response_handler.handle_post()
return individual_response_handler.handle_get()


@individual_response_blueprint.route(
Expand All @@ -130,10 +133,10 @@ def get_individual_response_post_address_confirm(
list_item_id=list_item_id,
)

if request.method == "GET" or not individual_response_handler.form.validate():
return individual_response_handler.handle_get()
if request.method == "POST" and individual_response_handler.form.validate():
return individual_response_handler.handle_post()

return individual_response_handler.handle_post()
return individual_response_handler.handle_get()


@individual_response_blueprint.route("/post/confirmation", methods=["GET", "POST"])
Expand All @@ -152,10 +155,31 @@ def get_individual_response_post_address_confirmation(schema, questionnaire_stor
list_item_id=None,
)

if request.method == "GET":
return render_template(
template="individual_response/confirmation",
display_address=questionnaire_store.metadata.get("display_address"),
)
if request.method == "POST":
return redirect(url_for("questionnaire.get_questionnaire"))

return render_template(
template="individual_response/confirmation",
display_address=questionnaire_store.metadata.get("display_address"),
)


@individual_response_blueprint.route("/who", methods=["GET", "POST"])
Copy link
Contributor

@MebinAbraham MebinAbraham Aug 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous link for this journey should back to the previous page. It should only go to the hub when on you are back on the IR request launcher page.

Scenarios:

  • 1 non-primary person only:

    • The previous link on the how page is also adding list item id to the url which is taking it back to the individual section from IR request launcher page instead of the hub. Regardless of the number of people, the previous should go back to the hub if we know they came from the hub?.

  • at least 2 non-primary person

    • The previous link from any IR page is going straight to the hub, instead of back sequentially to the IR request launcher page then the hub.

Hopefully that is clear, if not, happy to discuss further.

Copy link
Contributor Author

@iwootten iwootten Aug 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the card has overstated this somewhat which has made it confusing "Previous link should always go back to hub no matter how far along the IR journey you are (ensure alternate journey goes to individual section introduction page)."

In all honesty, I'm not sure why the this requirement was specified as it would be the expected behaviour?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that comment has overstated it @iwootten - I think it was added to differentiate between the hub originated journey and the individual section originated journey, but that would be default behaviour as you pointed out 😄

@login_required
@with_questionnaire_store
@with_schema
def get_individual_response_who(schema, questionnaire_store):
language_code = get_session_store().session_data.language_code

individual_response_handler = IndividualResponseWhoHandler(
schema=schema,
questionnaire_store=questionnaire_store,
language=language_code,
request_args=request.args,
form_data=request.form,
)

if request.method == "POST" and individual_response_handler.form.validate():
return individual_response_handler.handle_post()

return redirect(url_for("questionnaire.get_questionnaire"))
return individual_response_handler.handle_get()
48 changes: 26 additions & 22 deletions app/translations/messages.pot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ msgid ""
msgstr ""
"Project-Id-Version: PROJECT VERSION\n"
"Report-Msgid-Bugs-To: EMAIL@ADDRESS\n"
"POT-Creation-Date: 2020-08-13 21:29+0100\n"
"POT-Creation-Date: 2020-08-17 10:07+0100\n"
"PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
"Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
"Language-Team: LANGUAGE <[email protected]>\n"
Expand Down Expand Up @@ -273,71 +273,71 @@ msgstr ""
msgid " (You)"
msgstr ""

#: app/views/handlers/individual_response.py:177
#: app/views/handlers/individual_response.py:187
msgid "How would you like <em>{person_name}</em> to receive a separate census?"
msgstr ""

#: app/views/handlers/individual_response.py:183
#: app/views/handlers/individual_response.py:193
msgid ""
"For someone to complete a separate census, we need to send them an "
"individual access code."
msgstr ""

#: app/views/handlers/individual_response.py:186
#: app/views/handlers/individual_response.py:196
msgid "Select how to send access code"
msgstr ""

#: app/views/handlers/individual_response.py:196
#: app/views/handlers/individual_response.py:206
msgid "Post"
msgstr ""

#: app/views/handlers/individual_response.py:198
#: app/views/handlers/individual_response.py:208
msgid ""
"We can only send this to an unnamed resident at the registered household "
"address"
msgstr ""

#: app/views/handlers/individual_response.py:265
#: app/views/handlers/individual_response.py:282
msgid "How would you like to answer <em>{person_name_possessive}</em> questions?"
msgstr ""

#: app/views/handlers/individual_response.py:278
#: app/views/handlers/individual_response.py:295
msgid "I would like to request a separate census for them to complete"
msgstr ""

#: app/views/handlers/individual_response.py:284
#: app/views/handlers/individual_response.py:301
msgid "I will ask them to answer their own questions"
msgstr ""

#: app/views/handlers/individual_response.py:288
#: app/views/handlers/individual_response.py:305
msgid "They will need the household access code from the letter we sent you"
msgstr ""

#: app/views/handlers/individual_response.py:294
#: app/views/handlers/individual_response.py:311
msgid "I will answer for {person_name}"
msgstr ""

#: app/views/handlers/individual_response.py:405
#: app/views/handlers/individual_response.py:424
msgid "Do you want to send an individual access code for {person_name} by post?"
msgstr ""

#: app/views/handlers/individual_response.py:411
#: app/views/handlers/individual_response.py:430
msgid ""
"A letter with an individual access code will be sent to your registered "
"household address"
msgstr ""

#: app/views/handlers/individual_response.py:418
#: app/views/handlers/individual_response.py:437
msgid ""
"The letter will be addressed to <strong>Individual Resident</strong> "
"instead of the name provided"
msgstr ""

#: app/views/handlers/individual_response.py:431
#: app/views/handlers/individual_response.py:450
msgid "Yes, send the access code by post"
msgstr ""

#: app/views/handlers/individual_response.py:435
#: app/views/handlers/individual_response.py:454
msgid "No, send it another way"
msgstr ""

Expand Down Expand Up @@ -402,6 +402,14 @@ msgstr ""
msgid "Submitting"
msgstr ""

#: templates/hub.html:39
msgid "If you can’t answer someone else’s questions"
msgstr ""

#: templates/interstitial.html:23
msgid "If you can’t answer questions for this person"
msgstr ""

#: templates/introduction.html:15
#, python-format
msgid "You are completing this for <span>%(ru_name)s</span> (%(trading_as_name)s)"
Expand Down Expand Up @@ -693,16 +701,12 @@ msgid "Interviewer note:"
msgstr ""

#: templates/partials/answer-guidance.html:18
#: templates/partials/individual-response-guidance.html:15
#: templates/partials/individual-response-guidance.html:13
#: templates/partials/question-definition.html:19
msgid "Hide this"
msgstr ""

#: templates/partials/individual-response-guidance.html:3
msgid "If you can’t answer questions for this person"
msgstr ""

#: templates/partials/individual-response-guidance.html:26
#: templates/partials/individual-response-guidance.html:24
msgid ""
"You can <em>share your household access code</em> with the people you "
"live with so they can complete their own sections."
Expand Down
29 changes: 18 additions & 11 deletions app/views/contexts/hub_context.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
from functools import cached_property
from typing import List, Mapping, Union

from flask import url_for
from flask_babel import lazy_gettext

from app.data_model.progress_store import CompletionStatus
from app.questionnaire import QuestionnaireSchema
from app.views.contexts import Context


Expand Down Expand Up @@ -60,12 +60,11 @@ def get_context(self, survey_complete, enabled_section_ids) -> Mapping:
submit_button = lazy_gettext("Continue")
guidance = None
warning = None
individual_response_enabled = self._individual_response_enabled(
self._schema
)
individual_response_enabled = self._individual_response_enabled

return {
"individual_response_enabled": individual_response_enabled,
"individual_response_url": self._individual_response_url,
"guidance": guidance,
"rows": rows,
"submit_button": submit_button,
Expand Down Expand Up @@ -175,18 +174,26 @@ def _get_rows(self, enabled_section_ids) -> List[Mapping[str, Union[str, List]]]

return rows

def _individual_response_enabled(self, schema: QuestionnaireSchema) -> bool:
if not schema.json.get("individual_response"):
@cached_property
def _individual_response_enabled(self) -> bool:
if not self._schema.json.get("individual_response"):
return False

for_list = schema.json["individual_response"]["for_list"]
for_list = self._schema.json["individual_response"]["for_list"]

count_household_members = len(self._list_store[for_list])

if count_household_members == 0:
return False

if count_household_members == 1 and self._list_store[for_list].primary_person:
if count_household_members == 0 or (
count_household_members == 1 and self._list_store[for_list].primary_person
):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can now be simplied to:

if not self._list_store[for_list].non_primary_people:
	return False

return False

return True

@cached_property
def _individual_response_url(self) -> Union[str, None]:
if self._individual_response_enabled:
return url_for(
"individual_response.request_individual_response", return_to="hub"
)
return None
Loading