Merge pull request #920 from acelaya-forks/visits-redux-type

Simplify visits reducers types with a status discriminator prop

Author: acelaya

src/visits/DomainVisits.tsx

@@ -38,7 +38,10 @@ const DomainVisitsBase = boundToMercureHub<DomainVisitsProps>(({ ReportExporter:
       visitsInfo={domainVisits}
       exportCsv={exportCsv}
     >
-      <VisitsHeader visits={domainVisits.visits} title={`"${authority}" visits`} />
+      <VisitsHeader
+        visits={domainVisits.status === 'loaded' ? domainVisits.visits : []}
+        title={`"${authority}" visits`}
+      />
     </VisitsStats>
   );
 }, () => [Topics.visits]);

src/visits/NonOrphanVisits.tsx

@@ -38,7 +38,10 @@ const NonOrphanVisitsBase = boundToMercureHub<NonOrphanVisitsProps>(({ ReportExp
       exportCsv={exportCsv}
       domains={domainsList.domains}
     >
-      <VisitsHeader title="Non-orphan visits" visits={nonOrphanVisits.visits} />
+      <VisitsHeader
+        title="Non-orphan visits"
+        visits={nonOrphanVisits.status === 'loaded' ? nonOrphanVisits.visits : []}
+      />
     </VisitsStats>
   );
 }, () => [Topics.visits]);

src/visits/OrphanVisits.tsx

@@ -47,7 +47,7 @@ const OrphanVisitsBase = boundToMercureHub<OrphanVisitsProps>(({ ReportExporter:
       isOrphanVisits
       domains={domainsList.domains}
     >
-      <VisitsHeader title="Orphan visits" visits={orphanVisits.visits} />
+      <VisitsHeader title="Orphan visits" visits={orphanVisits.status === 'loaded' ? orphanVisits.visits : []} />
     </VisitsStats>
   );
 }, () => [Topics.orphanVisits]);

src/visits/ShortUrlVisitsHeader.tsx

@@ -31,7 +31,7 @@ const Date = ({ shortUrl }: { shortUrl?: ShlinkShortUrl }) => {
 };
 
 export const ShortUrlVisitsHeader = ({ shortUrl, loading, shortUrlVisits }: ShortUrlVisitsHeaderProps) => {
-  const { visits } = shortUrlVisits;
+  const { visits = [] } = shortUrlVisits.status === 'loaded' ? shortUrlVisits : {};
   const shortLink = shortUrl?.shortUrl ?? '';
   const longLink = shortUrl?.longUrl ?? '';
   const title = shortUrl?.title;

src/visits/TagVisitsHeader.tsx

@@ -9,11 +9,11 @@ interface TagVisitsHeaderProps {
 }
 
 export const TagVisitsHeader = ({ tagVisits, colorGenerator }: TagVisitsHeaderProps) => {
-  const { visits, tag } = tagVisits;
+  const { tag, visits = [] } = tagVisits.status === 'loaded' ? tagVisits : {};
   const visitsStatsTitle = (
     <span className="flex items-center justify-center">
       <span className="mr-2">Visits for</span>
-      <Tag text={tag} colorGenerator={colorGenerator} />
+      {tag && <Tag text={tag} colorGenerator={colorGenerator} />}
     </span>
   );
 

src/visits/VisitsStats.tsx

@@ -91,7 +91,9 @@ export const VisitsStats: FC<VisitsStatsProps> = (props) => {
     isOrphanVisits = false,
     domains,
   } = props;
-  const { visits, prevVisits, loading, errorData, fallbackInterval } = visitsInfo;
+  const { status } = visitsInfo;
+  const loading = status === 'loading';
+  const { visits, prevVisits } = status === 'loaded' ? visitsInfo : { visits: [] };
   const [{ dateRange, visitsFilter, loadPrevInterval, domain }, updateQuery] = useVisitsQuery();
   const visitsSettings = useSetting('visits');
   const [activeInterval, setActiveInterval] = useState<DateInterval>();
@@ -107,8 +109,9 @@ export const VisitsStats: FC<VisitsStatsProps> = (props) => {
     },
     [updateQuery],
   );
+  const { fallbackInterval } = status === 'fallback' ? visitsInfo : {};
   const [currentFallbackInterval, setCurrentFallbackInterval] = useState<DateInterval>(
-    fallbackInterval ?? visitsSettings?.defaultInterval ?? 'last30Days',
+    status === 'fallback' ? visitsInfo.fallbackInterval : (visitsSettings?.defaultInterval ?? 'last30Days'),
   );
   const [highlightedVisits, setHighlightedVisits] = useState<NormalizedVisit[]>([]);
   const [highlightedLabel, setHighlightedLabel] = useState<string | undefined>();
@@ -183,7 +186,7 @@ export const VisitsStats: FC<VisitsStatsProps> = (props) => {
     if (fallbackInterval && currentFallbackInterval === (visitsSettings?.defaultInterval ?? 'last30Days')) {
       setCurrentFallbackInterval(fallbackInterval);
     }
-  }, [currentFallbackInterval, fallbackInterval, visitsSettings?.defaultInterval]);
+  }, [currentFallbackInterval, status, fallbackInterval, visitsSettings?.defaultInterval]);
 
   return (
     <div className="flex flex-col gap-y-4">
@@ -240,8 +243,8 @@ export const VisitsStats: FC<VisitsStatsProps> = (props) => {
       </section>
 
       <section className="flex flex-col gap-4">
-        <VisitsLoadingFeedback info={visitsInfo} />
-        {!loading && !errorData && (
+        {status !== 'loaded' && status !== 'fallback' && <VisitsLoadingFeedback info={visitsInfo} />}
+        {!loading && status !== 'error' && (
           <>
             <NavPills fill className="sticky top-(--header-height) z-2">
               {Object.values(sections).map(({ title, icon, subPath, shouldRender }: VisitsNavLinkOptions, index) => (

src/visits/helpers/VisitsLoadingFeedback.tsx

@@ -29,21 +29,24 @@ const ProgressBar: FC<ProgressBarProps> = ({ className, value, ...rest }) => {
 };
 
 export const VisitsLoadingFeedback: FC<VisitsLoadingFeedbackProps> = ({ info }) => {
-  const { loading, errorData, progress } = info;
-  return (
-    <>
-      {loading && progress === null && <Message loading />}
-      {loading && progress !== null && (
-        <Message loading>
-          This is going to take a while... :S
-          <ProgressBar value={progress} className="mt-4" />
-        </Message>
-      )}
-      {errorData && (
-        <Result variant="error">
-          <ShlinkApiError errorData={errorData} fallbackMessage="An error occurred while loading visits :(" />
-        </Result>
-      )}
-    </>
-  );
+  const { status } = info;
+
+  if (status === 'error') {
+    return (
+      <Result variant="error">
+        <ShlinkApiError errorData={info.error} fallbackMessage="An error occurred while loading visits :(" />
+      </Result>
+    );
+  }
+
+  if (status === 'loading') {
+    return info.progress === null ? <Message loading /> : (
+      <Message loading>
+        This is going to take a while... :S
+        <ProgressBar value={info.progress} className="mt-4" />
+      </Message>
+    );
+  }
+
+  return null;
 };

src/visits/reducers/common/createVisitsReducer.ts

@@ -2,46 +2,50 @@ import type { ActionReducerMapBuilder } from '@reduxjs/toolkit';
 import { createSlice } from '@reduxjs/toolkit';
 import { parseApiError } from '../../../api-contract/utils';
 import type { CreateVisit } from '../../types';
-import type { VisitsInfo } from '../types';
+import type { LoadVisits, VisitsInfo } from '../types';
 import { createNewVisits } from '../visitCreation';
 import type { createVisitsAsyncThunk } from './createVisitsAsyncThunk';
 
-interface VisitsReducerOptions<State extends VisitsInfo, AT extends ReturnType<typeof createVisitsAsyncThunk>> {
+interface VisitsReducerOptions<State, T extends LoadVisits> {
   name: string;
-  asyncThunk: AT;
-  initialState: State;
-  filterCreatedVisits: (state: State, createdVisits: CreateVisit[]) => CreateVisit[];
-  extraReducers?: (builder: ActionReducerMapBuilder<State>) => void;
+  asyncThunk: ReturnType<typeof createVisitsAsyncThunk<T>>;
+  initialState: VisitsInfo<State>;
+  filterCreatedVisits: (state: VisitsInfo<State>, createdVisits: CreateVisit[]) => CreateVisit[];
+  extraReducers?: (builder: ActionReducerMapBuilder<VisitsInfo<State>>) => void;
 }
 
-export const createVisitsReducer = <State extends VisitsInfo, AT extends ReturnType<typeof createVisitsAsyncThunk>>(
-  { name, asyncThunk, initialState, filterCreatedVisits, extraReducers }: VisitsReducerOptions<State, AT>,
+export const createVisitsReducer = <State, T extends LoadVisits>(
+  { name, asyncThunk, initialState, filterCreatedVisits, extraReducers }: VisitsReducerOptions<State, T>,
 ) => {
   const { pending, rejected, fulfilled, progressChanged, fallbackToInterval } = asyncThunk;
   const { reducer, actions } = createSlice({
     name,
     initialState,
     reducers: {
-      cancelGetVisits: (state) => ({ ...state, cancelLoad: true }),
+      cancelGetVisits: () => ({ status: 'canceled' as const }),
     },
     extraReducers: (builder) => {
-      builder.addCase(pending, () => ({ ...initialState, loading: true }));
-      builder.addCase(rejected, (_, { error }) => (
-        { ...initialState, errorData: parseApiError(error) ?? null }
-      ));
-      builder.addCase(fulfilled, (state, { payload }) => (
-        // Unpack the whole payload, as it could have different props depending on the concrete reducer
-        { ...state, ...payload, loading: false, progress: null, errorData: null }
+      builder.addCase(pending, () => ({ status: 'loading', progress: null }));
+      builder.addCase(progressChanged, (state, { payload: progress }) => (
+        // Update progress only if already loading
+        state.status !== 'loading' ? state : { status: 'loading', progress }
       ));
 
-      builder.addCase(progressChanged, (state, { payload: progress }) => ({ ...state, progress }));
-      builder.addCase(fallbackToInterval, (state, { payload: fallbackInterval }) => (
-        { ...state, fallbackInterval }
+      builder.addCase(rejected, (_, { error }) => ({ status: 'error', error: parseApiError(error) }));
+
+      // Unpack the whole payload, as it could have different props depending on the concrete reducer
+      builder.addCase(fulfilled, (state, { payload }) => ({ ...state, ...payload, status: 'loaded' }));
+
+      builder.addCase(fallbackToInterval, (_, { payload: fallbackInterval }) => (
+        { status: 'fallback', fallbackInterval }
       ));
 
       builder.addCase(createNewVisits, (state, { payload }) => {
+        if (state.status !== 'loaded') {
+          return state;
+        }
+
         const { visits } = state;
-        // @ts-expect-error TODO Fix type inference
         const newVisits = filterCreatedVisits(state, payload.createdVisits).map(({ visit }) => visit);
 
         return !newVisits.length ? state : { ...state, visits: [...newVisits, ...visits] };

src/visits/reducers/domainVisits.ts

@@ -9,21 +9,16 @@ import type { LoadVisits, VisitsInfo } from './types';
 
 const REDUCER_PREFIX = 'shlink/domainVisits';
 
-interface WithDomain {
+type WithDomain = {
   domain: string;
-}
+};
 
-export interface DomainVisits extends VisitsInfo, WithDomain {}
+export type DomainVisits = VisitsInfo<WithDomain>;
 
-export interface LoadDomainVisits extends LoadVisits, WithDomain {}
+export type LoadDomainVisits = LoadVisits & WithDomain;
 
 const initialState: DomainVisits = {
-  visits: [],
-  domain: '',
-  loading: false,
-  cancelLoad: false,
-  errorData: null,
-  progress: null,
+  status: 'idle',
 };
 
 export const getDomainVisitsThunk = createVisitsAsyncThunk({
@@ -37,19 +32,16 @@ export const getDomainVisitsThunk = createVisitsAsyncThunk({
 
     return { visitsLoader, lastVisitLoader };
   },
-  shouldCancel: (getState) => getState().domainVisits.cancelLoad,
+  shouldCancel: (getState) => getState().domainVisits.status === 'canceled',
 });
 
 export const { reducer: domainVisitsReducer, cancelGetVisits: cancelGetDomainVisits } = createVisitsReducer({
   name: REDUCER_PREFIX,
-  initialState,
-  // @ts-expect-error TODO Fix type inference
+  initialState: initialState as DomainVisits,
   asyncThunk: getDomainVisitsThunk,
-  filterCreatedVisits: ({ domain, params }, createdVisits) => filterCreatedVisitsByDomain(
-    createdVisits,
-    domain,
-    params?.dateRange,
-  ),
+  filterCreatedVisits: (state, createdVisits) => state.status !== 'loaded'
+    ? createdVisits
+    : filterCreatedVisitsByDomain(createdVisits, state.domain, state.params?.dateRange),
 });
 
 export const useDomainVisits = () => {

src/visits/reducers/nonOrphanVisits.ts

@@ -10,11 +10,7 @@ import type { LoadWithDomainVisits, VisitsInfo } from './types';
 const REDUCER_PREFIX = 'shlink/orphanVisits';
 
 const initialState: VisitsInfo = {
-  visits: [],
-  loading: false,
-  cancelLoad: false,
-  errorData: null,
-  progress: null,
+  status: 'idle',
 };
 
 export const getNonOrphanVisitsThunk = createVisitsAsyncThunk({
@@ -28,16 +24,19 @@ export const getNonOrphanVisitsThunk = createVisitsAsyncThunk({
 
     return { visitsLoader, lastVisitLoader };
   },
-  shouldCancel: (getState) => getState().orphanVisits.cancelLoad,
+  shouldCancel: (getState) => getState().nonOrphanVisits.status === 'canceled',
 });
 
 export const { reducer: nonOrphanVisitsReducer, cancelGetVisits: cancelGetNonOrphanVisits } = createVisitsReducer({
   name: REDUCER_PREFIX,
-  initialState,
-  // @ts-expect-error TODO Fix type inference
+  initialState: initialState as VisitsInfo,
   asyncThunk: getNonOrphanVisitsThunk,
-  filterCreatedVisits: ({ params }, createdVisits) => {
-    const { startDate, endDate } = params?.dateRange ?? {};
+  filterCreatedVisits: (state, createdVisits) => {
+    if (state.status !== 'loaded') {
+      return createdVisits;
+    }
+
+    const { startDate, endDate } = state.params?.dateRange ?? {};
     return createdVisits.filter(({ visit }) => isBetween(visit.date, startDate, endDate));
   },
 });