Activity log filter by user (#2924)

* wip

- needs some clean up

* See description

- Deletes unused code
- Pulls out state access into the ActivityLog page context

* 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

* Renaming

Renames some declarations

* Refactors activity_log channel

* Fixes warning

Fix for identityFunctionCheck warning in console.

* Runs formatter

* Renaming

* Adds some tests

- channel test for activity log
- activity log state test

* Runs js code formatter

* Fix for failing tests

Adds a hook that passes in requisite state to the ActivityLogPage component

* Deletes an unused import; format

* Some cleanup

Specific users in list are not necessary

* Adds a system user

Previously a system user was not returned; now it is possible to filter the system activity log entries.

* Refactors ActivityLogPage test

* Adds test for activityLog users selector

* Adds tests for ActivityLogChannel

* Adds a test for a Users context function

* Addresses PR comments

Renaming

* Handles scenario with deleted users

Collapses usernames from deleted users into a username without timestamp suffix

* Renaming

alUsers -> activityLogUsers

* Adds test case for non-empty initial state

* Formatting

* Makes refresh interval a parameter

According to PR conversation, the update push interval from the channel  is now a compile time Application env variable.

* 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.

* Adds an additional jest test

This test makes use of non empty/non default state to assert on number of user filter options rendered.

* Updates tests

Some changes introduced post rebase were breaking a test. This commit fixes the failing test.

* Formatting

- import re-ordering/spacing, etc.

* Adds a saga test

* Renames users context function

Removes the trailing _ts

* Reodering imports

In order to minimize diffs/conflicts

assets/js/lib/test-utils/index.jsx

@@ -16,6 +16,9 @@ const middlewares = [];
 const mockStore = configureStore(middlewares);
 
 export const defaultInitialState = {
+  activityLog: {
+    users: [],
+  },
   user: {
     abilities: [{ name: 'all', resource: 'all' }],
   },

assets/js/pages/ActivityLogPage/ActivityLogPage.jsx

@@ -6,7 +6,7 @@ import { map, pipe } from 'lodash/fp';
 
 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';
@@ -20,6 +20,7 @@ import {
 } from './searchParams';
 
 function ActivityLogPage() {
+  const users = useSelector(getActivityLogUsers);
   const [searchParams, setSearchParams] = useSearchParams();
   const [activityLog, setActivityLog] = useState([]);
   const [isLoading, setLoading] = useState(true);
@@ -37,6 +38,12 @@ function ActivityLogPage() {
         map(([key, value]) => [key, value.label])
       )(abilities),
     },
+    {
+      key: 'actor',
+      type: 'select',
+      title: 'User',
+      options: users,
+    },
     {
       key: 'to_date',
       title: 'newer than',

assets/js/pages/ActivityLogPage/ActivityLogPage.test.jsx

@@ -1,12 +1,19 @@
 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 { renderWithRouter, withDefaultState } from '@lib/test-utils';
 
 import { networkClient } from '@lib/network';
+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 ActivityLogPage from './ActivityLogPage';
 
@@ -15,8 +22,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(<ActivityLogPage />);
-    await act(async () => renderWithRouter(StatefulActivityLogPage));
+    const [StatefulActivityLogPage, _] = withDefaultState(<ActivityLogPage />);
+    await act(() => renderWithRouter(StatefulActivityLogPage));
     expect(screen.getByText('No data available')).toBeVisible();
   });
 
@@ -39,7 +46,9 @@ describe('ActivityLogPage', () => {
       axiosMock
         .onGet('/api/v1/activity_log')
         .reply(responseStatus, responseBody);
-      const [StatefulActivityLogPage] = withDefaultState(<ActivityLogPage />);
+      const [StatefulActivityLogPage, _] = withDefaultState(
+        <ActivityLogPage />
+      );
       await act(() => renderWithRouter(StatefulActivityLogPage));
 
       expect(screen.getByText('No data available')).toBeVisible();
@@ -50,13 +59,29 @@ describe('ActivityLogPage', () => {
     axiosMock
       .onGet('/api/v1/activity_log')
       .reply(200, { data: activityLogEntryFactory.buildList(5) });
+    const [StatefulActivityLogPage, _] = withDefaultState(<ActivityLogPage />);
+    const { container } = await act(() =>
+      renderWithRouter(StatefulActivityLogPage)
+    );
+    expect(container.querySelectorAll('tbody > tr')).toHaveLength(5);
+  });
 
-    const [StatefulActivityLogPage] = withDefaultState(<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')
+      .reply(200, { data: activityLogEntryFactory.buildList(5) });
+    const [StatefulActivityLogPage, _] = withState(<ActivityLogPage />, {
+      ...defaultInitialState,
+      activityLog: { users },
+    });
     const { container } = await act(() =>
       renderWithRouter(StatefulActivityLogPage)
     );
 
-    expect(container.querySelectorAll('tbody > tr')).toHaveLength(5);
+    await userEvent.click(screen.getByTestId('filter-User'));
+    expect(container.querySelectorAll('ul > li[role="option"]')).toHaveLength(
+      users.length
+    );
   });
 });

assets/js/state/activityLog.js

@@ -0,0 +1,21 @@
+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 ACTIVITY_LOG_USERS_PUSHED = 'ACTIVITY_LOG_USERS_PUSHED';
+export const activityLogUsersPushed = createAction(ACTIVITY_LOG_USERS_PUSHED);
+export const { setUsers } = activityLogSlice.actions;
+
+export default activityLogSlice.reducer;

assets/js/state/activityLog.test.js

@@ -0,0 +1,27 @@
+import { userFactory } from '@lib/test-utils/factories/users';
+
+import activityLogReducer, { setUsers, initialState } 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,
+    });
+  });
+
+  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,
+    });
+  });
+});

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),

assets/js/state/sagas/activityLog.js

@@ -0,0 +1,10 @@
+import { put, takeEvery } from 'redux-saga/effects';
+import { setUsers, ACTIVITY_LOG_USERS_PUSHED } from '@state/activityLog';
+
+export function* activityLogUsersUpdate({ payload: { users } }) {
+  yield put(setUsers({ users }));
+}
+
+export function* watchActivityLogActions() {
+  yield takeEvery(ACTIVITY_LOG_USERS_PUSHED, activityLogUsersUpdate);
+}

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 })]);
+  });
+});

assets/js/state/sagas/channels.js

@@ -55,6 +55,7 @@ import {
 } from '@state/lastExecutions';
 
 import { userUpdated, userLocked, userDeleted } from '@state/user';
+import { activityLogUsersPushed } from '@state/activityLog';
 
 import { getUserProfile } from '@state/selectors/user';
 
@@ -236,6 +237,13 @@ const userEvents = [
   },
 ];
 
+const activityLogEvents = [
+  {
+    name: 'al_users_pushed',
+    action: activityLogUsersPushed,
+  },
+];
+
 const createEventChannel = (channel, events) =>
   eventChannel((emitter) => {
     events.forEach((event) => {
@@ -281,5 +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
+    ),
   ]);
 }

assets/js/state/sagas/channels.test.js

@@ -6,6 +6,7 @@ import {
   setExecutionStarted,
 } from '@state/lastExecutions';
 import { userUpdated } from '@state/user';
+import { activityLogUsersPushed } 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,18 @@ describe('Channels saga', () => {
 
     expect(dispatched).toEqual([userUpdated({ email: '[email protected]' })]);
   });
+  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([
+      activityLogUsersPushed({ users: ['user1', 'user2', 'user3'] }),
+    ]);
+  });
 });

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 { watchActivityLogActions } 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(),
+    watchActivityLogActions(),
   ]);
 }

assets/js/state/selectors/activityLog.js

@@ -0,0 +1,6 @@
+import { createSelector } from '@reduxjs/toolkit';
+
+export const getActivityLogUsers = createSelector(
+  [(state) => state.activityLog],
+  (activityLog) => activityLog.users
+);

assets/js/state/selectors/activityLog.test.js

@@ -0,0 +1,12 @@
+import { getActivityLogUsers } 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(getActivityLogUsers(state)).toEqual(users);
+  });
+});

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"

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

lib/trento/users.ex

@@ -37,6 +37,16 @@ defmodule Trento.Users do
     |> Repo.all()
   end
 
+  @doc """
+  Returns all usernames tupled with the deleted_at timestamp, including those for users that are soft-deleted.
+  """
+  @spec list_all_usernames :: list({String.t(), DateTime.t()})
+  def list_all_usernames do
+    User
+    |> select([u], {u.username, u.deleted_at})
+    |> Repo.all()
+  end
+
   def get_user(id) do
     case User
          |> where([u], is_nil(u.deleted_at) and u.id == ^id)