Trello: https://trello.com/c/caETStGa This is a step on the journey to move the edit user page to use the GOV.UK Design System. The new design calls for separate pages for different operations on a user and this is the next one. The `require_2sv` checkbox was previously only displayed on the "edit user" page (from `app/views/users/_form_fields.html.erb`) when `User#require_2sv?` was `false` and thus it was only ever possible to use this checkbox to *mandate* 2SV; it was not possible to use it to *unset* the `User#require_2sv` attribute. Unlike in commits like this one [1] where we first split out the new page and then separatly moved the new page to use the Design System, here I've created the new page using the Design System in one go, because there wasn't much markup in the "edit user" page to extract. The new `app/views/users/two_step_verification_mandations/edit.html.erb` template is based on other similar pages which we've recently extracted, e.g. `app/views/users/two_step_verification_resets/edit.html.erb`. I decided not to include a checkbox on the new page, because as mentioned above, this page should only ever be needed to *mandate* 2SV. The new `Users::TwoStepVerificationMandationsController` is closely based on the relevant bits of code from `UsersController`, even though some of it is probably overkill in the new context, i.e. the use of `UserUpdate`. However, I thought it was worth keeping this step as small as possible. I've hard-coded `user_params` to `{ require_2sv: true }`, since there's no checkbox in the form. I'm using `UserPolicy#mandate_2sv?` for the authorization in the new `Users::TwoStepVerificationRequirementsController`, because that seems to make the most sense. You could argue that I should also check `UserPolicy#edit?` & `UserPolicy#update?` like I did in `Users::OrganisationsController` in this commit [2], but I've gone off that idea (sorry, @chrisroos!). Instead I've matched what I did in `Users::TwoStepVerificationResetsController` in this commit [3]. There weren't *any* tests in `UsersControllerTest` for this behaviour, so I had to write `Users::TwoStepVerificationMandationsControllerTest` from scratch, but I based it closely on other similar controller tests. Luckily there was an integration test (`test/integration/managing_two_step_verification_test.rb`) covering the behaviour which I've been able to fixup to work with the new UI. Rather than creating a combinatorial explosion of tests in `Users::TwoStepVerificationRequirementsControllerTest` relating to whether a user with all the different roles can edit another user with all the different roles, I've resorted to stubbing `UserPolicy.new` and `UserPolicy#mandate_2sv?`. Although this is a bit ugly, since `UserPolicy` is thoroughly tested in `UserPolicyTest`, it seems like a pragmatic option. I'm using the `error_summary` component in combination with identical code we've used elsewhere on the off-chance there's a validation error, because the controller action does re-render the form in that case. Note that in normal operation there are unlikely to be validation errors on this page, but since it's theoretically possible I thought it was worth adding the error summary. We might want to consider changing this to work more like `Users::TwoStepVerificationResetsController` which raises an exception if there is a validation error. This makes sense particularly given that the only validation on `User#require_2sv` only applied on record creation. [1]: 899a8a1 [2]: bf207ef [3]: 1baf685

Commits

Commits on Nov 28, 2023

Reset 2SV in integration tests without JS

Remove pointless tests for unsetting User#require_2sv

Extract page for mandating 2SV for a user

Copy changes for the mandate 2SV page