Advance analytics#10434
Open
bhagyasakalanka wants to merge 12 commits into
Open
Conversation
Contributor
|
Warning Review limit reached
More reviews will be available in 37 minutes and 28 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds an "Enable Advanced Analytics" upgrade flow to the Insights feature. Introduces a ChangesAdvanced Analytics Upgrade
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
features/admin.org-insights.v1/pages/org-insights.tsx (1)
52-72: ⚡ Quick winExtract server error description in error alert.
The error alert uses a hardcoded fallback message. As per coding guidelines, error alerts should extract the server's error description when available:
dispatch(addAlert({ description: error?.response?.data?.description || fallback, level: AlertLevels.ERROR, message })).♻️ Proposed refactor to extract server error
- } catch (_error: unknown) { + } catch (error: unknown) { dispatch(addAlert({ - description: "Failed to enable advanced analytics. Please try again.", + description: (error as any)?.response?.data?.description || + "Failed to enable advanced analytics. Please try again.", level: AlertLevels.ERROR, message: "Operation Failed" }));🤖 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 `@features/admin.org-insights.v1/pages/org-insights.tsx` around lines 52 - 72, The catch block in the handleEnableAdvancedAnalytics function uses a hardcoded error message instead of extracting the actual server error description. Modify the catch block to capture the error (change the parameter from _error to error), extract the server error description using the pattern error?.response?.data?.description, and provide a fallback message if the server error is unavailable. Update the dispatch call for addAlert to use this extracted error description in the description field so users see the actual server error when available.Source: Coding guidelines
🤖 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 @.changeset/huge-berries-thank.md:
- Line 7: The changeset description contains a grammatical error where "advance
analytics" should be "advanced analytics". Update the text in the changeset file
to use the correct spelling "advanced" instead of "advance" to maintain proper
English grammar.
In `@features/admin.org-insights.v1/api/enable-advanced-analytics.ts`:
- Around line 37-64: In the enableAdvancedAnalytics function, validate that
dashboardInfoEndpoint contains the '/dashboard-info' path segment before
attempting the string replacement to derive enableAnalyticsEndpoint. If the
expected path segment is not present, throw an IdentityApicsApiException with a
descriptive error message rather than allowing the replacement to silently
return an unmodified endpoint, which would result in an incorrect endpoint being
used.
In
`@features/admin.org-insights.v1/components/advanced-analytics-upgrade-card.tsx`:
- Around line 119-124: The Chip component contains hardcoded styling values in
the sx prop that violate coding guidelines. Replace the hardcoded fontSize value
of "0.65rem" and height value of 18 with appropriate theme spacing and
typography values from the Material-UI theme object. Access these values through
the theme parameter in the sx prop callback function or use theme.spacing() and
theme.typography utilities to ensure styling remains consistent with the design
system.
- Around line 36-37: Move the hardcoded URL constants ASGARDEO_TOS_URL and
MOESIF_TOS_URL from the advanced-analytics-upgrade-card.tsx component to the
OrgInsightsConstants file, following the established pattern used in
SIWEConstants. Remove the constant declarations from the component level and
import them from OrgInsightsConstants instead. Additionally, verify the
accessibility of the Asgardeo ToS URL
(https://wso2.com/asgardeo/terms-of-service/) which is currently returning HTTP
403 Forbidden status and determine whether this is due to geolocation
restrictions or an actual accessibility issue that needs to be resolved.
---
Nitpick comments:
In `@features/admin.org-insights.v1/pages/org-insights.tsx`:
- Around line 52-72: The catch block in the handleEnableAdvancedAnalytics
function uses a hardcoded error message instead of extracting the actual server
error description. Modify the catch block to capture the error (change the
parameter from _error to error), extract the server error description using the
pattern error?.response?.data?.description, and provide a fallback message if
the server error is unavailable. Update the dispatch call for addAlert to use
this extracted error description in the description field so users see the
actual server error when available.
🪄 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.yml
Review profile: CHILL
Plan: Pro
Run ID: 7f7a7ad4-b516-4e02-9532-22038042fdc2
📒 Files selected for processing (7)
.changeset/huge-berries-thank.mdapps/console/java/org.wso2.identity.apps.console.server.feature/resources/deployment.config.json.j2apps/console/src/public/deployment.config.jsonfeatures/admin.analytics.v1/pages/insights-page.tsxfeatures/admin.org-insights.v1/api/enable-advanced-analytics.tsfeatures/admin.org-insights.v1/components/advanced-analytics-upgrade-card.tsxfeatures/admin.org-insights.v1/pages/org-insights.tsx
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #10434 +/- ##
==========================================
+ Coverage 72.70% 72.72% +0.02%
==========================================
Files 469 469
Lines 70831 70886 +55
Branches 484 240 -244
==========================================
+ Hits 51498 51553 +55
- Misses 19219 19222 +3
+ Partials 114 111 -3
🚀 New features to boost your workflow:
|
savindi7
reviewed
Jun 19, 2026
|
|
||
| <DialogContent> | ||
| <Typography variant="body2" color="text.secondary" sx={ { mb: 2 } }> | ||
| Before you proceed, please read the following carefully. |
Contributor
|
Topic of the box - "Enable advanced analytics with Moesif" and button should be "Enable" Is there a way we can inform users of what actually happens when they enable or do we expect them to come here with that idea? The UX is a bit unclear |
Contributor
|
Let's take the URLs from the deployment configs |
savindi7
reviewed
Jun 19, 2026
| /** | ||
| * Upgrade prompt shown in the legacy Insights page. | ||
| * | ||
| * Renders a simple card with an "Enable Advanced Analytics" button. Clicking the button |
Contributor
There was a problem hiding this comment.
are these comments necessary?
- Fix changeset typo (advance -> advanced). - Move advanced analytics upgrade card/dialog texts to the insights i18n namespace. - Read the terms of service URL from deployment config instead of hardcoding it. - Use theme values for the upgrade card badge instead of hardcoded sizing. - Surface the server-provided error description in the enable-analytics alert. - Trim verbose component documentation.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
features/admin.org-insights.v1/api/enable-advanced-analytics.ts (1)
35-43:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFail fast when endpoint or tenant domain is missing before issuing the POST.
tenantMoesifAnalyticsis initialized as an empty string in config state, andtenantDomainalso falls back to empty. In that state, this call can build an invalid URL and silently hit the wrong target instead of surfacing a clear configuration/auth readiness error.Proposed fix
export const enableAdvancedAnalytics = async (): Promise<void> => { const tenantDomain: string = store.getState()?.auth?.tenantDomain ?? ""; const enableAnalyticsEndpoint: string = store.getState().config.endpoints.tenantMoesifAnalytics; + + if (!enableAnalyticsEndpoint || !tenantDomain) { + throw new IdentityAppsApiException( + "Failed to enable Moesif advanced analytics due to missing endpoint or tenant domain.", + null, + null, + null, + null, + null + ); + } const response: { status: number } = await httpClient({ headers: { "Accept": "application/json" },🤖 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 `@features/admin.org-insights.v1/api/enable-advanced-analytics.ts` around lines 35 - 43, Add validation checks before the httpClient call to ensure both tenantDomain and enableAnalyticsEndpoint are non-empty strings. After extracting tenantDomain from the store state and before issuing the POST request via httpClient, verify that enableAnalyticsEndpoint is not an empty string and that tenantDomain is not empty (since it defaults to empty string). If either validation fails, throw an error or return early with a clear message indicating that the required configuration or authentication state is not ready, preventing the HTTP request from being made with an invalid URL.
🤖 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.
Outside diff comments:
In `@features/admin.org-insights.v1/api/enable-advanced-analytics.ts`:
- Around line 35-43: Add validation checks before the httpClient call to ensure
both tenantDomain and enableAnalyticsEndpoint are non-empty strings. After
extracting tenantDomain from the store state and before issuing the POST request
via httpClient, verify that enableAnalyticsEndpoint is not an empty string and
that tenantDomain is not empty (since it defaults to empty string). If either
validation fails, throw an error or return early with a clear message indicating
that the required configuration or authentication state is not ready, preventing
the HTTP request from being made with an invalid URL.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 6e8157ba-296f-41c0-9e22-c02499e1ef88
📒 Files selected for processing (11)
.changeset/huge-berries-thank.mdapps/console/src/public/deployment.config.jsonfeatures/admin.analytics.v1/pages/insights-page.tsxfeatures/admin.core.v1/store/reducers/config.tsfeatures/admin.org-insights.v1/api/enable-advanced-analytics.tsfeatures/admin.org-insights.v1/components/advanced-analytics-upgrade-card.tsxfeatures/admin.org-insights.v1/pages/org-insights.tsxfeatures/admin.tenants.v1/configs/endpoints.tsfeatures/admin.tenants.v1/models/endpoints.tsmodules/i18n/src/models/namespaces/insights-ns.tsmodules/i18n/src/translations/en-US/portals/insights.ts
✅ Files skipped from review due to trivial changes (2)
- features/admin.core.v1/store/reducers/config.ts
- .changeset/huge-berries-thank.md
🚧 Files skipped from review as they are similar to previous changes (4)
- features/admin.analytics.v1/pages/insights-page.tsx
- features/admin.org-insights.v1/components/advanced-analytics-upgrade-card.tsx
- features/admin.org-insights.v1/pages/org-insights.tsx
- apps/console/src/public/deployment.config.json
Use a plain white announcement banner with a "New" chip and an enable button, and restyle the confirmation dialog to match the CDS feature preview modal (content first, full-width preview image, primary action at the bottom right). Refine the copy for a more professional tone and read the terms of service URLs from config instead of hardcoded values.
coderabbitai
Bot
requested review from
DonOmalVindula,
JayaShakthi97 and
NipuniBhagya
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
features/admin.org-insights.v1/components/moesif-analytics-preview.tsx (1)
55-55: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueAdd explicit type annotation to
useTranslationdestructuring.As per coding guidelines, explicit type annotations should be used everywhere and type inference should not be relied upon.
✏️ Suggested type annotation
- const { t } = useTranslation(); + const { t }: { t: TFunction } = useTranslation();You'll need to import
TFunctionfromreact-i18next.🤖 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 `@features/admin.org-insights.v1/components/moesif-analytics-preview.tsx` at line 55, The `useTranslation` destructuring in the moesif-analytics-preview.tsx component is missing explicit type annotation for the `t` variable. Import `TFunction` from the `react-i18next` library and add it as an explicit type annotation to the destructured `t` variable in the line `const { t } = useTranslation();` to align with the coding guideline of using explicit type annotations instead of relying on type inference.Source: Coding guidelines
🤖 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 `@features/admin.analytics.v1/models/moesif-analytics.ts`:
- Around line 27-31: The enableAllPublishers property in the interface has
misleading JSDoc claiming it's set by the enableAdvancedAnalytics API call, but
that API returns Promise<void> and never processes or returns response data.
Additionally, this property is never referenced or assigned anywhere in the
codebase. Either remove the unused enableAllPublishers property and its JSDoc
from the interface, or update the enableAdvancedAnalytics function to capture
and return the response body data that would populate this property, then update
the calling code to handle the returned data.
In `@features/admin.org-insights.v1/components/moesif-analytics-preview.tsx`:
- Around line 48-53: The MoesifAnalyticsPreview component accepts a
termsOfServiceUrl prop that defaults to an empty string, which creates a broken
link when rendered with href="". Fix this by providing a fallback constant for
termsOfServiceUrl similar to how MOESIF_TOS_URL_FALLBACK is used for
moesifTermsOfServiceUrl in the component's function signature, or alternatively
conditionally render the Terms of Service link element only when a valid
non-empty termsOfServiceUrl is provided. Choose the approach that best fits your
use case where the link is rendered at line 90.
---
Nitpick comments:
In `@features/admin.org-insights.v1/components/moesif-analytics-preview.tsx`:
- Line 55: The `useTranslation` destructuring in the
moesif-analytics-preview.tsx component is missing explicit type annotation for
the `t` variable. Import `TFunction` from the `react-i18next` library and add it
as an explicit type annotation to the destructured `t` variable in the line
`const { t } = useTranslation();` to align with the coding guideline of using
explicit type annotations instead of relying on type inference.
🪄 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.yml
Review profile: CHILL
Plan: Pro
Run ID: df102568-c2ad-4d72-a7d4-8a90478e41ae
⛔ Files ignored due to path filters (1)
features/admin.org-insights.v1/assets/preview/analytics_preview.pngis excluded by!**/*.png
📒 Files selected for processing (5)
features/admin.analytics.v1/models/moesif-analytics.tsfeatures/admin.org-insights.v1/components/advanced-analytics-upgrade-card.tsxfeatures/admin.org-insights.v1/components/moesif-analytics-preview.tsxmodules/i18n/src/models/namespaces/insights-ns.tsmodules/i18n/src/translations/en-US/portals/insights.ts
✅ Files skipped from review due to trivial changes (1)
- modules/i18n/src/translations/en-US/portals/insights.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- modules/i18n/src/models/namespaces/insights-ns.ts
- features/admin.org-insights.v1/components/advanced-analytics-upgrade-card.tsx
Comment on lines
+27
to
+31
| /** | ||
| * When true, all event publishers are enabled (set by the server after the | ||
| * enableAdvancedAnalytics API call succeeds). | ||
| */ | ||
| enableAllPublishers?: boolean; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Search for references to enableAllPublishers in the codebase
# Search for reads/writes of enableAllPublishers
rg -n --type=ts --type=tsx -C3 'enableAllPublishers'Repository: wso2/identity-apps
Length of output: 90
🏁 Script executed:
#!/bin/bash
# Search for enableAllPublishers references with corrected command
rg -n 'enableAllPublishers' --type ts -C2
# Also search in the enable-advanced-analytics.ts file mentioned in the review
fd 'enable-advanced-analytics' -t fRepository: wso2/identity-apps
Length of output: 568
🏁 Script executed:
#!/bin/bash
# Find and examine the MoesifPublisherInterface definition file
fd 'moesif-analytics' -t fRepository: wso2/identity-apps
Length of output: 379
🏁 Script executed:
# View the enable-advanced-analytics API to verify return type and response handling
cat -n features/admin.org-insights.v1/api/enable-advanced-analytics.tsRepository: wso2/identity-apps
Length of output: 2449
🏁 Script executed:
# View the moesif-analytics model file to see the full interface
cat -n features/admin.analytics.v1/models/moesif-analytics.tsRepository: wso2/identity-apps
Length of output: 4032
🏁 Script executed:
# Search more broadly for enableAllPublishers property assignment or usage
rg -n 'enableAllPublishers' -A2 -B2Repository: wso2/identity-apps
Length of output: 504
Remove the unused enableAllPublishers property or update the API to populate it.
The JSDoc states this property is "set by the server after the enableAdvancedAnalytics API call succeeds," but enableAdvancedAnalytics returns Promise<void> and does not process the response body—it only validates response.status !== 200. Additionally, the property is never referenced or assigned anywhere in the codebase. Either remove this unused property from the interface or update the API to extract and return the response data.
🤖 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 `@features/admin.analytics.v1/models/moesif-analytics.ts` around lines 27 - 31,
The enableAllPublishers property in the interface has misleading JSDoc claiming
it's set by the enableAdvancedAnalytics API call, but that API returns
Promise<void> and never processes or returns response data. Additionally, this
property is never referenced or assigned anywhere in the codebase. Either remove
the unused enableAllPublishers property and its JSDoc from the interface, or
update the enableAdvancedAnalytics function to capture and return the response
body data that would populate this property, then update the calling code to
handle the returned data.
Delete the unused MoesifAnalyticsPreview component (flagged by knip) that was never wired into any page and imported preview images that do not exist, causing lint/build failures. Drop the now-orphaned preview i18n keys and their interface entries.
Determine advanced analytics enablement from the get-publishers API response instead of the dashboardEnabled deployment flag. Treat enableAllPublishers as enabled (the enablement map may be empty in that case), otherwise enable only when at least one publisher is on, and fall back to the legacy view when the publisher is not configured (404) or no publisher is enabled.
bhagyasakalanka
force-pushed
the
advance-analytics
branch
from
d5e7cc3 to
a0f79b2
Compare
Update the dialog text, enlarge it to match the feature preview modal, add spacing before the agreement checkbox, and fold the permanence note into the bullet list (removing the separate warning alert).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Provide capability to enable advanced moesif analytics