Skip to content

Commit

Permalink
Merge pull request #1930 from Hyperkid123/throttle-last-visited-calls
Browse files Browse the repository at this point in the history
Throttle last visited calls.
  • Loading branch information
Hyperkid123 authored Nov 1, 2023
2 parents ac37a38 + f27c734 commit e10ccfd
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 21 deletions.
8 changes: 4 additions & 4 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions packages/chrome/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@
"url": "https://github.com/RedHatInsights/frontend-components/issues"
},
"homepage": "https://github.com/RedHatInsights/frontend-components/tree/master/packages/chrome#readme",
"dependencies": {
"lodash": "^4.17.21"
},
"peerDependencies": {
"@scalprum/react-core": "^0.5.1",
"react": "^18.2.0",
Expand Down
18 changes: 8 additions & 10 deletions packages/chrome/src/ChromeProvider/ChromeProvider.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ import { PluginStore } from '@openshift/dynamic-plugin-sdk';
import ChromeProvider from './ChromeProvider';
import * as fetch from '../utils/fetch';

const flushPromises = () => new Promise(setImmediate);

describe('ChromeProvider', () => {
const getSpy = jest.spyOn(fetch, 'get');
const postSpy = jest.spyOn(fetch, 'post');
Expand All @@ -34,6 +32,7 @@ describe('ChromeProvider', () => {
});

test('should post new data on title change', async () => {
jest.useFakeTimers();
getSpy.mockResolvedValueOnce([]);
postSpy.mockResolvedValue(['/', '/bar']);
const DocumentMutator = () => {
Expand Down Expand Up @@ -75,17 +74,19 @@ describe('ChromeProvider', () => {
screen.getByText('/foo/bar').click();
});

// debounce timer
// wait for calls to be finished
await act(async () => {
await flushPromises();
await jest.advanceTimersByTime(5001);
});
expect(postSpy).toHaveBeenCalledTimes(2);
// should be called anly once because of the debounce
expect(postSpy).toHaveBeenCalledTimes(1);
expect(postSpy).toHaveBeenLastCalledWith('/api/chrome-service/v1/last-visited', {
pathname: '/foo/bar',
title: 'Foo title',
bundle: 'bundle-title',
});
});
}, 10000);

test('should not update state on mount if received error response', async () => {
const errorResponse = { errors: [{ status: 404, meta: { response_by: 'gateway' }, detail: 'Undefined Insights application' }] };
Expand Down Expand Up @@ -116,11 +117,8 @@ describe('ChromeProvider', () => {
);
});

expect(consoleSpy).toHaveBeenCalledTimes(2);
expect(consoleSpy.mock.calls).toEqual([
['Unable to update last visited pages!', errorResponse],
['Unable to initialize ChromeProvider!', errorResponse],
]);
expect(consoleSpy).toHaveBeenCalledTimes(1);
expect(consoleSpy.mock.calls).toEqual([['Unable to initialize ChromeProvider!', errorResponse]]);
consoleSpy.mockRestore();
});
});
22 changes: 16 additions & 6 deletions packages/chrome/src/ChromeProvider/ChromeProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,33 @@ import React, { useEffect, useRef, useState } from 'react';
import { useScalprum } from '@scalprum/react-core';
import { ChromeAPI } from '@redhat-cloud-services/types';
import { useLocation } from 'react-router-dom';
import debounce from 'lodash/debounce';

import { ChromeContext } from '../ChromeContext';
import chromeState, { LastVisitedPage, UserIdentity } from './chromeState';
import { IDENTITY_URL, LAST_VISITED_URL, get, post } from '../utils/fetch';

const getUserIdentity = () => get<UserIdentity>(IDENTITY_URL);
const postDataDebounced = debounce(async (pathname: string, title: string, bundle: string) => {
const data = await post<LastVisitedPage[], { pathname: string; title: string; bundle: string }>(LAST_VISITED_URL, {
pathname,
title,
bundle,
});
return data;
// count page as visited after 5 second of being on the page
// should help limit number of API calls
}, 5000);

const useLastPageVisitedUploader = (providerState: ReturnType<typeof chromeState>) => {
const scalprum = useScalprum<{ initialized: boolean; api: { chrome: ChromeAPI } }>();
const { pathname } = useLocation();
const postData = async (pathname: string, title: string, bundle: string) => {
try {
const data = await post<LastVisitedPage[], { pathname: string; title: string; bundle: string }>(LAST_VISITED_URL, {
pathname,
title,
bundle,
});
providerState.setLastVisited(data);
const data = await postDataDebounced(pathname, title, bundle);
if (data) {
providerState.setLastVisited(data);
}
} catch (error) {
console.error('Unable to update last visited pages!', error);
}
Expand Down Expand Up @@ -59,6 +68,7 @@ const useLastPageVisitedUploader = (providerState: ReturnType<typeof chromeState
}
return () => {
titleObserver?.disconnect();
postDataDebounced?.cancel();
};
}, [pathname]);
};
Expand Down
2 changes: 1 addition & 1 deletion packages/chrome/src/ChromeProvider/chromeState.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ describe('chromeState', () => {
expect(state.getState().lastVisitedPages).toEqual(lastVisitedPayload);
});

test('should correctly update favrite pages data via dedicated callback', () => {
test('should correctly update favorite pages data via dedicated callback', () => {
state.setFavoritePages([{ pathname: 'favorite', favorite: true }]);
expect(state.getState().favoritePages).toEqual([{ pathname: 'favorite', favorite: true }]);
});
Expand Down

0 comments on commit e10ccfd

Please sign in to comment.