34849 dashboard fixes#35695
Open
nicobytes wants to merge 44 commits into
Open
Conversation
…tching logic - Introduced `getTotalEvents` method in `DotAnalyticsService` to fetch total events from the new analytics event API, supporting both predefined ranges and custom date ranges. - Updated service methods to handle optional parameters like `granularity`, `eventType`, and `siteId`. - Refactored related store logic in `with-pageview.feature.ts` to utilize the new service methods, ensuring accurate state management. - Added comprehensive unit tests for the new functionality, ensuring robust coverage and reliability. This update modernizes the analytics data access layer and prepares for future enhancements.
- Introduced new methods in `DotAnalyticsService` for fetching unique visitors, top content, and page views by device/browser, utilizing the new analytics event API. - Updated the service to handle optional parameters and improved the request handling logic. - Enhanced unit tests for the new methods, ensuring comprehensive coverage and error handling. - Refactored related types and store logic to align with the new API structure. This update modernizes the analytics service and improves data retrieval capabilities.
…ling - Updated `DotAnalyticsService` to support new methods for fetching total events and unique visitors with improved granularity handling. - Refactored types to better represent API parameters and responses, including new types for total events and unique visitors. - Enhanced unit tests to cover error propagation for total events, top content, and pageviews by device/browser, ensuring robust error handling. - Cleaned up unused code in `with-pageview.feature.ts` to streamline the implementation. This update improves the analytics service's functionality and reliability in handling API responses.
…roved granularity handling - Updated `DotAnalyticsService` to replace deprecated methods with new ones for fetching total events and unique visitors, now supporting enhanced granularity options. - Refactored types in `analytics-api.types.ts` to remove obsolete granularity parameters and align with the new API structure. - Improved data handling in `fillMissingApiDates` utility to accommodate monthly granularity. - Cleaned up unused code and ensured consistency in type definitions for better maintainability. This update enhances the analytics service's functionality and prepares it for future improvements.
…Service - Added new methods in `DotAnalyticsService` for fetching session engagement data, including aggregate, time-series, and grouped by dimensions. - Updated types in `analytics-api.types.ts` to define parameters and responses for the new session engagement API. - Refactored data handling in the engagement feature to utilize the new session engagement methods, improving data retrieval and state management. - Removed deprecated engagement-related types and streamlined utility functions for better maintainability. This update enhances the analytics service's capabilities and prepares it for future enhancements.
…ormatting - Introduced a new `lint-staged.config.mjs` file to define linting and formatting commands for staged files. - Configured linting for TypeScript and JavaScript files using `yarn nx affected -t lint` and formatting with `yarn nx format:write`. - Updated `package.json` to include `lint-staged` as a dependency and removed the deprecated `compodoc` command. - Replaced the `prepare.js` script with a direct husky command in `package.json` for improved pre-commit hook execution. - Updated pre-commit hook to check for the readiness of the frontend toolchain before running checks. This update enhances the development workflow by ensuring code quality checks are performed on staged files before commits.
…gagement features - Updated `DotAnalyticsService` to improve data mapping for engagement metrics, ensuring proper handling of empty or null values for `name`. - Refactored engagement feature methods to allow independent loading and error handling for engagement KPIs, breakdowns, and sparkline data. - Introduced new error messages for loading engagement data in `Language.properties` to provide clearer feedback on failures. - Enhanced utility functions for engagement data to ensure consistent decimal precision across KPIs and sparkline data. These changes improve the robustness and clarity of engagement analytics data processing.
…849-engagement-tab
…hods to DotAnalyticsService - Introduced `getContentAttribution` and `getConversionsOverview` methods in `DotAnalyticsService` to fetch data from the new analytics endpoints. - Updated constants and types to support the new API structure, including `ContentAttributionData` and `ConversionOverviewData`. - Enhanced unit tests to cover the new methods and ensure proper data handling. - Refactored related components and services to integrate the new analytics features seamlessly. These changes improve the analytics capabilities by providing detailed insights into content attribution and conversion metrics.
…reshold - Added `completionRolePrompt` and `completionTextPrompt` to the AI configuration for improved user guidance. - Introduced `embeddingsSearchThreshold` with a default value of 0.5 to enhance search accuracy. - Updated related components and tests to accommodate the new configuration options. These changes aim to improve the AI's usability and effectiveness in generating content.
…checks and conversions - Refactored `HealthStatusTypes` to use a constant object for improved type safety and added an `ERROR` state. - Introduced a new health check endpoint in `DotAnalyticsService` to fetch application health status. - Updated constants and types to support the new health check API structure. - Enhanced unit tests to cover the new health check functionality and ensure proper response handling. - Improved the analytics dashboard with new conversion overview methods and refined data handling for better user experience. These changes enhance the analytics capabilities by providing detailed health insights and improving the overall robustness of the service.
…34849-dashboard-charts
…sService - Updated `healthCheck` method documentation to clarify its purpose and response types. - Refactored utility functions for better organization and added unit tests for `zipDailyUniqueVisitorsForTrafficChart` and `analyticsResponseBodyMessage`. - Enhanced error handling in `DotAnalyticsErrorComponent` to differentiate between network errors and service unavailability. - Updated `Language.properties` with new error messages for network issues. These changes improve the clarity and robustness of the analytics service, ensuring better user experience and error reporting.
…34849-dashboard-charts
…for improved layout - Replaced div elements with p-card components in various analytics reports for better visual structure. - Updated title handling to use ng-template within p-card for consistent display. - Adjusted component styles and test cases to reflect changes in title rendering and layout. These modifications enhance the user interface of the analytics dashboard, ensuring a more cohesive and modern design.
- Added a `route` property to `DashboardTabConfig` for improved navigation. - Updated `DotAnalyticsDashboardStore` to manage tab state based on the active route. - Refactored `DotAnalyticsDashboardComponent` to utilize `router-outlet` for nested report routes. - Removed unused `silentNavigate` utility to streamline navigation logic. - Adjusted tests to verify routing behavior and tab synchronization. These changes improve the user experience by ensuring that the dashboard reflects the current route and enhances the overall structure of the component.
- Added D3.js and ngx-charts support for pie chart visualizations in the analytics dashboard. - Introduced new utility functions to transform data for ngx-charts compatibility. - Updated existing components to utilize the new pie chart component, improving the visual representation of engagement and device breakdowns. - Refactored dashboard layout to accommodate the new chart types and enhance user experience. These changes significantly improve the analytics dashboard's data visualization capabilities, providing clearer insights into user engagement and device usage.
…34849-dashboard-charts
…pie charts - Changed health check endpoint from `/api/v1/health` to `/api/v1/analytics/health` to align with the analytics service. - Refactored health check logic to interpret the `available` property as either a boolean or string. - Introduced a new `PieChartEntry` interface for pie chart data representation, replacing `NgxChartsPieEntry`. - Updated utility functions and components to utilize the new pie chart format, enhancing data visualization in the analytics dashboard. - Adjusted tests to reflect changes in health check responses and pie chart data transformations. These updates improve the accuracy of health checks and streamline the integration of pie chart visualizations in the analytics dashboard.
…34849-dashboard-charts
… adjustments - Introduced a new `DotAnalyticsBarChartComponent` for visualizing engagement metrics by device, browser, and language. - Updated the engagement report layout to accommodate the new bar charts, adjusting column spans for better organization. - Refactored existing HTML and TypeScript files to integrate the new component and ensure proper data binding. - Enhanced unit tests to cover the new bar chart functionality and verify correct rendering of engagement metrics. These changes improve the analytics dashboard's data visualization capabilities, providing clearer insights into user engagement trends.
…tion - Changed chart titles in the engagement report from static strings to localized keys for device, browser, and language metrics. - This update enhances the internationalization of the analytics dashboard, ensuring that titles are dynamically fetched based on user locale. These modifications improve the user experience by providing contextually relevant titles in the engagement report.
… for pie charts - Renamed `transformDeviceBrowsersData` to `transformBrowsersToPieChartEntries` and `transformDeviceBrowsersToPieChartEntries` to `transformDevicesToPieChartEntries` for clarity. - Updated the implementation of these functions to aggregate data by browser and device, respectively, improving the accuracy of pie chart visualizations. - Adjusted related unit tests to reflect the new function names and ensure correct data transformation. - Modified HTML templates and component logic to utilize the updated functions, enhancing the overall analytics dashboard layout and data presentation. These changes improve the maintainability and clarity of the analytics codebase, ensuring better data visualization for user engagement metrics.
…n and related tests
github-actions
Bot
added
Area : Backend
Contributor
|
Claude finished @nicobytes's task in 4m 39s —— View job PR Review
🔴 Bug —
|
| Comment says | Actual |
|---|---|
Row 1: engagement-rate col-span-12 |
md:col-span-9 |
Row 3: breakdown doughnut col-span-12 |
breakdown is on row 1 at md:col-span-3 |
Row 4: language/browser/device each col-span-3 |
each md:col-span-4 |
Either update the comment to reflect the implemented grid, or delete it. Misleading docs at the top of the file are worse than no docs.
🟡 Deferred placeholder size mismatch (layout shift)
dot-analytics-engagement-report.component.html:50-53 — the breakdown defer placeholder is md:col-span-12 min-h-48, but the loaded content sits in md:col-span-3. When the defer fires, the chart snaps from a full-row placeholder to a one-quarter wedge — a visible reflow on first render. The pageview placeholders for browser/device line up correctly (md:col-span-6), so the same alignment is doable here.
🟡 PR description contradicts implementation
The body says "Change the device breakdown from a bar chart to a pie chart" and "Display browser breakdown as a bar chart and device breakdown as a pie chart". In the actual checked-in HTML, all three of language/browser/device use dot-analytics-bar-engagement-chart, and only the engaged-vs-bounced breakdown is a pie. The component tests confirm 1 pie + 3 bar engagement charts. The description seems to describe an intermediate state that was reverted (see commit history 7d7ed94d/1b2bca87). Worth updating the PR body so reviewers and git log aren't misled.
🟡 Inconsistent error message handling between features
with-pageview.feature.ts builds error strings via pageviewFeatureErrorMessage(), which prefers the API response body's message, then Error.message, then i18n fallback — avoiding Angular's generic HttpErrorResponse.message. with-engagement.feature.ts:132,193,273,369 uses plain error?.message || getErrorMessage(...), which is exactly the case the pageview helper exists to avoid. Result: engagement loaders surface less useful messages on failure ("Http failure response for ...") instead of the backend's reason.
Consider extracting pageviewFeatureErrorMessage to a shared util (or matching engagementFeatureErrorMessage) and using it on both sides.
🟡 Inconsistent "no site selected" reset behaviour
with-pageview.feature.ts:542-553 resets every pageview slice back to INIT when currentSiteId is empty. with-engagement.feature.ts:386-391 just returns. If a user switches to a site with no currentSiteId, the engagement slices will keep stale data from the previous site (LOADED with old numbers) instead of clearing. Easy to align.
🟢 Smaller things worth a look
mapSortedTotalsToPieChartEntries(analytics-data.utils.ts:458-480) — when overflow exists withmaxSlices=10andotherLabelset, you show 9 named slices + 1 "Other", not 10 + "Other". Probably intentional, but the JSDoc could call it out so callers aren't surprised.loginService.currentUserLanguageId?.replace('_', '-') ?? 'en-US'(with-engagement.feature.ts:344) — the field is typed asstringand initialised to'', so the optional chain is dead. Empty string passes the nullish coalescing too, soreplacereturns''andformatLanguageCodeForDisplaywill get an emptyuiLocale. Either guard with|| 'en-US', or guaranteeLoginServicehas populated this before the first call.formatSecondsToTime— sessions over an hour render as120m 30srather than2h 0m 30s. Probably fine for typical session lengths; flagging for awareness.
Tests look reasonable and the new utility split (transformDeviceBreakdownToPieChartEntries / transformBrowserBreakdownToPieChartEntries / toEngagementPlatformPieEntries) is cleaner than the old combined helper.
The pie-key bug is the only must-fix before merge. Everything else is cleanup that I'd address in this PR if possible, but won't break anything in production.
· Branch: 34849-dashboard-fixes
…function and related tests - Deleted the `transformDeviceBrowsersToPieChartEntries` function from the analytics data utilities as part of a cleanup effort. - Removed associated unit tests to streamline the codebase and improve maintainability. - Updated relevant imports and references in the analytics dashboard components to reflect these changes. This refactor enhances the clarity and efficiency of the analytics data access layer.
- Updated `getPageviewsByDeviceBrowser` method to support grouping by both device and browser. - Introduced new types for `DeviceBreakdownData` and `BrowserBreakdownData` to handle the response structure. - Modified related utility functions to transform data for pie chart entries based on the new grouping. - Adjusted component templates and state management to reflect the changes in data structure and improve the analytics dashboard. - Updated tests to ensure coverage for new functionality and maintain existing behavior. This refactor improves the flexibility and clarity of the analytics data access layer.
- Integrated `LoginService` to fetch the current user's language for improved engagement metrics. - Updated `toEngagementPlatforms` function to translate language codes into human-readable names based on the user's locale. - Enhanced `EngagementPlatformMetrics` interface to include `totalSessions` for better data representation. - Refactored engagement-related components and tests to accommodate new metrics and ensure accurate rendering of data. - Adjusted component templates to display stacked engagement charts for browser, device, and language metrics. This update improves the analytics dashboard's usability and data clarity.
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
1 participant
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.
This pull request refactors how device and browser analytics are displayed and processed in the engagement report, shifting device breakdowns from bar charts to pie charts and improving data aggregation utilities. The main changes include new utility functions for aggregating browser and device data, updates to the dashboard layout to reflect these changes, and corresponding updates to tests and documentation.
Analytics Data Aggregation Utilities:
transformBrowsersToPieChartEntriesandtransformDevicesToPieChartEntriesfunctions to aggregate and format analytics data for browsers and devices respectively, replacing the previoustransformDeviceBrowsersDatafunction. These new utilities sum values by browser or device, sort results, and cap them at 10 entries. [1] [2] [3]toEngagementPlatformPieEntriesfor mapping engagement platform metrics to pie chart entries, filtering out zero or invalid values. [1] [2] [3]Dashboard Layout and Visualization Updates:
dot-analytics-engagement-report.component.html) to:Testing Updates:
Documentation:
These changes improve the clarity and consistency of analytics visualizations, streamline data processing, and ensure tests accurately reflect the new dashboard structure.
This PR fixes: #34849