From 8010aaaad4c46aaf1da07f25220a30861b9fc186 Mon Sep 17 00:00:00 2001 From: Roman Dmytrenko Date: Mon, 27 Nov 2023 11:09:36 +0200 Subject: [PATCH] refactor(ui): move Rules to use Redux for storage Refs: #2435 --- ui/src/app/flags/rules/Rules.tsx | 84 ++++---- ui/src/app/flags/rulesApi.ts | 187 ++++++++++++++++++ .../rules/forms/QuickEditRuleForm.tsx | 52 +++-- ui/src/components/rules/forms/RuleForm.tsx | 35 ++-- ui/src/data/api.ts | 88 --------- ui/src/store.ts | 6 +- 6 files changed, 279 insertions(+), 173 deletions(-) create mode 100644 ui/src/app/flags/rulesApi.ts diff --git a/ui/src/app/flags/rules/Rules.tsx b/ui/src/app/flags/rules/Rules.tsx index 11ebf10f8a..c4dd22b2ad 100644 --- a/ui/src/app/flags/rules/Rules.tsx +++ b/ui/src/app/flags/rules/Rules.tsx @@ -14,39 +14,40 @@ import { verticalListSortingStrategy } from '@dnd-kit/sortable'; import { PlusIcon } from '@heroicons/react/24/outline'; -import { useCallback, useEffect, useMemo, useState } from 'react'; +import { useMemo, useState } from 'react'; import { useSelector } from 'react-redux'; -import { useNavigate, useOutletContext } from 'react-router-dom'; +import { useOutletContext } from 'react-router-dom'; import { FlagProps } from '~/app/flags/FlagProps'; +import { + useDeleteRuleMutation, + useListRulesQuery, + useOrderRulesMutation +} from '~/app/flags/rulesApi'; import { selectReadonly } from '~/app/meta/metaSlice'; import { selectCurrentNamespace } from '~/app/namespaces/namespacesSlice'; import { useListSegmentsQuery } from '~/app/segments/segmentsApi'; import EmptyState from '~/components/EmptyState'; import Button from '~/components/forms/buttons/Button'; +import Loading from '~/components/Loading'; import Modal from '~/components/Modal'; import DeletePanel from '~/components/panels/DeletePanel'; import RuleForm from '~/components/rules/forms/RuleForm'; import Rule from '~/components/rules/Rule'; import SortableRule from '~/components/rules/SortableRule'; import Slideover from '~/components/Slideover'; -import { deleteRule, listRules, orderRules } from '~/data/api'; import { useError } from '~/data/hooks/error'; import { useSuccess } from '~/data/hooks/success'; import { IDistribution } from '~/types/Distribution'; import { IEvaluatable } from '~/types/Evaluatable'; -import { FlagType } from '~/types/Flag'; -import { IRule, IRuleList } from '~/types/Rule'; +import { IRule } from '~/types/Rule'; import { ISegment, SegmentOperatorType } from '~/types/Segment'; import { IVariant } from '~/types/Variant'; export default function Rules() { const { flag } = useOutletContext(); - const [rules, setRules] = useState([]); - const [activeRule, setActiveRule] = useState(null); - const [rulesVersion, setRulesVersion] = useState(0); const [showRuleForm, setShowRuleForm] = useState(false); const [showDeleteRuleModal, setShowDeleteRuleModal] = @@ -56,21 +57,26 @@ export default function Rules() { const { setError, clearError } = useError(); const { setSuccess } = useSuccess(); - const navigate = useNavigate(); - const namespace = useSelector(selectCurrentNamespace); const readOnly = useSelector(selectReadonly); - const listSegments = useListSegmentsQuery(namespace.key); + const segmentsList = useListSegmentsQuery(namespace.key); const segments = useMemo( - () => listSegments.data?.segments || [], - [listSegments] + () => segmentsList.data?.segments || [], + [segmentsList] ); - const loadData = useCallback(async () => { - // TODO: move to redux - const ruleList = (await listRules(namespace.key, flag.key)) as IRuleList; + const [deleteRule] = useDeleteRuleMutation(); + const [orderRules] = useOrderRulesMutation(); - const rules = ruleList.rules.flatMap((rule: IRule) => { + const rulesList = useListRulesQuery({ + namespaceKey: namespace.key, + flagKey: flag.key + }); + + const ruleList = useMemo(() => rulesList.data?.rules || [], [rulesList]); + + const rules = useMemo(() => { + return ruleList.flatMap((rule: IRule) => { const rollouts = rule.distributions.flatMap( (distribution: IDistribution) => { const variant = flag?.variants?.find( @@ -132,13 +138,7 @@ export default function Rules() { updatedAt: rule.updatedAt }; }); - - setRules(rules); - }, [namespace.key, flag, segments]); - - const incrementRulesVersion = () => { - setRulesVersion(rulesVersion + 1); - }; + }, [flag, segments, ruleList]); const sensors = useSensors( useSensor(PointerSensor), @@ -148,13 +148,13 @@ export default function Rules() { ); const reorderRules = (rules: IEvaluatable[]) => { - orderRules( - namespace.key, - flag.key, - rules.map((rule) => rule.id) - ) + orderRules({ + namespaceKey: namespace.key, + flagKey: flag.key, + ruleIds: rules.map((rule) => rule.id) + }) + .unwrap() .then(() => { - incrementRulesVersion(); clearError(); setSuccess('Successfully reordered rules'); }) @@ -177,7 +177,6 @@ export default function Rules() { })(rules); reorderRules(reordered); - setRules(reordered); } setActiveRule(null); @@ -193,16 +192,9 @@ export default function Rules() { } }; - useEffect(() => { - loadData(); - }, [loadData, rulesVersion]); - - useEffect(() => { - if (flag.type === FlagType.BOOLEAN) { - setError('Boolean flags do not support evaluation rules'); - navigate(`/namespaces/${namespace.key}/flags/${flag.key}`); - } - }, [flag.type, navigate, setError, namespace.key, flag.key]); + if (segmentsList.isLoading || rulesList.isLoading) { + return ; + } return ( <> @@ -222,9 +214,12 @@ export default function Rules() { panelType="Rule" setOpen={setShowDeleteRuleModal} handleDelete={() => - deleteRule(namespace.key, flag.key, deletingRule?.id ?? '') + deleteRule({ + namespaceKey: namespace.key, + flagKey: flag.key, + ruleId: deletingRule?.id ?? '' + }).unwrap() } - onSuccess={incrementRulesVersion} /> @@ -236,7 +231,6 @@ export default function Rules() { segments={segments} setOpen={setShowRuleForm} onSuccess={() => { - incrementRulesVersion(); setShowRuleForm(false); }} /> @@ -306,11 +300,11 @@ export default function Rules() { flag={flag} rule={rule} segments={segments} - onSuccess={incrementRulesVersion} onDelete={() => { setDeletingRule(rule); setShowDeleteRuleModal(true); }} + onSuccess={clearError} readOnly={readOnly} /> ))} diff --git a/ui/src/app/flags/rulesApi.ts b/ui/src/app/flags/rulesApi.ts new file mode 100644 index 0000000000..6d180884a9 --- /dev/null +++ b/ui/src/app/flags/rulesApi.ts @@ -0,0 +1,187 @@ +import { createApi } from '@reduxjs/toolkit/query/react'; +import { IDistributionBase } from '~/types/Distribution'; +import { IRule, IRuleBase, IRuleList } from '~/types/Rule'; + +import { baseQuery } from '~/utils/redux-rtk'; + +export const rulesApi = createApi({ + reducerPath: 'rules', + baseQuery, + tagTypes: ['Rule'], + endpoints: (builder) => ({ + // get list of rules + listRules: builder.query< + IRuleList, + { namespaceKey: string; flagKey: string } + >({ + query: ({ namespaceKey, flagKey }) => + `/namespaces/${namespaceKey}/flags/${flagKey}/rules`, + providesTags: (_result, _error, { namespaceKey, flagKey }) => [ + { type: 'Rule', id: namespaceKey + '/' + flagKey } + ] + }), + // create a new rule + createRule: builder.mutation< + IRule, + { + namespaceKey: string; + flagKey: string; + values: IRuleBase; + distributions: IDistributionBase[]; + } + >({ + queryFn: async ( + { namespaceKey, flagKey, values, distributions }, + _api, + _extraOptions, + baseQuery + ) => { + const respRule = await baseQuery({ + url: `/namespaces/${namespaceKey}/flags/${flagKey}/rules`, + method: 'POST', + body: values + }); + if (respRule.error) { + return { error: respRule.error }; + } + const rule = respRule.data as IRule; + const ruleId = rule.id; + // then create the distributions + for (let distribution of distributions) { + const resp = await baseQuery({ + url: `/namespaces/${namespaceKey}/flags/${flagKey}/rules/${ruleId}/distributions`, + method: 'POST', + body: distribution + }); + if (resp.error) { + return { error: resp.error }; + } + } + return { data: rule }; + }, + invalidatesTags: (_result, _error, { namespaceKey, flagKey }) => [ + { type: 'Rule', id: namespaceKey + '/' + flagKey } + ] + }), + // delete the rule + deleteRule: builder.mutation< + void, + { namespaceKey: string; flagKey: string; ruleId: string } + >({ + query({ namespaceKey, flagKey, ruleId }) { + return { + url: `/namespaces/${namespaceKey}/flags/${flagKey}/rules/${ruleId}`, + method: 'DELETE' + }; + }, + invalidatesTags: (_result, _error, { namespaceKey, flagKey }) => [ + { type: 'Rule', id: namespaceKey + '/' + flagKey } + ] + }), + // update the rule + updateRule: builder.mutation< + IRule, + { + namespaceKey: string; + flagKey: string; + ruleId: string; + values: IRuleBase; + } + >({ + query({ namespaceKey, flagKey, ruleId, values }) { + return { + url: `/namespaces/${namespaceKey}/flags/${flagKey}/rules/${ruleId}`, + method: 'PUT', + body: values + }; + }, + invalidatesTags: (_result, _error, { namespaceKey, flagKey }) => [ + { type: 'Rule', id: namespaceKey + '/' + flagKey } + ] + }), + // reorder the rules + orderRules: builder.mutation< + IRule, + { + namespaceKey: string; + flagKey: string; + ruleIds: string[]; + } + >({ + query({ namespaceKey, flagKey, ruleIds }) { + return { + url: `/namespaces/${namespaceKey}/flags/${flagKey}/rules/order`, + method: 'PUT', + body: { ruleIds: ruleIds } + }; + }, + async onQueryStarted( + { namespaceKey, flagKey, ruleIds }, + { dispatch, queryFulfilled } + ) { + // this is manual optimistic cache update of the listRules + // to set a desire order of rules of the listRules while server is updating the state. + // If we don't do this we will have very strange UI state with rules in old order + // until the result will be get from the server. It's very visible on slow connections. + const patchResult = dispatch( + rulesApi.util.updateQueryData( + 'listRules', + { namespaceKey, flagKey }, + (draft: IRuleList) => { + const rules = draft.rules; + const resortedRules = rules.sort((a, b) => { + const ida = ruleIds.indexOf(a.id); + const idb = ruleIds.indexOf(b.id); + if (ida < idb) { + return -1; + } else if (ida > idb) { + return 1; + } + return 0; + }); + return Object.assign(draft, { rules: resortedRules }); + } + ) + ); + try { + await queryFulfilled; + } catch { + patchResult.undo(); + } + }, + invalidatesTags: (_result, _error, { namespaceKey, flagKey }) => [ + { type: 'Rule', id: namespaceKey + '/' + flagKey } + ] + }), + // update the dustribution + updateDistribution: builder.mutation< + IRule, + { + namespaceKey: string; + flagKey: string; + ruleId: string; + distributionId: string; + values: IDistributionBase; + } + >({ + query({ namespaceKey, flagKey, ruleId, distributionId, values }) { + return { + url: `/namespaces/${namespaceKey}/flags/${flagKey}/rules/${ruleId}/distributions/${distributionId}`, + method: 'PUT', + body: values + }; + }, + invalidatesTags: (_result, _error, { namespaceKey, flagKey }) => [ + { type: 'Rule', id: namespaceKey + '/' + flagKey } + ] + }) + }) +}); +export const { + useListRulesQuery, + useCreateRuleMutation, + useDeleteRuleMutation, + useUpdateRuleMutation, + useOrderRulesMutation, + useUpdateDistributionMutation +} = rulesApi; diff --git a/ui/src/components/rules/forms/QuickEditRuleForm.tsx b/ui/src/components/rules/forms/QuickEditRuleForm.tsx index 000184dc65..6f50900d4e 100644 --- a/ui/src/components/rules/forms/QuickEditRuleForm.tsx +++ b/ui/src/components/rules/forms/QuickEditRuleForm.tsx @@ -2,18 +2,22 @@ import { Field, FieldArray, Form, Formik } from 'formik'; import { useState } from 'react'; import { useSelector } from 'react-redux'; import { twMerge } from 'tailwind-merge'; +import { + useUpdateDistributionMutation, + useUpdateRuleMutation +} from '~/app/flags/rulesApi'; import { selectReadonly } from '~/app/meta/metaSlice'; import { selectCurrentNamespace } from '~/app/namespaces/namespacesSlice'; import TextButton from '~/components/forms/buttons/TextButton'; import Combobox from '~/components/forms/Combobox'; import SegmentsPicker from '~/components/forms/SegmentsPicker'; import Loading from '~/components/Loading'; -import { updateDistribution, updateRule } from '~/data/api'; import { useError } from '~/data/hooks/error'; import { useSuccess } from '~/data/hooks/success'; import { DistributionType } from '~/types/Distribution'; import { IEvaluatable, IVariantRollout } from '~/types/Evaluatable'; import { IFlag } from '~/types/Flag'; +import { INamespace } from '~/types/Namespace'; import { FilterableSegment, ISegment, @@ -52,7 +56,7 @@ export default function QuickEditRuleForm(props: QuickEditRuleFormProps) { const { setError, clearError } = useError(); const { setSuccess } = useSuccess(); - const namespace = useSelector(selectCurrentNamespace); + const namespace = useSelector(selectCurrentNamespace) as INamespace; const ruleType = rule.rollouts.length === 0 @@ -82,6 +86,9 @@ export default function QuickEditRuleForm(props: QuickEditRuleFormProps) { const readOnly = useSelector(selectReadonly); + const [updateRule] = useUpdateRuleMutation(); + const [updateDistribution] = useUpdateDistributionMutation(); + const handleSubmit = async (values: RuleFormValues) => { const originalRuleSegments = rule.segments.map((s) => s.key); const comparableRuleSegments = values.segmentKeys.map((s) => s.key); @@ -94,10 +101,15 @@ export default function QuickEditRuleForm(props: QuickEditRuleFormProps) { if (!segmentsDidntChange || values.operator !== rule.operator) { // update segment if changed try { - await updateRule(namespace.key, flag.key, rule.id, { - rank: rule.rank, - segmentKeys: comparableRuleSegments, - segmentOperator: values.operator + await updateRule({ + namespaceKey: namespace.key, + flagKey: flag.key, + ruleId: rule.id, + values: { + rank: rule.rank, + segmentKeys: comparableRuleSegments, + segmentOperator: values.operator + } }); } catch (err) { setError(err as Error); @@ -116,16 +128,16 @@ export default function QuickEditRuleForm(props: QuickEditRuleFormProps) { found && found.distribution.rollout !== rollout.distribution.rollout ) { - return updateDistribution( - namespace.key, - flag.key, - rule.id, - rollout.distribution.id, - { + return updateDistribution({ + namespaceKey: namespace.key, + flagKey: flag.key, + ruleId: rule.id, + distributionId: rollout.distribution.id, + values: { variantId: rollout.distribution.variantId, rollout: rollout.distribution.rollout } - ); + }); } return Promise.resolve(); }) @@ -134,16 +146,16 @@ export default function QuickEditRuleForm(props: QuickEditRuleFormProps) { // update variant if changed if (rule.rollouts[0].distribution.variantId !== selectedVariant.id) { try { - await updateDistribution( - namespace.key, - flag.key, - rule.id, - rule.rollouts[0].distribution.id, - { + await updateDistribution({ + namespaceKey: namespace.key, + flagKey: flag.key, + ruleId: rule.id, + distributionId: rule.rollouts[0].distribution.id, + values: { variantId: selectedVariant.id, rollout: 100 } - ); + }); } catch (err) { setError(err as Error); return; diff --git a/ui/src/components/rules/forms/RuleForm.tsx b/ui/src/components/rules/forms/RuleForm.tsx index 9495473591..c63eddb649 100644 --- a/ui/src/components/rules/forms/RuleForm.tsx +++ b/ui/src/components/rules/forms/RuleForm.tsx @@ -5,12 +5,12 @@ import { useEffect, useState } from 'react'; import { useSelector } from 'react-redux'; import { Link } from 'react-router-dom'; import * as Yup from 'yup'; +import { useCreateRuleMutation } from '~/app/flags/rulesApi'; import { selectCurrentNamespace } from '~/app/namespaces/namespacesSlice'; import Button from '~/components/forms/buttons/Button'; import SegmentsPicker from '~/components/forms/SegmentsPicker'; import Loading from '~/components/Loading'; import MoreInfo from '~/components/MoreInfo'; -import { createDistribution, createRule } from '~/data/api'; import { useError } from '~/data/hooks/error'; import { useSuccess } from '~/data/hooks/success'; import { keyValidation } from '~/data/validations'; @@ -107,6 +107,8 @@ export default function RuleForm(props: RuleFormProps) { })); }); + const [createRule] = useCreateRuleMutation(); + const initialSegmentKeys: FilterableSegment[] = []; useEffect(() => { @@ -126,28 +128,25 @@ export default function RuleForm(props: RuleFormProps) { throw new Error('No segments selected'); } - const rule = await createRule(namespace.key, flag.key, { - segmentKeys: values.segmentKeys.map((s) => s.key), - segmentOperator: values.operator, - rank - }); - - if (ruleType === DistributionType.Multi) { - const distPromises = distributions?.map((dist: IDistributionVariant) => - createDistribution(namespace.key, flag.key, rule.id, { - variantId: dist.variantId, - rollout: dist.rollout - }) - ); - if (distPromises) await Promise.all(distPromises); + const dist = []; + if (ruleType === DistributionType.Multi && distributions) { + dist.push(...distributions); } else if (selectedVariant) { - // we allow creating rules without variants - - await createDistribution(namespace.key, flag.key, rule.id, { + dist.push({ variantId: selectedVariant.id, rollout: 100 }); } + return createRule({ + namespaceKey: namespace.key, + flagKey: flag.key, + values: { + segmentKeys: values.segmentKeys.map((s) => s.key), + segmentOperator: values.operator, + rank + }, + distributions: dist + }); }; return ( diff --git a/ui/src/data/api.ts b/ui/src/data/api.ts index 9c1f12e0e1..fbca87a06c 100644 --- a/ui/src/data/api.ts +++ b/ui/src/data/api.ts @@ -1,9 +1,7 @@ /* eslint-disable @typescript-eslint/no-use-before-define */ import { IAuthTokenBase } from '~/types/auth/Token'; -import { IDistributionBase } from '~/types/Distribution'; import { FlagType, IFlagBase } from '~/types/Flag'; import { IRolloutBase } from '~/types/Rollout'; -import { IRuleBase } from '~/types/Rule'; import { IVariantBase } from '~/types/Variant'; export const apiURL = '/api/v1'; @@ -247,92 +245,6 @@ export async function orderRollouts( return res.ok; } -// -// rules -export async function listRules(namespaceKey: string, flagKey: string) { - return get(`/namespaces/${namespaceKey}/flags/${flagKey}/rules`); -} - -export async function createRule( - namespaceKey: string, - flagKey: string, - values: IRuleBase -) { - return post(`/namespaces/${namespaceKey}/flags/${flagKey}/rules`, values); -} - -export async function updateRule( - namespaceKey: string, - flagKey: string, - ruleId: string, - values: IRuleBase -) { - return put( - `/namespaces/${namespaceKey}/flags/${flagKey}/rules/${ruleId}`, - values - ); -} - -export async function deleteRule( - namespaceKey: string, - flagKey: string, - ruleId: string -) { - return del(`/namespaces/${namespaceKey}/flags/${flagKey}/rules/${ruleId}`); -} - -export async function orderRules( - namespaceKey: string, - flagKey: string, - ruleIds: string[] -) { - const req = { - method: 'PUT', - headers: { - ...headerCsrf(), - 'Content-Type': 'application/json' - }, - body: JSON.stringify({ - ruleIds - }) - }; - - const res = await fetch( - `${apiURL}/namespaces/${namespaceKey}/flags/${flagKey}/rules/order`, - req - ); - if (!res.ok) { - const err = await res.json(); - throw new Error(err.message); - } - return res.ok; -} - -export async function createDistribution( - namespaceKey: string, - flagKey: string, - ruleId: string, - values: IDistributionBase -) { - return post( - `/namespaces/${namespaceKey}/flags/${flagKey}/rules/${ruleId}/distributions`, - values - ); -} - -export async function updateDistribution( - namespaceKey: string, - flagKey: string, - ruleId: string, - distributionId: string, - values: IDistributionBase -) { - return put( - `/namespaces/${namespaceKey}/flags/${flagKey}/rules/${ruleId}/distributions/${distributionId}`, - values - ); -} - // // variants export async function createVariant( diff --git a/ui/src/store.ts b/ui/src/store.ts index 909cb4a728..ed0e7a80c1 100644 --- a/ui/src/store.ts +++ b/ui/src/store.ts @@ -5,6 +5,7 @@ import { } from '@reduxjs/toolkit'; import { flagsSlice } from './app/flags/flagsSlice'; +import { rulesApi } from './app/flags/rulesApi'; import { metaSlice } from './app/meta/metaSlice'; import { namespacesSlice } from './app/namespaces/namespacesSlice'; import { @@ -69,12 +70,13 @@ export const store = configureStore({ preferences: preferencesSlice.reducer, flags: flagsSlice.reducer, meta: metaSlice.reducer, - [segmentsApi.reducerPath]: segmentsApi.reducer + [segmentsApi.reducerPath]: segmentsApi.reducer, + [rulesApi.reducerPath]: rulesApi.reducer }, middleware: (getDefaultMiddleware) => getDefaultMiddleware() .prepend(listenerMiddleware.middleware) - .concat(segmentsApi.middleware) + .concat(segmentsApi.middleware, rulesApi.middleware) }); // Infer the `RootState` and `AppDispatch` types from the store itself