feat: Scheduled Dashboards UI #4485
Conversation
WalkthroughThe changes introduce a new feature for managing scheduled dashboard reports, enhancing user interaction with the dashboard. Key updates include a toggle for cached reports, a new component for displaying scheduled reports, and modifications to existing components to support this functionality. Additionally, localization updates and interface definitions have been added to support the new features. Changes
Assessment against linked issues
Suggested labels
Tip New featuresWalkthrough comment now includes:
Notes:
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (15)
- web/src/components/alerts/AlertList.vue (1 hunks)
- web/src/components/reports/CreateReport.vue (8 hunks)
- web/src/components/reports/ReportList.vue (5 hunks)
- web/src/locales/languages/en.json (2 hunks)
- web/src/services/reports.ts (1 hunks)
- web/src/ts/interfaces/alert.ts (1 hunks)
- web/src/ts/interfaces/dashboard.ts (1 hunks)
- web/src/ts/interfaces/global.ts (1 hunks)
- web/src/ts/interfaces/iam.ts (1 hunks)
- web/src/ts/interfaces/index.ts (1 hunks)
- web/src/ts/interfaces/ingestion.ts (1 hunks)
- web/src/ts/interfaces/query.ts (1 hunks)
- web/src/utils/date.ts (2 hunks)
- web/src/views/Dashboards/ScheduledDashboards.vue (1 hunks)
- web/src/views/Dashboards/ViewDashboard.vue (17 hunks)
Files skipped from review due to trivial changes (8)
- web/src/components/alerts/AlertList.vue
- web/src/components/reports/ReportList.vue
- web/src/ts/interfaces/alert.ts
- web/src/ts/interfaces/global.ts
- web/src/ts/interfaces/iam.ts
- web/src/ts/interfaces/index.ts
- web/src/ts/interfaces/ingestion.ts
- web/src/ts/interfaces/query.ts
Additional comments not posted (17)
web/src/ts/interfaces/dashboard.ts (1)
16-26: Interface Definition for ScheduledDashboardReportThe interface
ScheduledDashboardReportis well-defined and aligns with the PR objectives for managing scheduled reports. The properties cover essential aspects like scheduling frequency, last trigger time, and caching status.Consider verifying the type consistency for
orgId. It is defined asstring | number, which might lead to inconsistencies depending on how organization identifiers are handled elsewhere in the application.web/src/utils/date.ts (1)
243-249: New Function: convertUnixToQuasarFormatThe function
convertUnixToQuasarFormatis a valuable addition for converting Unix timestamps (in microseconds) to a formatted date string. The implementation is clear, and it correctly handles the conversion process, enhancing the module's functionality for date formatting.The function is well-implemented and aligns with the needs of the application for handling date and time conversions.
web/src/views/Dashboards/ScheduledDashboards.vue (2)
17-95: Review of Vue Template in ScheduledDashboards.vueThe template section is well-structured and makes good use of Quasar components for a consistent UI experience. Here are some observations and suggestions:
- Localization and Text Management: The use of the
tmethod for text ensures that the component can support internationalization. This is a good practice and should be maintained.- Data Binding: The
:rows="scheduledReports"and:columns="columns"bindings in theq-tableare correctly set up to display dynamic data.- Event Handling: The
@click="createNewReport"on theq-btnand@row-click="openReport"on theq-tableare appropriate for user interactions. Ensure that these methods (createNewReportandopenReport) handle errors gracefully and provide feedback to the user.- Accessibility: Consider adding
aria-labelsto interactive elements for better accessibility.Overall, the template follows good practices in Vue.js and Quasar framework usage.
282-307: Review of Style Section in ScheduledDashboards.vueThe SCSS styles are scoped to the component, which is good practice to avoid styling conflicts with other parts of the application. Here are some points to consider:
- Consistency: The styles use consistent naming conventions and SCSS features like variables and nesting. This makes the code easier to read and maintain.
- Responsiveness: Ensure that the styles handle different screen sizes and devices. If the application is meant to be responsive, consider using media queries or responsive units.
- Accessibility: Check if the color contrasts meet accessibility standards, especially for text and interactive elements.
Overall, the styling is well-implemented and follows good practices in modern CSS development.
web/src/components/reports/CreateReport.vue (7)
106-127: Add a toggle for cached reports with an informational tooltip.The addition of a
q-togglecomponent linked to theisCachedReportstate variable allows users to enable or disable cached reports. This is a crucial feature for managing the caching behavior of reports. The accompanying tooltip provides necessary information about the implications of using cached reports, specifically that sharing is disabled for these reports. This is a good implementation as it enhances user understanding and control over report settings.
630-630: Conditional rendering based on cached report status.The navigation button to continue to the next step is conditionally rendered based on the
isCachedReportstate. This ensures that users do not proceed with irrelevant options when cached reports are active. It's a good practice to conditionally render UI elements based on the state to prevent user errors and enhance the UI's responsiveness to user settings.
650-650: Conditional rendering of the share step.The share step is also conditionally rendered based on the
isCachedReportstate. This is consistent with the toggle's functionality, ensuring that sharing options are only available when the report is not cached. This consistency in UI behavior is crucial for maintaining a predictable and user-friendly interface.
856-859: Initialization of reactive state variables for cached reports and tooltips.The state variables
isCachedReportandshowInfoTooltipare initialized withref(false), making them reactive. This is necessary for Vue 3 composition API to ensure that changes to these variables trigger UI updates. Proper use of reactive state management is key to building dynamic and responsive interfaces.
1005-1026: Function to set initial report data based on query parameters.The
setInitialReportDatafunction is well-implemented to handle various query parameters that configure the report settings upon loading. This includes setting the report type as cached, selecting folders, dashboards, and tabs based on the provided IDs. This function is crucial for initializing the form with pre-filled values, reducing the effort required by the user to set up a report, and potentially reducing errors.
1296-1297: Handling of destinations based on cached report status.When a report is marked as cached, the destinations array is cleared. This aligns with the functionality that disables sharing for cached reports. It's a good practice to enforce such business rules at the data model level to ensure consistency and prevent the application state from entering invalid configurations.
1493-1494: Adjust cached report status based on the presence of destinations.This code segment adjusts the
isCachedReportstate based on whether there are any destinations set for the report. If there are no destinations, the report is considered cached. This is a sensible fallback mechanism to ensure the report's configuration is consistent with its intended use, especially in scenarios where the report might be edited and destinations are removed.web/src/views/Dashboards/ViewDashboard.vue (4)
169-186: Addition of Dropdown Button for Scheduled ReportsThe addition of a
<q-btn-dropdown>for accessing scheduled reports is well-implemented. It enhances the dashboard's functionality by providing a user-friendly interface to manage scheduled reports. TheopenScheduledReportsmethod is correctly bound to the click event, which should trigger the display of the scheduled reports dialog.
218-231: Scheduled Reports Dialog ImplementationThe
<q-dialog>component for displaying scheduled reports is correctly implemented. It uses theshowScheduledReportsDialogreactive reference for visibility control, which is a standard practice in Vue.js. The passing of props such asreports,loading,folderId,dashboardId, andtabIdto theScheduledDashboardscomponent is done correctly, ensuring that all necessary data is available for the component to function properly.
265-266: Import and Registration of ScheduledDashboards ComponentThe import and registration of the
ScheduledDashboardscomponent are correctly handled. This component is crucial for displaying the scheduled reports, and its registration in the components object of the Vue instance ensures it can be used in the template.Also applies to: 283-283
Line range hint
294-312: Reactive References and Computed Properties for Dashboard ContextThe setup of reactive references like
showScheduledReportsDialogand computed properties fordashboardId,folderId, andtabIdare appropriate. These are used to manage the state and context of the dashboard, which is essential for the functionality of scheduled reports.web/src/locales/languages/en.json (2)
649-651: Addition of Localization Entries for Scheduled ReportsThe new localization entries for "scheduledReports", "newReport", and "scheduledDashboards" are correctly added. These entries are essential for supporting multiple languages and enhancing the user interface's accessibility.
902-910: Localization Entries for Report AttributesThe addition of localization entries for various report attributes like "timeRange", "frequency", "lastTriggeredAt", "createdAt", "tab", "cached", "cachedReport", "destination", and "scheduled" is correctly implemented. These entries will help in displaying these attributes in the user interface, making it more user-friendly and accessible.
omkarK06
force-pushed
the
feat/scheduled_dashboards
branch
2 times, most recently
from
074feb3 to
7b04a13
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (16)
- web/src/components/alerts/AlertList.vue (1 hunks)
- web/src/components/reports/CreateReport.vue (8 hunks)
- web/src/components/reports/ReportList.vue (5 hunks)
- web/src/locales/languages/en.json (2 hunks)
- web/src/services/reports.ts (1 hunks)
- web/src/ts/interfaces/alert.ts (1 hunks)
- web/src/ts/interfaces/dashboard.ts (1 hunks)
- web/src/ts/interfaces/global.ts (1 hunks)
- web/src/ts/interfaces/iam.ts (1 hunks)
- web/src/ts/interfaces/index.ts (1 hunks)
- web/src/ts/interfaces/ingestion.ts (1 hunks)
- web/src/ts/interfaces/query.ts (1 hunks)
- web/src/utils/date.ts (2 hunks)
- web/src/views/Dashboards/Dashboards.vue (14 hunks)
- web/src/views/Dashboards/ScheduledDashboards.vue (1 hunks)
- web/src/views/Dashboards/ViewDashboard.vue (10 hunks)
Files skipped from review due to trivial changes (9)
- web/src/components/alerts/AlertList.vue
- web/src/components/reports/ReportList.vue
- web/src/locales/languages/en.json
- web/src/ts/interfaces/alert.ts
- web/src/ts/interfaces/global.ts
- web/src/ts/interfaces/index.ts
- web/src/ts/interfaces/ingestion.ts
- web/src/ts/interfaces/query.ts
- web/src/views/Dashboards/Dashboards.vue
Files skipped from review as they are similar to previous changes (6)
- web/src/components/reports/CreateReport.vue
- web/src/services/reports.ts
- web/src/ts/interfaces/dashboard.ts
- web/src/ts/interfaces/iam.ts
- web/src/utils/date.ts
- web/src/views/Dashboards/ScheduledDashboards.vue
Additional comments not posted (3)
web/src/views/Dashboards/ViewDashboard.vue (3)
214-227: Dialog Component for Displaying Scheduled ReportsThe
<q-dialog>component setup for displaying scheduled reports is correctly implemented. The use of reactive properties likeshowScheduledReportsDialogto control the visibility of the dialog is a standard Vue practice. Passing necessary props such asreports,loading,folderId,dashboardId, andtabIdensures that theScheduledDashboardscomponent has all the required data to function correctly.This implementation aligns well with the PR's objectives to enhance dashboard interactivity and manage scheduled reports effectively.
751-782: Method to Open Scheduled Reports and Fetch DataThe
openScheduledReportsmethod is well-structured. It handles the opening of the dialog, resetting thescheduledReportsarray, and setting the loading state. The use of the reports service to fetch scheduled reports is appropriate. However, ensure that error handling is implemented to manage potential failures in the reports service call.Consider adding error handling for the reports service call to improve robustness. For example:
reports .list(store.state.selectedOrganization.identifier, currentDashboardData.data?.folderId, currentDashboardData.data?.dashboardId) .then((response) => { response.data.forEach((report, index) => { scheduledReports.push({ "#": index + 1, name: report.name, tab: report.dashboards?.[0]?.tabs?.[0], time_range: getTimeRangeValue(report.dashboards?.[0]?.timerange), frequency: getFrequencyValue(report.frequency), last_triggered_at: convertUnixToQuasarFormat(report.lastTriggeredAt), created_at: convertUnixToQuasarFormat(new Date(report.createdAt).getTime() * 1000), orgId: report.orgId, isCached: !report.destinations.length, }); }); }) .catch((error) => { showErrorNotification(error.message || "Failed to fetch reports"); isLoadingReports.value = false; }) .finally(() => { isLoadingReports.value = false; });
169-180: Addition of Button for Scheduled ReportsThe addition of a
<q-btn>component to trigger theopenScheduledReportsmethod is well-implemented. The button is correctly configured with accessibility and test attributes (data-test). The tooltip is also a nice touch for enhancing user experience.However, ensure that the icon used (
outlinedDescription) is appropriately defined and accessible. It might be beneficial to verify that this icon is correctly rendered and accessible to all users.Verification successful
Icon
outlinedDescriptionis Defined and AccessibleThe
outlinedDescriptionicon is correctly imported from@quasar/extras/material-icons-outlinedand used within theViewDashboard.vuecomponent. This confirms that the icon is defined and accessible, aligning with the Quasar framework's standard practices. No further action is required regarding the icon's definition or accessibility.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the icon `outlinedDescription` is defined and accessible grep -r "outlinedDescription" ./web/srcLength of output: 445
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- web/src/services/reports.ts (1 hunks)
- web/src/views/Dashboards/ScheduledDashboards.vue (1 hunks)
- web/src/views/Dashboards/ViewDashboard.vue (9 hunks)
Files skipped from review as they are similar to previous changes (2)
- web/src/services/reports.ts
- web/src/views/Dashboards/ScheduledDashboards.vue
Additional comments not posted (5)
web/src/views/Dashboards/ViewDashboard.vue (5)
169-180: Addition of Button for Scheduled ReportsThe addition of the
<q-btn>component to trigger theopenScheduledReportsmethod is well-implemented. The button is correctly placed within the UI and uses theoutlinedDescriptionfor its icon, which should be verified to ensure it represents the functionality accurately. The tooltip provides additional context, which enhances user experience.
214-227: Dialog Component for Displaying Scheduled ReportsThe
<q-dialog>component setup for displaying scheduled reports is correctly implemented. It uses theshowScheduledReportsDialogreactive reference for its visibility control, which is standard practice in Vue.js. The passing of necessary props such asreports,loading,folderId,dashboardId, andtabIdto theScheduledDashboardscomponent is done properly, ensuring that all required data is available for the component to function correctly.
Line range hint
261-279: Import and Registration of ScheduledDashboards ComponentThe import and registration of the
ScheduledDashboardscomponent are correctly handled. This is crucial for the component's functionality within this file. The addition of this component enhances the modular architecture of the application by separating concerns and allowing for better maintainability.
Line range hint
290-308: Reactive References for Dialog and Scheduled ReportsThe setup of reactive references such as
showScheduledReportsDialog,scheduledReports,isLoadingReports,dashboardId,folderId, andtabIdis appropriate. These references are essential for managing the state and interactions of the scheduled reports feature. It's important to ensure that these references are used consistently throughout the component to manage state effectively.
750-769: Method to Open Scheduled Reports and Fetch DataThe
openScheduledReportsmethod is well-structured. It handles the opening of the dialog, resetting thescheduledReportsarray, and setting the loading state. The use of the reports service to fetch scheduled reports is appropriate. However, ensure that error handling is implemented to manage potential failures in the reports service call.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- web/src/views/Dashboards/ScheduledDashboards.vue (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- web/src/views/Dashboards/ScheduledDashboards.vue
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- web/src/views/Dashboards/Dashboards.vue (14 hunks)
- web/src/views/Dashboards/ScheduledDashboards.vue (1 hunks)
- web/src/views/Dashboards/ViewDashboard.vue (8 hunks)
Files skipped from review due to trivial changes (2)
- web/src/views/Dashboards/Dashboards.vue
- web/src/views/Dashboards/ScheduledDashboards.vue
Additional comments not posted (5)
web/src/views/Dashboards/ViewDashboard.vue (5)
169-180: Addition of Button for Scheduled ReportsThe addition of the
<q-btn>component to trigger theopenScheduledReportsmethod is aligned with the PR objectives to enhance dashboard functionality by allowing users to manage scheduled reports. The use of Quasar components for UI consistency is noted, and the tooltip provides a good user experience by explaining the button's purpose.
214-227: Introduction of Dialog for Scheduled ReportsThe new
<q-dialog>component for displaying scheduled reports is a crucial part of the feature, allowing users to interact with the reports linked to the dashboard. The binding toshowScheduledReportsDialogensures that the dialog's visibility is controlled programmatically. Passing necessary props such asreports,loading,folderId,dashboardId, andtabIdto theScheduledDashboardscomponent is well-handled, ensuring that all relevant data is available for display and interaction within the component.
Line range hint
261-279: Component and Service ImportsThe import of
ScheduledDashboardsandreportsservices is necessary for the functionality of displaying and fetching scheduled reports. The organization of imports and the addition of theScheduledDashboardscomponent to the local components registry are correctly implemented. This setup facilitates the use of theScheduledDashboardscomponent within this file, supporting the new feature's requirements.
Line range hint
290-308: Reactive References and Computed Properties for Scheduled ReportsThe setup of reactive references like
showScheduledReportsDialog,scheduledReports, andisLoadingReports, along with computed properties fordashboardId,folderId, andtabId, is well done. These reactive states and computed properties are essential for managing the UI state and data flow concerning the scheduled reports feature. It ensures that the component reacts appropriately to changes and provides a dynamic user experience.
750-769: Method to Open Scheduled Reports and Fetch DataThe
openScheduledReportsmethod is well-structured. It handles the opening of the dialog, resetting thescheduledReportsarray, and setting the loading state. The use of the reports service to fetch scheduled reports is appropriate. However, ensure that error handling is implemented to manage potential failures in the reports service call.Consider adding error handling for the reports service call to improve robustness. For example:
reports .list(store.state.selectedOrganization.identifier, currentDashboardData.data?.folderId, currentDashboardData.data?.dashboardId) .then((response) => { response.data.forEach((report, index) => { scheduledReports.push({ "#": index + 1, name: report.name, tab: report.dashboards?.[0]?.tabs?.[0], time_range: getTimeRangeValue(report.dashboards?.[0]?.timerange), frequency: getFrequencyValue(report.frequency), last_triggered_at: convertUnixToQuasarFormat(report.lastTriggeredAt), created_at: convertUnixToQuasarFormat(new Date(report.createdAt).getTime() * 1000), orgId: report.orgId, isCached: !report.destinations.length, }); }); }) .catch((error) => { showErrorNotification(error.message || "Failed to fetch reports"); isLoadingReports.value = false; }) .finally(() => { isLoadingReports.value = false; });
omkarK06
force-pushed
the
feat/scheduled_dashboards
branch
from
746ad2b to
6daac01
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (16)
- web/src/components/alerts/AlertList.vue (1 hunks)
- web/src/components/reports/CreateReport.vue (8 hunks)
- web/src/components/reports/ReportList.vue (5 hunks)
- web/src/locales/languages/en.json (2 hunks)
- web/src/services/reports.ts (1 hunks)
- web/src/ts/interfaces/alert.ts (1 hunks)
- web/src/ts/interfaces/dashboard.ts (1 hunks)
- web/src/ts/interfaces/global.ts (1 hunks)
- web/src/ts/interfaces/iam.ts (1 hunks)
- web/src/ts/interfaces/index.ts (1 hunks)
- web/src/ts/interfaces/ingestion.ts (1 hunks)
- web/src/ts/interfaces/query.ts (1 hunks)
- web/src/utils/date.ts (2 hunks)
- web/src/views/Dashboards/Dashboards.vue (14 hunks)
- web/src/views/Dashboards/ScheduledDashboards.vue (1 hunks)
- web/src/views/Dashboards/ViewDashboard.vue (8 hunks)
Files skipped from review due to trivial changes (8)
- web/src/components/alerts/AlertList.vue
- web/src/components/reports/ReportList.vue
- web/src/ts/interfaces/global.ts
- web/src/ts/interfaces/iam.ts
- web/src/ts/interfaces/index.ts
- web/src/ts/interfaces/ingestion.ts
- web/src/ts/interfaces/query.ts
- web/src/views/Dashboards/Dashboards.vue
Files skipped from review as they are similar to previous changes (7)
- web/src/components/reports/CreateReport.vue
- web/src/locales/languages/en.json
- web/src/services/reports.ts
- web/src/ts/interfaces/alert.ts
- web/src/ts/interfaces/dashboard.ts
- web/src/views/Dashboards/ScheduledDashboards.vue
- web/src/views/Dashboards/ViewDashboard.vue
Additional comments not posted (1)
web/src/utils/date.ts (1)
218-218: Approved minor style improvement.The addition of the trailing comma in
getConsumableRelativeTimealigns with modern JavaScript practices and improves code consistency.
| export const convertUnixToQuasarFormat = (unixMicroseconds: any) => { | ||
| if (!unixMicroseconds) return ""; | ||
| const unixSeconds = unixMicroseconds / 1e6; | ||
| const dateToFormat = new Date(unixSeconds * 1000); | ||
| const formattedDate = dateToFormat.toISOString(); | ||
| return date.formatDate(formattedDate, "YYYY-MM-DDTHH:mm:ssZ"); | ||
| }; |
There was a problem hiding this comment.
Enhance type safety and error handling in convertUnixToQuasarFormat.
The function convertUnixToQuasarFormat correctly converts Unix time in microseconds to a formatted date string. However, it lacks robust type checking and error handling for non-numeric inputs. Consider enhancing the function by explicitly checking the type of unixMicroseconds and handling potential errors during date conversion.
Here's a suggested improvement:
-export const convertUnixToQuasarFormat = (unixMicroseconds: any) => {
+export const convertUnixToQuasarFormat = (unixMicroseconds: number) => {
if (typeof unixMicroseconds !== 'number' || !unixMicroseconds) return "";
const unixSeconds = unixMicroseconds / 1e6;
const dateToFormat = new Date(unixSeconds * 1000);
if (isNaN(dateToFormat.getTime())) return "";
const formattedDate = dateToFormat.toISOString();
return date.formatDate(formattedDate, "YYYY-MM-DDTHH:mm:ssZ");
};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.
| export const convertUnixToQuasarFormat = (unixMicroseconds: any) => { | |
| if (!unixMicroseconds) return ""; | |
| const unixSeconds = unixMicroseconds / 1e6; | |
| const dateToFormat = new Date(unixSeconds * 1000); | |
| const formattedDate = dateToFormat.toISOString(); | |
| return date.formatDate(formattedDate, "YYYY-MM-DDTHH:mm:ssZ"); | |
| }; | |
| export const convertUnixToQuasarFormat = (unixMicroseconds: number) => { | |
| if (typeof unixMicroseconds !== 'number' || !unixMicroseconds) return ""; | |
| const unixSeconds = unixMicroseconds / 1e6; | |
| const dateToFormat = new Date(unixSeconds * 1000); | |
| if (isNaN(dateToFormat.getTime())) return ""; | |
| const formattedDate = dateToFormat.toISOString(); | |
| return date.formatDate(formattedDate, "YYYY-MM-DDTHH:mm:ssZ"); | |
| }; |
3 participants
fix #4346
Summary by CodeRabbit
New Features
Bug Fixes
Documentation