Skip to content

Commit 88c68a9

Browse files
authored
fix(ga): fix for #606 issue (#613)
* fix(ga): fix for #606 issue * [archive] * Create stale-bats-wink.md * fix(ga): change delay to 300ms in debounce * fix(ga): fix following comments * fix(ga): fix lint issue * [archive] * fix(ga): bump @types/lodash package * fix(ga): bump @types/redux-logger package * fix(ga): revert @types/redux-logger package --------- Co-authored-by: Ouin, Edouard <[email protected]>
1 parent 17bea3b commit 88c68a9

File tree

14 files changed

+335
-126
lines changed

14 files changed

+335
-126
lines changed

.changeset/stale-bats-wink.md

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
"@sap/guided-answers-extension-core": patch
3+
"sap-guided-answers-extension": patch
4+
"@sap/guided-answers-extension-webapp": patch
5+
---
6+
7+
fix(ga): fix for #606 issue
8+
- [x] add fix for pageSize issue
9+
- [x] add cancel previous call to API before doing a new one
10+
- [x] add debounce on search input onChange callback

packages/core/src/guided-answers-api.ts

+47-13
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type { AxiosResponse } from 'axios';
1+
import type { AxiosResponse, CancelTokenSource } from 'axios';
22
import axios from 'axios';
33
import { default as xss } from 'xss';
44
import type {
@@ -35,6 +35,7 @@ const FEEDBACK_COMMENT = `dtp/api/${VERSION}/feedback/comment`;
3535
const FEEDBACK_OUTCOME = `dtp/api/${VERSION}/feedback/outcome`;
3636
const DEFAULT_MAX_RESULTS = 9999;
3737

38+
const previousToken: CancelTokenSource[] = [];
3839
/**
3940
* Returns API to programmatically access Guided Answers.
4041
*
@@ -53,7 +54,7 @@ export function getGuidedAnswerApi(options?: APIOptions): GuidedAnswerAPI {
5354
enhanceNode({ node: await getNodeById(apiHost, id), extensions, logger, ide }),
5455
getTreeById: async (id: GuidedAnswerTreeId): Promise<GuidedAnswerTree> => getTreeById(apiHost, id),
5556
getTrees: async (queryOptions?: GuidedAnswersQueryOptions): Promise<GuidedAnswerTreeSearchResult> =>
56-
getTrees(apiHost, queryOptions),
57+
getTrees(apiHost, logger, queryOptions),
5758
getNodePath: async (nodeIdPath: GuidedAnswerNodeId[]): Promise<GuidedAnswerNode[]> => {
5859
let nodes = await getNodePath(apiHost, nodeIdPath);
5960
nodes = nodes.map((node) => enhanceNode({ node, extensions, logger, ide }));
@@ -156,13 +157,19 @@ async function getTreeById(host: string, id: GuidedAnswerTreeId): Promise<Guided
156157
}
157158

158159
/**
159-
* Returns an array of Guided Answers trees.
160+
* Fetches guided answer trees based on the provided query options.
160161
*
161-
* @param host - Guided Answer API host
162-
* @param queryOptions - options like query string, filters
163-
* @returns - Array of Guided Answer trees
162+
* @param {string} host - The host URL for the API.
163+
* @param {Logger} logger - The logger instance for logging debug information.
164+
* @param {GuidedAnswersQueryOptions} [queryOptions] - Optional query options including filters and paging.
165+
* @returns {Promise<GuidedAnswerTreeSearchResult>} A promise that resolves to the search result containing guided answer trees.
166+
* @throws {Error} Throws an error if the query is a number or if the response does not contain a 'trees' array.
164167
*/
165-
async function getTrees(host: string, queryOptions?: GuidedAnswersQueryOptions): Promise<GuidedAnswerTreeSearchResult> {
168+
async function getTrees(
169+
host: string,
170+
logger: Logger,
171+
queryOptions?: GuidedAnswersQueryOptions
172+
): Promise<GuidedAnswerTreeSearchResult> {
166173
if (typeof queryOptions?.query === 'number') {
167174
throw Error(
168175
`Invalid search for tree with number. Please use string or function getTreeById() to get a tree by id`
@@ -171,13 +178,40 @@ async function getTrees(host: string, queryOptions?: GuidedAnswersQueryOptions):
171178
const query = queryOptions?.query ? encodeURIComponent(`"${queryOptions.query}"`) : '*';
172179
const urlGetParamString = convertQueryOptionsToGetParams(queryOptions?.filters, queryOptions?.paging);
173180
const url = `${host}${TREE_PATH}${query}${urlGetParamString}`;
174-
const response: AxiosResponse<GuidedAnswerTreeSearchResult> = await axios.get<GuidedAnswerTreeSearchResult>(url);
175-
const searchResult = response.data;
176-
if (!Array.isArray(searchResult?.trees)) {
177-
throw Error(
178-
`Query result from call '${url}' does not contain property 'trees' as array. Received response: '${searchResult}'`
179-
);
181+
182+
// Cancel the previous request if it exists
183+
if (previousToken.length) {
184+
const prev = previousToken.pop();
185+
prev?.cancel('Canceling previous request');
186+
}
187+
188+
// Create a new CancelToken for the current request
189+
const source = axios.CancelToken.source();
190+
previousToken.push(source);
191+
192+
let searchResult: GuidedAnswerTreeSearchResult = {
193+
resultSize: -1,
194+
trees: [],
195+
productFilters: [],
196+
componentFilters: []
197+
};
198+
199+
try {
200+
const response = await axios.get<GuidedAnswerTreeSearchResult>(url, {
201+
cancelToken: source.token
202+
});
203+
searchResult = response.data;
204+
if (!Array.isArray(searchResult?.trees)) {
205+
throw Error(`Query result from call '${url}' does not contain property 'trees' as array`);
206+
}
207+
} catch (error) {
208+
if (axios.isCancel(error)) {
209+
logger.logDebug(`Request canceled: '${error.message}'`);
210+
} else {
211+
throw error;
212+
}
180213
}
214+
181215
return searchResult;
182216
}
183217

packages/core/test/guided-answers-api.test.ts

+28
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import axios from 'axios';
2+
import type { CancelTokenStatic } from 'axios';
23
import type {
34
APIOptions,
45
FeedbackCommentPayload,
@@ -12,6 +13,7 @@ import { getGuidedAnswerApi } from '../src';
1213

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

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

2628
describe('Guided Answers Api: getTrees()', () => {
29+
/**
30+
* Class representing a CancelToken.
31+
*/
32+
class CancelToken {
33+
/**
34+
* Creates a cancel token source.
35+
* @returns {Object} An object containing the cancel function and token.
36+
*/
37+
public static source() {
38+
const cancel: Canceler = jest.fn();
39+
const token = new CancelToken();
40+
return {
41+
cancel,
42+
token
43+
};
44+
}
45+
}
46+
47+
mockedAxios.CancelToken = {
48+
source: jest.fn(() => ({
49+
cancel: jest.fn(),
50+
token: new CancelToken()
51+
}))
52+
} as any;
53+
2754
beforeEach(() => {
2855
jest.clearAllMocks();
2956
});
@@ -67,6 +94,7 @@ describe('Guided Answers Api: getTrees()', () => {
6794
componentFilters: [{ COMPONENT: 'C1', COUNT: 1 }],
6895
productFilters: [{ PRODUCT: 'P_one', COUNT: 1 }]
6996
};
97+
7098
let requestUrl = '';
7199
mockedAxios.get.mockImplementation((url) => {
72100
requestUrl = url;

packages/ide-extension/src/panel/guidedAnswersPanel.ts

+4-2
Original file line numberDiff line numberDiff line change
@@ -243,14 +243,16 @@ export class GuidedAnswersPanel {
243243
}
244244
this.loadingTimeout = setTimeout(() => {
245245
this.postActionToWebview(updateNetworkStatus('LOADING'));
246-
}, 2000);
246+
}, 50);
247247
}
248248
try {
249249
const trees = await this.guidedAnswerApi.getTrees(queryOptions);
250250
if (this.loadingTimeout) {
251251
clearTimeout(this.loadingTimeout);
252252
}
253-
this.postActionToWebview(updateNetworkStatus('OK'));
253+
if (trees.resultSize !== -1) {
254+
this.postActionToWebview(updateNetworkStatus('OK'));
255+
}
254256
return trees;
255257
} catch (e) {
256258
if (this.loadingTimeout) {

packages/webapp/package.json

+4
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,12 @@
2222
"@testing-library/dom": "8.20.1",
2323
"@testing-library/jest-dom": "6.2.0",
2424
"@testing-library/react": "12.1.5",
25+
"@types/lodash": "4.17.7",
2526
"@types/react": "17.0.62",
2627
"@types/react-copy-to-clipboard": "5.0.7",
2728
"@types/react-dom": "17.0.20",
2829
"@types/react-redux": "7.1.33",
30+
"@types/redux-logger": "3.0.9",
2931
"@types/redux-mock-store": "1.0.6",
3032
"autoprefixer": "10.4.16",
3133
"esbuild": "0.19.11",
@@ -37,13 +39,15 @@
3739
"i18next": "23.7.16",
3840
"jest-css-modules-transform": "4.4.2",
3941
"jest-environment-jsdom": "29.7.0",
42+
"lodash": "4.17.21",
4043
"path": "0.12.7",
4144
"postcss": "8.4.33",
4245
"react": "16.14.0",
4346
"react-dom": "16.14.0",
4447
"react-i18next": "13.2.2",
4548
"react-redux": "8.1.2",
4649
"redux": "4.2.1",
50+
"redux-logger": "3.0.6",
4751
"redux-mock-store": "1.5.4",
4852
"uuid": "9.0.1"
4953
},

packages/webapp/src/webview/state/middleware.ts

+6-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import i18next from 'i18next';
22
import type { Middleware, MiddlewareAPI, Dispatch, Action } from 'redux';
3+
import { createLogger } from 'redux-logger';
34
import type { GuidedAnswerActions } from '@sap/guided-answers-extension-types';
45
import {
56
GO_TO_PREVIOUS_PAGE,
@@ -40,7 +41,6 @@ export const communicationMiddleware: Middleware<
4041
// Add event handler, this will dispatch incoming state updates
4142
window.addEventListener('message', (event: MessageEvent) => {
4243
if (event.origin === window.origin) {
43-
console.log(i18next.t('MESSAGE_RECEIVED'), event);
4444
if (event.data && typeof event.data.type === 'string') {
4545
store.dispatch(event.data);
4646
}
@@ -58,7 +58,6 @@ export const communicationMiddleware: Middleware<
5858
action = next(action);
5959
if (action && typeof action.type === 'string') {
6060
window.vscode.postMessage(action);
61-
console.log(i18next.t('REACT_ACTION_POSTED'), action);
6261
}
6362
return action;
6463
};
@@ -81,6 +80,7 @@ const allowedTelemetryActions = new Set([
8180
UPDATE_BOOKMARKS,
8281
SYNCHRONIZE_BOOKMARK
8382
]);
83+
8484
export const telemetryMiddleware: Middleware<
8585
Dispatch<GuidedAnswerActions>,
8686
AppState,
@@ -115,3 +115,7 @@ export const restoreMiddleware: Middleware<Dispatch<GuidedAnswerActions>, AppSta
115115
return action;
116116
};
117117
};
118+
119+
export const loggerMiddleware = createLogger({
120+
duration: true
121+
});

packages/webapp/src/webview/state/store.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
import { configureStore } from '@reduxjs/toolkit';
22
import { bindActionCreators } from 'redux';
3-
import { telemetryMiddleware, communicationMiddleware, restoreMiddleware } from './middleware';
3+
import { telemetryMiddleware, communicationMiddleware, restoreMiddleware, loggerMiddleware } from './middleware';
44
import { getInitialState, reducer } from './reducers';
55
import * as AllActions from './actions';
66

77
export const store = configureStore({
88
reducer,
99
preloadedState: getInitialState(),
1010
devTools: false,
11-
middleware: [communicationMiddleware, telemetryMiddleware, restoreMiddleware]
11+
middleware: [communicationMiddleware, telemetryMiddleware, restoreMiddleware, loggerMiddleware]
1212
});
1313

1414
// bind actions to store

packages/webapp/src/webview/ui/components/App/App.tsx

+6-19
Original file line numberDiff line numberDiff line change
@@ -26,28 +26,15 @@ initIcons();
2626
export function App(): ReactElement {
2727
const appState = useSelector<AppState, AppState>((state) => state);
2828
useEffect(() => {
29-
const resultsContainer = document.getElementById('results-container');
30-
if (!resultsContainer) {
31-
return undefined;
32-
}
33-
//tree-item element height is ~100px, using 50px here to be on the safe side.
29+
//tree-item element height is ~100px, using 50px here to be on the safe side. Header uses ~300, minimum page size is 5.
3430
const setPageSizeByContainerHeight = (pxHeight: number): void => {
35-
actions.setPageSize(Math.ceil(pxHeight / 50));
31+
actions.setPageSize(Math.max(5, Math.ceil((pxHeight - 300) / 50)));
3632
};
37-
const resizeObserver = new ResizeObserver((entries: ResizeObserverEntry[]) => {
38-
const containerEntry = entries.find((entry) => entry?.target?.id === 'results-container');
39-
if (containerEntry) {
40-
setPageSizeByContainerHeight(containerEntry.contentRect.height);
41-
}
42-
});
43-
// Set initial page size
44-
setPageSizeByContainerHeight(resultsContainer.clientHeight);
45-
resizeObserver.observe(resultsContainer);
46-
return () => {
47-
if (resizeObserver) {
48-
resizeObserver.unobserve(resultsContainer);
49-
}
33+
window.onresize = () => {
34+
setPageSizeByContainerHeight(window.innerHeight);
5035
};
36+
setPageSizeByContainerHeight(window.innerHeight);
37+
return undefined;
5138
}, []);
5239

5340
function fetchData() {

0 commit comments

Comments
 (0)