Enhance visits overview reducer to handle bot and non-bots visits amounts

src/visits/reducers/visitsOverview.ts

@@ -3,14 +3,24 @@ import { createSlice } from '@reduxjs/toolkit';
 import type { ShlinkApiClientBuilder } from '../../api/services/ShlinkApiClientBuilder';
 import type { ShlinkVisitsOverview } from '../../api/types';
 import { createAsyncThunk } from '../../utils/helpers/redux';
+import type { CreateVisit } from '../types';
 import { groupNewVisitsByType } from '../types/helpers';
 import { createNewVisits } from './visitCreation';
 
 const REDUCER_PREFIX = 'shlink/visitsOverview';
 
-export interface VisitsOverview {
+export type PartialVisitsSummary = {
-  visitsCount: number;
+  total: number;
-  orphanVisitsCount: number;
+  nonBots?: number;
+  bots?: number;
+};
+
+export type ParsedVisitsOverview = {
+  nonOrphanVisits: PartialVisitsSummary;
+  orphanVisits: PartialVisitsSummary;
+};
+
+export interface VisitsOverview extends ParsedVisitsOverview {
   loading: boolean;
   error: boolean;
 }
@@ -18,15 +28,34 @@ export interface VisitsOverview {
 export type GetVisitsOverviewAction = PayloadAction<ShlinkVisitsOverview>;
 
 const initialState: VisitsOverview = {
-  visitsCount: 0,
+  nonOrphanVisits: {
-  orphanVisitsCount: 0,
+    total: 0,
+  },
+  orphanVisits: {
+    total: 0,
+  },
   loading: false,
   error: false,
 };
 
+const countBots = (visits: CreateVisit[]) => visits.filter(({ visit }) => visit.potentialBot).length;
+
 export const loadVisitsOverview = (buildShlinkApiClient: ShlinkApiClientBuilder) => createAsyncThunk(
   `${REDUCER_PREFIX}/loadVisitsOverview`,
-  (_: void, { getState }): Promise<ShlinkVisitsOverview> => buildShlinkApiClient(getState).getVisitsOverview(),
+  (_: void, { getState }): Promise<ParsedVisitsOverview> => buildShlinkApiClient(getState).getVisitsOverview().then(
+    (resp) => ({
+      nonOrphanVisits: {
+        total: resp.nonOrphanVisits?.total ?? resp.visitsCount,
+        nonBots: resp.nonOrphanVisits?.nonBots,
+        bots: resp.nonOrphanVisits?.bots,
+      },
+      orphanVisits: {
+        total: resp.orphanVisits?.total ?? resp.orphanVisitsCount,
+        nonBots: resp.orphanVisits?.nonBots,
+        bots: resp.orphanVisits?.bots,
+      },
+    }),
+  ),
 );
 
 export const visitsOverviewReducerCreator = (
@@ -40,13 +69,31 @@ export const visitsOverviewReducerCreator = (
     builder.addCase(loadVisitsOverviewThunk.rejected, () => ({ ...initialState, error: true }));
     builder.addCase(loadVisitsOverviewThunk.fulfilled, (_, { payload }) => ({ ...initialState, ...payload }));
 
-    builder.addCase(createNewVisits, ({ visitsCount, orphanVisitsCount = 0, ...rest }, { payload }) => {
+    builder.addCase(createNewVisits, ({ nonOrphanVisits, orphanVisits, ...rest }, { payload }) => {
-      const { createdVisits } = payload;
+      const { nonOrphanVisits: newNonOrphanVisits, orphanVisits: newOrphanVisits } = groupNewVisitsByType(
-      const { regularVisits, orphanVisits } = groupNewVisitsByType(createdVisits);
+        payload.createdVisits,
+      );
+
+      const newNonOrphanTotalVisits = newNonOrphanVisits.length;
+      const newNonOrphanBotVisits = countBots(newNonOrphanVisits);
+      const newNonOrphanNonBotVisits = newNonOrphanTotalVisits - newNonOrphanBotVisits;
+
+      const newOrphanTotalVisits = newOrphanVisits.length;
+      const newOrphanBotVisits = countBots(newOrphanVisits);
+      const newOrphanNonBotVisits = newOrphanTotalVisits - newOrphanBotVisits;
+
       return {
         ...rest,
-        visitsCount: visitsCount + regularVisits.length,
+        nonOrphanVisits: {
-        orphanVisitsCount: orphanVisitsCount + orphanVisits.length,
+          total: nonOrphanVisits.total + newNonOrphanTotalVisits,
+          bots: nonOrphanVisits.bots && nonOrphanVisits.bots + newNonOrphanBotVisits,
+          nonBots: nonOrphanVisits.nonBots && nonOrphanVisits.nonBots + newNonOrphanNonBotVisits,
+        },
+        orphanVisits: {
+          total: orphanVisits.total + newOrphanTotalVisits,
+          bots: orphanVisits.bots && orphanVisits.bots + newOrphanBotVisits,
+          nonBots: orphanVisits.nonBots && orphanVisits.nonBots + newOrphanNonBotVisits,
+        },
       };
     });
   },

src/visits/types/helpers.ts

@@ -10,13 +10,13 @@ export const isNormalizedOrphanVisit = (visit: NormalizedVisit): visit is Normal
 
 export interface GroupedNewVisits {
   orphanVisits: CreateVisit[];
-  regularVisits: CreateVisit[];
+  nonOrphanVisits: CreateVisit[];
 }
 
 export const groupNewVisitsByType = pipe(
-  groupBy((newVisit: CreateVisit) => (isOrphanVisit(newVisit.visit) ? 'orphanVisits' : 'regularVisits')),
+  groupBy((newVisit: CreateVisit) => (isOrphanVisit(newVisit.visit) ? 'orphanVisits' : 'nonOrphanVisits')),
   // @ts-expect-error Type declaration on groupBy is not correct. It can return undefined props
-  (result): GroupedNewVisits => ({ orphanVisits: [], regularVisits: [], ...result }),
+  (result): GroupedNewVisits => ({ orphanVisits: [], nonOrphanVisits: [], ...result }),
 );
 
 export type HighlightableProps<T extends NormalizedVisit> = T extends NormalizedOrphanVisit

test/visits/reducers/visitsOverview.test.ts

@@ -5,8 +5,8 @@ import type { ShlinkState } from '../../../src/container/types';
 import type { CreateVisitsAction } from '../../../src/visits/reducers/visitCreation';
 import { createNewVisits } from '../../../src/visits/reducers/visitCreation';
 import type {
-  GetVisitsOverviewAction,
+  GetVisitsOverviewAction, ParsedVisitsOverview,
-  VisitsOverview } from '../../../src/visits/reducers/visitsOverview';
+  PartialVisitsSummary, VisitsOverview } from '../../../src/visits/reducers/visitsOverview';
 import {
   loadVisitsOverview as loadVisitsOverviewCreator,
   visitsOverviewReducerCreator,
@@ -46,27 +46,26 @@ describe('visitsOverviewReducer', () => {
     });
 
     it('return visits overview on GET_OVERVIEW', () => {
-      const { loading, error, visitsCount } = reducer(state({ loading: true, error: false }), {
+      const action = loadVisitsOverview.fulfilled(Mock.of<ParsedVisitsOverview>({
-        type: loadVisitsOverview.fulfilled.toString(),
+        nonOrphanVisits: { total: 100 },
-        payload: { visitsCount: 100 },
+      }), 'requestId');
-      });
+      const { loading, error, nonOrphanVisits } = reducer(state({ loading: true, error: false }), action);
 
       expect(loading).toEqual(false);
       expect(error).toEqual(false);
-      expect(visitsCount).toEqual(100);
+      expect(nonOrphanVisits.total).toEqual(100);
     });
 
     it.each([
       [50, 53],
       [0, 3],
-      [undefined, 3],
     ])('returns updated amounts on CREATE_VISITS', (providedOrphanVisitsCount, expectedOrphanVisitsCount) => {
-      const { visitsCount, orphanVisitsCount } = reducer(
+      const { nonOrphanVisits, orphanVisits } = reducer(
-        state({ visitsCount: 100, orphanVisitsCount: providedOrphanVisitsCount }),
+        state({
-        {
+          nonOrphanVisits: { total: 100 },
-          type: createNewVisits.toString(),
+          orphanVisits: { total: providedOrphanVisitsCount },
-          payload: {
+        }),
-            createdVisits: [
+        createNewVisits([
           Mock.of<CreateVisit>({ visit: Mock.all<Visit>() }),
           Mock.of<CreateVisit>({ visit: Mock.all<Visit>() }),
           Mock.of<CreateVisit>({
@@ -78,13 +77,67 @@ describe('visitsOverviewReducer', () => {
           Mock.of<CreateVisit>({
             visit: Mock.of<OrphanVisit>({ visitedUrl: '' }),
           }),
-            ],
+        ]),
-          },
-        } as unknown as GetVisitsOverviewAction & CreateVisitsAction,
       );
 
-      expect(visitsCount).toEqual(102);
+      expect(nonOrphanVisits.total).toEqual(102);
-      expect(orphanVisitsCount).toEqual(expectedOrphanVisitsCount);
+      expect(orphanVisits.total).toEqual(expectedOrphanVisitsCount);
+    });
+
+    it.each([
+      [
+        {} satisfies Omit<PartialVisitsSummary, 'total'>,
+        {} satisfies Omit<PartialVisitsSummary, 'total'>,
+        { total: 103 } satisfies PartialVisitsSummary,
+        { total: 203 } satisfies PartialVisitsSummary,
+      ],
+      [
+        { bots: 35 } satisfies Omit<PartialVisitsSummary, 'total'>,
+        { bots: 35 } satisfies Omit<PartialVisitsSummary, 'total'>,
+        { total: 103, bots: 37 } satisfies PartialVisitsSummary,
+        { total: 203, bots: 36 } satisfies PartialVisitsSummary,
+      ],
+      [
+        { nonBots: 41, bots: 85 } satisfies Omit<PartialVisitsSummary, 'total'>,
+        { nonBots: 63, bots: 27 } satisfies Omit<PartialVisitsSummary, 'total'>,
+        { total: 103, nonBots: 42, bots: 87 } satisfies PartialVisitsSummary,
+        { total: 203, nonBots: 65, bots: 28 } satisfies PartialVisitsSummary,
+      ],
+      [
+        { nonBots: 56 } satisfies Omit<PartialVisitsSummary, 'total'>,
+        { nonBots: 99 } satisfies Omit<PartialVisitsSummary, 'total'>,
+        { total: 103, nonBots: 57 } satisfies PartialVisitsSummary,
+        { total: 203, nonBots: 101 } satisfies PartialVisitsSummary,
+      ],
+    ])('takes bots and non-bots into consideration when creating visits', (
+      initialNonOrphanVisits,
+      initialOrphanVisits,
+      expectedNonOrphanVisits,
+      expectedOrphanVisits,
+    ) => {
+      const { nonOrphanVisits, orphanVisits } = reducer(
+        state({
+          nonOrphanVisits: { total: 100, ...initialNonOrphanVisits },
+          orphanVisits: { total: 200, ...initialOrphanVisits },
+        }),
+        createNewVisits([
+          Mock.of<CreateVisit>({ visit: Mock.all<Visit>() }),
+          Mock.of<CreateVisit>({ visit: Mock.of<Visit>({ potentialBot: true }) }),
+          Mock.of<CreateVisit>({ visit: Mock.of<Visit>({ potentialBot: true }) }),
+          Mock.of<CreateVisit>({
+            visit: Mock.of<OrphanVisit>({ visitedUrl: '' }),
+          }),
+          Mock.of<CreateVisit>({
+            visit: Mock.of<OrphanVisit>({ visitedUrl: '' }),
+          }),
+          Mock.of<CreateVisit>({
+            visit: Mock.of<OrphanVisit>({ visitedUrl: '', potentialBot: true }),
+          }),
+        ]),
+      );
+
+      expect(nonOrphanVisits).toEqual(expectedNonOrphanVisits);
+      expect(orphanVisits).toEqual(expectedOrphanVisits);
     });
   });
 
@@ -109,8 +162,30 @@ describe('visitsOverviewReducer', () => {
       expect(getVisitsOverview).toHaveBeenCalledTimes(1);
     });
 
-    it('dispatches start and success when promise is resolved', async () => {
+    it.each([
-      const resolvedOverview = Mock.of<ShlinkVisitsOverview>({ visitsCount: 50 });
+      [
+        // Shlink <3.5.0
+        { visitsCount: 50, orphanVisitsCount: 20 } satisfies ShlinkVisitsOverview,
+        {
+          nonOrphanVisits: { total: 50, nonBots: undefined, bots: undefined },
+          orphanVisits: { total: 20, nonBots: undefined, bots: undefined },
+        },
+      ],
+      [
+        // Shlink >=3.5.0
+        {
+          nonOrphanVisits: { total: 50, nonBots: 20, bots: 30 },
+          orphanVisits: { total: 50, nonBots: 20, bots: 30 },
+          visitsCount: 3,
+          orphanVisitsCount: 3,
+        } satisfies ShlinkVisitsOverview,
+        {
+          nonOrphanVisits: { total: 50, nonBots: 20, bots: 30 },
+          orphanVisits: { total: 50, nonBots: 20, bots: 30 },
+        },
+      ],
+    ])('dispatches start and success when promise is resolved', async (serverResult, dispatchedPayload) => {
+      const resolvedOverview = Mock.of<ShlinkVisitsOverview>(serverResult);
       getVisitsOverview.mockResolvedValue(resolvedOverview);
 
       await loadVisitsOverview()(dispatchMock, getState, {});
@@ -121,7 +196,7 @@ describe('visitsOverviewReducer', () => {
       }));
       expect(dispatchMock).toHaveBeenNthCalledWith(2, expect.objectContaining({
         type: loadVisitsOverview.fulfilled.toString(),
-        payload: { visitsCount: 50 },
+        payload: dispatchedPayload,
       }));
       expect(getVisitsOverview).toHaveBeenCalledTimes(1);
     });

test/visits/types/helpers.test.ts

@@ -8,7 +8,7 @@ import { groupNewVisitsByType, toApiParams } from '../../../src/visits/types/hel
 describe('visitsTypeHelpers', () => {
   describe('groupNewVisitsByType', () => {
     it.each([
-      [[], { orphanVisits: [], regularVisits: [] }],
+      [[], { orphanVisits: [], nonOrphanVisits: [] }],
       ((): [CreateVisit[], GroupedNewVisits] => {
         const orphanVisits: CreateVisit[] = [
           Mock.of<CreateVisit>({
@@ -18,7 +18,7 @@ describe('visitsTypeHelpers', () => {
             visit: Mock.of<OrphanVisit>({ visitedUrl: '' }),
           }),
         ];
-        const regularVisits: CreateVisit[] = [
+        const nonOrphanVisits: CreateVisit[] = [
           Mock.of<CreateVisit>({ visit: Mock.all<Visit>() }),
           Mock.of<CreateVisit>({ visit: Mock.all<Visit>() }),
           Mock.of<CreateVisit>({ visit: Mock.all<Visit>() }),
@@ -27,8 +27,8 @@ describe('visitsTypeHelpers', () => {
         ];
 
         return [
-          [...orphanVisits, ...regularVisits],
+          [...orphanVisits, ...nonOrphanVisits],
-          { orphanVisits, regularVisits },
+          { orphanVisits, nonOrphanVisits },
         ];
       })(),
       ((): [CreateVisit[], GroupedNewVisits] => {
@@ -44,16 +44,16 @@ describe('visitsTypeHelpers', () => {
           }),
         ];
 
-        return [orphanVisits, { orphanVisits, regularVisits: [] }];
+        return [orphanVisits, { orphanVisits, nonOrphanVisits: [] }];
       })(),
       ((): [CreateVisit[], GroupedNewVisits] => {
-        const regularVisits: CreateVisit[] = [
+        const nonOrphanVisits: CreateVisit[] = [
           Mock.of<CreateVisit>({ visit: Mock.all<Visit>() }),
           Mock.of<CreateVisit>({ visit: Mock.all<Visit>() }),
           Mock.of<CreateVisit>({ visit: Mock.all<Visit>() }),
         ];
 
-        return [regularVisits, { orphanVisits: [], regularVisits }];
+        return [nonOrphanVisits, { orphanVisits: [], nonOrphanVisits }];
       })(),
     ])('groups new visits as expected', (createdVisits, expectedResult) => {
       expect(groupNewVisitsByType(createdVisits)).toEqual(expectedResult);