Improve Subscribe APIs dialog loading performance - #1410
Improve Subscribe APIs dialog loading performance#1410nisan-abeywickrama wants to merge 5 commits into
Conversation
📝 WalkthroughWalkthroughAPICardView now resolves subscription status per listed entity through Subscription API calls. Subscribe dialogs use independent refresh keys to reload API and MCP listings without loading complete subscription collections. ChangesSubscription refresh flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Subscriptions
participant APIList
participant APICardView
participant Subscription
Subscriptions->>Subscriptions: increment API or MCP refreshKey
Subscriptions->>APIList: pass refreshKey
APIList->>APICardView: reload listing
APICardView->>Subscription: query subscription status per entity
Subscription-->>APICardView: return subscription results
APICardView->>APICardView: filter displayed entities
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@portals/devportal/src/main/webapp/source/src/app/components/Apis/Listing/APICardView.jsx`:
- Around line 143-151: Update the subscription lookup flow in
updateUnsubscribedAPIsList so getSubscriptions failures are not swallowed by the
empty catch. Propagate the rejection to the existing page-load error handler, or
preserve the entity as an unknown status and disable subscription actions for
that APICardView instead of treating it as unsubscribed.
- Around line 105-107: Update the asynchronous load flows in getData and
changePage to share one monotonically increasing load ID for every request.
Capture each request’s ID and, before updating data, showing errors, or clearing
loading, confirm it is still the current ID so stale refresh, search, or page
results cannot overwrite the active page.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f71803b6-5d21-4183-ad0b-dc414f36dbb6
📒 Files selected for processing (2)
portals/devportal/src/main/webapp/source/src/app/components/Apis/Listing/APICardView.jsxportals/devportal/src/main/webapp/source/src/app/components/Applications/Details/Subscriptions.jsx
|
@coderabbitai full review |
✅ Action performedFull review finished. |
|
@CodeRabbit full review |
|
eacf8d9 to
0c7514f
Compare
|
@CodeRabbit full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@portals/devportal/src/main/webapp/source/src/app/components/Apis/Listing/APICardView.jsx`:
- Around line 101-102: Update the reload flow in the method containing
apiLoadRequestId so it sets loading to true immediately after allocating
requestId and before starting xhrRequest(). Preserve the existing finally
behavior that clears loading only for the current request.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4a2080ac-351f-4c3a-a6a6-b5dea600f1ba
📒 Files selected for processing (3)
portals/devportal/src/main/webapp/source/src/app/components/Apis/Listing/APICardView.jsxportals/devportal/src/main/webapp/source/src/app/components/Applications/Details/Subscriptions.jsxportals/devportal/src/main/webapp/source/src/app/data/Subscription.jsx
| const requestId = ++this.apiLoadRequestId; | ||
| this.xhrRequest() |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Set loading before starting every reload.
When refreshKey changes after a successful subscription, this method keeps the previous data visible until all status lookups finish. The old SubscriptionPolicySelect remains actionable during that interval. A user can submit a second subscription request for the same API.
Set loading: true immediately after allocating requestId. The existing finally block will clear it only for the current request.
Proposed fix
const requestId = ++this.apiLoadRequestId;
+this.setState({ loading: true });
this.xhrRequest()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const requestId = ++this.apiLoadRequestId; | |
| this.xhrRequest() | |
| const requestId = ++this.apiLoadRequestId; | |
| this.setState({ loading: true }); | |
| this.xhrRequest() |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@portals/devportal/src/main/webapp/source/src/app/components/Apis/Listing/APICardView.jsx`
around lines 101 - 102, Update the reload flow in the method containing
apiLoadRequestId so it sets loading to true immediately after allocating
requestId and before starting xhrRequest(). Preserve the existing finally
behavior that clears loading only for the current request.
|
1 participant
Description
The "Subscribe APIs" dialog lists APIs 10 per page, but it loaded the application's entire subscription list up front just to mark which ones were already subscribed. For an application with 270 subscriptions the dialog blocked on a spinner for over 3 minutes.
Approach
Subscriptions.jsxno longer callsloadAllSubscriptionsfor the dialog.APICardViewnow resolves subscription status only for the APIs on the current page, calling the subscriptions API with bothapiIdandapplicationIdso each lookup returns at most one record. ArefreshKeyprop replaces the subscription list as the reload trigger, used on dialog open and after a successful subscribe.Related Issue
wso2/api-manager#5148