darkhelm/notifications-admin
1
0
Fork 0
mirror of https://github.com/GSA/notifications-admin.git synced 2026-08-16 20:49:00 -04:00
Code Issues Packages Projects Releases Wiki Activity
Files
33d074c00aaf330b105a81a1bb39c3a6df62e7d1
notifications-admin/tests/app/main/views/test_manage_users.py

1462 lines
46 KiB
Python
Raw Normal View History

Change display of cancelled users, fix edit link It’s confusing showing green ticks for cancelled invites. This commit changes the appearance so that only pending or active users (ie those that could actually do some damage) get green ticks. Also fixes missing edit links caused by instances of `User` having `.state` but instances of `InvitedUser` having `.status`. Right now these are two separate lists. Which makes it harder to add improvements that will make large numbers of users easier to manage.
2018-01-26 17:14:00 +00:00
import copy
Refactor filtering out accepted invites to client None of our model or view layer code should need to know about accepted invites. We don’t use them anywhere because once an invite is accepted that person is now a user. Putting this logic in the client means that: - none of the code calling the client needs to care about accepted invites - it’s easier to (if we want) update the API code to not return accepted invites
2018-11-30 17:39:39 +00:00
import uuid
Enforce order and style of imports Done using isort[1], with the following command: ``` isort -rc ./app ./tests ``` Adds linting to the `run_tests.sh` script to stop badly-sorted imports getting re-introduced. Chosen style is ‘Vertical Hanging Indent’ with trailing commas, because I think it gives the cleanest diffs, eg: ``` from third_party import ( lib1, lib2, lib3, lib4, ) ``` 1. https://pypi.python.org/pypi/isort
2018-02-20 11:22:17 +00:00
Add test to invite nonwhitelist user and refactor
2016-10-25 18:13:50 +01:00
import pytest
Enforce order and style of imports Done using isort[1], with the following command: ``` isort -rc ./app ./tests ``` Adds linting to the `run_tests.sh` script to stop badly-sorted imports getting re-introduced. Chosen style is ‘Vertical Hanging Indent’ with trailing commas, because I think it gives the cleanest diffs, eg: ``` from third_party import ( lib1, lib2, lib3, lib4, ) ``` 1. https://pypi.python.org/pypi/isort
2018-02-20 11:22:17 +00:00
from flask import url_for
define isort first party (app and tests) we were seeing isort produce different outputs locally and in docker - this was due to it having different opinions about whether the tests module (ie all our unit tests) is a first party (local) or third party (pip installed) import. It's a first party import, so by defining this in the setup.cfg isort settings, we can force it to be consistent between environments. Note: I don't know why it was different in the first place though
2018-04-25 14:12:58 +01:00
import app
Split models to prevent circular imports This commit is the first step to disentangling the models from the API clients. With the models in the same folder as the API clients it makes it hard to import the API clients within the model without getting a circular import. After this commit the user API clients still has this problem, but at least the service API client doesn’t.
2018-10-26 15:58:44 +01:00
from app.models.user import InvitedUser
define isort first party (app and tests) we were seeing isort produce different outputs locally and in docker - this was due to it having different opinions about whether the tests module (ie all our unit tests) is a first party (local) or third party (pip installed) import. It's a first party import, so by defining this in the setup.cfg isort settings, we can force it to be consistent between environments. Note: I don't know why it was different in the first place though
2018-04-25 14:12:58 +01:00
from app.utils import is_gov_user
Make existing tests use ``client_request fixture This commit makes the existing tests around user permissions less verbose by using the new `client_request` fixture. This fixture takes care of: - setting up a service - asserting that the response is `200` It also tests that the page titles, some of which didn’t match with the `<h1>`s, so this commit also fixes that mismatch.
2017-08-17 10:44:36 +01:00
from tests.conftest import (
SERVICE_ONE_ID,
Don’t allow editing of users from other services Currently when you load the ‘edit user’ page (which has a URL like `/service/<service_id>/users/<user_id>`) we check that: - you belong to the service represented by `service_id` - you have permission to edit users on this service We don’t check that: - the user represented by `user_id` belongs to this service This means that if you could somehow determine another user’s `user_id` (which I don’t think is possible if you don’t already have the manage service permission for that service) then you could: - edit their permissions on your service (weird, but wouldn’t have any effect) - change their email address (bad) This commit adds checks to return a `404` any time you’re looking at a service and trying to do stuff to a user who doesn’t belong to that service. We can’t add this check to the API easily because there are still times that we want to get/modify users outside the context of a service (eg platform admin pages, or users who have no services).
2019-02-25 16:51:37 +00:00
USER_ONE_ID,
Allow setting of caseworking on a user This commit changes the form that the user sees when inviting or editing another user, if the service has the ‘caseworking’ permission set. This will allow creating a new type of user, one who only has the `send_messages` permission, without the `view_activity` permission. We are doing this because we think there are a number of services with a lot of users who don’t need to see the dashboard, or the other team members, and that we can make a simpler interface for these users.
2018-06-12 14:29:47 +01:00
active_caseworking_user,
Add a ‘See dashboard’ permission Our research and prototyping around ‘basic view’ found that: - a lot of users who send messages rarely or never look at the dashboard (yet it’s the first page they see when they sign in) - team managers like the idea of taking away things that users don’t need in order to make the interface simpler We’ve disentangled the simpler way of sending messages from being part of ‘basic view’. This means we can give managers the option of taking away the dashboard as an independent choice, not something that’s wrapped up in a separate ‘view’. I think that this checkbox is a more straightforward proposition than ‘basic view’ ever was (despite all the work we did to explain it and develop the nested checkbox pattern). In research users would often explain the feature back to us as being about hiding the dashboard – we should try to make Notify operate in terms of concepts that come naturally to people wherever possible.
2018-08-06 11:10:37 +01:00
active_user_empty_permissions,
Add a fourth, ‘manage templates’ permission We’ve seen from research (a long time ago) that the ‘manage service’ permission is too broad, and gives too much control to someone who only needs the ability to edit templates. In other words, editing content should be its own, separate permission, rather than being rolled up into manage service. Since this is already disaggregated on the API side, making this change just means changing the mapping on the admin side and adding an extra checkbox on the invite/edit page. Which is what this commit does. So for now, an existing user who has the manage service permission gets both manage service and manage templates (ie no change to what they can do). Newly invited users will get to choose if they have both, either, or neither.
2017-08-17 11:14:26 +01:00
active_user_manage_template_permission,
Enforce order and style of imports Done using isort[1], with the following command: ``` isort -rc ./app ./tests ``` Adds linting to the `run_tests.sh` script to stop badly-sorted imports getting re-introduced. Chosen style is ‘Vertical Hanging Indent’ with trailing commas, because I think it gives the cleanest diffs, eg: ``` from third_party import ( lib1, lib2, lib3, lib4, ) ``` 1. https://pypi.python.org/pypi/isort
2018-02-20 11:22:17 +00:00
active_user_no_mobile,
active_user_view_permissions,
active_user_with_permissions,
normalize_spaces,
Don’t allow editing of users from other services Currently when you load the ‘edit user’ page (which has a URL like `/service/<service_id>/users/<user_id>`) we check that: - you belong to the service represented by `service_id` - you have permission to edit users on this service We don’t check that: - the user represented by `user_id` belongs to this service This means that if you could somehow determine another user’s `user_id` (which I don’t think is possible if you don’t already have the manage service permission for that service) then you could: - edit their permissions on your service (weird, but wouldn’t have any effect) - change their email address (bad) This commit adds checks to return a `404` any time you’re looking at a service and trying to do stuff to a user who doesn’t belong to that service. We can’t add this check to the API easily because there are still times that we want to get/modify users outside the context of a service (eg platform admin pages, or users who have no services).
2019-02-25 16:51:37 +00:00
sample_uuid,
Make existing tests use ``client_request fixture This commit makes the existing tests around user permissions less verbose by using the new `client_request` fixture. This fixture takes care of: - setting up a service - asserting that the response is `200` It also tests that the page titles, some of which didn’t match with the `<h1>`s, so this commit also fixes that mismatch.
2017-08-17 10:44:36 +01:00
)
Enforce order and style of imports Done using isort[1], with the following command: ``` isort -rc ./app ./tests ``` Adds linting to the `run_tests.sh` script to stop badly-sorted imports getting re-introduced. Chosen style is ‘Vertical Hanging Indent’ with trailing commas, because I think it gives the cleanest diffs, eg: ``` from third_party import ( lib1, lib2, lib3, lib4, ) ``` 1. https://pypi.python.org/pypi/isort
2018-02-20 11:22:17 +00:00
Add pages to invite, edit, and delete users This takes the original prototype version of this page, and, using the same fake data (ie nothing is wired up): - adds an invite users page - adds an edit (and delete) user page Both these pages allow the user to set another user’s permissions. This commit adds images for the ticks and crosses, so we have control over their appearance.
2016-02-19 15:02:13 +00:00
Change display of cancelled users, fix edit link It’s confusing showing green ticks for cancelled invites. This commit changes the appearance so that only pending or active users (ie those that could actually do some damage) get green ticks. Also fixes missing edit links caused by instances of `User` having `.state` but instances of `InvitedUser` having `.status`. Right now these are two separate lists. Which makes it harder to add improvements that will make large numbers of users easier to manage.
2018-01-26 17:14:00 +00:00
@pytest.mark.parametrize('user, expected_self_text, expected_coworker_text', [
Test that the right permissions in page This commit makes sure that the right permission choices are shown in these pages: - manage team page - invite a user page - edit permissions page This is in order to make changing these pages easier (see subsequent commits).
2017-08-17 10:49:21 +01:00
(
active_user_with_permissions,
(
'Test User (you) '
Make the permission labels shorter They were feeling a bit heavy on the _Team members_ page. We can remove some words without losing any meaning.
2018-08-08 14:23:24 +01:00
'Can See dashboard '
'Can Send messages '
Add a ‘See dashboard’ permission Our research and prototyping around ‘basic view’ found that: - a lot of users who send messages rarely or never look at the dashboard (yet it’s the first page they see when they sign in) - team managers like the idea of taking away things that users don’t need in order to make the interface simpler We’ve disentangled the simpler way of sending messages from being part of ‘basic view’. This means we can give managers the option of taking away the dashboard as an independent choice, not something that’s wrapped up in a separate ‘view’. I think that this checkbox is a more straightforward proposition than ‘basic view’ ever was (despite all the work we did to explain it and develop the nested checkbox pattern). In research users would often explain the feature back to us as being about hiding the dashboard – we should try to make Notify operate in terms of concepts that come naturally to people wherever possible.
2018-08-06 11:10:37 +01:00
'Can Add and edit templates '
Make the permission labels shorter They were feeling a bit heavy on the _Team members_ page. We can remove some words without losing any meaning.
2018-08-08 14:23:24 +01:00
'Can Manage settings, team and usage '
Relabel existing permissions Since we have added a new, 5th permission the existing permissions should be relabelled so that the five make sense as a coherent set. We especially want to make sure that: - the labels work against the checkboxes and against the tick/crosses on the manage users page (a long time ago this page was layed out differently so didn’t have space for full labels) - there is no confusion between usage and reports This commit also: - re-adds a line about what all users can see (‘sent messages’) but continues to omit the additional bullet points about templates and team members (because we think this is clear enough from reading the permissions) - refactors the `Form` subclass so that the content and order of the permissions only have to be defined once - brings back the ‘permissions’ legend on the `fieldset`
2018-08-08 08:45:58 +01:00
'Can Manage API integration'
Test that the right permissions in page This commit makes sure that the right permission choices are shown in these pages: - manage team page - invite a user page - edit permissions page This is in order to make changing these pages easier (see subsequent commits).
2017-08-17 10:49:21 +01:00
),
Change display of cancelled users, fix edit link It’s confusing showing green ticks for cancelled invites. This commit changes the appearance so that only pending or active users (ie those that could actually do some damage) get green ticks. Also fixes missing edit links caused by instances of `User` having `.state` but instances of `InvitedUser` having `.status`. Right now these are two separate lists. Which makes it harder to add improvements that will make large numbers of users easier to manage.
2018-01-26 17:14:00 +00:00
(
'ZZZZZZZZ zzzzzzz@example.gov.uk '
Make the permission labels shorter They were feeling a bit heavy on the _Team members_ page. We can remove some words without losing any meaning.
2018-08-08 14:23:24 +01:00
'Can See dashboard '
'Can’t Send messages '
Add a ‘See dashboard’ permission Our research and prototyping around ‘basic view’ found that: - a lot of users who send messages rarely or never look at the dashboard (yet it’s the first page they see when they sign in) - team managers like the idea of taking away things that users don’t need in order to make the interface simpler We’ve disentangled the simpler way of sending messages from being part of ‘basic view’. This means we can give managers the option of taking away the dashboard as an independent choice, not something that’s wrapped up in a separate ‘view’. I think that this checkbox is a more straightforward proposition than ‘basic view’ ever was (despite all the work we did to explain it and develop the nested checkbox pattern). In research users would often explain the feature back to us as being about hiding the dashboard – we should try to make Notify operate in terms of concepts that come naturally to people wherever possible.
2018-08-06 11:10:37 +01:00
'Can’t Add and edit templates '
Make the permission labels shorter They were feeling a bit heavy on the _Team members_ page. We can remove some words without losing any meaning.
2018-08-08 14:23:24 +01:00
'Can’t Manage settings, team and usage '
Relabel existing permissions Since we have added a new, 5th permission the existing permissions should be relabelled so that the five make sense as a coherent set. We especially want to make sure that: - the labels work against the checkboxes and against the tick/crosses on the manage users page (a long time ago this page was layed out differently so didn’t have space for full labels) - there is no confusion between usage and reports This commit also: - re-adds a line about what all users can see (‘sent messages’) but continues to omit the additional bullet points about templates and team members (because we think this is clear enough from reading the permissions) - refactors the `Form` subclass so that the content and order of the permissions only have to be defined once - brings back the ‘permissions’ legend on the `fieldset`
2018-08-08 08:45:58 +01:00
'Can’t Manage API integration '
Content changes following consultation with content and design pros :)
2019-02-20 17:34:58 +00:00
'Edit team member'
Change display of cancelled users, fix edit link It’s confusing showing green ticks for cancelled invites. This commit changes the appearance so that only pending or active users (ie those that could actually do some damage) get green ticks. Also fixes missing edit links caused by instances of `User` having `.state` but instances of `InvitedUser` having `.status`. Right now these are two separate lists. Which makes it harder to add improvements that will make large numbers of users easier to manage.
2018-01-26 17:14:00 +00:00
)
Test that the right permissions in page This commit makes sure that the right permission choices are shown in these pages: - manage team page - invite a user page - edit permissions page This is in order to make changing these pages easier (see subsequent commits).
2017-08-17 10:49:21 +01:00
),
Add a ‘See dashboard’ permission Our research and prototyping around ‘basic view’ found that: - a lot of users who send messages rarely or never look at the dashboard (yet it’s the first page they see when they sign in) - team managers like the idea of taking away things that users don’t need in order to make the interface simpler We’ve disentangled the simpler way of sending messages from being part of ‘basic view’. This means we can give managers the option of taking away the dashboard as an independent choice, not something that’s wrapped up in a separate ‘view’. I think that this checkbox is a more straightforward proposition than ‘basic view’ ever was (despite all the work we did to explain it and develop the nested checkbox pattern). In research users would often explain the feature back to us as being about hiding the dashboard – we should try to make Notify operate in terms of concepts that come naturally to people wherever possible.
2018-08-06 11:10:37 +01:00
(
active_user_empty_permissions,
(
'Test User With Empty Permissions (you) '
Make the permission labels shorter They were feeling a bit heavy on the _Team members_ page. We can remove some words without losing any meaning.
2018-08-08 14:23:24 +01:00
'Can’t See dashboard '
'Can’t Send messages '
Add a ‘See dashboard’ permission Our research and prototyping around ‘basic view’ found that: - a lot of users who send messages rarely or never look at the dashboard (yet it’s the first page they see when they sign in) - team managers like the idea of taking away things that users don’t need in order to make the interface simpler We’ve disentangled the simpler way of sending messages from being part of ‘basic view’. This means we can give managers the option of taking away the dashboard as an independent choice, not something that’s wrapped up in a separate ‘view’. I think that this checkbox is a more straightforward proposition than ‘basic view’ ever was (despite all the work we did to explain it and develop the nested checkbox pattern). In research users would often explain the feature back to us as being about hiding the dashboard – we should try to make Notify operate in terms of concepts that come naturally to people wherever possible.
2018-08-06 11:10:37 +01:00
'Can’t Add and edit templates '
Make the permission labels shorter They were feeling a bit heavy on the _Team members_ page. We can remove some words without losing any meaning.
2018-08-08 14:23:24 +01:00
'Can’t Manage settings, team and usage '
Relabel existing permissions Since we have added a new, 5th permission the existing permissions should be relabelled so that the five make sense as a coherent set. We especially want to make sure that: - the labels work against the checkboxes and against the tick/crosses on the manage users page (a long time ago this page was layed out differently so didn’t have space for full labels) - there is no confusion between usage and reports This commit also: - re-adds a line about what all users can see (‘sent messages’) but continues to omit the additional bullet points about templates and team members (because we think this is clear enough from reading the permissions) - refactors the `Form` subclass so that the content and order of the permissions only have to be defined once - brings back the ‘permissions’ legend on the `fieldset`
2018-08-08 08:45:58 +01:00
'Can’t Manage API integration'
Add a ‘See dashboard’ permission Our research and prototyping around ‘basic view’ found that: - a lot of users who send messages rarely or never look at the dashboard (yet it’s the first page they see when they sign in) - team managers like the idea of taking away things that users don’t need in order to make the interface simpler We’ve disentangled the simpler way of sending messages from being part of ‘basic view’. This means we can give managers the option of taking away the dashboard as an independent choice, not something that’s wrapped up in a separate ‘view’. I think that this checkbox is a more straightforward proposition than ‘basic view’ ever was (despite all the work we did to explain it and develop the nested checkbox pattern). In research users would often explain the feature back to us as being about hiding the dashboard – we should try to make Notify operate in terms of concepts that come naturally to people wherever possible.
2018-08-06 11:10:37 +01:00
),
(
'ZZZZZZZZ zzzzzzz@example.gov.uk '
Make the permission labels shorter They were feeling a bit heavy on the _Team members_ page. We can remove some words without losing any meaning.
2018-08-08 14:23:24 +01:00
'Can See dashboard '
'Can’t Send messages '
Add a ‘See dashboard’ permission Our research and prototyping around ‘basic view’ found that: - a lot of users who send messages rarely or never look at the dashboard (yet it’s the first page they see when they sign in) - team managers like the idea of taking away things that users don’t need in order to make the interface simpler We’ve disentangled the simpler way of sending messages from being part of ‘basic view’. This means we can give managers the option of taking away the dashboard as an independent choice, not something that’s wrapped up in a separate ‘view’. I think that this checkbox is a more straightforward proposition than ‘basic view’ ever was (despite all the work we did to explain it and develop the nested checkbox pattern). In research users would often explain the feature back to us as being about hiding the dashboard – we should try to make Notify operate in terms of concepts that come naturally to people wherever possible.
2018-08-06 11:10:37 +01:00
'Can’t Add and edit templates '
Make the permission labels shorter They were feeling a bit heavy on the _Team members_ page. We can remove some words without losing any meaning.
2018-08-08 14:23:24 +01:00
'Can’t Manage settings, team and usage '
Relabel existing permissions Since we have added a new, 5th permission the existing permissions should be relabelled so that the five make sense as a coherent set. We especially want to make sure that: - the labels work against the checkboxes and against the tick/crosses on the manage users page (a long time ago this page was layed out differently so didn’t have space for full labels) - there is no confusion between usage and reports This commit also: - re-adds a line about what all users can see (‘sent messages’) but continues to omit the additional bullet points about templates and team members (because we think this is clear enough from reading the permissions) - refactors the `Form` subclass so that the content and order of the permissions only have to be defined once - brings back the ‘permissions’ legend on the `fieldset`
2018-08-08 08:45:58 +01:00
'Can’t Manage API integration'
Add a ‘See dashboard’ permission Our research and prototyping around ‘basic view’ found that: - a lot of users who send messages rarely or never look at the dashboard (yet it’s the first page they see when they sign in) - team managers like the idea of taking away things that users don’t need in order to make the interface simpler We’ve disentangled the simpler way of sending messages from being part of ‘basic view’. This means we can give managers the option of taking away the dashboard as an independent choice, not something that’s wrapped up in a separate ‘view’. I think that this checkbox is a more straightforward proposition than ‘basic view’ ever was (despite all the work we did to explain it and develop the nested checkbox pattern). In research users would often explain the feature back to us as being about hiding the dashboard – we should try to make Notify operate in terms of concepts that come naturally to people wherever possible.
2018-08-06 11:10:37 +01:00
),
),
Test that the right permissions in page This commit makes sure that the right permission choices are shown in these pages: - manage team page - invite a user page - edit permissions page This is in order to make changing these pages easier (see subsequent commits).
2017-08-17 10:49:21 +01:00
(
active_user_view_permissions,
(
'Test User With Permissions (you) '
Make the permission labels shorter They were feeling a bit heavy on the _Team members_ page. We can remove some words without losing any meaning.
2018-08-08 14:23:24 +01:00
'Can See dashboard '
'Can’t Send messages '
Add a ‘See dashboard’ permission Our research and prototyping around ‘basic view’ found that: - a lot of users who send messages rarely or never look at the dashboard (yet it’s the first page they see when they sign in) - team managers like the idea of taking away things that users don’t need in order to make the interface simpler We’ve disentangled the simpler way of sending messages from being part of ‘basic view’. This means we can give managers the option of taking away the dashboard as an independent choice, not something that’s wrapped up in a separate ‘view’. I think that this checkbox is a more straightforward proposition than ‘basic view’ ever was (despite all the work we did to explain it and develop the nested checkbox pattern). In research users would often explain the feature back to us as being about hiding the dashboard – we should try to make Notify operate in terms of concepts that come naturally to people wherever possible.
2018-08-06 11:10:37 +01:00
'Can’t Add and edit templates '
Make the permission labels shorter They were feeling a bit heavy on the _Team members_ page. We can remove some words without losing any meaning.
2018-08-08 14:23:24 +01:00
'Can’t Manage settings, team and usage '
Relabel existing permissions Since we have added a new, 5th permission the existing permissions should be relabelled so that the five make sense as a coherent set. We especially want to make sure that: - the labels work against the checkboxes and against the tick/crosses on the manage users page (a long time ago this page was layed out differently so didn’t have space for full labels) - there is no confusion between usage and reports This commit also: - re-adds a line about what all users can see (‘sent messages’) but continues to omit the additional bullet points about templates and team members (because we think this is clear enough from reading the permissions) - refactors the `Form` subclass so that the content and order of the permissions only have to be defined once - brings back the ‘permissions’ legend on the `fieldset`
2018-08-08 08:45:58 +01:00
'Can’t Manage API integration'
Add a fourth, ‘manage templates’ permission We’ve seen from research (a long time ago) that the ‘manage service’ permission is too broad, and gives too much control to someone who only needs the ability to edit templates. In other words, editing content should be its own, separate permission, rather than being rolled up into manage service. Since this is already disaggregated on the API side, making this change just means changing the mapping on the admin side and adding an extra checkbox on the invite/edit page. Which is what this commit does. So for now, an existing user who has the manage service permission gets both manage service and manage templates (ie no change to what they can do). Newly invited users will get to choose if they have both, either, or neither.
2017-08-17 11:14:26 +01:00
),
Change display of cancelled users, fix edit link It’s confusing showing green ticks for cancelled invites. This commit changes the appearance so that only pending or active users (ie those that could actually do some damage) get green ticks. Also fixes missing edit links caused by instances of `User` having `.state` but instances of `InvitedUser` having `.status`. Right now these are two separate lists. Which makes it harder to add improvements that will make large numbers of users easier to manage.
2018-01-26 17:14:00 +00:00
(
'ZZZZZZZZ zzzzzzz@example.gov.uk '
Make the permission labels shorter They were feeling a bit heavy on the _Team members_ page. We can remove some words without losing any meaning.
2018-08-08 14:23:24 +01:00
'Can See dashboard '
'Can’t Send messages '
Add a ‘See dashboard’ permission Our research and prototyping around ‘basic view’ found that: - a lot of users who send messages rarely or never look at the dashboard (yet it’s the first page they see when they sign in) - team managers like the idea of taking away things that users don’t need in order to make the interface simpler We’ve disentangled the simpler way of sending messages from being part of ‘basic view’. This means we can give managers the option of taking away the dashboard as an independent choice, not something that’s wrapped up in a separate ‘view’. I think that this checkbox is a more straightforward proposition than ‘basic view’ ever was (despite all the work we did to explain it and develop the nested checkbox pattern). In research users would often explain the feature back to us as being about hiding the dashboard – we should try to make Notify operate in terms of concepts that come naturally to people wherever possible.
2018-08-06 11:10:37 +01:00
'Can’t Add and edit templates '
Make the permission labels shorter They were feeling a bit heavy on the _Team members_ page. We can remove some words without losing any meaning.
2018-08-08 14:23:24 +01:00
'Can’t Manage settings, team and usage '
Relabel existing permissions Since we have added a new, 5th permission the existing permissions should be relabelled so that the five make sense as a coherent set. We especially want to make sure that: - the labels work against the checkboxes and against the tick/crosses on the manage users page (a long time ago this page was layed out differently so didn’t have space for full labels) - there is no confusion between usage and reports This commit also: - re-adds a line about what all users can see (‘sent messages’) but continues to omit the additional bullet points about templates and team members (because we think this is clear enough from reading the permissions) - refactors the `Form` subclass so that the content and order of the permissions only have to be defined once - brings back the ‘permissions’ legend on the `fieldset`
2018-08-08 08:45:58 +01:00
'Can’t Manage API integration'
Change display of cancelled users, fix edit link It’s confusing showing green ticks for cancelled invites. This commit changes the appearance so that only pending or active users (ie those that could actually do some damage) get green ticks. Also fixes missing edit links caused by instances of `User` having `.state` but instances of `InvitedUser` having `.status`. Right now these are two separate lists. Which makes it harder to add improvements that will make large numbers of users easier to manage.
2018-01-26 17:14:00 +00:00
)
Add a fourth, ‘manage templates’ permission We’ve seen from research (a long time ago) that the ‘manage service’ permission is too broad, and gives too much control to someone who only needs the ability to edit templates. In other words, editing content should be its own, separate permission, rather than being rolled up into manage service. Since this is already disaggregated on the API side, making this change just means changing the mapping on the admin side and adding an extra checkbox on the invite/edit page. Which is what this commit does. So for now, an existing user who has the manage service permission gets both manage service and manage templates (ie no change to what they can do). Newly invited users will get to choose if they have both, either, or neither.
2017-08-17 11:14:26 +01:00
),
(
active_user_manage_template_permission,
(
'Test User With Permissions (you) '
Make the permission labels shorter They were feeling a bit heavy on the _Team members_ page. We can remove some words without losing any meaning.
2018-08-08 14:23:24 +01:00
'Can See dashboard '
'Can’t Send messages '
Add a ‘See dashboard’ permission Our research and prototyping around ‘basic view’ found that: - a lot of users who send messages rarely or never look at the dashboard (yet it’s the first page they see when they sign in) - team managers like the idea of taking away things that users don’t need in order to make the interface simpler We’ve disentangled the simpler way of sending messages from being part of ‘basic view’. This means we can give managers the option of taking away the dashboard as an independent choice, not something that’s wrapped up in a separate ‘view’. I think that this checkbox is a more straightforward proposition than ‘basic view’ ever was (despite all the work we did to explain it and develop the nested checkbox pattern). In research users would often explain the feature back to us as being about hiding the dashboard – we should try to make Notify operate in terms of concepts that come naturally to people wherever possible.
2018-08-06 11:10:37 +01:00
'Can Add and edit templates '
Make the permission labels shorter They were feeling a bit heavy on the _Team members_ page. We can remove some words without losing any meaning.
2018-08-08 14:23:24 +01:00
'Can’t Manage settings, team and usage '
Relabel existing permissions Since we have added a new, 5th permission the existing permissions should be relabelled so that the five make sense as a coherent set. We especially want to make sure that: - the labels work against the checkboxes and against the tick/crosses on the manage users page (a long time ago this page was layed out differently so didn’t have space for full labels) - there is no confusion between usage and reports This commit also: - re-adds a line about what all users can see (‘sent messages’) but continues to omit the additional bullet points about templates and team members (because we think this is clear enough from reading the permissions) - refactors the `Form` subclass so that the content and order of the permissions only have to be defined once - brings back the ‘permissions’ legend on the `fieldset`
2018-08-08 08:45:58 +01:00
'Can’t Manage API integration'
Test that the right permissions in page This commit makes sure that the right permission choices are shown in these pages: - manage team page - invite a user page - edit permissions page This is in order to make changing these pages easier (see subsequent commits).
2017-08-17 10:49:21 +01:00
),
Change display of cancelled users, fix edit link It’s confusing showing green ticks for cancelled invites. This commit changes the appearance so that only pending or active users (ie those that could actually do some damage) get green ticks. Also fixes missing edit links caused by instances of `User` having `.state` but instances of `InvitedUser` having `.status`. Right now these are two separate lists. Which makes it harder to add improvements that will make large numbers of users easier to manage.
2018-01-26 17:14:00 +00:00
(
'ZZZZZZZZ zzzzzzz@example.gov.uk '
Make the permission labels shorter They were feeling a bit heavy on the _Team members_ page. We can remove some words without losing any meaning.
2018-08-08 14:23:24 +01:00
'Can See dashboard '
'Can’t Send messages '
Add a ‘See dashboard’ permission Our research and prototyping around ‘basic view’ found that: - a lot of users who send messages rarely or never look at the dashboard (yet it’s the first page they see when they sign in) - team managers like the idea of taking away things that users don’t need in order to make the interface simpler We’ve disentangled the simpler way of sending messages from being part of ‘basic view’. This means we can give managers the option of taking away the dashboard as an independent choice, not something that’s wrapped up in a separate ‘view’. I think that this checkbox is a more straightforward proposition than ‘basic view’ ever was (despite all the work we did to explain it and develop the nested checkbox pattern). In research users would often explain the feature back to us as being about hiding the dashboard – we should try to make Notify operate in terms of concepts that come naturally to people wherever possible.
2018-08-06 11:10:37 +01:00
'Can’t Add and edit templates '
Make the permission labels shorter They were feeling a bit heavy on the _Team members_ page. We can remove some words without losing any meaning.
2018-08-08 14:23:24 +01:00
'Can’t Manage settings, team and usage '
Relabel existing permissions Since we have added a new, 5th permission the existing permissions should be relabelled so that the five make sense as a coherent set. We especially want to make sure that: - the labels work against the checkboxes and against the tick/crosses on the manage users page (a long time ago this page was layed out differently so didn’t have space for full labels) - there is no confusion between usage and reports This commit also: - re-adds a line about what all users can see (‘sent messages’) but continues to omit the additional bullet points about templates and team members (because we think this is clear enough from reading the permissions) - refactors the `Form` subclass so that the content and order of the permissions only have to be defined once - brings back the ‘permissions’ legend on the `fieldset`
2018-08-08 08:45:58 +01:00
'Can’t Manage API integration'
Change display of cancelled users, fix edit link It’s confusing showing green ticks for cancelled invites. This commit changes the appearance so that only pending or active users (ie those that could actually do some damage) get green ticks. Also fixes missing edit links caused by instances of `User` having `.state` but instances of `InvitedUser` having `.status`. Right now these are two separate lists. Which makes it harder to add improvements that will make large numbers of users easier to manage.
2018-01-26 17:14:00 +00:00
)
Test that the right permissions in page This commit makes sure that the right permission choices are shown in these pages: - manage team page - invite a user page - edit permissions page This is in order to make changing these pages easier (see subsequent commits).
2017-08-17 10:49:21 +01:00
),
Allow setting of caseworking on a user This commit changes the form that the user sees when inviting or editing another user, if the service has the ‘caseworking’ permission set. This will allow creating a new type of user, one who only has the `send_messages` permission, without the `view_activity` permission. We are doing this because we think there are a number of services with a lot of users who don’t need to see the dashboard, or the other team members, and that we can make a simpler interface for these users.
2018-06-12 14:29:47 +01:00
(
active_user_manage_template_permission,
(
'Test User With Permissions (you) '
Make the permission labels shorter They were feeling a bit heavy on the _Team members_ page. We can remove some words without losing any meaning.
2018-08-08 14:23:24 +01:00
'Can See dashboard '
'Can’t Send messages '
Add a ‘See dashboard’ permission Our research and prototyping around ‘basic view’ found that: - a lot of users who send messages rarely or never look at the dashboard (yet it’s the first page they see when they sign in) - team managers like the idea of taking away things that users don’t need in order to make the interface simpler We’ve disentangled the simpler way of sending messages from being part of ‘basic view’. This means we can give managers the option of taking away the dashboard as an independent choice, not something that’s wrapped up in a separate ‘view’. I think that this checkbox is a more straightforward proposition than ‘basic view’ ever was (despite all the work we did to explain it and develop the nested checkbox pattern). In research users would often explain the feature back to us as being about hiding the dashboard – we should try to make Notify operate in terms of concepts that come naturally to people wherever possible.
2018-08-06 11:10:37 +01:00
'Can Add and edit templates '
Make the permission labels shorter They were feeling a bit heavy on the _Team members_ page. We can remove some words without losing any meaning.
2018-08-08 14:23:24 +01:00
'Can’t Manage settings, team and usage '
Relabel existing permissions Since we have added a new, 5th permission the existing permissions should be relabelled so that the five make sense as a coherent set. We especially want to make sure that: - the labels work against the checkboxes and against the tick/crosses on the manage users page (a long time ago this page was layed out differently so didn’t have space for full labels) - there is no confusion between usage and reports This commit also: - re-adds a line about what all users can see (‘sent messages’) but continues to omit the additional bullet points about templates and team members (because we think this is clear enough from reading the permissions) - refactors the `Form` subclass so that the content and order of the permissions only have to be defined once - brings back the ‘permissions’ legend on the `fieldset`
2018-08-08 08:45:58 +01:00
'Can’t Manage API integration'
Allow setting of caseworking on a user This commit changes the form that the user sees when inviting or editing another user, if the service has the ‘caseworking’ permission set. This will allow creating a new type of user, one who only has the `send_messages` permission, without the `view_activity` permission. We are doing this because we think there are a number of services with a lot of users who don’t need to see the dashboard, or the other team members, and that we can make a simpler interface for these users.
2018-06-12 14:29:47 +01:00
),
(
'ZZZZZZZZ zzzzzzz@example.gov.uk '
Make the permission labels shorter They were feeling a bit heavy on the _Team members_ page. We can remove some words without losing any meaning.
2018-08-08 14:23:24 +01:00
'Can See dashboard '
'Can’t Send messages '
Add a ‘See dashboard’ permission Our research and prototyping around ‘basic view’ found that: - a lot of users who send messages rarely or never look at the dashboard (yet it’s the first page they see when they sign in) - team managers like the idea of taking away things that users don’t need in order to make the interface simpler We’ve disentangled the simpler way of sending messages from being part of ‘basic view’. This means we can give managers the option of taking away the dashboard as an independent choice, not something that’s wrapped up in a separate ‘view’. I think that this checkbox is a more straightforward proposition than ‘basic view’ ever was (despite all the work we did to explain it and develop the nested checkbox pattern). In research users would often explain the feature back to us as being about hiding the dashboard – we should try to make Notify operate in terms of concepts that come naturally to people wherever possible.
2018-08-06 11:10:37 +01:00
'Can’t Add and edit templates '
Make the permission labels shorter They were feeling a bit heavy on the _Team members_ page. We can remove some words without losing any meaning.
2018-08-08 14:23:24 +01:00
'Can’t Manage settings, team and usage '
Relabel existing permissions Since we have added a new, 5th permission the existing permissions should be relabelled so that the five make sense as a coherent set. We especially want to make sure that: - the labels work against the checkboxes and against the tick/crosses on the manage users page (a long time ago this page was layed out differently so didn’t have space for full labels) - there is no confusion between usage and reports This commit also: - re-adds a line about what all users can see (‘sent messages’) but continues to omit the additional bullet points about templates and team members (because we think this is clear enough from reading the permissions) - refactors the `Form` subclass so that the content and order of the permissions only have to be defined once - brings back the ‘permissions’ legend on the `fieldset`
2018-08-08 08:45:58 +01:00
'Can’t Manage API integration'
Allow setting of caseworking on a user This commit changes the form that the user sees when inviting or editing another user, if the service has the ‘caseworking’ permission set. This will allow creating a new type of user, one who only has the `send_messages` permission, without the `view_activity` permission. We are doing this because we think there are a number of services with a lot of users who don’t need to see the dashboard, or the other team members, and that we can make a simpler interface for these users.
2018-06-12 14:29:47 +01:00
)
),
Test that the right permissions in page This commit makes sure that the right permission choices are shown in these pages: - manage team page - invite a user page - edit permissions page This is in order to make changing these pages easier (see subsequent commits).
2017-08-17 10:49:21 +01:00
])
[WIP] Start of user accepting invite. This commit only deals with acceptance by users who are already in system. Changed invite client to return invited user objects instead of dictionaries. Added commented out test. fixed up fixtures to return invited user object for invites
2016-02-29 17:35:21 +00:00
def test_should_show_overview_page(
Make existing tests use ``client_request fixture This commit makes the existing tests around user permissions less verbose by using the new `client_request` fixture. This fixture takes care of: - setting up a service - asserting that the response is `200` It also tests that the page titles, some of which didn’t match with the `<h1>`s, so this commit also fixes that mismatch.
2017-08-17 10:44:36 +01:00
client_request,
Normalize whitespace in test arguments We have a bunch of different styles of handling when function definitions span multiple lines, which they almost always do with tests. Here’s why an argument per line, single indent is best: - cleaner diffs when you change the name of a method (one line change instead of multiple lines) - works better on narrow screens, eg Github’s diff view, or with two terminals side by side on a laptop screen - works with any editor’s indenting shortcuts, no need for an IDE Also, trailing comma in the list of arguments is good because adding a new argument to a method becomes a one line, not two line diff.
2017-02-03 10:42:01 +00:00
mocker,
mock_get_invites_for_service,
Add a ‘See dashboard’ permission Our research and prototyping around ‘basic view’ found that: - a lot of users who send messages rarely or never look at the dashboard (yet it’s the first page they see when they sign in) - team managers like the idea of taking away things that users don’t need in order to make the interface simpler We’ve disentangled the simpler way of sending messages from being part of ‘basic view’. This means we can give managers the option of taking away the dashboard as an independent choice, not something that’s wrapped up in a separate ‘view’. I think that this checkbox is a more straightforward proposition than ‘basic view’ ever was (despite all the work we did to explain it and develop the nested checkbox pattern). In research users would often explain the feature back to us as being about hiding the dashboard – we should try to make Notify operate in terms of concepts that come naturally to people wherever possible.
2018-08-06 11:10:37 +01:00
mock_has_no_jobs,
Test that the right permissions in page This commit makes sure that the right permission choices are shown in these pages: - manage team page - invite a user page - edit permissions page This is in order to make changing these pages easier (see subsequent commits).
2017-08-17 10:49:21 +01:00
fake_uuid,
Allow setting of caseworking on a user This commit changes the form that the user sees when inviting or editing another user, if the service has the ‘caseworking’ permission set. This will allow creating a new type of user, one who only has the `send_messages` permission, without the `view_activity` permission. We are doing this because we think there are a number of services with a lot of users who don’t need to see the dashboard, or the other team members, and that we can make a simpler interface for these users.
2018-06-12 14:29:47 +01:00
service_one,
Test that the right permissions in page This commit makes sure that the right permission choices are shown in these pages: - manage team page - invite a user page - edit permissions page This is in order to make changing these pages easier (see subsequent commits).
2017-08-17 10:49:21 +01:00
user,
Change display of cancelled users, fix edit link It’s confusing showing green ticks for cancelled invites. This commit changes the appearance so that only pending or active users (ie those that could actually do some damage) get green ticks. Also fixes missing edit links caused by instances of `User` having `.state` but instances of `InvitedUser` having `.status`. Right now these are two separate lists. Which makes it harder to add improvements that will make large numbers of users easier to manage.
2018-01-26 17:14:00 +00:00
expected_self_text,
expected_coworker_text,
active_user_view_permissions,
[WIP] Start of user accepting invite. This commit only deals with acceptance by users who are already in system. Changed invite client to return invited user objects instead of dictionaries. Added commented out test. fixed up fixtures to return invited user object for invites
2016-02-29 17:35:21 +00:00
):
Change display of cancelled users, fix edit link It’s confusing showing green ticks for cancelled invites. This commit changes the appearance so that only pending or active users (ie those that could actually do some damage) get green ticks. Also fixes missing edit links caused by instances of `User` having `.state` but instances of `InvitedUser` having `.status`. Right now these are two separate lists. Which makes it harder to add improvements that will make large numbers of users easier to manage.
2018-01-26 17:14:00 +00:00
current_user = user(fake_uuid)
other_user = copy.deepcopy(active_user_view_permissions)
other_user.email_address = 'zzzzzzz@example.gov.uk'
other_user.name = 'ZZZZZZZZ'
other_user.id = 'zzzzzzzz-zzzz-zzzz-zzzz-zzzzzzzzzzzz'
mocker.patch('app.user_api_client.get_user', return_value=current_user)
mocker.patch('app.user_api_client.get_users_for_service', return_value=[
current_user,
other_user,
])
Make existing tests use ``client_request fixture This commit makes the existing tests around user permissions less verbose by using the new `client_request` fixture. This fixture takes care of: - setting up a service - asserting that the response is `200` It also tests that the page titles, some of which didn’t match with the `<h1>`s, so this commit also fixes that mismatch.
2017-08-17 10:44:36 +01:00
page = client_request.get('main.manage_users', service_id=SERVICE_ONE_ID)
[WIP] Start of user accepting invite. This commit only deals with acceptance by users who are already in system. Changed invite client to return invited user objects instead of dictionaries. Added commented out test. fixed up fixtures to return invited user object for invites
2016-02-29 17:35:21 +00:00
Make existing tests use ``client_request fixture This commit makes the existing tests around user permissions less verbose by using the new `client_request` fixture. This fixture takes care of: - setting up a service - asserting that the response is `200` It also tests that the page titles, some of which didn’t match with the `<h1>`s, so this commit also fixes that mismatch.
2017-08-17 10:44:36 +01:00
assert normalize_spaces(page.select_one('h1').text) == 'Team members'
Change display of cancelled users, fix edit link It’s confusing showing green ticks for cancelled invites. This commit changes the appearance so that only pending or active users (ie those that could actually do some damage) get green ticks. Also fixes missing edit links caused by instances of `User` having `.state` but instances of `InvitedUser` having `.status`. Right now these are two separate lists. Which makes it harder to add improvements that will make large numbers of users easier to manage.
2018-01-26 17:14:00 +00:00
assert normalize_spaces(page.select('.user-list-item')[0].text) == expected_self_text
# [1:5] are invited users
assert normalize_spaces(page.select('.user-list-item')[6].text) == expected_coworker_text
Make existing tests use ``client_request fixture This commit makes the existing tests around user permissions less verbose by using the new `client_request` fixture. This fixture takes care of: - setting up a service - asserting that the response is `200` It also tests that the page titles, some of which didn’t match with the `<h1>`s, so this commit also fixes that mismatch.
2017-08-17 10:44:36 +01:00
app.user_api_client.get_users_for_service.assert_called_once_with(service_id=SERVICE_ONE_ID)
Add pages to invite, edit, and delete users This takes the original prototype version of this page, and, using the same fake data (ie nothing is wired up): - adds an invite users page - adds an edit (and delete) user page Both these pages allow the user to set another user’s permissions. This commit adds images for the ticks and crosses, so we have control over their appearance.
2016-02-19 15:02:13 +00:00
Allow setting of caseworking on a user This commit changes the form that the user sees when inviting or editing another user, if the service has the ‘caseworking’ permission set. This will allow creating a new type of user, one who only has the `send_messages` permission, without the `view_activity` permission. We are doing this because we think there are a number of services with a lot of users who don’t need to see the dashboard, or the other team members, and that we can make a simpler interface for these users.
2018-06-12 14:29:47 +01:00
def test_should_show_caseworker_on_overview_page(
client_request,
mocker,
mock_get_invites_for_service,
fake_uuid,
service_one,
):
service_one['permissions'].append('caseworking')
current_user = active_user_view_permissions(active_user_view_permissions)
other_user = active_caseworking_user(fake_uuid)
other_user.email_address = 'zzzzzzz@example.gov.uk'
mocker.patch('app.user_api_client.get_user', return_value=current_user)
mocker.patch('app.user_api_client.get_users_for_service', return_value=[
current_user,
other_user,
])
page = client_request.get('main.manage_users', service_id=SERVICE_ONE_ID)
assert normalize_spaces(page.select_one('h1').text) == 'Team members'
assert normalize_spaces(page.select('.user-list-item')[0].text) == (
'Test User With Permissions (you) '
Make the permission labels shorter They were feeling a bit heavy on the _Team members_ page. We can remove some words without losing any meaning.
2018-08-08 14:23:24 +01:00
'Can See dashboard '
'Can’t Send messages '
Allow setting of caseworking on a user This commit changes the form that the user sees when inviting or editing another user, if the service has the ‘caseworking’ permission set. This will allow creating a new type of user, one who only has the `send_messages` permission, without the `view_activity` permission. We are doing this because we think there are a number of services with a lot of users who don’t need to see the dashboard, or the other team members, and that we can make a simpler interface for these users.
2018-06-12 14:29:47 +01:00
'Can’t Add and edit templates '
Make the permission labels shorter They were feeling a bit heavy on the _Team members_ page. We can remove some words without losing any meaning.
2018-08-08 14:23:24 +01:00
'Can’t Manage settings, team and usage '
Relabel existing permissions Since we have added a new, 5th permission the existing permissions should be relabelled so that the five make sense as a coherent set. We especially want to make sure that: - the labels work against the checkboxes and against the tick/crosses on the manage users page (a long time ago this page was layed out differently so didn’t have space for full labels) - there is no confusion between usage and reports This commit also: - re-adds a line about what all users can see (‘sent messages’) but continues to omit the additional bullet points about templates and team members (because we think this is clear enough from reading the permissions) - refactors the `Form` subclass so that the content and order of the permissions only have to be defined once - brings back the ‘permissions’ legend on the `fieldset`
2018-08-08 08:45:58 +01:00
'Can’t Manage API integration'
Allow setting of caseworking on a user This commit changes the form that the user sees when inviting or editing another user, if the service has the ‘caseworking’ permission set. This will allow creating a new type of user, one who only has the `send_messages` permission, without the `view_activity` permission. We are doing this because we think there are a number of services with a lot of users who don’t need to see the dashboard, or the other team members, and that we can make a simpler interface for these users.
2018-06-12 14:29:47 +01:00
)
# [1:5] are invited users
assert normalize_spaces(page.select('.user-list-item')[6].text) == (
'Test User zzzzzzz@example.gov.uk '
Make the permission labels shorter They were feeling a bit heavy on the _Team members_ page. We can remove some words without losing any meaning.
2018-08-08 14:23:24 +01:00
'Can’t See dashboard '
'Can Send messages '
Add a ‘See dashboard’ permission Our research and prototyping around ‘basic view’ found that: - a lot of users who send messages rarely or never look at the dashboard (yet it’s the first page they see when they sign in) - team managers like the idea of taking away things that users don’t need in order to make the interface simpler We’ve disentangled the simpler way of sending messages from being part of ‘basic view’. This means we can give managers the option of taking away the dashboard as an independent choice, not something that’s wrapped up in a separate ‘view’. I think that this checkbox is a more straightforward proposition than ‘basic view’ ever was (despite all the work we did to explain it and develop the nested checkbox pattern). In research users would often explain the feature back to us as being about hiding the dashboard – we should try to make Notify operate in terms of concepts that come naturally to people wherever possible.
2018-08-06 11:10:37 +01:00
'Can’t Add and edit templates '
Make the permission labels shorter They were feeling a bit heavy on the _Team members_ page. We can remove some words without losing any meaning.
2018-08-08 14:23:24 +01:00
'Can’t Manage settings, team and usage '
Relabel existing permissions Since we have added a new, 5th permission the existing permissions should be relabelled so that the five make sense as a coherent set. We especially want to make sure that: - the labels work against the checkboxes and against the tick/crosses on the manage users page (a long time ago this page was layed out differently so didn’t have space for full labels) - there is no confusion between usage and reports This commit also: - re-adds a line about what all users can see (‘sent messages’) but continues to omit the additional bullet points about templates and team members (because we think this is clear enough from reading the permissions) - refactors the `Form` subclass so that the content and order of the permissions only have to be defined once - brings back the ‘permissions’ legend on the `fieldset`
2018-08-08 08:45:58 +01:00
'Can’t Manage API integration'
Allow setting of caseworking on a user This commit changes the form that the user sees when inviting or editing another user, if the service has the ‘caseworking’ permission set. This will allow creating a new type of user, one who only has the `send_messages` permission, without the `view_activity` permission. We are doing this because we think there are a number of services with a lot of users who don’t need to see the dashboard, or the other team members, and that we can make a simpler interface for these users.
2018-06-12 14:29:47 +01:00
)
Email auth for inviting members and editing permissions
2017-11-01 15:36:27 +00:00
@pytest.mark.parametrize('endpoint, extra_args, service_has_email_auth, auth_options_hidden', [
(
'main.edit_user_permissions',
Don’t allow editing of users from other services Currently when you load the ‘edit user’ page (which has a URL like `/service/<service_id>/users/<user_id>`) we check that: - you belong to the service represented by `service_id` - you have permission to edit users on this service We don’t check that: - the user represented by `user_id` belongs to this service This means that if you could somehow determine another user’s `user_id` (which I don’t think is possible if you don’t already have the manage service permission for that service) then you could: - edit their permissions on your service (weird, but wouldn’t have any effect) - change their email address (bad) This commit adds checks to return a `404` any time you’re looking at a service and trying to do stuff to a user who doesn’t belong to that service. We can’t add this check to the API easily because there are still times that we want to get/modify users outside the context of a service (eg platform admin pages, or users who have no services).
2019-02-25 16:51:37 +00:00
{'user_id': sample_uuid()},
Email auth for inviting members and editing permissions
2017-11-01 15:36:27 +00:00
True,
False
),
(
'main.edit_user_permissions',
Don’t allow editing of users from other services Currently when you load the ‘edit user’ page (which has a URL like `/service/<service_id>/users/<user_id>`) we check that: - you belong to the service represented by `service_id` - you have permission to edit users on this service We don’t check that: - the user represented by `user_id` belongs to this service This means that if you could somehow determine another user’s `user_id` (which I don’t think is possible if you don’t already have the manage service permission for that service) then you could: - edit their permissions on your service (weird, but wouldn’t have any effect) - change their email address (bad) This commit adds checks to return a `404` any time you’re looking at a service and trying to do stuff to a user who doesn’t belong to that service. We can’t add this check to the API easily because there are still times that we want to get/modify users outside the context of a service (eg platform admin pages, or users who have no services).
2019-02-25 16:51:37 +00:00
{'user_id': sample_uuid()},
Email auth for inviting members and editing permissions
2017-11-01 15:36:27 +00:00
False,
True
),
(
'main.invite_user',
{},
True,
False
),
(
'main.invite_user',
{},
False,
True
)
])
def test_service_with_no_email_auth_hides_auth_type_options(
client_request,
endpoint,
extra_args,
service_has_email_auth,
auth_options_hidden,
Don’t allow editing of users from other services Currently when you load the ‘edit user’ page (which has a URL like `/service/<service_id>/users/<user_id>`) we check that: - you belong to the service represented by `service_id` - you have permission to edit users on this service We don’t check that: - the user represented by `user_id` belongs to this service This means that if you could somehow determine another user’s `user_id` (which I don’t think is possible if you don’t already have the manage service permission for that service) then you could: - edit their permissions on your service (weird, but wouldn’t have any effect) - change their email address (bad) This commit adds checks to return a `404` any time you’re looking at a service and trying to do stuff to a user who doesn’t belong to that service. We can’t add this check to the API easily because there are still times that we want to get/modify users outside the context of a service (eg platform admin pages, or users who have no services).
2019-02-25 16:51:37 +00:00
service_one,
mock_get_users_by_service,
Display nested folders permissions form on user permissions page We're reusing the logic for the `move_to` nested radios field for the user folder permissions nested checkboxes. The main difference between the two forms (aside from the different input type) is that "Move" form contains the root "Templates" as an option, whereas the folder permissions doesn't. It turns out that, because of the way NestedFieldMixin.children and select_nested macro are implemented the easiest way to get the desired folder permissions behaviour is to add the root folder as a choice with a `None` value and `NONE_OPTION_VALUE = None` set on the field, which allows the `child_map` to be constructed but doesn't display the root folder checkbox itself since it gets overwritten in the final `child_map`.
2019-02-25 16:23:28 +00:00
mock_get_template_folders
Email auth for inviting members and editing permissions
2017-11-01 15:36:27 +00:00
):
if service_has_email_auth:
service_one['permissions'].append('email_auth')
page = client_request.get(endpoint, service_id=service_one['id'], **extra_args)
assert (page.find('input', attrs={"name": "login_authentication"}) is None) == auth_options_hidden
Add a ‘See dashboard’ permission Our research and prototyping around ‘basic view’ found that: - a lot of users who send messages rarely or never look at the dashboard (yet it’s the first page they see when they sign in) - team managers like the idea of taking away things that users don’t need in order to make the interface simpler We’ve disentangled the simpler way of sending messages from being part of ‘basic view’. This means we can give managers the option of taking away the dashboard as an independent choice, not something that’s wrapped up in a separate ‘view’. I think that this checkbox is a more straightforward proposition than ‘basic view’ ever was (despite all the work we did to explain it and develop the nested checkbox pattern). In research users would often explain the feature back to us as being about hiding the dashboard – we should try to make Notify operate in terms of concepts that come naturally to people wherever possible.
2018-08-06 11:10:37 +01:00
@pytest.mark.parametrize('service_has_caseworking', (True, False))
@pytest.mark.parametrize('endpoint, extra_args', [
Allow setting of caseworking on a user This commit changes the form that the user sees when inviting or editing another user, if the service has the ‘caseworking’ permission set. This will allow creating a new type of user, one who only has the `send_messages` permission, without the `view_activity` permission. We are doing this because we think there are a number of services with a lot of users who don’t need to see the dashboard, or the other team members, and that we can make a simpler interface for these users.
2018-06-12 14:29:47 +01:00
(
'main.edit_user_permissions',
Don’t allow editing of users from other services Currently when you load the ‘edit user’ page (which has a URL like `/service/<service_id>/users/<user_id>`) we check that: - you belong to the service represented by `service_id` - you have permission to edit users on this service We don’t check that: - the user represented by `user_id` belongs to this service This means that if you could somehow determine another user’s `user_id` (which I don’t think is possible if you don’t already have the manage service permission for that service) then you could: - edit their permissions on your service (weird, but wouldn’t have any effect) - change their email address (bad) This commit adds checks to return a `404` any time you’re looking at a service and trying to do stuff to a user who doesn’t belong to that service. We can’t add this check to the API easily because there are still times that we want to get/modify users outside the context of a service (eg platform admin pages, or users who have no services).
2019-02-25 16:51:37 +00:00
{'user_id': sample_uuid()},
Allow setting of caseworking on a user This commit changes the form that the user sees when inviting or editing another user, if the service has the ‘caseworking’ permission set. This will allow creating a new type of user, one who only has the `send_messages` permission, without the `view_activity` permission. We are doing this because we think there are a number of services with a lot of users who don’t need to see the dashboard, or the other team members, and that we can make a simpler interface for these users.
2018-06-12 14:29:47 +01:00
),
(
'main.invite_user',
{},
),
])
def test_service_without_caseworking_doesnt_show_admin_vs_caseworker(
client_request,
Don’t allow editing of users from other services Currently when you load the ‘edit user’ page (which has a URL like `/service/<service_id>/users/<user_id>`) we check that: - you belong to the service represented by `service_id` - you have permission to edit users on this service We don’t check that: - the user represented by `user_id` belongs to this service This means that if you could somehow determine another user’s `user_id` (which I don’t think is possible if you don’t already have the manage service permission for that service) then you could: - edit their permissions on your service (weird, but wouldn’t have any effect) - change their email address (bad) This commit adds checks to return a `404` any time you’re looking at a service and trying to do stuff to a user who doesn’t belong to that service. We can’t add this check to the API easily because there are still times that we want to get/modify users outside the context of a service (eg platform admin pages, or users who have no services).
2019-02-25 16:51:37 +00:00
mock_get_users_by_service,
Display nested folders permissions form on user permissions page We're reusing the logic for the `move_to` nested radios field for the user folder permissions nested checkboxes. The main difference between the two forms (aside from the different input type) is that "Move" form contains the root "Templates" as an option, whereas the folder permissions doesn't. It turns out that, because of the way NestedFieldMixin.children and select_nested macro are implemented the easiest way to get the desired folder permissions behaviour is to add the root folder as a choice with a `None` value and `NONE_OPTION_VALUE = None` set on the field, which allows the `child_map` to be constructed but doesn't display the root folder checkbox itself since it gets overwritten in the final `child_map`.
2019-02-25 16:23:28 +00:00
mock_get_template_folders,
Allow setting of caseworking on a user This commit changes the form that the user sees when inviting or editing another user, if the service has the ‘caseworking’ permission set. This will allow creating a new type of user, one who only has the `send_messages` permission, without the `view_activity` permission. We are doing this because we think there are a number of services with a lot of users who don’t need to see the dashboard, or the other team members, and that we can make a simpler interface for these users.
2018-06-12 14:29:47 +01:00
endpoint,
service_has_caseworking,
Add a ‘See dashboard’ permission Our research and prototyping around ‘basic view’ found that: - a lot of users who send messages rarely or never look at the dashboard (yet it’s the first page they see when they sign in) - team managers like the idea of taking away things that users don’t need in order to make the interface simpler We’ve disentangled the simpler way of sending messages from being part of ‘basic view’. This means we can give managers the option of taking away the dashboard as an independent choice, not something that’s wrapped up in a separate ‘view’. I think that this checkbox is a more straightforward proposition than ‘basic view’ ever was (despite all the work we did to explain it and develop the nested checkbox pattern). In research users would often explain the feature back to us as being about hiding the dashboard – we should try to make Notify operate in terms of concepts that come naturally to people wherever possible.
2018-08-06 11:10:37 +01:00
extra_args,
Allow setting of caseworking on a user This commit changes the form that the user sees when inviting or editing another user, if the service has the ‘caseworking’ permission set. This will allow creating a new type of user, one who only has the `send_messages` permission, without the `view_activity` permission. We are doing this because we think there are a number of services with a lot of users who don’t need to see the dashboard, or the other team members, and that we can make a simpler interface for these users.
2018-06-12 14:29:47 +01:00
):
Add a ‘See dashboard’ permission Our research and prototyping around ‘basic view’ found that: - a lot of users who send messages rarely or never look at the dashboard (yet it’s the first page they see when they sign in) - team managers like the idea of taking away things that users don’t need in order to make the interface simpler We’ve disentangled the simpler way of sending messages from being part of ‘basic view’. This means we can give managers the option of taking away the dashboard as an independent choice, not something that’s wrapped up in a separate ‘view’. I think that this checkbox is a more straightforward proposition than ‘basic view’ ever was (despite all the work we did to explain it and develop the nested checkbox pattern). In research users would often explain the feature back to us as being about hiding the dashboard – we should try to make Notify operate in terms of concepts that come naturally to people wherever possible.
2018-08-06 11:10:37 +01:00
page = client_request.get(
endpoint,
service_id=SERVICE_ONE_ID,
**extra_args
)
assert page.select('input[type=checkbox]')[0]['name'] == 'view_activity'
assert page.select('input[type=checkbox]')[1]['name'] == 'send_messages'
assert page.select('input[type=checkbox]')[2]['name'] == 'manage_templates'
assert page.select('input[type=checkbox]')[3]['name'] == 'manage_service'
assert page.select('input[type=checkbox]')[4]['name'] == 'manage_api_keys'
Allow setting of caseworking on a user This commit changes the form that the user sees when inviting or editing another user, if the service has the ‘caseworking’ permission set. This will allow creating a new type of user, one who only has the `send_messages` permission, without the `view_activity` permission. We are doing this because we think there are a number of services with a lot of users who don’t need to see the dashboard, or the other team members, and that we can make a simpler interface for these users.
2018-06-12 14:29:47 +01:00
Email auth for inviting members and editing permissions
2017-11-01 15:36:27 +00:00
@pytest.mark.parametrize('service_has_email_auth, displays_auth_type', [
(True, True),
(False, False)
])
def test_manage_users_page_shows_member_auth_type_if_service_has_email_auth_activated(
client_request,
service_has_email_auth,
service_one,
mock_get_users_by_service,
mock_get_invites_for_service,
displays_auth_type
):
if service_has_email_auth:
service_one['permissions'].append('email_auth')
page = client_request.get('main.manage_users', service_id=service_one['id'])
assert bool(page.select_one('.tick-cross-list-hint')) == displays_auth_type
Add a hint to explain why SMS auth is unavailable If we’re going to ‘disable’ radio buttons then we should always tell users why the radio button is disabled. This is what we found with the API key choices anyway.
2017-11-15 16:04:50 +00:00
@pytest.mark.parametrize('user, sms_option_disabled, expected_label', [
Email auth for inviting members and editing permissions
2017-11-01 15:36:27 +00:00
(
active_user_no_mobile,
True,
Add a hint to explain why SMS auth is unavailable If we’re going to ‘disable’ radio buttons then we should always tell users why the radio button is disabled. This is what we found with the API key choices anyway.
2017-11-15 16:04:50 +00:00
"""
Text message code
Not available because this team member hasn’t added a
phone number to their profile
""",
Email auth for inviting members and editing permissions
2017-11-01 15:36:27 +00:00
),
(
active_user_with_permissions,
False,
Add a hint to explain why SMS auth is unavailable If we’re going to ‘disable’ radio buttons then we should always tell users why the radio button is disabled. This is what we found with the API key choices anyway.
2017-11-15 16:04:50 +00:00
"""
Text message code
""",
Email auth for inviting members and editing permissions
2017-11-01 15:36:27 +00:00
),
])
def test_user_with_no_mobile_number_cant_be_set_to_sms_auth(
client_request,
Don’t allow editing of users from other services Currently when you load the ‘edit user’ page (which has a URL like `/service/<service_id>/users/<user_id>`) we check that: - you belong to the service represented by `service_id` - you have permission to edit users on this service We don’t check that: - the user represented by `user_id` belongs to this service This means that if you could somehow determine another user’s `user_id` (which I don’t think is possible if you don’t already have the manage service permission for that service) then you could: - edit their permissions on your service (weird, but wouldn’t have any effect) - change their email address (bad) This commit adds checks to return a `404` any time you’re looking at a service and trying to do stuff to a user who doesn’t belong to that service. We can’t add this check to the API easily because there are still times that we want to get/modify users outside the context of a service (eg platform admin pages, or users who have no services).
2019-02-25 16:51:37 +00:00
mock_get_users_by_service,
Display nested folders permissions form on user permissions page We're reusing the logic for the `move_to` nested radios field for the user folder permissions nested checkboxes. The main difference between the two forms (aside from the different input type) is that "Move" form contains the root "Templates" as an option, whereas the folder permissions doesn't. It turns out that, because of the way NestedFieldMixin.children and select_nested macro are implemented the easiest way to get the desired folder permissions behaviour is to add the root folder as a choice with a `None` value and `NONE_OPTION_VALUE = None` set on the field, which allows the `child_map` to be constructed but doesn't display the root folder checkbox itself since it gets overwritten in the final `child_map`.
2019-02-25 16:23:28 +00:00
mock_get_template_folders,
Email auth for inviting members and editing permissions
2017-11-01 15:36:27 +00:00
user,
sms_option_disabled,
Add a hint to explain why SMS auth is unavailable If we’re going to ‘disable’ radio buttons then we should always tell users why the radio button is disabled. This is what we found with the API key choices anyway.
2017-11-15 16:04:50 +00:00
expected_label,
Email auth for inviting members and editing permissions
2017-11-01 15:36:27 +00:00
service_one,
mocker
):
service_one['permissions'].append('email_auth')
Don’t allow editing of users from other services Currently when you load the ‘edit user’ page (which has a URL like `/service/<service_id>/users/<user_id>`) we check that: - you belong to the service represented by `service_id` - you have permission to edit users on this service We don’t check that: - the user represented by `user_id` belongs to this service This means that if you could somehow determine another user’s `user_id` (which I don’t think is possible if you don’t already have the manage service permission for that service) then you could: - edit their permissions on your service (weird, but wouldn’t have any effect) - change their email address (bad) This commit adds checks to return a `404` any time you’re looking at a service and trying to do stuff to a user who doesn’t belong to that service. We can’t add this check to the API easily because there are still times that we want to get/modify users outside the context of a service (eg platform admin pages, or users who have no services).
2019-02-25 16:51:37 +00:00
mocker.patch('app.user_api_client.get_user', return_value=user(mocker))
Email auth for inviting members and editing permissions
2017-11-01 15:36:27 +00:00
page = client_request.get(
'main.edit_user_permissions',
service_id=service_one['id'],
Don’t allow editing of users from other services Currently when you load the ‘edit user’ page (which has a URL like `/service/<service_id>/users/<user_id>`) we check that: - you belong to the service represented by `service_id` - you have permission to edit users on this service We don’t check that: - the user represented by `user_id` belongs to this service This means that if you could somehow determine another user’s `user_id` (which I don’t think is possible if you don’t already have the manage service permission for that service) then you could: - edit their permissions on your service (weird, but wouldn’t have any effect) - change their email address (bad) This commit adds checks to return a `404` any time you’re looking at a service and trying to do stuff to a user who doesn’t belong to that service. We can’t add this check to the API easily because there are still times that we want to get/modify users outside the context of a service (eg platform admin pages, or users who have no services).
2019-02-25 16:51:37 +00:00
user_id=sample_uuid(),
Email auth for inviting members and editing permissions
2017-11-01 15:36:27 +00:00
)
sms_auth_radio_button = page.select_one('input[value="sms_auth"]')
assert sms_auth_radio_button.has_attr("disabled") == sms_option_disabled
Add a hint to explain why SMS auth is unavailable If we’re going to ‘disable’ radio buttons then we should always tell users why the radio button is disabled. This is what we found with the API key choices anyway.
2017-11-15 16:04:50 +00:00
assert normalize_spaces(
page.select_one('label[for=login_authentication-0]').text
) == normalize_spaces(expected_label)
Email auth for inviting members and editing permissions
2017-11-01 15:36:27 +00:00
Test that the right permissions in page This commit makes sure that the right permission choices are shown in these pages: - manage team page - invite a user page - edit permissions page This is in order to make changing these pages easier (see subsequent commits).
2017-08-17 10:49:21 +01:00
@pytest.mark.parametrize('endpoint, extra_args, expected_checkboxes', [
(
'main.edit_user_permissions',
Don’t allow editing of users from other services Currently when you load the ‘edit user’ page (which has a URL like `/service/<service_id>/users/<user_id>`) we check that: - you belong to the service represented by `service_id` - you have permission to edit users on this service We don’t check that: - the user represented by `user_id` belongs to this service This means that if you could somehow determine another user’s `user_id` (which I don’t think is possible if you don’t already have the manage service permission for that service) then you could: - edit their permissions on your service (weird, but wouldn’t have any effect) - change their email address (bad) This commit adds checks to return a `404` any time you’re looking at a service and trying to do stuff to a user who doesn’t belong to that service. We can’t add this check to the API easily because there are still times that we want to get/modify users outside the context of a service (eg platform admin pages, or users who have no services).
2019-02-25 16:51:37 +00:00
{'user_id': sample_uuid()},
Test that the right permissions in page This commit makes sure that the right permission choices are shown in these pages: - manage team page - invite a user page - edit permissions page This is in order to make changing these pages easier (see subsequent commits).
2017-08-17 10:49:21 +01:00
[
Add a ‘See dashboard’ permission Our research and prototyping around ‘basic view’ found that: - a lot of users who send messages rarely or never look at the dashboard (yet it’s the first page they see when they sign in) - team managers like the idea of taking away things that users don’t need in order to make the interface simpler We’ve disentangled the simpler way of sending messages from being part of ‘basic view’. This means we can give managers the option of taking away the dashboard as an independent choice, not something that’s wrapped up in a separate ‘view’. I think that this checkbox is a more straightforward proposition than ‘basic view’ ever was (despite all the work we did to explain it and develop the nested checkbox pattern). In research users would often explain the feature back to us as being about hiding the dashboard – we should try to make Notify operate in terms of concepts that come naturally to people wherever possible.
2018-08-06 11:10:37 +01:00
('view_activity', True),
Test that the right permissions in page This commit makes sure that the right permission choices are shown in these pages: - manage team page - invite a user page - edit permissions page This is in order to make changing these pages easier (see subsequent commits).
2017-08-17 10:49:21 +01:00
('send_messages', True),
Add a fourth, ‘manage templates’ permission We’ve seen from research (a long time ago) that the ‘manage service’ permission is too broad, and gives too much control to someone who only needs the ability to edit templates. In other words, editing content should be its own, separate permission, rather than being rolled up into manage service. Since this is already disaggregated on the API side, making this change just means changing the mapping on the admin side and adding an extra checkbox on the invite/edit page. Which is what this commit does. So for now, an existing user who has the manage service permission gets both manage service and manage templates (ie no change to what they can do). Newly invited users will get to choose if they have both, either, or neither.
2017-08-17 11:14:26 +01:00
('manage_templates', True),
Test that the right permissions in page This commit makes sure that the right permission choices are shown in these pages: - manage team page - invite a user page - edit permissions page This is in order to make changing these pages easier (see subsequent commits).
2017-08-17 10:49:21 +01:00
('manage_service', True),
('manage_api_keys', True),
]
),
(
'main.invite_user',
{},
[
Add a ‘See dashboard’ permission Our research and prototyping around ‘basic view’ found that: - a lot of users who send messages rarely or never look at the dashboard (yet it’s the first page they see when they sign in) - team managers like the idea of taking away things that users don’t need in order to make the interface simpler We’ve disentangled the simpler way of sending messages from being part of ‘basic view’. This means we can give managers the option of taking away the dashboard as an independent choice, not something that’s wrapped up in a separate ‘view’. I think that this checkbox is a more straightforward proposition than ‘basic view’ ever was (despite all the work we did to explain it and develop the nested checkbox pattern). In research users would often explain the feature back to us as being about hiding the dashboard – we should try to make Notify operate in terms of concepts that come naturally to people wherever possible.
2018-08-06 11:10:37 +01:00
('view_activity', False),
Test that the right permissions in page This commit makes sure that the right permission choices are shown in these pages: - manage team page - invite a user page - edit permissions page This is in order to make changing these pages easier (see subsequent commits).
2017-08-17 10:49:21 +01:00
('send_messages', False),
Add a fourth, ‘manage templates’ permission We’ve seen from research (a long time ago) that the ‘manage service’ permission is too broad, and gives too much control to someone who only needs the ability to edit templates. In other words, editing content should be its own, separate permission, rather than being rolled up into manage service. Since this is already disaggregated on the API side, making this change just means changing the mapping on the admin side and adding an extra checkbox on the invite/edit page. Which is what this commit does. So for now, an existing user who has the manage service permission gets both manage service and manage templates (ie no change to what they can do). Newly invited users will get to choose if they have both, either, or neither.
2017-08-17 11:14:26 +01:00
('manage_templates', False),
Test that the right permissions in page This commit makes sure that the right permission choices are shown in these pages: - manage team page - invite a user page - edit permissions page This is in order to make changing these pages easier (see subsequent commits).
2017-08-17 10:49:21 +01:00
('manage_service', False),
('manage_api_keys', False),
]
),
])
Add pages to invite, edit, and delete users This takes the original prototype version of this page, and, using the same fake data (ie nothing is wired up): - adds an invite users page - adds an edit (and delete) user page Both these pages allow the user to set another user’s permissions. This commit adds images for the ticks and crosses, so we have control over their appearance.
2016-02-19 15:02:13 +00:00
def test_should_show_page_for_one_user(
Make existing tests use ``client_request fixture This commit makes the existing tests around user permissions less verbose by using the new `client_request` fixture. This fixture takes care of: - setting up a service - asserting that the response is `200` It also tests that the page titles, some of which didn’t match with the `<h1>`s, so this commit also fixes that mismatch.
2017-08-17 10:44:36 +01:00
client_request,
Don’t allow editing of users from other services Currently when you load the ‘edit user’ page (which has a URL like `/service/<service_id>/users/<user_id>`) we check that: - you belong to the service represented by `service_id` - you have permission to edit users on this service We don’t check that: - the user represented by `user_id` belongs to this service This means that if you could somehow determine another user’s `user_id` (which I don’t think is possible if you don’t already have the manage service permission for that service) then you could: - edit their permissions on your service (weird, but wouldn’t have any effect) - change their email address (bad) This commit adds checks to return a `404` any time you’re looking at a service and trying to do stuff to a user who doesn’t belong to that service. We can’t add this check to the API easily because there are still times that we want to get/modify users outside the context of a service (eg platform admin pages, or users who have no services).
2019-02-25 16:51:37 +00:00
mock_get_users_by_service,
Display nested folders permissions form on user permissions page We're reusing the logic for the `move_to` nested radios field for the user folder permissions nested checkboxes. The main difference between the two forms (aside from the different input type) is that "Move" form contains the root "Templates" as an option, whereas the folder permissions doesn't. It turns out that, because of the way NestedFieldMixin.children and select_nested macro are implemented the easiest way to get the desired folder permissions behaviour is to add the root folder as a choice with a `None` value and `NONE_OPTION_VALUE = None` set on the field, which allows the `child_map` to be constructed but doesn't display the root folder checkbox itself since it gets overwritten in the final `child_map`.
2019-02-25 16:23:28 +00:00
mock_get_template_folders,
Test that the right permissions in page This commit makes sure that the right permission choices are shown in these pages: - manage team page - invite a user page - edit permissions page This is in order to make changing these pages easier (see subsequent commits).
2017-08-17 10:49:21 +01:00
endpoint,
extra_args,
expected_checkboxes,
Add pages to invite, edit, and delete users This takes the original prototype version of this page, and, using the same fake data (ie nothing is wired up): - adds an invite users page - adds an edit (and delete) user page Both these pages allow the user to set another user’s permissions. This commit adds images for the ticks and crosses, so we have control over their appearance.
2016-02-19 15:02:13 +00:00
):
Test that the right permissions in page This commit makes sure that the right permission choices are shown in these pages: - manage team page - invite a user page - edit permissions page This is in order to make changing these pages easier (see subsequent commits).
2017-08-17 10:49:21 +01:00
page = client_request.get(endpoint, service_id=SERVICE_ONE_ID, **extra_args)
checkboxes = page.select('input[type=checkbox]')
Add a ‘See dashboard’ permission Our research and prototyping around ‘basic view’ found that: - a lot of users who send messages rarely or never look at the dashboard (yet it’s the first page they see when they sign in) - team managers like the idea of taking away things that users don’t need in order to make the interface simpler We’ve disentangled the simpler way of sending messages from being part of ‘basic view’. This means we can give managers the option of taking away the dashboard as an independent choice, not something that’s wrapped up in a separate ‘view’. I think that this checkbox is a more straightforward proposition than ‘basic view’ ever was (despite all the work we did to explain it and develop the nested checkbox pattern). In research users would often explain the feature back to us as being about hiding the dashboard – we should try to make Notify operate in terms of concepts that come naturally to people wherever possible.
2018-08-06 11:10:37 +01:00
assert len(checkboxes) == 5
Test that the right permissions in page This commit makes sure that the right permission choices are shown in these pages: - manage team page - invite a user page - edit permissions page This is in order to make changing these pages easier (see subsequent commits).
2017-08-17 10:49:21 +01:00
for index, expected in enumerate(expected_checkboxes):
expected_input_name, expected_checked = expected
assert checkboxes[index]['name'] == expected_input_name
assert checkboxes[index].has_attr('checked') == expected_checked
Add pages to invite, edit, and delete users This takes the original prototype version of this page, and, using the same fake data (ie nothing is wired up): - adds an invite users page - adds an edit (and delete) user page Both these pages allow the user to set another user’s permissions. This commit adds images for the ticks and crosses, so we have control over their appearance.
2016-02-19 15:02:13 +00:00
Test if auth type can be chosen while inviting user
2019-03-12 16:53:46 +00:00
def test_invite_user_allows_to_choose_auth(
client_request,
mock_get_users_by_service,
mock_get_template_folders,
service_one,
):
service_one['permissions'].append('email_auth')
page = client_request.get('main.invite_user', service_id=SERVICE_ONE_ID)
sms_auth_radio_button = page.select_one('input[value="sms_auth"]')
assert sms_auth_radio_button.has_attr("disabled") is False
Don’t allow editing of users from other services Currently when you load the ‘edit user’ page (which has a URL like `/service/<service_id>/users/<user_id>`) we check that: - you belong to the service represented by `service_id` - you have permission to edit users on this service We don’t check that: - the user represented by `user_id` belongs to this service This means that if you could somehow determine another user’s `user_id` (which I don’t think is possible if you don’t already have the manage service permission for that service) then you could: - edit their permissions on your service (weird, but wouldn’t have any effect) - change their email address (bad) This commit adds checks to return a `404` any time you’re looking at a service and trying to do stuff to a user who doesn’t belong to that service. We can’t add this check to the API easily because there are still times that we want to get/modify users outside the context of a service (eg platform admin pages, or users who have no services).
2019-02-25 16:51:37 +00:00
def test_should_not_show_page_for_non_team_member(
client_request,
mock_get_users_by_service,
):
client_request.get(
'main.edit_user_permissions',
service_id=SERVICE_ONE_ID,
user_id=USER_ONE_ID,
_expected_status=404,
)
Add a ‘See dashboard’ permission Our research and prototyping around ‘basic view’ found that: - a lot of users who send messages rarely or never look at the dashboard (yet it’s the first page they see when they sign in) - team managers like the idea of taking away things that users don’t need in order to make the interface simpler We’ve disentangled the simpler way of sending messages from being part of ‘basic view’. This means we can give managers the option of taking away the dashboard as an independent choice, not something that’s wrapped up in a separate ‘view’. I think that this checkbox is a more straightforward proposition than ‘basic view’ ever was (despite all the work we did to explain it and develop the nested checkbox pattern). In research users would often explain the feature back to us as being about hiding the dashboard – we should try to make Notify operate in terms of concepts that come naturally to people wherever possible.
2018-08-06 11:10:37 +01:00
@pytest.mark.parametrize('submitted_permissions, permissions_sent_to_api', [
(
{
'view_activity': 'y',
'send_messages': 'y',
'manage_templates': 'y',
'manage_service': 'y',
'manage_api_keys': 'y',
},
{
'view_activity',
remove all instances of db style permissions lots of renaming of send_texts/emails/letters to send_messages, and manage_settings/users to manage_service
2018-02-28 17:22:20 +00:00
'send_messages',
'manage_service',
Use `client` and `logged_in_client` fixtures Wherever possible, because Don’t Repeat Yourself.
2017-02-03 12:07:21 +00:00
'manage_templates',
'manage_api_keys',
}
Add a ‘See dashboard’ permission Our research and prototyping around ‘basic view’ found that: - a lot of users who send messages rarely or never look at the dashboard (yet it’s the first page they see when they sign in) - team managers like the idea of taking away things that users don’t need in order to make the interface simpler We’ve disentangled the simpler way of sending messages from being part of ‘basic view’. This means we can give managers the option of taking away the dashboard as an independent choice, not something that’s wrapped up in a separate ‘view’. I think that this checkbox is a more straightforward proposition than ‘basic view’ ever was (despite all the work we did to explain it and develop the nested checkbox pattern). In research users would often explain the feature back to us as being about hiding the dashboard – we should try to make Notify operate in terms of concepts that come naturally to people wherever possible.
2018-08-06 11:10:37 +01:00
),
(
{
'view_activity': 'y',
'send_messages': 'y',
'manage_templates': '',
},
{
'view_activity',
'send_messages',
}
),
(
{},
set(),
),
])
def test_edit_user_permissions(
client_request,
Normalize whitespace in test arguments We have a bunch of different styles of handling when function definitions span multiple lines, which they almost always do with tests. Here’s why an argument per line, single indent is best: - cleaner diffs when you change the name of a method (one line change instead of multiple lines) - works better on narrow screens, eg Github’s diff view, or with two terminals side by side on a laptop screen - works with any editor’s indenting shortcuts, no need for an IDE Also, trailing comma in the list of arguments is good because adding a new argument to a method becomes a one line, not two line diff.
2017-02-03 10:42:01 +00:00
mocker,
Don’t allow editing of users from other services Currently when you load the ‘edit user’ page (which has a URL like `/service/<service_id>/users/<user_id>`) we check that: - you belong to the service represented by `service_id` - you have permission to edit users on this service We don’t check that: - the user represented by `user_id` belongs to this service This means that if you could somehow determine another user’s `user_id` (which I don’t think is possible if you don’t already have the manage service permission for that service) then you could: - edit their permissions on your service (weird, but wouldn’t have any effect) - change their email address (bad) This commit adds checks to return a `404` any time you’re looking at a service and trying to do stuff to a user who doesn’t belong to that service. We can’t add this check to the API easily because there are still times that we want to get/modify users outside the context of a service (eg platform admin pages, or users who have no services).
2019-02-25 16:51:37 +00:00
mock_get_users_by_service,
Normalize whitespace in test arguments We have a bunch of different styles of handling when function definitions span multiple lines, which they almost always do with tests. Here’s why an argument per line, single indent is best: - cleaner diffs when you change the name of a method (one line change instead of multiple lines) - works better on narrow screens, eg Github’s diff view, or with two terminals side by side on a laptop screen - works with any editor’s indenting shortcuts, no need for an IDE Also, trailing comma in the list of arguments is good because adding a new argument to a method becomes a one line, not two line diff.
2017-02-03 10:42:01 +00:00
mock_get_invites_for_service,
mock_set_user_permissions,
Display nested folders permissions form on user permissions page We're reusing the logic for the `move_to` nested radios field for the user folder permissions nested checkboxes. The main difference between the two forms (aside from the different input type) is that "Move" form contains the root "Templates" as an option, whereas the folder permissions doesn't. It turns out that, because of the way NestedFieldMixin.children and select_nested macro are implemented the easiest way to get the desired folder permissions behaviour is to add the root folder as a choice with a `None` value and `NONE_OPTION_VALUE = None` set on the field, which allows the `child_map` to be constructed but doesn't display the root folder checkbox itself since it gets overwritten in the final `child_map`.
2019-02-25 16:23:28 +00:00
mock_get_template_folders,
Add a ‘See dashboard’ permission Our research and prototyping around ‘basic view’ found that: - a lot of users who send messages rarely or never look at the dashboard (yet it’s the first page they see when they sign in) - team managers like the idea of taking away things that users don’t need in order to make the interface simpler We’ve disentangled the simpler way of sending messages from being part of ‘basic view’. This means we can give managers the option of taking away the dashboard as an independent choice, not something that’s wrapped up in a separate ‘view’. I think that this checkbox is a more straightforward proposition than ‘basic view’ ever was (despite all the work we did to explain it and develop the nested checkbox pattern). In research users would often explain the feature back to us as being about hiding the dashboard – we should try to make Notify operate in terms of concepts that come naturally to people wherever possible.
2018-08-06 11:10:37 +01:00
fake_uuid,
submitted_permissions,
permissions_sent_to_api,
Add pages to invite, edit, and delete users This takes the original prototype version of this page, and, using the same fake data (ie nothing is wired up): - adds an invite users page - adds an edit (and delete) user page Both these pages allow the user to set another user’s permissions. This commit adds images for the ticks and crosses, so we have control over their appearance.
2016-02-19 15:02:13 +00:00
):
Add a ‘See dashboard’ permission Our research and prototyping around ‘basic view’ found that: - a lot of users who send messages rarely or never look at the dashboard (yet it’s the first page they see when they sign in) - team managers like the idea of taking away things that users don’t need in order to make the interface simpler We’ve disentangled the simpler way of sending messages from being part of ‘basic view’. This means we can give managers the option of taking away the dashboard as an independent choice, not something that’s wrapped up in a separate ‘view’. I think that this checkbox is a more straightforward proposition than ‘basic view’ ever was (despite all the work we did to explain it and develop the nested checkbox pattern). In research users would often explain the feature back to us as being about hiding the dashboard – we should try to make Notify operate in terms of concepts that come naturally to people wherever possible.
2018-08-06 11:10:37 +01:00
client_request.post(
'main.edit_user_permissions',
service_id=SERVICE_ONE_ID,
user_id=fake_uuid,
_data=dict(
email_address="test@example.com",
**submitted_permissions
),
_expected_status=302,
_expected_redirect=url_for(
'main.manage_users',
service_id=SERVICE_ONE_ID,
_external=True,
),
Use `client` and `logged_in_client` fixtures Wherever possible, because Don’t Repeat Yourself.
2017-02-03 12:07:21 +00:00
)
mock_set_user_permissions.assert_called_with(
Add a ‘See dashboard’ permission Our research and prototyping around ‘basic view’ found that: - a lot of users who send messages rarely or never look at the dashboard (yet it’s the first page they see when they sign in) - team managers like the idea of taking away things that users don’t need in order to make the interface simpler We’ve disentangled the simpler way of sending messages from being part of ‘basic view’. This means we can give managers the option of taking away the dashboard as an independent choice, not something that’s wrapped up in a separate ‘view’. I think that this checkbox is a more straightforward proposition than ‘basic view’ ever was (despite all the work we did to explain it and develop the nested checkbox pattern). In research users would often explain the feature back to us as being about hiding the dashboard – we should try to make Notify operate in terms of concepts that come naturally to people wherever possible.
2018-08-06 11:10:37 +01:00
fake_uuid,
SERVICE_ONE_ID,
permissions=permissions_sent_to_api,
Send updated user folder permissions to the API Integrates the folder permissions form with the updated API endpoint to store changes in the user folders. Since user folder permissions are returned in the full list of template folders for the service we need to invalidate the cache key for it each time we update user permissions.
2019-02-27 15:45:18 +00:00
folder_permissions=None
)
def test_edit_user_folder_permissions(
client_request,
mocker,
service_one,
mock_get_users_by_service,
mock_get_invites_for_service,
mock_set_user_permissions,
mock_get_template_folders,
fake_uuid,
):
service_one['permissions'] = ['edit_folder_permissions']
mock_get_template_folders.return_value = [
{'id': 'folder-id-1', 'name': 'folder_one', 'parent_id': None, 'users_with_permission': []},
{'id': 'folder-id-2', 'name': 'folder_one', 'parent_id': None, 'users_with_permission': []},
{'id': 'folder-id-3', 'name': 'folder_one', 'parent_id': 'folder-id-1', 'users_with_permission': []},
]
client_request.post(
'main.edit_user_permissions',
service_id=SERVICE_ONE_ID,
user_id=fake_uuid,
_data=dict(
folder_permissions=['folder-id-1', 'folder-id-3']
),
_expected_status=302,
_expected_redirect=url_for(
'main.manage_users',
service_id=SERVICE_ONE_ID,
_external=True,
),
)
mock_set_user_permissions.assert_called_with(
fake_uuid,
SERVICE_ONE_ID,
permissions=set(),
folder_permissions=['folder-id-1', 'folder-id-3']
Use `client` and `logged_in_client` fixtures Wherever possible, because Don’t Repeat Yourself.
2017-02-03 12:07:21 +00:00
)
Add pages to invite, edit, and delete users This takes the original prototype version of this page, and, using the same fake data (ie nothing is wired up): - adds an invite users page - adds an edit (and delete) user page Both these pages allow the user to set another user’s permissions. This commit adds images for the ticks and crosses, so we have control over their appearance.
2016-02-19 15:02:13 +00:00
Don’t allow editing of users from other services Currently when you load the ‘edit user’ page (which has a URL like `/service/<service_id>/users/<user_id>`) we check that: - you belong to the service represented by `service_id` - you have permission to edit users on this service We don’t check that: - the user represented by `user_id` belongs to this service This means that if you could somehow determine another user’s `user_id` (which I don’t think is possible if you don’t already have the manage service permission for that service) then you could: - edit their permissions on your service (weird, but wouldn’t have any effect) - change their email address (bad) This commit adds checks to return a `404` any time you’re looking at a service and trying to do stuff to a user who doesn’t belong to that service. We can’t add this check to the API easily because there are still times that we want to get/modify users outside the context of a service (eg platform admin pages, or users who have no services).
2019-02-25 16:51:37 +00:00
def test_cant_edit_non_member_user_permissions(
client_request,
mocker,
mock_get_users_by_service,
mock_set_user_permissions,
):
client_request.post(
'main.edit_user_permissions',
service_id=SERVICE_ONE_ID,
user_id=USER_ONE_ID,
_data={
'email_address': 'test@example.com',
'manage_service': 'y',
},
_expected_status=404,
)
assert mock_set_user_permissions.called is False
Email auth for inviting members and editing permissions
2017-11-01 15:36:27 +00:00
@pytest.mark.parametrize('auth_type', ['email_auth', 'sms_auth'])
def test_edit_user_permissions_including_authentication_with_email_auth_service(
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
client_request,
service_one,
Email auth for inviting members and editing permissions
2017-11-01 15:36:27 +00:00
active_user_with_permissions,
Don’t allow editing of users from other services Currently when you load the ‘edit user’ page (which has a URL like `/service/<service_id>/users/<user_id>`) we check that: - you belong to the service represented by `service_id` - you have permission to edit users on this service We don’t check that: - the user represented by `user_id` belongs to this service This means that if you could somehow determine another user’s `user_id` (which I don’t think is possible if you don’t already have the manage service permission for that service) then you could: - edit their permissions on your service (weird, but wouldn’t have any effect) - change their email address (bad) This commit adds checks to return a `404` any time you’re looking at a service and trying to do stuff to a user who doesn’t belong to that service. We can’t add this check to the API easily because there are still times that we want to get/modify users outside the context of a service (eg platform admin pages, or users who have no services).
2019-02-25 16:51:37 +00:00
mock_get_users_by_service,
Email auth for inviting members and editing permissions
2017-11-01 15:36:27 +00:00
mock_get_invites_for_service,
mock_set_user_permissions,
mock_update_user_attribute,
Display nested folders permissions form on user permissions page We're reusing the logic for the `move_to` nested radios field for the user folder permissions nested checkboxes. The main difference between the two forms (aside from the different input type) is that "Move" form contains the root "Templates" as an option, whereas the folder permissions doesn't. It turns out that, because of the way NestedFieldMixin.children and select_nested macro are implemented the easiest way to get the desired folder permissions behaviour is to add the root folder as a choice with a `None` value and `NONE_OPTION_VALUE = None` set on the field, which allows the `child_map` to be constructed but doesn't display the root folder checkbox itself since it gets overwritten in the final `child_map`.
2019-02-25 16:23:28 +00:00
auth_type,
mock_get_template_folders
Email auth for inviting members and editing permissions
2017-11-01 15:36:27 +00:00
):
service_one['permissions'].append('email_auth')
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
client_request.post(
'main.edit_user_permissions',
service_id=SERVICE_ONE_ID,
user_id=active_user_with_permissions.id,
_data={
Email auth for inviting members and editing permissions
2017-11-01 15:36:27 +00:00
'email_address': active_user_with_permissions.email_address,
'send_messages': 'y',
'manage_templates': 'y',
'manage_service': 'y',
'manage_api_keys': 'y',
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
'login_authentication': auth_type,
},
_expected_status=302,
_expected_redirect=url_for(
'main.manage_users',
service_id=SERVICE_ONE_ID,
_external=True,
),
Email auth for inviting members and editing permissions
2017-11-01 15:36:27 +00:00
)
mock_set_user_permissions.assert_called_with(
str(active_user_with_permissions.id),
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
SERVICE_ONE_ID,
Email auth for inviting members and editing permissions
2017-11-01 15:36:27 +00:00
permissions={
remove all instances of db style permissions lots of renaming of send_texts/emails/letters to send_messages, and manage_settings/users to manage_service
2018-02-28 17:22:20 +00:00
'send_messages',
Email auth for inviting members and editing permissions
2017-11-01 15:36:27 +00:00
'manage_templates',
remove all instances of db style permissions lots of renaming of send_texts/emails/letters to send_messages, and manage_settings/users to manage_service
2018-02-28 17:22:20 +00:00
'manage_service',
Email auth for inviting members and editing permissions
2017-11-01 15:36:27 +00:00
'manage_api_keys',
Send updated user folder permissions to the API Integrates the folder permissions form with the updated API endpoint to store changes in the user folders. Since user folder permissions are returned in the full list of template folders for the service we need to invalidate the cache key for it each time we update user permissions.
2019-02-27 15:45:18 +00:00
},
folder_permissions=None
Email auth for inviting members and editing permissions
2017-11-01 15:36:27 +00:00
)
mock_update_user_attribute.assert_called_with(
str(active_user_with_permissions.id),
auth_type=auth_type
)
Add pages to invite, edit, and delete users This takes the original prototype version of this page, and, using the same fake data (ie nothing is wired up): - adds an invite users page - adds an edit (and delete) user page Both these pages allow the user to set another user’s permissions. This commit adds images for the ticks and crosses, so we have control over their appearance.
2016-02-19 15:02:13 +00:00
def test_should_show_page_for_inviting_user(
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
client_request,
Set folder permissions when creating and accepting invites to services Added a folder permissions form to the page to invite users to services. This only shows if the service has 'edit_folder_permissions' enabled, and all folder checkboxes are checked by default. This change means that InviteApiClient.create_invite now sends folder_permissions through to notifications_api (so invites get created with folder permissions). Started passing the folder_permissions through to notifications-api when accepting an invite. This changes UserApiClient.add_user_to_service to send folder_permissions to notifications_api so that new users get folder permissions when they are added to the service.
2019-03-15 14:57:39 +00:00
mock_get_template_folders,
Add pages to invite, edit, and delete users This takes the original prototype version of this page, and, using the same fake data (ie nothing is wired up): - adds an invite users page - adds an edit (and delete) user page Both these pages allow the user to set another user’s permissions. This commit adds images for the ticks and crosses, so we have control over their appearance.
2016-02-19 15:02:13 +00:00
):
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
page = client_request.get(
'main.invite_user',
service_id=SERVICE_ONE_ID,
)
Add pages to invite, edit, and delete users This takes the original prototype version of this page, and, using the same fake data (ie nothing is wired up): - adds an invite users page - adds an edit (and delete) user page Both these pages allow the user to set another user’s permissions. This commit adds images for the ticks and crosses, so we have control over their appearance.
2016-02-19 15:02:13 +00:00
Set folder permissions when creating and accepting invites to services Added a folder permissions form to the page to invite users to services. This only shows if the service has 'edit_folder_permissions' enabled, and all folder checkboxes are checked by default. This change means that InviteApiClient.create_invite now sends folder_permissions through to notifications_api (so invites get created with folder permissions). Started passing the folder_permissions through to notifications-api when accepting an invite. This changes UserApiClient.add_user_to_service to send folder_permissions to notifications_api so that new users get folder permissions when they are added to the service.
2019-03-15 14:57:39 +00:00
assert 'Invite a team member' in page.find('h1').text.strip()
assert not page.find('div', class_='checkboxes-nested')
def test_should_show_folder_permission_form_if_service_has_folder_permissions_enabled(
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
client_request,
Set folder permissions when creating and accepting invites to services Added a folder permissions form to the page to invite users to services. This only shows if the service has 'edit_folder_permissions' enabled, and all folder checkboxes are checked by default. This change means that InviteApiClient.create_invite now sends folder_permissions through to notifications_api (so invites get created with folder permissions). Started passing the folder_permissions through to notifications-api when accepting an invite. This changes UserApiClient.add_user_to_service to send folder_permissions to notifications_api so that new users get folder permissions when they are added to the service.
2019-03-15 14:57:39 +00:00
mocker,
mock_get_template_folders,
service_one
):
service_one['permissions'].append('edit_folder_permissions')
mock_get_template_folders.return_value = [
{'id': 'folder-id-1', 'name': 'folder_one', 'parent_id': None, 'users_with_permission': []},
{'id': 'folder-id-2', 'name': 'folder_two', 'parent_id': None, 'users_with_permission': []},
{'id': 'folder-id-3', 'name': 'folder_three', 'parent_id': 'folder-id-1', 'users_with_permission': []},
]
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
page = client_request.get(
'main.invite_user',
service_id=SERVICE_ONE_ID,
)
Set folder permissions when creating and accepting invites to services Added a folder permissions form to the page to invite users to services. This only shows if the service has 'edit_folder_permissions' enabled, and all folder checkboxes are checked by default. This change means that InviteApiClient.create_invite now sends folder_permissions through to notifications_api (so invites get created with folder permissions). Started passing the folder_permissions through to notifications-api when accepting an invite. This changes UserApiClient.add_user_to_service to send folder_permissions to notifications_api so that new users get folder permissions when they are added to the service.
2019-03-15 14:57:39 +00:00
assert 'Invite a team member' in page.find('h1').text.strip()
folder_checkboxes = page.find('div', class_='checkboxes-nested').find_all('li')
assert len(folder_checkboxes) == 3
Add pages to invite, edit, and delete users This takes the original prototype version of this page, and, using the same fake data (ie nothing is wired up): - adds an invite users page - adds an edit (and delete) user page Both these pages allow the user to set another user’s permissions. This commit adds images for the ticks and crosses, so we have control over their appearance.
2016-02-19 15:02:13 +00:00
[WIP] Start of user accepting invite. This commit only deals with acceptance by users who are already in system. Changed invite client to return invited user objects instead of dictionaries. Added commented out test. fixed up fixtures to return invited user object for invites
2016-02-29 17:35:21 +00:00
Add tests to ensure non gov user cannot see, access or create service
2016-10-28 10:48:29 +01:00
@pytest.mark.parametrize('email_address, gov_user', [
Add test to invite nonwhitelist user and refactor
2016-10-25 18:13:50 +01:00
('test@example.gov.uk', True),
('test@nonwhitelist.com', False)
])
[WIP] Start of user accepting invite. This commit only deals with acceptance by users who are already in system. Changed invite client to return invited user objects instead of dictionaries. Added commented out test. fixed up fixtures to return invited user object for invites
2016-02-29 17:35:21 +00:00
def test_invite_user(
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
client_request,
Normalize whitespace in test arguments We have a bunch of different styles of handling when function definitions span multiple lines, which they almost always do with tests. Here’s why an argument per line, single indent is best: - cleaner diffs when you change the name of a method (one line change instead of multiple lines) - works better on narrow screens, eg Github’s diff view, or with two terminals side by side on a laptop screen - works with any editor’s indenting shortcuts, no need for an IDE Also, trailing comma in the list of arguments is good because adding a new argument to a method becomes a one line, not two line diff.
2017-02-03 10:42:01 +00:00
active_user_with_permissions,
mocker,
sample_invite,
email_address,
gov_user,
Set folder permissions when creating and accepting invites to services Added a folder permissions form to the page to invite users to services. This only shows if the service has 'edit_folder_permissions' enabled, and all folder checkboxes are checked by default. This change means that InviteApiClient.create_invite now sends folder_permissions through to notifications_api (so invites get created with folder permissions). Started passing the folder_permissions through to notifications-api when accepting an invite. This changes UserApiClient.add_user_to_service to send folder_permissions to notifications_api so that new users get folder permissions when they are added to the service.
2019-03-15 14:57:39 +00:00
mock_get_template_folders,
[WIP] Start of user accepting invite. This commit only deals with acceptance by users who are already in system. Changed invite client to return invited user objects instead of dictionaries. Added commented out test. fixed up fixtures to return invited user object for invites
2016-02-29 17:35:21 +00:00
):
[WIP]: fixing unit tests
2016-03-18 10:49:22 +00:00
sample_invite['email_address'] = 'test@example.gov.uk'
[WIP] Start of user accepting invite. This commit only deals with acceptance by users who are already in system. Changed invite client to return invited user objects instead of dictionaries. Added commented out test. fixed up fixtures to return invited user object for invites
2016-02-29 17:35:21 +00:00
[WIP]: fixing unit tests
2016-03-18 10:49:22 +00:00
data = [InvitedUser(**sample_invite)]
Use `client` and `logged_in_client` fixtures Wherever possible, because Don’t Repeat Yourself.
2017-02-03 12:07:21 +00:00
assert is_gov_user(email_address) == gov_user
mocker.patch('app.invite_api_client.get_invites_for_service', return_value=data)
mocker.patch('app.user_api_client.get_users_for_service', return_value=[active_user_with_permissions])
mocker.patch('app.invite_api_client.create_invite', return_value=InvitedUser(**sample_invite))
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
page = client_request.post(
'main.invite_user',
service_id=SERVICE_ONE_ID,
_data={
'email_address': email_address,
'view_activity': 'y',
'send_messages': 'y',
'manage_templates': 'y',
'manage_service': 'y',
'manage_api_keys': 'y',
},
_follow_redirects=True,
Use `client` and `logged_in_client` fixtures Wherever possible, because Don’t Repeat Yourself.
2017-02-03 12:07:21 +00:00
)
assert page.h1.string.strip() == 'Team members'
flash_banner = page.find('div', class_='banner-default-with-tick').string.strip()
assert flash_banner == 'Invite sent to test@example.gov.uk'
remove all instances of db style permissions lots of renaming of send_texts/emails/letters to send_messages, and manage_settings/users to manage_service
2018-02-28 17:22:20 +00:00
expected_permissions = {'manage_api_keys', 'manage_service', 'manage_templates', 'send_messages', 'view_activity'}
Use `client` and `logged_in_client` fixtures Wherever possible, because Don’t Repeat Yourself.
2017-02-03 12:07:21 +00:00
app.invite_api_client.create_invite.assert_called_once_with(sample_invite['from_user'],
sample_invite['service'],
email_address,
Email auth for inviting members and editing permissions
2017-11-01 15:36:27 +00:00
expected_permissions,
Set folder permissions when creating and accepting invites to services Added a folder permissions form to the page to invite users to services. This only shows if the service has 'edit_folder_permissions' enabled, and all folder checkboxes are checked by default. This change means that InviteApiClient.create_invite now sends folder_permissions through to notifications_api (so invites get created with folder permissions). Started passing the folder_permissions through to notifications-api when accepting an invite. This changes UserApiClient.add_user_to_service to send folder_permissions to notifications_api so that new users get folder permissions when they are added to the service.
2019-03-15 14:57:39 +00:00
'sms_auth',
Send folders if inviting user for service without edit folder permissions If a new user is being invited for a service which doesn't have edit folder permissions turned on, we want to send all folders for that service to api.
2019-03-19 14:16:23 +00:00
[])
Email auth for inviting members and editing permissions
2017-11-01 15:36:27 +00:00
@pytest.mark.parametrize('auth_type', [
('sms_auth'),
('email_auth')
])
@pytest.mark.parametrize('email_address, gov_user', [
('test@example.gov.uk', True),
('test@nonwhitelist.com', False)
])
def test_invite_user_with_email_auth_service(
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
client_request,
service_one,
Email auth for inviting members and editing permissions
2017-11-01 15:36:27 +00:00
active_user_with_permissions,
sample_invite,
email_address,
gov_user,
mocker,
Set folder permissions when creating and accepting invites to services Added a folder permissions form to the page to invite users to services. This only shows if the service has 'edit_folder_permissions' enabled, and all folder checkboxes are checked by default. This change means that InviteApiClient.create_invite now sends folder_permissions through to notifications_api (so invites get created with folder permissions). Started passing the folder_permissions through to notifications-api when accepting an invite. This changes UserApiClient.add_user_to_service to send folder_permissions to notifications_api so that new users get folder permissions when they are added to the service.
2019-03-15 14:57:39 +00:00
auth_type,
mock_get_template_folders,
Email auth for inviting members and editing permissions
2017-11-01 15:36:27 +00:00
):
service_one['permissions'].append('email_auth')
sample_invite['email_address'] = 'test@example.gov.uk'
data = [InvitedUser(**sample_invite)]
assert is_gov_user(email_address) == gov_user
mocker.patch('app.invite_api_client.get_invites_for_service', return_value=data)
mocker.patch('app.user_api_client.get_users_for_service', return_value=[active_user_with_permissions])
mocker.patch('app.invite_api_client.create_invite', return_value=InvitedUser(**sample_invite))
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
page = client_request.post(
'main.invite_user',
service_id=SERVICE_ONE_ID,
_data={
'email_address': email_address,
'view_activity': 'y',
'send_messages': 'y',
'manage_templates': 'y',
'manage_service': 'y',
'manage_api_keys': 'y',
'login_authentication': auth_type,
},
_follow_redirects=True,
_expected_status=200,
Email auth for inviting members and editing permissions
2017-11-01 15:36:27 +00:00
)
assert page.h1.string.strip() == 'Team members'
flash_banner = page.find('div', class_='banner-default-with-tick').string.strip()
assert flash_banner == 'Invite sent to test@example.gov.uk'
remove all instances of db style permissions lots of renaming of send_texts/emails/letters to send_messages, and manage_settings/users to manage_service
2018-02-28 17:22:20 +00:00
expected_permissions = {'manage_api_keys', 'manage_service', 'manage_templates', 'send_messages', 'view_activity'}
Email auth for inviting members and editing permissions
2017-11-01 15:36:27 +00:00
app.invite_api_client.create_invite.assert_called_once_with(sample_invite['from_user'],
sample_invite['service'],
email_address,
expected_permissions,
Set folder permissions when creating and accepting invites to services Added a folder permissions form to the page to invite users to services. This only shows if the service has 'edit_folder_permissions' enabled, and all folder checkboxes are checked by default. This change means that InviteApiClient.create_invite now sends folder_permissions through to notifications_api (so invites get created with folder permissions). Started passing the folder_permissions through to notifications-api when accepting an invite. This changes UserApiClient.add_user_to_service to send folder_permissions to notifications_api so that new users get folder permissions when they are added to the service.
2019-03-15 14:57:39 +00:00
auth_type,
Send folders if inviting user for service without edit folder permissions If a new user is being invited for a service which doesn't have edit folder permissions turned on, we want to send all folders for that service to api.
2019-03-19 14:16:23 +00:00
[])
def test_invite_user_sends_invite_with_all_folders_if_folder_permissions_not_enabled(
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
client_request,
Send folders if inviting user for service without edit folder permissions If a new user is being invited for a service which doesn't have edit folder permissions turned on, we want to send all folders for that service to api.
2019-03-19 14:16:23 +00:00
mocker,
mock_get_template_folders,
service_one
):
mocker.patch('app.invite_api_client.get_invites_for_service')
mocker.patch('app.user_api_client.get_users_for_service')
mock_get_template_folders.return_value = [
{'id': 'folder-id-1', 'name': 'folder_one', 'parent_id': None, 'users_with_permission': []},
{'id': 'folder-id-2', 'name': 'folder_two', 'parent_id': None, 'users_with_permission': []},
{'id': 'folder-id-3', 'name': 'folder_three', 'parent_id': 'folder-id-1', 'users_with_permission': []},
]
invite_mock = mocker.patch('app.invite_api_client.create_invite')
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
client_request.post(
'main.invite_user',
service_id=SERVICE_ONE_ID,
_data={
'email_address': 'user@example.com',
'send_messages': 'y',
},
_follow_redirects=True,
_expected_status=200,
Send folders if inviting user for service without edit folder permissions If a new user is being invited for a service which doesn't have edit folder permissions turned on, we want to send all folders for that service to api.
2019-03-19 14:16:23 +00:00
)
folder_data_sent = invite_mock.call_args[0][-1]
assert len(folder_data_sent) == 3
assert 'folder-id-1' in folder_data_sent
assert 'folder-id-2' in folder_data_sent
assert 'folder-id-3' in folder_data_sent
Add button to cancel invitation of invited user.
2016-03-01 16:12:26 +00:00
Normalize whitespace in test arguments We have a bunch of different styles of handling when function definitions span multiple lines, which they almost always do with tests. Here’s why an argument per line, single indent is best: - cleaner diffs when you change the name of a method (one line change instead of multiple lines) - works better on narrow screens, eg Github’s diff view, or with two terminals side by side on a laptop screen - works with any editor’s indenting shortcuts, no need for an IDE Also, trailing comma in the list of arguments is good because adding a new argument to a method becomes a one line, not two line diff.
2017-02-03 10:42:01 +00:00
def test_cancel_invited_user_cancels_user_invitations(
Don’t allow editing of users from other services Currently when you load the ‘edit user’ page (which has a URL like `/service/<service_id>/users/<user_id>`) we check that: - you belong to the service represented by `service_id` - you have permission to edit users on this service We don’t check that: - the user represented by `user_id` belongs to this service This means that if you could somehow determine another user’s `user_id` (which I don’t think is possible if you don’t already have the manage service permission for that service) then you could: - edit their permissions on your service (weird, but wouldn’t have any effect) - change their email address (bad) This commit adds checks to return a `404` any time you’re looking at a service and trying to do stuff to a user who doesn’t belong to that service. We can’t add this check to the API easily because there are still times that we want to get/modify users outside the context of a service (eg platform admin pages, or users who have no services).
2019-02-25 16:51:37 +00:00
client_request,
mock_get_invites_for_service,
Normalize whitespace in test arguments We have a bunch of different styles of handling when function definitions span multiple lines, which they almost always do with tests. Here’s why an argument per line, single indent is best: - cleaner diffs when you change the name of a method (one line change instead of multiple lines) - works better on narrow screens, eg Github’s diff view, or with two terminals side by side on a laptop screen - works with any editor’s indenting shortcuts, no need for an IDE Also, trailing comma in the list of arguments is good because adding a new argument to a method becomes a one line, not two line diff.
2017-02-03 10:42:01 +00:00
active_user_with_permissions,
mocker,
):
Don’t allow editing of users from other services Currently when you load the ‘edit user’ page (which has a URL like `/service/<service_id>/users/<user_id>`) we check that: - you belong to the service represented by `service_id` - you have permission to edit users on this service We don’t check that: - the user represented by `user_id` belongs to this service This means that if you could somehow determine another user’s `user_id` (which I don’t think is possible if you don’t already have the manage service permission for that service) then you could: - edit their permissions on your service (weird, but wouldn’t have any effect) - change their email address (bad) This commit adds checks to return a `404` any time you’re looking at a service and trying to do stuff to a user who doesn’t belong to that service. We can’t add this check to the API easily because there are still times that we want to get/modify users outside the context of a service (eg platform admin pages, or users who have no services).
2019-02-25 16:51:37 +00:00
mock_cancel = mocker.patch('app.invite_api_client.cancel_invited_user')
client_request.get(
'main.cancel_invited_user',
service_id=SERVICE_ONE_ID,
invited_user_id=sample_uuid(),
_expected_status=302,
_expected_redirect=url_for(
'main.manage_users', service_id=SERVICE_ONE_ID, _external=True
),
)
mock_cancel.assert_called_once_with(
service_id=SERVICE_ONE_ID,
invited_user_id=sample_uuid(),
)
Add button to cancel invitation of invited user.
2016-03-01 16:12:26 +00:00
Don’t allow editing of users from other services Currently when you load the ‘edit user’ page (which has a URL like `/service/<service_id>/users/<user_id>`) we check that: - you belong to the service represented by `service_id` - you have permission to edit users on this service We don’t check that: - the user represented by `user_id` belongs to this service This means that if you could somehow determine another user’s `user_id` (which I don’t think is possible if you don’t already have the manage service permission for that service) then you could: - edit their permissions on your service (weird, but wouldn’t have any effect) - change their email address (bad) This commit adds checks to return a `404` any time you’re looking at a service and trying to do stuff to a user who doesn’t belong to that service. We can’t add this check to the API easily because there are still times that we want to get/modify users outside the context of a service (eg platform admin pages, or users who have no services).
2019-02-25 16:51:37 +00:00
def test_cancel_invited_user_doesnt_work_if_user_not_invited_to_this_service(
client_request,
mock_get_invites_for_service,
mocker,
):
mock_cancel = mocker.patch('app.invite_api_client.cancel_invited_user')
client_request.get(
'main.cancel_invited_user',
service_id=SERVICE_ONE_ID,
invited_user_id=USER_ONE_ID,
_expected_status=404,
)
assert mock_cancel.called is False
Filter out accepted invites from manage users page Added basic test of invite client.
2016-03-07 10:19:44 +00:00
Change display of cancelled users, fix edit link It’s confusing showing green ticks for cancelled invites. This commit changes the appearance so that only pending or active users (ie those that could actually do some damage) get green ticks. Also fixes missing edit links caused by instances of `User` having `.state` but instances of `InvitedUser` having `.status`. Right now these are two separate lists. Which makes it harder to add improvements that will make large numbers of users easier to manage.
2018-01-26 17:14:00 +00:00
@pytest.mark.parametrize('invite_status, expected_text', [
('pending', (
'invited_user@test.gov.uk (invited) '
Make the permission labels shorter They were feeling a bit heavy on the _Team members_ page. We can remove some words without losing any meaning.
2018-08-08 14:23:24 +01:00
'Can See dashboard '
'Can Send messages '
remove all instances of db style permissions lots of renaming of send_texts/emails/letters to send_messages, and manage_settings/users to manage_service
2018-02-28 17:22:20 +00:00
'Can’t Add and edit templates '
Make the permission labels shorter They were feeling a bit heavy on the _Team members_ page. We can remove some words without losing any meaning.
2018-08-08 14:23:24 +01:00
'Can Manage settings, team and usage '
Relabel existing permissions Since we have added a new, 5th permission the existing permissions should be relabelled so that the five make sense as a coherent set. We especially want to make sure that: - the labels work against the checkboxes and against the tick/crosses on the manage users page (a long time ago this page was layed out differently so didn’t have space for full labels) - there is no confusion between usage and reports This commit also: - re-adds a line about what all users can see (‘sent messages’) but continues to omit the additional bullet points about templates and team members (because we think this is clear enough from reading the permissions) - refactors the `Form` subclass so that the content and order of the permissions only have to be defined once - brings back the ‘permissions’ legend on the `fieldset`
2018-08-08 08:45:58 +01:00
'Can Manage API integration '
Change display of cancelled users, fix edit link It’s confusing showing green ticks for cancelled invites. This commit changes the appearance so that only pending or active users (ie those that could actually do some damage) get green ticks. Also fixes missing edit links caused by instances of `User` having `.state` but instances of `InvitedUser` having `.status`. Right now these are two separate lists. Which makes it harder to add improvements that will make large numbers of users easier to manage.
2018-01-26 17:14:00 +00:00
'Cancel invitation'
)),
('cancelled', (
'invited_user@test.gov.uk (cancelled invite) '
remove all instances of db style permissions lots of renaming of send_texts/emails/letters to send_messages, and manage_settings/users to manage_service
2018-02-28 17:22:20 +00:00
# all permissions are greyed out
Make the permission labels shorter They were feeling a bit heavy on the _Team members_ page. We can remove some words without losing any meaning.
2018-08-08 14:23:24 +01:00
'Can’t See dashboard '
'Can’t Send messages '
remove all instances of db style permissions lots of renaming of send_texts/emails/letters to send_messages, and manage_settings/users to manage_service
2018-02-28 17:22:20 +00:00
'Can’t Add and edit templates '
Make the permission labels shorter They were feeling a bit heavy on the _Team members_ page. We can remove some words without losing any meaning.
2018-08-08 14:23:24 +01:00
'Can’t Manage settings, team and usage '
Relabel existing permissions Since we have added a new, 5th permission the existing permissions should be relabelled so that the five make sense as a coherent set. We especially want to make sure that: - the labels work against the checkboxes and against the tick/crosses on the manage users page (a long time ago this page was layed out differently so didn’t have space for full labels) - there is no confusion between usage and reports This commit also: - re-adds a line about what all users can see (‘sent messages’) but continues to omit the additional bullet points about templates and team members (because we think this is clear enough from reading the permissions) - refactors the `Form` subclass so that the content and order of the permissions only have to be defined once - brings back the ‘permissions’ legend on the `fieldset`
2018-08-08 08:45:58 +01:00
'Can’t Manage API integration'
Change display of cancelled users, fix edit link It’s confusing showing green ticks for cancelled invites. This commit changes the appearance so that only pending or active users (ie those that could actually do some damage) get green ticks. Also fixes missing edit links caused by instances of `User` having `.state` but instances of `InvitedUser` having `.status`. Right now these are two separate lists. Which makes it harder to add improvements that will make large numbers of users easier to manage.
2018-01-26 17:14:00 +00:00
)),
])
Normalize whitespace in test arguments We have a bunch of different styles of handling when function definitions span multiple lines, which they almost always do with tests. Here’s why an argument per line, single indent is best: - cleaner diffs when you change the name of a method (one line change instead of multiple lines) - works better on narrow screens, eg Github’s diff view, or with two terminals side by side on a laptop screen - works with any editor’s indenting shortcuts, no need for an IDE Also, trailing comma in the list of arguments is good because adding a new argument to a method becomes a one line, not two line diff.
2017-02-03 10:42:01 +00:00
def test_manage_users_shows_invited_user(
Make existing tests use ``client_request fixture This commit makes the existing tests around user permissions less verbose by using the new `client_request` fixture. This fixture takes care of: - setting up a service - asserting that the response is `200` It also tests that the page titles, some of which didn’t match with the `<h1>`s, so this commit also fixes that mismatch.
2017-08-17 10:44:36 +01:00
client_request,
Normalize whitespace in test arguments We have a bunch of different styles of handling when function definitions span multiple lines, which they almost always do with tests. Here’s why an argument per line, single indent is best: - cleaner diffs when you change the name of a method (one line change instead of multiple lines) - works better on narrow screens, eg Github’s diff view, or with two terminals side by side on a laptop screen - works with any editor’s indenting shortcuts, no need for an IDE Also, trailing comma in the list of arguments is good because adding a new argument to a method becomes a one line, not two line diff.
2017-02-03 10:42:01 +00:00
mocker,
active_user_with_permissions,
sample_invite,
Change display of cancelled users, fix edit link It’s confusing showing green ticks for cancelled invites. This commit changes the appearance so that only pending or active users (ie those that could actually do some damage) get green ticks. Also fixes missing edit links caused by instances of `User` having `.state` but instances of `InvitedUser` having `.status`. Right now these are two separate lists. Which makes it harder to add improvements that will make large numbers of users easier to manage.
2018-01-26 17:14:00 +00:00
invite_status,
expected_text,
Normalize whitespace in test arguments We have a bunch of different styles of handling when function definitions span multiple lines, which they almost always do with tests. Here’s why an argument per line, single indent is best: - cleaner diffs when you change the name of a method (one line change instead of multiple lines) - works better on narrow screens, eg Github’s diff view, or with two terminals side by side on a laptop screen - works with any editor’s indenting shortcuts, no need for an IDE Also, trailing comma in the list of arguments is good because adding a new argument to a method becomes a one line, not two line diff.
2017-02-03 10:42:01 +00:00
):
Change display of cancelled users, fix edit link It’s confusing showing green ticks for cancelled invites. This commit changes the appearance so that only pending or active users (ie those that could actually do some damage) get green ticks. Also fixes missing edit links caused by instances of `User` having `.state` but instances of `InvitedUser` having `.status`. Right now these are two separate lists. Which makes it harder to add improvements that will make large numbers of users easier to manage.
2018-01-26 17:14:00 +00:00
sample_invite['status'] = invite_status
Filter out accepted invites from manage users page Added basic test of invite client.
2016-03-07 10:19:44 +00:00
data = [InvitedUser(**sample_invite)]
Use `client` and `logged_in_client` fixtures Wherever possible, because Don’t Repeat Yourself.
2017-02-03 12:07:21 +00:00
mocker.patch('app.invite_api_client.get_invites_for_service', return_value=data)
mocker.patch('app.user_api_client.get_users_for_service', return_value=[active_user_with_permissions])
Filter out accepted invites from manage users page Added basic test of invite client.
2016-03-07 10:19:44 +00:00
Make existing tests use ``client_request fixture This commit makes the existing tests around user permissions less verbose by using the new `client_request` fixture. This fixture takes care of: - setting up a service - asserting that the response is `200` It also tests that the page titles, some of which didn’t match with the `<h1>`s, so this commit also fixes that mismatch.
2017-08-17 10:44:36 +01:00
page = client_request.get('main.manage_users', service_id=SERVICE_ONE_ID)
Use `client` and `logged_in_client` fixtures Wherever possible, because Don’t Repeat Yourself.
2017-02-03 12:07:21 +00:00
assert page.h1.string.strip() == 'Team members'
Change display of cancelled users, fix edit link It’s confusing showing green ticks for cancelled invites. This commit changes the appearance so that only pending or active users (ie those that could actually do some damage) get green ticks. Also fixes missing edit links caused by instances of `User` having `.state` but instances of `InvitedUser` having `.status`. Right now these are two separate lists. Which makes it harder to add improvements that will make large numbers of users easier to manage.
2018-01-26 17:14:00 +00:00
assert normalize_spaces(page.select('.user-list-item')[0].text) == expected_text
Filter out accepted invites in template not client.
2016-03-07 13:59:54 +00:00
Normalize whitespace in test arguments We have a bunch of different styles of handling when function definitions span multiple lines, which they almost always do with tests. Here’s why an argument per line, single indent is best: - cleaner diffs when you change the name of a method (one line change instead of multiple lines) - works better on narrow screens, eg Github’s diff view, or with two terminals side by side on a laptop screen - works with any editor’s indenting shortcuts, no need for an IDE Also, trailing comma in the list of arguments is good because adding a new argument to a method becomes a one line, not two line diff.
2017-02-03 10:42:01 +00:00
def test_manage_users_does_not_show_accepted_invite(
Refactor filtering out accepted invites to client None of our model or view layer code should need to know about accepted invites. We don’t use them anywhere because once an invite is accepted that person is now a user. Putting this logic in the client means that: - none of the code calling the client needs to care about accepted invites - it’s easier to (if we want) update the API code to not return accepted invites
2018-11-30 17:39:39 +00:00
client_request,
Normalize whitespace in test arguments We have a bunch of different styles of handling when function definitions span multiple lines, which they almost always do with tests. Here’s why an argument per line, single indent is best: - cleaner diffs when you change the name of a method (one line change instead of multiple lines) - works better on narrow screens, eg Github’s diff view, or with two terminals side by side on a laptop screen - works with any editor’s indenting shortcuts, no need for an IDE Also, trailing comma in the list of arguments is good because adding a new argument to a method becomes a one line, not two line diff.
2017-02-03 10:42:01 +00:00
mocker,
active_user_with_permissions,
sample_invite,
):
Filter out accepted invites in template not client.
2016-03-07 13:59:54 +00:00
invited_user_id = uuid.uuid4()
sample_invite['id'] = invited_user_id
sample_invite['status'] = 'accepted'
Use `client` and `logged_in_client` fixtures Wherever possible, because Don’t Repeat Yourself.
2017-02-03 12:07:21 +00:00
mocker.patch('app.user_api_client.get_users_for_service', return_value=[active_user_with_permissions])
Refactor filtering out accepted invites to client None of our model or view layer code should need to know about accepted invites. We don’t use them anywhere because once an invite is accepted that person is now a user. Putting this logic in the client means that: - none of the code calling the client needs to care about accepted invites - it’s easier to (if we want) update the API code to not return accepted invites
2018-11-30 17:39:39 +00:00
mocker.patch('app.invite_api_client._get_invites_for_service', return_value=[sample_invite])
Filter out accepted invites in template not client.
2016-03-07 13:59:54 +00:00
Refactor filtering out accepted invites to client None of our model or view layer code should need to know about accepted invites. We don’t use them anywhere because once an invite is accepted that person is now a user. Putting this logic in the client means that: - none of the code calling the client needs to care about accepted invites - it’s easier to (if we want) update the API code to not return accepted invites
2018-11-30 17:39:39 +00:00
page = client_request.get('main.manage_users', service_id=SERVICE_ONE_ID)
Filter out accepted invites in template not client.
2016-03-07 13:59:54 +00:00
Use `client` and `logged_in_client` fixtures Wherever possible, because Don’t Repeat Yourself.
2017-02-03 12:07:21 +00:00
assert page.h1.string.strip() == 'Team members'
user_lists = page.find_all('div', {'class': 'user-list'})
assert len(user_lists) == 1
assert not page.find(text='invited_user@test.gov.uk')
Added validation to ensure user can't invite themselves. Refactored Invited user form into permissions and invite forms for use in invite and edit permissions. Added template for edit permissions.
2016-03-09 13:00:52 +00:00
def test_user_cant_invite_themselves(
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
client_request,
Normalize whitespace in test arguments We have a bunch of different styles of handling when function definitions span multiple lines, which they almost always do with tests. Here’s why an argument per line, single indent is best: - cleaner diffs when you change the name of a method (one line change instead of multiple lines) - works better on narrow screens, eg Github’s diff view, or with two terminals side by side on a laptop screen - works with any editor’s indenting shortcuts, no need for an IDE Also, trailing comma in the list of arguments is good because adding a new argument to a method becomes a one line, not two line diff.
2017-02-03 10:42:01 +00:00
mocker,
active_user_with_permissions,
mock_create_invite,
Set folder permissions when creating and accepting invites to services Added a folder permissions form to the page to invite users to services. This only shows if the service has 'edit_folder_permissions' enabled, and all folder checkboxes are checked by default. This change means that InviteApiClient.create_invite now sends folder_permissions through to notifications_api (so invites get created with folder permissions). Started passing the folder_permissions through to notifications-api when accepting an invite. This changes UserApiClient.add_user_to_service to send folder_permissions to notifications_api so that new users get folder permissions when they are added to the service.
2019-03-15 14:57:39 +00:00
mock_get_template_folders,
Added validation to ensure user can't invite themselves. Refactored Invited user form into permissions and invite forms for use in invite and edit permissions. Added template for edit permissions.
2016-03-09 13:00:52 +00:00
):
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
page = client_request.post(
'main.invite_user',
service_id=SERVICE_ONE_ID,
_data={
'email_address': active_user_with_permissions.email_address,
'send_messages': 'y',
'manage_service': 'y',
'manage_api_keys': 'y',
},
_follow_redirects=True,
_expected_status=200,
Use `client` and `logged_in_client` fixtures Wherever possible, because Don’t Repeat Yourself.
2017-02-03 12:07:21 +00:00
)
assert page.h1.string.strip() == 'Invite a team member'
form_error = page.find('span', class_='error-message').string.strip()
assert form_error == "You can’t send an invitation to yourself"
assert not mock_create_invite.called
Manage team, now has a view only version of the page which requires no permissions.
2016-03-21 15:25:19 +00:00
Normalize whitespace in test arguments We have a bunch of different styles of handling when function definitions span multiple lines, which they almost always do with tests. Here’s why an argument per line, single indent is best: - cleaner diffs when you change the name of a method (one line change instead of multiple lines) - works better on narrow screens, eg Github’s diff view, or with two terminals side by side on a laptop screen - works with any editor’s indenting shortcuts, no need for an IDE Also, trailing comma in the list of arguments is good because adding a new argument to a method becomes a one line, not two line diff.
2017-02-03 10:42:01 +00:00
def test_no_permission_manage_users_page(
use client_request to always check the response this'll avoid us accidentally 503ing and ignoring it in tests where we check for "assert my_content not present"
2018-05-03 17:00:05 +01:00
client_request,
Normalize whitespace in test arguments We have a bunch of different styles of handling when function definitions span multiple lines, which they almost always do with tests. Here’s why an argument per line, single indent is best: - cleaner diffs when you change the name of a method (one line change instead of multiple lines) - works better on narrow screens, eg Github’s diff view, or with two terminals side by side on a laptop screen - works with any editor’s indenting shortcuts, no need for an IDE Also, trailing comma in the list of arguments is good because adding a new argument to a method becomes a one line, not two line diff.
2017-02-03 10:42:01 +00:00
service_one,
use client_request to always check the response this'll avoid us accidentally 503ing and ignoring it in tests where we check for "assert my_content not present"
2018-05-03 17:00:05 +01:00
mock_get_users_by_service,
mock_get_invites_for_service,
Normalize whitespace in test arguments We have a bunch of different styles of handling when function definitions span multiple lines, which they almost always do with tests. Here’s why an argument per line, single indent is best: - cleaner diffs when you change the name of a method (one line change instead of multiple lines) - works better on narrow screens, eg Github’s diff view, or with two terminals side by side on a laptop screen - works with any editor’s indenting shortcuts, no need for an IDE Also, trailing comma in the list of arguments is good because adding a new argument to a method becomes a one line, not two line diff.
2017-02-03 10:42:01 +00:00
api_user_active,
mocker,
):
use client_request to always check the response this'll avoid us accidentally 503ing and ignoring it in tests where we check for "assert my_content not present"
2018-05-03 17:00:05 +01:00
resp_text = client_request.get('main.manage_users', service_id=service_one['id'])
Use `client` and `logged_in_client` fixtures Wherever possible, because Don’t Repeat Yourself.
2017-02-03 12:07:21 +00:00
assert url_for('.invite_user', service_id=service_one['id']) not in resp_text
assert "Edit permission" not in resp_text
assert "Team members" not in resp_text
Functionality added and all tests working. Update correct use of permissions form.
2016-03-23 10:46:31 +00:00
add count of folders visible per user to the team members page Shows a count of how many folders that user can see - this doesn't do anything smart with parent folder stuff, it's just "how many checkboxes are ticked on the edit page". * doesn't show if service has no folders * doesn't show if service hasn't got folder permissions enabled
2019-04-04 17:55:37 +01:00
@pytest.mark.parametrize('folders_user_can_see, expected_message', [
(3, 'Can see all folders'),
(2, 'Can see 2 folders'),
(1, 'Can see 1 folder'),
(0, 'Cannot see any folders'),
])
def test_manage_user_page_shows_how_many_folders_user_can_view(
client_request,
service_one,
mock_get_template_folders,
mock_get_users_by_service,
mock_get_invites_for_service,
api_user_active,
folders_user_can_see,
expected_message
):
service_one['permissions'] = ['edit_folder_permissions']
mock_get_template_folders.return_value = [
{'id': 'folder-id-1', 'name': 'f1', 'parent_id': None, 'users_with_permission': []},
{'id': 'folder-id-2', 'name': 'f2', 'parent_id': None, 'users_with_permission': []},
{'id': 'folder-id-3', 'name': 'f3', 'parent_id': None, 'users_with_permission': []},
]
for i in range(folders_user_can_see):
mock_get_template_folders.return_value[i]['users_with_permission'].append(api_user_active.id)
page = client_request.get('main.manage_users', service_id=service_one['id'])
user_div = page.select_one("h3[title='notify@digital.cabinet-office.gov.uk']").parent
assert user_div.select_one('.tick-cross-list-hint:last-child').text.strip() == expected_message
def test_manage_user_page_doesnt_show_folder_hint_if_service_has_no_folders(
client_request,
service_one,
mock_get_template_folders,
mock_get_users_by_service,
mock_get_invites_for_service,
api_user_active,
):
service_one['permissions'] = ['edit_folder_permissions']
mock_get_template_folders.return_value = []
page = client_request.get('main.manage_users', service_id=service_one['id'])
user_div = page.select_one("h3[title='notify@digital.cabinet-office.gov.uk']").parent
assert user_div.find('.tick-cross-list-hint:last-child') is None
def test_manage_user_page_doesnt_show_folder_hint_if_service_cant_edit_folder_permissions(
client_request,
service_one,
mock_get_template_folders,
mock_get_users_by_service,
mock_get_invites_for_service,
api_user_active
):
service_one['permissions'] = []
mock_get_template_folders.return_value = [
{'id': 'folder-id-1', 'name': 'f1', 'parent_id': None, 'users_with_permission': [api_user_active.id]},
]
page = client_request.get('main.manage_users', service_id=service_one['id'])
user_div = page.select_one("h3[title='notify@digital.cabinet-office.gov.uk']").parent
assert user_div.find('.tick-cross-list-hint:last-child') is None
remove the remove-user get method remove `confirm` from `confirm_remove_user_from_service` as there's only one action now that the initial confirmation prompt takes place on the edit permissions page
2019-03-26 15:51:44 +00:00
def test_remove_user_from_service(
fix user permissions save button sometimes deleting when you hit the delete button, it flashes the delete button and takes you to the `/service/../user/../delete` url. If you then click the save button, it would make a POST to the delete URL... and delete the user. now the page stays on the edit url, but adds a `?delete=yes` query string. The dangerous flash banner now has an action field which defines where the browser will make the POST to (which remains at /delete).
2019-03-14 17:31:51 +00:00
client_request,
Normalize whitespace in test arguments We have a bunch of different styles of handling when function definitions span multiple lines, which they almost always do with tests. Here’s why an argument per line, single indent is best: - cleaner diffs when you change the name of a method (one line change instead of multiple lines) - works better on narrow screens, eg Github’s diff view, or with two terminals side by side on a laptop screen - works with any editor’s indenting shortcuts, no need for an IDE Also, trailing comma in the list of arguments is good because adding a new argument to a method becomes a one line, not two line diff.
2017-02-03 10:42:01 +00:00
active_user_with_permissions,
service_one,
mock_remove_user_from_service,
):
fix user permissions save button sometimes deleting when you hit the delete button, it flashes the delete button and takes you to the `/service/../user/../delete` url. If you then click the save button, it would make a POST to the delete URL... and delete the user. now the page stays on the edit url, but adds a `?delete=yes` query string. The dangerous flash banner now has an action field which defines where the browser will make the POST to (which remains at /delete).
2019-03-14 17:31:51 +00:00
client_request.post(
remove the remove-user get method remove `confirm` from `confirm_remove_user_from_service` as there's only one action now that the initial confirmation prompt takes place on the edit permissions page
2019-03-26 15:51:44 +00:00
'main.remove_user_from_service',
fix user permissions save button sometimes deleting when you hit the delete button, it flashes the delete button and takes you to the `/service/../user/../delete` url. If you then click the save button, it would make a POST to the delete URL... and delete the user. now the page stays on the edit url, but adds a `?delete=yes` query string. The dangerous flash banner now has an action field which defines where the browser will make the POST to (which remains at /delete).
2019-03-14 17:31:51 +00:00
service_id=service_one['id'],
user_id=active_user_with_permissions.id,
_expected_redirect=url_for('main.manage_users', service_id=service_one['id'], _external=True)
)
mock_remove_user_from_service.assert_called_once_with(
service_one['id'],
str(active_user_with_permissions.id)
)
Added invite user button for platform admins on any service
2016-06-03 11:10:38 +01:00
def test_can_invite_user_as_platform_admin(
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
client_request,
Normalize whitespace in test arguments We have a bunch of different styles of handling when function definitions span multiple lines, which they almost always do with tests. Here’s why an argument per line, single indent is best: - cleaner diffs when you change the name of a method (one line change instead of multiple lines) - works better on narrow screens, eg Github’s diff view, or with two terminals side by side on a laptop screen - works with any editor’s indenting shortcuts, no need for an IDE Also, trailing comma in the list of arguments is good because adding a new argument to a method becomes a one line, not two line diff.
2017-02-03 10:42:01 +00:00
service_one,
platform_admin_user,
active_user_with_permissions,
mock_get_invites_for_service,
mocker,
):
Use `client` and `logged_in_client` fixtures Wherever possible, because Don’t Repeat Yourself.
2017-02-03 12:07:21 +00:00
mocker.patch('app.user_api_client.get_users_for_service', return_value=[active_user_with_permissions])
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
page = client_request.get(
'main.manage_users',
service_id=SERVICE_ONE_ID,
)
assert url_for('.invite_user', service_id=service_one['id']) in str(page)
Add a get view and template that enable changing team members email
2019-02-19 15:35:51 +00:00
def test_edit_user_email_page(
client_request,
active_user_with_permissions,
service_one,
Don’t allow editing of users from other services Currently when you load the ‘edit user’ page (which has a URL like `/service/<service_id>/users/<user_id>`) we check that: - you belong to the service represented by `service_id` - you have permission to edit users on this service We don’t check that: - the user represented by `user_id` belongs to this service This means that if you could somehow determine another user’s `user_id` (which I don’t think is possible if you don’t already have the manage service permission for that service) then you could: - edit their permissions on your service (weird, but wouldn’t have any effect) - change their email address (bad) This commit adds checks to return a `404` any time you’re looking at a service and trying to do stuff to a user who doesn’t belong to that service. We can’t add this check to the API easily because there are still times that we want to get/modify users outside the context of a service (eg platform admin pages, or users who have no services).
2019-02-25 16:51:37 +00:00
mock_get_users_by_service,
Add a get view and template that enable changing team members email
2019-02-19 15:35:51 +00:00
mocker
):
user = active_user_with_permissions
Don’t allow editing of users from other services Currently when you load the ‘edit user’ page (which has a URL like `/service/<service_id>/users/<user_id>`) we check that: - you belong to the service represented by `service_id` - you have permission to edit users on this service We don’t check that: - the user represented by `user_id` belongs to this service This means that if you could somehow determine another user’s `user_id` (which I don’t think is possible if you don’t already have the manage service permission for that service) then you could: - edit their permissions on your service (weird, but wouldn’t have any effect) - change their email address (bad) This commit adds checks to return a `404` any time you’re looking at a service and trying to do stuff to a user who doesn’t belong to that service. We can’t add this check to the API easily because there are still times that we want to get/modify users outside the context of a service (eg platform admin pages, or users who have no services).
2019-02-25 16:51:37 +00:00
mocker.patch('app.user_api_client.get_user', return_value=user)
Add a get view and template that enable changing team members email
2019-02-19 15:35:51 +00:00
page = client_request.get(
'main.edit_user_email',
service_id=service_one['id'],
Don’t allow editing of users from other services Currently when you load the ‘edit user’ page (which has a URL like `/service/<service_id>/users/<user_id>`) we check that: - you belong to the service represented by `service_id` - you have permission to edit users on this service We don’t check that: - the user represented by `user_id` belongs to this service This means that if you could somehow determine another user’s `user_id` (which I don’t think is possible if you don’t already have the manage service permission for that service) then you could: - edit their permissions on your service (weird, but wouldn’t have any effect) - change their email address (bad) This commit adds checks to return a `404` any time you’re looking at a service and trying to do stuff to a user who doesn’t belong to that service. We can’t add this check to the API easily because there are still times that we want to get/modify users outside the context of a service (eg platform admin pages, or users who have no services).
2019-02-25 16:51:37 +00:00
user_id=sample_uuid()
Add a get view and template that enable changing team members email
2019-02-19 15:35:51 +00:00
)
Content changes following consultation with content and design pros :)
2019-02-20 17:34:58 +00:00
assert page.find('h1').text == "Change team member’s email address"
assert page.select('p[id=user_name]')[0].text == "This will change the email address for {}.".format(user.name)
Add a get view and template that enable changing team members email
2019-02-19 15:35:51 +00:00
assert page.select('input[type=email]')[0].attrs["value"] == user.email_address
Content changes following consultation with content and design pros :)
2019-02-20 17:34:58 +00:00
assert page.select('button[type=submit]')[0].text == "Save"
Redirect to confirmation page
2019-02-19 18:01:01 +00:00
Don’t allow editing of users from other services Currently when you load the ‘edit user’ page (which has a URL like `/service/<service_id>/users/<user_id>`) we check that: - you belong to the service represented by `service_id` - you have permission to edit users on this service We don’t check that: - the user represented by `user_id` belongs to this service This means that if you could somehow determine another user’s `user_id` (which I don’t think is possible if you don’t already have the manage service permission for that service) then you could: - edit their permissions on your service (weird, but wouldn’t have any effect) - change their email address (bad) This commit adds checks to return a `404` any time you’re looking at a service and trying to do stuff to a user who doesn’t belong to that service. We can’t add this check to the API easily because there are still times that we want to get/modify users outside the context of a service (eg platform admin pages, or users who have no services).
2019-02-25 16:51:37 +00:00
def test_edit_user_email_page_404_for_non_team_member(
client_request,
mock_get_users_by_service,
):
client_request.get(
'main.edit_user_email',
service_id=SERVICE_ONE_ID,
user_id=USER_ONE_ID,
_expected_status=404,
)
Redirect to confirmation page
2019-02-19 18:01:01 +00:00
def test_edit_user_email_redirects_to_confirmation(
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
client_request,
Redirect to confirmation page
2019-02-19 18:01:01 +00:00
active_user_with_permissions,
Don’t allow editing of users from other services Currently when you load the ‘edit user’ page (which has a URL like `/service/<service_id>/users/<user_id>`) we check that: - you belong to the service represented by `service_id` - you have permission to edit users on this service We don’t check that: - the user represented by `user_id` belongs to this service This means that if you could somehow determine another user’s `user_id` (which I don’t think is possible if you don’t already have the manage service permission for that service) then you could: - edit their permissions on your service (weird, but wouldn’t have any effect) - change their email address (bad) This commit adds checks to return a `404` any time you’re looking at a service and trying to do stuff to a user who doesn’t belong to that service. We can’t add this check to the API easily because there are still times that we want to get/modify users outside the context of a service (eg platform admin pages, or users who have no services).
2019-02-25 16:51:37 +00:00
mock_get_users_by_service,
Redirect to confirmation page
2019-02-19 18:01:01 +00:00
):
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
client_request.post(
'main.edit_user_email',
service_id=SERVICE_ONE_ID,
Redirect to confirmation page
2019-02-19 18:01:01 +00:00
user_id=active_user_with_permissions.id,
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
_expected_status=302,
_expected_redirect=url_for(
'main.confirm_edit_user_email',
service_id=SERVICE_ONE_ID,
user_id=active_user_with_permissions.id,
_external=True,
),
Redirect to confirmation page
2019-02-19 18:01:01 +00:00
)
Confirm edit user email changes user email
2019-02-20 11:50:34 +00:00
Don’t error if email address hasn’t changed When updating a user’s email address you currently get an validation error if you save without changing it. Instead it should just obey your command. And no need for the confirmation step because nothing is actually changing.
2019-02-25 12:04:07 +00:00
def test_edit_user_email_without_changing_goes_back_to_team_members(
client_request,
active_user_with_permissions,
mock_get_user,
Don’t allow editing of users from other services Currently when you load the ‘edit user’ page (which has a URL like `/service/<service_id>/users/<user_id>`) we check that: - you belong to the service represented by `service_id` - you have permission to edit users on this service We don’t check that: - the user represented by `user_id` belongs to this service This means that if you could somehow determine another user’s `user_id` (which I don’t think is possible if you don’t already have the manage service permission for that service) then you could: - edit their permissions on your service (weird, but wouldn’t have any effect) - change their email address (bad) This commit adds checks to return a `404` any time you’re looking at a service and trying to do stuff to a user who doesn’t belong to that service. We can’t add this check to the API easily because there are still times that we want to get/modify users outside the context of a service (eg platform admin pages, or users who have no services).
2019-02-25 16:51:37 +00:00
mock_get_users_by_service,
Don’t error if email address hasn’t changed When updating a user’s email address you currently get an validation error if you save without changing it. Instead it should just obey your command. And no need for the confirmation step because nothing is actually changing.
2019-02-25 12:04:07 +00:00
mock_update_user_attribute,
):
client_request.post(
'main.edit_user_email',
service_id=SERVICE_ONE_ID,
user_id=active_user_with_permissions.id,
_data={
'email_address': active_user_with_permissions.email_address
},
_expected_status=302,
_expected_redirect=url_for(
'main.manage_users',
service_id=SERVICE_ONE_ID,
_external=True
),
)
assert mock_update_user_attribute.called is False
Allow non-gov email addresses to be changed to gov email addresses When a user's email address is updated, we not allowing it to be changed to a non-government email address. We now allow a non-gov email address to be changed to another non-gov email address. Government email addresses still cannot be changed to non-government email addresses. Also fixes the link in the error message on the ChangeEmailAddress form - this was being escaped before.
2019-04-18 16:03:13 +01:00
@pytest.mark.parametrize('original_email_address', ['test@gov.uk', 'test@example.com'])
def test_edit_user_email_can_change_any_email_address_to_a_gov_email_address(
client_request,
active_user_with_permissions,
mock_get_user,
mock_get_users_by_service,
mock_update_user_attribute,
original_email_address
):
active_user_with_permissions.email_address = original_email_address
client_request.post(
'main.edit_user_email',
service_id=SERVICE_ONE_ID,
user_id=active_user_with_permissions.id,
_data={
'email_address': 'new-email-address@gov.uk'
},
_expected_status=302,
_expected_redirect=url_for(
'main.confirm_edit_user_email',
service_id=SERVICE_ONE_ID,
user_id=active_user_with_permissions.id,
_external=True
),
)
def test_edit_user_email_can_change_a_non_gov_email_address_to_another_non_gov_email_address(
client_request,
active_user_with_permissions,
mock_get_user,
mock_get_users_by_service,
mock_update_user_attribute,
):
active_user_with_permissions.email_address = 'old@example.com'
client_request.post(
'main.edit_user_email',
service_id=SERVICE_ONE_ID,
user_id=active_user_with_permissions.id,
_data={
'email_address': 'new@example.com'
},
_expected_status=302,
_expected_redirect=url_for(
'main.confirm_edit_user_email',
service_id=SERVICE_ONE_ID,
user_id=active_user_with_permissions.id,
_external=True
),
)
def test_edit_user_email_cannot_change_a_gov_email_address_to_a_non_gov_email_address(
client_request,
active_user_with_permissions,
mock_get_user,
mock_get_users_by_service,
mock_update_user_attribute,
):
page = client_request.post(
'main.edit_user_email',
service_id=SERVICE_ONE_ID,
user_id=active_user_with_permissions.id,
_data={
'email_address': 'new_email@example.com'
},
_expected_status=200,
)
assert 'Enter a government email address.' in page.find('span', class_='error-message').text
Confirm edit user email changes user email
2019-02-20 11:50:34 +00:00
def test_confirm_edit_user_email_page(
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
client_request,
Confirm edit user email changes user email
2019-02-20 11:50:34 +00:00
active_user_with_permissions,
Don’t allow editing of users from other services Currently when you load the ‘edit user’ page (which has a URL like `/service/<service_id>/users/<user_id>`) we check that: - you belong to the service represented by `service_id` - you have permission to edit users on this service We don’t check that: - the user represented by `user_id` belongs to this service This means that if you could somehow determine another user’s `user_id` (which I don’t think is possible if you don’t already have the manage service permission for that service) then you could: - edit their permissions on your service (weird, but wouldn’t have any effect) - change their email address (bad) This commit adds checks to return a `404` any time you’re looking at a service and trying to do stuff to a user who doesn’t belong to that service. We can’t add this check to the API easily because there are still times that we want to get/modify users outside the context of a service (eg platform admin pages, or users who have no services).
2019-02-25 16:51:37 +00:00
mock_get_users_by_service,
Confirm edit user email changes user email
2019-02-20 11:50:34 +00:00
mock_get_user,
):
new_email = 'new_email@gov.uk'
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
with client_request.session_transaction() as session:
Confirm edit user email changes user email
2019-02-20 11:50:34 +00:00
session['team_member_email_change'] = new_email
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
page = client_request.get(
Confirm edit user email changes user email
2019-02-20 11:50:34 +00:00
'main.confirm_edit_user_email',
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
service_id=SERVICE_ONE_ID,
user_id=active_user_with_permissions.id,
)
Confirm edit user email changes user email
2019-02-20 11:50:34 +00:00
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
assert 'Confirm change of email address' in page.text
Content changes following consultation with content and design pros :)
2019-02-20 17:34:58 +00:00
for text in [
'New email address:',
new_email,
'We will send {} an email to tell them about the change.'.format(active_user_with_permissions.name)
]:
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
assert text in page.text
assert 'Confirm' in page.text
Confirm edit user email changes user email
2019-02-20 11:50:34 +00:00
Redirect from confirmation page if session empty
2019-02-22 16:13:46 +00:00
def test_confirm_edit_user_email_page_redirects_if_session_empty(
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
client_request,
Don’t allow editing of users from other services Currently when you load the ‘edit user’ page (which has a URL like `/service/<service_id>/users/<user_id>`) we check that: - you belong to the service represented by `service_id` - you have permission to edit users on this service We don’t check that: - the user represented by `user_id` belongs to this service This means that if you could somehow determine another user’s `user_id` (which I don’t think is possible if you don’t already have the manage service permission for that service) then you could: - edit their permissions on your service (weird, but wouldn’t have any effect) - change their email address (bad) This commit adds checks to return a `404` any time you’re looking at a service and trying to do stuff to a user who doesn’t belong to that service. We can’t add this check to the API easily because there are still times that we want to get/modify users outside the context of a service (eg platform admin pages, or users who have no services).
2019-02-25 16:51:37 +00:00
mock_get_users_by_service,
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
active_user_with_permissions,
Redirect from confirmation page if session empty
2019-02-22 16:13:46 +00:00
):
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
page = client_request.get(
Redirect from confirmation page if session empty
2019-02-22 16:13:46 +00:00
'main.confirm_edit_user_email',
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
service_id=SERVICE_ONE_ID,
user_id=active_user_with_permissions.id,
_follow_redirects=True,
)
assert 'Confirm change of email address' not in page.text
Redirect from confirmation page if session empty
2019-02-22 16:13:46 +00:00
Don’t allow editing of users from other services Currently when you load the ‘edit user’ page (which has a URL like `/service/<service_id>/users/<user_id>`) we check that: - you belong to the service represented by `service_id` - you have permission to edit users on this service We don’t check that: - the user represented by `user_id` belongs to this service This means that if you could somehow determine another user’s `user_id` (which I don’t think is possible if you don’t already have the manage service permission for that service) then you could: - edit their permissions on your service (weird, but wouldn’t have any effect) - change their email address (bad) This commit adds checks to return a `404` any time you’re looking at a service and trying to do stuff to a user who doesn’t belong to that service. We can’t add this check to the API easily because there are still times that we want to get/modify users outside the context of a service (eg platform admin pages, or users who have no services).
2019-02-25 16:51:37 +00:00
def test_confirm_edit_user_email_page_404s_for_non_team_member(
client_request,
mock_get_users_by_service,
):
client_request.get(
'main.confirm_edit_user_email',
service_id=SERVICE_ONE_ID,
user_id=USER_ONE_ID,
_expected_status=404,
)
Confirm edit user email changes user email
2019-02-20 11:50:34 +00:00
def test_confirm_edit_user_email_changes_user_email(
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
client_request,
Confirm edit user email changes user email
2019-02-20 11:50:34 +00:00
active_user_with_permissions,
Add audit event when a service manager changes someones profile We should audit when a service manager changes a user profile that is not their own. This can be recorded in our events table, which is currently only used to record successful logins. This adds two new types of event, `update_user_email` and `update_user_mobile_number` which store the - browser fingerprint - IP address - user id of the user being updated - user id of the service manager making the change - original email address and new email address (for `update_user_email` events) - original mobile number and new mobile number (for `update_user_mobile_number` events)
2019-04-03 11:47:46 +01:00
api_user_active,
Confirm edit user email changes user email
2019-02-20 11:50:34 +00:00
service_one,
mocker,
Add audit event when a service manager changes someones profile We should audit when a service manager changes a user profile that is not their own. This can be recorded in our events table, which is currently only used to record successful logins. This adds two new types of event, `update_user_email` and `update_user_mobile_number` which store the - browser fingerprint - IP address - user id of the user being updated - user id of the service manager making the change - original email address and new email address (for `update_user_email` events) - original mobile number and new mobile number (for `update_user_mobile_number` events)
2019-04-03 11:47:46 +01:00
mock_update_user_attribute,
Confirm edit user email changes user email
2019-02-20 11:50:34 +00:00
):
Add audit event when a service manager changes someones profile We should audit when a service manager changes a user profile that is not their own. This can be recorded in our events table, which is currently only used to record successful logins. This adds two new types of event, `update_user_email` and `update_user_mobile_number` which store the - browser fingerprint - IP address - user id of the user being updated - user id of the service manager making the change - original email address and new email address (for `update_user_email` events) - original mobile number and new mobile number (for `update_user_mobile_number` events)
2019-04-03 11:47:46 +01:00
# We want active_user_with_permissions (the current user) to update the email address for api_user_active
# By default both users would have the same id, so we change the id of api_user_active
api_user_active.id = str(uuid.uuid4())
mocker.patch('app.user_api_client.get_users_for_service',
return_value=[api_user_active, active_user_with_permissions])
# get_user gets called twice - first to check if current user can see the page, then to see if the team member
# whose email address we're changing belongs to the service
mocker.patch('app.models.service.user_api_client.get_user',
side_effect=[active_user_with_permissions, api_user_active])
mock_event_handler = mocker.patch('app.main.views.manage_users.create_email_change_event')
Confirm edit user email changes user email
2019-02-20 11:50:34 +00:00
new_email = 'new_email@gov.uk'
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
with client_request.session_transaction() as session:
Confirm edit user email changes user email
2019-02-20 11:50:34 +00:00
session['team_member_email_change'] = new_email
Add audit event when a service manager changes someones profile We should audit when a service manager changes a user profile that is not their own. This can be recorded in our events table, which is currently only used to record successful logins. This adds two new types of event, `update_user_email` and `update_user_mobile_number` which store the - browser fingerprint - IP address - user id of the user being updated - user id of the service manager making the change - original email address and new email address (for `update_user_email` events) - original mobile number and new mobile number (for `update_user_mobile_number` events)
2019-04-03 11:47:46 +01:00
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
client_request.post(
'main.confirm_edit_user_email',
service_id=service_one['id'],
Add audit event when a service manager changes someones profile We should audit when a service manager changes a user profile that is not their own. This can be recorded in our events table, which is currently only used to record successful logins. This adds two new types of event, `update_user_email` and `update_user_mobile_number` which store the - browser fingerprint - IP address - user id of the user being updated - user id of the service manager making the change - original email address and new email address (for `update_user_email` events) - original mobile number and new mobile number (for `update_user_mobile_number` events)
2019-04-03 11:47:46 +01:00
user_id=api_user_active.id,
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
_expected_status=302,
_expected_redirect=url_for(
'main.manage_users',
service_id=SERVICE_ONE_ID,
_external=True,
),
)
Add audit event when a service manager changes someones profile We should audit when a service manager changes a user profile that is not their own. This can be recorded in our events table, which is currently only used to record successful logins. This adds two new types of event, `update_user_email` and `update_user_mobile_number` which store the - browser fingerprint - IP address - user id of the user being updated - user id of the service manager making the change - original email address and new email address (for `update_user_email` events) - original mobile number and new mobile number (for `update_user_mobile_number` events)
2019-04-03 11:47:46 +01:00
Tell api when mobile or email updated by colleague So that API can send a confirmation email to the affected user
2019-02-26 16:28:05 +00:00
mock_update_user_attribute.assert_called_once_with(
Add audit event when a service manager changes someones profile We should audit when a service manager changes a user profile that is not their own. This can be recorded in our events table, which is currently only used to record successful logins. This adds two new types of event, `update_user_email` and `update_user_mobile_number` which store the - browser fingerprint - IP address - user id of the user being updated - user id of the service manager making the change - original email address and new email address (for `update_user_email` events) - original mobile number and new mobile number (for `update_user_mobile_number` events)
2019-04-03 11:47:46 +01:00
api_user_active.id,
Tell api when mobile or email updated by colleague So that API can send a confirmation email to the affected user
2019-02-26 16:28:05 +00:00
email_address=new_email,
Add audit event when a service manager changes someones profile We should audit when a service manager changes a user profile that is not their own. This can be recorded in our events table, which is currently only used to record successful logins. This adds two new types of event, `update_user_email` and `update_user_mobile_number` which store the - browser fingerprint - IP address - user id of the user being updated - user id of the service manager making the change - original email address and new email address (for `update_user_email` events) - original mobile number and new mobile number (for `update_user_mobile_number` events)
2019-04-03 11:47:46 +01:00
updated_by=active_user_with_permissions.id
Tell api when mobile or email updated by colleague So that API can send a confirmation email to the affected user
2019-02-26 16:28:05 +00:00
)
Add audit event when a service manager changes someones profile We should audit when a service manager changes a user profile that is not their own. This can be recorded in our events table, which is currently only used to record successful logins. This adds two new types of event, `update_user_email` and `update_user_mobile_number` which store the - browser fingerprint - IP address - user id of the user being updated - user id of the service manager making the change - original email address and new email address (for `update_user_email` events) - original mobile number and new mobile number (for `update_user_mobile_number` events)
2019-04-03 11:47:46 +01:00
mock_event_handler.assert_called_once_with(
api_user_active.id,
active_user_with_permissions.id,
api_user_active.email_address,
new_email)
Don’t allow editing of users from other services Currently when you load the ‘edit user’ page (which has a URL like `/service/<service_id>/users/<user_id>`) we check that: - you belong to the service represented by `service_id` - you have permission to edit users on this service We don’t check that: - the user represented by `user_id` belongs to this service This means that if you could somehow determine another user’s `user_id` (which I don’t think is possible if you don’t already have the manage service permission for that service) then you could: - edit their permissions on your service (weird, but wouldn’t have any effect) - change their email address (bad) This commit adds checks to return a `404` any time you’re looking at a service and trying to do stuff to a user who doesn’t belong to that service. We can’t add this check to the API easily because there are still times that we want to get/modify users outside the context of a service (eg platform admin pages, or users who have no services).
2019-02-25 16:51:37 +00:00
def test_confirm_edit_user_email_doesnt_change_user_email_for_non_team_member(
client_request,
mock_get_users_by_service,
):
with client_request.session_transaction() as session:
session['team_member_email_change'] = 'new_email@gov.uk'
client_request.post(
'main.confirm_edit_user_email',
service_id=SERVICE_ONE_ID,
user_id=USER_ONE_ID,
_expected_status=404,
)
Edit user permissions page shows redacted phone number and Change link Also make plan for story development
2019-02-21 13:03:06 +00:00
Edit mobile number page
2019-02-22 11:31:35 +00:00
def test_edit_user_permissions_page_displays_redacted_mobile_number_and_change_link(
Edit user permissions page shows redacted phone number and Change link Also make plan for story development
2019-02-21 13:03:06 +00:00
client_request,
active_user_with_permissions,
Edit mobile page allows users to proceed without changing mobile number Also: - change formatting of mobile number redact characters - redirect if session empty - update a test with new mock after rebase
2019-02-25 14:27:37 +00:00
mock_get_users_by_service,
Send updated user folder permissions to the API Integrates the folder permissions form with the updated API endpoint to store changes in the user folders. Since user folder permissions are returned in the full list of template folders for the service we need to invalidate the cache key for it each time we update user permissions.
2019-02-27 15:45:18 +00:00
mock_get_template_folders,
Edit user permissions page shows redacted phone number and Change link Also make plan for story development
2019-02-21 13:03:06 +00:00
service_one,
mocker
):
user = active_user_with_permissions
mocker.patch('app.user_api_client.get_user', return_value=user)
page = client_request.get(
'main.edit_user_permissions',
service_id=service_one['id'],
user_id=user.id
)
assert user.name in page.find('h1').text
Edit mobile number page
2019-02-22 11:31:35 +00:00
mobile_number_paragraph = page.select('p[id=user_mobile_number]')[0]
Edit mobile page allows users to proceed without changing mobile number Also: - change formatting of mobile number redact characters - redirect if session empty - update a test with new mock after rebase
2019-02-25 14:27:37 +00:00
assert '0770 • • • • 762' in mobile_number_paragraph.text
Edit mobile number page
2019-02-22 11:31:35 +00:00
change_link = mobile_number_paragraph.findChild()
assert change_link.attrs['href'] == '/services/{}/users/{}/edit-mobile-number'.format(
Edit user permissions page shows redacted phone number and Change link Also make plan for story development
2019-02-21 13:03:06 +00:00
service_one['id'], user.id
)
remove the remove-user get method remove `confirm` from `confirm_remove_user_from_service` as there's only one action now that the initial confirmation prompt takes place on the edit permissions page
2019-03-26 15:51:44 +00:00
def test_edit_user_permissions_with_delete_query_shows_banner(
client_request,
active_user_with_permissions,
mock_get_users_by_service,
mock_get_template_folders,
service_one
):
page = client_request.get(
'main.edit_user_permissions',
service_id=service_one['id'],
user_id=active_user_with_permissions.id,
delete=1
)
banner = page.find('div', class_='banner-dangerous')
assert banner.contents[0].strip() == "Are you sure you want to remove Test User?"
assert banner.form.attrs['action'] == url_for(
'main.remove_user_from_service',
service_id=service_one['id'],
user_id=active_user_with_permissions.id
)
Edit mobile number page
2019-02-22 11:31:35 +00:00
def test_edit_user_mobile_number_page(
client_request,
active_user_with_permissions,
Ensure that mobile of user not belonging to service cannot be edited
2019-02-26 11:47:15 +00:00
mock_get_users_by_service,
Edit mobile number page
2019-02-22 11:31:35 +00:00
service_one,
mocker
):
user = active_user_with_permissions
mocker.patch('app.user_api_client.get_user', return_value=user)
page = client_request.get(
'main.edit_user_mobile_number',
service_id=service_one['id'],
user_id=user.id
)
assert page.find('h1').text == "Change team member’s mobile number"
assert page.select('p[id=user_name]')[0].text == "This will change the mobile number for {}.".format(user.name)
Edit mobile page allows users to proceed without changing mobile number Also: - change formatting of mobile number redact characters - redirect if session empty - update a test with new mock after rebase
2019-02-25 14:27:37 +00:00
assert page.select('input[name=mobile_number]')[0].attrs["value"] == "0770••••762"
Edit mobile number page
2019-02-22 11:31:35 +00:00
assert page.select('button[type=submit]')[0].text == "Save"
Edit user permissions page shows redacted phone number and Change link Also make plan for story development
2019-02-21 13:03:06 +00:00
Redirect to mobile number confirmation page
2019-02-22 12:28:18 +00:00
def test_edit_user_mobile_number_redirects_to_confirmation(
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
client_request,
Redirect to mobile number confirmation page
2019-02-22 12:28:18 +00:00
active_user_with_permissions,
Ensure that mobile of user not belonging to service cannot be edited
2019-02-26 11:47:15 +00:00
mock_get_users_by_service,
Redirect to mobile number confirmation page
2019-02-22 12:28:18 +00:00
):
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
client_request.post(
'main.edit_user_mobile_number',
service_id=SERVICE_ONE_ID,
Redirect to mobile number confirmation page
2019-02-22 12:28:18 +00:00
user_id=active_user_with_permissions.id,
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
_data={'mobile_number': '07554080636'},
_expected_status=302,
_expected_redirect=url_for(
'main.confirm_edit_user_mobile_number',
service_id=SERVICE_ONE_ID,
user_id=active_user_with_permissions.id,
_external=True,
),
Redirect to mobile number confirmation page
2019-02-22 12:28:18 +00:00
)
Edit user permissions page shows redacted phone number and Change link Also make plan for story development
2019-02-21 13:03:06 +00:00
Edit mobile page allows users to proceed without changing mobile number Also: - change formatting of mobile number redact characters - redirect if session empty - update a test with new mock after rebase
2019-02-25 14:27:37 +00:00
def test_edit_user_mobile_number_redirects_to_manage_users_if_number_not_changed(
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
client_request,
Edit mobile page allows users to proceed without changing mobile number Also: - change formatting of mobile number redact characters - redirect if session empty - update a test with new mock after rebase
2019-02-25 14:27:37 +00:00
active_user_with_permissions,
Ensure that mobile of user not belonging to service cannot be edited
2019-02-26 11:47:15 +00:00
mock_get_users_by_service,
Edit mobile page allows users to proceed without changing mobile number Also: - change formatting of mobile number redact characters - redirect if session empty - update a test with new mock after rebase
2019-02-25 14:27:37 +00:00
service_one,
mocker,
mock_get_user,
):
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
client_request.post(
'main.edit_user_mobile_number',
service_id=SERVICE_ONE_ID,
user_id=active_user_with_permissions.id,
_data={'mobile_number': '0770••••762'},
_expected_status=302,
_expected_redirect=url_for(
'main.manage_users',
service_id=SERVICE_ONE_ID,
_external=True,
),
)
Edit mobile page allows users to proceed without changing mobile number Also: - change formatting of mobile number redact characters - redirect if session empty - update a test with new mock after rebase
2019-02-25 14:27:37 +00:00
Confirm change of team member's phone number
2019-02-22 15:06:42 +00:00
def test_confirm_edit_user_mobile_number_page(
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
client_request,
Confirm change of team member's phone number
2019-02-22 15:06:42 +00:00
active_user_with_permissions,
Ensure that mobile of user not belonging to service cannot be edited
2019-02-26 11:47:15 +00:00
mock_get_users_by_service,
Confirm change of team member's phone number
2019-02-22 15:06:42 +00:00
service_one,
mocker,
mock_get_user,
):
new_number = '07554080636'
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
with client_request.session_transaction() as session:
Confirm change of team member's phone number
2019-02-22 15:06:42 +00:00
session['team_member_mobile_change'] = new_number
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
page = client_request.get(
Confirm change of team member's phone number
2019-02-22 15:06:42 +00:00
'main.confirm_edit_user_mobile_number',
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
service_id=SERVICE_ONE_ID,
user_id=active_user_with_permissions.id,
)
Confirm change of team member's phone number
2019-02-22 15:06:42 +00:00
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
assert 'Confirm change of mobile number' in page.text
Confirm change of team member's phone number
2019-02-22 15:06:42 +00:00
for text in [
'New mobile number:',
new_number,
Update the message when changing a users mobile phone number.
2019-03-04 11:14:35 +00:00
'We will send {} a text message to tell them about the change.'.format(active_user_with_permissions.name)
Confirm change of team member's phone number
2019-02-22 15:06:42 +00:00
]:
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
assert text in page.text
assert 'Confirm' in page.text
Edit user permissions page shows redacted phone number and Change link Also make plan for story development
2019-02-21 13:03:06 +00:00
Edit mobile page allows users to proceed without changing mobile number Also: - change formatting of mobile number redact characters - redirect if session empty - update a test with new mock after rebase
2019-02-25 14:27:37 +00:00
def test_confirm_edit_user_mobile_number_page_redirects_if_session_empty(
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
client_request,
Edit mobile page allows users to proceed without changing mobile number Also: - change formatting of mobile number redact characters - redirect if session empty - update a test with new mock after rebase
2019-02-25 14:27:37 +00:00
active_user_with_permissions,
Ensure that mobile of user not belonging to service cannot be edited
2019-02-26 11:47:15 +00:00
mock_get_users_by_service,
Edit mobile page allows users to proceed without changing mobile number Also: - change formatting of mobile number redact characters - redirect if session empty - update a test with new mock after rebase
2019-02-25 14:27:37 +00:00
service_one,
mocker,
mock_get_user,
):
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
page = client_request.get(
Edit mobile page allows users to proceed without changing mobile number Also: - change formatting of mobile number redact characters - redirect if session empty - update a test with new mock after rebase
2019-02-25 14:27:37 +00:00
'main.confirm_edit_user_mobile_number',
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
service_id=SERVICE_ONE_ID,
user_id=active_user_with_permissions.id,
_expected_status=302,
)
assert 'Confirm change of mobile number' not in page.text
Edit mobile page allows users to proceed without changing mobile number Also: - change formatting of mobile number redact characters - redirect if session empty - update a test with new mock after rebase
2019-02-25 14:27:37 +00:00
Change team member's mobile number upon confirmation Also add edit user mobile number urls to navigation exceptions
2019-02-22 16:01:04 +00:00
def test_confirm_edit_user_mobile_number_changes_user_mobile_number(
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
client_request,
Change team member's mobile number upon confirmation Also add edit user mobile number urls to navigation exceptions
2019-02-22 16:01:04 +00:00
active_user_with_permissions,
Add audit event when a service manager changes someones profile We should audit when a service manager changes a user profile that is not their own. This can be recorded in our events table, which is currently only used to record successful logins. This adds two new types of event, `update_user_email` and `update_user_mobile_number` which store the - browser fingerprint - IP address - user id of the user being updated - user id of the service manager making the change - original email address and new email address (for `update_user_email` events) - original mobile number and new mobile number (for `update_user_mobile_number` events)
2019-04-03 11:47:46 +01:00
api_user_active,
Change team member's mobile number upon confirmation Also add edit user mobile number urls to navigation exceptions
2019-02-22 16:01:04 +00:00
service_one,
mocker,
mock_update_user_attribute
):
Add audit event when a service manager changes someones profile We should audit when a service manager changes a user profile that is not their own. This can be recorded in our events table, which is currently only used to record successful logins. This adds two new types of event, `update_user_email` and `update_user_mobile_number` which store the - browser fingerprint - IP address - user id of the user being updated - user id of the service manager making the change - original email address and new email address (for `update_user_email` events) - original mobile number and new mobile number (for `update_user_mobile_number` events)
2019-04-03 11:47:46 +01:00
# We want active_user_with_permissions (the current user) to update the mobile number for api_user_active
# By default both users would have the same id, so we change the id of api_user_active
api_user_active.id = str(uuid.uuid4())
mocker.patch('app.user_api_client.get_users_for_service',
return_value=[api_user_active, active_user_with_permissions])
# get_user gets called twice - first to check if current user can see the page, then to see if the team member
# whose mobile number we're changing belongs to the service
mocker.patch('app.models.service.user_api_client.get_user',
side_effect=[active_user_with_permissions, api_user_active])
mock_event_handler = mocker.patch('app.main.views.manage_users.create_mobile_number_change_event')
Change team member's mobile number upon confirmation Also add edit user mobile number urls to navigation exceptions
2019-02-22 16:01:04 +00:00
new_number = '07554080636'
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
with client_request.session_transaction() as session:
Change team member's mobile number upon confirmation Also add edit user mobile number urls to navigation exceptions
2019-02-22 16:01:04 +00:00
session['team_member_mobile_change'] = new_number
Add audit event when a service manager changes someones profile We should audit when a service manager changes a user profile that is not their own. This can be recorded in our events table, which is currently only used to record successful logins. This adds two new types of event, `update_user_email` and `update_user_mobile_number` which store the - browser fingerprint - IP address - user id of the user being updated - user id of the service manager making the change - original email address and new email address (for `update_user_email` events) - original mobile number and new mobile number (for `update_user_mobile_number` events)
2019-04-03 11:47:46 +01:00
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
client_request.post(
'main.confirm_edit_user_mobile_number',
service_id=SERVICE_ONE_ID,
Add audit event when a service manager changes someones profile We should audit when a service manager changes a user profile that is not their own. This can be recorded in our events table, which is currently only used to record successful logins. This adds two new types of event, `update_user_email` and `update_user_mobile_number` which store the - browser fingerprint - IP address - user id of the user being updated - user id of the service manager making the change - original email address and new email address (for `update_user_email` events) - original mobile number and new mobile number (for `update_user_mobile_number` events)
2019-04-03 11:47:46 +01:00
user_id=api_user_active.id,
Use `client_request` fixture where possible It: - saves repetetive boilerplate code - does some extra checks (eg checking for a `200` response) - makes the codebase less confusing to consistently do the same thing in the same way
2019-03-26 12:35:32 +00:00
_expected_status=302,
_expected_redirect=url_for(
'main.manage_users',
service_id=SERVICE_ONE_ID,
_external=True,
),
)
Tell api when mobile or email updated by colleague So that API can send a confirmation email to the affected user
2019-02-26 16:28:05 +00:00
mock_update_user_attribute.assert_called_once_with(
Add audit event when a service manager changes someones profile We should audit when a service manager changes a user profile that is not their own. This can be recorded in our events table, which is currently only used to record successful logins. This adds two new types of event, `update_user_email` and `update_user_mobile_number` which store the - browser fingerprint - IP address - user id of the user being updated - user id of the service manager making the change - original email address and new email address (for `update_user_email` events) - original mobile number and new mobile number (for `update_user_mobile_number` events)
2019-04-03 11:47:46 +01:00
api_user_active.id,
Tell api when mobile or email updated by colleague So that API can send a confirmation email to the affected user
2019-02-26 16:28:05 +00:00
mobile_number=new_number,
Add audit event when a service manager changes someones profile We should audit when a service manager changes a user profile that is not their own. This can be recorded in our events table, which is currently only used to record successful logins. This adds two new types of event, `update_user_email` and `update_user_mobile_number` which store the - browser fingerprint - IP address - user id of the user being updated - user id of the service manager making the change - original email address and new email address (for `update_user_email` events) - original mobile number and new mobile number (for `update_user_mobile_number` events)
2019-04-03 11:47:46 +01:00
updated_by=active_user_with_permissions.id
Tell api when mobile or email updated by colleague So that API can send a confirmation email to the affected user
2019-02-26 16:28:05 +00:00
)
Add audit event when a service manager changes someones profile We should audit when a service manager changes a user profile that is not their own. This can be recorded in our events table, which is currently only used to record successful logins. This adds two new types of event, `update_user_email` and `update_user_mobile_number` which store the - browser fingerprint - IP address - user id of the user being updated - user id of the service manager making the change - original email address and new email address (for `update_user_email` events) - original mobile number and new mobile number (for `update_user_mobile_number` events)
2019-04-03 11:47:46 +01:00
mock_event_handler.assert_called_once_with(
api_user_active.id,
active_user_with_permissions.id,
api_user_active.mobile_number,
new_number)
Ensure that mobile of user not belonging to service cannot be edited
2019-02-26 11:47:15 +00:00
def test_confirm_edit_user_mobile_number_doesnt_change_user_mobile_for_non_team_member(
client_request,
mock_get_users_by_service,
):
with client_request.session_transaction() as session:
session['team_member_mobile_change'] = '07554080636'
client_request.post(
'main.confirm_edit_user_mobile_number',
service_id=SERVICE_ONE_ID,
user_id=USER_ONE_ID,
_expected_status=404,
)
Reference in New Issue Copy Permalink