Skip to content

Commit f4894e5

Browse files
committed
Address CodeRabbit review: avoid unnecessary full-list walks, evict rejected cache entries, preserve retry state on load failure, drop redundant per-row subscription fetch
1 parent 0335f08 commit f4894e5

2 files changed

Lines changed: 52 additions & 30 deletions

File tree

‎portals/devportal/src/main/webapp/source/src/app/components/Applications/Details/SubscriptionTableData.jsx‎

Lines changed: 13 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ import { FormattedMessage } from 'react-intl';
3737
import { ScopeValidation, resourceMethods, resourcePaths } from 'AppComponents/Shared/ScopeValidation';
3838
import PropTypes from 'prop-types';
3939
import CONSTANTS from 'AppData/Constants';
40-
import Subscription from 'AppData/Subscription';
4140
import { getBasePath } from 'AppUtils/utils';
4241
import { mdiOpenInNew } from '@mdi/js';
4342
import { Icon as MDIcon } from '@mdi/react';
@@ -103,7 +102,7 @@ class SubscriptionTableData extends React.Component {
103102
this.mounted = true;
104103
this.checkIfWebhookAPI();
105104
this.populateAPIData(subscription.apiId);
106-
this.checkIfDynamicUsagePolicy(subscription.subscriptionId);
105+
this.checkIfDynamicUsagePolicy(subscription.throttlingPolicy);
107106
}
108107

109108
componentWillUnmount() {
@@ -208,26 +207,19 @@ class SubscriptionTableData extends React.Component {
208207

209208
/**
210209
* Check if the policy is dynamic usage type
211-
* @param {string} subscriptionUUID subscription UUID
210+
* @param {string} throttlingPolicy throttling policy name, already available on the subscription
212211
*/
213-
checkIfDynamicUsagePolicy(subscriptionUUID) {
214-
const client = new Subscription();
215-
const promisedSubscription = client.getSubscription(subscriptionUUID);
216-
promisedSubscription.then((response) => {
217-
if (this.mounted && response && response.body) {
218-
const subscriptionData = JSON.parse(response.data);
219-
if (subscriptionData.throttlingPolicy) {
220-
const { getSubscriptionPolicyByName } = this.props;
221-
const promisedPolicy = getSubscriptionPolicyByName(subscriptionData.throttlingPolicy);
222-
promisedPolicy.then((policyData) => {
223-
if (this.mounted
224-
&& policyData
225-
&& policyData.monetizationAttributes
226-
&& policyData.monetizationAttributes.billingType === 'DYNAMICRATE') {
227-
this.setState({ isDynamicUsagePolicy: true });
228-
}
229-
});
230-
}
212+
checkIfDynamicUsagePolicy(throttlingPolicy) {
213+
if (!throttlingPolicy) {
214+
return;
215+
}
216+
const { getSubscriptionPolicyByName } = this.props;
217+
getSubscriptionPolicyByName(throttlingPolicy).then((policyData) => {
218+
if (this.mounted
219+
&& policyData
220+
&& policyData.monetizationAttributes
221+
&& policyData.monetizationAttributes.billingType === 'DYNAMICRATE') {
222+
this.setState({ isDynamicUsagePolicy: true });
231223
}
232224
});
233225
}

‎portals/devportal/src/main/webapp/source/src/app/components/Applications/Details/Subscriptions.jsx‎

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -314,8 +314,11 @@ class SubscriptionsBase extends React.Component {
314314
*/
315315
componentDidMount() {
316316
this.mounted = true;
317-
this.ensureLoaded(SUBSCRIPTIONS_PER_PAGE, SUBSCRIPTIONS_PER_PAGE)
318-
.then(() => this.refreshDerivedState());
317+
const { apisAccessible, mcpServersAccessible } = this.props;
318+
this.ensureLoaded(
319+
apisAccessible ? SUBSCRIPTIONS_PER_PAGE : 0,
320+
mcpServersAccessible ? SUBSCRIPTIONS_PER_PAGE : 0,
321+
).then(() => this.refreshDerivedState());
319322
}
320323

321324
componentWillUnmount() {
@@ -364,15 +367,23 @@ class SubscriptionsBase extends React.Component {
364367
getAPIById(apiUUID) {
365368
if (!this.apiDetailsById[apiUUID]) {
366369
const apiClient = new Api();
367-
this.apiDetailsById[apiUUID] = apiClient.getAPIById(apiUUID).then(parseResponseData);
370+
this.apiDetailsById[apiUUID] = apiClient.getAPIById(apiUUID).then(parseResponseData)
371+
.catch((error) => {
372+
delete this.apiDetailsById[apiUUID];
373+
throw error;
374+
});
368375
}
369376
return this.apiDetailsById[apiUUID];
370377
}
371378

372379
getMCPServerById(apiUUID) {
373380
if (!this.mcpDetailsById[apiUUID]) {
374381
const mcpClient = new MCPServer();
375-
this.mcpDetailsById[apiUUID] = mcpClient.getMCPServerById(apiUUID).then(parseResponseData);
382+
this.mcpDetailsById[apiUUID] = mcpClient.getMCPServerById(apiUUID).then(parseResponseData)
383+
.catch((error) => {
384+
delete this.mcpDetailsById[apiUUID];
385+
throw error;
386+
});
376387
}
377388
return this.mcpDetailsById[apiUUID];
378389
}
@@ -382,7 +393,11 @@ class SubscriptionsBase extends React.Component {
382393
const apiClient = new Api();
383394
this.subscriptionPoliciesByName[policyName] = apiClient
384395
.getTierByName(policyName, 'subscription')
385-
.then(parseResponseData);
396+
.then(parseResponseData)
397+
.catch((error) => {
398+
delete this.subscriptionPoliciesByName[policyName];
399+
throw error;
400+
});
386401
}
387402
return this.subscriptionPoliciesByName[policyName];
388403
}
@@ -445,13 +460,27 @@ class SubscriptionsBase extends React.Component {
445460
if (!this.mounted || requestId !== this.subscriptionsRequestId) {
446461
return;
447462
}
448-
this.fullyLoaded = true;
449463
const { status } = error;
450464
if (status === 404) {
465+
// The application itself wasn't found — a genuinely terminal state.
466+
this.fullyLoaded = true;
451467
this.setState({ subscriptionsNotFound: true, subscriptionsLoaded: true });
452-
} else if (status === 401) {
468+
return;
469+
}
470+
if (status === 401) {
471+
this.fullyLoaded = true;
453472
this.setState({ isAuthorize: false });
473+
return;
454474
}
475+
// Transient failure (5xx/network) — do not mark the walk complete, so a later
476+
// ensureLoaded call (e.g. the user retrying pagination) resumes from this same
477+
// backendOffset instead of being permanently short-circuited.
478+
this.loadChain = null;
479+
this.setState({ subscriptionsLoaded: true });
480+
Alert.error(this.props.intl.formatMessage({
481+
id: 'Applications.Details.Subscriptions.error.loading.subscriptions',
482+
defaultMessage: 'Error occurred while loading subscriptions',
483+
}));
455484
});
456485
};
457486

@@ -557,8 +586,9 @@ class SubscriptionsBase extends React.Component {
557586
this.subscriptionsRequestId += 1;
558587
this.resetAccumulation();
559588
const { apiPage, mcpPage } = this.state;
560-
const requiredApi = (apiPage + 1) * SUBSCRIPTIONS_PER_PAGE;
561-
const requiredMcp = (mcpPage + 1) * SUBSCRIPTIONS_PER_PAGE;
589+
const { apisAccessible, mcpServersAccessible } = this.props;
590+
const requiredApi = apisAccessible ? (apiPage + 1) * SUBSCRIPTIONS_PER_PAGE : 0;
591+
const requiredMcp = mcpServersAccessible ? (mcpPage + 1) * SUBSCRIPTIONS_PER_PAGE : 0;
562592
this.ensureLoaded(requiredApi, requiredMcp).then(() => this.refreshDerivedState());
563593
}
564594

0 commit comments

Comments
 (0)