Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(ui): move Segments to use Redux storage #2441

Merged
merged 12 commits into from
Nov 27, 2023
18 changes: 5 additions & 13 deletions ui/src/app/flags/rollouts/Rollouts.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { useCallback, useEffect, useRef, useState } from 'react';
import { useSelector } from 'react-redux';
import { selectReadonly } from '~/app/meta/metaSlice';
import { selectCurrentNamespace } from '~/app/namespaces/namespacesSlice';
import { useListSegmentsQuery } from '~/app/segments/segmentsApi';
import Button from '~/components/forms/buttons/Button';
import Modal from '~/components/Modal';
import DeletePanel from '~/components/panels/DeletePanel';
Expand All @@ -26,17 +27,12 @@ import RolloutForm from '~/components/rollouts/forms/RolloutForm';
import Rollout from '~/components/rollouts/Rollout';
import SortableRollout from '~/components/rollouts/SortableRollout';
import Slideover from '~/components/Slideover';
import {
deleteRollout,
listRollouts,
listSegments,
orderRollouts
} from '~/data/api';
import { deleteRollout, listRollouts, orderRollouts } from '~/data/api';
import { useError } from '~/data/hooks/error';
import { useSuccess } from '~/data/hooks/success';
import { IFlag } from '~/types/Flag';
import { IRollout, IRolloutList } from '~/types/Rollout';
import { ISegment, ISegmentList, SegmentOperatorType } from '~/types/Segment';
import { SegmentOperatorType } from '~/types/Segment';

type RolloutsProps = {
flag: IFlag;
Expand All @@ -45,7 +41,6 @@ type RolloutsProps = {
export default function Rollouts(props: RolloutsProps) {
const { flag } = props;

const [segments, setSegments] = useState<ISegment[]>([]);
const [rollouts, setRollouts] = useState<IRollout[]>([]);

const [activeRollout, setActiveRollout] = useState<IRollout | null>(null);
Expand All @@ -68,13 +63,10 @@ export default function Rollouts(props: RolloutsProps) {

const namespace = useSelector(selectCurrentNamespace);
const readOnly = useSelector(selectReadonly);
const segments = useListSegmentsQuery(namespace.key)?.data?.segments || [];

const loadData = useCallback(async () => {
// TODO: move to redux
const segmentList = (await listSegments(namespace.key)) as ISegmentList;
const { segments } = segmentList;
setSegments(segments);

const rolloutList = (await listRollouts(
namespace.key,
flag.key
Expand Down Expand Up @@ -284,7 +276,7 @@ export default function Rollouts(props: RolloutsProps) {
</p>
</div>
<div
className="border-gray-200 pattern-boxes w-full border p-4 pattern-bg-gray-50 pattern-gray-100 pattern-opacity-100 pattern-size-2 dark:pattern-bg-black dark:pattern-gray-900
className="border-gray-200 pattern-boxes w-full border p-4 pattern-bg-gray-50 pattern-gray-100 pattern-opacity-100 pattern-size-2 dark:pattern-bg-black dark:pattern-gray-900
lg:p-6"
>
{rollouts && rollouts.length > 0 && (
Expand Down
16 changes: 7 additions & 9 deletions ui/src/app/flags/rules/Rules.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import { FlagProps } from '~/app/flags/FlagProps';
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 Modal from '~/components/Modal';
Expand All @@ -28,20 +29,19 @@
import Rule from '~/components/rules/Rule';
import SortableRule from '~/components/rules/SortableRule';
import Slideover from '~/components/Slideover';
import { deleteRule, listRules, listSegments, orderRules } from '~/data/api';
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 { ISegment, ISegmentList, SegmentOperatorType } from '~/types/Segment';
import { ISegment, SegmentOperatorType } from '~/types/Segment';
import { IVariant } from '~/types/Variant';

export default function Rules() {
const { flag } = useOutletContext<FlagProps>();

const [segments, setSegments] = useState<ISegment[]>([]);
const [rules, setRules] = useState<IEvaluatable[]>([]);

const [activeRule, setActiveRule] = useState<IEvaluatable | null>(null);
Expand All @@ -61,12 +61,10 @@
const namespace = useSelector(selectCurrentNamespace);
const readOnly = useSelector(selectReadonly);

const segments = useListSegmentsQuery(namespace.key)?.data?.segments || [];

Check warning on line 64 in ui/src/app/flags/rules/Rules.tsx

View workflow job for this annotation

GitHub Actions / Lint UI

The 'segments' logical expression could make the dependencies of useCallback Hook (at line 134) change on every render. To fix this, wrap the initialization of 'segments' in its own useMemo() Hook

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@erka is this something to worry about? ☝🏻

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CleanShot 2023-11-26 at 12 53 59

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I modified the code to address it.

const loadData = useCallback(async () => {
// TODO: move to redux
const segmentList = (await listSegments(namespace.key)) as ISegmentList;
const { segments } = segmentList;
setSegments(segments);

const ruleList = (await listRules(namespace.key, flag.key)) as IRuleList;

const rules = ruleList.rules.flatMap((rule: IRule) => {
Expand Down Expand Up @@ -133,7 +131,7 @@
});

setRules(rules);
}, [namespace.key, flag]);
}, [namespace.key, flag, segments]);

const incrementRulesVersion = () => {
setRulesVersion(rulesVersion + 1);
Expand Down Expand Up @@ -283,7 +281,7 @@
</p>
</div>
<div
className="border-gray-200 pattern-boxes w-full border p-4 pattern-bg-gray-50 pattern-gray-100 pattern-opacity-100 pattern-size-2 dark:pattern-bg-black dark:pattern-gray-900
className="border-gray-200 pattern-boxes w-full border p-4 pattern-bg-gray-50 pattern-gray-100 pattern-opacity-100 pattern-size-2 dark:pattern-bg-black dark:pattern-gray-900
lg:w-3/4 lg:p-6"
>
<DndContext
Expand Down
81 changes: 39 additions & 42 deletions ui/src/app/segments/Segment.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,19 @@ import {
TrashIcon
} from '@heroicons/react/24/outline';
import { formatDistanceToNowStrict, parseISO } from 'date-fns';
import { useEffect, useMemo, useRef, useState } from 'react';
import { useMemo, useRef, useState } from 'react';
import { useSelector } from 'react-redux';
import { useNavigate, useParams } from 'react-router-dom';
import { selectReadonly } from '~/app/meta/metaSlice';
import {
selectCurrentNamespace,
selectNamespaces
} from '~/app/namespaces/namespacesSlice';
import {
useDeleteConstraintMutation,
useDeleteSegmentMutation,
useGetSegmentQuery
} from '~/app/segments/segmentsApi';
import Chips from '~/components/Chips';
import EmptyState from '~/components/EmptyState';
import Button from '~/components/forms/buttons/Button';
Expand All @@ -24,12 +29,7 @@ import DeletePanel from '~/components/panels/DeletePanel';
import ConstraintForm from '~/components/segments/ConstraintForm';
import SegmentForm from '~/components/segments/SegmentForm';
import Slideover from '~/components/Slideover';
import {
copySegment,
deleteConstraint,
deleteSegment,
getSegment
} from '~/data/api';
import { copySegment } from '~/data/api';
import { useError } from '~/data/hooks/error';
import { useSuccess } from '~/data/hooks/success';
import { useTimezone } from '~/data/hooks/timezone';
Expand All @@ -39,7 +39,6 @@ import {
constraintTypeToLabel,
IConstraint
} from '~/types/Constraint';
import { ISegment } from '~/types/Segment';

function ConstraintArrayValue({ value }: { value: string | undefined }) {
const items: string[] | number[] = useMemo(() => {
Expand Down Expand Up @@ -75,9 +74,6 @@ export default function Segment() {
let { segmentKey } = useParams();
const { inTimezone } = useTimezone();

const [segment, setSegment] = useState<ISegment | null>(null);
const [segmentVersion, setSegmentVersion] = useState(0);

const [showConstraintForm, setShowConstraintForm] = useState<boolean>(false);
const [editingConstraint, setEditingConstraint] =
useState<IConstraint | null>(null);
Expand All @@ -99,26 +95,27 @@ export default function Segment() {
const namespace = useSelector(selectCurrentNamespace);
const readOnly = useSelector(selectReadonly);

const incrementSegmentVersion = () => {
setSegmentVersion(segmentVersion + 1);
};

useEffect(() => {
if (!segmentKey) return;

getSegment(namespace.key, segmentKey)
.then((segment: ISegment) => {
setSegment(segment);
clearError();
})
.catch((err) => {
setError(err);
});
}, [segmentVersion, namespace.key, segmentKey, clearError, setError]);
const {
data: segment,
error,
isLoading,
isError
} = useGetSegmentQuery({
namespaceKey: namespace.key,
segmentKey: segmentKey || ''
});
const [deleteSegment] = useDeleteSegmentMutation();
const [deleteSegmentConstraint] = useDeleteConstraintMutation();

const constraintFormRef = useRef(null);

if (!segment) return <Loading />;
if (isError) {
setError(error);
}

if (isLoading || !segment) {
return <Loading />;
}

return (
<>
Expand All @@ -134,7 +131,7 @@ export default function Segment() {
constraint={editingConstraint || undefined}
setOpen={setShowConstraintForm}
onSuccess={() => {
incrementSegmentVersion();
clearError();
setShowConstraintForm(false);
}}
/>
Expand All @@ -159,15 +156,13 @@ export default function Segment() {
setOpen={setShowDeleteConstraintModal}
handleDelete={
() =>
deleteConstraint(
namespace.key,
segment.key,
deletingConstraint?.id ?? ''
) // TODO: Determine impact of blank ID param
deleteSegmentConstraint({
namespaceKey: namespace.key,
segmentKey: segment.key,
constraintId: deletingConstraint?.id ?? ''
}) // TODO: Determine impact of blank ID param
}
onSuccess={() => {
incrementSegmentVersion();
}}
onSuccess={() => {}}
/>
</Modal>

Expand All @@ -183,7 +178,12 @@ export default function Segment() {
}
panelType="Segment"
setOpen={setShowDeleteSegmentModal}
handleDelete={() => deleteSegment(namespace.key, segment.key)}
handleDelete={() => {
return deleteSegment({
namespaceKey: namespace.key,
segmentKey: segment.key
});
}}
onSuccess={() => {
navigate(`/namespaces/${namespace.key}/segments`);
}}
Expand Down Expand Up @@ -279,10 +279,7 @@ export default function Segment() {
</MoreInfo>
</div>
<div className="mt-5 md:col-span-2 md:mt-0">
<SegmentForm
segment={segment}
segmentChanged={incrementSegmentVersion}
/>
<SegmentForm segment={segment} />
</div>
</div>
</div>
Expand Down
9 changes: 3 additions & 6 deletions ui/src/app/segments/Segments.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,21 @@ import { PlusIcon } from '@heroicons/react/24/outline';
import { useEffect } from 'react';
import { useSelector } from 'react-redux';
import { Link, useNavigate } from 'react-router-dom';
import useSWR from 'swr';
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 SegmentTable from '~/components/segments/SegmentTable';
import { useError } from '~/data/hooks/error';
import { ISegmentList } from '~/types/Segment';

export default function Segments() {
const namespace = useSelector(selectCurrentNamespace);

const path = `/namespaces/${namespace.key}/segments`;

const { data, error } = useSWR<ISegmentList>(path);

const segments = data?.segments;

const { data, error } = useListSegmentsQuery(namespace.key);
const segments = data?.segments || [];
const navigate = useNavigate();
const { setError, clearError } = useError();

Expand Down
Loading
Loading