diff --git a/contentcuration/contentcuration/frontend/administration/pages/Users/UserTable.vue b/contentcuration/contentcuration/frontend/administration/pages/Users/UserTable.vue index c409ddd82b..6eee3a48e3 100644 --- a/contentcuration/contentcuration/frontend/administration/pages/Users/UserTable.vue +++ b/contentcuration/contentcuration/frontend/administration/pages/Users/UserTable.vue @@ -119,24 +119,23 @@ - - + @@ -208,8 +207,10 @@ import { ref, onMounted, computed, getCurrentInstance } from 'vue'; import { mapGetters } from 'vuex'; + import pick from 'lodash/pick'; import transform from 'lodash/transform'; import { saveAs } from 'file-saver'; + import { useRoute } from 'vue-router/composables'; import { useTable } from '../../composables/useTable'; import { RouteNames, rowsPerPageItems } from '../../constants'; import EmailUsersDialog from './EmailUsersDialog'; @@ -217,6 +218,7 @@ import client from 'shared/client'; import { useFilter } from 'shared/composables/useFilter'; import { useKeywordSearch } from 'shared/composables/useKeywordSearch'; + import { useQueryParams } from 'shared/composables/useQueryParams'; import { routerMixin } from 'shared/mixins'; import IconButton from 'shared/views/IconButton'; import Checkbox from 'shared/views/form/Checkbox'; @@ -230,6 +232,19 @@ sushichef: { label: 'Sushi chef', params: { chef: true } }, }; + const TABLE_STATE_QUERY_PARAMS = ['page', 'page_size', 'sortBy', 'descending']; + + // Mirrors the defaultValue each filter below declares. + const FILTER_DEFAULTS = { + userType: undefined, + location: undefined, + keywords: undefined, + joinedWithin: 'any', + activeWithin: 'any', + hasPublished: 'no', + hasEdits: 'no', + }; + const DATE_WINDOWS = [ { key: 'any', label: 'Any time', months: null }, { key: '1mo', label: 'Last month', months: 1 }, @@ -301,6 +316,8 @@ setup() { const { proxy } = getCurrentInstance(); const store = proxy.$store; + const route = useRoute(); + const { updateQueryParams } = useQueryParams(); const { filter: _userTypeFilter, @@ -368,7 +385,7 @@ const { filter: hasEditsFilter, fetchQueryParams: hasEditsFetchQueryParams } = useBooleanFilter({ name: 'hasEdits', - label: 'Has Studio edits', + label: 'Has Studio activity', paramName: 'has_edits', }); @@ -401,6 +418,16 @@ }; }); + const hasActiveFilters = computed(() => + Object.entries(FILTER_DEFAULTS).some( + ([name, defaultValue]) => (route.query[name] ?? defaultValue) !== defaultValue, + ), + ); + + function clearFilters() { + updateQueryParams(pick(route.query, TABLE_STATE_QUERY_PARAMS)); + } + function loadUsers(fetchParams) { return store.dispatch('userAdmin/loadUsers', fetchParams); } @@ -424,6 +451,8 @@ activeWithinOptions, hasPublishedFilter, hasEditsFilter, + hasActiveFilters, + clearFilters, pagination, loading, loadItems, @@ -525,4 +554,13 @@ - + diff --git a/contentcuration/contentcuration/frontend/administration/pages/Users/__tests__/userTable.spec.js b/contentcuration/contentcuration/frontend/administration/pages/Users/__tests__/userTable.spec.js index c116e78fe2..5d9ddd9d79 100644 --- a/contentcuration/contentcuration/frontend/administration/pages/Users/__tests__/userTable.spec.js +++ b/contentcuration/contentcuration/frontend/administration/pages/Users/__tests__/userTable.spec.js @@ -1,5 +1,6 @@ -import { mount, createLocalVue } from '@vue/test-utils'; -import Vuex, { Store } from 'vuex'; +import { render, screen, waitFor, within, configure } from '@testing-library/vue'; +import userEvent from '@testing-library/user-event'; +import { Store } from 'vuex'; import router from '../../../router'; import { RouteNames } from '../../../constants'; import UserTable from '../UserTable'; @@ -10,201 +11,332 @@ jest.mock('shared/client', () => ({ })); jest.mock('file-saver', () => ({ saveAs: jest.fn() })); -const localVue = createLocalVue(); +// Studio's IconButton passes `ariaLabel="text"` as a literal rather than binding it +// (shared/views/IconButton.vue), so icon buttons have no usable accessible name and +// have to be reached by their existing data-test hooks. +configure({ testIdAttribute: 'data-test' }); -localVue.use(Vuex); -localVue.use(router); +const USER_IDS = ['user-a', 'user-b', 'user-c']; +const ISO_DATE = /^\d{4}-\d{2}-\d{2}$/; -const userList = ['test', 'user', 'table']; +const mockLoadUsers = jest.fn(() => Promise.resolve({})); +const mockSendEmail = jest.fn(() => Promise.resolve()); -function makeWrapper(store) { - router.replace({ name: RouteNames.USERS }); - - const wrapper = mount(UserTable, { - router, - store, - localVue, - stubs: { - UserItem: true, - EmailUsersDialog: true, +function createStore({ users = USER_IDS } = {}) { + return new Store({ + modules: { + userAdmin: { + namespaced: true, + actions: { + loadUsers: mockLoadUsers, + sendEmail: mockSendEmail, + }, + getters: { + users: () => users, + count: () => users.length, + getUsers: () => ids => ids.map(id => ({ id, email: `${id}@test.com` })), + }, + }, }, }); +} - return wrapper; +function renderComponent({ users, query = {} } = {}) { + router.replace({ name: RouteNames.USERS, query }).catch(() => {}); + return render(UserTable, { + store: createStore({ users }), + routes: router, + stubs: { UserItem: true }, + }); } -describe('userTable', () => { - let wrapper, store; - const loadUsers = jest.fn(() => Promise.resolve({})); +/** + * Vuetify's VSelect menu does not open reliably under jsdom, so select-backed + * filters are reached through the URL the control would produce. A shared or + * bookmarked filter link is a real entry point, but it is navigation rather than + * a click — each test relying on it says so. + */ +const renderWithFilters = query => renderComponent({ query }); + +/** Payload of the most recent user fetch. */ +function lastFetchParams() { + const { calls } = mockLoadUsers.mock; + return calls[calls.length - 1][1]; +} + +/** + * KButton renders `appearance="basic-link"` as an anchor with no href, which has + * no implicit role, so the clear action is matched by its text. + */ +const clearFiltersLink = () => screen.queryByText('Clear filters'); + +/** The select-all checkbox lives in the table header, after the filter checkboxes. */ +const selectAllCheckbox = () => within(screen.getByRole('table')).getAllByRole('checkbox')[0]; + +describe('UserTable', () => { + let user; beforeEach(() => { - store = new Store({ - modules: { - userAdmin: { - namespaced: true, - actions: { - loadUsers, - }, - getters: { - users: () => userList, - count: () => userList.length, - }, - }, - }, + user = userEvent.setup(); + jest.clearAllMocks(); + require('shared/client').default.get.mockResolvedValue({ + data: new Blob(['col1,col2\n1,2'], { type: 'text/csv' }), }); - wrapper = makeWrapper(store); - }); - afterEach(() => { - loadUsers.mockRestore(); }); - describe('filters', () => { - it('changing user type filter should set query params', () => { - wrapper.vm.userTypeFilter = 'administrator'; - expect(router.currentRoute.query.userType).toBe('administrator'); + describe('filter controls', () => { + it('renders every filter control', () => { + renderComponent(); + + expect(screen.getByLabelText('User Type')).toBeInTheDocument(); + expect(screen.getByLabelText('Target location')).toBeInTheDocument(); + expect(screen.getByLabelText('Search for a user...')).toBeInTheDocument(); + expect(screen.getByLabelText('Joined within')).toBeInTheDocument(); + expect(screen.getByLabelText('Active within')).toBeInTheDocument(); + expect(screen.getByLabelText('Has published a channel')).toBeInTheDocument(); + expect(screen.getByLabelText('Has Studio activity')).toBeInTheDocument(); }); - it('changing location filter should set query params', () => { - wrapper.vm.locationFilter = 'Afghanistan'; - expect(router.currentRoute.query.location).toBe('Afghanistan'); + it('typing a search term fetches users filtered by keyword', async () => { + renderComponent(); + + await user.type(screen.getByLabelText('Search for a user...'), 'keyword test'); + + await waitFor(() => { + expect(lastFetchParams()).toMatchObject({ keywords: 'keyword test' }); + }); }); - it('changing search text should set query params', () => { - jest.useFakeTimers(); - wrapper.vm.keywordInput = 'keyword test'; - wrapper.vm.setKeywords(); - jest.runAllTimers(); - jest.useRealTimers(); + it('ticking "has published a channel" fetches users filtered by published_channel', async () => { + renderComponent(); + + await user.click(screen.getByLabelText('Has published a channel')); - expect(router.currentRoute.query.keywords).toBe('keyword test'); + await waitFor(() => { + expect(lastFetchParams()).toMatchObject({ published_channel: true }); + }); }); - it('changing joined-within filter sets joined_since query param to an ISO date', () => { - wrapper.vm.joinedWithinFilter = '3mo'; - const params = wrapper.vm.filterFetchQueryParams; - expect(params.joined_since).toMatch(/^\d{4}-\d{2}-\d{2}$/); + it('ticking "has Studio activity" fetches users filtered by has_edits', async () => { + renderComponent(); + + await user.click(screen.getByLabelText('Has Studio activity')); + + await waitFor(() => { + expect(lastFetchParams()).toMatchObject({ has_edits: true }); + }); }); - it('changing active-within filter sets active_since query param to an ISO date', () => { - wrapper.vm.activeWithinFilter = '1mo'; - const params = wrapper.vm.filterFetchQueryParams; - expect(params.active_since).toMatch(/^\d{4}-\d{2}-\d{2}$/); + // Reached by URL rather than by opening the select — see renderWithFilters. + it('a user type selection fetches users filtered by that type', async () => { + renderWithFilters({ userType: 'administrator' }); + + await waitFor(() => { + expect(lastFetchParams()).toMatchObject({ is_admin: true }); + }); }); - it('toggling has-published filter sets published_channel=true', () => { - wrapper.vm.hasPublishedFilter = true; - const params = wrapper.vm.filterFetchQueryParams; - expect(params.published_channel).toBe(true); + // Reached by URL rather than by opening the select — see renderWithFilters. + it('a joined-within selection fetches users filtered by an ISO joined_since date', async () => { + renderWithFilters({ joinedWithin: '3mo' }); + + await waitFor(() => { + expect(lastFetchParams().joined_since).toMatch(ISO_DATE); + }); }); - it('toggling has-edits filter sets has_edits=true', () => { - wrapper.vm.hasEditsFilter = true; - const params = wrapper.vm.filterFetchQueryParams; - expect(params.has_edits).toBe(true); + // Reached by URL rather than by opening the select — see renderWithFilters. + it('an active-within selection fetches users filtered by an ISO active_since date', async () => { + renderWithFilters({ activeWithin: '1mo' }); + + await waitFor(() => { + expect(lastFetchParams().active_since).toMatch(ISO_DATE); + }); + }); + + // Reached by URL rather than by opening the select — see renderWithFilters. + it('a target location selection fetches users filtered by that location', async () => { + renderWithFilters({ location: 'Afghanistan' }); + + await waitFor(() => { + expect(lastFetchParams()).toMatchObject({ location: 'Afghanistan' }); + }); }); }); - describe('selection', () => { - it('selectAll should set selected to channel list', () => { - wrapper.vm.selectAll = true; - expect(wrapper.vm.selected).toEqual(userList); + describe('clearing filters', () => { + it('is not offered on a page with no filters applied', () => { + renderComponent(); + + expect(clearFiltersLink()).not.toBeInTheDocument(); }); - it('removing selectAll should set selected to empty list', () => { - wrapper.vm.selected = userList; - wrapper.vm.selectAll = false; - wrapper.vm.$nextTick(() => { - expect(wrapper.vm.selected).toEqual([]); + it('is offered once a filter is applied', async () => { + renderComponent(); + + await user.click(screen.getByLabelText('Has published a channel')); + + await waitFor(() => { + expect(clearFiltersLink()).toBeInTheDocument(); }); }); - it('selectedCount should match the selected length', () => { - wrapper.vm.selected = ['test']; - expect(wrapper.vm.selectedCount).toBe(1); - }); + // Reached by URL rather than by opening the select — see renderWithFilters. + it('is offered for a user type of "All", which narrows nothing but is still a selection', async () => { + renderWithFilters({ userType: 'all' }); - it('selected should clear on query changes', () => { - wrapper.vm.selected = ['test']; - router.push({ - ...wrapper.vm.$route, - query: { - param: 'test', - }, + await waitFor(() => { + expect(clearFiltersLink()).toBeInTheDocument(); }); - wrapper.vm.$nextTick(() => { - expect(wrapper.vm.selected).toEqual([]); + await waitFor(() => { + expect(mockLoadUsers).toHaveBeenCalled(); }); + expect(lastFetchParams()).not.toHaveProperty('is_admin'); }); - }); - describe('bulk actions', () => { - it('should be hidden if no items are selected', () => { - expect(wrapper.find('[data-test="email"]').exists()).toBe(false); + // Reached by URL rather than by opening the select — see renderWithFilters. + it('stays unoffered for date windows left at their default', () => { + renderWithFilters({ joinedWithin: 'any', activeWithin: 'any' }); + + expect(clearFiltersLink()).not.toBeInTheDocument(); }); - it('should be visible if items are selected', async () => { - wrapper.vm.selected = userList; - await wrapper.vm.$nextTick(); - expect(wrapper.find('[data-test="email"]').exists()).toBe(true); + it('is withdrawn again after a checkbox is ticked and unticked', async () => { + renderComponent(); + const checkbox = screen.getByLabelText('Has published a channel'); + + await user.click(checkbox); + await waitFor(() => { + expect(clearFiltersLink()).toBeInTheDocument(); + }); + + await user.click(checkbox); + + await waitFor(() => { + expect(clearFiltersLink()).not.toBeInTheDocument(); + }); }); - it('email should open email dialog', async () => { - wrapper.vm.selected = userList; - await wrapper.vm.$nextTick(); - await wrapper.findComponent('[data-test="email"]').trigger('click'); - expect(wrapper.vm.showEmailDialog).toBe(true); + it('clears the checkboxes and the keyword search', async () => { + renderComponent(); + + await user.type(screen.getByLabelText('Search for a user...'), 'keyword test'); + // The search is debounced; let it reach the URL before clearing, otherwise a + // pending write lands after the clear and restores the term. + await waitFor(() => { + expect(router.currentRoute.query.keywords).toBe('keyword test'); + }); + await user.click(screen.getByLabelText('Has published a channel')); + await user.click(screen.getByLabelText('Has Studio activity')); + await waitFor(() => { + expect(clearFiltersLink()).toBeInTheDocument(); + }); + + await user.click(clearFiltersLink()); + + await waitFor(() => { + expect(screen.getByLabelText('Search for a user...')).toHaveValue(''); + }); + expect(screen.getByLabelText('Has published a channel')).not.toBeChecked(); + expect(screen.getByLabelText('Has Studio activity')).not.toBeChecked(); + expect(clearFiltersLink()).not.toBeInTheDocument(); + }); + + it('removes every filter query param while preserving pagination and sorting', async () => { + renderWithFilters({ + userType: 'administrator', + location: 'Afghanistan', + joinedWithin: '3mo', + activeWithin: '1mo', + hasPublished: 'yes', + hasEdits: 'yes', + keywords: 'keyword test', + page: '3', + page_size: '25', + sortBy: 'email', + descending: 'false', + }); + + await user.click(clearFiltersLink()); + + await waitFor(() => { + expect(Object.keys(router.currentRoute.query).sort()).toEqual([ + 'descending', + 'page', + 'page_size', + 'sortBy', + ]); + }); + expect(router.currentRoute.query.sortBy).toBe('email'); }); }); - describe('csv download', () => { - beforeEach(() => { - const client = require('shared/client').default; - const { saveAs } = require('file-saver'); - client.get.mockReset(); - client.get.mockResolvedValue({ - data: new Blob(['col1,col2\n1,2'], { type: 'text/csv' }), + describe('selection and bulk actions', () => { + it('offers no bulk email action until users are selected', () => { + renderComponent(); + + expect(screen.queryByTestId('email')).not.toBeInTheDocument(); + }); + + it('selecting all users offers a bulk email action for them', async () => { + renderComponent(); + + await user.click(selectAllCheckbox()); + + expect(await screen.findByTestId('email')).toBeInTheDocument(); + expect(screen.getByText(`(${USER_IDS.length})`)).toBeInTheDocument(); + }); + + it('discards the selection when the filters change', async () => { + renderComponent(); + + await user.click(selectAllCheckbox()); + expect(await screen.findByTestId('email')).toBeInTheDocument(); + + await user.click(screen.getByLabelText('Has published a channel')); + + await waitFor(() => { + expect(screen.queryByTestId('email')).not.toBeInTheDocument(); }); - saveAs.mockClear(); }); - it('renders the Download CSV button when count > 0', () => { - expect(wrapper.find('[data-test="csv"]').exists()).toBe(true); + it('the bulk email action opens the send email dialog', async () => { + renderComponent(); + + await user.click(selectAllCheckbox()); + await user.click(await screen.findByTestId('email')); + + expect(await screen.findByRole('heading', { name: 'Send email' })).toBeInTheDocument(); + }); + }); + + describe('CSV download', () => { + it('offers the download when there are users to export', () => { + renderComponent(); + + expect(screen.getByTestId('csv')).toBeEnabled(); }); - it('clicking Download CSV calls the API with the current filter params', async () => { - await wrapper.findComponent('[data-test="csv"]').trigger('click'); - // Flush the microtask queue so the chained .then() runs. - await new Promise(resolve => setImmediate(resolve)); + it('is unavailable when there are no users to export', () => { + renderComponent({ users: [] }); + expect(screen.getByTestId('csv')).toBeDisabled(); + }); + + it('downloads a dated CSV built from the current filters', async () => { const client = require('shared/client').default; const { saveAs } = require('file-saver'); - expect(client.get).toHaveBeenCalled(); - const [, options] = client.get.mock.calls[0]; - expect(options.responseType).toBe('blob'); - expect(saveAs).toHaveBeenCalled(); + renderComponent(); + + await user.click(screen.getByTestId('csv')); + + await waitFor(() => { + expect(saveAs).toHaveBeenCalled(); + }); + expect(client.get.mock.calls[0][1].responseType).toBe('blob'); const [savedBlob, savedName] = saveAs.mock.calls[0]; expect(savedBlob).toBeInstanceOf(Blob); expect(savedName).toMatch(/^studio_users_\d{4}-\d{2}-\d{2}\.csv$/); }); }); - - describe('csv download disabled state', () => { - it('disables Download CSV when count is zero', () => { - const emptyStore = new Store({ - modules: { - userAdmin: { - namespaced: true, - actions: { loadUsers }, - getters: { - users: () => [], - count: () => 0, - }, - }, - }, - }); - const emptyWrapper = makeWrapper(emptyStore); - const button = emptyWrapper.find('[data-test="csv"]'); - expect(button.attributes('disabled') !== undefined || button.props().disabled).toBe(true); - }); - }); }); diff --git a/contentcuration/contentcuration/tests/views/test_settings.py b/contentcuration/contentcuration/tests/views/test_settings.py index ed23fb0d70..2c75541fc1 100644 --- a/contentcuration/contentcuration/tests/views/test_settings.py +++ b/contentcuration/contentcuration/tests/views/test_settings.py @@ -15,6 +15,63 @@ def setUp(self): self.view.request = mock.Mock() self.view.request.user = testdata.user(email="tester@tester.com") + def _form(self, **overrides): + data = dict( + storage="storage", + kind="kind", + resource_count="resource_count", + resource_size="resource_size", + creators="creators", + sample_link="sample_link", + license="license", + public="channel1, channel2", + audience="audience", + import_count="import_count", + location="location", + uploading_for="uploading_for", + organization_type="organization_type", + time_constraint="time_constraint", + message="message", + ) + data.update(overrides) + form = StorageRequestForm(data=data) + self.assertTrue(form.is_valid()) + return form + + def test_storage_request_records_requested_storage(self): + user = self.view.request.user + user.information = {"space_needed": "500MB", "heard_from": "newsletter"} + user.save() + + with mock.patch("contentcuration.views.settings.send_mail"): + self.view.form_valid(self._form(storage="10GB")) + + user.refresh_from_db() + self.assertEqual(user.information["latest_storage_request"], "10GB") + self.assertEqual(user.information["space_needed"], "500MB") + self.assertEqual(user.information["heard_from"], "newsletter") + + def test_storage_request_records_requested_storage_without_prior_information(self): + user = self.view.request.user + user.information = None + user.save() + + with mock.patch("contentcuration.views.settings.send_mail"): + self.view.form_valid(self._form(storage="1TB")) + + user.refresh_from_db() + self.assertEqual(user.information["latest_storage_request"], "1TB") + + def test_storage_request_overwrites_the_previous_request(self): + user = self.view.request.user + + with mock.patch("contentcuration.views.settings.send_mail"): + self.view.form_valid(self._form(storage="1GB")) + self.view.form_valid(self._form(storage="2GB")) + + user.refresh_from_db() + self.assertEqual(user.information["latest_storage_request"], "2GB") + def test_storage_request(self): with mock.patch("contentcuration.views.settings.send_mail") as send_mail: diff --git a/contentcuration/contentcuration/tests/viewsets/test_user.py b/contentcuration/contentcuration/tests/viewsets/test_user.py index 4e050888d2..8e37fbe055 100644 --- a/contentcuration/contentcuration/tests/viewsets/test_user.py +++ b/contentcuration/contentcuration/tests/viewsets/test_user.py @@ -306,6 +306,26 @@ def test_admin_users_download_csv_streams_filtered_users(self): self.assertIn("United States", body) self.assertIn("Mexico", body) + def test_admin_users_download_csv_prefers_the_latest_storage_request(self): + target = testdata.user(email="csv-storage@e.com") + target.information = { + "space_needed": "500MB", + "latest_storage_request": "10GB", + } + target.save() + + self.user.is_admin = True + self.user.save() + self.client.force_authenticate(user=self.user) + + response = self.client.get(self._csv_url() + f"?ids={target.id}") + self.assertEqual(response.status_code, 200) + + body = self._csv_body(response) + self.assertIn("Has Studio activity", body) + self.assertIn("10GB", body) + self.assertNotIn("500MB", body) + def test_admin_users_download_csv_handles_null_information(self): user_no_info = testdata.user(email="no-info@e.com") user_no_info.information = None diff --git a/contentcuration/contentcuration/views/settings.py b/contentcuration/contentcuration/views/settings.py index 8f2444b158..e8caaceaf1 100644 --- a/contentcuration/contentcuration/views/settings.py +++ b/contentcuration/contentcuration/views/settings.py @@ -177,6 +177,8 @@ class StorageSettingsView(PostFormMixin, FormView): form_class = StorageRequestForm def form_valid(self, form): + self.record_storage_request(self.request.user, form.cleaned_data["storage"]) + channels = [c for c in form.cleaned_data["public"].split(", ") if c] message = render_to_string( "settings/storage_request_email.txt", @@ -194,6 +196,13 @@ def form_valid(self, form): [ccsettings.SPACE_REQUEST_EMAIL, self.request.user.email], ) + @staticmethod + def record_storage_request(user, storage): + information = user.information or {} + information["latest_storage_request"] = storage + user.information = information + user.save(update_fields=["information"]) + class PolicyAcceptView(PostFormMixin, FormView): form_class = PolicyAcceptForm diff --git a/contentcuration/contentcuration/viewsets/user.py b/contentcuration/contentcuration/viewsets/user.py index 126a342319..61bb051e79 100644 --- a/contentcuration/contentcuration/viewsets/user.py +++ b/contentcuration/contentcuration/viewsets/user.py @@ -463,7 +463,7 @@ class AdminUserCSVFilter(AdminUserFilter, RequiredFilterSet): "Has viewable channels", "Has published a channel", "Most recent publish date", - "Has Studio edits", + "Has Studio activity", "Locations (country names)", "Primary location", "Location count", @@ -500,6 +500,10 @@ def _iso_date(value): return value.date().isoformat() if hasattr(value, "date") else value.isoformat() +def _storage_needed(info): + return info.get("latest_storage_request") or info.get("space_needed") or "" + + def _build_csv_row(values, country_names): """Translate one user .values() dict to a CSV row. @@ -528,7 +532,7 @@ def _build_csv_row(values, country_names): ", ".join(location_names), location_names[0] if location_names else "", len(location_codes), - info.get("space_needed") or "", + _storage_needed(info), info.get("heard_from") or "", ]