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(ga): fix for #606 issue #613

Merged
merged 11 commits into from
Sep 11, 2024
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
10 changes: 10 additions & 0 deletions .changeset/stale-bats-wink.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
"@sap/guided-answers-extension-core": patch
"sap-guided-answers-extension": patch
"@sap/guided-answers-extension-webapp": patch
---

fix(ga): fix for #606 issue
- [x] add fix for pageSize issue
- [x] add cancel previous call to API before doing a new one
- [x] add debounce on search input onChange callback
60 changes: 47 additions & 13 deletions packages/core/src/guided-answers-api.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { AxiosResponse } from 'axios';
import type { AxiosResponse, CancelTokenSource } from 'axios';
import axios from 'axios';
import { default as xss } from 'xss';
import type {
Expand Down Expand Up @@ -35,6 +35,7 @@ const FEEDBACK_COMMENT = `dtp/api/${VERSION}/feedback/comment`;
const FEEDBACK_OUTCOME = `dtp/api/${VERSION}/feedback/outcome`;
const DEFAULT_MAX_RESULTS = 9999;

const previousToken: CancelTokenSource[] = [];
/**
* Returns API to programmatically access Guided Answers.
*
Expand All @@ -53,7 +54,7 @@ export function getGuidedAnswerApi(options?: APIOptions): GuidedAnswerAPI {
enhanceNode({ node: await getNodeById(apiHost, id), extensions, logger, ide }),
getTreeById: async (id: GuidedAnswerTreeId): Promise<GuidedAnswerTree> => getTreeById(apiHost, id),
getTrees: async (queryOptions?: GuidedAnswersQueryOptions): Promise<GuidedAnswerTreeSearchResult> =>
getTrees(apiHost, queryOptions),
getTrees(apiHost, logger, queryOptions),
getNodePath: async (nodeIdPath: GuidedAnswerNodeId[]): Promise<GuidedAnswerNode[]> => {
let nodes = await getNodePath(apiHost, nodeIdPath);
nodes = nodes.map((node) => enhanceNode({ node, extensions, logger, ide }));
Expand Down Expand Up @@ -156,13 +157,19 @@ async function getTreeById(host: string, id: GuidedAnswerTreeId): Promise<Guided
}

/**
* Returns an array of Guided Answers trees.
* Fetches guided answer trees based on the provided query options.
*
* @param host - Guided Answer API host
* @param queryOptions - options like query string, filters
* @returns - Array of Guided Answer trees
* @param {string} host - The host URL for the API.
* @param {Logger} logger - The logger instance for logging debug information.
* @param {GuidedAnswersQueryOptions} [queryOptions] - Optional query options including filters and paging.
* @returns {Promise<GuidedAnswerTreeSearchResult>} A promise that resolves to the search result containing guided answer trees.
* @throws {Error} Throws an error if the query is a number or if the response does not contain a 'trees' array.
*/
async function getTrees(host: string, queryOptions?: GuidedAnswersQueryOptions): Promise<GuidedAnswerTreeSearchResult> {
async function getTrees(
host: string,
logger: Logger,
queryOptions?: GuidedAnswersQueryOptions
): Promise<GuidedAnswerTreeSearchResult> {
if (typeof queryOptions?.query === 'number') {
throw Error(
`Invalid search for tree with number. Please use string or function getTreeById() to get a tree by id`
Expand All @@ -171,13 +178,40 @@ async function getTrees(host: string, queryOptions?: GuidedAnswersQueryOptions):
const query = queryOptions?.query ? encodeURIComponent(`"${queryOptions.query}"`) : '*';
const urlGetParamString = convertQueryOptionsToGetParams(queryOptions?.filters, queryOptions?.paging);
const url = `${host}${TREE_PATH}${query}${urlGetParamString}`;
const response: AxiosResponse<GuidedAnswerTreeSearchResult> = await axios.get<GuidedAnswerTreeSearchResult>(url);
const searchResult = response.data;
if (!Array.isArray(searchResult?.trees)) {
throw Error(
`Query result from call '${url}' does not contain property 'trees' as array. Received response: '${searchResult}'`
);

// Cancel the previous request if it exists
if (previousToken.length) {
const prev = previousToken.pop();
prev?.cancel('Canceling previous request');
}

// Create a new CancelToken for the current request
const source = axios.CancelToken.source();
previousToken.push(source);

let searchResult: GuidedAnswerTreeSearchResult = {
resultSize: -1,
trees: [],
productFilters: [],
componentFilters: []
};

try {
const response = await axios.get<GuidedAnswerTreeSearchResult>(url, {
cancelToken: source.token
});
searchResult = response.data;
if (!Array.isArray(searchResult?.trees)) {
throw Error(`Query result from call '${url}' does not contain property 'trees' as array`);
}
} catch (error) {
if (axios.isCancel(error)) {
logger.logDebug(`Request canceled: '${error.message}'`);
} else {
throw error;
}
}

return searchResult;
}

Expand Down
28 changes: 28 additions & 0 deletions packages/core/test/guided-answers-api.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import axios from 'axios';
import type { CancelTokenStatic } from 'axios';
import type {
APIOptions,
FeedbackCommentPayload,
Expand All @@ -12,6 +13,7 @@ import { getGuidedAnswerApi } from '../src';

jest.mock('axios');
const mockedAxios = axios as jest.Mocked<typeof axios>;
type Canceler = (message?: string) => void;
const currentVersion = getGuidedAnswerApi().getApiInfo().version;

describe('Guided Answers Api: getApiInfo()', () => {
Expand All @@ -24,6 +26,31 @@ describe('Guided Answers Api: getApiInfo()', () => {
});

describe('Guided Answers Api: getTrees()', () => {
/**
* Class representing a CancelToken.
*/
class CancelToken {
/**
* Creates a cancel token source.
* @returns {Object} An object containing the cancel function and token.
*/
public static source() {
const cancel: Canceler = jest.fn();
const token = new CancelToken();
return {
cancel,
token
};
}
}

mockedAxios.CancelToken = {
source: jest.fn(() => ({
cancel: jest.fn(),
token: new CancelToken()
}))
} as any;

beforeEach(() => {
jest.clearAllMocks();
});
Expand Down Expand Up @@ -67,6 +94,7 @@ describe('Guided Answers Api: getTrees()', () => {
componentFilters: [{ COMPONENT: 'C1', COUNT: 1 }],
productFilters: [{ PRODUCT: 'P_one', COUNT: 1 }]
};

let requestUrl = '';
mockedAxios.get.mockImplementation((url) => {
requestUrl = url;
Expand Down
6 changes: 4 additions & 2 deletions packages/ide-extension/src/panel/guidedAnswersPanel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -243,14 +243,16 @@ export class GuidedAnswersPanel {
}
this.loadingTimeout = setTimeout(() => {
this.postActionToWebview(updateNetworkStatus('LOADING'));
}, 2000);
}, 50);
}
try {
const trees = await this.guidedAnswerApi.getTrees(queryOptions);
if (this.loadingTimeout) {
clearTimeout(this.loadingTimeout);
}
this.postActionToWebview(updateNetworkStatus('OK'));
if (trees.resultSize !== -1) {
this.postActionToWebview(updateNetworkStatus('OK'));
}
return trees;
} catch (e) {
if (this.loadingTimeout) {
Expand Down
4 changes: 4 additions & 0 deletions packages/webapp/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,12 @@
"@testing-library/dom": "8.20.1",
"@testing-library/jest-dom": "6.2.0",
"@testing-library/react": "12.1.5",
"@types/lodash": "4.17.7",
"@types/react": "17.0.62",
"@types/react-copy-to-clipboard": "5.0.7",
"@types/react-dom": "17.0.20",
"@types/react-redux": "7.1.33",
"@types/redux-logger": "3.0.9",
eouin marked this conversation as resolved.
Show resolved Hide resolved
"@types/redux-mock-store": "1.0.6",
"autoprefixer": "10.4.16",
"esbuild": "0.19.11",
Expand All @@ -37,13 +39,15 @@
"i18next": "23.7.16",
"jest-css-modules-transform": "4.4.2",
"jest-environment-jsdom": "29.7.0",
"lodash": "4.17.21",
"path": "0.12.7",
"postcss": "8.4.33",
"react": "16.14.0",
"react-dom": "16.14.0",
"react-i18next": "13.2.2",
"react-redux": "8.1.2",
"redux": "4.2.1",
"redux-logger": "3.0.6",
"redux-mock-store": "1.5.4",
"uuid": "9.0.1"
},
Expand Down
8 changes: 6 additions & 2 deletions packages/webapp/src/webview/state/middleware.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import i18next from 'i18next';
import type { Middleware, MiddlewareAPI, Dispatch, Action } from 'redux';
import { createLogger } from 'redux-logger';
import type { GuidedAnswerActions } from '@sap/guided-answers-extension-types';
import {
GO_TO_PREVIOUS_PAGE,
Expand Down Expand Up @@ -40,7 +41,6 @@ export const communicationMiddleware: Middleware<
// Add event handler, this will dispatch incoming state updates
window.addEventListener('message', (event: MessageEvent) => {
if (event.origin === window.origin) {
console.log(i18next.t('MESSAGE_RECEIVED'), event);
if (event.data && typeof event.data.type === 'string') {
store.dispatch(event.data);
}
Expand All @@ -58,7 +58,6 @@ export const communicationMiddleware: Middleware<
action = next(action);
if (action && typeof action.type === 'string') {
window.vscode.postMessage(action);
console.log(i18next.t('REACT_ACTION_POSTED'), action);
}
return action;
};
Expand All @@ -81,6 +80,7 @@ const allowedTelemetryActions = new Set([
UPDATE_BOOKMARKS,
SYNCHRONIZE_BOOKMARK
]);

export const telemetryMiddleware: Middleware<
Dispatch<GuidedAnswerActions>,
AppState,
Expand Down Expand Up @@ -115,3 +115,7 @@ export const restoreMiddleware: Middleware<Dispatch<GuidedAnswerActions>, AppSta
return action;
};
};

export const loggerMiddleware = createLogger({
duration: true
});
4 changes: 2 additions & 2 deletions packages/webapp/src/webview/state/store.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import { configureStore } from '@reduxjs/toolkit';
import { bindActionCreators } from 'redux';
import { telemetryMiddleware, communicationMiddleware, restoreMiddleware } from './middleware';
import { telemetryMiddleware, communicationMiddleware, restoreMiddleware, loggerMiddleware } from './middleware';
import { getInitialState, reducer } from './reducers';
import * as AllActions from './actions';

export const store = configureStore({
reducer,
preloadedState: getInitialState(),
devTools: false,
middleware: [communicationMiddleware, telemetryMiddleware, restoreMiddleware]
middleware: [communicationMiddleware, telemetryMiddleware, restoreMiddleware, loggerMiddleware]
});

// bind actions to store
Expand Down
25 changes: 6 additions & 19 deletions packages/webapp/src/webview/ui/components/App/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,28 +26,15 @@ initIcons();
export function App(): ReactElement {
const appState = useSelector<AppState, AppState>((state) => state);
useEffect(() => {
const resultsContainer = document.getElementById('results-container');
if (!resultsContainer) {
return undefined;
}
//tree-item element height is ~100px, using 50px here to be on the safe side.
//tree-item element height is ~100px, using 50px here to be on the safe side. Header uses ~300, minimum page size is 5.
const setPageSizeByContainerHeight = (pxHeight: number): void => {
actions.setPageSize(Math.ceil(pxHeight / 50));
actions.setPageSize(Math.max(5, Math.ceil((pxHeight - 300) / 50)));
};
const resizeObserver = new ResizeObserver((entries: ResizeObserverEntry[]) => {
const containerEntry = entries.find((entry) => entry?.target?.id === 'results-container');
if (containerEntry) {
setPageSizeByContainerHeight(containerEntry.contentRect.height);
}
});
// Set initial page size
setPageSizeByContainerHeight(resultsContainer.clientHeight);
resizeObserver.observe(resultsContainer);
return () => {
if (resizeObserver) {
resizeObserver.unobserve(resultsContainer);
}
window.onresize = () => {
setPageSizeByContainerHeight(window.innerHeight);
};
setPageSizeByContainerHeight(window.innerHeight);
return undefined;
}, []);

function fetchData() {
Expand Down
Loading
Loading