From f59b77571c760d4c7732232549797af5fc7f85c8 Mon Sep 17 00:00:00 2001 From: Thomas Heartman Date: Wed, 14 Aug 2024 15:27:22 +0200 Subject: [PATCH] chore: code cleanup: event log filters (#7870) Adds tests for event log filters (to ensure we show the right filters) and refactors the implementation of eventlogfilters. Primary goal of refactoring: - Make it so that all filters are created in one single list (instead of injected from different variables) - Avoid making a requests for features (and to a lesser extent: projects) if you can't use them for filters - Improve code structure --- .../events/EventLog/EventLogFilters.test.tsx | 68 +++++++ .../events/EventLog/EventLogFilters.tsx | 171 +++++++++++------- 2 files changed, 169 insertions(+), 70 deletions(-) create mode 100644 frontend/src/component/events/EventLog/EventLogFilters.test.tsx diff --git a/frontend/src/component/events/EventLog/EventLogFilters.test.tsx b/frontend/src/component/events/EventLog/EventLogFilters.test.tsx new file mode 100644 index 0000000000..ae2b024823 --- /dev/null +++ b/frontend/src/component/events/EventLog/EventLogFilters.test.tsx @@ -0,0 +1,68 @@ +import { renderHook } from '@testing-library/react'; +import { useEventLogFilters } from './EventLogFilters'; + +const allFilterKeys = ['from', 'to', 'createdBy', 'type', 'project', 'feature']; + +allFilterKeys.sort(); + +test('When you have no projects or flags, you should not get a project or flag filters', () => { + const { result } = renderHook(() => + useEventLogFilters( + () => ({ projects: [] }), + () => ({ features: [] }), + ), + ); + const filterKeys = result.current.map((filter) => filter.filterKey); + filterKeys.sort(); + + expect(filterKeys).not.toContain('project'); + expect(filterKeys).not.toContain('feature'); +}); + +test('When you have no projects, you should not get a project filter', () => { + const { result } = renderHook(() => + useEventLogFilters( + () => ({ projects: [] }), + // @ts-expect-error: omitting other properties we don't need + () => ({ features: [{ name: 'flag' }] }), + ), + ); + const filterKeys = result.current.map((filter) => filter.filterKey); + filterKeys.sort(); + + expect(filterKeys).not.toContain('project'); +}); + +test('When you have only one project, you should not get a project filter', () => { + const { result } = renderHook(() => + useEventLogFilters( + // @ts-expect-error: omitting other properties we don't need + () => ({ projects: [{ id: 'a', name: 'A' }] }), + () => ({ features: [] }), + ), + ); + const filterKeys = result.current.map((filter) => filter.filterKey); + filterKeys.sort(); + + expect(filterKeys).not.toContain('project'); +}); + +test('When you have two one project, you should not get a project filter', () => { + const { result } = renderHook(() => + useEventLogFilters( + () => ({ + projects: [ + // @ts-expect-error: omitting other properties we don't need + { id: 'a', name: 'A' }, + // @ts-expect-error: omitting other properties we don't need + { id: 'b', name: 'B' }, + ], + }), + () => ({ features: [] }), + ), + ); + const filterKeys = result.current.map((filter) => filter.filterKey); + filterKeys.sort(); + + expect(filterKeys).toContain('project'); +}); diff --git a/frontend/src/component/events/EventLog/EventLogFilters.tsx b/frontend/src/component/events/EventLog/EventLogFilters.tsx index 2e80b9bb03..8bfe29e9bf 100644 --- a/frontend/src/component/events/EventLog/EventLogFilters.tsx +++ b/frontend/src/component/events/EventLog/EventLogFilters.tsx @@ -6,87 +6,82 @@ import { } from 'component/filter/Filters/Filters'; import useProjects from 'hooks/api/getters/useProjects/useProjects'; import { useFeatureSearch } from 'hooks/api/getters/useFeatureSearch/useFeatureSearch'; -import { EventSchemaType } from 'openapi'; +import { + EventSchemaType, + type FeatureSearchResponseSchema, + type SearchFeaturesParams, +} from 'openapi'; import type { IProjectCard } from 'interfaces/project'; import { useEventCreators } from 'hooks/api/getters/useEventCreators/useEventCreators'; -const useSharedFilters = (): IFilterItem[] => { +export const useEventLogFilters = ( + projectsHook: () => { projects: IProjectCard[] }, + featuresHook: (params: SearchFeaturesParams) => { + features: FeatureSearchResponseSchema[]; + }, +) => { + const { projects } = projectsHook(); + const { features } = featuresHook({}); const { eventCreators } = useEventCreators(); - return [ - { - label: 'Date From', - icon: 'today', - options: [], - filterKey: 'from', - dateOperators: ['IS'], - }, - { - label: 'Date To', - icon: 'today', - options: [], - filterKey: 'to', - dateOperators: ['IS'], - }, - { - label: 'Created by', - icon: 'person', - options: eventCreators.map((creator) => ({ - label: creator.name, - value: creator.id.toString(), - })), - filterKey: 'createdBy', - singularOperators: ['IS'], - pluralOperators: ['IS_ANY_OF'], - }, - { - label: 'Event type', - icon: 'announcement', - options: Object.entries(EventSchemaType).map(([key, value]) => ({ - label: key, - value: value, - })), - filterKey: 'type', - singularOperators: ['IS'], - pluralOperators: ['IS_ANY_OF'], - }, - ]; -}; - -type EventLogFiltersProps = { - logType: 'flag' | 'project' | 'global'; - className?: string; - state: FilterItemParamHolder; - onChange: (value: FilterItemParamHolder) => void; -}; -export const EventLogFilters: FC = ({ - logType, - className, - state, - onChange, -}) => { - const { projects } = useProjects(); - const { features } = useFeatureSearch({}); - const sharedFilters = useSharedFilters(); const [availableFilters, setAvailableFilters] = useState([]); useEffect(() => { - const projectOptions = (projects || []).map( - (project: IProjectCard) => ({ + const projectOptions = + projects?.map((project: IProjectCard) => ({ label: project.name, value: project.id, + })) ?? []; + + const flagOptions = + features?.map((flag) => ({ + label: flag.name, + value: flag.name, + })) ?? []; + + const eventCreatorOptions = eventCreators.map((creator) => ({ + label: creator.name, + value: creator.id.toString(), + })); + + const eventTypeOptions = Object.entries(EventSchemaType).map( + ([key, value]) => ({ + label: key, + value: value, }), ); - const hasMultipleProjects = projectOptions.length > 1; - - const flagOptions = (features || []).map((flag) => ({ - label: flag.name, - value: flag.name, - })); - const availableFilters: IFilterItem[] = [ - ...sharedFilters, - ...(hasMultipleProjects && logType === 'global' + { + label: 'Date From', + icon: 'today', + options: [], + filterKey: 'from', + dateOperators: ['IS'], + }, + { + label: 'Date To', + icon: 'today', + options: [], + filterKey: 'to', + dateOperators: ['IS'], + }, + { + label: 'Created by', + icon: 'person', + options: eventCreatorOptions, + filterKey: 'createdBy', + singularOperators: ['IS'], + pluralOperators: ['IS_ANY_OF'], + }, + { + label: 'Event type', + icon: 'announcement', + options: eventTypeOptions, + filterKey: 'type', + singularOperators: ['IS'], + pluralOperators: ['IS_ANY_OF'], + }, + ...(projectOptions.length > 1 ? ([ { label: 'Project', @@ -98,7 +93,7 @@ export const EventLogFilters: FC = ({ }, ] as IFilterItem[]) : []), - ...(logType !== 'flag' + ...(flagOptions.length > 0 ? ([ { label: 'Feature Flag', @@ -116,9 +111,45 @@ export const EventLogFilters: FC = ({ }, [ JSON.stringify(features), JSON.stringify(projects), - JSON.stringify(sharedFilters), + JSON.stringify(eventCreators), ]); + return availableFilters; +}; + +type LogType = 'flag' | 'project' | 'global'; +const useEventLogFiltersFromLogType = (logType: LogType) => { + switch (logType) { + case 'flag': + return useEventLogFilters( + () => ({ projects: [] }), + () => ({ features: [] }), + ); + case 'project': + return useEventLogFilters( + () => ({ projects: [] }), + useFeatureSearch, + ); + case 'global': + return useEventLogFilters(useProjects, useFeatureSearch); + } +}; + +type EventLogFiltersProps = { + logType: LogType; + className?: string; + state: FilterItemParamHolder; + onChange: (value: FilterItemParamHolder) => void; +}; + +export const EventLogFilters: FC = ({ + logType, + className, + state, + onChange, +}) => { + const availableFilters = useEventLogFiltersFromLogType(logType); + return (