The GDS Way™[1] recommends using Flake8 to lint Python projects.
This commit takes the Flake8 config from Digital Marketplace API[2] and
removes the bits we don’t need.
It changes the `max_complexity` setting to 14, which is the most complex
code we have in this repo currently (we shouldn’t be writing code _more_
complex than what we already have).
This commit also fixes the errors found by Flake8, which includes 6(!)
tests which were never getting run because they had the same names as
existing tests.
Here is a full list of the errors that were found and fixed:
```
./app/__init__.py:2:1: F401 're' imported but unused
./app/__init__.py:4:1: F401 'json' imported but unused
./app/__init__.py:8:1: F401 'dateutil' imported but unused
./app/__init__.py:11:1: F401 'flask.escape' imported but unused
./app/__init__.py:41:1: F401 'app.proxy_fix' imported but unused
./app/__init__.py:129:5: F821 undefined name 'proxy_fix'
./app/__init__.py:221:19: F821 undefined name 'highlight'
./app/__init__.py:221:35: F821 undefined name 'JavascriptLexer'
./app/__init__.py:221:54: F821 undefined name 'HtmlFormatter'
./app/config.py:2:1: F401 'datetime.timedelta' imported but unused
./app/event_handlers.py:2:1: F401 'flask_login.current_user' imported but unused
./app/utils.py:11:1: F401 'dateutil.parser' imported but unused
./app/main/__init__.py:5:1: F401 'app.main.views.two_factor' imported but unused
./app/main/__init__.py:5:1: F401 'app.main.views.notifications' imported but unused
./app/main/__init__.py:5:1: F401 'app.main.views.add_service' imported but unused
./app/main/__init__.py:5:1: F401 'app.main.views.forgot_password' imported but unused
./app/main/__init__.py:5:1: F401 'app.main.views.inbound_number' imported but unused
./app/main/__init__.py:5:1: F401 'app.main.views.styleguide' imported but unused
./app/main/__init__.py:5:1: F401 'app.main.views.organisations' imported but unused
./app/main/__init__.py:5:1: F401 'app.main.views.letter_jobs' imported but unused
./app/main/__init__.py:5:1: F401 'app.main.views.verify' imported but unused
./app/main/__init__.py:5:1: F401 'app.main.views.conversation' imported but unused
./app/main/__init__.py:5:1: F401 'app.main.views.api_keys' imported but unused
./app/main/__init__.py:5:1: F401 'app.main.views.send' imported but unused
./app/main/__init__.py:5:1: F401 'app.main.views.dashboard' imported but unused
./app/main/__init__.py:5:1: F401 'app.main.views.jobs' imported but unused
./app/main/__init__.py:5:1: F401 'app.main.views.manage_users' imported but unused
./app/main/__init__.py:5:1: F401 'app.main.views.sign_in' imported but unused
./app/main/__init__.py:5:1: F401 'app.main.views.sign_out' imported but unused
./app/main/__init__.py:5:1: F401 'app.main.views.code_not_received' imported but unused
./app/main/__init__.py:5:1: F401 'app.main.views.invites' imported but unused
./app/main/__init__.py:5:1: F401 'app.main.views.platform_admin' imported but unused
./app/main/__init__.py:5:1: F401 'app.main.views.providers' imported but unused
./app/main/__init__.py:5:1: F401 'app.main.views.service_settings' imported but unused
./app/main/__init__.py:5:1: F401 'app.main.views.index' imported but unused
./app/main/__init__.py:5:1: F401 'app.main.views.new_password' imported but unused
./app/main/__init__.py:5:1: F401 'app.main.views.user_profile' imported but unused
./app/main/__init__.py:5:1: F401 'app.main.views.feedback' imported but unused
./app/main/__init__.py:5:1: F401 'app.main.views.choose_service' imported but unused
./app/main/__init__.py:5:1: F401 'app.main.views.templates' imported but unused
./app/main/__init__.py:5:1: F401 'app.main.views.register' imported but unused
./app/main/forms.py:12:1: F401 'wtforms.SelectField' imported but unused
./app/main/views/api_keys.py:37:29: E241 multiple spaces after ':'
./app/main/views/feedback.py:3:1: F401 'flask.flash' imported but unused
./app/main/views/feedback.py:122:17: E123 closing bracket does not match indentation of opening bracket's line
./app/main/views/inbound_number.py:1:1: F401 'flask.url_for' imported but unused
./app/main/views/inbound_number.py:1:1: F401 'flask.session' imported but unused
./app/main/views/inbound_number.py:1:1: F401 'flask.redirect' imported but unused
./app/main/views/inbound_number.py:1:1: F401 'flask.request' imported but unused
./app/main/views/inbound_number.py:13:1: F401 'flask.jsonify' imported but unused
./app/main/views/jobs.py:31:1: F401 'app.utils.get_template' imported but unused
./app/main/views/letter_jobs.py:1:1: F401 'datetime' imported but unused
./app/main/views/letter_jobs.py:6:1: F401 'app.format_datetime_24h' imported but unused
./app/main/views/manage_users.py:111:9: E123 closing bracket does not match indentation of opening bracket's line
./app/main/views/notifications.py:121:5: F841 local variable 'status_args' is assigned to but never used
./app/main/views/organisations.py:1:1: F401 'flask.request' imported but unused
./app/main/views/service_settings.py:77:9: E123 closing bracket does not match indentation of opening bracket's line
./app/main/views/service_settings.py:82:9: E123 closing bracket does not match indentation of opening bracket's line
./app/main/views/service_settings.py:420:13: E123 closing bracket does not match indentation of opening bracket's line
./app/main/views/sign_in.py:12:1: F401 'flask_login.confirm_login' imported but unused
./app/main/views/sign_in.py:17:1: F401 'app.service_api_client' imported but unused
./app/main/views/sign_in.py:62:13: E123 closing bracket does not match indentation of opening bracket's line
./app/main/views/templates.py:4:1: F401 'flask.json' imported but unused
./app/main/views/templates.py:17:1: F401 'notifications_utils.formatters.escape_html' imported but unused
./app/main/views/templates.py:23:1: F401 'app.utils.get_help_argument' imported but unused
./app/main/views/templates.py:64:13: E123 closing bracket does not match indentation of opening bracket's line
./app/notify_client/service_api_client.py:6:1: F401 '.notification_api_client' imported but unused
./app/notify_client/user_api_client.py:1:1: F401 'uuid' imported but unused
./app/notify_client/user_api_client.py:3:1: F401 'flask.session' imported but unused
./tests/__init__.py:1:1: F401 'csv' imported but unused
./tests/app/main/test_asset_fingerprinter.py:2:1: F401 'os' imported but unused
./tests/app/main/test_asset_fingerprinter.py:4:1: F401 'unittest.mock' imported but unused
./tests/app/main/test_asset_fingerprinter.py:98:9: F841 local variable 'string_with_unicode_character' is assigned to but never used
./tests/app/main/test_errorhandlers.py:2:1: F401 'flask.url_for' imported but unused
./tests/app/main/test_permissions.py:26:13: F841 local variable 'response' is assigned to but never used
./tests/app/main/test_placeholder_form.py:3:1: F401 'wtforms.Label' imported but unused
./tests/app/main/test_placeholder_form.py:11:10: F841 local variable 'req' is assigned to but never used
./tests/app/main/test_two_factor_form.py:10:67: F841 local variable 'req' is assigned to but never used
./tests/app/main/test_two_factor_form.py:23:65: F841 local variable 'req' is assigned to but never used
./tests/app/main/test_two_factor_form.py:37:48: F841 local variable 'req' is assigned to but never used
./tests/app/main/test_two_factor_form.py:51:67: F841 local variable 'req' is assigned to but never used
./tests/app/main/test_two_factor_form.py:65:67: F841 local variable 'req' is assigned to but never used
./tests/app/main/views/test_accept_invite.py:356:5: F841 local variable 'element' is assigned to but never used
./tests/app/main/views/test_activity.py:11:1: F811 redefinition of unused 'mock_get_notifications' from line 11
./tests/app/main/views/test_activity.py:18:1: F401 'datetime.datetime' imported but unused
./tests/app/main/views/test_activity.py:102:5: F841 local variable 'content' is assigned to but never used
./tests/app/main/views/test_activity.py:104:5: F841 local variable 'notification' is assigned to but never used
./tests/app/main/views/test_activity.py:337:5: F841 local variable '_notifications_mock' is assigned to but never used
./tests/app/main/views/test_activity.py:373:13: E126 continuation line over-indented for hanging indent
./tests/app/main/views/test_activity.py:378:9: E121 continuation line under-indented for hanging indent
./tests/app/main/views/test_activity.py:404:13: E126 continuation line over-indented for hanging indent
./tests/app/main/views/test_activity.py:407:9: E121 continuation line under-indented for hanging indent
./tests/app/main/views/test_api_keys.py:354:5: F841 local variable 'response' is assigned to but never used
./tests/app/main/views/test_conversation.py:5:1: F401 'bs4.BeautifulSoup' imported but unused
./tests/app/main/views/test_conversation.py:198:5: F841 local variable 'mock_get_inbound_sms' is assigned to but never used
./tests/app/main/views/test_dashboard.py:53:5: F841 local variable 'mock_template_stats' is assigned to but never used
./tests/app/main/views/test_dashboard.py:72:5: F841 local variable 'mock_template_stats' is assigned to but never used
./tests/app/main/views/test_jobs.py:2:1: F401 'uuid' imported but unused
./tests/app/main/views/test_jobs.py:3:1: F401 'urllib.parse.urlparse' imported but unused
./tests/app/main/views/test_jobs.py:3:1: F401 'urllib.parse.quote' imported but unused
./tests/app/main/views/test_jobs.py:3:1: F401 'urllib.parse.parse_qs' imported but unused
./tests/app/main/views/test_jobs.py:9:1: F401 'app.main.views.jobs.get_status_filters' imported but unused
./tests/app/main/views/test_jobs.py:10:1: F401 'tests.notification_json' imported but unused
./tests/app/main/views/test_letters.py:6:1: F401 'tests.service_json' imported but unused
./tests/app/main/views/test_notifications.py:5:1: F401 'app.utils.REQUESTED_STATUSES' imported but unused
./tests/app/main/views/test_notifications.py:5:1: F401 'app.utils.DELIVERED_STATUSES' imported but unused
./tests/app/main/views/test_notifications.py:5:1: F401 'app.utils.SENDING_STATUSES' imported but unused
./tests/app/main/views/test_notifications.py:5:1: F401 'app.utils.FAILURE_STATUSES' imported but unused
./tests/app/main/views/test_platform_admin.py:242:13: E126 continuation line over-indented for hanging indent
./tests/app/main/views/test_platform_admin.py:247:13: E126 continuation line over-indented for hanging indent
./tests/app/main/views/test_send.py:3:1: F401 'unittest.mock.Mock' imported but unused
./tests/app/main/views/test_send.py:18:1: F811 redefinition of unused 'mock_get_service' from line 18
./tests/app/main/views/test_send.py:18:1: F401 'tests.conftest.multiple_letter_contact_blocks' imported but unused
./tests/app/main/views/test_send.py:18:1: F401 'tests.conftest.no_sms_senders' imported but unused
./tests/app/main/views/test_send.py:18:1: F401 'tests.conftest.multiple_sms_senders' imported but unused
./tests/app/main/views/test_send.py:18:1: F401 'tests.conftest.no_letter_contact_blocks' imported but unused
./tests/app/main/views/test_send.py:102:5: F841 local variable 'response' is assigned to but never used
./tests/app/main/views/test_send.py:870:5: F841 local variable 'response' is assigned to but never used
./tests/app/main/views/test_send.py:1367:5: F841 local variable 'service_id' is assigned to but never used
./tests/app/main/views/test_send.py:1451:13: E126 continuation line over-indented for hanging indent
./tests/app/main/views/test_send.py:1620:80: E226 missing whitespace around arithmetic operator
./tests/app/main/views/test_send.py:1909:13: E126 continuation line over-indented for hanging indent
./tests/app/main/views/test_send.py:1912:9: E121 continuation line under-indented for hanging indent
./tests/app/main/views/test_service_settings.py:13:1: F811 redefinition of unused 'no_reply_to_email_addresses' from line 13
./tests/app/main/views/test_service_settings.py:13:1: F401 'tests.conftest.single_reply_to_email_address' imported but unused
./tests/app/main/views/test_service_settings.py:28:5: E123 closing bracket does not match indentation of opening bracket's line
./tests/app/main/views/test_service_settings.py:104:1: F811 redefinition of unused 'single_reply_to_email_address' from line 13
./tests/app/main/views/test_service_settings.py:166:1: F811 redefinition of unused 'single_reply_to_email_address' from line 13
./tests/app/main/views/test_service_settings.py:186:5: F841 local variable 'mocked_get_fn' is assigned to but never used
./tests/app/main/views/test_service_settings.py:217:1: F811 redefinition of unused 'single_reply_to_email_address' from line 13
./tests/app/main/views/test_service_settings.py:237:1: F811 redefinition of unused 'single_reply_to_email_address' from line 13
./tests/app/main/views/test_service_settings.py:257:1: F811 redefinition of unused 'single_reply_to_email_address' from line 13
./tests/app/main/views/test_service_settings.py:307:1: F811 redefinition of unused 'single_reply_to_email_address' from line 13
./tests/app/main/views/test_service_settings.py:340:1: F811 redefinition of unused 'single_reply_to_email_address' from line 13
./tests/app/main/views/test_service_settings.py:466:1: F811 redefinition of unused 'single_reply_to_email_address' from line 13
./tests/app/main/views/test_service_settings.py:555:1: F811 redefinition of unused 'single_reply_to_email_address' from line 13
./tests/app/main/views/test_service_settings.py:615:1: F811 redefinition of unused 'single_reply_to_email_address' from line 13
./tests/app/main/views/test_service_settings.py:719:1: F811 redefinition of unused 'single_reply_to_email_address' from line 13
./tests/app/main/views/test_service_settings.py:874:5: F841 local variable 'page' is assigned to but never used
./tests/app/main/views/test_service_settings.py:902:5: F841 local variable 'page' is assigned to but never used
./tests/app/main/views/test_service_settings.py:954:5: F841 local variable 'page' is assigned to but never used
./tests/app/main/views/test_service_settings.py:986:5: F841 local variable 'page' is assigned to but never used
./tests/app/main/views/test_service_settings.py:1101:1: F811 redefinition of unused 'single_reply_to_email_address' from line 13
./tests/app/main/views/test_service_settings.py:1121:1: F811 redefinition of unused 'single_reply_to_email_address' from line 13
./tests/app/main/views/test_service_settings.py:1271:1: F811 redefinition of unused 'test_set_letter_contact_block_saves' from line 1189
./tests/app/main/views/test_service_settings.py:1433:5: F841 local variable 'page' is assigned to but never used
./tests/app/main/views/test_service_settings.py:1495:5: F841 local variable 'mocked_get_fn' is assigned to but never used
./tests/app/main/views/test_service_settings.py:1540:5: F841 local variable 'mocked_get_fn' is assigned to but never used
./tests/app/main/views/test_service_settings.py:1570:1: F811 redefinition of unused 'single_reply_to_email_address' from line 13
./tests/app/main/views/test_service_settings.py:1589:1: F811 redefinition of unused 'single_reply_to_email_address' from line 13
./tests/app/main/views/test_service_settings.py:1621:1: F811 redefinition of unused 'single_reply_to_email_address' from line 13
./tests/app/main/views/test_service_settings.py:1641:1: F811 redefinition of unused 'single_reply_to_email_address' from line 13
./tests/app/main/views/test_service_settings.py:1658:1: F811 redefinition of unused 'single_reply_to_email_address' from line 13
./tests/app/main/views/test_service_settings.py:1676:1: F811 redefinition of unused 'single_reply_to_email_address' from line 13
./tests/app/main/views/test_service_settings.py:1697:1: F811 redefinition of unused 'single_reply_to_email_address' from line 13
./tests/app/main/views/test_service_settings.py:1759:1: F811 redefinition of unused 'single_reply_to_email_address' from line 13
./tests/app/main/views/test_service_settings.py:1775:1: F811 redefinition of unused 'single_reply_to_email_address' from line 13
./tests/app/main/views/test_templates.py:3:1: F401 'uuid' imported but unused
./tests/app/main/views/test_templates.py:11:1: F401 'tests.conftest.mock_get_user' imported but unused
./tests/app/main/views/test_templates.py:514:1: F811 redefinition of unused 'mock_get_user' from line 11
./tests/app/main/views/test_templates.py:672:1: F811 redefinition of unused 'mock_get_user' from line 11
./tests/app/main/views/test_templates.py:795:1: F811 redefinition of unused 'mock_get_user' from line 11
./tests/app/main/views/test_templates.py:835:1: F811 redefinition of unused 'mock_get_user' from line 11
./tests/app/main/views/test_two_factor.py:67:13: E126 continuation line over-indented for hanging indent
./tests/app/notify_client/test_notification_client.py:79:5: F841 local variable 'mock_post' is assigned to but never used
```
1. https://gds-way.cloudapps.digital/manuals/programming-languages/python/linting.html#how-to-use-flake8
2. d5ab8afef4/.flake8
If you’ve picked an email reply to address, it’s good UI design to have
your choice played back to you, so you can be confident it’s worked.
This commit does that by making it part of the email preview.
Uses:
- [ ] https://github.com/alphagov/notifications-utils/pull/227
If you miss ‘postcode’ from your file then you get told that you need
‘address_line_1’, ‘address_line_2’, ‘address_line_3’, etc.
This is incorrect – the only required address columns are lines 1 and 2,
plus the postcode. So this commit corrects the error message to be
factually accurate.
We had a user report this to Fajer as a bug.
It’s useful to be able to play with the CSV upload thing and find out
how it works before you’re interrupted by the error telling you that
you’ve got as far as you can in trial mode.
We don’t want users in trial mode sending real letters. So we’ve
introduced an error message. This error message is also showing up when
users in trial mode and making a test letter (and having the knock on
effect of hiding the download button).
They should be able to make a test letter in trial mode, because it
doesn’t cost anything.
Telling users that they can’t send to more than 50 recipients in trial
mode doesn’t apply for letters (they can’t send to _any_ recipients).
So we should make sure that the error message about not being able to
send to any recipients always comes up instead of the 50 recipients one,
whether you’re trying to upload a file with 1 or 111 rows.
Users in trial mode haven’t signed the MOU. This means that they haven’t
agreed to pay for any costs they incur.
Unlike text messages and emails, we don’t give you any free allowance of
letters. Sending _any_ letters will cost the user money.
Therefore we shouldn’t let users who haven’t agreed that they will pay
for the service to incur costs by sending letters.
The pattern used for this is roughly the same as other trial mode errors
that we have already, ie a red box that says you’re not allowed. Not
sure if this is exactly right because it’s not exactly an error so the
pattern might feel too heavy-handed.
Getting this in place means we can turn letters on for users in trial
mode without worrying that they’ll accidentally send real letters, which
would result in:
- us having to absorb those costs
- some awkward conversations
This was causing a 500 in production.
This commit:
- reverts the code the working state it was before 68a1426e58
- figures out a way to make the tests pass without breaking the actual
app
- confirms that mocking things is hard
When I split up the error messages on the check CSV page into multiple
templates, I also reduced the repetition of wrapping `<div>`s and macro
calls by moving them up outside the conditional blocks (see
8e947f315d).
Unfortunately I didn’t make the same adjustments for the one-off flow,
which meant that errors on these pages lost their styling. This commit
re-adds the styling for these error messages.
This was causing a 500 because of a bug in utils. It only occurred when
there was missing data for a cell in the recipient column. I’ve also
added a test for missing data in a non-recipient column just in case.
There’s no immediate feedback with letter jobs, unlike email or text
messages jobs where you see the numbers starting to tick over straight
away.
We need to reassure the user that the thing they asked us to do (send
letters) is underway. ‘Printing’ feels like the natural first state of
the letter-making process. So this commit adds a banner to tell the
user that printing is the thing that’s happening.
If sending SMS is disabled for a service, it should not be possible to
add or modify SMS templates. If a user tries to do this, they should see
a different page with a link to go back. The same thing should happen
with email templates.
`test_utils.py` is actually a file for testing some utils, not
some utils for testing.
Test helper functions and fixtures are better placed in conftest,
and also means that we can use them in conftest.
The following errors may happen:
* Number outside of service if service in trial mode
* Message too long for sms
* Service over daily limit
We need to handle these. They only return on send, rather than in a
separate validation step (for now).
‘One-off’ is a bit wooly. Feels like our name for the thing. ‘Send to
one recipient’ matches ‘Upload recipients’.
This also means making the `<h1>` on job page ‘Report’ for one-off
messages. It doesn’t make sense to call the feature ‘send to one
recipient’ when we’re not using the language of one-off any more.
It’s a confusing proposition to have two features which are almost
identical. Even differentiating between them in the template menu would
be tricky.
I think the better thing to do is rename the whole feature to ‘send
one-off message’.
Then if someone wants to use there own phone number or email address,
give them a quick shortcut to doing that, once they’re in the flow.
In the background this reuses the ‘send yourself a test’ code, but
the user is never aware that they’re going through a different route
to send an individual message. So the proposition stays nice and clean.
We have some fairly complicated nested if statements in our Jinja that
decide what the page titles should be. It’s only going to get more
complicated with the send individual message routes.
So this commit:
- moves the logic from Jinja to Python
- adds tests to check things are working as expected
- sets the page titles to the right thing for each flow
This commit adds an extra, initial, step to the ‘send one-off message’
flow to ask for a phone number or email address. This is the first pass
at making a feature which caseworkers or similar could use Notify to
send individual messages while they’re working a case.
This commit adds a route which is identical to send yourself a test, but
with its own endpoint. This will let us add a slightly different
‘send a one-off message’ flow. This commit just adds the route though,
and makes sure that the tests pass for both routes.
Because we put the step in the URL, users could:
- skip ahead to a later step
- navigate to a step which doesn’t exist (ie an index greater than the
number of placeholders)
This commit adds some checks to do the sensible thing in the unlikely
event that either of these situations occur.
