diff --git a/app/assets/javascripts/modules/accessible-autocomplete.js b/app/assets/javascripts/modules/accessible-autocomplete.js index f8a7edec8..3a2234ce1 100644 --- a/app/assets/javascripts/modules/accessible-autocomplete.js +++ b/app/assets/javascripts/modules/accessible-autocomplete.js @@ -3,7 +3,7 @@ window.GOVUK = window.GOVUK || {} window.GOVUK.Modules = window.GOVUK.Modules || {} -;(function (Modules) { +; (function (Modules) { 'use strict' function AccessibleAutocomplete ($module) { @@ -23,17 +23,17 @@ window.GOVUK.Modules = window.GOVUK.Modules || {} new window.accessibleAutocomplete.enhanceSelectElement(configOptions) // eslint-disable-line no-new, new-cap const autocompleteElement = selectElement.parentNode.querySelector('.autocomplete__input') - enableArrow(autocompleteElement) - enableAddButton() + enableArrow(this.$module, autocompleteElement) + enableAddButton(this.$module) resetSelectWhenDesynced(selectElement, autocompleteElement) - enableClearButton(selectElement, autocompleteElement) + enableClearButton(this.$module, selectElement, autocompleteElement) } Modules.AccessibleAutocomplete = AccessibleAutocomplete })(window.GOVUK.Modules) -function enableArrow (autocompleteElement) { - const arrowElement = autocompleteElement.parentNode.querySelector('.autocomplete__dropdown-arrow-down') +function enableArrow (module, autocompleteElement) { + const arrowElement = module.querySelector('.autocomplete__dropdown-arrow-down') arrowElement.addEventListener('click', function () { autocompleteElement.click() @@ -41,10 +41,10 @@ function enableArrow (autocompleteElement) { }) } -function enableAddButton () { - const addButton = document.querySelector('.js-autocomplete__add-button') - const addAndFinishButton = document.querySelector('.js-autocomplete__add-and-finish-button') - const addMoreInput = document.querySelector('input[name="application[add_more]"]') +function enableAddButton (module) { + const addButton = module.querySelector('.js-autocomplete__add-button') + const addAndFinishButton = module.querySelector('.js-autocomplete__add-and-finish-button') + const addMoreInput = module.querySelector('input[name="application[add_more]"]') if (addButton) { addAndFinishButton.type = 'button' @@ -84,8 +84,8 @@ function resetSelectWhenDesynced (selectElement, autocompleteElement) { }) } -function enableClearButton (selectElement, autocompleteElement) { - const clearButton = document.querySelector('.js-autocomplete__clear-button') +function enableClearButton (module, selectElement, autocompleteElement) { + const clearButton = module.querySelector('.js-autocomplete__clear-button') if (clearButton) { clearButton.addEventListener('click', function () { diff --git a/app/views/devise/invitations/new.html.erb b/app/views/devise/invitations/new.html.erb index 56ea38eec..715fb8000 100644 --- a/app/views/devise/invitations/new.html.erb +++ b/app/views/devise/invitations/new.html.erb @@ -49,12 +49,23 @@ autocomplete: "off", } %> - <%= render "govuk_publishing_components/components/select", { - id: "user_organisation_id", - name: "user[organisation_id]", - label: "Organisation", - options: options_for_organisation_select(selected: f.object.organisation_id) - } %> +
+ <%= render "govuk_publishing_components/components/select", { + id: "user_organisation_id", + name: "user[organisation_id]", + label: "Organisation", + options: options_for_organisation_select(selected: f.object.organisation_id) + } %> + +
+ <%= render "govuk_publishing_components/components/button", { + text: "Clear selection", + type: "button", + classes: "js-autocomplete__clear-button", + secondary_solid: true + } %> +
+
<% if policy(User).assign_role? %> <%= render "govuk_publishing_components/components/select", { diff --git a/app/views/shared/_add_permissions_with_autocomplete_form.html.erb b/app/views/shared/_add_permissions_with_autocomplete_form.html.erb index 0a3d3bce4..84c718ef3 100644 --- a/app/views/shared/_add_permissions_with_autocomplete_form.html.erb +++ b/app/views/shared/_add_permissions_with_autocomplete_form.html.erb @@ -1,5 +1,5 @@ -<%= form_tag action, method: :patch do |f| %> -
+
+ <%= form_tag action, method: :patch do |f| %> <%= render "govuk_publishing_components/components/select", { id: "new_permission_id", heading_size: "m", @@ -8,32 +8,33 @@ name: "application[new_permission_id]", options: unassigned_permission_options.unshift({ text: '', value: nil }) } %> -
- <% assigned_permissions.map(&:id).each do |id| %> - <%= hidden_field_tag "application[current_permission_ids][]", id %> - <% end %> +
+ <%= render "govuk_publishing_components/components/button", { + text: "Add", + aria_label: "Add permission", + classes: "js-autocomplete__add-button" + } %> - <%= hidden_field_tag "application[add_more]", "false" %> + <%= render "govuk_publishing_components/components/button", { + text: "Add and finish", + aria_label: "Add permission and finish", + classes: "js-autocomplete__add-and-finish-button" + } %> -
- <%= render "govuk_publishing_components/components/button", { - text: "Add", - aria_label: "Add permission", - classes: "js-autocomplete__add-button" - } %> + <%= render "govuk_publishing_components/components/button", { + text: "Clear selection", + type: "button", + classes: "js-autocomplete__clear-button", + secondary_solid: true + } %> +
- <%= render "govuk_publishing_components/components/button", { - text: "Add and finish", - aria_label: "Add permission and finish", - classes: "js-autocomplete__add-and-finish-button" - } %> - <%= render "govuk_publishing_components/components/button", { - text: "Clear selection", - type: "button", - classes: "js-autocomplete__clear-button", - secondary_solid: true - } %> -
-<% end %> + <% assigned_permissions.map(&:id).each do |id| %> + <%= hidden_field_tag "application[current_permission_ids][]", id %> + <% end %> + + <%= hidden_field_tag "application[add_more]", "false" %> + <% end %> +
diff --git a/spec/javascripts/modules/accessible-autocomplete-dropdown-spec.js b/spec/javascripts/modules/accessible-autocomplete-dropdown-spec.js deleted file mode 100644 index 18418f37c..000000000 --- a/spec/javascripts/modules/accessible-autocomplete-dropdown-spec.js +++ /dev/null @@ -1,46 +0,0 @@ -describe('GOVUK.Modules.AccessibleAutocomplete', function () { - let component, module - - beforeEach(async function () { - component = document.createElement('div') - component.setAttribute('data-module', 'accessible-autocomplete') - component.innerHTML = ` - -
- Search for the permission you want to add. - -
- ` - module = new GOVUK.Modules.AccessibleAutocomplete(component) - module.init() - }) - - it('opens the menu when clicking the arrow', async function () { - const menuElement = component.querySelector('.autocomplete__menu') - const menuElementClassesBefore = Array.from(menuElement.classList) - expect(menuElementClassesBefore.includes('autocomplete__menu--visible')).toBe(false) - expect(menuElementClassesBefore.includes('autocomplete__menu--hidden')).toBe(true) - - const arrowElement = component.querySelector('.autocomplete__dropdown-arrow-down') - arrowElement.dispatchEvent(new Event('click')) - - await wait() - - const menuElementClassesAfter = Array.from(menuElement.classList) - expect(menuElementClassesAfter.includes('autocomplete__menu--visible')).toBe(true) - expect(menuElementClassesAfter.includes('autocomplete__menu--hidden')).toBe(false) - }) -}) - -const wait = async () => await new Promise(resolve => setTimeout(resolve, 100)) diff --git a/spec/javascripts/modules/accessible-autocomplete-spec.js b/spec/javascripts/modules/accessible-autocomplete-spec.js new file mode 100644 index 000000000..8d32cb0f1 --- /dev/null +++ b/spec/javascripts/modules/accessible-autocomplete-spec.js @@ -0,0 +1,93 @@ +describe('GOVUK.Modules.AccessibleAutocomplete', function () { + let component, module, autocompleteInput, selectInput + + beforeEach(async function () { + component = document.createElement('div') + component.setAttribute('data-module', 'accessible-autocomplete') + component.innerHTML = ` +
+ +
+ Search for the permission you want to add. +
+ +
+
+ + + +
+ + ` + + module = new GOVUK.Modules.AccessibleAutocomplete(component) + module.init() + + autocompleteInput = component.querySelector('.autocomplete__input') + selectInput = component.querySelector('select') + + autocompleteInput.value = 'per' + await wait() + + expect(selectInput.value).toBe('') + const firstAutocompleteListItem = component.querySelector('#new_permission_id__option--0') + firstAutocompleteListItem.click() + expect(autocompleteInput.value).toBe('permission-1') + expect(selectInput.value).toBe('1') + }) + + it('opens the menu when clicking the arrow', async function () { + const menuElement = component.querySelector('.autocomplete__menu') + const menuElementClassesBefore = Array.from(menuElement.classList) + expect(menuElementClassesBefore.includes('autocomplete__menu--visible')).toBe(false) + expect(menuElementClassesBefore.includes('autocomplete__menu--hidden')).toBe(true) + + const arrowElement = component.querySelector('.autocomplete__dropdown-arrow-down') + arrowElement.dispatchEvent(new Event('click')) + + await wait() + + const menuElementClassesAfter = Array.from(menuElement.classList) + expect(menuElementClassesAfter.includes('autocomplete__menu--visible')).toBe(true) + expect(menuElementClassesAfter.includes('autocomplete__menu--hidden')).toBe(false) + }) + + it("resets the value of the select element when it no longer matches what's shown in the autocomplete input", async function () { + autocompleteInput.value = 'permission-' + autocompleteInput.dispatchEvent(new KeyboardEvent('keyup')) + await wait() + + expect(selectInput.value).toBe('') + }) + + it('clears the value of the select and autocomplete elements when clicking the clear button', async function () { + const clearButton = component.querySelector('.js-autocomplete__clear-button') + clearButton.click() + await wait() + + expect(component.querySelector('select').value).toBe('') + }) + + it('clears the value of the select and autocomplete elements when hitting space on the clear button', async function () { + const clearButton = component.querySelector('.js-autocomplete__clear-button') + clearButton.dispatchEvent(new KeyboardEvent('keydown', { key: ' ' })) + await wait() + + expect(selectInput.value).toBe('') + }) + + it('clears the value of the select and autocomplete elements when hitting enter on the clear button', async function () { + const clearButton = component.querySelector('.js-autocomplete__clear-button') + clearButton.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter' })) + await wait() + + expect(selectInput.value).toBe('') + }) +}) + +const wait = async () => await new Promise(resolve => setTimeout(resolve, 1000)) diff --git a/test/integration/updating_permissions_for_apps_with_many_permissions_test.rb b/test/integration/updating_permissions_for_apps_with_many_permissions_test.rb index d6684e970..93c8bdfe7 100644 --- a/test/integration/updating_permissions_for_apps_with_many_permissions_test.rb +++ b/test/integration/updating_permissions_for_apps_with_many_permissions_test.rb @@ -3,221 +3,84 @@ class UpdatingPermissionsForAppsWithManyPermissionsTest < ActionDispatch::IntegrationTest # Also see: Account::UpdatingPermissionsTest, Users::UpdatingPermissionsTest - def shared_setup - @grantee_is_self = rand(2).zero? - - @application = create(:application) - @old_permissions_to_keep = create_list(:supported_permission, 3, application: @application) - @old_permission_to_remove_without_javascript = create(:supported_permission, application: @application) - @new_permissions_to_leave = create_list(:supported_permission, 4, application: @application) - @new_permission_to_grant = create(:supported_permission, application: @application, name: "adding") - - @current_user = create(:superadmin_user) - @grantee = @grantee_is_self ? @current_user : create(:user) - @grantee.grant_application_signin_permission(@application) - @grantee.grant_application_permissions(@application, [*@old_permissions_to_keep, @old_permission_to_remove_without_javascript].map(&:name)) - - visit new_user_session_path - signin_with @current_user - end - - def assert_select_permission_to_grant_with_javascript - @grantee_is_self ? assert_edit_self : assert_edit_other_user(@grantee) - - click_link "Update permissions for #{@application.name}" - - @autocomplete_input = find(".autocomplete__input") - @select_element = find("#new_permission_id-select", visible: false) - assert_equal "", @autocomplete_input.value - assert_equal "", @select_element.value - - # when I type a few characters from a permission called "adding" - @autocomplete_input.fill_in with: "add" - autocomplete_option = find(".autocomplete__option") + context "with apps that have more than eight permissions" do + setup do + @grantee_is_self = rand(2).zero? + + @application = create(:application) + @old_permissions_to_keep = create_list(:supported_permission, 3, application: @application) + @old_permission_to_remove_without_javascript = create(:supported_permission, application: @application) + @new_permissions_to_leave = create_list(:supported_permission, 4, application: @application) + @new_permission_to_grant = create(:supported_permission, application: @application, name: "adding") + + @current_user = create(:superadmin_user) + @grantee = @grantee_is_self ? @current_user : create(:user) + @grantee.grant_application_signin_permission(@application) + @grantee.grant_application_permissions(@application, [*@old_permissions_to_keep, @old_permission_to_remove_without_javascript].map(&:name)) + + visit new_user_session_path + signin_with @current_user + end - # the autcomplete value reflects what I typed, a matching option appears, but the select element remains empty - assert_equal "add", @autocomplete_input.value - assert_equal @new_permission_to_grant.name, autocomplete_option.text - assert_equal "", @select_element.value + should "be able to grant permissions" do + @grantee_is_self ? assert_edit_self : assert_edit_other_user(@grantee) - # when I click on the matching option - autocomplete_option.click + click_link "Update permissions for #{@application.name}" + select @new_permission_to_grant.name + click_button "Add and finish" - # the autocomplete and select elements reflect my selection - assert_equal @new_permission_to_grant.name, @autocomplete_input.value - assert_equal @new_permission_to_grant.id.to_s, @select_element.value - end + click_link "Update permissions for #{@application.name}" + uncheck @old_permission_to_remove_without_javascript.name + click_button "Update permissions" - def assert_permissions_unchanged - expected_permissions = [*@old_permissions_to_keep, @old_permission_to_remove_without_javascript] - expected_permissions.each { |expected_permission| assert @grantee.has_permission?(expected_permission) } + expected_permissions = [*@old_permissions_to_keep, @new_permission_to_grant] + assert_flash_content(expected_permissions.map(&:name)) + expected_permissions.each { |expected_permission| assert @grantee.has_permission?(expected_permission) } - unexpected_permissions = [*@new_permissions_to_leave, @new_permission_to_grant] - unexpected_permissions.each { |unexpected_permission| assert_not @grantee.has_permission?(unexpected_permission) } - end - - context "with apps that have more than eight permissions" do - context "with JavaScript disabled" do - setup { shared_setup } + unexpected_permissions = [@old_permission_to_remove_without_javascript, *@new_permissions_to_leave] + refute_flash_content(unexpected_permissions.map(&:name)) + unexpected_permissions.each { |unexpected_permission| assert_not @grantee.has_permission?(unexpected_permission) } + end - should "be able to grant permissions" do + context "when the grantee already has some but not all permissions" do + should "display the new and current permissions forms" do @grantee_is_self ? assert_edit_self : assert_edit_other_user(@grantee) click_link "Update permissions for #{@application.name}" - select @new_permission_to_grant.name - click_button "Add and finish" - - click_link "Update permissions for #{@application.name}" - uncheck @old_permission_to_remove_without_javascript.name - click_button "Update permissions" - - expected_permissions = [*@old_permissions_to_keep, @new_permission_to_grant] - assert_flash_content(expected_permissions.map(&:name)) - expected_permissions.each { |expected_permission| assert @grantee.has_permission?(expected_permission) } - unexpected_permissions = [@old_permission_to_remove_without_javascript, *@new_permissions_to_leave] - refute_flash_content(unexpected_permissions.map(&:name)) - unexpected_permissions.each { |unexpected_permission| assert_not @grantee.has_permission?(unexpected_permission) } - end - - context "when the grantee already has some but not all permissions" do - should "display the new and current permissions forms" do - @grantee_is_self ? assert_edit_self : assert_edit_other_user(@grantee) - - click_link "Update permissions for #{@application.name}" - - assert_selector ".govuk-label", text: "Add a permission" - assert_selector "legend", text: "Current permissions" - end - end - - context "when the grantee has all permissions" do - setup do - @grantee.grant_application_permissions(@application, [*@new_permissions_to_leave, @new_permission_to_grant].map(&:name)) - end - - should "only display the current permissions form" do - @grantee_is_self ? assert_edit_self : assert_edit_other_user(@grantee) - - click_link "Update permissions for #{@application.name}" - - assert_no_selector ".govuk-label", text: "Add a permission" - assert_selector "legend", text: "Current permissions" - end - end - - context "when the grantee has no permissions" do - setup do - old_permission_ids = [*@old_permissions_to_keep, @old_permission_to_remove_without_javascript].pluck(:id) - UserApplicationPermission.where(user: @grantee, supported_permission_id: old_permission_ids).destroy_all - end - - should "only display the new permissions form" do - @grantee_is_self ? assert_edit_self : assert_edit_other_user(@grantee) - - click_link "Update permissions for #{@application.name}" - - assert_selector ".govuk-label", text: "Add a permission" - assert_no_selector "legend", text: "Current permissions" - end + assert_selector ".govuk-label", text: "Add a permission" + assert_selector "legend", text: "Current permissions" end end - context "with JavaScript enabled" do + context "when the grantee has all permissions" do setup do - use_javascript_driver - shared_setup - end - - should "be able to grant permissions" do - assert_select_permission_to_grant_with_javascript - - click_button "Add and finish" - - expected_permissions = [*@old_permissions_to_keep, @new_permission_to_grant, @old_permission_to_remove_without_javascript] - assert_flash_content(expected_permissions.map(&:name)) - expected_permissions.each { |expected_permission| assert @grantee.has_permission?(expected_permission) } - - unexpected_permissions = @new_permissions_to_leave - refute_flash_content(unexpected_permissions.map(&:name)) - unexpected_permissions.each { |unexpected_permission| assert_not @grantee.has_permission?(unexpected_permission) } - end - - should "grant permissions then redirect back to the form when clicking 'Add'" do - assert_select_permission_to_grant_with_javascript - - click_button "Add" - - expected_permissions = [*@old_permissions_to_keep, @new_permission_to_grant, @old_permission_to_remove_without_javascript] - expected_permissions.each { |expected_permission| assert @grantee.has_permission?(expected_permission) } - - unexpected_permissions = @new_permissions_to_leave - unexpected_permissions.each { |unexpected_permission| assert_not @grantee.has_permission?(unexpected_permission) } - - h1_content = @grantee_is_self ? "Update permissions for #{@application.name}" : "Update #{@grantee.name}'s permissions for #{@application.name}" - assert page.has_selector?("h1", text: h1_content) - assert_flash_content("You have successfully added the permission '#{@new_permission_to_grant.name}'.") - end - - should "reset the value of the select element when it no longer matches what's shown in the autocomplete input" do - assert_select_permission_to_grant_with_javascript - - @autocomplete_input.fill_in with: "addin" - autocomplete_option = find(".autocomplete__option") - - assert_equal "addin", @autocomplete_input.value - assert_equal @new_permission_to_grant.name, autocomplete_option.text - assert_equal "", @select_element.value - - @autocomplete_input.native.send_keys :escape - click_button "Add and finish" - - assert_permissions_unchanged - assert_flash_content("You must select a permission.") + @grantee.grant_application_permissions(@application, [*@new_permissions_to_leave, @new_permission_to_grant].map(&:name)) end - should "clear the value of the select and autocomplete elements when clicking the clear button" do - assert_select_permission_to_grant_with_javascript - - click_button "Clear selection" - - assert_equal "", @autocomplete_input.value - assert_equal "", @select_element.value + should "only display the current permissions form" do + @grantee_is_self ? assert_edit_self : assert_edit_other_user(@grantee) - click_button "Add and finish" + click_link "Update permissions for #{@application.name}" - assert_permissions_unchanged - assert_flash_content("You must select a permission.") + assert_no_selector ".govuk-label", text: "Add a permission" + assert_selector "legend", text: "Current permissions" end + end - should "clear the value of the select and autocomplete elements when hitting space on the clear button" do - assert_select_permission_to_grant_with_javascript - - clear_button = find(".js-autocomplete__clear-button") - clear_button.native.send_keys :space - - assert_equal "", @autocomplete_input.value - assert_equal "", @select_element.value - - click_button "Add and finish" - - assert_permissions_unchanged - assert_flash_content("You must select a permission.") + context "when the grantee has no permissions" do + setup do + old_permission_ids = [*@old_permissions_to_keep, @old_permission_to_remove_without_javascript].pluck(:id) + UserApplicationPermission.where(user: @grantee, supported_permission_id: old_permission_ids).destroy_all end - should "clear the value of the select and autocomplete elements when hitting enter on the clear button" do - assert_select_permission_to_grant_with_javascript - - clear_button = find(".js-autocomplete__clear-button") - clear_button.native.send_keys :enter - - assert_equal "", @autocomplete_input.value - assert_equal "", @select_element.value + should "only display the new permissions form" do + @grantee_is_self ? assert_edit_self : assert_edit_other_user(@grantee) - click_button "Add and finish" + click_link "Update permissions for #{@application.name}" - assert_permissions_unchanged - assert_flash_content("You must select a permission.") + assert_selector ".govuk-label", text: "Add a permission" + assert_no_selector "legend", text: "Current permissions" end end end