From 76c7932b0912a4f1e42f7fef688300c277043b8c Mon Sep 17 00:00:00 2001 From: Suzanne Aitchison Date: Thu, 14 Jul 2022 14:52:42 +0100 Subject: [PATCH] Member index: Filter by joining date (#17975) * add the date picker into view * allow date picker to overflow modal * allow aria labels to be passed to inputs * filter records * add missing comment * set earliest date * specs and a small fix * cypress spec * fix typo * tidying up * fetch date_format * commit code review suggestion to params * Revert "commit code review suggestion to params" This reverts commit 84fb0e0d3acc13257a362eb41c17b2bb8089606a. * show added filter pill * add cypress specs for applied filters * woops! fix unintended change to filter code --- app/assets/stylesheets/components/modals.scss | 7 ++- app/controllers/admin/users_controller.rb | 4 ++ app/javascript/crayons/Modal/Modal.jsx | 2 + .../DateRangePicker/DateRangePicker.jsx | 12 +++- .../__stories__/DateRangePicker.stories.jsx | 10 +++ .../DateRangePicker/dateRangeUtils.js | 9 +++ .../packs/admin/users/filtersModal.js | 63 +++++++++++++++++++ .../packs/admin/users/memberIndex.js | 29 ++++++--- app/queries/admin/users_query.rb | 32 +++++++++- .../users/index/_applied_filters.html.erb | 14 ++++- .../admin/users/index/_filters_modal.html.erb | 14 ++++- .../adminFlows/users/filterUserIndex.spec.js | 52 +++++++++++++++ spec/queries/admin/users_query_spec.rb | 59 ++++++++++++++--- spec/support/seeds/seeds_e2e.rb | 3 +- 14 files changed, 285 insertions(+), 25 deletions(-) diff --git a/app/assets/stylesheets/components/modals.scss b/app/assets/stylesheets/components/modals.scss index 507d345e7..c1d00f9d2 100644 --- a/app/assets/stylesheets/components/modals.scss +++ b/app/assets/stylesheets/components/modals.scss @@ -10,6 +10,7 @@ var(--modal-padding); --modal-alignment: unset; --modal-header-alignment: space-between; + --modal-overflow: hidden; @media (min-width: $breakpoint-s) { --modal-padding: var(--su-4); @@ -80,7 +81,7 @@ max-height: 100%; width: 100%; height: 100%; - overflow: hidden; + overflow: var(--modal-overflow); position: relative; pointer-events: auto; border-radius: var(--radius-large-auto); @@ -149,6 +150,10 @@ } } + &--overflow-visible { + --modal-overflow: visible; + } + &--prompt { @media (min-width: $breakpoint-s) { --modal-max-width: 480px; diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 0482bfff3..f0979f5b1 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -51,11 +51,15 @@ module Admin search: params[:search], role: params[:role], roles: params[:roles], + joining_start: params[:joining_start], + joining_end: params[:joining_end], + date_format: params[:date_format], organizations: params[:organizations], ).page(params[:page]).per(50) @organization_limit = 3 @organizations = Organization.order(name: :desc) + @earliest_join_date = User.first.registered_at.to_s end def edit diff --git a/app/javascript/crayons/Modal/Modal.jsx b/app/javascript/crayons/Modal/Modal.jsx index 9da5e2145..490141eee 100644 --- a/app/javascript/crayons/Modal/Modal.jsx +++ b/app/javascript/crayons/Modal/Modal.jsx @@ -18,6 +18,7 @@ export const Modal = ({ showHeader = true, sheetAlign = 'center', backdropDismissible = false, + allowOverflow = false, onClose = () => {}, focusTrapSelector = '.crayons-modal__box', document = window.document, @@ -29,6 +30,7 @@ export const Modal = ({ 'crayons-modal--prompt': prompt, 'crayons-modal--centered': centered && prompt, 'crayons-modal--bg-dismissible': !noBackdrop && backdropDismissible, + 'crayons-modal--overflow-visible': allowOverflow, [className]: className, }); diff --git a/app/javascript/crayons/formElements/DateRangePicker/DateRangePicker.jsx b/app/javascript/crayons/formElements/DateRangePicker/DateRangePicker.jsx index 1d64e81fd..f92fa4238 100644 --- a/app/javascript/crayons/formElements/DateRangePicker/DateRangePicker.jsx +++ b/app/javascript/crayons/formElements/DateRangePicker/DateRangePicker.jsx @@ -251,6 +251,8 @@ const useDateRangeValidation = ({ * @param {Date} props.minStartDate The oldest date that may be selected * @param {Function} props.onDatesChanged Callback function for when dates are selected. Receives an object with startDate and endDate values. * @param {[string]} props.presetRanges Quick-select preset date ranges to offer in the calendar. These will only be shown if they fall within the min and max dates. + * @param {string} props.startDateAriaLabel Custom aria-label for start date input + * @param {string} props.endDateAriaLabel Custom aria-label for end date input * @param {Date} props.todaysDate Optional param to pass in today's Date, primarily for testing purposes */ export const DateRangePicker = ({ @@ -262,6 +264,8 @@ export const DateRangePicker = ({ minStartDate = new Date(), onDatesChanged, presetRanges = [], + startDateAriaLabel, + endDateAriaLabel, todaysDate = new Date(), }) => { const [focusedInput, setFocusedInput] = useState(START_DATE); @@ -312,10 +316,12 @@ export const DateRangePicker = ({ )} /> + {/* This hidden input provides information that may be needed by implementing forms to properly parse dates */} +
{ @@ -82,6 +90,8 @@ DateRangePickerWithPresetRanges.args = { defaultEndDate: undefined, minStartDate: new Date(2020, 0, 1), maxEndDate: new Date(), + startDateAriaLabel: undefined, + endDateAriaLabel: undefined, presetRanges: [ MONTH_UNTIL_TODAY, LAST_FULL_MONTH, diff --git a/app/javascript/crayons/formElements/DateRangePicker/dateRangeUtils.js b/app/javascript/crayons/formElements/DateRangePicker/dateRangeUtils.js index 3f4628bfb..1660738b0 100644 --- a/app/javascript/crayons/formElements/DateRangePicker/dateRangeUtils.js +++ b/app/javascript/crayons/formElements/DateRangePicker/dateRangeUtils.js @@ -5,6 +5,15 @@ export const LAST_FULL_MONTH = 'LAST_FULL_MONTH'; export const LAST_FULL_QUARTER = 'LAST_FULL_QUARTER'; export const LAST_FULL_YEAR = 'LAST_FULL_YEAR'; +export const ALL_PRESET_RANGES = [ + MONTH_UNTIL_TODAY, + LAST_FULL_MONTH, + QUARTER_UNTIL_TODAY, + LAST_FULL_QUARTER, + YEAR_UNTIL_TODAY, + LAST_FULL_YEAR, +]; + export const RANGE_LABELS = { MONTH_UNTIL_TODAY: 'This month', QUARTER_UNTIL_TODAY: 'This quarter', diff --git a/app/javascript/packs/admin/users/filtersModal.js b/app/javascript/packs/admin/users/filtersModal.js index 40ade49fc..81af97289 100644 --- a/app/javascript/packs/admin/users/filtersModal.js +++ b/app/javascript/packs/admin/users/filtersModal.js @@ -4,6 +4,67 @@ import { WINDOW_MODAL_ID, } from '@utilities/showModal'; +const updateDateRangeFilterIndicator = ({ startDate, endDate, indicator }) => { + const hasValues = startDate || endDate; + if (hasValues) { + indicator.classList.remove('hidden'); + } else { + indicator.classList.add('hidden'); + } +}; + +/** + * Some sections require the Preact DateRangePicker. + * This function imports the required packages and adds the pickers to the modal. + */ +const initializeDateRangePickers = async () => { + const joiningRangeContainer = document.querySelector( + `#${WINDOW_MODAL_ID} .js-joining-date-range`, + ); + + const joiningDateIndicator = document.querySelector( + `#${WINDOW_MODAL_ID} .js-filtered-indicator-joining-date`, + ); + + const [ + { render, h }, + { DateRangePicker, ALL_PRESET_RANGES }, + { default: moment }, + ] = await Promise.all([ + import('preact'), + import('@crayons'), + import('moment'), + ]); + + const { defaultStart, defaultEnd, defaultDatesFormat, earliestDate } = + joiningRangeContainer.dataset; + + render( + + updateDateRangeFilterIndicator({ + ...dates, + indicator: joiningDateIndicator, + }) + } + defaultStartDate={ + defaultStart && moment(defaultStart, defaultDatesFormat).toDate() + } + defaultEndDate={ + defaultEnd && moment(defaultEnd, defaultDatesFormat).toDate() + } + minStartDate={new Date(earliestDate)} + maxEndDate={new Date()} + presetRanges={ALL_PRESET_RANGES} + />, + joiningRangeContainer, + ); +}; + /** * Details panels will automatically expand on click when required. * We want to make sure only _one_ panel is expanded at any given time, @@ -145,11 +206,13 @@ export const initializeFiltersModal = () => { sheet: true, sheetAlign: 'right', size: 'small', + allowOverflow: true, onOpen: () => { initializeModalCloseButton(); initializeFilterDetailsToggles(); initializeShowHideRoles(); initializeFilterClearButtons(); + initializeDateRangePickers(); }, }); }); diff --git a/app/javascript/packs/admin/users/memberIndex.js b/app/javascript/packs/admin/users/memberIndex.js index efa206de2..46ddff93f 100644 --- a/app/javascript/packs/admin/users/memberIndex.js +++ b/app/javascript/packs/admin/users/memberIndex.js @@ -34,16 +34,27 @@ const initializeFilterPills = () => { } = relevantElement; const urlParams = new URLSearchParams(document.location.search); - const allFiltersMatchingKey = urlParams.getAll(filterKey); - // Remove the clicked filter name from the array - allFiltersMatchingKey.splice(allFiltersMatchingKey.indexOf(filterValue), 1); - // Delete _all_ filters with given key - urlParams.delete(filterKey); - // Re-attach any filters with given key that we want to keep - allFiltersMatchingKey.forEach((filterVal) => - urlParams.append(filterKey, filterVal), - ); + if (filterKey === 'joining_date') { + urlParams.delete('joining_start'); + urlParams.delete('joining_end'); + } else { + // In all other cases, we may have filters with a matching key that we want to keep + const allFiltersMatchingKey = urlParams.getAll(filterKey); + + // Remove the clicked filter name from the array + allFiltersMatchingKey.splice( + allFiltersMatchingKey.indexOf(filterValue), + 1, + ); + // Delete _all_ filters with given key + urlParams.delete(filterKey); + // Re-attach any filters with given key that we want to keep + allFiltersMatchingKey.forEach((filterVal) => + urlParams.append(filterKey, filterVal), + ); + } + // Redirect the page document.location.search = urlParams; }); diff --git a/app/queries/admin/users_query.rb b/app/queries/admin/users_query.rb index c8aea9065..b8b9e45b1 100644 --- a/app/queries/admin/users_query.rb +++ b/app/queries/admin/users_query.rb @@ -10,7 +10,17 @@ module Admin # @param search [String, nil] # @param roles [Array, nil] # @param organizations [Array, nil] - def self.call(relation: User.registered, role: nil, search: nil, roles: [], organizations: []) + # @param joining_start [String, nil] + # @param joining_end [String, nil] + # @param date_format [String] + def self.call(relation: User.registered, + role: nil, + search: nil, + roles: [], + organizations: [], + joining_start: nil, + joining_end: nil, + date_format: "DD/MM/YYYY") # We are at an interstitial moment where we are exposing both the role and roles param. We # need to favor one or the other. if role.presence @@ -23,6 +33,11 @@ module Admin relation = filter_organization_memberships(relation: relation, organizations: organizations) end + if joining_start.presence || joining_end.presence + relation = filter_joining_date(relation: relation, joining_start: joining_start, joining_end: joining_end, + date_format: date_format) + end + relation = search_relation(relation, search) if search.presence relation.distinct.order(created_at: :desc) end @@ -31,6 +46,21 @@ module Admin relation.where(QUERY_CLAUSE, search: "%#{search.strip}%") end + def self.filter_joining_date(relation:, joining_start:, joining_end:, date_format:) + ui_formats_to_parse_format = { + "DD/MM/YYYY" => "%d/%m/%Y", + "MM/DD/YYYY" => "%m/%d/%Y" + } + parse_format = ui_formats_to_parse_format.fetch(date_format) + if joining_start.presence + relation = relation.where("registered_at >= ?", DateTime.strptime(joining_start, parse_format).beginning_of_day) + end + + return relation unless joining_end.presence + + relation.where("registered_at <= ?", DateTime.strptime(joining_end, parse_format).end_of_day) + end + def self.filter_organization_memberships(relation:, organizations:) sub_query = OrganizationMembership.select(:user_id).where(organization_id: organizations) relation.where(id: sub_query) diff --git a/app/views/admin/users/index/_applied_filters.html.erb b/app/views/admin/users/index/_applied_filters.html.erb index 3fa47e744..3a67a465d 100644 --- a/app/views/admin/users/index/_applied_filters.html.erb +++ b/app/views/admin/users/index/_applied_filters.html.erb @@ -23,7 +23,19 @@ <%= crayons_icon_tag("x.svg", class: "c-pill__action-icon", aria_hidden: true) %> <% end %> - <% if params[:roles] || params[:organizations] %> + <% if params[:joining_start]&.present? || params[:joining_end]&.present? %> + + <% end %> + <% if params[:roles] || params[:organizations] || params[:joining_start]&.present? || params[:joining_end]&.present? %> <% end %>
diff --git a/app/views/admin/users/index/_filters_modal.html.erb b/app/views/admin/users/index/_filters_modal.html.erb index 6b178f907..1d9e71a7e 100644 --- a/app/views/admin/users/index/_filters_modal.html.erb +++ b/app/views/admin/users/index/_filters_modal.html.erb @@ -43,8 +43,18 @@ Status options
- Joining date<%= crayons_icon_tag("chevron-down", aria_hidden: true, class: "summary-icon") %> - Joining date options + + + Joining date + "> + + + + <%= crayons_icon_tag("chevron-down", aria_hidden: true, class: "summary-icon") %> + +
+ <%# This hidden field is only here to reserve the correct amount of vertical space for where we later add the Preact inputs %> +
diff --git a/cypress/e2e/seededFlows/adminFlows/users/filterUserIndex.spec.js b/cypress/e2e/seededFlows/adminFlows/users/filterUserIndex.spec.js index e4403b5fb..44d3de44e 100644 --- a/cypress/e2e/seededFlows/adminFlows/users/filterUserIndex.spec.js +++ b/cypress/e2e/seededFlows/adminFlows/users/filterUserIndex.spec.js @@ -170,6 +170,58 @@ describe('Filter user index', () => { }); }); + describe('Joining date', () => { + it('filters by joining date', () => { + cy.findByRole('button', { name: 'Filter' }).click(); + cy.getModal().within(() => { + cy.findAllByText('Joining date').first().click(); + + // We need to use a partial name match here, because we can't force the Cypress browser locale to e.g. en-us, and we + // want to void flake caused by DD/MM/YYYY format vs MM/DD/YYYY + cy.findByRole('textbox', { name: /Joined after/ }).type('01/01/2020'); + cy.findByRole('textbox', { name: /Joined before/ }).type('01/01/2020'); + cy.findByRole('button', { name: 'Apply filters' }).click(); + }); + + // Admin user is deliberately seeded with very early registered date, and should be the only result + cy.findAllByRole('row').should('have.length', 2); + cy.findByRole('button', { + name: 'Remove filter: Joining date', + }).should('contain', '01/01/2020 - 01/01/2020'); + }); + + it('shows community creation in filter pill if no start date selected', () => { + cy.findByRole('button', { name: 'Filter' }).click(); + cy.getModal().within(() => { + cy.findAllByText('Joining date').first().click(); + + // We need to use a partial name match here, because we can't force the Cypress browser locale to e.g. en-us, and we + // want to void flake caused by DD/MM/YYYY format vs MM/DD/YYYY + cy.findByRole('textbox', { name: /Joined before/ }).type('01/01/2020'); + cy.findByRole('button', { name: 'Apply filters' }).click(); + }); + cy.findByRole('button', { + name: 'Remove filter: Joining date', + }).should('contain', 'Community creation - 01/01/2020'); + }); + + it('shows today in filter pill if no end date selected', () => { + cy.findByRole('button', { name: 'Filter' }).click(); + cy.getModal().within(() => { + cy.findAllByText('Joining date').first().click(); + + // We need to use a partial name match here, because we can't force the Cypress browser locale to e.g. en-us, and we + // want to void flake caused by DD/MM/YYYY format vs MM/DD/YYYY + cy.findByRole('textbox', { name: /Joined after/ }).type('01/01/2020'); + cy.findByRole('button', { name: 'Apply filters' }).click(); + }); + cy.findByRole('button', { name: 'Remove filter: Joining date' }).should( + 'contain', + '01/01/2020 - Today', + ); + }); + }); + describe('Multiple filters', () => { it('filters by multiple criteria', () => { cy.findAllByRole('button', { name: 'Filter' }).last().click(); diff --git a/spec/queries/admin/users_query_spec.rb b/spec/queries/admin/users_query_spec.rb index 860b6847d..70bd916ec 100644 --- a/spec/queries/admin/users_query_spec.rb +++ b/spec/queries/admin/users_query_spec.rb @@ -1,23 +1,33 @@ require "rails_helper" RSpec.describe Admin::UsersQuery, type: :query do - subject { described_class.call(search: search, role: role, roles: roles, organizations: organizations) } + subject do + described_class.call(search: search, role: role, roles: roles, organizations: organizations, + joining_start: joining_start, joining_end: joining_end, date_format: date_format) + end let(:role) { nil } let(:roles) { [] } let(:organizations) { [] } let(:search) { [] } + let(:joining_start) { nil } + let(:joining_end) { nil } + let(:date_format) { "DD/MM/YYYY" } let!(:org1) { create(:organization, name: "Org1") } let!(:org2) { create(:organization, name: "Org2") } - let!(:user) { create(:user, :trusted, name: "Greg") } - let!(:user2) { create(:user, :trusted, name: "Gregory") } - let!(:user3) { create(:user, :tag_moderator, name: "Paul") } - let!(:user4) { create(:user, :admin, name: "Susi") } - let!(:user5) { create(:user, :trusted, :admin, name: "Beth") } - let!(:user6) { create(:user, :super_admin, name: "Jean") } - let!(:user7) { create(:user).tap { |u| u.add_role(:single_resource_admin, DataUpdateScript) } } + let!(:user) { create(:user, :trusted, name: "Greg", registered_at: "2020-05-06T13:09:47+0000") } + let!(:user2) { create(:user, :trusted, name: "Gregory", registered_at: "2020-05-08T13:09:47+0000") } + let!(:user3) { create(:user, :tag_moderator, name: "Paul", registered_at: "2020-05-10T13:09:47+0000") } + let!(:user4) { create(:user, :admin, name: "Susi", registered_at: "2020-10-05T13:09:47+0000") } + let!(:user5) { create(:user, :trusted, :admin, name: "Beth", registered_at: "2020-10-07T13:09:47+0000") } + let!(:user6) { create(:user, :super_admin, name: "Jean", registered_at: "2020-10-08T13:09:47+0000") } + let!(:user7) do + create(:user, registered_at: "2020-12-05T13:09:47+0000").tap do |u| + u.add_role(:single_resource_admin, DataUpdateScript) + end + end describe ".call" do context "when no arguments are given" do @@ -91,5 +101,38 @@ RSpec.describe Admin::UsersQuery, type: :query do it { is_expected.to eq([user4]) } end + + context "when filtering by joining_start and default DD/MM/YYYY date format" do + let(:joining_start) { "01/12/2020" } + + it { is_expected.to eq([user7]) } + end + + context "when filtering by joining_start and alternative MM/DD/YYYY date format" do + let(:joining_start) { "12/01/2020" } + let(:date_format) { "MM/DD/YYYY" } + + it { is_expected.to eq([user7]) } + end + + context "when filtering by joining_end and default DD/MM/YYYY date format" do + let(:joining_end) { "07/05/2020" } + + it { is_expected.to eq([user]) } + end + + context "when filtering by joining_end and alternative MM/DD/YYYY date format" do + let(:joining_end) { "05/07/2020" } + let(:date_format) { "MM/DD/YYYY" } + + it { is_expected.to eq([user]) } + end + + context "when filtering by both joining_start and joining_end" do + let(:joining_start) { "01/05/2020" } + let(:joining_end) { "31/05/2020" } + + it { is_expected.to eq([user3, user2, user]) } + end end end diff --git a/spec/support/seeds/seeds_e2e.rb b/spec/support/seeds/seeds_e2e.rb index cf7459ccf..c9bd1a826 100644 --- a/spec/support/seeds/seeds_e2e.rb +++ b/spec/support/seeds/seeds_e2e.rb @@ -42,7 +42,8 @@ seeder.create_if_doesnt_exist(User, "email", "admin@forem.local") do username: "Admin_McAdmin", profile_image: File.open(Rails.root.join("app/assets/images/#{rand(1..40)}.png")), confirmed_at: Time.current, - registered_at: Time.current, + registered_at: "2020-01-01T13:09:47+0000", + created_at: "2020-01-01T13:09:47+0000", password: "password", password_confirmation: "password", saw_onboarding: true,