diff --git a/app/assets/config/manifest.js b/app/assets/config/manifest.js index e1d6e089d..f2c77e945 100644 --- a/app/assets/config/manifest.js +++ b/app/assets/config/manifest.js @@ -3,3 +3,5 @@ //= link legacy_layout.css //= link application.js //= link legacy_layout.js + +//= link components/_option-select.css diff --git a/app/assets/images/option-select/input-icon.svg b/app/assets/images/option-select/input-icon.svg new file mode 100644 index 000000000..ad3de76f1 --- /dev/null +++ b/app/assets/images/option-select/input-icon.svg @@ -0,0 +1,3 @@ + + + diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js index 5bd8106c0..7de4925fa 100644 --- a/app/assets/javascripts/application.js +++ b/app/assets/javascripts/application.js @@ -1,7 +1,5 @@ //= require govuk_publishing_components/dependencies //= require govuk_publishing_components/all_components //= require_tree ./modules -//= require rails-ujs - -// Components from this application //= require_tree ./components +//= require rails-ujs diff --git a/app/assets/javascripts/components/option-select.js b/app/assets/javascripts/components/option-select.js new file mode 100644 index 000000000..7fd99fbc0 --- /dev/null +++ b/app/assets/javascripts/components/option-select.js @@ -0,0 +1,304 @@ +window.GOVUK = window.GOVUK || {} +window.GOVUK.Modules = window.GOVUK.Modules || {}; + +(function (Modules) { + /* This JavaScript provides two functional enhancements to option-select components: + 1) A count that shows how many results have been checked in the option-container + 2) Open/closing of the list of checkboxes + */ + function OptionSelect ($module) { + this.$optionSelect = $module + this.$options = this.$optionSelect.querySelectorAll("input[type='checkbox']") + this.$optionsContainer = this.$optionSelect.querySelector('.js-options-container') + this.$optionList = this.$optionsContainer.querySelector('.js-auto-height-inner') + this.$allCheckboxes = this.$optionsContainer.querySelectorAll('.govuk-checkboxes__item') + this.hasFilter = this.$optionSelect.getAttribute('data-filter-element') || '' + + this.checkedCheckboxes = [] + } + + OptionSelect.prototype.init = function () { + if (this.hasFilter.length) { + var filterEl = document.createElement('div') + filterEl.innerHTML = this.hasFilter + + var optionSelectFilter = document.createElement('div') + optionSelectFilter.classList.add('app-c-option-select__filter') + optionSelectFilter.innerHTML = filterEl.childNodes[0].nodeValue + + this.$optionsContainer.parentNode.insertBefore(optionSelectFilter, this.$optionsContainer) + + this.$filter = this.$optionSelect.querySelector('input[name="option-select-filter"]') + this.$filterCount = document.getElementById(this.$filter.getAttribute('aria-describedby')) + this.filterTextSingle = ' ' + this.$filterCount.getAttribute('data-single') + this.filterTextMultiple = ' ' + this.$filterCount.getAttribute('data-multiple') + this.filterTextSelected = ' ' + this.$filterCount.getAttribute('data-selected') + this.checkboxLabels = [] + this.filterTimeout = 0 + + this.getAllCheckedCheckboxes() + for (var i = 0; i < this.$allCheckboxes.length; i++) { + this.checkboxLabels.push(this.cleanString(this.$allCheckboxes[i].textContent)) + } + + this.$filter.addEventListener('keyup', this.typeFilterText.bind(this)) + } + + // Attach listener to update checked count + this.$optionsContainer.querySelector('.gem-c-checkboxes__list').addEventListener('change', this.updateCheckedCount.bind(this)) + + // Replace div.container-head with a button + this.replaceHeadingSpanWithButton() + + // Add js-collapsible class to parent for CSS + this.$optionSelect.classList.add('js-collapsible') + + // Add open/close listeners + var button = this.$optionSelect.querySelector('.js-container-button') + button.addEventListener('click', this.toggleOptionSelect.bind(this)) + + var closedOnLoad = this.$optionSelect.getAttribute('data-closed-on-load') + var closedOnLoadMobile = this.$optionSelect.getAttribute('data-closed-on-load-mobile') + + // By default the .filter-content container is hidden on mobile + // By checking if .filter-content is hidden, we are in mobile view given the current implementation + var isFacetsContentHidden = this.isFacetsContainerHidden() + + // Check if the option select should be closed for mobile screen sizes + var closedForMobile = closedOnLoadMobile === 'true' && isFacetsContentHidden + + // Always set the contain height to 200px for mobile screen sizes + if (closedForMobile) { + this.setContainerHeight(200) + } + + if (closedOnLoad === 'true' || closedForMobile) { + this.close() + } else { + this.setupHeight() + } + + var checkedString = this.checkedString() + if (checkedString) { + this.attachCheckedCounter(checkedString) + } + } + + OptionSelect.prototype.typeFilterText = function (event) { + event.stopPropagation() + var ENTER_KEY = 13 + + if (event.keyCode !== ENTER_KEY) { + clearTimeout(this.filterTimeout) + this.filterTimeout = setTimeout( + function () { this.doFilter(this) }.bind(this), + 300 + ) + } else { + event.preventDefault() // prevents finder forms from being submitted when user presses ENTER + } + } + + OptionSelect.prototype.cleanString = function cleanString (text) { + text = text.replace(/&/g, 'and') + text = text.replace(/[’',:–-]/g, '') // remove punctuation characters + text = text.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') // escape special characters + return text.trim().replace(/\s\s+/g, ' ').toLowerCase() // replace multiple spaces with one + } + + OptionSelect.prototype.getAllCheckedCheckboxes = function getAllCheckedCheckboxes () { + this.checkedCheckboxes = [] + + for (var i = 0; i < this.$options.length; i++) { + if (this.$options[i].checked) { + this.checkedCheckboxes.push(i) + } + } + } + + OptionSelect.prototype.doFilter = function doFilter (obj) { + var filterBy = obj.cleanString(obj.$filter.value) + var showCheckboxes = obj.checkedCheckboxes.slice() + var i = 0 + + for (i = 0; i < obj.$allCheckboxes.length; i++) { + if (showCheckboxes.indexOf(i) === -1 && obj.checkboxLabels[i].search(filterBy) !== -1) { + showCheckboxes.push(i) + } + } + + for (i = 0; i < obj.$allCheckboxes.length; i++) { + obj.$allCheckboxes[i].style.display = 'none' + } + + for (i = 0; i < showCheckboxes.length; i++) { + obj.$allCheckboxes[showCheckboxes[i]].style.display = 'block' + } + + var lenChecked = obj.$optionsContainer.querySelectorAll('.govuk-checkboxes__input:checked').length + var len = showCheckboxes.length + lenChecked + var html = len + (len === 1 ? obj.filterTextSingle : obj.filterTextMultiple) + ', ' + lenChecked + obj.filterTextSelected + obj.$filterCount.innerHTML = html + } + + OptionSelect.prototype.replaceHeadingSpanWithButton = function replaceHeadingSpanWithButton () { + /* Replace the span within the heading with a button element. This is based on feedback from Léonie Watson. + * The button has all of the accessibility hooks that are used by screen readers and etc. + * We do this in the JavaScript because if the JavaScript is not active then the button shouldn't + * be there as there is no JS to handle the click event. + */ + var containerHead = this.$optionSelect.querySelector('.js-container-button') + var jsContainerHeadHTML = containerHead.innerHTML + + // Create button and replace the preexisting html with the button. + var button = document.createElement('button') + button.setAttribute('class', 'js-container-button app-c-option-select__title app-c-option-select__button') + // Add type button to override default type submit when this component is used within a form + button.setAttribute('type', 'button') + button.setAttribute('aria-expanded', true) + button.setAttribute('id', containerHead.getAttribute('id')) + button.setAttribute('aria-controls', this.$optionsContainer.getAttribute('id')) + button.innerHTML = jsContainerHeadHTML + containerHead.parentNode.replaceChild(button, containerHead) + + // GA4 Accordion tracking. Relies on the ga4-finder-tracker setting the index first, so we wrap this in a custom event. + window.addEventListener('ga4-filter-indexes-added', function () { + if (window.GOVUK.analyticsGa4) { + if (window.GOVUK.analyticsGa4.Ga4FinderTracker) { + window.GOVUK.analyticsGa4.Ga4FinderTracker.addFilterButtonTracking(button, button.innerHTML) + } + } + }) + } + + OptionSelect.prototype.attachCheckedCounter = function attachCheckedCounter (checkedString) { + var element = document.createElement('div') + element.setAttribute('class', 'app-c-option-select__selected-counter js-selected-counter') + element.innerHTML = checkedString + this.$optionSelect.querySelector('.js-container-button').insertAdjacentElement('afterend', element) + } + + OptionSelect.prototype.updateCheckedCount = function updateCheckedCount () { + var checkedString = this.checkedString() + var checkedStringElement = this.$optionSelect.querySelector('.js-selected-counter') + + if (checkedString) { + if (checkedStringElement === null) { + this.attachCheckedCounter(checkedString) + } else { + checkedStringElement.textContent = checkedString + } + } else if (checkedStringElement) { + checkedStringElement.parentNode.removeChild(checkedStringElement) + } + } + + OptionSelect.prototype.checkedString = function checkedString () { + this.getAllCheckedCheckboxes() + var count = this.checkedCheckboxes.length + var checkedString = false + if (count > 0) { + checkedString = count + ' selected' + } + + return checkedString + } + + OptionSelect.prototype.toggleOptionSelect = function toggleOptionSelect (e) { + if (this.isClosed()) { + this.open() + } else { + this.close() + } + e.preventDefault() + } + + OptionSelect.prototype.open = function open () { + if (this.isClosed()) { + this.$optionSelect.querySelector('.js-container-button').setAttribute('aria-expanded', true) + this.$optionSelect.classList.remove('js-closed') + this.$optionSelect.classList.add('js-opened') + if (!this.$optionsContainer.style.height) { + this.setupHeight() + } + } + } + + OptionSelect.prototype.close = function close () { + this.$optionSelect.classList.remove('js-opened') + this.$optionSelect.classList.add('js-closed') + this.$optionSelect.querySelector('.js-container-button').setAttribute('aria-expanded', false) + } + + OptionSelect.prototype.isClosed = function isClosed () { + return this.$optionSelect.classList.contains('js-closed') + } + + OptionSelect.prototype.setContainerHeight = function setContainerHeight (height) { + this.$optionsContainer.style.height = height + 'px' + } + + OptionSelect.prototype.isCheckboxVisible = function isCheckboxVisible (option) { + var initialOptionContainerHeight = this.$optionsContainer.clientHeight + var optionListOffsetTop = this.$optionList.getBoundingClientRect().top + var distanceFromTopOfContainer = option.getBoundingClientRect().top - optionListOffsetTop + return distanceFromTopOfContainer < initialOptionContainerHeight + } + + OptionSelect.prototype.getVisibleCheckboxes = function getVisibleCheckboxes () { + var visibleCheckboxes = [] + for (var i = 0; i < this.$options.length; i++) { + if (this.isCheckboxVisible(this.$options[i])) { + visibleCheckboxes.push(this.$options[i]) + } + } + + // add an extra checkbox, if the label of the first is too long it collapses onto itself + if (this.$options[visibleCheckboxes.length]) { + visibleCheckboxes.push(this.$options[visibleCheckboxes.length]) + } + return visibleCheckboxes + } + + OptionSelect.prototype.isFacetsContainerHidden = function isFacetsContainerHidden () { + var facetsContent = this.$optionSelect.parentElement + var isFacetsContentHidden = false + // check whether this is hidden by progressive disclosure, + // because height calculations won't work + // would use offsetParent === null but for IE10+ + if (facetsContent) { + isFacetsContentHidden = !(facetsContent.offsetWidth || facetsContent.offsetHeight || facetsContent.getClientRects().length) + } + + return isFacetsContentHidden + } + + OptionSelect.prototype.setupHeight = function setupHeight () { + var initialOptionContainerHeight = this.$optionsContainer.clientHeight + var height = this.$optionList.offsetHeight + + var isFacetsContainerHidden = this.isFacetsContainerHidden() + + if (isFacetsContainerHidden) { + initialOptionContainerHeight = 200 + height = 200 + } + + // Resize if the list is only slightly bigger than its container + // If isFacetsContainerHidden is true, then 200 < 250 + // And the container height is always set to 201px + if (height < initialOptionContainerHeight + 50) { + this.setContainerHeight(height + 1) + return + } + + // Resize to cut last item cleanly in half + var visibleCheckboxes = this.getVisibleCheckboxes() + + var lastVisibleCheckbox = visibleCheckboxes[visibleCheckboxes.length - 1] + var position = lastVisibleCheckbox.parentNode.offsetTop // parent element is relative + this.setContainerHeight(position + (lastVisibleCheckbox.clientHeight / 1.5)) + } + + Modules.OptionSelect = OptionSelect +})(window.GOVUK.Modules) diff --git a/app/assets/javascripts/legacy/modules/dropdown_filter.js b/app/assets/javascripts/legacy/modules/dropdown_filter.js deleted file mode 100644 index 79572bbb4..000000000 --- a/app/assets/javascripts/legacy/modules/dropdown_filter.js +++ /dev/null @@ -1,49 +0,0 @@ -(function (Modules) { - 'use strict' - Modules.DropdownFilter = function () { - var that = this - that.start = function (element) { - var list = element.find('.js-filter-list') - var listItems = list.find('li:not(:first)') - var listInput = element.find('.js-filter-list-input') - - // Prevent dropdowns with text inputs from closing when - // interacting with them - element.on('click', '.js-filter-list-input', function (event) { event.stopPropagation() }) - - element.on('shown.bs.dropdown', focusInput) - element.on('keyup change', '.js-filter-list-input', filterListBasedOnInput) - element.on('submit', 'form', openFirstVisibleLink) - - // Set explicit width inline, so filtering doesn't change dropdown size - list.width(list.width()) - - function filterListBasedOnInput (event) { - var searchString = $.trim(listInput.val()) - var regExp = new RegExp(searchString, 'i') - - listItems.each(function () { - var item = $(this) - if (item.text().search(regExp) > -1) { - item.show() - } else { - item.hide() - } - }) - } - - function openFirstVisibleLink (evt) { - evt.preventDefault() - var link = list.find('a:visible').first() - GOVUKAdmin.redirect(link.attr('href')) - } - - function focusInput (event) { - var container = $(event.target) - setTimeout(function () { - container.find('input[type="text"]').focus() - }, 50) - } - } - } -})(window.GOVUKAdmin.Modules) diff --git a/app/assets/javascripts/modules/auto-submit-form.js b/app/assets/javascripts/modules/auto-submit-form.js new file mode 100644 index 000000000..a400db3d2 --- /dev/null +++ b/app/assets/javascripts/modules/auto-submit-form.js @@ -0,0 +1,21 @@ +window.GOVUK = window.GOVUK || {} +window.GOVUK.Modules = window.GOVUK.Modules || {}; + +(function (Modules) { + 'use strict' + + function AutoSubmitForm (module) { + this.module = module + this.module.ignore = this.module.getAttribute('data-auto-submit-ignore').split(',') + } + + AutoSubmitForm.prototype.init = function () { + this.module.addEventListener('change', function (e) { + if (!this.module.ignore.includes(e.target.getAttribute('name'))) { + this.module.submit() + } + }.bind(this)) + } + + Modules.AutoSubmitForm = AutoSubmitForm +})(window.GOVUK.Modules) diff --git a/app/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss index 58486e4f8..a3d8c3f3d 100644 --- a/app/assets/stylesheets/application.scss +++ b/app/assets/stylesheets/application.scss @@ -1,3 +1,5 @@ +$govuk-page-width: 1140px; + @import "govuk_publishing_components/all_components"; @import "user_research_recruitment_banner"; @@ -42,6 +44,10 @@ padding: 7px 10px; } +.app-bottom-separator { + border-bottom: 1px solid $govuk-border-colour; +} + .govuk-error-message p { margin: 0; } @@ -73,3 +79,9 @@ .govuk-table__cell--line-through { text-decoration: line-through; } + +.users-index-button-group { + @include govuk-media-query($from: tablet) { + float: right; + } +} diff --git a/app/assets/stylesheets/components/_option-select.scss b/app/assets/stylesheets/components/_option-select.scss new file mode 100644 index 000000000..2470abbdb --- /dev/null +++ b/app/assets/stylesheets/components/_option-select.scss @@ -0,0 +1,168 @@ +@import "govuk_publishing_components/individual_component_support"; + +.app-c-option-select { + position: relative; + padding: 0 0 govuk-spacing(2); + margin-bottom: govuk-spacing(2); + border-bottom: 1px solid $govuk-border-colour; + + @include govuk-media-query($from: desktop) { + // Redefine scrollbars on desktop where these lists are scrollable + // so they are always visible in option lists + ::-webkit-scrollbar { + -webkit-appearance: none; + width: 7px; + } + + ::-webkit-scrollbar-thumb { + border-radius: 4px; + + background-color: rgba(0, 0, 0, .5); + -webkit-box-shadow: 0 0 1px rgba(255, 255, 255, .87); + } + } + + .gem-c-checkboxes { + margin: 0; + } +} + +.app-c-option-select__title { + @include govuk-font(19); + margin: 0; +} + +.app-c-option-select__button { + z-index: 1; + background: none; + border: 0; + text-align: left; + padding: 0; + cursor: pointer; + color: $govuk-link-colour; + + &:hover { + text-decoration: underline; + text-underline-offset: .1em; + @include govuk-link-hover-decoration; + } + + &::-moz-focus-inner { + border: 0; + } + + &:focus { + outline: none; + text-decoration: none; + @include govuk-focused-text; + } + + &[disabled] { + background-image: none; + color: inherit; + } + + // Extend the touch area of the button to span the heading + &::after { + content: ""; + position: absolute; + top: 0; + right: 0; + bottom: 0; + left: 0; + z-index: 2; + } +} + +.app-c-option-select__icon { + display: none; + position: absolute; + top: 0; + left: 9px; + width: 30px; + height: 40px; + fill: govuk-colour("black"); +} + +.app-c-option-select__container { + position: relative; + max-height: 200px; + overflow-y: auto; + overflow-x: hidden; + background-color: govuk-colour("white"); + + &:focus { + outline: 0; + } +} + +.app-c-option-select__container--large { + max-height: 600px; +} + +.app-c-option-select__container-inner { + padding: govuk-spacing(1) 13px; +} + +.app-c-option-select__filter { + position: relative; + background: govuk-colour("white"); + padding: 13px 13px govuk-spacing(2) 13px; +} + +.app-c-option-select__filter-input { + @include govuk-font(19); + padding-left: 33px; + background: image-url("option-select/input-icon.svg") govuk-colour("white") no-repeat -5px -3px; + + @include govuk-media-query($from: tablet) { + @include govuk-font(16); + } +} + +.js-enabled { + .app-c-option-select__heading { + position: relative; + padding: 10px 8px 5px 43px; + } + + [aria-expanded="true"] ~ .app-c-option-select__icon--up { + display: block; + } + + [aria-expanded="false"] ~ .app-c-option-select__icon--down { + display: block; + } + + .app-c-option-select__container { + height: 200px; + } + + .app-c-option-select__container--large { + height: 600px; + } + + [data-closed-on-load="true"] .app-c-option-select__container { + display: none; + } +} + +.app-c-option-select__selected-counter { + @include govuk-font($size: 14); + color: $govuk-text-colour; + margin-top: 3px; +} + +.app-c-option-select.js-closed { + .app-c-option-select__filter, + .app-c-option-select__container { + display: none; + } +} + +.app-c-option-select.js-opened { + .app-c-option-select__filter, + .app-c-option-select__container { + display: block; + } +} diff --git a/app/assets/stylesheets/components/_table.scss b/app/assets/stylesheets/components/_table.scss index 3390b5345..b44c60574 100644 --- a/app/assets/stylesheets/components/_table.scss +++ b/app/assets/stylesheets/components/_table.scss @@ -6,6 +6,7 @@ $table-border-colour: govuk-colour("mid-grey", $legacy: "grey-2"); $table-header-border-width: 2px; $table-header-background-colour: govuk-colour("light-grey", $legacy: "grey-3"); $vertical-row-bottom-border-width: 3px; +$vertical-on-smallscreen-breakpoint: 940px; $sort-link-active-colour: govuk-colour("white"); $sort-link-arrow-size: 14px; $sort-link-arrow-size-small: 8px; @@ -157,7 +158,7 @@ $table-row-even-background-colour: govuk-colour("light-grey", $legacy: "grey-4") text-align: right; } - @include govuk-media-query($until: tablet) { + @include govuk-media-query($until: $vertical-on-smallscreen-breakpoint) { .govuk-table__cell { padding-right: 0; } @@ -178,7 +179,7 @@ $table-row-even-background-colour: govuk-colour("light-grey", $legacy: "grey-4") word-break: initial; } - @include govuk-media-query($from: tablet) { + @include govuk-media-query($from: $vertical-on-smallscreen-breakpoint) { .govuk-table__head { clip: auto; -webkit-clip-path: none; diff --git a/app/assets/stylesheets/legacy/_filters.scss b/app/assets/stylesheets/legacy/_filters.scss deleted file mode 100644 index ef4e4b290..000000000 --- a/app/assets/stylesheets/legacy/_filters.scss +++ /dev/null @@ -1,102 +0,0 @@ -// stylelint-disable selector-no-qualifying-type -- There are a large number -// of violations of this rule in this file that should be fixed. - -.filters .panel-heading { - background: transparent; -} - -a.filter-option, -a.filter-option:visited, -.filter-option { - color: inherit !important; // stylelint-disable-line declaration-no-important - - .glyphicon { - @extend %glyphicon-smaller-than-text; - } - - &.filter-selected { - font-weight: bold; - background: #eeeeee; - - &:hover, - &:focus, - &:active { - background: #dddddd; - } - } - - .selected-and-available & { - float: left; - - &:first-child { - border-top-right-radius: 0; - border-bottom-right-radius: 0; - } - - &:first-child + .filter-option { - border-top-left-radius: 0; - border-bottom-left-radius: 0; - border-left: 1px solid #dddddd; - } - } -} - -.remove-filters { - float: right; - margin: 5px; -} - -.filter-by-name-field { - width: 290px; - - &::placeholder { - color: #757575; - } -} - -// Nav compact - -.nav-compact > li > a { - padding: 5px 10px; -} - -// Nav pills - -.nav-pill-text { - position: relative; - display: block; - padding: 10px 15px; -} - -.nav-compact .nav-pill-text { - padding: 5px 10px; -} - -.nav-compact .nav-pill-text:first-child { - padding-left: 0; -} - -// Dropdown filter for Organisation & Permission - -.filter-by-organisation-menu, -.filter-by-permission-menu { - .dropdown-menu { - max-height: 300px; - overflow-y: scroll; - } - - abbr { - cursor: pointer; - } -} - -// List filter - -.list-filter { - background: #eeeeee; - padding: 10px $default-horizontal-margin; - margin-top: -5px; - margin-bottom: $default-vertical-margin / 2; - border-radius: $border-radius-base $border-radius-base 0 0; - border-bottom: 1px solid $dropdown-border; -} diff --git a/app/assets/stylesheets/legacy_layout.scss b/app/assets/stylesheets/legacy_layout.scss index 9ec297016..8c34c62b8 100644 --- a/app/assets/stylesheets/legacy_layout.scss +++ b/app/assets/stylesheets/legacy_layout.scss @@ -2,6 +2,5 @@ @import "chosen"; @import "legacy/bootstrap_chosen"; -@import "legacy/filters"; @import "legacy/admin"; @import "legacy/thin_form"; diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 9e8cdba12..a81e219e7 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -3,12 +3,13 @@ class UsersController < ApplicationController include UserPermissionsControllerMethods - layout "admin_layout", only: %w[edit_email_or_password event_logs require_2sv] + layout "admin_layout", only: %w[index edit_email_or_password event_logs require_2sv] before_action :authenticate_user!, except: :show before_action :load_and_authorize_user, except: %i[index show] before_action :allow_no_application_access, only: [:update] - helper_method :applications_and_permissions, :any_filter? + before_action :redirect_legacy_filters, only: [:index] + helper_method :applications_and_permissions, :filter_params respond_to :html before_action :doorkeeper_authorize!, only: :show @@ -28,13 +29,14 @@ def show def index authorize User - @users = policy_scope(User).includes(:organisation).order(:name) - filter_users if any_filter? + @filter = UsersFilter.new(policy_scope(User), current_user, filter_params) + respond_to do |format| format.html do - paginate_users + @users = @filter.paginated_users end format.csv do + @users = @filter.users headers["Content-Disposition"] = 'attachment; filename="signon_users.csv"' render plain: export, content_type: "text/csv" end @@ -133,37 +135,10 @@ def load_and_authorize_user authorize @user end - def filter_users - @users = @users.filter_by_name(params[:filter]) if params[:filter].present? - @users = @users.with_role(params[:role]) if can_filter_role? - @users = @users.with_permission(params[:permission]) if params[:permission].present? - @users = @users.with_organisation(params[:organisation]) if params[:organisation].present? - @users = @users.with_status(params[:status]) if params[:status].present? - @users = @users.with_2sv_enabled(params[:two_step_status]) if params[:two_step_status].present? - end - - def can_filter_role? - params[:role].present? && - current_user.manageable_roles.include?(params[:role]) - end - def should_include_permissions? params[:format] == "csv" end - def paginate_users - @users = @users.page(params[:page]).per(25) - end - - def any_filter? - params[:filter].present? || - params[:role].present? || - params[:permission].present? || - params[:status].present? || - params[:organisation].present? || - params[:two_step_status].present? - end - def validate_token_matches_client_id # FIXME: Once gds-sso is updated everywhere, this should always validate # the client_id param. It should 401 if no client_id is given. @@ -177,7 +152,7 @@ def export CSV.generate do |csv| presenter = UserExportPresenter.new(applications) csv << presenter.header_row - @users.includes(:organisation).find_each do |user| + @users.find_each do |user| csv << presenter.row(user) end end @@ -212,4 +187,19 @@ def password_params :password_confirmation, ) end + + def filter_params + params.permit( + :filter, :page, :format, :"option-select-filter", + *LegacyUsersFilter::PARAM_KEYS, + **UsersFilter::PERMITTED_CHECKBOX_FILTER_PARAMS + ).except(:"option-select-filter") + end + + def redirect_legacy_filters + filter = LegacyUsersFilter.new(filter_params) + if filter.redirect? + redirect_to users_path(filter.options) + end + end end diff --git a/app/helpers/user_filter_helper.rb b/app/helpers/user_filter_helper.rb deleted file mode 100644 index 9ad1777bc..000000000 --- a/app/helpers/user_filter_helper.rb +++ /dev/null @@ -1,114 +0,0 @@ -module UserFilterHelper - def current_path_with_filter(filter_type, filter_value) - query_parameters = (request.query_parameters.clone || {}) - filter_value.nil? ? query_parameters.delete(filter_type) : query_parameters[filter_type] = filter_value - query_string = query_parameters.map { |k, v| "#{k}=#{v}" }.join("&") - "#{request.path_info}?#{query_string}" - end - - def user_role_text - "#{params[:role]} users".strip.humanize.capitalize - end - - def two_step_abbr_tag - tag.abbr("2SV", title: "Two step verification") - end - - def title_from(filter_type) - if filter_type == :two_step_status - safe_join([two_step_abbr_tag, "Status"], " ") - else - filter_type.to_s.humanize.capitalize - end - end - - def user_filter_list_items(filter_type) - items = case filter_type - when :role - filtered_user_roles - when :permission - Doorkeeper::Application - .joins(:supported_permissions) - .order("oauth_applications.name", "supported_permissions.name") - .pluck("supported_permissions.id", "oauth_applications.name", "supported_permissions.name") - .map { |e| [e[0], "#{e[1]} #{e[2]}"] } - when :status - User::USER_STATUSES - when :organisation - if current_user.super_organisation_admin? - current_user.organisation.subtree.order(:name).joins(:users).uniq.map { |org| [org.id, org.name_with_abbreviation] } - else - Organisation.order(:name).joins(:users).uniq.map { |org| [org.id, org.name_with_abbreviation] } - end - when :two_step_status - [["true", "Enabled"], ["false", "Not set up"], ["exempt", "Exempted"]] - end - - list_items = items.map do |item| - if item.is_a? String - item_id = item - item_name = item.humanize - else - item_id = item[0].to_s - item_name = item[1] - end - tag.li( - link_to(item_name, current_path_with_filter(filter_type, item_id)), - class: params[filter_type] == item_id ? "active" : "", - ) - end - - list_items << tag.li( - link_to( - "All #{title_from(filter_type).pluralize}".html_safe, - current_path_with_filter(filter_type, nil), - ), - ) - - list_items.join("\n").html_safe - end - - def filtered_user_roles - current_user.manageable_roles - end - - def value_from(filter_type) - value = params[filter_type] - return nil if value.blank? - - case filter_type - when :organisation - org = Organisation.find(value) - if org.abbreviation.presence - tag.abbr(org.abbreviation, title: org.name) - else - org.name - end - when :permission - Doorkeeper::Application - .joins(:supported_permissions) - .where("supported_permissions.id = ?", value) - .pick("oauth_applications.name", "supported_permissions.name") - .join(" ") - when :two_step_status - case value - when "true" - "Enabled" - when "exempt" - "Exempted" - else - "Not set up" - end - else - value.humanize.capitalize - end - end - - def any_filter? - params[:filter].present? || - params[:role].present? || - params[:permission].present? || - params[:status].present? || - params[:organisation].present? - end -end diff --git a/app/helpers/users_helper.rb b/app/helpers/users_helper.rb index 2623f963f..fd5f5c2f0 100644 --- a/app/helpers/users_helper.rb +++ b/app/helpers/users_helper.rb @@ -1,12 +1,10 @@ module UsersHelper + def two_step_abbr_tag + tag.abbr("2SV", title: "Two step verification") + end + def two_step_status(user) - if user.has_2sv? - "Enabled" - elsif user.exempt_from_2sv? - "Exempted" - else - "Not set up" - end + user.two_step_status.humanize.capitalize end def organisation_options(form_builder) @@ -41,12 +39,25 @@ def sync_needed?(permissions) max_updated_at.present? && max_last_synced_at.present? ? max_updated_at > max_last_synced_at : false end - def link_to_users_csv(text, params, options = {}) - merged_params = params.permit(:filter, :role, :permission, :status, :organisation, :two_step_status).merge(format: "csv") - link_to text, merged_params, options - end - def formatted_number_of_users(users) pluralize(number_with_delimiter(users.total_count), "user") end + + def filtered_users_heading(users) + count = formatted_number_of_users(users) + if current_user.manageable_organisations.one? + "#{count} in #{current_user.manageable_organisations.first.name}" + else + count + end + end + + def assignable_user_roles + current_user.manageable_roles + end + + def user_name(user) + anchor_tag = link_to(user.name, edit_user_path(user), class: "govuk-link") + user.suspended? ? content_tag(:del, anchor_tag) : anchor_tag + end end diff --git a/app/models/legacy_users_filter.rb b/app/models/legacy_users_filter.rb new file mode 100644 index 000000000..257855f35 --- /dev/null +++ b/app/models/legacy_users_filter.rb @@ -0,0 +1,32 @@ +class LegacyUsersFilter + PARAM_KEYS = %i[status two_step_status role permission organisation].freeze + LEGACY_TWO_STEP_STATUS_VS_TWO_STEP_STATUS = { + "true" => User::TWO_STEP_STATUS_ENABLED, + "false" => User::TWO_STEP_STATUS_NOT_SET_UP, + "exempt" => User::TWO_STEP_STATUS_EXEMPTED, + }.freeze + + def initialize(options = {}) + @options = options + end + + def redirect? + !@options.slice(*PARAM_KEYS).empty? + end + + def options + @options.except(*PARAM_KEYS).tap do |o| + o[:statuses] = [@options[:status]] if @options[:status].present? + o[:two_step_statuses] = [two_step_status_from(@options[:two_step_status])] if @options[:two_step_status].present? + o[:roles] = [@options[:role]] if @options[:role].present? + o[:organisations] = [@options[:organisation]] if @options[:organisation].present? + o[:permissions] = [@options[:permission]] if @options[:permission].present? + end + end + +private + + def two_step_status_from(legacy_two_step_status) + LEGACY_TWO_STEP_STATUS_VS_TWO_STEP_STATUS[legacy_two_step_status] + end +end diff --git a/app/models/user.rb b/app/models/user.rb index 063bbcec9..3937790bf 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -20,6 +20,16 @@ class User < ApplicationRecord USER_STATUS_LOCKED, USER_STATUS_ACTIVE].freeze + TWO_STEP_STATUS_ENABLED = "enabled".freeze + TWO_STEP_STATUS_NOT_SET_UP = "not_set_up".freeze + TWO_STEP_STATUS_EXEMPTED = "exempted".freeze + + TWO_STEP_STATUSES_VS_NAMED_SCOPES = { + TWO_STEP_STATUS_ENABLED => "has_2sv", + TWO_STEP_STATUS_NOT_SET_UP => "not_setup_2sv", + TWO_STEP_STATUS_EXEMPTED => "exempt_from_2sv", + }.freeze + devise :database_authenticatable, :recoverable, :trackable, @@ -33,6 +43,8 @@ class User < ApplicationRecord :confirmable, :password_archivable # in signon/lib/devise/models/password_archivable.rb + delegate :manageable_roles, to: :role_class + encrypts :otp_secret_key validates :name, presence: true @@ -60,11 +72,28 @@ class User < ApplicationRecord before_save :strip_whitespace_from_name scope :web_users, -> { where(api_user: false) } + + scope :suspended, -> { where.not(suspended_at: nil) } scope :not_suspended, -> { where(suspended_at: nil) } - scope :with_role, ->(role_name) { where(role: role_name) } - scope :with_permission, ->(permission_id) { joins(:supported_permissions).where("supported_permissions.id = ?", permission_id) } - scope :with_organisation, ->(org_id) { where(organisation_id: org_id) } - scope :filter_by_name, ->(filter_param) { where("users.email like ? OR users.name like ?", "%#{filter_param.strip}%", "%#{filter_param.strip}%") } + scope :invited, -> { where.not(invitation_sent_at: nil).where(invitation_accepted_at: nil) } + scope :not_invited, -> { where(invitation_sent_at: nil).or(where.not(invitation_accepted_at: nil)) } + scope :locked, -> { where.not(locked_at: nil) } + scope :not_locked, -> { where(locked_at: nil) } + scope :active, -> { not_suspended.not_invited.not_locked } + + scope :exempt_from_2sv, -> { where.not(reason_for_2sv_exemption: nil) } + scope :not_exempt_from_2sv, -> { where(reason_for_2sv_exemption: nil) } + scope :has_2sv, -> { where.not(otp_secret_key: nil) } + scope :does_not_have_2sv, -> { where(otp_secret_key: nil) } + scope :not_setup_2sv, -> { not_exempt_from_2sv.does_not_have_2sv } + + scope :with_role, ->(role) { where(role:) } + scope :with_permission, ->(permission) { joins(:supported_permissions).merge(SupportedPermission.where(id: permission)) } + scope :with_organisation, ->(organisation) { where(organisation:) } + scope :with_partially_matching_name, ->(name) { where(arel_table[:name].matches("%#{name}%")) } + scope :with_partially_matching_email, ->(email) { where(arel_table[:email].matches("%#{email}%")) } + scope :with_partially_matching_name_or_email, ->(value) { with_partially_matching_name(value).or(with_partially_matching_email(value)) } + scope :last_signed_in_on, ->(date) { web_users.not_suspended.where("date(current_sign_in_at) = date(?)", date) } scope :last_signed_in_before, ->(date) { web_users.not_suspended.where("date(current_sign_in_at) < date(?)", date) } scope :last_signed_in_after, ->(date) { web_users.not_suspended.where("date(current_sign_in_at) >= date(?)", date) } @@ -72,35 +101,26 @@ class User < ApplicationRecord scope :expired_never_signed_in, -> { never_signed_in.where("invitation_sent_at < ?", NEVER_SIGNED_IN_EXPIRY_PERIOD.ago) } scope :not_recently_unsuspended, -> { where(["unsuspended_at IS NULL OR unsuspended_at < ?", UNSUSPENSION_GRACE_PERIOD.ago]) } scope :with_access_to_application, ->(application) { UsersWithAccess.new(self, application).users } - scope :with_2sv_enabled, - lambda { |enabled| - case enabled - when "exempt" - where("reason_for_2sv_exemption IS NOT NULL") - when "true" - where("otp_secret_key IS NOT NULL") - else - where("otp_secret_key IS NULL AND reason_for_2sv_exemption IS NULL") - end - } - - scope :with_status, - lambda { |status| - case status - when USER_STATUS_SUSPENDED - where.not(suspended_at: nil) - when USER_STATUS_INVITED - where.not(invitation_sent_at: nil).where(invitation_accepted_at: nil) - when USER_STATUS_LOCKED - where.not(locked_at: nil) - when USER_STATUS_ACTIVE - where(suspended_at: nil, locked_at: nil) - .where(arel_table[:invitation_sent_at].eq(nil) - .or(arel_table[:invitation_accepted_at].not_eq(nil))) - else - raise NotImplementedError, "Filtering by status '#{status}' not implemented." - end - } + + def self.with_statuses(statuses) + permitted_statuses = statuses.intersection(USER_STATUSES) + relations = permitted_statuses.map { |s| public_send(s) } + relation = relations.pop || all + while (next_relation = relations.pop) + relation = relation.or(next_relation) + end + relation + end + + def self.with_2sv_statuses(scope_names) + permitted_scopes = scope_names.intersection(TWO_STEP_STATUSES_VS_NAMED_SCOPES.values) + relations = permitted_scopes.map { |s| public_send(s) } + relation = relations.pop || all + while (next_relation = relations.pop) + relation = relation.or(next_relation) + end + relation + end def require_2sv? return require_2sv unless organisation @@ -259,8 +279,26 @@ def status USER_STATUS_ACTIVE end - def manageable_roles - "Roles::#{role.camelize}".constantize.manageable_roles + def two_step_status + if has_2sv? + TWO_STEP_STATUS_ENABLED + elsif exempt_from_2sv? + TWO_STEP_STATUS_EXEMPTED + else + TWO_STEP_STATUS_NOT_SET_UP + end + end + + def role_class + Roles.const_get(role.classify) + end + + def can_manage?(other_user) + manageable_roles.include?(other_user.role) + end + + def manageable_organisations + role_class.manageable_organisations_for(self).order(:name) end # Make devise send all emails using ActiveJob diff --git a/app/models/users_filter.rb b/app/models/users_filter.rb new file mode 100644 index 000000000..299a17a15 --- /dev/null +++ b/app/models/users_filter.rb @@ -0,0 +1,98 @@ +class UsersFilter + CHECKBOX_FILTER_KEYS = %i[statuses two_step_statuses roles permissions organisations].freeze + PERMITTED_CHECKBOX_FILTER_PARAMS = CHECKBOX_FILTER_KEYS.each.with_object({}) { |k, h| h[k] = [] }.freeze + + attr_reader :options + + def self.with_checked_at_top(options) + options.sort_by { |o| o[:checked] ? 0 : 1 } + end + + def initialize(users, current_user, options = {}) + @users = users + @current_user = current_user + @options = options + @options[:per_page] ||= 25 + end + + def users + filtered_users = @users + filtered_users = filtered_users.with_partially_matching_name_or_email(options[:filter].strip) if options[:filter] + filtered_users = filtered_users.with_statuses(options[:statuses]) if options[:statuses] + filtered_users = filtered_users.with_2sv_statuses(options[:two_step_statuses]) if options[:two_step_statuses] + filtered_users = filtered_users.with_role(options[:roles]) if options[:roles] + filtered_users = filtered_users.with_permission(options[:permissions]) if options[:permissions] + filtered_users = filtered_users.with_organisation(options[:organisations]) if options[:organisations] + filtered_users.includes(:organisation).order(:name) + end + + def paginated_users + users.page(options[:page]).per(options[:per_page]) + end + + def status_option_select_options(aria_controls_id: nil) + User::USER_STATUSES.map do |status| + { + label: status.humanize.capitalize, + controls: aria_controls_id, + value: status, + checked: Array(options[:statuses]).include?(status), + }.compact + end + end + + def two_step_status_option_select_options(aria_controls_id: nil) + User::TWO_STEP_STATUSES_VS_NAMED_SCOPES.map do |status, scope_name| + { + label: status.humanize.capitalize, + controls: aria_controls_id, + value: scope_name, + checked: Array(options[:two_step_statuses]).include?(scope_name), + }.compact + end + end + + def role_option_select_options(aria_controls_id: nil) + @current_user.manageable_roles.map do |role| + { + label: role.humanize.capitalize, + controls: aria_controls_id, + value: role, + checked: Array(options[:roles]).include?(role), + }.compact + end + end + + def permission_option_select_options(aria_controls_id: nil) + Doorkeeper::Application.includes(:supported_permissions).flat_map do |application| + application.supported_permissions.map do |permission| + { + label: "#{application.name} #{permission.name}", + controls: aria_controls_id, + value: permission.to_param, + checked: Array(options[:permissions]).include?(permission.to_param), + }.compact + end + end + end + + def organisation_option_select_options(aria_controls_id: nil) + scope = @current_user.manageable_organisations + scope.order(:name).joins(:users).uniq.map do |organisation| + { + label: organisation.name_with_abbreviation, + controls: aria_controls_id, + value: organisation.to_param, + checked: Array(options[:organisations]).include?(organisation.to_param), + }.compact + end + end + + def no_options_selected_for?(key) + Array(options[key]).none? + end + + def any_options_selected? + options.slice(*CHECKBOX_FILTER_KEYS).values.any? + end +end diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb index 2cac400f9..6d26412b7 100644 --- a/app/policies/user_policy.rb +++ b/app/policies/user_policy.rb @@ -71,7 +71,7 @@ def allow_self_only end def can_manage? - Roles.const_get(current_user.role.classify).can_manage?(record.role) + current_user.can_manage?(record) end class Scope < ::BasePolicy::Scope diff --git a/app/views/components/_option_select.html.erb b/app/views/components/_option_select.html.erb new file mode 100644 index 000000000..144b3a9e3 --- /dev/null +++ b/app/views/components/_option_select.html.erb @@ -0,0 +1,69 @@ +<% add_app_component_stylesheet("option-select") %> +<% + title_id = "option-select-title-#{title.parameterize}" + checkboxes_id = "checkboxes-#{SecureRandom.hex(4)}" + checkboxes_count_id = checkboxes_id + "-count" + show_filter ||= false + large ||= false + + classes = %w[app-c-option-select__container js-options-container] + classes << "app-c-option-select__container--large" if large +%> + +<% if show_filter %> + <% + filter_id ||= "input-#{SecureRandom.hex(4)}" + %> + <% filter = capture do %> + <%= tag.label for: filter_id, class: "govuk-label govuk-visually-hidden" do %> + Filter <%= title %> + <% end %> + + <%= tag.input name: "option-select-filter", + id: filter_id, + class: "app-c-option-select__filter-input govuk-input", + type: "text", + aria: { + describedby: checkboxes_count_id, + controls: checkboxes_id + } + %> + <% end %> + <% filter_element = CGI::escapeHTML(filter) %> +<% end %> + +
data-closed-on-load="true"<% end %> + <% if local_assigns.include?(:closed_on_load_mobile) && closed_on_load_mobile %>data-closed-on-load-mobile="true"<% end %> + <% if local_assigns.include?(:aria_controls_id) %>data-input-aria-controls="<%= aria_controls_id %>"<% end %> + <% if show_filter %>data-filter-element="<%= filter_element %>"<% end %> +> +

+ <%= title %> + + +

+ + <%= content_tag(:div, role: "group", aria: { labelledby: title_id }, class: classes, id: options_container_id, tabindex: "-1") do %> +
+ <% if show_filter %> + + <% end %> + <%= render "govuk_publishing_components/components/checkboxes", { + name: "#{key}[]", + id: checkboxes_id, + heading: title, + small: true, + visually_hide_heading: true, + no_hint_text: true, + items: options + } %> +
+ <% end %> +
diff --git a/app/views/components/docs/option_select.yml b/app/views/components/docs/option_select.yml new file mode 100644 index 000000000..4ffc68686 --- /dev/null +++ b/app/views/components/docs/option_select.yml @@ -0,0 +1,343 @@ +name: Option select +description: A scrollable list of checkboxes to be displayed on a form where one might + otherwise use a multi-select +body: +accessibility_criteria: | + The option select must: + + - indicate that the option select is expandable/collapsible + - indicate the initial state of expandable content + - indicate where the state of expandable content has changed + + The option select inputs must: + + - have a margin to the right when the box is scrollable so that users can interact with a scrollbar without accidentally clicking an option + - only include an `aria-controls` attribute if an element with the ID specified exists on the page + - be [grouped with a label](https://www.w3.org/WAI/GL/wiki/Using_grouping_roles_to_identify_related_form_controls) + + The option select filter must: + + - be focusable with a keyboard + - indicate when it has keyboard focus + - inform the user that is it an editable field + - inform the user when there are matches, or if there are no matches + - inform the user as the number of matches changes +examples: + default: + data: + key: market_sector + title: Market sector + options_container_id: list_of_sectors + options: + - value: aerospace + label: Aerospace + id: aerospace + - value: agriculture-environment-and-natural-resources + label: Agriculture, environment and natural resources + id: agriculture-environment-and-natural-resources + - value: building-and-construction + label: Building and construction + id: building-and-construction + - value: chemicals + label: Chemicals + id: chemicals + - value: clothing-footwear-and-fashion + label: Clothing, footwear and fashion + id: clothing-footwear-and-fashion + - value: defence + label: Defence + id: defence + - value: distribution-and-service-industries + label: Distribution and Service Industries + id: distribution-and-service-industries + - value: electronics-industry + label: Electronics Industry + id: electronics-industry + - value: energy + label: Energy + id: energy + - value: engineering + label: Engineering + id: engineering + - value: financial-services + label: Financial services + id: financial-services + - value: fire-police-and-security + label: Fire, police, and security + id: fire-police-and-security + - value: food-manufacturing + label: Food manufacturing + id: food-manufacturing + - value: giftware-jewellery-and-tableware + label: Giftware, jewellery and tableware + id: giftware-jewellery-and-tableware + with_option_pre_checked: + data: + key: with_checked_value_set + title: List of options + options_container_id: list_of_vegetables + options: + - value: potatoes + label: Potatoes + checked: true + - value: carrots + label: Carrots + id: carrots + - value: mash + label: Mash + id: mash + with_aria_controls: + description: | + aria_controls_id adds an [`aria-controls` attribute](https://tink.uk/using-the-aria-controls-attribute/) to each checkbox input. This makes it easier for users of assitive tech to jump from them to the part of the page they’re updating. + + The aria_controls_id must be set to the ID of an element that’s on the page or it won’t be included. + data: + key: with_aria-control_set + title: List of options (with aria controls) + aria_controls_id: wrapper + options_container_id: list_of_countries + options: + - value: britain + label: Britain + id: britain + - value: france + label: France + id: france + - value: spain + label: Spain + id: spain + closed_on_load: + data: + key: closed_on_load + title: List of hats + closed_on_load: true + options_container_id: list_of_hats + options: + - value: bobble_hat + label: Bobble hat + id: bobble_hat + - value: fez + label: Fez + id: fez + - value: sombrero + label: Sombrero + id: sombrero + with_tracking: + data: + key: list_of_shoes + title: List of shoes + options_container_id: list_of_shoes + options: + - value: trainers + label: Trainers + id: trainers + data_attributes: + track_category: "filterClicked" + track_action: "List of shoes" + track_label: "trainers" + track_options: + dimension28: 1 + - value: plimsolls + label: Plimsolls + id: plimsolls + data_attributes: + track_category: "filterClicked" + track_action: "List of shoes" + track_label: "plimsolls" + track_options: + dimension28: 1 + - value: high_heels + label: High heels + id: high_heels + data_attributes: + track_category: "filterClicked" + track_action: "List of shoes" + track_label: "high_heels" + track_options: + dimension28: 1 + with_filter: + description: Adds a filter to allow users to narrow the checkboxes down. Checkboxes will only show if they match what the user has typed, or if they are already checked. The filter is case insensitive and strips out punctuation characters and duplicate whitespace, and sees '&' and 'and' as the same, to make filtering easier. + data: + key: filter_demo + title: Countries + options_container_id: list_of_countries_to_filter + show_filter: true + options: + - value: afghanistan + label: Afghanistan + id: afghanistan + - value: albania + label: Albania + id: albania + - value: algeria + label: Algeria + id: algeria + checked: true + - value: american_samoa + label: American Samoa + id: american_samoa + - value: andorra + label: Andorra + id: andorra + checked: true + - value: angola + label: Angola + id: angola + - value: anguilla + label: Anguilla + id: anguilla + - value: antigua_and_barbuda + label: Antigua and Barbuda + id: antigua_and_barbuda + - value: argentina + label: Argentina + id: argentina + - value: armenia + label: Armenia + id: armenia + - value: aruba + label: Aruba + id: aruba + - value: australia + label: Australia + id: australia + - value: austria + label: Austria + id: austria + - value: azerbaijan + label: Azerbaijan + id: azerbaijan + - value: bahamas + label: Bahamas + id: bahamas + - value: bahrain + label: Bahrain + id: bahrain + - value: bangladesh + label: Bangladesh + id: bangladesh + - value: barbados + label: Barbados + id: barbados + - value: belarus + label: Belarus + id: belarus + - value: belgium + label: Belgium + id: belgium + - value: belize + label: Belize + id: belize + - value: benin + label: Benin + id: benin + - value: bermuda + label: Bermuda + id: bermuda + - value: bhutan + label: Bhutan + id: bhutan + - value: bolivia + label: Bolivia + id: bolivia + - value: cote + label: Cote d’Ivoire + id: cote + - value: sthelena + label: St Helena, Ascension and Tristan da Cunha + id: sthelena + - value: trinidad + label: Trinidad & Tobago + id: trinidad + large: + description: When `large` is set to `true`, the option-select container has an increased max-height of 600px, allowing more checkbox items to be visible. + data: + key: large_demo + title: Countries + options_container_id: list_of_countries_to_filter_large + large: true + options: + - value: afghanistan + label: Afghanistan + id: afghanistanLargeExample + - value: albania + label: Albania + id: albaniaLargeExample + - value: algeria + label: Algeria + id: algeriaLargeExample + - value: american_samoa + label: American Samoa + id: american_samoaLargeExample + - value: andorra + label: Andorra + id: andorraLargeExample + - value: angola + label: Angola + id: angolaLargeExample + - value: anguilla + label: Anguilla + id: anguillaLargeExample + - value: antigua_and_barbuda + label: Antigua and Barbuda + id: antigua_and_barbudaLargeExample + - value: argentina + label: Argentina + id: argentinaLargeExample + - value: armenia + label: Armenia + id: armeniaLargeExample + - value: aruba + label: Aruba + id: arubaLargeExample + - value: australia + label: Australia + id: australiaLargeExample + - value: austria + label: Austria + id: austriaLargeExample + - value: azerbaijan + label: Azerbaijan + id: azerbaijanLargeExample + - value: bahamas + label: Bahamas + id: bahamasLargeExample + - value: bahrain + label: Bahrain + id: bahrainLargeExample + - value: bangladesh + label: Bangladesh + id: bangladeshLargeExample + - value: barbados + label: Barbados + id: barbadosLargeExample + - value: belarus + label: Belarus + id: belarusLargeExample + - value: belgium + label: Belgium + id: belgiumLargeExample + - value: belize + label: Belize + id: belizeLargeExample + - value: benin + label: Benin + id: beninLargeExample + - value: bermuda + label: Bermuda + id: bermudaLargeExample + - value: bhutan + label: Bhutan + id: bhutanLargeExample + - value: bolivia + label: Bolivia + id: boliviaLargeExample + - value: cote + label: Cote d'Ivoire + id: coteLargeExample + - value: sthelena + label: St Helena, Ascension and Tristan da Cunha + id: sthelenaLargeExample + - value: trinidad + label: Trinidad & Tobago + id: trinidadLargeExample diff --git a/app/views/layouts/admin_layout.html.erb b/app/views/layouts/admin_layout.html.erb index 10bb75669..a994b1de5 100644 --- a/app/views/layouts/admin_layout.html.erb +++ b/app/views/layouts/admin_layout.html.erb @@ -36,8 +36,7 @@ <%= yield(:error_summary) %>
-
- <%= yield(:context) %> +

<% if yield(:page_heading).present? %> <%= yield(:page_heading) %> @@ -46,6 +45,11 @@ <% end %>

+ <% if yield(:top_right) %> +
+ <%= yield(:top_right) %> +
+ <% end %>
<%= yield %> diff --git a/app/views/users/_form_fields.html.erb b/app/views/users/_form_fields.html.erb index 5dc3484c1..90fb69452 100644 --- a/app/views/users/_form_fields.html.erb +++ b/app/views/users/_form_fields.html.erb @@ -28,7 +28,7 @@ <% if policy(User).assign_role? && @user.reason_for_2sv_exemption.blank? %>

<%= f.label :role %>
- <%= f.select :role, options_for_select(filtered_user_roles.map(&:humanize).zip(filtered_user_roles), f.object.role), {}, class: "chosen-select form-control", 'data-module' => 'chosen' %> + <%= f.select :role, options_for_select(assignable_user_roles.map(&:humanize).zip(assignable_user_roles), f.object.role), {}, class: "chosen-select form-control", 'data-module' => 'chosen' %> Superadmins can create and edit all user types and edit applications.
Admins can create and edit normal users.
diff --git a/app/views/users/_user_filter.html.erb b/app/views/users/_user_filter.html.erb deleted file mode 100644 index 92520bf14..000000000 --- a/app/views/users/_user_filter.html.erb +++ /dev/null @@ -1,45 +0,0 @@ -

- -<%= render partial: 'user_filter_group_no_js', locals: {filter_type: :role} %> -<%= render partial: 'user_filter_group_no_js', locals: {filter_type: :permission} %> -<%= render partial: 'user_filter_group_no_js', locals: {filter_type: :status} %> -<%= render partial: 'user_filter_group_no_js', locals: {filter_type: :organisation} %> -<%= render partial: 'user_filter_group_no_js', locals: {filter_type: :two_step_status} %> diff --git a/app/views/users/_user_filter_group.html.erb b/app/views/users/_user_filter_group.html.erb deleted file mode 100644 index 93f69ba84..000000000 --- a/app/views/users/_user_filter_group.html.erb +++ /dev/null @@ -1,40 +0,0 @@ -<% - filter_title = title_from(filter_type) - filter_value = value_from(filter_type) -%> -
  • <% if params[filter_type] %>selected-and-available<% end %>" <% if filter_type.in?([:organisation, :permission]) %>data-module="dropdown-filter"<% end %>> - <% if params[filter_type] %> - <%= link_to current_path_with_filter(filter_type, nil), - class: 'filter-option filter-selected', - title: 'Remove filter' do %> - Remove - <% end %> - <% end %> - <%= link_to current_path_with_filter(filter_type, ''), - class: "filter-option #{params[filter_type] ? 'filter-selected' : ''}", - data: { toggle: 'dropdown' }, - role: 'button' do %> - <%= filter_value || filter_title %> - <%= filter_title %> - - <% end %> - -
  • diff --git a/app/views/users/_user_filter_group_no_js.html.erb b/app/views/users/_user_filter_group_no_js.html.erb deleted file mode 100644 index 4c216a274..000000000 --- a/app/views/users/_user_filter_group_no_js.html.erb +++ /dev/null @@ -1,17 +0,0 @@ -<% if params[filter_type] %> - <% filter_title = title_from(filter_type) %> -
    - -
    -<% end %> diff --git a/app/views/users/_users_filter.html.erb b/app/views/users/_users_filter.html.erb new file mode 100644 index 000000000..e8164b974 --- /dev/null +++ b/app/views/users/_users_filter.html.erb @@ -0,0 +1,80 @@ +
    + <%= form_with url: users_path, method: :get, data: { module: "auto-submit-form", "auto-submit-ignore": "option-select-filter" } do |form| %> +
    + <%= render "govuk_publishing_components/components/search", { + id: "name_or_email_filter", + name: "filter", + label_text: "Search by name or email", + label_size: "s", + aria_controls: filtered_users_id, + value: filter.options[:filter], + margin_bottom: 4, + } %> +
    + + <%= render "govuk_publishing_components/components/heading", { + text: "Filter results", + padding: true, + } %> + + <%= render "components/option_select", { + key: "statuses", + title: "Status", + options_container_id: "statuses_filter", + closed_on_load: @filter.no_options_selected_for?(:statuses) && @filter.any_options_selected?, + options: @filter.status_option_select_options(aria_controls_id: filtered_users_id), + } %> + + <%= render "components/option_select", { + key: "two_step_statuses", + title: "2SV Status", + options_container_id: "two_step_statuses_filter", + closed_on_load: @filter.no_options_selected_for?(:two_step_statuses), + options: @filter.two_step_status_option_select_options(aria_controls_id: filtered_users_id), + } %> + + <%= render "components/option_select", { + key: "roles", + title: "Roles", + options_container_id: "roles_filter", + closed_on_load: @filter.no_options_selected_for?(:roles), + options: @filter.role_option_select_options(aria_controls_id: filtered_users_id), + } %> + + <% if current_user.manageable_organisations.many? %> + <%= render "components/option_select", { + key: "organisations", + title: "Organisations", + options_container_id: "organisations_filter", + show_filter: true, + closed_on_load: @filter.no_options_selected_for?(:organisations), + options: UsersFilter.with_checked_at_top( + @filter.organisation_option_select_options(aria_controls_id: filtered_users_id) + ), + } %> + <% end %> + + <%= render "components/option_select", { + key: "permissions", + title: "Permissions", + options_container_id: "permissions_filter", + show_filter: true, + closed_on_load: @filter.no_options_selected_for?(:permissions), + options: UsersFilter.with_checked_at_top( + @filter.permission_option_select_options(aria_controls_id: filtered_users_id) + ), + } %> + +
    + <%= render "govuk_publishing_components/components/button", { + text: "Update results", + } %> + + <%= link_to "Clear all filters", users_path, class: "govuk-link govuk-link--no-visited-state" %> +
    + <% end %> + +
    + <%= link_to "Export #{formatted_number_of_users(@users)} as CSV", url_for(filter_params.merge(format: "csv")), class: "govuk-link govuk-link--no-visited-state" %> +
    +
    diff --git a/app/views/users/index.html.erb b/app/views/users/index.html.erb index c5a92234e..c50858579 100644 --- a/app/views/users/index.html.erb +++ b/app/views/users/index.html.erb @@ -1,75 +1,75 @@ -<% content_for :title, user_role_text %> +<% content_for :title, "Users" %> +<% content_for :breadcrumbs, + render("govuk_publishing_components/components/breadcrumbs", { + collapse_on_mobile: true, + breadcrumbs: [ + { + title: "Dashboard", + url: root_path, + }, + { + title: "Users", + } + ] + }) +%> -
    -

    <%= user_role_text %>

    -
    +<% content_for :top_right do %> +
    <% if policy(User).new? %> - <%= link_to "Create user", new_user_invitation_path, class: "btn btn-success add-right-margin" %> - <%= link_to "Upload a batch of users", new_batch_invitation_path, class: "btn btn-default add-right-margin" %> + <%= render "govuk_publishing_components/components/button", { + text: "Create user", + href: new_user_invitation_path, + margin_bottom: 4, + } %> + <%= render "govuk_publishing_components/components/button", { + text: "Upload a batch of users", + href: new_batch_invitation_path, + secondary: true, + margin_bottom: 4, + } %> <% end %> <% if policy(BulkGrantPermissionSet).new? %> - <%= link_to "Grant access to all users", new_bulk_grant_permission_set_path, class: "btn btn-default" %> + <%= render "govuk_publishing_components/components/button", { + text: "Grant access to all users", + href: new_bulk_grant_permission_set_path, + secondary: true, + margin_bottom: 4, + } %> <% end %>
    -
    - -
    -

    <%= formatted_number_of_users(@users) %>

    - <%= link_to_users_csv "Export #{formatted_number_of_users(@users)} as CSV", params, class: "btn btn-default pull-right" %> -
    +<% end %> -<%= render partial: "user_filter" %> +<% filtered_users_id = "filtered-users" %> - - - - - - - - - - - - - - - <% @users.each do |user| %> - - - - - - - - - - +
    +
    + <%= render "users_filter", filter: @filter, filtered_users_id: %> +
    +
    + <%= content_tag :div, id: filtered_users_id do %> + <%= render "components/table", { + caption: filtered_users_heading(@users), + vertical_on_small_screen: true, + head: [ + { text: "Name" }, + { text: "Email" }, + { text: "Role" }, + { text: "Status" }, + { text: "#{two_step_abbr_tag} Status".html_safe }, + ], + rows: @users.map do |user| + [ + { text: user_name(user) }, + { text: user.email }, + { text: user.role.humanize }, + { text: user.status.humanize }, + { text: two_step_status(user) }, + ] + end, + } %> <% end %> -
    -
    NameEmailRoleOrganisationLast sign-inStatus<%= two_step_abbr_tag %> StatusActions
    - <%= user.unusable_account? ? "".html_safe : "" %> - - <%= link_to "#{user.name}", edit_user_path(user) %> - - <%= user.unusable_account? ? "".html_safe : "" %> - <%= user.role.humanize %><%= user.organisation.try(:name) %><%= user.status.humanize %><%= two_step_status(user) %> - <% if user.invited_but_not_accepted %> - <%= form_tag resend_user_invitation_path(user) do %> - <%= submit_tag "Resend signup email", :class => 'btn btn-sm btn-default' %> - <% end %> - <% end %> - <% if user.access_locked? %> - <%= form_tag unlock_user_path(user) do %> - <%= submit_tag "Unlock account", :class => 'btn btn-default' %> - <% end %> - <% end %> -
    -<%= paginate(@users, theme: 'twitter-bootstrap-3') %> + <%= paginate(@users, theme: "gds") %> +
    +
    diff --git a/config/locales/components/option_select.yml b/config/locales/components/option_select.yml new file mode 100644 index 000000000..1eb1060a0 --- /dev/null +++ b/config/locales/components/option_select.yml @@ -0,0 +1,6 @@ +en: + components: + option_select: + found_single: option found + found_multiple: options found + selected: selected diff --git a/lib/roles/admin.rb b/lib/roles/admin.rb index c3cd2d207..c7ec2edc3 100644 --- a/lib/roles/admin.rb +++ b/lib/roles/admin.rb @@ -26,5 +26,9 @@ def self.level def self.manageable_roles %w[normal organisation_admin super_organisation_admin admin] end + + def self.manageable_organisations_for(_) + Organisation.all + end end end diff --git a/lib/roles/base.rb b/lib/roles/base.rb index 9e36e1dbd..5b0a5cd86 100644 --- a/lib/roles/base.rb +++ b/lib/roles/base.rb @@ -1,7 +1,3 @@ module Roles - class Base - def self.can_manage?(role) - manageable_roles.include?(role) - end - end + class Base; end end diff --git a/lib/roles/normal.rb b/lib/roles/normal.rb index cd6000b3c..8fd67cfff 100644 --- a/lib/roles/normal.rb +++ b/lib/roles/normal.rb @@ -15,5 +15,9 @@ def self.level def self.manageable_roles [] end + + def self.manageable_organisations_for(_) + Organisation.where("false") + end end end diff --git a/lib/roles/organisation_admin.rb b/lib/roles/organisation_admin.rb index 4a106eb12..9f2500fb7 100644 --- a/lib/roles/organisation_admin.rb +++ b/lib/roles/organisation_admin.rb @@ -26,5 +26,9 @@ def self.level def self.manageable_roles %w[normal organisation_admin] end + + def self.manageable_organisations_for(user) + Organisation.where(id: user.organisation) + end end end diff --git a/lib/roles/super_organisation_admin.rb b/lib/roles/super_organisation_admin.rb index 38adba77a..0e64c410a 100644 --- a/lib/roles/super_organisation_admin.rb +++ b/lib/roles/super_organisation_admin.rb @@ -26,5 +26,9 @@ def self.level def self.manageable_roles %w[normal organisation_admin super_organisation_admin] end + + def self.manageable_organisations_for(user) + Organisation.where(id: user.organisation.subtree) + end end end diff --git a/lib/roles/superadmin.rb b/lib/roles/superadmin.rb index 78d8a202b..44a5def39 100644 --- a/lib/roles/superadmin.rb +++ b/lib/roles/superadmin.rb @@ -27,5 +27,9 @@ def self.level def self.manageable_roles User.roles end + + def self.manageable_organisations_for(_) + Organisation.all + end end end diff --git a/spec/javascripts/legacy/modules/dropdown_filter.spec.js b/spec/javascripts/legacy/modules/dropdown_filter.spec.js deleted file mode 100644 index 02561019f..000000000 --- a/spec/javascripts/legacy/modules/dropdown_filter.spec.js +++ /dev/null @@ -1,81 +0,0 @@ -describe('A dropdown filter module', function () { - 'use strict' - - var dropdownFilter - var listElement - - beforeEach(function () { - listElement = $('
    ' + - '' + - '
    ') - - $('body').append(listElement) - dropdownFilter = new GOVUKAdmin.Modules.DropdownFilter() - dropdownFilter.start(listElement) - }) - - afterEach(function () { - listElement.remove() - }) - - it('filters the dropdown list based on input', function () { - filterBy('another') - expect(listElement.find('.first').is(':visible')).toBe(false) - expect(listElement.find('.second').is(':visible')).toBe(true) - - filterBy('something') - expect(listElement.find('.first').is(':visible')).toBe(true) - expect(listElement.find('.second').is(':visible')).toBe(false) - - filterBy('thing') - expect(listElement.find('.first').is(':visible')).toBe(true) - expect(listElement.find('.second').is(':visible')).toBe(true) - - filterBy('not a thing') - expect(listElement.find('.first').is(':visible')).toBe(false) - expect(listElement.find('.second').is(':visible')).toBe(false) - }) - - it('keeps the first list item visible, the filter input', function () { - filterBy('another') - expect(listElement.find('li:first').is(':visible')).toBe(true) - - filterBy('') - expect(listElement.find('li:first').is(':visible')).toBe(true) - - filterBy('not a thing') - expect(listElement.find('li:first').is(':visible')).toBe(true) - }) - - it('shows all items when no input is entered', function () { - filterBy('another') - filterBy('') - expect(listElement.find('.first').is(':visible')).toBe(true) - expect(listElement.find('.second').is(':visible')).toBe(true) - }) - - describe('when the form is submitted', function () { - it('opens the first visible link', function () { - spyOn(GOVUKAdmin, 'redirect') - filterBy('another') - listElement.find('form').trigger('submit') - expect(GOVUKAdmin.redirect).toHaveBeenCalledWith('/second-link') - }) - }) - - function filterBy (value) { - listElement.find('input').val(value).trigger('change') - } -}) diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb index 82f45e744..07b482526 100644 --- a/test/controllers/users_controller_test.rb +++ b/test/controllers/users_controller_test.rb @@ -271,13 +271,13 @@ def change_user_password(user_factory, new_password) should "list users" do create(:user, email: "another_user@email.com") get :index - assert_select "td.email", /another_user@email.com/ + assert_select "tr td:nth-child(2)", /another_user@email.com/ end should "not list api users" do create(:api_user, email: "api_user@email.com") get :index - assert_select "td.email", count: 0, text: /api_user@email.com/ + assert_select "tr td:nth-child(2)", count: 0, text: /api_user@email.com/ end should "not show superadmin users" do @@ -286,7 +286,7 @@ def change_user_password(user_factory, new_password) get :index assert_select "tbody tr", count: 1 - assert_select "td.email", /#{@user.email}/ + assert_select "tr td:nth-child(2)", /#{@user.email}/ end should "show user roles" do @@ -296,127 +296,142 @@ def change_user_password(user_factory, new_password) get :index - assert_select "td.role", "Normal" - assert_select "td.role", "Organisation admin" - assert_select "td.role", "Super organisation admin" - assert_select "td.role", "Admin" - assert_select "td.role", count: 4 + assert_select "tr td:nth-child(3)", /Normal/ + assert_select "tr td:nth-child(3)", /Organisation admin/ + assert_select "tr td:nth-child(3)", /Super organisation admin/ + assert_select "tr td:nth-child(3)", /Admin/ end - should "show user organisation" do - user = create(:user_in_organisation) - - get :index + context "filter" do + should "filter by partially matching name" do + create(:user, name: "does-match1") + create(:user, name: "does-match2") + create(:user, name: "does-not-match") - assert_select "td.organisation", user.organisation.name - end + get :index, params: { filter: "does-match" } - context "CSV export" do - should "respond to CSV format" do - get :index, params: { format: :csv } - assert_response :success - assert_equal "text/csv", @response.media_type + assert_select "tr td:nth-child(1)", text: /does-match/, count: 2 end - should "export filtered users by role" do - create(:user) - get :index, params: { role: "normal", format: :csv } - lines = @response.body.lines - assert_equal(2, lines.length) - end + should "filter by partially matching email" do + create(:user, email: "does-match1@example.com") + create(:user, email: "does-match2@example.com") + create(:user, email: "does-not-match@example.com") - should "export filtered users by all fields" do - organisation = create(:organisation, name: "Cookies Of Other Lands", abbreviation: "COOL") + get :index, params: { filter: "does-match" } - create(:suspended_user, name: "Special cookie", email: "specialcookie@email.com", role: "normal", organisation:) - create(:suspended_user, name: "Normal cookie", email: "normalcookie@email.com", role: "normal", organisation:) + assert_select "tr td:nth-child(2)", text: /does-match/, count: 2 + end - assert_equal(3, User.count) + should "filter by statuses" do + create(:active_user, name: "active-user") + create(:suspended_user, name: "suspended-user") + create(:invited_user, name: "invited-user") + create(:locked_user, name: "locked-user") - get :index, params: { role: "normal", status: "suspended", organisation:, two_step_status: false, filter: "special", format: :csv } + get :index, params: { statuses: %w[locked suspended] } - lines = @response.body.lines - assert_equal(2, lines.length) + assert_select "tr td:nth-child(1)", text: /active-user/, count: 0 + assert_select "tr td:nth-child(1)", text: /suspended-user/ + assert_select "tr td:nth-child(1)", text: /invited-user/, count: 0 + assert_select "tr td:nth-child(1)", text: /locked-user/ end - should "export all users when no filter selected" do - create(:user) - get :index, params: { format: :csv } - lines = @response.body.lines - assert_equal(3, lines.length) - end - end + should "filter by 2SV statuses" do + create(:user, name: "not-set-up-user") + create(:two_step_exempted_user, name: "exempted-user") + create(:two_step_enabled_user, name: "enabled-user") - context "filter" do - setup do - create(:user, email: "not_admin@gov.uk") + get :index, params: { two_step_statuses: %w[not_setup_2sv exempt_from_2sv] } + + assert_select "tr td:nth-child(1)", text: /not-set-up-user/ + assert_select "tr td:nth-child(1)", text: /exempted-user/ + assert_select "tr td:nth-child(1)", text: /enabled-user/, count: 0 end - should "filter results to users where their name or email contains the string" do - create(:user, email: "special@gov.uk") - create(:user, name: "Someone special", email: "someone@gov.uk") + should "filter by roles" do + create(:admin_user, name: "admin-user") + create(:organisation_admin_user, name: "organisation-admin-user") + create(:user, name: "normal-user") - get :index, params: { filter: "special" } + get :index, params: { roles: %w[admin normal] } - assert_select "tbody tr", count: 2 - assert_select "td.email", /special@gov.uk/ - assert_select "td.email", /someone@gov.uk/ + assert_select "tr td:nth-child(1)", text: /admin-user/ + assert_select "tr td:nth-child(1)", text: /organisation-admin-user/, count: 0 + assert_select "tr td:nth-child(1)", text: /normal-user/ end - should "scope list of users by role" do - get :index, params: { role: "normal" } + should "filter by permissions" do + app1 = create(:application, name: "App 1") + app2 = create(:application, name: "App 2") - assert_select "tbody tr", count: 1 - assert_select "td.email", /admin@gov.uk/ - end + permission1 = create(:supported_permission, application: app1, name: "Permission 1") - should "scope filtered list of users by role" do - create(:organisation_admin_user, email: "xyz@gov.uk") + create(:user, name: "user1", supported_permissions: [app1.signin_permission, permission1]) + create(:user, name: "user2", supported_permissions: []) + create(:user, name: "user3", supported_permissions: [app2.signin_permission, permission1]) - get :index, params: { filter: "admin", role: Roles::Admin.role_name } + get :index, params: { permissions: [app1.signin_permission, app2.signin_permission] } - assert_select "tbody tr", count: 1 - assert_select "td.email", /admin@gov.uk/ + assert_select "tr td:nth-child(1)", text: /user1/ + assert_select "tr td:nth-child(1)", text: /user2/, count: 0 + assert_select "tr td:nth-child(1)", text: /user3/ end - should "scope list of users by permission" do - user_application_permissions = create_list(:user_application_permission, 2) + should "filter by organisations" do + organisation1 = create(:organisation, name: "Organisation 1") + organisation2 = create(:organisation, name: "Organisation 2") + organisation3 = create(:organisation, name: "Organisation 3") - user_application_permissions.each do |uap| - get :index, params: { permission: uap.supported_permission_id } + create(:user, name: "user1-in-organisation1", organisation: organisation1) + create(:user, name: "user2-in-organisation1", organisation: organisation1) + create(:user, name: "user3-in-organisation2", organisation: organisation2) + create(:user, name: "user4-in-organisation3", organisation: organisation3) - assert_select "tbody tr", count: 1 - assert_select "td.email", /#{uap.user.email}/ - end - end - end + get :index, params: { organisations: [organisation1, organisation3] } - should "scope list of users by status" do - create(:suspended_user, email: "suspended_user@gov.uk") + assert_select "tr td:nth-child(1)", text: /user1-in-organisation1/ + assert_select "tr td:nth-child(1)", text: /user2-in-organisation1/ + assert_select "tr td:nth-child(1)", text: /user3-in-organisation2/, count: 0 + assert_select "tr td:nth-child(1)", text: /user4-in-organisation3/ + end - get :index, params: { status: "suspended" } + should "display link to clear all filters" do + get :index - assert_select "tbody tr", count: 1 - assert_select "td.email", /suspended_user@gov.uk/ - end + assert_select "a", text: "Clear all filters", href: users_path + end - should "scope list of users by status and role" do - create(:suspended_user, email: "suspended_user@gov.uk", role: Roles::Admin.role_name) - create(:suspended_user, email: "normal_suspended_user@gov.uk") + should "redirect legacy filters" do + organisation1 = create(:organisation, name: "Organisation 1") - get :index, params: { status: "suspended", role: Roles::Admin.role_name } + get :index, params: { organisation: organisation1 } - assert_select "tbody tr", count: 1 - assert_select "td.email", /suspended_user@gov.uk/ + assert_redirected_to users_path(organisations: [organisation1]) + end end - should "scope list of users by organisation" do - user = create(:user_in_organisation, email: "orgmember@gov.uk") + context "CSV export" do + should "respond to CSV format" do + get :index, params: { format: :csv } + assert_response :success + assert_equal "text/csv", @response.media_type + end - get :index, params: { organisation: user.organisation.id } + should "only include filtered users" do + create(:user, name: "does-match") + create(:user, name: "does-not-match") + get :index, params: { filter: "does-match", format: :csv } + number_of_users = CSV.parse(@response.body, headers: true).length + assert_equal 1, number_of_users + end - assert_select "tbody tr", count: 1 - assert_select "td.email", /orgmember@gov.uk/ + should "include all users when no filter selected" do + create(:user) + get :index, params: { format: :csv } + number_of_users = CSV.parse(@response.body, headers: true).length + assert_equal User.count, number_of_users + end end context "as superadmin" do @@ -426,7 +441,7 @@ def change_user_password(user_factory, new_password) get :index - assert_select "td.email", count: 0, text: /api_user@email.com/ + assert_select "tr td:nth-child(2)", count: 0, text: /api_user@email.com/ end end end @@ -940,6 +955,12 @@ def change_user_password(user_factory, new_password) assert_select "a", text: "Grant access to all users", count: 0 end + + should "not display organisations filter" do + get :index + + assert_select "#organisations_filter", count: 0 + end end end end diff --git a/test/factories/users.rb b/test/factories/users.rb index c78a3d7df..94965ae90 100644 --- a/test/factories/users.rb +++ b/test/factories/users.rb @@ -83,11 +83,24 @@ role { Roles::OrganisationAdmin.role_name } end + factory :invited_user, parent: :user do + invitation_sent_at { 1.minute.ago } + invitation_accepted_at { nil } + end + + factory :active_user, parent: :invited_user do + invitation_accepted_at { Time.zone.now } + end + factory :suspended_user, parent: :user do suspended_at { Time.zone.now } reason_for_suspension { "Testing" } end + factory :locked_user, parent: :user do + locked_at { Time.zone.now } + end + factory :user_in_organisation, parent: :user do association :organisation, factory: :organisation end diff --git a/test/helpers/users_helper_test.rb b/test/helpers/users_helper_test.rb index 7036071ed..cfdcc7e48 100644 --- a/test/helpers/users_helper_test.rb +++ b/test/helpers/users_helper_test.rb @@ -20,4 +20,27 @@ class UsersHelperTest < ActionView::TestCase test "two_step_status should reflect the user's status accurately when the user does not have 2sv set up" do assert_equal "Not set up", two_step_status(create(:user)) end + + context "#filtered_users_heading" do + setup do + @users = [build(:user), build(:user)] + @current_user = build(:user) + stubs(:formatted_number_of_users).with(@users).returns("2 users") + stubs(:current_user).returns(@current_user) + @organisation = build(:organisation) + end + + should "return formatted number of users" do + another_organisation = build(:organisation) + @current_user.stubs(:manageable_organisations).returns([@organisation, another_organisation]) + + assert_equal "2 users", filtered_users_heading(@users) + end + + should "return formatted number of users in specified organisation" do + @current_user.stubs(:manageable_organisations).returns([@organisation]) + + assert_equal "2 users in #{@organisation.name}", filtered_users_heading(@users) + end + end end diff --git a/test/integration/admin_user_index_test.rb b/test/integration/admin_user_index_test.rb index 33f310539..aab86e893 100644 --- a/test/integration/admin_user_index_test.rb +++ b/test/integration/admin_user_index_test.rb @@ -3,6 +3,8 @@ class AdminUserIndexTest < ActionDispatch::IntegrationTest context "logged in as an admin" do setup do + use_javascript_driver + current_time = Time.zone.now Timecop.freeze(current_time) @@ -13,14 +15,21 @@ class AdminUserIndexTest < ActionDispatch::IntegrationTest org1 = create(:organisation, name: "Org 1") org2 = create(:organisation, name: "Org 2") - create(:user, name: "Aardvark", email: "aardvark@example.com", current_sign_in_at: current_time - 5.minutes) - create(:two_step_enabled_user, name: "Abbey", email: "abbey@example.com") - create(:user, name: "Abbot", email: "mr_ab@example.com") - create(:user, name: "Bert", email: "bbbert@example.com") - create(:user, name: "Ed", email: "ed@example.com", organisation: org1) - create(:user, name: "Eddie", email: "eddie_bb@example.com") - create(:two_step_exempted_user, name: "Ernie", email: "ernie@example.com", organisation: org2) - create(:suspended_user, name: "Suspended McFee", email: "suspenders@example.com") + @aardvark = create(:user, name: "Aardvark", email: "aardvark@example.com", current_sign_in_at: current_time - 5.minutes) + @abbey = create(:two_step_enabled_user, name: "Abbey", email: "abbey@example.com") + @abbot = create(:user, name: "Abbot", email: "mr_ab@example.com") + @bert = create(:user, name: "Bert", email: "bbbert@example.com") + @ed = create(:user, name: "Ed", email: "ed@example.com", organisation: org1) + @eddie = create(:user, name: "Eddie", email: "eddie_bb@example.com") + @ernie = create(:two_step_exempted_user, name: "Ernie", email: "ernie@example.com", organisation: org2) + @suspended_mcfee = create(:suspended_user, name: "Suspended McFee", email: "suspenders@example.com") + + application = create(:application, name: "App name") + create(:supported_permission, application:, name: "App permission") + @aardvark.grant_application_signin_permission(application) + @bert.grant_application_permission(application, "App permission") + + visit "/users" end teardown do @@ -28,182 +37,77 @@ class AdminUserIndexTest < ActionDispatch::IntegrationTest end should "display the 2SV enrollment status for users" do - visit "/users" - within "table" do assert has_css?("td", text: "Enabled", count: 1) assert has_css?("td", text: "Not set up", count: 7) end end - should "see when the user last logged in" do - visit "/users" - - assert page.has_content?("Last sign-in") - - actual_last_sign_in_strings = page.all("table tr td.last-sign-in").map(&:text).map(&:strip)[0..1] - assert_equal ["5 minutes ago", "never signed in"], actual_last_sign_in_strings + should "list all users" do + assert_results @admin, @aardvark, @abbey, @abbot, @bert, @ed, @eddie, @ernie, @suspended_mcfee end - should "be able to filter users" do - visit "/users" - - fill_in "Name or email", with: "bb" - click_on "Filter" - - assert page.has_content?("Abbey abbey@example.com") - assert page.has_content?("Abbot mr_ab@example.com") - assert page.has_content?("Bert bbbert@example.com") - assert page.has_content?("Eddie eddie_bb@example.com") + should "filter users by search string" do + fill_in "Search", with: "bb" + click_on "Search" - assert_not page.has_content?("Aardvark aardvark@example.com") - assert_not page.has_content?("Ernie ernie@example.com") - - click_on "Users" - - assert page.has_content?("Aardvark aardvark@example.com") + assert_results @abbey, @abbot, @bert, @eddie end - should "filter users by role" do - visit "/users" - - assert_role_not_present("Superadmin") - - select_role("Normal") - - assert_equal User.with_role(:normal).count, page.all("table tbody tr").count - assert_not page.has_content?("Admin User admin@example.com") - User.with_role(:normal).each do |normal_user| - assert page.has_content?(normal_user.email) - end - - select_role("All Roles") + should "filter users by status" do + check "Suspended", allow_label_click: true - %w[Aardvark Abbot Abbey Admin].each do |user_name| - assert page.has_content?(user_name) - end + assert_results @suspended_mcfee end - should "filter users by permission" do - uap = create(:user_application_permission, user: User.find_by(name: "Ernie")) - visit "/users" + should "filter users by 2sv status" do + click_on "2SV Status" - select_permission("#{uap.application.name} #{uap.supported_permission.name}") - - assert_equal 1, page.all("table tbody tr").count - within ".table" do - assert page.has_content?("Ernie") - (User.pluck(:name) - %w[Ernie]).each do |name| - assert_not page.has_content?(name) - end - end + check "Exempted", allow_label_click: true + assert_results @ernie - select_permission("All Permissions") - - %w[Aardvark Abbot Abbey Admin].each do |user_name| - assert page.has_content?(user_name) - end + check "Enable", allow_label_click: true + assert_results @ernie, @abbey end - should "filter users by status" do - visit "/users" - - select_status("Suspended") - - assert_equal 1, page.all("table tbody tr").count - assert_not page.has_content?("Aardvark") - assert page.has_content?("Suspended McFee") - - select_status("All Statuses") + should "filter users by role" do + click_on "Role" - %w[Aardvark Abbot Abbey Admin Suspended].each do |user_name| - assert page.has_content?(user_name) - end + check "Admin", allow_label_click: true + assert_results @admin end should "filter users by organisation" do - visit "/users" - - select_organisation("Org 1") - assert_equal 1, page.all("table tbody tr").count - assert_not page.has_content?("Aardvark") - assert page.has_content?("Ed") + click_on "Organisation" - select_organisation("All Organisations") + check "Org 1", allow_label_click: true + assert_results @ed - %w[Aardvark Abbot Abbey Admin Suspended].each do |user_name| - assert page.has_content?(user_name) - end + check "Org 2", allow_label_click: true + assert_results @ed, @ernie end - should "filter users by 2SV status" do - visit "/users" - total_enabled = 1 - total_disabled = 7 - total_exempted = 1 - - within ".filter-by-two_step_status-menu .dropdown-menu" do - click_on "Enabled" - end - - assert has_css?("td", text: "Enabled", count: total_enabled) - assert has_css?(".filter-by-two_step_status-menu .filter-selected span", text: "Enabled") - assert has_no_css?("td", text: "Not set up") - assert has_no_css?("td", text: "Exempted") - - within ".filter-by-two_step_status-menu .dropdown-menu" do - click_on "Not set up" - end - - assert has_no_css?("td", text: "Enabled") - assert has_css?("td", text: "Not set up", count: total_disabled) - assert has_css?(".filter-by-two_step_status-menu .filter-selected span", text: "Not set up") - assert has_no_css?("td", text: "Exempted") + should "filter users by permission" do + click_on "Permissions" - within ".filter-by-two_step_status-menu .dropdown-menu" do - click_on "Exempted" - end + check "App name signin", allow_label_click: true + assert_results @aardvark - assert has_css?("td", text: "Exempted", count: total_exempted) - assert has_css?(".filter-by-two_step_status-menu .filter-selected span", text: "Exempted") - assert has_no_css?("td", text: "Not set up") - assert has_no_css?("td", text: "Enabled") + check "App name App permission", allow_label_click: true + assert_results @aardvark, @bert end end - def select_organisation(organisation_name) - within ".filter-by-organisation-menu .dropdown-menu" do - click_on organisation_name - end - end - - def select_status(status_name) - within ".filter-by-status-menu .dropdown-menu" do - click_on status_name - end - end +private - def assert_role_not_present(role_name) - within ".filter-by-role-menu" do - click_on "Role", match: :prefer_exact - within ".dropdown-menu" do - assert page.has_no_content? role_name - end - end - end + def assert_results(*users) + expected_table_caption = [users.count, "user".pluralize(users.count)].join(" ") - def select_role(role_name) - within ".filter-by-role-menu" do - click_on "Role", match: :prefer_exact - within ".dropdown-menu" do - click_on role_name - end - end - end + table = find("table caption", text: expected_table_caption).ancestor(:table) + assert table.has_css?("tbody tr", count: users.count) - def select_permission(permission_name) - within ".filter-by-permission-menu" do - click_on permission_name + users.each do |user| + assert table.has_content?(user.name) end end end diff --git a/test/integration/event_log_creation_test.rb b/test/integration/event_log_creation_test.rb index d709cfe34..b3206a970 100644 --- a/test/integration/event_log_creation_test.rb +++ b/test/integration/event_log_creation_test.rb @@ -129,9 +129,8 @@ class EventLogCreationIntegrationTest < ActionDispatch::IntegrationTest visit root_path signin_with(@admin) - first_letter_of_name = @user.name[0] - visit users_path(letter: first_letter_of_name) - click_on "Unlock" + visit user_path(@user) + click_on "Unlock account" visit event_logs_user_path(@user) assert page.has_content?("#{EventLog::MANUAL_ACCOUNT_UNLOCK.description} by #{@admin.name}") diff --git a/test/integration/user_locking_test.rb b/test/integration/user_locking_test.rb index a70c83f8e..52416e154 100644 --- a/test/integration/user_locking_test.rb +++ b/test/integration/user_locking_test.rb @@ -34,21 +34,6 @@ class UserLockingTest < ActionDispatch::IntegrationTest end end - should "be reversible by admins" do - admin = create(:admin_user) - user = create(:user) - user.lock_access! - - visit root_path - signin_with(admin) - first_letter_of_name = user.name[0] - visit users_path(letter: first_letter_of_name) - click_button "Unlock account" - - user.reload - assert_not user.access_locked? - end - should "be reversible from the user edit page" do admin = create(:admin_user) user = create(:user) diff --git a/test/models/legacy_users_filter_test.rb b/test/models/legacy_users_filter_test.rb new file mode 100644 index 000000000..1b42267a4 --- /dev/null +++ b/test/models/legacy_users_filter_test.rb @@ -0,0 +1,63 @@ +require "test_helper" + +class LegacyUsersFilterTest < ActiveSupport::TestCase + context "#redirect?" do + should "return true if there are any legacy filter params" do + filter = LegacyUsersFilter.new({ status: User::USER_STATUS_ACTIVE }) + + assert filter.redirect? + end + + should "return false if there are no legacy filter params" do + filter = LegacyUsersFilter.new({ non_legacy_filter_key: 456 }) + + assert_not filter.redirect? + end + end + + context "#options" do + should "transform status legacy filter -> statuses array" do + filter = LegacyUsersFilter.new({ status: User::USER_STATUS_ACTIVE }) + + assert_equal({ statuses: [User::USER_STATUS_ACTIVE] }, filter.options) + end + + should "transform two_step_status legacy filter -> two_step_statuses array when value is 'true'" do + filter = LegacyUsersFilter.new({ two_step_status: "true" }) + + assert_equal({ two_step_statuses: [User::TWO_STEP_STATUS_ENABLED] }, filter.options) + end + + should "transform two_step_status legacy filter -> two_step_statuses array when value is 'false'" do + filter = LegacyUsersFilter.new({ two_step_status: "false" }) + + assert_equal({ two_step_statuses: [User::TWO_STEP_STATUS_NOT_SET_UP] }, filter.options) + end + + should "transform two_step_status legacy filter -> two_step_statuses array when value is 'exempt'" do + filter = LegacyUsersFilter.new({ two_step_status: "exempt" }) + + assert_equal({ two_step_statuses: [User::TWO_STEP_STATUS_EXEMPTED] }, filter.options) + end + + should "transform role legacy filter -> roles array" do + filter = LegacyUsersFilter.new({ role: Roles::Admin.role_name }) + + assert_equal({ roles: [Roles::Admin.role_name] }, filter.options) + end + + should "transform organisation legacy filter -> organisations array" do + organisation = create(:organisation) + filter = LegacyUsersFilter.new({ organisation: organisation.to_param }) + + assert_equal({ organisations: [organisation.to_param] }, filter.options) + end + + should "transform permission legacy filter -> permissions array" do + permission = create(:supported_permission) + filter = LegacyUsersFilter.new({ permission: permission.to_param }) + + assert_equal({ permissions: [permission.to_param] }, filter.options) + end + end +end diff --git a/test/models/user_test.rb b/test/models/user_test.rb index 8670684f3..710a53a75 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -229,37 +229,6 @@ def setup assert_equal [never_signed_in_ages_ago], User.expired_never_signed_in end - context ".with_role" do - setup do - @admin = create(:admin_user) - @normal = create(:user) - end - - should "return users with specified role" do - assert_includes User.with_role(:admin), @admin - end - - should "not return users with a role other than the specified role" do - assert_not_includes User.with_role(:admin), @normal - end - end - - context ".with_permission" do - should "only return users with the specified permission" do - joe = create(:user) - amy = create(:user) - app = create(:application, with_supported_permissions: %w[manage]) - signin_perm = app.signin_permission # created in a callback in the Application model - manage_perm = app.supported_permissions.find_by(name: "manage") - create(:user_application_permission, user: joe, application: app, supported_permission: signin_perm) - create(:user_application_permission, user: amy, application: app, supported_permission: signin_perm) - create(:user_application_permission, user: amy, application: app, supported_permission: manage_perm) - - assert_equal User.with_permission(signin_perm.id), [joe, amy] - assert_equal User.with_permission(manage_perm.id), [amy] - end - end - context ".not_recently_unsuspended" do should "return users who have never been unsuspended" do assert_includes User.not_recently_unsuspended, @user @@ -280,28 +249,9 @@ def setup end end - context ".with_2sv_enabled" do - should "return users with 2SV enabled when true" do - enabled_user = create(:two_step_enabled_user) - exempt_user = create(:two_step_exempted_user) - - enabled_users = User.with_2sv_enabled("true") - assert_equal 1, enabled_users.count - assert_equal enabled_user, enabled_users.first - - disabled_users = User.with_2sv_enabled("false") - assert_equal 1, disabled_users.count - assert_equal @user, disabled_users.first - - exempt_users = User.with_2sv_enabled("exempt") - assert_equal 1, exempt_users.count - assert_equal exempt_user, exempt_users.first - end - - should "encrypt otp_secret_key" do - enabled_user = create(:two_step_enabled_user) - assert enabled_user.encrypted_attribute?(:otp_secret_key) - end + should "encrypt otp_secret_key" do + enabled_user = create(:two_step_enabled_user) + assert enabled_user.encrypted_attribute?(:otp_secret_key) end context "email validation" do @@ -645,30 +595,6 @@ def setup @invited = User.invite!(name: "Oberyn Martell", email: "redviper@dorne.com") end - context "filtering" do - should "be able to filter by all statuses" do - User::USER_STATUSES.each do |status| - assert_not_empty User.with_status(status) - end - end - - should "filter suspended" do - assert_equal [@suspended], User.with_status(User::USER_STATUS_SUSPENDED).all - end - - should "filter invited" do - assert_equal [@invited], User.with_status(User::USER_STATUS_INVITED).all - end - - should "filter locked" do - assert_equal [@locked], User.with_status(User::USER_STATUS_LOCKED).all - end - - should "filter active" do - assert_equal [@user], User.with_status(User::USER_STATUS_ACTIVE).all - end - end - context "detecting" do should "detect suspended" do assert_equal "suspended", @suspended.status @@ -715,6 +641,91 @@ def setup end end + context "#two_step_status" do + should "return 'enabled' when user has 2SV" do + user = build(:two_step_enabled_user) + assert_equal User::TWO_STEP_STATUS_ENABLED, user.two_step_status + end + + should "return 'exempt' when user has been exempted from 2SV" do + user = build(:two_step_exempted_user) + assert_equal User::TWO_STEP_STATUS_EXEMPTED, user.two_step_status + end + + should "return 'not_set_up' when user does not have 2SV and has not been exempted" do + user = build(:user) + assert_equal User::TWO_STEP_STATUS_NOT_SET_UP, user.two_step_status + end + end + + context "#role_class" do + should "return the role class" do + assert_equal Roles::Normal, build(:user).role_class + assert_equal Roles::OrganisationAdmin, build(:organisation_admin_user).role_class + assert_equal Roles::SuperOrganisationAdmin, build(:super_organisation_admin_user).role_class + assert_equal Roles::Admin, build(:admin_user).role_class + assert_equal Roles::Superadmin, build(:superadmin_user).role_class + end + end + + context "#manageable_roles" do + should "return names of roles that the user is allowed to manage" do + assert_equal %w[], build(:user).manageable_roles + assert_equal %w[normal organisation_admin], build(:organisation_admin_user).manageable_roles + assert_equal %w[normal organisation_admin super_organisation_admin], build(:super_organisation_admin_user).manageable_roles + assert_equal %w[normal organisation_admin super_organisation_admin admin], build(:admin_user).manageable_roles + assert_equal User.roles, build(:superadmin_user).manageable_roles + end + end + + context "#can_manage?" do + should "indicate whether user is allowed to manage another user" do + assert_not build(:user).can_manage?(build(:user)) + assert_not build(:user).can_manage?(build(:organisation_admin_user)) + assert_not build(:user).can_manage?(build(:super_organisation_admin_user)) + assert_not build(:user).can_manage?(build(:admin_user)) + assert_not build(:user).can_manage?(build(:superadmin_user)) + + assert build(:organisation_admin_user).can_manage?(build(:user)) + assert build(:organisation_admin_user).can_manage?(build(:organisation_admin_user)) + assert_not build(:organisation_admin_user).can_manage?(build(:super_organisation_admin_user)) + assert_not build(:organisation_admin_user).can_manage?(build(:admin_user)) + assert_not build(:organisation_admin_user).can_manage?(build(:superadmin_user)) + + assert build(:super_organisation_admin_user).can_manage?(build(:user)) + assert build(:super_organisation_admin_user).can_manage?(build(:organisation_admin_user)) + assert build(:super_organisation_admin_user).can_manage?(build(:super_organisation_admin_user)) + assert_not build(:super_organisation_admin_user).can_manage?(build(:admin_user)) + assert_not build(:super_organisation_admin_user).can_manage?(build(:superadmin_user)) + + assert build(:admin_user).can_manage?(build(:user)) + assert build(:admin_user).can_manage?(build(:organisation_admin_user)) + assert build(:admin_user).can_manage?(build(:super_organisation_admin_user)) + assert build(:admin_user).can_manage?(build(:admin_user)) + assert_not build(:admin_user).can_manage?(build(:superadmin_user)) + + assert build(:superadmin_user).can_manage?(build(:user)) + assert build(:superadmin_user).can_manage?(build(:organisation_admin_user)) + assert build(:superadmin_user).can_manage?(build(:super_organisation_admin_user)) + assert build(:superadmin_user).can_manage?(build(:admin_user)) + assert build(:superadmin_user).can_manage?(build(:superadmin_user)) + end + end + + context "#manageable_organisations" do + should "return relation for organisations that the user is allowed to manage" do + organisation = create(:organisation, name: "Org1") + child_organisation = create(:organisation, parent: organisation, name: "Org2") + create(:organisation, name: "Org3") + + assert_equal [], create(:user, organisation:).manageable_organisations + assert_equal [organisation], create(:organisation_admin_user, organisation:).manageable_organisations + assert_equal [organisation, child_organisation], create(:super_organisation_admin_user, organisation:).manageable_organisations + assert_equal Organisation.order(:name), create(:admin_user, organisation:).manageable_organisations + assert_equal Organisation.order(:name), create(:superadmin_user, organisation:).manageable_organisations + end + end + context "authorised applications" do setup do @user = create(:user) @@ -772,6 +783,258 @@ def setup end end + context ".suspended" do + should "only return suspended users" do + suspended_user = create(:suspended_user) + + assert_equal [suspended_user], User.suspended + end + end + + context ".not_suspended" do + should "only return users that have not been suspended" do + create(:suspended_user) + active_user = create(:active_user) + + assert_equal [@user, active_user], User.not_suspended + end + end + + context ".invited" do + should "only return users that have been invited but have not accepted" do + invited_user = create(:invited_user) + create(:active_user) + + assert_equal [invited_user], User.invited + end + end + + context ".not_invited" do + should "only return users that have not been invited or have accepted" do + create(:invited_user) + active_user = create(:active_user) + + assert_equal [@user, active_user], User.not_invited + end + end + + context ".locked" do + should "only return users that have been locked" do + locked_user = create(:locked_user) + create(:active_user) + + assert_equal [locked_user], User.locked + end + end + + context ".not_locked" do + should "only return users that have not been locked" do + create(:locked_user) + active_user = create(:active_user) + + assert_equal [@user, active_user], User.not_locked + end + end + + context ".active" do + should "only return users that are considered active" do + create(:invited_user) + create(:locked_user) + create(:suspended_user) + active_user = create(:active_user) + + assert_equal [@user, active_user], User.active + end + end + + context ".exempt_from_2sv" do + should "only return users that have been exempted from 2SV" do + exempted_user = create(:two_step_exempted_user) + create(:two_step_enabled_user) + + assert_equal [exempted_user], User.exempt_from_2sv + end + end + + context ".not_exempt_from_2sv" do + should "only return users that have not been exempted from 2SV" do + create(:two_step_exempted_user) + enabled_user = create(:two_step_enabled_user) + + assert_equal [@user, enabled_user], User.not_exempt_from_2sv + end + end + + context ".has_2sv" do + should "only return users that have 2SV" do + create(:two_step_exempted_user) + enabled_user = create(:two_step_enabled_user) + + assert_equal [enabled_user], User.has_2sv + end + end + + context ".does_not_have_2sv" do + should "only return users that do not have 2SV" do + exempted_user = create(:two_step_exempted_user) + create(:two_step_enabled_user) + + assert_equal [@user, exempted_user], User.does_not_have_2sv + end + end + + context ".not_setup_2sv" do + should "only return users that have not been exempted from 2SV and do not have 2SV" do + not_set_up_user = create(:user) + create(:two_step_exempted_user) + create(:two_step_enabled_user) + + assert_equal [@user, not_set_up_user], User.not_setup_2sv + end + end + + context ".with_partially_matching_name" do + should "only return users with a name that includes the value" do + user1 = create(:user, name: "does-match1") + user2 = create(:user, name: "does-match2") + create(:user, name: "does-not-match") + + assert_equal [user1, user2], User.with_partially_matching_name("does-match") + end + end + + context ".with_partially_matching_email" do + should "only return users with an email that includes the value" do + user1 = create(:user, email: "does-match1@example.com") + user2 = create(:user, email: "does-match2@example.com") + create(:user, email: "does-not-match@example.com") + + assert_equal [user1, user2], User.with_partially_matching_email("does-match") + end + end + + context ".with_partially_matching_name_or_email" do + should "only return users with a name OR email that includes the value" do + user1 = create(:user, name: "does-match", email: "does-not-match@example.com") + user2 = create(:user, name: "does-not-match", email: "does-match@example.com") + create(:user, name: "does-not-match", email: "does-not-match-either@example.com") + + assert_equal [user1, user2], User.with_partially_matching_name_or_email("does-match") + end + end + + context ".with_statuses" do + should "only return suspended or invited users" do + suspended_user = create(:suspended_user) + invited_user = create(:invited_user) + create(:locked_user) + create(:active_user) + + assert_equal [suspended_user, invited_user], User.with_statuses(%w[suspended invited]) + end + + should "only return active or locked users" do + create(:suspended_user) + create(:invited_user) + locked_user = create(:locked_user) + active_user = create(:active_user) + + assert_equal [@user, locked_user, active_user], User.with_statuses(%w[active locked]) + end + + should "return all users if no statuses specified" do + suspended_user = create(:suspended_user) + invited_user = create(:invited_user) + locked_user = create(:locked_user) + active_user = create(:active_user) + + assert_equal [@user, suspended_user, invited_user, locked_user, active_user], User.with_statuses([]) + end + + should "ignore any non-existent statuses" do + suspended_user = create(:suspended_user) + invited_user = create(:invited_user) + locked_user = create(:locked_user) + active_user = create(:active_user) + + assert_equal [@user, suspended_user, invited_user, locked_user, active_user], User.with_statuses(%w[non-existent]) + end + end + + context ".with_2sv_statuses" do + should "only return not_set_up and exempted users" do + not_set_up_user = create(:user) + exempted_user = create(:two_step_exempted_user) + create(:two_step_enabled_user) + + assert_equal [@user, not_set_up_user, exempted_user], User.with_2sv_statuses(%w[not_setup_2sv exempt_from_2sv]) + end + + should "only return enabled users" do + create(:user) + create(:two_step_exempted_user) + enabled_user = create(:two_step_enabled_user) + + assert_equal [enabled_user], User.with_2sv_statuses(%w[has_2sv]) + end + + should "return all users if no statuses specified" do + not_set_up_user = create(:user) + exempted_user = create(:two_step_exempted_user) + enabled_user = create(:two_step_enabled_user) + + assert_equal [@user, not_set_up_user, exempted_user, enabled_user], User.with_2sv_statuses([]) + end + + should "ignore any non-existent statuses" do + not_set_up_user = create(:user) + exempted_user = create(:two_step_exempted_user) + enabled_user = create(:two_step_enabled_user) + + assert_equal [@user, not_set_up_user, exempted_user, enabled_user], User.with_2sv_statuses(%w[non-existent]) + end + end + + context ".with_role" do + should "only return users with specified role(s)" do + admin_user = create(:admin_user) + organisation_admin = create(:organisation_admin_user) + create(:user) + + assert_equal [admin_user, organisation_admin], User.with_role(%w[admin organisation_admin]) + end + end + + context ".with_permission" do + should "only return users with specified permission(s)" do + app1 = create(:application) + app2 = create(:application) + + permission1 = create(:supported_permission, application: app1) + + user1 = create(:user, supported_permissions: [app1.signin_permission, permission1]) + create(:user, supported_permissions: []) + user2 = create(:user, supported_permissions: [app2.signin_permission, permission1]) + + specified_permissions = [app1.signin_permission, app2.signin_permission] + assert_equal [user1, user2], User.with_permission(specified_permissions.map(&:to_param)) + end + end + + context ".with_organisation" do + should "only return users with specified organisation(s)" do + org1 = create(:organisation) + org2 = create(:organisation) + org3 = create(:organisation) + + user_in_org1 = create(:user, organisation: org1) + create(:user, organisation: org2) + user_in_org3 = create(:user, organisation: org3) + + assert_equal [user_in_org1, user_in_org3], User.with_organisation([org1, org3].map(&:to_param)) + end + end + def authenticate_access(user, app) ::Doorkeeper::AccessToken.create!(resource_owner_id: user.id, application_id: app.id) end diff --git a/test/models/users_filter_test.rb b/test/models/users_filter_test.rb new file mode 100644 index 000000000..55b1c05b5 --- /dev/null +++ b/test/models/users_filter_test.rb @@ -0,0 +1,375 @@ +require "test_helper" + +class UsersFilterTest < ActiveSupport::TestCase + setup do + @current_user = User.new + end + + context ".with_checked_at_top" do + should "put all checked options before all unchecked options" do + options = [ + { label: "A", checked: false }, + { label: "B", checked: true }, + { label: "C", checked: false }, + { label: "D", checked: true }, + ] + + expected_options = [ + { label: "B", checked: true }, + { label: "D", checked: true }, + { label: "A", checked: false }, + { label: "C", checked: false }, + ] + + assert_equal expected_options, UsersFilter.with_checked_at_top(options) + end + end + + should "return all users in alphabetical name order" do + create(:user, name: "beta") + create(:user, name: "alpha") + + filter = UsersFilter.new(User.all, @current_user) + + assert_equal %w[alpha beta], filter.users.map(&:name) + end + + should "return pages of users" do + 3.times { create(:user) } + + filter = UsersFilter.new(User.all, @current_user, page: 1, per_page: 2) + assert_equal 2, filter.paginated_users.count + + filter = UsersFilter.new(User.all, @current_user, page: 2, per_page: 2) + assert_equal 1, filter.paginated_users.count + end + + context "when filtering by name or email" do + should "return users with partially matching name or email" do + create(:user, name: "does-not-match") + create(:user, name: "does-match") + + filter = UsersFilter.new(User.all, @current_user, filter: "does-match") + + assert_equal %w[does-match], filter.users.map(&:name) + end + + should "ignore leading/trailing spaces in search term" do + create(:user, name: "does-not-match") + create(:user, name: "does-match") + + filter = UsersFilter.new(User.all, @current_user, filter: " does-match ") + + assert_equal %w[does-match], filter.users.map(&:name) + end + end + + context "when filtering by status" do + should "return users matching any of the specified statuses" do + create(:suspended_user, name: "suspended-user") + create(:invited_user, name: "invited-user") + create(:locked_user, name: "locked-user") + create(:active_user, name: "active-user") + + filter = UsersFilter.new(User.all, @current_user, statuses: %w[suspended locked]) + + assert_equal %w[locked-user suspended-user], filter.users.map(&:name) + end + end + + context "#status_option_select_options" do + context "when no statuses are selected" do + should "return options for statuses with none checked" do + filter = UsersFilter.new(User.all, @current_user, {}) + options = filter.status_option_select_options + + expected_options = [ + { label: "Suspended", value: "suspended", checked: false }, + { label: "Invited", value: "invited", checked: false }, + { label: "Locked", value: "locked", checked: false }, + { label: "Active", value: "active", checked: false }, + ] + assert_equal expected_options, options + end + end + + context "when some statuses are selected" do + should "return options for statuses with relevant options checked" do + filter = UsersFilter.new(User.all, @current_user, statuses: %w[invited active]) + options = filter.status_option_select_options + + expected_options = [ + { label: "Suspended", value: "suspended", checked: false }, + { label: "Invited", value: "invited", checked: true }, + { label: "Locked", value: "locked", checked: false }, + { label: "Active", value: "active", checked: true }, + ] + assert_equal expected_options, options + end + end + end + + context "when filtering by 2SV status" do + should "return users matching any of the specified 2SV statuses" do + create(:user, name: "not-set-up-user") + create(:two_step_exempted_user, name: "exempted-user") + create(:two_step_enabled_user, name: "enabled-user") + + filter = UsersFilter.new(User.all, @current_user, two_step_statuses: %w[not_setup_2sv exempt_from_2sv]) + + assert_equal %w[exempted-user not-set-up-user], filter.users.map(&:name) + end + end + + context "#two_step_status_option_select_options" do + context "when no 2SV statuses are selected" do + should "return options for 2SV statuses with none checked" do + filter = UsersFilter.new(User.all, @current_user, {}) + options = filter.two_step_status_option_select_options + + expected_options = [ + { label: "Enabled", value: "has_2sv", checked: false }, + { label: "Not set up", value: "not_setup_2sv", checked: false }, + { label: "Exempted", value: "exempt_from_2sv", checked: false }, + ] + assert_equal expected_options, options + end + end + + context "when some 2SV statuses are selected" do + should "return options for statuses with relevant options checked" do + filter = UsersFilter.new(User.all, @current_user, two_step_statuses: %w[not_setup_2sv exempt_from_2sv]) + options = filter.two_step_status_option_select_options + + expected_options = [ + { label: "Enabled", value: "has_2sv", checked: false }, + { label: "Not set up", value: "not_setup_2sv", checked: true }, + { label: "Exempted", value: "exempt_from_2sv", checked: true }, + ] + assert_equal expected_options, options + end + end + end + + context "when filtering by role" do + should "return users matching any of the specified roles" do + create(:admin_user, name: "admin-user") + create(:organisation_admin_user, name: "organisation-admin-user") + create(:user, name: "normal-user") + + filter = UsersFilter.new(User.all, @current_user, roles: %w[admin normal]) + + assert_equal %w[admin-user normal-user], filter.users.map(&:name) + end + end + + context "#role_option_select_options" do + context "when no roles are selected" do + should "return options for roles manageable by the current user with none checked" do + @current_user.stubs(:manageable_roles).returns(%w[normal organisation_admin]) + + filter = UsersFilter.new(User.all, @current_user, {}) + options = filter.role_option_select_options + + expected_options = [ + { label: "Normal", value: "normal", checked: false }, + { label: "Organisation admin", value: "organisation_admin", checked: false }, + ] + assert_equal expected_options, options + end + end + + context "when some roles are selected" do + should "return options for roles manageable by the current user with relevant options checked" do + @current_user.stubs(:manageable_roles).returns(%w[normal organisation_admin super_organisation_admin]) + + filter = UsersFilter.new(User.all, @current_user, roles: %w[normal super_organisation_admin]) + options = filter.role_option_select_options + + expected_options = [ + { label: "Normal", value: "normal", checked: true }, + { label: "Organisation admin", value: "organisation_admin", checked: false }, + { label: "Super organisation admin", value: "super_organisation_admin", checked: true }, + ] + assert_equal expected_options, options + end + end + end + + context "when filtering by permission" do + should "return users matching any of the specified permissions" do + app1 = create(:application, name: "App 1") + app2 = create(:application, name: "App 2") + + permission1 = create(:supported_permission, application: app1, name: "Permission 1") + + create(:user, name: "user1", supported_permissions: [app1.signin_permission, permission1]) + create(:user, name: "user2", supported_permissions: []) + create(:user, name: "user3", supported_permissions: [app2.signin_permission, permission1]) + + filter = UsersFilter.new(User.all, @current_user, permissions: [permission1].map(&:to_param)) + + assert_equal %w[user1 user3], filter.users.map(&:name) + end + end + + context "#permission_option_select_options" do + setup do + @app1 = create(:application, name: "App 1") + @app2 = create(:application, name: "App 2") + + @permission1 = create(:supported_permission, application: @app1, name: "Permission 1") + end + + context "when no permissions are selected" do + should "return options for application permissions in alphabetical order with none checked" do + filter = UsersFilter.new(User.all, @current_user, {}) + options = filter.permission_option_select_options + + expected_options = [ + { label: "App 1 Permission 1", value: @permission1.to_param, checked: false }, + { label: "App 1 signin", value: @app1.signin_permission.to_param, checked: false }, + { label: "App 2 signin", value: @app2.signin_permission.to_param, checked: false }, + ] + + assert_equal expected_options, options + end + end + + context "when some permissions are selected" do + should "return options for application permissions with relevant options checked" do + selected_permissions = [@app2.signin_permission, @permission1] + filter = UsersFilter.new(User.all, @current_user, permissions: selected_permissions.map(&:to_param)) + options = filter.permission_option_select_options + + expected_options = [ + { label: "App 1 Permission 1", value: @permission1.to_param, checked: true }, + { label: "App 1 signin", value: @app1.signin_permission.to_param, checked: false }, + { label: "App 2 signin", value: @app2.signin_permission.to_param, checked: true }, + ] + + assert_equal expected_options, options + end + end + end + + context "when filtering by organisation" do + should "return users matching any of the specified organisations" do + organisation1 = create(:organisation, name: "Organisation 1") + organisation2 = create(:organisation, name: "Organisation 2") + organisation3 = create(:organisation, name: "Organisation 3") + + create(:user, name: "user1-in-organisation1", organisation: organisation1) + create(:user, name: "user2-in-organisation1", organisation: organisation1) + create(:user, name: "user3-in-organisation2", organisation: organisation2) + create(:user, name: "user4-in-organisation3", organisation: organisation3) + + filter = UsersFilter.new(User.all, @current_user, organisations: [organisation1, organisation3].map(&:to_param)) + + assert_equal %w[user1-in-organisation1 user2-in-organisation1 user4-in-organisation3], filter.users.map(&:name) + end + end + + context "#organisation_option_select_options" do + context "when current user is an admin" do + setup do + @organisation = create(:organisation, name: "Org1") + @current_user = create(:admin_user, organisation: @organisation) + end + + should "return select options for organisations that have any users" do + another_organisation = create(:organisation, name: "Org2") + create(:user, organisation: another_organisation) + + filter = UsersFilter.new(User.all, @current_user, {}) + options = filter.organisation_option_select_options + + expected_options = [ + { label: @organisation.name, value: @organisation.to_param, checked: false }, + { label: another_organisation.name, value: another_organisation.to_param, checked: false }, + ] + assert_equal expected_options, options + end + + should "return select options with `selected` set appropriately" do + another_organisation = create(:organisation, name: "Org2") + create(:user, organisation: another_organisation) + + filter = UsersFilter.new(User.all, @current_user, organisations: [another_organisation.to_param]) + options = filter.organisation_option_select_options + + expected_options = [ + { label: @organisation.name, value: @organisation.to_param, checked: false }, + { label: another_organisation.name, value: another_organisation.to_param, checked: true }, + ] + assert_equal expected_options, options + end + end + + context "when current user is a super organisation admin" do + setup do + @organisation = create(:organisation, name: "Org1") + @current_user = create(:super_organisation_admin_user, organisation: @organisation) + end + + should "return select options for organisation and sub-organisations that have any users" do + sub_organisation = create(:organisation, parent: @organisation, name: "Org2") + create(:organisation, parent: @organisation, name: "Org3") + create(:user, organisation: sub_organisation) + + filter = UsersFilter.new(User.all, @current_user, {}) + options = filter.organisation_option_select_options + + expected_options = [ + { label: @organisation.name, value: @organisation.to_param, checked: false }, + { label: sub_organisation.name, value: sub_organisation.to_param, checked: false }, + ] + assert_equal expected_options, options + end + end + + context "when current user is an organisation admin" do + setup do + @organisation = create(:organisation, name: "Org1") + @current_user = create(:organisation_admin_user, organisation: @organisation) + end + + should "return select options for only the user's organisation" do + another_organisation = create(:organisation, name: "Org2") + create(:user, organisation: another_organisation) + + filter = UsersFilter.new(User.all, @current_user, {}) + options = filter.organisation_option_select_options + + expected_options = [ + { label: @organisation.name, value: @organisation.to_param, checked: false }, + ] + assert_equal expected_options, options + end + end + end + + context "#no_options_selected_for?" do + should "return true when there are no options selected for the specified key" do + filter = UsersFilter.new(User.all, @current_user, {}) + assert filter.no_options_selected_for?(:statuses) + end + + should "return false when there is at least one option selected for the specified key" do + filter = UsersFilter.new(User.all, @current_user, { statuses: %w[active] }) + assert_not filter.no_options_selected_for?(:statuses) + end + end + + context "#any_options_selected?" do + should "return true when this is at least one checkbox filter option selected" do + filter = UsersFilter.new(User.all, @current_user, { statuses: %w[active] }) + assert filter.any_options_selected? + end + + should "return false when there are no checkbox filter options selected" do + filter = UsersFilter.new(User.all, @current_user, { filter: "Jill", page: 1, per_page: 25 }) + assert_not filter.any_options_selected? + end + end +end