From 4bd60c2801787fa2ad285c9d015e35b2d5b7487c Mon Sep 17 00:00:00 2001 From: Gagandeep Bhatia Date: Thu, 22 Aug 2024 14:12:26 +0200 Subject: [PATCH 01/31] wip - needs some clean up --- .../common/ComposedFilter/ComposedFilter.jsx | 6 + .../js/common/RemoteFilter/Filter.stories.jsx | 89 ++++ .../js/common/RemoteFilter/RemoteFilter.jsx | 159 ++++++ .../RemoteFilter/RemoteFilter.stories.jsx | 89 ++++ .../common/RemoteFilter/RemoteFilter.test.jsx | 462 ++++++++++++++++++ assets/js/common/RemoteFilter/index.js | 3 + .../pages/ActivityLogPage/ActivityLogPage.jsx | 29 ++ assets/js/state/activityLog.js | 22 + assets/js/state/index.js | 2 + assets/js/state/sagas/activityLog.js | 14 + assets/js/state/sagas/channels.js | 10 + assets/js/state/sagas/index.js | 2 + .../channels/activity_log_channel.ex | 40 ++ lib/trento_web/channels/user_socket.ex | 1 + 14 files changed, 928 insertions(+) create mode 100644 assets/js/common/RemoteFilter/Filter.stories.jsx create mode 100644 assets/js/common/RemoteFilter/RemoteFilter.jsx create mode 100644 assets/js/common/RemoteFilter/RemoteFilter.stories.jsx create mode 100644 assets/js/common/RemoteFilter/RemoteFilter.test.jsx create mode 100644 assets/js/common/RemoteFilter/index.js create mode 100644 assets/js/state/activityLog.js create mode 100644 assets/js/state/sagas/activityLog.js create mode 100644 lib/trento_web/channels/activity_log_channel.ex diff --git a/assets/js/common/ComposedFilter/ComposedFilter.jsx b/assets/js/common/ComposedFilter/ComposedFilter.jsx index 1e9babb17c..f5e3e93cfc 100644 --- a/assets/js/common/ComposedFilter/ComposedFilter.jsx +++ b/assets/js/common/ComposedFilter/ComposedFilter.jsx @@ -1,10 +1,16 @@ import React, { useState } from 'react'; import Button from '@common/Button'; import Filter from '@common/Filter'; +import RemoteFilter from '@common/RemoteFilter'; import DateFilter from '@common/DateFilter'; const renderFilter = (key, { type, ...filterProps }, value, onChange) => { switch (type) { + case 'remoteselect': + return ( + + ); + case 'select': return ( diff --git a/assets/js/common/RemoteFilter/Filter.stories.jsx b/assets/js/common/RemoteFilter/Filter.stories.jsx new file mode 100644 index 0000000000..5f649930b4 --- /dev/null +++ b/assets/js/common/RemoteFilter/Filter.stories.jsx @@ -0,0 +1,89 @@ +import React, { useState } from 'react'; +import { action } from '@storybook/addon-actions'; + +import RemoteFilter from '.'; + +export default { + title: 'Components/Filter', + component: RemoteFilter, + argTypes: { + options: { + type: { name: 'array', required: true }, + description: 'List of options available', + control: { type: 'object' }, + }, + title: { + type: { name: 'string', required: true }, + description: + 'Title of the filter, will appear as placeholder when no value is selected', + control: { type: 'text' }, + }, + value: { + type: { name: 'array', required: false, defaultValue: [] }, + description: 'Selected options', + control: { type: 'object' }, + }, + onChange: { + type: { name: 'function', required: false }, + description: 'Function to call when the selected options change', + control: { type: null }, + }, + }, + render: (args) => { + const [value, setValue] = useState(args.value); + + return ( + { + setValue(newValue); + action('onChange')(newValue); + }} + /> + ); + }, +}; + +export const Default = { + args: { + title: 'Title', + options: [ + 'Tony Kekw', + 'Chad Carbonara', + 'Chuck Amatriciana', + 'K.C.O. Pepe', + 'Virginia Gricia', + ], + value: [], + }, +}; + +export const WithValue = { + args: { + ...Default.args, + value: ['Tony Kekw'], + }, +}; + +export const WithMultipleValues = { + args: { + ...Default.args, + value: ['Tony Kekw', 'Chad Carbonara'], + }, +}; + +export const WithLabel = { + args: { + ...Default.args, + options: [ + ['tony-kekw', 'Tony Kekw'], + ['chad-carbonara', 'Chad Carbonara'], + ['chuck-amatriciana', 'Chuck Amatriciana'], + ['kco-pepe', 'K.C.O. Pepe'], + ['virginia-gricia', 'Virginia Gricia'], + ], + value: ['tony-kekw'], + }, +}; diff --git a/assets/js/common/RemoteFilter/RemoteFilter.jsx b/assets/js/common/RemoteFilter/RemoteFilter.jsx new file mode 100644 index 0000000000..a39f1d12fc --- /dev/null +++ b/assets/js/common/RemoteFilter/RemoteFilter.jsx @@ -0,0 +1,159 @@ +/* eslint-disable react/no-array-index-key */ + +import React, { Fragment, useState, useRef } from 'react'; +import classNames from 'classnames'; +import { Transition } from '@headlessui/react'; +import { useSelector, connect } from 'react-redux'; +import { EOS_CLOSE, EOS_CHECK } from 'eos-icons-react'; + +import { toggle, hasOne } from '@lib/lists'; +import useOnClickOutside from '@hooks/useOnClickOutside'; + +const getLabel = (value, placeholder) => + value.length === 0 ? placeholder : value.join(', '); + +/** + * RemoteFilter component + * + * @param {string[]|[string, string][]} props.options List of options to filter, e.g. ['Option 1', 'Option 2'] + * @param {string} props.title Title of the filter. It will be displayed in the button when the filter is empty + * @param {string|string[]} props.value Selected options. Default is an empty array + * @param {function} props.onChange Function to call when the selected options change + */ +function RemoteFilter({ options, title, value = [], onChange}) { + const ref = useRef(); + const [open, setOpen] = useState(false); + const [query, setQuery] = useState(''); + + const labeledOptions = options + .filter((option) => option !== undefined && option !== null) + .map((option) => (typeof option === 'string' ? [option, option] : option)); + + const filteredOptions = labeledOptions.filter((option) => + option[0].toLowerCase().includes(query.toLowerCase()) + ); + + const parsedValue = typeof value === 'string' ? [value] : value; + + const selectedLabels = parsedValue.reduce((acc, key) => { + const element = labeledOptions.find(([optionKey]) => optionKey === key); + return element ? [...acc, element[1]] : acc; + }, []); + + useOnClickOutside(ref, () => setOpen(false)); + + return ( +
+
+ {parsedValue.length !== 0 && ( + + )} + + setQuery('')} + show={open} + > +
+
+
+ + setQuery(newValue) + } + /> +
+
    + {filteredOptions.map(([key, label], index) => ( + + ))} +
+
+
+ +
+
+ ); +} + +const mapStateToProps = (state) => ( { + options: !state.activityLog.users ? ["Loading"] : state.activityLog.users +}); + + +export default connect(mapStateToProps)(RemoteFilter); + +// export default RemoteFilter; diff --git a/assets/js/common/RemoteFilter/RemoteFilter.stories.jsx b/assets/js/common/RemoteFilter/RemoteFilter.stories.jsx new file mode 100644 index 0000000000..5f649930b4 --- /dev/null +++ b/assets/js/common/RemoteFilter/RemoteFilter.stories.jsx @@ -0,0 +1,89 @@ +import React, { useState } from 'react'; +import { action } from '@storybook/addon-actions'; + +import RemoteFilter from '.'; + +export default { + title: 'Components/Filter', + component: RemoteFilter, + argTypes: { + options: { + type: { name: 'array', required: true }, + description: 'List of options available', + control: { type: 'object' }, + }, + title: { + type: { name: 'string', required: true }, + description: + 'Title of the filter, will appear as placeholder when no value is selected', + control: { type: 'text' }, + }, + value: { + type: { name: 'array', required: false, defaultValue: [] }, + description: 'Selected options', + control: { type: 'object' }, + }, + onChange: { + type: { name: 'function', required: false }, + description: 'Function to call when the selected options change', + control: { type: null }, + }, + }, + render: (args) => { + const [value, setValue] = useState(args.value); + + return ( + { + setValue(newValue); + action('onChange')(newValue); + }} + /> + ); + }, +}; + +export const Default = { + args: { + title: 'Title', + options: [ + 'Tony Kekw', + 'Chad Carbonara', + 'Chuck Amatriciana', + 'K.C.O. Pepe', + 'Virginia Gricia', + ], + value: [], + }, +}; + +export const WithValue = { + args: { + ...Default.args, + value: ['Tony Kekw'], + }, +}; + +export const WithMultipleValues = { + args: { + ...Default.args, + value: ['Tony Kekw', 'Chad Carbonara'], + }, +}; + +export const WithLabel = { + args: { + ...Default.args, + options: [ + ['tony-kekw', 'Tony Kekw'], + ['chad-carbonara', 'Chad Carbonara'], + ['chuck-amatriciana', 'Chuck Amatriciana'], + ['kco-pepe', 'K.C.O. Pepe'], + ['virginia-gricia', 'Virginia Gricia'], + ], + value: ['tony-kekw'], + }, +}; diff --git a/assets/js/common/RemoteFilter/RemoteFilter.test.jsx b/assets/js/common/RemoteFilter/RemoteFilter.test.jsx new file mode 100644 index 0000000000..8b28e193fd --- /dev/null +++ b/assets/js/common/RemoteFilter/RemoteFilter.test.jsx @@ -0,0 +1,462 @@ +import React, { act } from 'react'; +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import '@testing-library/jest-dom'; +import RemoteFilter from '.'; + +describe('Filter component', () => { + it('should render the options correctly', async () => { + const user = userEvent.setup(); + + const options = [ + 'John Doe', + 'Jane Smith', + 'Michael Scott', + 'Ella Fitzgerald', + ]; + + render(); + + // options are not rendered initially + options.forEach((option) => { + expect(screen.queryByText(option)).not.toBeInTheDocument(); + }); + + await act(() => + // select a button which text starts with 'Filter ' + // click on the button to open the options + user.click(screen.getByText(/Filter *.../)) + ); + + // assert that the options are rendered + options.forEach((option) => { + expect(screen.getByText(option)).toBeInTheDocument(); + }); + }); + + it('should select an option when clicked', async () => { + const user = userEvent.setup(); + const mockOnChange = jest.fn(); + const options = [ + 'John Doe', + 'Jane Smith', + 'Michael Scott', + 'Ella Fitzgerald', + ]; + + render(); + + await act(() => user.click(screen.getByText(/Filter *.../))); + + await act(() => user.click(screen.getByText('Michael Scott'))); + + expect(mockOnChange).toHaveBeenCalledWith(['Michael Scott']); + }); + + it('should select multiple items', async () => { + const user = userEvent.setup(); + const mockOnChange = jest.fn(); + const options = [ + 'John Doe', + 'Jane Smith', + 'Michael Scott', + 'Ella Fitzgerald', + ]; + + const selectedItem = 'Michael Scott'; + + render( + + ); + + await act(() => user.click(screen.getByText(selectedItem))); + + await act(() => user.click(screen.getByText('Jane Smith'))); + + expect(mockOnChange).toHaveBeenCalledTimes(1); + + expect(mockOnChange).toHaveBeenCalledWith([selectedItem, 'Jane Smith']); + }); + + it('should select with one element if passed as string', async () => { + const user = userEvent.setup(); + const mockOnChange = jest.fn(); + const options = [ + 'John Doe', + 'Jane Smith', + 'Michael Scott', + 'Ella Fitzgerald', + ]; + + const selectedItem = 'Michael Scott'; + + render( + + ); + + await act(() => user.click(screen.getByText(selectedItem))); + + await act(() => user.click(screen.getByText('Jane Smith'))); + + expect(mockOnChange).toHaveBeenCalledTimes(1); + + expect(mockOnChange).toHaveBeenCalledWith([selectedItem, 'Jane Smith']); + }); + + it('should deselect an item on multiple item selected', async () => { + const user = userEvent.setup(); + const mockOnChange = jest.fn(); + const options = [ + 'John Doe', + 'Jane Smith', + 'Michael Scott', + 'Ella Fitzgerald', + ]; + + const selectedItem1 = 'Michael Scott'; + const selectedItem2 = 'Jane Smith'; + + render( + + ); + + const filterText = [selectedItem1, selectedItem2].join(', '); + await act(() => user.click(screen.getByText(filterText))); + + await act(() => user.click(screen.getByText('Jane Smith'))); + + expect(mockOnChange).toHaveBeenCalledTimes(1); + + expect(mockOnChange).toHaveBeenCalledWith([selectedItem1]); + + // the list of items is still visible + options.forEach((option) => { + expect(screen.getByText(option)).toBeInTheDocument(); + }); + }); + + it('should deselect an item on single item selected', async () => { + const user = userEvent.setup(); + const mockOnChange = jest.fn(); + const options = [ + 'John Doe', + 'Jane Smith', + 'Michael Scott', + 'Ella Fitzgerald', + ]; + + const selectedItem = 'Michael Scott'; + + render( + + ); + + await act(() => user.click(screen.getByText(selectedItem))); + + await act(() => user.click(screen.getAllByText(selectedItem)[1])); + + expect(mockOnChange).toHaveBeenCalledWith([]); + }); + + it('should use options with label', async () => { + const user = userEvent.setup(); + const mockOnChange = jest.fn(); + const options = [ + ['john-doe', 'John Doe'], + ['jane-smith', 'Jane Smith'], + ['michael-scott', 'Michael Scott'], + ['ella-fitzgerald', 'Ella Fitzgerald'], + ]; + + const selectedItem = 'michael-scott'; + const selectedItemLabel = 'Michael Scott'; + + const anotherSelectedItem = 'jane-smith'; + const anotherSelectedItemLabel = 'Jane Smith'; + + render( + + ); + + await act(() => user.click(screen.getByText(selectedItemLabel))); + + await act(() => { + options.forEach(([, label]) => { + expect(screen.getAllByText(label)[0]).toBeInTheDocument(); + }); + }); + + await act(() => + user.click(screen.getAllByText(anotherSelectedItemLabel)[0]) + ); + + expect(mockOnChange).toHaveBeenCalledWith([ + selectedItem, + anotherSelectedItem, + ]); + }); + + it('should render correctly mixed options', async () => { + const user = userEvent.setup(); + const mockOnChange = jest.fn(); + const options = [ + 'Michael Scott', + undefined, + ['john-doe', 'John Doe'], + 'Jane Smith', + ]; + + render( + + ); + + await act(() => user.click(screen.getByText('Michael Scott'))); + + await act(() => { + expect(screen.getByText('John Doe')).toBeInTheDocument(); + expect(screen.getByText('Jane Smith')).toBeInTheDocument(); + }); + + /* Remember that the component is stateless, + it does not change its internal state on selection. */ + + await act(() => user.click(screen.getByText('John Doe'))); + expect(mockOnChange).toHaveBeenCalledWith(['Michael Scott', 'john-doe']); + + await act(() => user.click(screen.getByText('Jane Smith'))); + expect(mockOnChange).toHaveBeenCalledWith(['Michael Scott', 'Jane Smith']); + }); + + it('should ignore undefined values', async () => { + const user = userEvent.setup(); + const mockOnChange = jest.fn(); + const options = [ + 'John Doe', + 'Jane Smith', + 'Michael Scott', + 'Ella Fitzgerald', + ]; + + render( + + ); + + await act(() => user.click(screen.getByText('Michael Scott'))); + + await act(() => { + options.forEach((label) => { + expect(screen.getAllByText(label)[0]).toBeInTheDocument(); + }); + }); + }); + + it('should ignore undefined options', async () => { + const user = userEvent.setup(); + const mockOnChange = jest.fn(); + const options = [ + '5E8', + undefined, + '32S', + '4B9', + 'MF5', + 'PRD', + 'QAS', + 'HA1', + 'HA2', + ]; + const value = ['PRD']; + + render( + + ); + + await act(() => user.click(screen.getByText('PRD'))); + + await act(() => { + options.filter(Boolean).forEach((label) => { + expect(screen.getAllByText(label)[0]).toBeInTheDocument(); + }); + }); + }); + + it('should use options with label', async () => { + const user = userEvent.setup(); + const mockOnChange = jest.fn(); + const options = [ + ['john-doe', 'John Doe'], + ['jane-smith', 'Jane Smith'], + ['michael-scott', 'Michael Scott'], + ['ella-fitzgerald', 'Ella Fitzgerald'], + ]; + + const selectedItem = 'michael-scott'; + const selectedItemLabel = 'Michael Scott'; + + const anotherSelectedItem = 'jane-smith'; + const anotherSelectedItemLabel = 'Jane Smith'; + + render( + + ); + + await act(() => user.click(screen.getByText(selectedItemLabel))); + + await act(() => { + options.forEach(([, label]) => { + expect(screen.getAllByText(label)[0]).toBeInTheDocument(); + }); + }); + + await act(() => + user.click(screen.getAllByText(anotherSelectedItemLabel)[0]) + ); + + expect(mockOnChange).toHaveBeenCalledWith([ + selectedItem, + anotherSelectedItem, + ]); + }); + + it('should render correctly mixed options', async () => { + const user = userEvent.setup(); + const mockOnChange = jest.fn(); + const options = [ + 'Michael Scott', + undefined, + ['john-doe', 'John Doe'], + 'Jane Smith', + ]; + + render( + + ); + + await act(() => user.click(screen.getByText('Michael Scott'))); + + await act(() => { + expect(screen.getByText('John Doe')).toBeInTheDocument(); + expect(screen.getByText('Jane Smith')).toBeInTheDocument(); + }); + + /* Remember that the component is stateless, + it does not change its internal state on selection. */ + + await act(() => user.click(screen.getByText('John Doe'))); + expect(mockOnChange).toHaveBeenCalledWith(['Michael Scott', 'john-doe']); + + await act(() => user.click(screen.getByText('Jane Smith'))); + expect(mockOnChange).toHaveBeenCalledWith(['Michael Scott', 'Jane Smith']); + }); + + it('should ignore undefined values', async () => { + const user = userEvent.setup(); + const mockOnChange = jest.fn(); + const options = [ + 'John Doe', + 'Jane Smith', + 'Michael Scott', + 'Ella Fitzgerald', + ]; + + render( + + ); + + await act(() => user.click(screen.getByText('Michael Scott'))); + + await act(() => { + options.forEach((label) => { + expect(screen.getAllByText(label)[0]).toBeInTheDocument(); + }); + }); + }); + + it('should ignore undefined options', async () => { + const user = userEvent.setup(); + const mockOnChange = jest.fn(); + const options = [ + '5E8', + undefined, + '32S', + '4B9', + 'MF5', + 'PRD', + 'QAS', + 'HA1', + 'HA2', + ]; + const value = ['PRD']; + + render( + + ); + + await act(() => user.click(screen.getByText('PRD'))); + + await act(() => { + options.filter(Boolean).forEach((label) => { + expect(screen.getAllByText(label)[0]).toBeInTheDocument(); + }); + }); + }); +}); diff --git a/assets/js/common/RemoteFilter/index.js b/assets/js/common/RemoteFilter/index.js new file mode 100644 index 0000000000..010f7ba04d --- /dev/null +++ b/assets/js/common/RemoteFilter/index.js @@ -0,0 +1,3 @@ +import RemoteFilter from './RemoteFilter'; + +export default RemoteFilter; diff --git a/assets/js/pages/ActivityLogPage/ActivityLogPage.jsx b/assets/js/pages/ActivityLogPage/ActivityLogPage.jsx index 7ed8b592e5..28db00efa0 100644 --- a/assets/js/pages/ActivityLogPage/ActivityLogPage.jsx +++ b/assets/js/pages/ActivityLogPage/ActivityLogPage.jsx @@ -20,6 +20,35 @@ import { } from './searchParams'; function ActivityLogPage() { + const filters = [ + { + key: 'type', + type: 'select', + title: 'Resource type', + options: Object.entries(ACTIVITY_TYPES_CONFIG).map(([key, value]) => [ + key, + value.label, + ]), + }, + { + key: 'actor', + type: 'remoteselect', + title: 'User', + }, + { + key: 'to_date', + title: 'newer than', + type: 'date', + prefilled: true, + }, + { + key: 'from_date', + title: 'older than', + type: 'date', + prefilled: true, + }, + ]; + const [searchParams, setSearchParams] = useSearchParams(); const [activityLog, setActivityLog] = useState([]); const [isLoading, setLoading] = useState(true); diff --git a/assets/js/state/activityLog.js b/assets/js/state/activityLog.js new file mode 100644 index 0000000000..5c1861de4e --- /dev/null +++ b/assets/js/state/activityLog.js @@ -0,0 +1,22 @@ +import { createAction, createSlice } from '@reduxjs/toolkit'; + +export const initialState = { + users: [] +}; + +export const activityLogSlice = createSlice({ + name: 'activityLog', + initialState, + reducers: { + setUsers( state, { payload: { users }, }) { + state.users = users; + }, + }, +}); + +export const AL_USERS_PUSHED = 'AL_USERS_PUSHED'; +export const alUsersPushed = createAction(AL_USERS_PUSHED, ({users}) => ({payload: { users}})); +export const { setUsers } = + activityLogSlice.actions; + +export default activityLogSlice.reducer; diff --git a/assets/js/state/index.js b/assets/js/state/index.js index b2a16edca7..5da328ce03 100644 --- a/assets/js/state/index.js +++ b/assets/js/state/index.js @@ -14,6 +14,7 @@ import settingsReducer from './settings'; import userReducer from './user'; import softwareUpdatesReducer from './softwareUpdates'; import activityLogsSettingsReducer from './activityLogsSettings'; +import activityLogReducer from './activityLog'; import rootSaga from './sagas'; export const createStore = (router) => { @@ -38,6 +39,7 @@ export const createStore = (router) => { user: userReducer, softwareUpdates: softwareUpdatesReducer, activityLogsSettings: activityLogsSettingsReducer, + activityLog: activityLogReducer, }, middleware: (getDefaultMiddleware) => getDefaultMiddleware().concat(sagaMiddleware), diff --git a/assets/js/state/sagas/activityLog.js b/assets/js/state/sagas/activityLog.js new file mode 100644 index 0000000000..52eaebee87 --- /dev/null +++ b/assets/js/state/sagas/activityLog.js @@ -0,0 +1,14 @@ +import { put, takeEvery } from 'redux-saga/effects'; +import { + setUsers, + AL_USERS_PUSHED +} from '@state/activityLog'; + +export function* alUsersUpdate({payload: {users}}) { + yield put(setUsers({users})) +} + +export function* watchAlActions() { + yield takeEvery(AL_USERS_PUSHED, alUsersUpdate); + +} diff --git a/assets/js/state/sagas/channels.js b/assets/js/state/sagas/channels.js index 5ec791be63..bea014902d 100644 --- a/assets/js/state/sagas/channels.js +++ b/assets/js/state/sagas/channels.js @@ -55,6 +55,7 @@ import { } from '@state/lastExecutions'; import { userUpdated, userLocked, userDeleted } from '@state/user'; +import { alUsersPushed } from '@state/activityLog'; import { getUserProfile } from '@state/selectors/user'; @@ -236,6 +237,14 @@ const userEvents = [ }, ]; +const activityLogEvents = [ + { + name: 'al_users_pushed', + action: alUsersPushed, + } +]; + + const createEventChannel = (channel, events) => eventChannel((emitter) => { events.forEach((event) => { @@ -281,5 +290,6 @@ export function* watchSocketEvents(socket) { fork(watchChannelEvents, socket, 'monitoring:databases', databaseEvents), fork(watchChannelEvents, socket, 'monitoring:executions', executionEvents), fork(watchChannelEvents, socket, `users:${userID}`, userEvents), + fork(watchChannelEvents, socket, `activity_log:${userID}`, activityLogEvents), ]); } diff --git a/assets/js/state/sagas/index.js b/assets/js/state/sagas/index.js index 1e3dae42d1..8683edb24d 100644 --- a/assets/js/state/sagas/index.js +++ b/assets/js/state/sagas/index.js @@ -80,6 +80,7 @@ import { watchActivityLogsSettings } from '@state/sagas/activityLogsSettings'; import { watchSoftwareUpdates } from '@state/sagas/softwareUpdates'; import { watchSocketEvents } from '@state/sagas/channels'; +import { watchAlActions } from '@state/sagas/activityLog'; import { checkApiKeyExpiration } from '@state/sagas/settings'; const RESET_STATE = 'RESET_STATE'; @@ -249,5 +250,6 @@ export default function* rootSaga() { watchUserLoggedIn(), watchActivityLogsSettings(), watchSoftwareUpdates(), + watchAlActions(), ]); } diff --git a/lib/trento_web/channels/activity_log_channel.ex b/lib/trento_web/channels/activity_log_channel.ex new file mode 100644 index 0000000000..32bb954ef6 --- /dev/null +++ b/lib/trento_web/channels/activity_log_channel.ex @@ -0,0 +1,40 @@ +defmodule TrentoWeb.ActivityLogChannel do + @moduledoc """ + Activity Log channel, each user is subscribed to this channel, + + """ + require Logger + use TrentoWeb, :channel + + @impl true + def join( + "activity_log:" <> user_id, + _payload, + %{assigns: %{current_user_id: current_user_id}} = socket + ) do + user_id = String.to_integer(user_id) + + if user_id == current_user_id do + send(self(), :after_join) + {:ok, socket} + else + Logger.error( + "Could not join user channel, requested user id: #{user_id}, authenticated user id: #{current_user_id}" + ) + + {:error, :unauthorized} + end + end + + def join("activity_log:" <> _user_id, _payload, _socket) do + {:error, :user_not_logged} + end + + @impl true + def handle_info(:after_join, socket) do + Logger.warning("updates users list") + push(socket, "al_users_pushed", %{users: ["foo", "bar", "baz", "admin"]}) + Process.send_after(self(), :after_join, 5000) + {:noreply, socket} + end +end diff --git a/lib/trento_web/channels/user_socket.ex b/lib/trento_web/channels/user_socket.ex index 7e20cdb649..fbebeb6b43 100644 --- a/lib/trento_web/channels/user_socket.ex +++ b/lib/trento_web/channels/user_socket.ex @@ -13,6 +13,7 @@ defmodule TrentoWeb.UserSocket do channel "monitoring:*", TrentoWeb.MonitoringChannel channel "users:*", TrentoWeb.UserChannel + channel "activity_log:*", TrentoWeb.ActivityLogChannel # Socket params are passed from the client and can # be used to verify and authenticate a user. After From ede1402a9f8a364ea8e1c83fd09c6c673dfdd9f1 Mon Sep 17 00:00:00 2001 From: Gagandeep Bhatia Date: Thu, 22 Aug 2024 16:22:26 +0200 Subject: [PATCH 02/31] See description - Deletes unused code - Pulls out state access into the ActivityLog page context --- .../js/common/RemoteFilter/Filter.stories.jsx | 89 ---- .../js/common/RemoteFilter/RemoteFilter.jsx | 159 ------ .../RemoteFilter/RemoteFilter.stories.jsx | 89 ---- .../common/RemoteFilter/RemoteFilter.test.jsx | 462 ------------------ assets/js/common/RemoteFilter/index.js | 3 - .../pages/ActivityLogPage/ActivityLogPage.jsx | 6 +- assets/js/state/selectors/activityLog.js | 7 + 7 files changed, 12 insertions(+), 803 deletions(-) delete mode 100644 assets/js/common/RemoteFilter/Filter.stories.jsx delete mode 100644 assets/js/common/RemoteFilter/RemoteFilter.jsx delete mode 100644 assets/js/common/RemoteFilter/RemoteFilter.stories.jsx delete mode 100644 assets/js/common/RemoteFilter/RemoteFilter.test.jsx delete mode 100644 assets/js/common/RemoteFilter/index.js create mode 100644 assets/js/state/selectors/activityLog.js diff --git a/assets/js/common/RemoteFilter/Filter.stories.jsx b/assets/js/common/RemoteFilter/Filter.stories.jsx deleted file mode 100644 index 5f649930b4..0000000000 --- a/assets/js/common/RemoteFilter/Filter.stories.jsx +++ /dev/null @@ -1,89 +0,0 @@ -import React, { useState } from 'react'; -import { action } from '@storybook/addon-actions'; - -import RemoteFilter from '.'; - -export default { - title: 'Components/Filter', - component: RemoteFilter, - argTypes: { - options: { - type: { name: 'array', required: true }, - description: 'List of options available', - control: { type: 'object' }, - }, - title: { - type: { name: 'string', required: true }, - description: - 'Title of the filter, will appear as placeholder when no value is selected', - control: { type: 'text' }, - }, - value: { - type: { name: 'array', required: false, defaultValue: [] }, - description: 'Selected options', - control: { type: 'object' }, - }, - onChange: { - type: { name: 'function', required: false }, - description: 'Function to call when the selected options change', - control: { type: null }, - }, - }, - render: (args) => { - const [value, setValue] = useState(args.value); - - return ( - { - setValue(newValue); - action('onChange')(newValue); - }} - /> - ); - }, -}; - -export const Default = { - args: { - title: 'Title', - options: [ - 'Tony Kekw', - 'Chad Carbonara', - 'Chuck Amatriciana', - 'K.C.O. Pepe', - 'Virginia Gricia', - ], - value: [], - }, -}; - -export const WithValue = { - args: { - ...Default.args, - value: ['Tony Kekw'], - }, -}; - -export const WithMultipleValues = { - args: { - ...Default.args, - value: ['Tony Kekw', 'Chad Carbonara'], - }, -}; - -export const WithLabel = { - args: { - ...Default.args, - options: [ - ['tony-kekw', 'Tony Kekw'], - ['chad-carbonara', 'Chad Carbonara'], - ['chuck-amatriciana', 'Chuck Amatriciana'], - ['kco-pepe', 'K.C.O. Pepe'], - ['virginia-gricia', 'Virginia Gricia'], - ], - value: ['tony-kekw'], - }, -}; diff --git a/assets/js/common/RemoteFilter/RemoteFilter.jsx b/assets/js/common/RemoteFilter/RemoteFilter.jsx deleted file mode 100644 index a39f1d12fc..0000000000 --- a/assets/js/common/RemoteFilter/RemoteFilter.jsx +++ /dev/null @@ -1,159 +0,0 @@ -/* eslint-disable react/no-array-index-key */ - -import React, { Fragment, useState, useRef } from 'react'; -import classNames from 'classnames'; -import { Transition } from '@headlessui/react'; -import { useSelector, connect } from 'react-redux'; -import { EOS_CLOSE, EOS_CHECK } from 'eos-icons-react'; - -import { toggle, hasOne } from '@lib/lists'; -import useOnClickOutside from '@hooks/useOnClickOutside'; - -const getLabel = (value, placeholder) => - value.length === 0 ? placeholder : value.join(', '); - -/** - * RemoteFilter component - * - * @param {string[]|[string, string][]} props.options List of options to filter, e.g. ['Option 1', 'Option 2'] - * @param {string} props.title Title of the filter. It will be displayed in the button when the filter is empty - * @param {string|string[]} props.value Selected options. Default is an empty array - * @param {function} props.onChange Function to call when the selected options change - */ -function RemoteFilter({ options, title, value = [], onChange}) { - const ref = useRef(); - const [open, setOpen] = useState(false); - const [query, setQuery] = useState(''); - - const labeledOptions = options - .filter((option) => option !== undefined && option !== null) - .map((option) => (typeof option === 'string' ? [option, option] : option)); - - const filteredOptions = labeledOptions.filter((option) => - option[0].toLowerCase().includes(query.toLowerCase()) - ); - - const parsedValue = typeof value === 'string' ? [value] : value; - - const selectedLabels = parsedValue.reduce((acc, key) => { - const element = labeledOptions.find(([optionKey]) => optionKey === key); - return element ? [...acc, element[1]] : acc; - }, []); - - useOnClickOutside(ref, () => setOpen(false)); - - return ( -
-
- {parsedValue.length !== 0 && ( - - )} - - setQuery('')} - show={open} - > -
-
-
- - setQuery(newValue) - } - /> -
-
    - {filteredOptions.map(([key, label], index) => ( - - ))} -
-
-
- -
-
- ); -} - -const mapStateToProps = (state) => ( { - options: !state.activityLog.users ? ["Loading"] : state.activityLog.users -}); - - -export default connect(mapStateToProps)(RemoteFilter); - -// export default RemoteFilter; diff --git a/assets/js/common/RemoteFilter/RemoteFilter.stories.jsx b/assets/js/common/RemoteFilter/RemoteFilter.stories.jsx deleted file mode 100644 index 5f649930b4..0000000000 --- a/assets/js/common/RemoteFilter/RemoteFilter.stories.jsx +++ /dev/null @@ -1,89 +0,0 @@ -import React, { useState } from 'react'; -import { action } from '@storybook/addon-actions'; - -import RemoteFilter from '.'; - -export default { - title: 'Components/Filter', - component: RemoteFilter, - argTypes: { - options: { - type: { name: 'array', required: true }, - description: 'List of options available', - control: { type: 'object' }, - }, - title: { - type: { name: 'string', required: true }, - description: - 'Title of the filter, will appear as placeholder when no value is selected', - control: { type: 'text' }, - }, - value: { - type: { name: 'array', required: false, defaultValue: [] }, - description: 'Selected options', - control: { type: 'object' }, - }, - onChange: { - type: { name: 'function', required: false }, - description: 'Function to call when the selected options change', - control: { type: null }, - }, - }, - render: (args) => { - const [value, setValue] = useState(args.value); - - return ( - { - setValue(newValue); - action('onChange')(newValue); - }} - /> - ); - }, -}; - -export const Default = { - args: { - title: 'Title', - options: [ - 'Tony Kekw', - 'Chad Carbonara', - 'Chuck Amatriciana', - 'K.C.O. Pepe', - 'Virginia Gricia', - ], - value: [], - }, -}; - -export const WithValue = { - args: { - ...Default.args, - value: ['Tony Kekw'], - }, -}; - -export const WithMultipleValues = { - args: { - ...Default.args, - value: ['Tony Kekw', 'Chad Carbonara'], - }, -}; - -export const WithLabel = { - args: { - ...Default.args, - options: [ - ['tony-kekw', 'Tony Kekw'], - ['chad-carbonara', 'Chad Carbonara'], - ['chuck-amatriciana', 'Chuck Amatriciana'], - ['kco-pepe', 'K.C.O. Pepe'], - ['virginia-gricia', 'Virginia Gricia'], - ], - value: ['tony-kekw'], - }, -}; diff --git a/assets/js/common/RemoteFilter/RemoteFilter.test.jsx b/assets/js/common/RemoteFilter/RemoteFilter.test.jsx deleted file mode 100644 index 8b28e193fd..0000000000 --- a/assets/js/common/RemoteFilter/RemoteFilter.test.jsx +++ /dev/null @@ -1,462 +0,0 @@ -import React, { act } from 'react'; -import { render, screen } from '@testing-library/react'; -import userEvent from '@testing-library/user-event'; -import '@testing-library/jest-dom'; -import RemoteFilter from '.'; - -describe('Filter component', () => { - it('should render the options correctly', async () => { - const user = userEvent.setup(); - - const options = [ - 'John Doe', - 'Jane Smith', - 'Michael Scott', - 'Ella Fitzgerald', - ]; - - render(); - - // options are not rendered initially - options.forEach((option) => { - expect(screen.queryByText(option)).not.toBeInTheDocument(); - }); - - await act(() => - // select a button which text starts with 'Filter ' - // click on the button to open the options - user.click(screen.getByText(/Filter *.../)) - ); - - // assert that the options are rendered - options.forEach((option) => { - expect(screen.getByText(option)).toBeInTheDocument(); - }); - }); - - it('should select an option when clicked', async () => { - const user = userEvent.setup(); - const mockOnChange = jest.fn(); - const options = [ - 'John Doe', - 'Jane Smith', - 'Michael Scott', - 'Ella Fitzgerald', - ]; - - render(); - - await act(() => user.click(screen.getByText(/Filter *.../))); - - await act(() => user.click(screen.getByText('Michael Scott'))); - - expect(mockOnChange).toHaveBeenCalledWith(['Michael Scott']); - }); - - it('should select multiple items', async () => { - const user = userEvent.setup(); - const mockOnChange = jest.fn(); - const options = [ - 'John Doe', - 'Jane Smith', - 'Michael Scott', - 'Ella Fitzgerald', - ]; - - const selectedItem = 'Michael Scott'; - - render( - - ); - - await act(() => user.click(screen.getByText(selectedItem))); - - await act(() => user.click(screen.getByText('Jane Smith'))); - - expect(mockOnChange).toHaveBeenCalledTimes(1); - - expect(mockOnChange).toHaveBeenCalledWith([selectedItem, 'Jane Smith']); - }); - - it('should select with one element if passed as string', async () => { - const user = userEvent.setup(); - const mockOnChange = jest.fn(); - const options = [ - 'John Doe', - 'Jane Smith', - 'Michael Scott', - 'Ella Fitzgerald', - ]; - - const selectedItem = 'Michael Scott'; - - render( - - ); - - await act(() => user.click(screen.getByText(selectedItem))); - - await act(() => user.click(screen.getByText('Jane Smith'))); - - expect(mockOnChange).toHaveBeenCalledTimes(1); - - expect(mockOnChange).toHaveBeenCalledWith([selectedItem, 'Jane Smith']); - }); - - it('should deselect an item on multiple item selected', async () => { - const user = userEvent.setup(); - const mockOnChange = jest.fn(); - const options = [ - 'John Doe', - 'Jane Smith', - 'Michael Scott', - 'Ella Fitzgerald', - ]; - - const selectedItem1 = 'Michael Scott'; - const selectedItem2 = 'Jane Smith'; - - render( - - ); - - const filterText = [selectedItem1, selectedItem2].join(', '); - await act(() => user.click(screen.getByText(filterText))); - - await act(() => user.click(screen.getByText('Jane Smith'))); - - expect(mockOnChange).toHaveBeenCalledTimes(1); - - expect(mockOnChange).toHaveBeenCalledWith([selectedItem1]); - - // the list of items is still visible - options.forEach((option) => { - expect(screen.getByText(option)).toBeInTheDocument(); - }); - }); - - it('should deselect an item on single item selected', async () => { - const user = userEvent.setup(); - const mockOnChange = jest.fn(); - const options = [ - 'John Doe', - 'Jane Smith', - 'Michael Scott', - 'Ella Fitzgerald', - ]; - - const selectedItem = 'Michael Scott'; - - render( - - ); - - await act(() => user.click(screen.getByText(selectedItem))); - - await act(() => user.click(screen.getAllByText(selectedItem)[1])); - - expect(mockOnChange).toHaveBeenCalledWith([]); - }); - - it('should use options with label', async () => { - const user = userEvent.setup(); - const mockOnChange = jest.fn(); - const options = [ - ['john-doe', 'John Doe'], - ['jane-smith', 'Jane Smith'], - ['michael-scott', 'Michael Scott'], - ['ella-fitzgerald', 'Ella Fitzgerald'], - ]; - - const selectedItem = 'michael-scott'; - const selectedItemLabel = 'Michael Scott'; - - const anotherSelectedItem = 'jane-smith'; - const anotherSelectedItemLabel = 'Jane Smith'; - - render( - - ); - - await act(() => user.click(screen.getByText(selectedItemLabel))); - - await act(() => { - options.forEach(([, label]) => { - expect(screen.getAllByText(label)[0]).toBeInTheDocument(); - }); - }); - - await act(() => - user.click(screen.getAllByText(anotherSelectedItemLabel)[0]) - ); - - expect(mockOnChange).toHaveBeenCalledWith([ - selectedItem, - anotherSelectedItem, - ]); - }); - - it('should render correctly mixed options', async () => { - const user = userEvent.setup(); - const mockOnChange = jest.fn(); - const options = [ - 'Michael Scott', - undefined, - ['john-doe', 'John Doe'], - 'Jane Smith', - ]; - - render( - - ); - - await act(() => user.click(screen.getByText('Michael Scott'))); - - await act(() => { - expect(screen.getByText('John Doe')).toBeInTheDocument(); - expect(screen.getByText('Jane Smith')).toBeInTheDocument(); - }); - - /* Remember that the component is stateless, - it does not change its internal state on selection. */ - - await act(() => user.click(screen.getByText('John Doe'))); - expect(mockOnChange).toHaveBeenCalledWith(['Michael Scott', 'john-doe']); - - await act(() => user.click(screen.getByText('Jane Smith'))); - expect(mockOnChange).toHaveBeenCalledWith(['Michael Scott', 'Jane Smith']); - }); - - it('should ignore undefined values', async () => { - const user = userEvent.setup(); - const mockOnChange = jest.fn(); - const options = [ - 'John Doe', - 'Jane Smith', - 'Michael Scott', - 'Ella Fitzgerald', - ]; - - render( - - ); - - await act(() => user.click(screen.getByText('Michael Scott'))); - - await act(() => { - options.forEach((label) => { - expect(screen.getAllByText(label)[0]).toBeInTheDocument(); - }); - }); - }); - - it('should ignore undefined options', async () => { - const user = userEvent.setup(); - const mockOnChange = jest.fn(); - const options = [ - '5E8', - undefined, - '32S', - '4B9', - 'MF5', - 'PRD', - 'QAS', - 'HA1', - 'HA2', - ]; - const value = ['PRD']; - - render( - - ); - - await act(() => user.click(screen.getByText('PRD'))); - - await act(() => { - options.filter(Boolean).forEach((label) => { - expect(screen.getAllByText(label)[0]).toBeInTheDocument(); - }); - }); - }); - - it('should use options with label', async () => { - const user = userEvent.setup(); - const mockOnChange = jest.fn(); - const options = [ - ['john-doe', 'John Doe'], - ['jane-smith', 'Jane Smith'], - ['michael-scott', 'Michael Scott'], - ['ella-fitzgerald', 'Ella Fitzgerald'], - ]; - - const selectedItem = 'michael-scott'; - const selectedItemLabel = 'Michael Scott'; - - const anotherSelectedItem = 'jane-smith'; - const anotherSelectedItemLabel = 'Jane Smith'; - - render( - - ); - - await act(() => user.click(screen.getByText(selectedItemLabel))); - - await act(() => { - options.forEach(([, label]) => { - expect(screen.getAllByText(label)[0]).toBeInTheDocument(); - }); - }); - - await act(() => - user.click(screen.getAllByText(anotherSelectedItemLabel)[0]) - ); - - expect(mockOnChange).toHaveBeenCalledWith([ - selectedItem, - anotherSelectedItem, - ]); - }); - - it('should render correctly mixed options', async () => { - const user = userEvent.setup(); - const mockOnChange = jest.fn(); - const options = [ - 'Michael Scott', - undefined, - ['john-doe', 'John Doe'], - 'Jane Smith', - ]; - - render( - - ); - - await act(() => user.click(screen.getByText('Michael Scott'))); - - await act(() => { - expect(screen.getByText('John Doe')).toBeInTheDocument(); - expect(screen.getByText('Jane Smith')).toBeInTheDocument(); - }); - - /* Remember that the component is stateless, - it does not change its internal state on selection. */ - - await act(() => user.click(screen.getByText('John Doe'))); - expect(mockOnChange).toHaveBeenCalledWith(['Michael Scott', 'john-doe']); - - await act(() => user.click(screen.getByText('Jane Smith'))); - expect(mockOnChange).toHaveBeenCalledWith(['Michael Scott', 'Jane Smith']); - }); - - it('should ignore undefined values', async () => { - const user = userEvent.setup(); - const mockOnChange = jest.fn(); - const options = [ - 'John Doe', - 'Jane Smith', - 'Michael Scott', - 'Ella Fitzgerald', - ]; - - render( - - ); - - await act(() => user.click(screen.getByText('Michael Scott'))); - - await act(() => { - options.forEach((label) => { - expect(screen.getAllByText(label)[0]).toBeInTheDocument(); - }); - }); - }); - - it('should ignore undefined options', async () => { - const user = userEvent.setup(); - const mockOnChange = jest.fn(); - const options = [ - '5E8', - undefined, - '32S', - '4B9', - 'MF5', - 'PRD', - 'QAS', - 'HA1', - 'HA2', - ]; - const value = ['PRD']; - - render( - - ); - - await act(() => user.click(screen.getByText('PRD'))); - - await act(() => { - options.filter(Boolean).forEach((label) => { - expect(screen.getAllByText(label)[0]).toBeInTheDocument(); - }); - }); - }); -}); diff --git a/assets/js/common/RemoteFilter/index.js b/assets/js/common/RemoteFilter/index.js deleted file mode 100644 index 010f7ba04d..0000000000 --- a/assets/js/common/RemoteFilter/index.js +++ /dev/null @@ -1,3 +0,0 @@ -import RemoteFilter from './RemoteFilter'; - -export default RemoteFilter; diff --git a/assets/js/pages/ActivityLogPage/ActivityLogPage.jsx b/assets/js/pages/ActivityLogPage/ActivityLogPage.jsx index 28db00efa0..7190e89b66 100644 --- a/assets/js/pages/ActivityLogPage/ActivityLogPage.jsx +++ b/assets/js/pages/ActivityLogPage/ActivityLogPage.jsx @@ -10,8 +10,10 @@ import { allowedActivities } from '@lib/model/activityLog'; import { getUserProfile } from '@state/selectors/user'; import PageHeader from '@common/PageHeader'; +import { useSelector } from 'react-redux'; import ActivityLogOverview from '@common/ActivityLogOverview'; import ComposedFilter from '@common/ComposedFilter'; +import { getAlUsers } from '@state/selectors/activityLog'; import { filterValueToSearchParams, @@ -20,6 +22,7 @@ import { } from './searchParams'; function ActivityLogPage() { + const users = useSelector(getAlUsers); const filters = [ { key: 'type', @@ -32,8 +35,9 @@ function ActivityLogPage() { }, { key: 'actor', - type: 'remoteselect', + type: 'select', title: 'User', + options: users, }, { key: 'to_date', diff --git a/assets/js/state/selectors/activityLog.js b/assets/js/state/selectors/activityLog.js new file mode 100644 index 0000000000..4382b8d7ce --- /dev/null +++ b/assets/js/state/selectors/activityLog.js @@ -0,0 +1,7 @@ +import { createSelector } from '@reduxjs/toolkit'; + + +export const getAlUsers = createSelector( + [(state) => state.activityLog.users], + (users) => users +); From 27127ff9944ee256260deb89295a7ba344d4c38c Mon Sep 17 00:00:00 2001 From: Gagandeep Bhatia Date: Fri, 23 Aug 2024 16:36:11 +0200 Subject: [PATCH 03/31] More code deletion; Adds user context function - Deletes unnecessary code from the composed filter - Adds a new function the users context - Adds needed context calls to the ActivityLog channel module --- assets/js/common/ComposedFilter/ComposedFilter.jsx | 6 ------ lib/trento/users.ex | 9 +++++++++ lib/trento_web/channels/activity_log_channel.ex | 7 ++++--- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/assets/js/common/ComposedFilter/ComposedFilter.jsx b/assets/js/common/ComposedFilter/ComposedFilter.jsx index f5e3e93cfc..1e9babb17c 100644 --- a/assets/js/common/ComposedFilter/ComposedFilter.jsx +++ b/assets/js/common/ComposedFilter/ComposedFilter.jsx @@ -1,16 +1,10 @@ import React, { useState } from 'react'; import Button from '@common/Button'; import Filter from '@common/Filter'; -import RemoteFilter from '@common/RemoteFilter'; import DateFilter from '@common/DateFilter'; const renderFilter = (key, { type, ...filterProps }, value, onChange) => { switch (type) { - case 'remoteselect': - return ( - - ); - case 'select': return ( diff --git a/lib/trento/users.ex b/lib/trento/users.ex index bb12353c4b..d51b6d8575 100644 --- a/lib/trento/users.ex +++ b/lib/trento/users.ex @@ -37,6 +37,15 @@ defmodule Trento.Users do |> Repo.all() end + @doc """ + Returns all usernames, including those for users that are soft-deleted. + """ + def list_all_usernames do + User + |> select([u], u.username) + |> Repo.all() + end + def get_user(id) do case User |> where([u], is_nil(u.deleted_at) and u.id == ^id) diff --git a/lib/trento_web/channels/activity_log_channel.ex b/lib/trento_web/channels/activity_log_channel.ex index 32bb954ef6..6e6511fb0a 100644 --- a/lib/trento_web/channels/activity_log_channel.ex +++ b/lib/trento_web/channels/activity_log_channel.ex @@ -5,6 +5,7 @@ defmodule TrentoWeb.ActivityLogChannel do """ require Logger use TrentoWeb, :channel + alias Trento.Users @impl true def join( @@ -32,9 +33,9 @@ defmodule TrentoWeb.ActivityLogChannel do @impl true def handle_info(:after_join, socket) do - Logger.warning("updates users list") - push(socket, "al_users_pushed", %{users: ["foo", "bar", "baz", "admin"]}) - Process.send_after(self(), :after_join, 5000) + users = Users.list_all_usernames() + push(socket, "al_users_pushed", %{users: users}) + Process.send_after(self(), :after_join, 60_000) {:noreply, socket} end end From 91634fde48fefc2fe39983efdd1c55f5fea3cbe9 Mon Sep 17 00:00:00 2001 From: Gagandeep Bhatia Date: Mon, 26 Aug 2024 09:24:11 +0200 Subject: [PATCH 04/31] Renaming Renames some declarations --- assets/js/state/sagas/activityLog.js | 6 +++--- assets/js/state/sagas/index.js | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/assets/js/state/sagas/activityLog.js b/assets/js/state/sagas/activityLog.js index 52eaebee87..4aa1216d13 100644 --- a/assets/js/state/sagas/activityLog.js +++ b/assets/js/state/sagas/activityLog.js @@ -4,11 +4,11 @@ import { AL_USERS_PUSHED } from '@state/activityLog'; -export function* alUsersUpdate({payload: {users}}) { +export function* activityLogUsersUpdate({payload: {users}}) { yield put(setUsers({users})) } -export function* watchAlActions() { - yield takeEvery(AL_USERS_PUSHED, alUsersUpdate); +export function* watchActivityLogActions() { + yield takeEvery(AL_USERS_PUSHED, activityLogUsersUpdate); } diff --git a/assets/js/state/sagas/index.js b/assets/js/state/sagas/index.js index 8683edb24d..de70adf3c1 100644 --- a/assets/js/state/sagas/index.js +++ b/assets/js/state/sagas/index.js @@ -80,7 +80,7 @@ import { watchActivityLogsSettings } from '@state/sagas/activityLogsSettings'; import { watchSoftwareUpdates } from '@state/sagas/softwareUpdates'; import { watchSocketEvents } from '@state/sagas/channels'; -import { watchAlActions } from '@state/sagas/activityLog'; +import { watchActivityLogActions } from '@state/sagas/activityLog'; import { checkApiKeyExpiration } from '@state/sagas/settings'; const RESET_STATE = 'RESET_STATE'; @@ -250,6 +250,6 @@ export default function* rootSaga() { watchUserLoggedIn(), watchActivityLogsSettings(), watchSoftwareUpdates(), - watchAlActions(), + watchActivityLogActions(), ]); } From 7f0b2be7bfcf54c483be5dba9a9bef231dfd40af Mon Sep 17 00:00:00 2001 From: Gagandeep Bhatia Date: Mon, 26 Aug 2024 11:05:50 +0200 Subject: [PATCH 05/31] Refactors activity_log channel --- lib/trento_web/channels/activity_log_channel.ex | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/trento_web/channels/activity_log_channel.ex b/lib/trento_web/channels/activity_log_channel.ex index 6e6511fb0a..487a4e3822 100644 --- a/lib/trento_web/channels/activity_log_channel.ex +++ b/lib/trento_web/channels/activity_log_channel.ex @@ -13,14 +13,12 @@ defmodule TrentoWeb.ActivityLogChannel do _payload, %{assigns: %{current_user_id: current_user_id}} = socket ) do - user_id = String.to_integer(user_id) - - if user_id == current_user_id do + if allowed?(user_id, current_user_id) do send(self(), :after_join) {:ok, socket} else Logger.error( - "Could not join user channel, requested user id: #{user_id}, authenticated user id: #{current_user_id}" + "Could not join activity_log channel, requested user id: #{user_id}, authenticated user id: #{current_user_id}" ) {:error, :unauthorized} @@ -38,4 +36,6 @@ defmodule TrentoWeb.ActivityLogChannel do Process.send_after(self(), :after_join, 60_000) {:noreply, socket} end + + defp allowed?(user_id, current_user_id), do: String.to_integer(user_id) == current_user_id end From ca1ad1790258fedbc313d8c92aedec6c78563c59 Mon Sep 17 00:00:00 2001 From: Gagandeep Bhatia Date: Mon, 26 Aug 2024 15:38:34 +0200 Subject: [PATCH 06/31] Fixes warning Fix for identityFunctionCheck warning in console. --- assets/js/state/selectors/activityLog.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/assets/js/state/selectors/activityLog.js b/assets/js/state/selectors/activityLog.js index 4382b8d7ce..1741945b4f 100644 --- a/assets/js/state/selectors/activityLog.js +++ b/assets/js/state/selectors/activityLog.js @@ -2,6 +2,6 @@ import { createSelector } from '@reduxjs/toolkit'; export const getAlUsers = createSelector( - [(state) => state.activityLog.users], - (users) => users + [(state) => state.activityLog], + (activityLog) => activityLog.users ); From c7e768ae3d79438fd8f9fd6bc826e4175c45ea53 Mon Sep 17 00:00:00 2001 From: Gagandeep Bhatia Date: Mon, 26 Aug 2024 15:42:39 +0200 Subject: [PATCH 07/31] Runs formatter --- assets/js/state/activityLog.js | 11 ++++++----- assets/js/state/sagas/activityLog.js | 12 ++++-------- assets/js/state/sagas/channels.js | 10 +++++++--- assets/js/state/selectors/activityLog.js | 1 - 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/assets/js/state/activityLog.js b/assets/js/state/activityLog.js index 5c1861de4e..83e3f9cd29 100644 --- a/assets/js/state/activityLog.js +++ b/assets/js/state/activityLog.js @@ -1,22 +1,23 @@ import { createAction, createSlice } from '@reduxjs/toolkit'; export const initialState = { - users: [] + users: [], }; export const activityLogSlice = createSlice({ name: 'activityLog', initialState, reducers: { - setUsers( state, { payload: { users }, }) { + setUsers(state, { payload: { users } }) { state.users = users; }, }, }); export const AL_USERS_PUSHED = 'AL_USERS_PUSHED'; -export const alUsersPushed = createAction(AL_USERS_PUSHED, ({users}) => ({payload: { users}})); -export const { setUsers } = - activityLogSlice.actions; +export const alUsersPushed = createAction(AL_USERS_PUSHED, ({ users }) => ({ + payload: { users }, +})); +export const { setUsers } = activityLogSlice.actions; export default activityLogSlice.reducer; diff --git a/assets/js/state/sagas/activityLog.js b/assets/js/state/sagas/activityLog.js index 4aa1216d13..d9e48b38fe 100644 --- a/assets/js/state/sagas/activityLog.js +++ b/assets/js/state/sagas/activityLog.js @@ -1,14 +1,10 @@ -import { put, takeEvery } from 'redux-saga/effects'; -import { - setUsers, - AL_USERS_PUSHED -} from '@state/activityLog'; +import { put, takeEvery } from 'redux-saga/effects'; +import { setUsers, AL_USERS_PUSHED } from '@state/activityLog'; -export function* activityLogUsersUpdate({payload: {users}}) { - yield put(setUsers({users})) +export function* activityLogUsersUpdate({ payload: { users } }) { + yield put(setUsers({ users })); } export function* watchActivityLogActions() { yield takeEvery(AL_USERS_PUSHED, activityLogUsersUpdate); - } diff --git a/assets/js/state/sagas/channels.js b/assets/js/state/sagas/channels.js index bea014902d..f3f3320b6e 100644 --- a/assets/js/state/sagas/channels.js +++ b/assets/js/state/sagas/channels.js @@ -241,10 +241,9 @@ const activityLogEvents = [ { name: 'al_users_pushed', action: alUsersPushed, - } + }, ]; - const createEventChannel = (channel, events) => eventChannel((emitter) => { events.forEach((event) => { @@ -290,6 +289,11 @@ export function* watchSocketEvents(socket) { fork(watchChannelEvents, socket, 'monitoring:databases', databaseEvents), fork(watchChannelEvents, socket, 'monitoring:executions', executionEvents), fork(watchChannelEvents, socket, `users:${userID}`, userEvents), - fork(watchChannelEvents, socket, `activity_log:${userID}`, activityLogEvents), + fork( + watchChannelEvents, + socket, + `activity_log:${userID}`, + activityLogEvents + ), ]); } diff --git a/assets/js/state/selectors/activityLog.js b/assets/js/state/selectors/activityLog.js index 1741945b4f..957ef65188 100644 --- a/assets/js/state/selectors/activityLog.js +++ b/assets/js/state/selectors/activityLog.js @@ -1,6 +1,5 @@ import { createSelector } from '@reduxjs/toolkit'; - export const getAlUsers = createSelector( [(state) => state.activityLog], (activityLog) => activityLog.users From e7b39c1528e23a570830931b8bd9480bdbf52522 Mon Sep 17 00:00:00 2001 From: Gagandeep Bhatia Date: Mon, 26 Aug 2024 17:07:37 +0200 Subject: [PATCH 08/31] Renaming --- assets/js/pages/ActivityLogPage/ActivityLogPage.jsx | 4 ++-- assets/js/state/selectors/activityLog.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/assets/js/pages/ActivityLogPage/ActivityLogPage.jsx b/assets/js/pages/ActivityLogPage/ActivityLogPage.jsx index 7190e89b66..f848863344 100644 --- a/assets/js/pages/ActivityLogPage/ActivityLogPage.jsx +++ b/assets/js/pages/ActivityLogPage/ActivityLogPage.jsx @@ -13,7 +13,7 @@ import PageHeader from '@common/PageHeader'; import { useSelector } from 'react-redux'; import ActivityLogOverview from '@common/ActivityLogOverview'; import ComposedFilter from '@common/ComposedFilter'; -import { getAlUsers } from '@state/selectors/activityLog'; +import { getALUsers } from '@state/selectors/activityLog'; import { filterValueToSearchParams, @@ -22,7 +22,7 @@ import { } from './searchParams'; function ActivityLogPage() { - const users = useSelector(getAlUsers); + const users = useSelector(getALUsers); const filters = [ { key: 'type', diff --git a/assets/js/state/selectors/activityLog.js b/assets/js/state/selectors/activityLog.js index 957ef65188..e1aeb4a6a3 100644 --- a/assets/js/state/selectors/activityLog.js +++ b/assets/js/state/selectors/activityLog.js @@ -1,6 +1,6 @@ import { createSelector } from '@reduxjs/toolkit'; -export const getAlUsers = createSelector( +export const getALUsers = createSelector( [(state) => state.activityLog], (activityLog) => activityLog.users ); From fbd3cbc2ca271c5d17cad34bce48db40185e6f1e Mon Sep 17 00:00:00 2001 From: Gagandeep Bhatia Date: Wed, 28 Aug 2024 14:12:29 +0200 Subject: [PATCH 09/31] Adds some tests - channel test for activity log - activity log state test --- assets/js/state/activityLog.test.js | 19 +++++++++++++++++++ assets/js/state/sagas/channels.test.js | 16 +++++++++++++++- 2 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 assets/js/state/activityLog.test.js diff --git a/assets/js/state/activityLog.test.js b/assets/js/state/activityLog.test.js new file mode 100644 index 0000000000..d24ee89d99 --- /dev/null +++ b/assets/js/state/activityLog.test.js @@ -0,0 +1,19 @@ +import { userFactory } from '@lib/test-utils/factories/users'; + +import activityLogReducer, { + setUsers, + initialState, + // alUsersPushed, +} from './activityLog'; + +describe('activityLog reducer', () => { + it('should set the users for activity log when setUsers is dispatched', () => { + const {username: username1} = userFactory.build(); + const {username: username2} = userFactory.build(); + const users = [username1, username2]; + const action = setUsers({users}); + expect(activityLogReducer(initialState, action)).toEqual({ + users + }); + }); +}); diff --git a/assets/js/state/sagas/channels.test.js b/assets/js/state/sagas/channels.test.js index b6bf070cdf..78122f6b68 100644 --- a/assets/js/state/sagas/channels.test.js +++ b/assets/js/state/sagas/channels.test.js @@ -6,6 +6,7 @@ import { setExecutionStarted, } from '@state/lastExecutions'; import { userUpdated } from '@state/user'; +import { alUsersPushed } from '@state/activityLog'; import { watchSocketEvents } from './channels'; @@ -48,6 +49,7 @@ const channels = { 'monitoring:databases': new MockChannel(), 'monitoring:executions': new MockChannel(), [`users:${USER_ID}`]: new MockChannel(), + [`activity_log:${USER_ID}`]: new MockChannel(), }; const mockSocket = { @@ -125,4 +127,16 @@ describe('Channels saga', () => { expect(dispatched).toEqual([userUpdated({ email: 'new@email.com' })]); }); -}); + it('should listen to specific activity log events', async () => { + const { saga, dispatched } = runWatchSocketEventsSaga(mockSocket); + + channels[`activity_log:${USER_ID}`].emit('al_users_pushed', { + users: ["user1", "user2", "user3"], + }); + + closeSocket(); + await saga; + + expect(dispatched).toEqual([alUsersPushed({ users: ["user1", "user2", "user3"] })]); + }); + }); From 63b4d0ca3e9b1737d4e323e8274d0ee7a80843f1 Mon Sep 17 00:00:00 2001 From: Gagandeep Bhatia Date: Wed, 28 Aug 2024 14:16:33 +0200 Subject: [PATCH 10/31] Runs js code formatter --- assets/js/state/activityLog.test.js | 8 ++++---- assets/js/state/sagas/channels.test.js | 18 ++++++++++-------- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/assets/js/state/activityLog.test.js b/assets/js/state/activityLog.test.js index d24ee89d99..55fd9360d2 100644 --- a/assets/js/state/activityLog.test.js +++ b/assets/js/state/activityLog.test.js @@ -8,12 +8,12 @@ import activityLogReducer, { describe('activityLog reducer', () => { it('should set the users for activity log when setUsers is dispatched', () => { - const {username: username1} = userFactory.build(); - const {username: username2} = userFactory.build(); + const { username: username1 } = userFactory.build(); + const { username: username2 } = userFactory.build(); const users = [username1, username2]; - const action = setUsers({users}); + const action = setUsers({ users }); expect(activityLogReducer(initialState, action)).toEqual({ - users + users, }); }); }); diff --git a/assets/js/state/sagas/channels.test.js b/assets/js/state/sagas/channels.test.js index 78122f6b68..acb8ee2bd2 100644 --- a/assets/js/state/sagas/channels.test.js +++ b/assets/js/state/sagas/channels.test.js @@ -128,15 +128,17 @@ describe('Channels saga', () => { expect(dispatched).toEqual([userUpdated({ email: 'new@email.com' })]); }); it('should listen to specific activity log events', async () => { - const { saga, dispatched } = runWatchSocketEventsSaga(mockSocket); + const { saga, dispatched } = runWatchSocketEventsSaga(mockSocket); - channels[`activity_log:${USER_ID}`].emit('al_users_pushed', { - users: ["user1", "user2", "user3"], - }); + channels[`activity_log:${USER_ID}`].emit('al_users_pushed', { + users: ['user1', 'user2', 'user3'], + }); - closeSocket(); - await saga; + closeSocket(); + await saga; - expect(dispatched).toEqual([alUsersPushed({ users: ["user1", "user2", "user3"] })]); - }); + expect(dispatched).toEqual([ + alUsersPushed({ users: ['user1', 'user2', 'user3'] }), + ]); }); +}); From acd65cbd7c5f347c8eb711702c5dbf860f416d32 Mon Sep 17 00:00:00 2001 From: Gagandeep Bhatia Date: Wed, 28 Aug 2024 17:27:11 +0200 Subject: [PATCH 11/31] Fix for failing tests Adds a hook that passes in requisite state to the ActivityLogPage component --- assets/js/lib/test-utils/index.jsx | 3 +++ assets/js/pages/ActivityLogPage/ActivityLogPage.test.jsx | 9 ++++----- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/assets/js/lib/test-utils/index.jsx b/assets/js/lib/test-utils/index.jsx index a427ea6bcc..5aaaff9895 100644 --- a/assets/js/lib/test-utils/index.jsx +++ b/assets/js/lib/test-utils/index.jsx @@ -16,6 +16,9 @@ const middlewares = []; const mockStore = configureStore(middlewares); export const defaultInitialState = { + activityLog: { + users: ['user1', 'user2'], + }, user: { abilities: [{ name: 'all', resource: 'all' }], }, diff --git a/assets/js/pages/ActivityLogPage/ActivityLogPage.test.jsx b/assets/js/pages/ActivityLogPage/ActivityLogPage.test.jsx index 60b1c1a849..ee72ad015a 100644 --- a/assets/js/pages/ActivityLogPage/ActivityLogPage.test.jsx +++ b/assets/js/pages/ActivityLogPage/ActivityLogPage.test.jsx @@ -1,12 +1,11 @@ -import React, { act } from 'react'; -import { screen } from '@testing-library/react'; import '@testing-library/jest-dom'; -import MockAdapter from 'axios-mock-adapter'; -import { renderWithRouter, withDefaultState } from '@lib/test-utils'; - import { networkClient } from '@lib/network'; +import { renderWithRouter, withDefaultState } from '@lib/test-utils'; import { activityLogEntryFactory } from '@lib/test-utils/factories/activityLog'; +import { screen } from '@testing-library/react'; +import MockAdapter from 'axios-mock-adapter'; +import React, { act } from 'react'; import ActivityLogPage from './ActivityLogPage'; From e291eb2eb7120cb1df421e9c75902fbf42be028f Mon Sep 17 00:00:00 2001 From: Gagandeep Bhatia Date: Wed, 28 Aug 2024 17:29:50 +0200 Subject: [PATCH 12/31] Deletes an unused import; format --- assets/js/state/activityLog.test.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/assets/js/state/activityLog.test.js b/assets/js/state/activityLog.test.js index 55fd9360d2..bd9468c10e 100644 --- a/assets/js/state/activityLog.test.js +++ b/assets/js/state/activityLog.test.js @@ -1,10 +1,6 @@ import { userFactory } from '@lib/test-utils/factories/users'; -import activityLogReducer, { - setUsers, - initialState, - // alUsersPushed, -} from './activityLog'; +import activityLogReducer, { setUsers, initialState } from './activityLog'; describe('activityLog reducer', () => { it('should set the users for activity log when setUsers is dispatched', () => { From a4818246bc55409abecd9d3532888e539dbe8e0c Mon Sep 17 00:00:00 2001 From: Gagandeep Bhatia Date: Fri, 30 Aug 2024 14:24:00 +0200 Subject: [PATCH 13/31] Some cleanup Specific users in list are not necessary --- assets/js/lib/test-utils/index.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assets/js/lib/test-utils/index.jsx b/assets/js/lib/test-utils/index.jsx index 5aaaff9895..1cb8f6ffc9 100644 --- a/assets/js/lib/test-utils/index.jsx +++ b/assets/js/lib/test-utils/index.jsx @@ -17,7 +17,7 @@ const mockStore = configureStore(middlewares); export const defaultInitialState = { activityLog: { - users: ['user1', 'user2'], + users: [], }, user: { abilities: [{ name: 'all', resource: 'all' }], From d4ff429b7cb79954c10146e8a12d2a532ae339f5 Mon Sep 17 00:00:00 2001 From: Gagandeep Bhatia Date: Mon, 2 Sep 2024 10:16:32 +0200 Subject: [PATCH 14/31] Adds a system user Previously a system user was not returned; now it is possible to filter the system activity log entries. --- lib/trento_web/channels/activity_log_channel.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/trento_web/channels/activity_log_channel.ex b/lib/trento_web/channels/activity_log_channel.ex index 487a4e3822..fe2a13eba8 100644 --- a/lib/trento_web/channels/activity_log_channel.ex +++ b/lib/trento_web/channels/activity_log_channel.ex @@ -31,7 +31,7 @@ defmodule TrentoWeb.ActivityLogChannel do @impl true def handle_info(:after_join, socket) do - users = Users.list_all_usernames() + users = ["system" | Users.list_all_usernames()] push(socket, "al_users_pushed", %{users: users}) Process.send_after(self(), :after_join, 60_000) {:noreply, socket} From 8562e4d3b55f98ee939d88ae8d647e297492750f Mon Sep 17 00:00:00 2001 From: Gagandeep Bhatia Date: Thu, 5 Sep 2024 15:00:25 +0200 Subject: [PATCH 15/31] Refactors ActivityLogPage test --- assets/js/pages/ActivityLogPage/ActivityLogPage.test.jsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/assets/js/pages/ActivityLogPage/ActivityLogPage.test.jsx b/assets/js/pages/ActivityLogPage/ActivityLogPage.test.jsx index ee72ad015a..cd0c60d98e 100644 --- a/assets/js/pages/ActivityLogPage/ActivityLogPage.test.jsx +++ b/assets/js/pages/ActivityLogPage/ActivityLogPage.test.jsx @@ -1,5 +1,11 @@ import '@testing-library/jest-dom'; +<<<<<<< HEAD +======= +import MockAdapter from 'axios-mock-adapter'; +import { withDefaultState, renderWithRouter } from '@lib/test-utils'; + +>>>>>>> d9845ca54 (Refactors ActivityLogPage test) import { networkClient } from '@lib/network'; import { renderWithRouter, withDefaultState } from '@lib/test-utils'; import { activityLogEntryFactory } from '@lib/test-utils/factories/activityLog'; @@ -55,7 +61,6 @@ describe('ActivityLogPage', () => { const { container } = await act(() => renderWithRouter(StatefulActivityLogPage) ); - expect(container.querySelectorAll('tbody > tr')).toHaveLength(5); }); }); From 03f5a8b79cbbdcff1096e363edfb9b70cf27ee49 Mon Sep 17 00:00:00 2001 From: Gagandeep Bhatia Date: Thu, 5 Sep 2024 15:15:12 +0200 Subject: [PATCH 16/31] Adds test for activityLog users selector --- assets/js/state/selectors/activityLog.test.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 assets/js/state/selectors/activityLog.test.js diff --git a/assets/js/state/selectors/activityLog.test.js b/assets/js/state/selectors/activityLog.test.js new file mode 100644 index 0000000000..5618c7029a --- /dev/null +++ b/assets/js/state/selectors/activityLog.test.js @@ -0,0 +1,12 @@ +import { getALUsers } from './activityLog'; + +describe('Activity Log users selector', () => { + it('should return a list of users from activity log state', () => { + const users = ['user1', 'user2', 'user3']; + const state = { + activityLog: { users }, + }; + + expect(getALUsers(state)).toEqual(users); + }); +}); From 800c90d9c7988b7203b1e66a5995ca39bd3fca97 Mon Sep 17 00:00:00 2001 From: Gagandeep Bhatia Date: Fri, 6 Sep 2024 12:30:52 +0200 Subject: [PATCH 17/31] Adds tests for ActivityLogChannel --- .../channels/activity_log_channel_test.exs | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 test/trento_web/channels/activity_log_channel_test.exs diff --git a/test/trento_web/channels/activity_log_channel_test.exs b/test/trento_web/channels/activity_log_channel_test.exs new file mode 100644 index 0000000000..770b1578a2 --- /dev/null +++ b/test/trento_web/channels/activity_log_channel_test.exs @@ -0,0 +1,26 @@ +defmodule TrentoWeb.ActivityLogChannelTest do + use TrentoWeb.ChannelCase + + test "Socket users can only join the activity log channel" do + assert {:ok, _, _socket} = + TrentoWeb.UserSocket + |> socket("user_id", %{current_user_id: 876}) + |> join(TrentoWeb.ActivityLogChannel, "activity_log:876") + + assert_push("al_users_pushed", %{users: _}) + end + + test "Unauthorized users cannot join the activity log channel" do + assert {:error, :unauthorized} = + TrentoWeb.UserSocket + |> socket("user_id", %{current_user_id: 788}) + |> join(TrentoWeb.ActivityLogChannel, "activity_log:876") + end + + test "Non logged users cannot join an activity log channel" do + assert {:error, :user_not_logged} = + TrentoWeb.UserSocket + |> socket("user_id", %{}) + |> join(TrentoWeb.ActivityLogChannel, "activity_log:8989") + end +end From 0b29e6498ee1c826c0828a40e0a1e5ca451a95d2 Mon Sep 17 00:00:00 2001 From: Gagandeep Bhatia Date: Fri, 6 Sep 2024 13:45:30 +0200 Subject: [PATCH 18/31] Adds a test for a Users context function --- test/trento/users_test.exs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/trento/users_test.exs b/test/trento/users_test.exs index 7f1f69ab78..3a687bdb5d 100644 --- a/test/trento/users_test.exs +++ b/test/trento/users_test.exs @@ -117,6 +117,17 @@ defmodule Trento.UsersTest do assert length(users) == 1 end + test "list_all_usernames returns all usernames including those for deleted users" do + %{username: username1} = insert(:user) + %{username: username2} = insert(:user, deleted_at: DateTime.utc_now()) + inserted_sorted_usernames = Enum.sort([username1, username2]) + sorted_usernames = Enum.sort(Users.list_all_usernames()) + + assert inserted_sorted_usernames == sorted_usernames + assert length(sorted_usernames) == 2 + assert length(inserted_sorted_usernames) == 2 + end + test "get_user returns a user when the user exist" do %{id: user_id} = insert(:user) From 2f3f066e2617b4d5e2350d2a022b159af36764a8 Mon Sep 17 00:00:00 2001 From: Gagandeep Bhatia Date: Mon, 9 Sep 2024 13:44:01 +0200 Subject: [PATCH 19/31] Addresses PR comments Renaming --- .../pages/ActivityLogPage/ActivityLogPage.test.jsx | 12 ++++++------ assets/js/state/activityLog.js | 6 ++---- assets/js/state/sagas/activityLog.js | 4 ++-- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/assets/js/pages/ActivityLogPage/ActivityLogPage.test.jsx b/assets/js/pages/ActivityLogPage/ActivityLogPage.test.jsx index cd0c60d98e..22aa56ab8f 100644 --- a/assets/js/pages/ActivityLogPage/ActivityLogPage.test.jsx +++ b/assets/js/pages/ActivityLogPage/ActivityLogPage.test.jsx @@ -20,8 +20,8 @@ const axiosMock = new MockAdapter(networkClient); describe('ActivityLogPage', () => { it('should render table without data', async () => { axiosMock.onGet('/api/v1/activity_log').reply(200, { data: [] }); - const [StatefulActivityLogPage] = withDefaultState(); - await act(async () => renderWithRouter(StatefulActivityLogPage)); + const [StatefulActivityLogPage, _] = withDefaultState(); + await act(() => renderWithRouter(StatefulActivityLogPage)); expect(screen.getByText('No data available')).toBeVisible(); }); @@ -44,7 +44,9 @@ describe('ActivityLogPage', () => { axiosMock .onGet('/api/v1/activity_log') .reply(responseStatus, responseBody); - const [StatefulActivityLogPage] = withDefaultState(); + const [StatefulActivityLogPage, _] = withDefaultState( + + ); await act(() => renderWithRouter(StatefulActivityLogPage)); expect(screen.getByText('No data available')).toBeVisible(); @@ -55,9 +57,7 @@ describe('ActivityLogPage', () => { axiosMock .onGet('/api/v1/activity_log') .reply(200, { data: activityLogEntryFactory.buildList(5) }); - - const [StatefulActivityLogPage] = withDefaultState(); - + const [StatefulActivityLogPage, _] = withDefaultState(); const { container } = await act(() => renderWithRouter(StatefulActivityLogPage) ); diff --git a/assets/js/state/activityLog.js b/assets/js/state/activityLog.js index 83e3f9cd29..4792ea10bc 100644 --- a/assets/js/state/activityLog.js +++ b/assets/js/state/activityLog.js @@ -14,10 +14,8 @@ export const activityLogSlice = createSlice({ }, }); -export const AL_USERS_PUSHED = 'AL_USERS_PUSHED'; -export const alUsersPushed = createAction(AL_USERS_PUSHED, ({ users }) => ({ - payload: { users }, -})); +export const ACTIVITY_LOG_USERS_PUSHED = 'ACTIVITY_LOG_USERS_PUSHED'; +export const alUsersPushed = createAction(ACTIVITY_LOG_USERS_PUSHED); export const { setUsers } = activityLogSlice.actions; export default activityLogSlice.reducer; diff --git a/assets/js/state/sagas/activityLog.js b/assets/js/state/sagas/activityLog.js index d9e48b38fe..83cb159823 100644 --- a/assets/js/state/sagas/activityLog.js +++ b/assets/js/state/sagas/activityLog.js @@ -1,10 +1,10 @@ import { put, takeEvery } from 'redux-saga/effects'; -import { setUsers, AL_USERS_PUSHED } from '@state/activityLog'; +import { setUsers, ACTIVITY_LOG_USERS_PUSHED } from '@state/activityLog'; export function* activityLogUsersUpdate({ payload: { users } }) { yield put(setUsers({ users })); } export function* watchActivityLogActions() { - yield takeEvery(AL_USERS_PUSHED, activityLogUsersUpdate); + yield takeEvery(ACTIVITY_LOG_USERS_PUSHED, activityLogUsersUpdate); } From bca30938f2fe29dcdb6cfa3cd6cc0cbdb4b6ace8 Mon Sep 17 00:00:00 2001 From: Gagandeep Bhatia Date: Mon, 9 Sep 2024 15:01:32 +0200 Subject: [PATCH 20/31] Handles scenario with deleted users Collapses usernames from deleted users into a username without timestamp suffix --- .../channels/activity_log_channel.ex | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/lib/trento_web/channels/activity_log_channel.ex b/lib/trento_web/channels/activity_log_channel.ex index fe2a13eba8..85332c0888 100644 --- a/lib/trento_web/channels/activity_log_channel.ex +++ b/lib/trento_web/channels/activity_log_channel.ex @@ -31,11 +31,33 @@ defmodule TrentoWeb.ActivityLogChannel do @impl true def handle_info(:after_join, socket) do - users = ["system" | Users.list_all_usernames()] + all_usernames = Users.list_all_usernames() + collapsed_usernames = collapse_usernames(all_usernames) + + users = ["system" | collapsed_usernames] push(socket, "al_users_pushed", %{users: users}) Process.send_after(self(), :after_join, 60_000) {:noreply, socket} end defp allowed?(user_id, current_user_id), do: String.to_integer(user_id) == current_user_id + + defp collapse_usernames(usernames) do + usernames + |> Enum.map(fn username -> + maybe_ts = + username + |> String.split("__") + |> List.last() + + case DateTime.from_iso8601(maybe_ts) do + {:ok, _, _} -> + String.trim_trailing(username, "__" <> maybe_ts) + + _ -> + username + end + end) + |> Enum.uniq() + end end From 6e0dbd14e0bf915e54cdc9530b2785494fa060ea Mon Sep 17 00:00:00 2001 From: Gagandeep Bhatia Date: Tue, 10 Sep 2024 11:29:42 +0200 Subject: [PATCH 21/31] Renaming alUsers -> activityLogUsers --- assets/js/pages/ActivityLogPage/ActivityLogPage.jsx | 4 ++-- assets/js/state/activityLog.js | 2 +- assets/js/state/sagas/channels.js | 4 ++-- assets/js/state/sagas/channels.test.js | 4 ++-- assets/js/state/selectors/activityLog.js | 2 +- assets/js/state/selectors/activityLog.test.js | 4 ++-- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/assets/js/pages/ActivityLogPage/ActivityLogPage.jsx b/assets/js/pages/ActivityLogPage/ActivityLogPage.jsx index f848863344..f0c3c6e346 100644 --- a/assets/js/pages/ActivityLogPage/ActivityLogPage.jsx +++ b/assets/js/pages/ActivityLogPage/ActivityLogPage.jsx @@ -13,7 +13,7 @@ import PageHeader from '@common/PageHeader'; import { useSelector } from 'react-redux'; import ActivityLogOverview from '@common/ActivityLogOverview'; import ComposedFilter from '@common/ComposedFilter'; -import { getALUsers } from '@state/selectors/activityLog'; +import { getActivityLogUsers } from '@state/selectors/activityLog'; import { filterValueToSearchParams, @@ -22,7 +22,7 @@ import { } from './searchParams'; function ActivityLogPage() { - const users = useSelector(getALUsers); + const users = useSelector(getActivityLogUsers); const filters = [ { key: 'type', diff --git a/assets/js/state/activityLog.js b/assets/js/state/activityLog.js index 4792ea10bc..b9da15352a 100644 --- a/assets/js/state/activityLog.js +++ b/assets/js/state/activityLog.js @@ -15,7 +15,7 @@ export const activityLogSlice = createSlice({ }); export const ACTIVITY_LOG_USERS_PUSHED = 'ACTIVITY_LOG_USERS_PUSHED'; -export const alUsersPushed = createAction(ACTIVITY_LOG_USERS_PUSHED); +export const activityLogUsersPushed = createAction(ACTIVITY_LOG_USERS_PUSHED); export const { setUsers } = activityLogSlice.actions; export default activityLogSlice.reducer; diff --git a/assets/js/state/sagas/channels.js b/assets/js/state/sagas/channels.js index f3f3320b6e..c0e07511f3 100644 --- a/assets/js/state/sagas/channels.js +++ b/assets/js/state/sagas/channels.js @@ -55,7 +55,7 @@ import { } from '@state/lastExecutions'; import { userUpdated, userLocked, userDeleted } from '@state/user'; -import { alUsersPushed } from '@state/activityLog'; +import { activityLogUsersPushed } from '@state/activityLog'; import { getUserProfile } from '@state/selectors/user'; @@ -240,7 +240,7 @@ const userEvents = [ const activityLogEvents = [ { name: 'al_users_pushed', - action: alUsersPushed, + action: activityLogUsersPushed, }, ]; diff --git a/assets/js/state/sagas/channels.test.js b/assets/js/state/sagas/channels.test.js index acb8ee2bd2..58f81867ff 100644 --- a/assets/js/state/sagas/channels.test.js +++ b/assets/js/state/sagas/channels.test.js @@ -6,7 +6,7 @@ import { setExecutionStarted, } from '@state/lastExecutions'; import { userUpdated } from '@state/user'; -import { alUsersPushed } from '@state/activityLog'; +import { activityLogUsersPushed } from '@state/activityLog'; import { watchSocketEvents } from './channels'; @@ -138,7 +138,7 @@ describe('Channels saga', () => { await saga; expect(dispatched).toEqual([ - alUsersPushed({ users: ['user1', 'user2', 'user3'] }), + activityLogUsersPushed({ users: ['user1', 'user2', 'user3'] }), ]); }); }); diff --git a/assets/js/state/selectors/activityLog.js b/assets/js/state/selectors/activityLog.js index e1aeb4a6a3..6c28a68be3 100644 --- a/assets/js/state/selectors/activityLog.js +++ b/assets/js/state/selectors/activityLog.js @@ -1,6 +1,6 @@ import { createSelector } from '@reduxjs/toolkit'; -export const getALUsers = createSelector( +export const getActivityLogUsers = createSelector( [(state) => state.activityLog], (activityLog) => activityLog.users ); diff --git a/assets/js/state/selectors/activityLog.test.js b/assets/js/state/selectors/activityLog.test.js index 5618c7029a..e01d04c941 100644 --- a/assets/js/state/selectors/activityLog.test.js +++ b/assets/js/state/selectors/activityLog.test.js @@ -1,4 +1,4 @@ -import { getALUsers } from './activityLog'; +import { getActivityLogUsers } from './activityLog'; describe('Activity Log users selector', () => { it('should return a list of users from activity log state', () => { @@ -7,6 +7,6 @@ describe('Activity Log users selector', () => { activityLog: { users }, }; - expect(getALUsers(state)).toEqual(users); + expect(getActivityLogUsers(state)).toEqual(users); }); }); From dcf0da527fae227348e2a069d622eb601e8927d5 Mon Sep 17 00:00:00 2001 From: Gagandeep Bhatia Date: Tue, 10 Sep 2024 13:44:52 +0200 Subject: [PATCH 22/31] Adds test case for non-empty initial state --- assets/js/state/activityLog.test.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/assets/js/state/activityLog.test.js b/assets/js/state/activityLog.test.js index bd9468c10e..e617ece28c 100644 --- a/assets/js/state/activityLog.test.js +++ b/assets/js/state/activityLog.test.js @@ -12,4 +12,15 @@ describe('activityLog reducer', () => { users, }); }); + it('should set the users for activity log when setUsers is dispatched with non empty initial state', () => { + const { username: username1 } = userFactory.build(); + const { username: username2 } = userFactory.build(); + const { username: username3 } = userFactory.build(); + const users = [username1, username2]; + const nonEmptyInitialState = {users: [username3]}; + const action = setUsers({ users }); + expect(activityLogReducer(nonEmptyInitialState, action)).toEqual({ + users + }); + }); }); From d8a0f6b248d6f26b78c66c25d5487737108e8a49 Mon Sep 17 00:00:00 2001 From: Gagandeep Bhatia Date: Tue, 10 Sep 2024 13:47:32 +0200 Subject: [PATCH 23/31] Formatting --- assets/js/state/activityLog.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/assets/js/state/activityLog.test.js b/assets/js/state/activityLog.test.js index e617ece28c..e4834ab5f4 100644 --- a/assets/js/state/activityLog.test.js +++ b/assets/js/state/activityLog.test.js @@ -17,10 +17,10 @@ describe('activityLog reducer', () => { const { username: username2 } = userFactory.build(); const { username: username3 } = userFactory.build(); const users = [username1, username2]; - const nonEmptyInitialState = {users: [username3]}; + const nonEmptyInitialState = { users: [username3] }; const action = setUsers({ users }); expect(activityLogReducer(nonEmptyInitialState, action)).toEqual({ - users + users, }); }); }); From 9a5a4e34b744c7c4d9ab7628ce6975ee4c7a56a6 Mon Sep 17 00:00:00 2001 From: Gagandeep Bhatia Date: Wed, 11 Sep 2024 10:22:16 +0200 Subject: [PATCH 24/31] Makes refresh interval a parameter According to PR conversation, the update push interval from the channel is now a compile time Application env variable. --- config/config.exs | 2 ++ config/test.exs | 2 ++ lib/trento_web/channels/activity_log_channel.ex | 3 ++- test/trento_web/channels/activity_log_channel_test.exs | 2 ++ 4 files changed, 8 insertions(+), 1 deletion(-) diff --git a/config/config.exs b/config/config.exs index abbea1856f..a526aa6efe 100644 --- a/config/config.exs +++ b/config/config.exs @@ -221,6 +221,8 @@ config :flop, repo: Trento.Repo config :trento, admin_user: "admin" +config :trento, :activity_log, refresh_interval: 60_000 + # Import environment specific config. This must remain at the bottom # of this file so it overrides the configuration defined above. import_config "#{config_env()}.exs" diff --git a/config/test.exs b/config/test.exs index e780cd81e7..e8aaff9925 100644 --- a/config/test.exs +++ b/config/test.exs @@ -91,3 +91,5 @@ config :trento, Trento.Infrastructure.SoftwareUpdates.MockSuma, # 448 matches to "test" fqdn 448 ] + +config :trento, :activity_log, refresh_interval: 1 diff --git a/lib/trento_web/channels/activity_log_channel.ex b/lib/trento_web/channels/activity_log_channel.ex index 85332c0888..6835659afe 100644 --- a/lib/trento_web/channels/activity_log_channel.ex +++ b/lib/trento_web/channels/activity_log_channel.ex @@ -6,6 +6,7 @@ defmodule TrentoWeb.ActivityLogChannel do require Logger use TrentoWeb, :channel alias Trento.Users + @refresh_interval Application.compile_env!(:trento, [:activity_log, :refresh_interval]) @impl true def join( @@ -36,7 +37,7 @@ defmodule TrentoWeb.ActivityLogChannel do users = ["system" | collapsed_usernames] push(socket, "al_users_pushed", %{users: users}) - Process.send_after(self(), :after_join, 60_000) + Process.send_after(self(), :after_join, @refresh_interval) {:noreply, socket} end diff --git a/test/trento_web/channels/activity_log_channel_test.exs b/test/trento_web/channels/activity_log_channel_test.exs index 770b1578a2..ad1398d6bd 100644 --- a/test/trento_web/channels/activity_log_channel_test.exs +++ b/test/trento_web/channels/activity_log_channel_test.exs @@ -8,6 +8,8 @@ defmodule TrentoWeb.ActivityLogChannelTest do |> join(TrentoWeb.ActivityLogChannel, "activity_log:876") assert_push("al_users_pushed", %{users: _}) + assert_push("al_users_pushed", %{users: _}) + assert_push("al_users_pushed", %{users: _}) end test "Unauthorized users cannot join the activity log channel" do From cb5b4bf8e54c9d53304d3b0418479e98aeeec269 Mon Sep 17 00:00:00 2001 From: Gagandeep Bhatia Date: Wed, 11 Sep 2024 15:20:47 +0200 Subject: [PATCH 25/31] Addresses PR comment The users context function modified here now returns username tupled with the deleted_at timestamp. This simplifies somewhat the implementation of the collapse_usernames function in the ActivityLogChannel module. Specifically, this function no longer needs to do any DateTime parsing to infer soft deleted users/associated usernames. --- lib/trento/users.ex | 7 +++--- .../channels/activity_log_channel.ex | 23 +++++++------------ test/trento/users_test.exs | 18 +++++++++++---- 3 files changed, 25 insertions(+), 23 deletions(-) diff --git a/lib/trento/users.ex b/lib/trento/users.ex index d51b6d8575..cc8355309f 100644 --- a/lib/trento/users.ex +++ b/lib/trento/users.ex @@ -38,11 +38,12 @@ defmodule Trento.Users do end @doc """ - Returns all usernames, including those for users that are soft-deleted. + Returns all usernames tupled with the deleted_at timestamp, including those for users that are soft-deleted. """ - def list_all_usernames do + @spec list_all_usernames_ts :: list({String.t(), DateTime.t()}) + def list_all_usernames_ts do User - |> select([u], u.username) + |> select([u], {u.username, u.deleted_at}) |> Repo.all() end diff --git a/lib/trento_web/channels/activity_log_channel.ex b/lib/trento_web/channels/activity_log_channel.ex index 6835659afe..d0530e2b1b 100644 --- a/lib/trento_web/channels/activity_log_channel.ex +++ b/lib/trento_web/channels/activity_log_channel.ex @@ -32,8 +32,8 @@ defmodule TrentoWeb.ActivityLogChannel do @impl true def handle_info(:after_join, socket) do - all_usernames = Users.list_all_usernames() - collapsed_usernames = collapse_usernames(all_usernames) + all_usernames_ts = Users.list_all_usernames_ts() + collapsed_usernames = collapse_usernames(all_usernames_ts) users = ["system" | collapsed_usernames] push(socket, "al_users_pushed", %{users: users}) @@ -43,21 +43,14 @@ defmodule TrentoWeb.ActivityLogChannel do defp allowed?(user_id, current_user_id), do: String.to_integer(user_id) == current_user_id - defp collapse_usernames(usernames) do - usernames - |> Enum.map(fn username -> - maybe_ts = + defp collapse_usernames(usernames_ts) do + usernames_ts + |> Enum.map(fn + {username, nil} -> username - |> String.split("__") - |> List.last() - case DateTime.from_iso8601(maybe_ts) do - {:ok, _, _} -> - String.trim_trailing(username, "__" <> maybe_ts) - - _ -> - username - end + {username, deleted_at} -> + String.trim_trailing(username, "__" <> DateTime.to_string(deleted_at)) end) |> Enum.uniq() end diff --git a/test/trento/users_test.exs b/test/trento/users_test.exs index 3a687bdb5d..df89c2a221 100644 --- a/test/trento/users_test.exs +++ b/test/trento/users_test.exs @@ -117,11 +117,19 @@ defmodule Trento.UsersTest do assert length(users) == 1 end - test "list_all_usernames returns all usernames including those for deleted users" do - %{username: username1} = insert(:user) - %{username: username2} = insert(:user, deleted_at: DateTime.utc_now()) - inserted_sorted_usernames = Enum.sort([username1, username2]) - sorted_usernames = Enum.sort(Users.list_all_usernames()) + test "list_all_usernames returns all usernames tupled with the deleted_at field, including those for deleted users" do + %{username: username1, deleted_at: deleted_at1} = insert(:user) + + %{username: username2, deleted_at: deleted_at2} = + insert(:user, deleted_at: DateTime.utc_now()) + + sorter_fn = fn {username, _deleted_at} -> username end + + inserted_sorted_usernames = + Enum.sort_by([{username1, deleted_at1}, {username2, deleted_at2}], sorter_fn) + + sorted_usernames = + Enum.sort_by(Users.list_all_usernames_ts(), sorter_fn) assert inserted_sorted_usernames == sorted_usernames assert length(sorted_usernames) == 2 From 09791833dde41a5325bc234f33bb66b99c31de7d Mon Sep 17 00:00:00 2001 From: Gagandeep Bhatia Date: Thu, 12 Sep 2024 12:56:10 +0200 Subject: [PATCH 26/31] Adds an additional jest test This test makes use of non empty/non default state to assert on number of user filter options rendered. --- .../pages/ActivityLogPage/ActivityLogPage.jsx | 55 +++++-------------- .../ActivityLogPage/ActivityLogPage.test.jsx | 24 +++++--- 2 files changed, 30 insertions(+), 49 deletions(-) diff --git a/assets/js/pages/ActivityLogPage/ActivityLogPage.jsx b/assets/js/pages/ActivityLogPage/ActivityLogPage.jsx index f0c3c6e346..dbf16c3723 100644 --- a/assets/js/pages/ActivityLogPage/ActivityLogPage.jsx +++ b/assets/js/pages/ActivityLogPage/ActivityLogPage.jsx @@ -1,19 +1,14 @@ -import React, { useState, useEffect } from 'react'; -import { useSearchParams } from 'react-router-dom'; -import { useSelector } from 'react-redux'; - -import { map, pipe } from 'lodash/fp'; - +import ActivityLogOverview from '@common/ActivityLogOverview'; +import ComposedFilter from '@common/ComposedFilter'; +import PageHeader from '@common/PageHeader'; import { getActivityLog } from '@lib/api/activityLogs'; import { allowedActivities } from '@lib/model/activityLog'; - +import { getActivityLogUsers } from '@state/selectors/activityLog'; import { getUserProfile } from '@state/selectors/user'; - -import PageHeader from '@common/PageHeader'; +import { map, pipe } from 'lodash/fp'; +import React, { useEffect, useState } from 'react'; import { useSelector } from 'react-redux'; -import ActivityLogOverview from '@common/ActivityLogOverview'; -import ComposedFilter from '@common/ComposedFilter'; -import { getActivityLogUsers } from '@state/selectors/activityLog'; +import { useSearchParams } from 'react-router-dom'; import { filterValueToSearchParams, @@ -23,36 +18,6 @@ import { function ActivityLogPage() { const users = useSelector(getActivityLogUsers); - const filters = [ - { - key: 'type', - type: 'select', - title: 'Resource type', - options: Object.entries(ACTIVITY_TYPES_CONFIG).map(([key, value]) => [ - key, - value.label, - ]), - }, - { - key: 'actor', - type: 'select', - title: 'User', - options: users, - }, - { - key: 'to_date', - title: 'newer than', - type: 'date', - prefilled: true, - }, - { - key: 'from_date', - title: 'older than', - type: 'date', - prefilled: true, - }, - ]; - const [searchParams, setSearchParams] = useSearchParams(); const [activityLog, setActivityLog] = useState([]); const [isLoading, setLoading] = useState(true); @@ -70,6 +35,12 @@ function ActivityLogPage() { map(([key, value]) => [key, value.label]) )(abilities), }, + { + key: 'actor', + type: 'select', + title: 'User', + options: users, + }, { key: 'to_date', title: 'newer than', diff --git a/assets/js/pages/ActivityLogPage/ActivityLogPage.test.jsx b/assets/js/pages/ActivityLogPage/ActivityLogPage.test.jsx index 22aa56ab8f..094734a7a5 100644 --- a/assets/js/pages/ActivityLogPage/ActivityLogPage.test.jsx +++ b/assets/js/pages/ActivityLogPage/ActivityLogPage.test.jsx @@ -1,15 +1,11 @@ import '@testing-library/jest-dom'; -<<<<<<< HEAD -======= -import MockAdapter from 'axios-mock-adapter'; -import { withDefaultState, renderWithRouter } from '@lib/test-utils'; - ->>>>>>> d9845ca54 (Refactors ActivityLogPage test) import { networkClient } from '@lib/network'; -import { renderWithRouter, withDefaultState } from '@lib/test-utils'; +import { renderWithRouter, withDefaultState, withState } from '@lib/test-utils'; import { activityLogEntryFactory } from '@lib/test-utils/factories/activityLog'; +import { userFactory } from '@lib/test-utils/factories/users'; import { screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; import MockAdapter from 'axios-mock-adapter'; import React, { act } from 'react'; @@ -63,4 +59,18 @@ describe('ActivityLogPage', () => { ); expect(container.querySelectorAll('tbody > tr')).toHaveLength(5); }); + + it('should render tracked activity log and the users filter with non-default/non-empty state', async () => { + const users = userFactory.buildList(5).map((user) => user.username); + axiosMock.onGet('/api/v1/activity_log'); + const [StatefulActivityLogPage, _] = withState(, { + activityLog: { users }, + }); + const { container } = await act(() => + renderWithRouter(StatefulActivityLogPage) + ); + + await userEvent.click(screen.getByTestId('filter-User')); + expect(container.querySelectorAll('ul > li')).toHaveLength(users.length); + }); }); From bd7ca7c0dc9d48b02575ff2084649f7c71f3dc9f Mon Sep 17 00:00:00 2001 From: Gagandeep Bhatia Date: Thu, 12 Sep 2024 17:53:34 +0200 Subject: [PATCH 27/31] Updates tests Some changes introduced post rebase were breaking a test. This commit fixes the failing test. --- .../ActivityLogPage/ActivityLogPage.test.jsx | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/assets/js/pages/ActivityLogPage/ActivityLogPage.test.jsx b/assets/js/pages/ActivityLogPage/ActivityLogPage.test.jsx index 094734a7a5..728aeab8d8 100644 --- a/assets/js/pages/ActivityLogPage/ActivityLogPage.test.jsx +++ b/assets/js/pages/ActivityLogPage/ActivityLogPage.test.jsx @@ -1,7 +1,12 @@ import '@testing-library/jest-dom'; import { networkClient } from '@lib/network'; -import { renderWithRouter, withDefaultState, withState } from '@lib/test-utils'; +import { + renderWithRouter, + withDefaultState, + withState, + defaultInitialState, +} from '@lib/test-utils'; import { activityLogEntryFactory } from '@lib/test-utils/factories/activityLog'; import { userFactory } from '@lib/test-utils/factories/users'; import { screen } from '@testing-library/react'; @@ -62,8 +67,11 @@ describe('ActivityLogPage', () => { it('should render tracked activity log and the users filter with non-default/non-empty state', async () => { const users = userFactory.buildList(5).map((user) => user.username); - axiosMock.onGet('/api/v1/activity_log'); + axiosMock + .onGet('/api/v1/activity_log') + .reply(200, { data: activityLogEntryFactory.buildList(5) }); const [StatefulActivityLogPage, _] = withState(, { + ...defaultInitialState, activityLog: { users }, }); const { container } = await act(() => @@ -71,6 +79,8 @@ describe('ActivityLogPage', () => { ); await userEvent.click(screen.getByTestId('filter-User')); - expect(container.querySelectorAll('ul > li')).toHaveLength(users.length); + expect(container.querySelectorAll('ul > li[role="option"]')).toHaveLength( + users.length + ); }); }); From 726c17165bd29519bd6f0927238cbf53d8fd8052 Mon Sep 17 00:00:00 2001 From: Gagandeep Bhatia Date: Fri, 13 Sep 2024 11:29:37 +0200 Subject: [PATCH 28/31] Formatting - import re-ordering/spacing, etc. --- assets/js/pages/ActivityLogPage/ActivityLogPage.jsx | 10 ++++++---- .../js/pages/ActivityLogPage/ActivityLogPage.test.jsx | 9 +++++---- assets/js/state/activityLog.test.js | 1 + 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/assets/js/pages/ActivityLogPage/ActivityLogPage.jsx b/assets/js/pages/ActivityLogPage/ActivityLogPage.jsx index dbf16c3723..da96e8071c 100644 --- a/assets/js/pages/ActivityLogPage/ActivityLogPage.jsx +++ b/assets/js/pages/ActivityLogPage/ActivityLogPage.jsx @@ -1,3 +1,9 @@ +import React, { useEffect, useState } from 'react'; +import { useSelector } from 'react-redux'; +import { useSearchParams } from 'react-router-dom'; + +import { map, pipe } from 'lodash/fp'; + import ActivityLogOverview from '@common/ActivityLogOverview'; import ComposedFilter from '@common/ComposedFilter'; import PageHeader from '@common/PageHeader'; @@ -5,10 +11,6 @@ import { getActivityLog } from '@lib/api/activityLogs'; import { allowedActivities } from '@lib/model/activityLog'; import { getActivityLogUsers } from '@state/selectors/activityLog'; import { getUserProfile } from '@state/selectors/user'; -import { map, pipe } from 'lodash/fp'; -import React, { useEffect, useState } from 'react'; -import { useSelector } from 'react-redux'; -import { useSearchParams } from 'react-router-dom'; import { filterValueToSearchParams, diff --git a/assets/js/pages/ActivityLogPage/ActivityLogPage.test.jsx b/assets/js/pages/ActivityLogPage/ActivityLogPage.test.jsx index 728aeab8d8..b2e703c001 100644 --- a/assets/js/pages/ActivityLogPage/ActivityLogPage.test.jsx +++ b/assets/js/pages/ActivityLogPage/ActivityLogPage.test.jsx @@ -1,4 +1,9 @@ +import React, { act } from 'react'; +import { screen } from '@testing-library/react'; import '@testing-library/jest-dom'; +import userEvent from '@testing-library/user-event'; + +import MockAdapter from 'axios-mock-adapter'; import { networkClient } from '@lib/network'; import { @@ -9,10 +14,6 @@ import { } from '@lib/test-utils'; import { activityLogEntryFactory } from '@lib/test-utils/factories/activityLog'; import { userFactory } from '@lib/test-utils/factories/users'; -import { screen } from '@testing-library/react'; -import userEvent from '@testing-library/user-event'; -import MockAdapter from 'axios-mock-adapter'; -import React, { act } from 'react'; import ActivityLogPage from './ActivityLogPage'; diff --git a/assets/js/state/activityLog.test.js b/assets/js/state/activityLog.test.js index e4834ab5f4..84bf130973 100644 --- a/assets/js/state/activityLog.test.js +++ b/assets/js/state/activityLog.test.js @@ -12,6 +12,7 @@ describe('activityLog reducer', () => { users, }); }); + it('should set the users for activity log when setUsers is dispatched with non empty initial state', () => { const { username: username1 } = userFactory.build(); const { username: username2 } = userFactory.build(); From b32e3f6523cfa1fbfb83b114194199779c76fcea Mon Sep 17 00:00:00 2001 From: Gagandeep Bhatia Date: Fri, 13 Sep 2024 11:36:49 +0200 Subject: [PATCH 29/31] Adds a saga test --- assets/js/state/sagas/activityLog.test.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 assets/js/state/sagas/activityLog.test.js diff --git a/assets/js/state/sagas/activityLog.test.js b/assets/js/state/sagas/activityLog.test.js new file mode 100644 index 0000000000..f1a0d77e98 --- /dev/null +++ b/assets/js/state/sagas/activityLog.test.js @@ -0,0 +1,16 @@ +import { recordSaga } from '@lib/test-utils'; +import { userFactory } from '@lib/test-utils/factories/users'; +import { setUsers } from '@state/activityLog'; +import { activityLogUsersUpdate } from './activityLog'; + +describe('Activity Logs saga', () => { + it('should set users when activity log users are updated', async () => { + const users = userFactory.buildList(5).map((user) => user.username); + + const dispatched = await recordSaga(activityLogUsersUpdate, { + payload: { users }, + }); + + expect(dispatched).toEqual([setUsers({ users })]); + }); +}); From 468347a1d6b4c2cc0d1d78842a64ed0feaa0bf4a Mon Sep 17 00:00:00 2001 From: Gagandeep Bhatia Date: Fri, 13 Sep 2024 11:37:28 +0200 Subject: [PATCH 30/31] Renames users context function Removes the trailing _ts --- lib/trento/users.ex | 4 ++-- lib/trento_web/channels/activity_log_channel.ex | 2 +- test/trento/users_test.exs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/trento/users.ex b/lib/trento/users.ex index cc8355309f..a641c14182 100644 --- a/lib/trento/users.ex +++ b/lib/trento/users.ex @@ -40,8 +40,8 @@ defmodule Trento.Users do @doc """ Returns all usernames tupled with the deleted_at timestamp, including those for users that are soft-deleted. """ - @spec list_all_usernames_ts :: list({String.t(), DateTime.t()}) - def list_all_usernames_ts do + @spec list_all_usernames :: list({String.t(), DateTime.t()}) + def list_all_usernames do User |> select([u], {u.username, u.deleted_at}) |> Repo.all() diff --git a/lib/trento_web/channels/activity_log_channel.ex b/lib/trento_web/channels/activity_log_channel.ex index d0530e2b1b..64fa177e28 100644 --- a/lib/trento_web/channels/activity_log_channel.ex +++ b/lib/trento_web/channels/activity_log_channel.ex @@ -32,7 +32,7 @@ defmodule TrentoWeb.ActivityLogChannel do @impl true def handle_info(:after_join, socket) do - all_usernames_ts = Users.list_all_usernames_ts() + all_usernames_ts = Users.list_all_usernames() collapsed_usernames = collapse_usernames(all_usernames_ts) users = ["system" | collapsed_usernames] diff --git a/test/trento/users_test.exs b/test/trento/users_test.exs index df89c2a221..40494a0ef9 100644 --- a/test/trento/users_test.exs +++ b/test/trento/users_test.exs @@ -129,7 +129,7 @@ defmodule Trento.UsersTest do Enum.sort_by([{username1, deleted_at1}, {username2, deleted_at2}], sorter_fn) sorted_usernames = - Enum.sort_by(Users.list_all_usernames_ts(), sorter_fn) + Enum.sort_by(Users.list_all_usernames(), sorter_fn) assert inserted_sorted_usernames == sorted_usernames assert length(sorted_usernames) == 2 From 6fb127be30fe3153e20a12ee9857a3d37d95ed81 Mon Sep 17 00:00:00 2001 From: Gagandeep Bhatia Date: Fri, 13 Sep 2024 12:11:55 +0200 Subject: [PATCH 31/31] Reodering imports In order to minimize diffs/conflicts --- assets/js/pages/ActivityLogPage/ActivityLogPage.jsx | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/assets/js/pages/ActivityLogPage/ActivityLogPage.jsx b/assets/js/pages/ActivityLogPage/ActivityLogPage.jsx index da96e8071c..a27440f045 100644 --- a/assets/js/pages/ActivityLogPage/ActivityLogPage.jsx +++ b/assets/js/pages/ActivityLogPage/ActivityLogPage.jsx @@ -1,17 +1,18 @@ -import React, { useEffect, useState } from 'react'; -import { useSelector } from 'react-redux'; +import React, { useState, useEffect } from 'react'; import { useSearchParams } from 'react-router-dom'; +import { useSelector } from 'react-redux'; import { map, pipe } from 'lodash/fp'; -import ActivityLogOverview from '@common/ActivityLogOverview'; -import ComposedFilter from '@common/ComposedFilter'; -import PageHeader from '@common/PageHeader'; import { getActivityLog } from '@lib/api/activityLogs'; import { allowedActivities } from '@lib/model/activityLog'; import { getActivityLogUsers } from '@state/selectors/activityLog'; import { getUserProfile } from '@state/selectors/user'; +import PageHeader from '@common/PageHeader'; +import ActivityLogOverview from '@common/ActivityLogOverview'; +import ComposedFilter from '@common/ComposedFilter'; + import { filterValueToSearchParams, searchParamsToAPIParams,