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

Fix visited pages title values. #1921

Merged
merged 2 commits into from
Oct 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions config/setupTests.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,16 @@ import 'babel-polyfill';
import '@testing-library/jest-dom';
import { expect } from '@jest/globals';
import * as matchers from '@testing-library/jest-dom/dist/matchers';
import MutationObserver from 'mutation-observer';

// ensure the expect is picked up from jest not cypress
global.expect = expect;
// extends with RTL
global.expect.extend(matchers);
configure({ adapter: new Adapter() });
global.SVGPathElement = function () {};

global.MutationObserver = class {
constructor(callback) {}
disconnect() {}
observe(element, initObject) {}
};
// real MutationObserver polyfill for JSDOM
global.MutationObserver = MutationObserver;

global.window.insights = {
...(window.insights || {}),
Expand Down
16 changes: 16 additions & 0 deletions package-lock.json

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

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@
"jest-canvas-mock": "^2.4.0",
"jest-environment-jsdom": "^29.6.2",
"lerna": "^5.6.2",
"mutation-observer": "^1.0.3",
"node-sass-package-importer": "^5.3.2",
"prettier": "^2.7.1",
"redux-mock-store": "^1.5.4",
Expand Down
2 changes: 2 additions & 0 deletions packages/chrome/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,13 @@
},
"homepage": "https://github.com/RedHatInsights/frontend-components/tree/master/packages/chrome#readme",
"peerDependencies": {
"@scalprum/react-core": "^0.5.1",
"react": "^18.2.0",
"react-dom": "^18.2.0",
"react-router-dom": "^6.0.0"
},
"devDependencies": {
"@redhat-cloud-services/types": "^1.0.3",
"@types/react": "^18.0.0",
"glob": "10.3.3"
},
Expand Down
65 changes: 55 additions & 10 deletions packages/chrome/src/ChromeProvider/ChromeProvider.test.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import React from 'react';
import React, { useEffect } from 'react';
import { Link, MemoryRouter, Route, Routes } from 'react-router-dom';
import { render, screen } from '@testing-library/react';
import { act } from 'react-dom/test-utils';
import { ScalprumContext } from '@scalprum/react-core';
import { PluginStore } from '@openshift/dynamic-plugin-sdk';

import ChromeProvider from './ChromeProvider';
import * as fetch from '../utils/fetch';
Expand Down Expand Up @@ -31,17 +33,41 @@ describe('ChromeProvider', () => {
expect(getSpy).toHaveBeenCalledWith('/api/chrome-service/v1/user');
});

test('should post new data on pathname change', async () => {
test('should post new data on title change', async () => {
getSpy.mockResolvedValueOnce([]);
postSpy.mockResolvedValue(['/', '/bar']);
const DocumentMutator = () => {
useEffect(() => {
document.title = 'Foo title';
}, []);
return null;
};
// mock the initial document title
document.title = 'Initial title';
await act(async () => {
render(
<MemoryRouter initialEntries={['/']} initialIndex={0}>
<Routes>
<Route path="*" element={<Link to="/foo/bar">/foo/bar</Link>}></Route>
</Routes>
<ChromeProvider bundle="bundle-title" />
</MemoryRouter>
<ScalprumContext.Provider
value={{
florkbr marked this conversation as resolved.
Show resolved Hide resolved
config: {},
initialized: true,
pluginStore: new PluginStore(),
api: {
chrome: {
getBundleData: () => ({
bundleTitle: 'bundle-title',
}),
},
},
}}
>
<MemoryRouter initialEntries={['/']} initialIndex={0}>
<Routes>
<Route path="/foo/bar" element={<DocumentMutator />}></Route>
<Route path="*" element={<Link to="/foo/bar">/foo/bar</Link>}></Route>
</Routes>
<ChromeProvider />
</MemoryRouter>
</ScalprumContext.Provider>
);
});
// change location
Expand All @@ -54,7 +80,11 @@ describe('ChromeProvider', () => {
await flushPromises();
});
expect(postSpy).toHaveBeenCalledTimes(2);
expect(postSpy).toHaveBeenLastCalledWith('/api/chrome-service/v1/last-visited', { pathname: '/foo/bar', title: '', bundle: 'bundle-title' });
expect(postSpy).toHaveBeenLastCalledWith('/api/chrome-service/v1/last-visited', {
pathname: '/foo/bar',
title: 'Foo title',
bundle: 'bundle-title',
});
});

test('should not update state on mount if received error response', async () => {
Expand All @@ -66,7 +96,22 @@ describe('ChromeProvider', () => {
await act(async () => {
await render(
<MemoryRouter>
<ChromeProvider />
<ScalprumContext.Provider
value={{
config: {},
initialized: true,
pluginStore: new PluginStore(),
api: {
chrome: {
getBundleData: () => ({
bundleTitle: 'bundle-title',
}),
},
},
}}
>
<ChromeProvider />
</ScalprumContext.Provider>
</MemoryRouter>
);
});
Expand Down
63 changes: 53 additions & 10 deletions packages/chrome/src/ChromeProvider/ChromeProvider.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
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 { ChromeContext } from '../ChromeContext';
Expand All @@ -7,28 +9,69 @@ import { IDENTITY_URL, LAST_VISITED_URL, get, post } from '../utils/fetch';

const getUserIdentity = () => get<UserIdentity>(IDENTITY_URL);

const useLastPageVisitedUploader = (providerState: ReturnType<typeof chromeState>, bundle = '') => {
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);
} catch (error) {
console.error('Unable to update last visited pages!', error);
}
};
useEffect(() => {
post<LastVisitedPage[], { pathname: string; title: string; bundle: string }>(LAST_VISITED_URL, {
pathname,
title: document.title,
bundle,
})
.then((data) => providerState.setLastVisited(data))
.catch((error) => console.error('Unable to update last visited pages!', error));
let titleObserver: MutationObserver | undefined;
let prevTitle: string | null;
const titleTarget = document.querySelector('title');
if (titleTarget) {
prevTitle = titleTarget.textContent;
// initial api call on mount
postData(pathname, prevTitle ?? '', scalprum.api.chrome.getBundleData().bundleTitle);
/**
* Use Mutation observer to trigger the updates.
* Using the observer will ensure the last visited pages gets updated on document title change rather than just location change.
* The chrome service uses pathname as identifier and updates title according.
* Multiple calls with the same pathname and different title will ensure that the latest correct title is assigned to a pathname. *
* */
titleObserver = new MutationObserver((mutations) => {
// grab text from the title element
const currentTitle = mutations[0]?.target.textContent;
// trigger only if the titles are different
if (typeof currentTitle === 'string' && currentTitle !== prevTitle) {
try {
prevTitle = currentTitle;
postData(pathname, currentTitle, scalprum.api.chrome.getBundleData().bundleTitle);
} catch (error) {
// catch sync errors
console.error('Unable to update last visited pages!', error);
florkbr marked this conversation as resolved.
Show resolved Hide resolved
}
}
});
titleObserver.observe(titleTarget, {
// observe only the children
childList: true,
});
}
return () => {
titleObserver?.disconnect();
};
}, [pathname]);
};

const ChromeProvider: React.FC<React.PropsWithChildren<{ bundle?: string }>> = ({ children, bundle }) => {
const ChromeProvider: React.FC<React.PropsWithChildren> = ({ children }) => {
const isMounted = useRef(false);
const [initialRequest, setInitialRequest] = useState(false);
const providerState = useRef<ReturnType<typeof chromeState>>();
if (!providerState.current) {
providerState.current = chromeState();
}

useLastPageVisitedUploader(providerState.current, bundle);
useLastPageVisitedUploader(providerState.current);

useEffect(() => {
isMounted.current = true;
Expand Down