You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "richardfogaca (via GitHub)" <gi...@apache.org> on 2026/01/27 00:25:40 UTC

[PR] Feat/auto refresh dashboard [superset]

richardfogaca opened a new pull request, #37459:
URL: https://github.com/apache/superset/pull/37459

   ### SUMMARY
   
   This PR implements a comprehensive real-time dashboard auto-refresh feature with an enhanced user experience. The main goals are to provide clear visual feedback about refresh status, prevent UI flickering during refreshes, and give users control over the refresh behavior.
   
   #### Key Features
   
   **1. Auto-Refresh Status Indicator**
   - A colored dot in the dashboard header indicates the current refresh state:
     - **Green (Success)**: Dashboard refreshed successfully on schedule
     - **Blue (Idle/Fetching)**: Waiting for first refresh or currently fetching data
     - **Yellow (Delayed)**: Refresh taking longer than expected OR 1 consecutive error
     - **Red (Error)**: 2+ consecutive errors
     - **White (Paused)**: Auto-refresh is paused (manually or by tab visibility)
   - Hovering over the indicator shows a tooltip with detailed status information including last refresh time, configured interval, and error counts
   
   **2. Pause/Resume Controls**
   - Users can manually pause and resume auto-refresh via a play/pause button next to the status indicator
   - When resuming, charts are immediately refreshed before the timer restarts
   - The pause state is independent of tab visibility pause
   
   **3. Tab Visibility Auto-Pause (Opt-in)**
   - Optional feature to automatically pause auto-refresh when the browser tab becomes inactive
   - When the user returns to the tab, data is immediately fetched and the timer restarts
   - Respects manual pause state - returning to tab won't auto-resume if manually paused
   - Implemented using the Page Visibility API via `useTabVisibility` hook
   
   **4. Flickering Prevention**
   Multiple improvements to ensure smooth visual updates during auto-refresh:
   - **Loading spinner suppression**: Charts maintain their previous data display during refresh instead of showing loading spinners
   - **FiltersBadge preservation**: Filter indicators remain visible during refresh cycles
   - **Stable series ordering**: Added secondary sort by name to prevent legend reordering flicker in ECharts
   - **Tree chart animation control**: Disabled expand/collapse animations during refresh
   - **Status dot debouncing**: Smooth color transitions with CSS and state debouncing
   
   **5. Timer Race Condition Fix**
   - Added sequence guard mechanism to prevent stale refresh cycles from re-arming timers with old intervals
   - Each refresh cycle captures a sequence ID; old cycles bail out if the sequence no longer matches
   - This prevents the bug where changing intervals while a refresh is in-flight would cause the old interval to persist
   
   **6. Configuration Changes**
   - Dropped the 5-second preset from auto-refresh options (too aggressive for most use cases)
   - Allowed custom intervals as low as 1 second for real-time use cases
   - Added validation for minimum refresh intervals with configurable server-side limits
   
   #### Architecture
   
   **New Files:**
   - `dashboard/types/autoRefresh.ts` - Type definitions (`AutoRefreshStatus` enum, `AutoRefreshState` interface) and state defaults
   - `dashboard/actions/autoRefresh.ts` - Redux actions for refresh state management (status, pause, errors, etc.)
   - `dashboard/contexts/AutoRefreshContext.tsx` - React context for propagating refresh state to charts (enables spinner suppression)
   - `dashboard/hooks/useRealTimeDashboard.ts` - Main hook for accessing and managing refresh state with Redux selectors
   - `dashboard/hooks/useTabVisibility.ts` - Hook for tracking browser tab visibility via Page Visibility API
   - `dashboard/hooks/useAutoRefreshTabPause.ts` - Hook that orchestrates tab-based pause/resume behavior
   - `dashboard/hooks/useCurrentTime.ts` - Real-time clock hook for tooltip time displays
   - `dashboard/components/AutoRefreshStatus/` - Status indicator components (dot, tooltip content)
   - `dashboard/components/AutoRefreshIndicator/` - Unified indicator with status dot and pause button
   - `dashboard/components/AutoRefreshControls/` - Standalone pause/resume and refresh buttons
   - `dashboard/components/RefreshButton/` - Manual refresh button component
   - `dashboard/components/RefreshFrequency/` - Shared refresh frequency select component with validation
   
   **Modified Files:**
   - `dashboard/reducers/dashboardState.js` - Added auto-refresh state handling with error threshold logic
   - `dashboard/components/Header/index.tsx` - Converted from JSX, integrated auto-refresh orchestration with sequence guard
   - `dashboard/containers/DashboardPage.tsx` - Wrapped dashboard in `AutoRefreshProvider`
   - `components/Chart/ChartRenderer.jsx` - Added `suppressLoadingSpinner` prop for auto-refresh cycles
   - `dashboard/components/FiltersBadge/index.tsx` - Added auto-refresh awareness to preserve filter indicators
   - `dashboard/components/gridComponents/Chart.jsx` - Integrated `useIsAutoRefreshing` context hook
   - `dashboard/components/gridComponents/Tab.jsx` - Added lazy-load refresh handling with `isLazyLoad` flag
   - `dashboard/components/PropertiesModal/index.tsx` - Integrated refresh settings section
   - `dashboard/components/RefreshIntervalModal.tsx` - Added pause-on-inactive-tab checkbox
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   **Before:** Auto-refresh showed toast notifications on each refresh cycle, charts flickered with loading spinners, no status indicator, no pause control.
   
   **After:**
   - Clean status indicator dot with informative tooltip showing last refresh time, interval, and status
   - Pause/play controls for manual control of auto-refresh
   - Smooth refresh without flickering or loading spinners during auto-refresh cycles
   - Optional tab visibility pause with immediate refresh on return
   
   <img width="681" height="194" alt="Screenshot 2026-01-26 at 20 56 03" src="https://github.com/user-attachments/assets/54d5ce80-1c5f-49db-a66c-9c64e30e31c2" />
   
   <img width="279" height="132" alt="Screenshot 2026-01-26 at 20 56 18" src="https://github.com/user-attachments/assets/29126fb3-2941-43e0-8c35-24aca701d235" />
   
   <img width="394" height="539" alt="Screenshot 2026-01-26 at 21 18 30" src="https://github.com/user-attachments/assets/ff1a8924-645c-417e-8b6e-133e80e35830" />
   
   
   ### TESTING INSTRUCTIONS
   
   1. **Basic Auto-Refresh Status:**
      - Open a dashboard and set an auto-refresh interval (e.g., 10 seconds)
      - Observe the status indicator dot appears next to the refresh controls
      - Watch the dot turn blue during fetch, then green on success
      - Hover over the indicator to see tooltip with last refresh time and interval
   
   2. **Pause/Resume:**
      - With auto-refresh enabled, click the pause button
      - Observe the dot turns white (paused state)
      - Click play to resume - charts should refresh immediately
      - Verify the timer restarts after resuming
   
   3. **Tab Visibility Pause:**
      - Enable the "Pause on inactive tab" option in refresh settings
      - Switch to another browser tab for longer than the refresh interval
      - Return to the dashboard tab
      - Verify data refreshes immediately and timer restarts
   
   4. **Interval Change While Refreshing:**
      - Set a short refresh interval (e.g., 10 seconds)
      - During an active refresh cycle, open settings and change to 30 seconds
      - Verify the dashboard now refreshes at 30-second intervals (not the old 10s)
   
   5. **Flickering Prevention:**
      - Set a short refresh interval (e.g., 10 seconds)
      - Observe that charts don't show loading spinners during auto-refresh
      - Verify filter badges remain visible during refresh
      - Check that chart legends don't flicker or reorder
   
   6. **Error Handling:**
      - Simulate network issues or chart errors
      - Verify the status dot turns yellow on first error, red on 2+ errors
      - Verify tooltip shows error information and count
   
   7. **Lazy-Loaded Tabs:**
      - Create a dashboard with multiple tabs
      - Enable auto-refresh and switch between tabs
      - Verify lazy-loaded tabs refresh correctly without triggering full dashboard refresh state
   
   ### ADDITIONAL INFORMATION
   
   - [x] Has associated issue:
   - [ ] Required feature flags:
   - [x] Changes UI
   - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [x] Introduces new feature or API
   - [ ] Removes existing feature or API
   
   #### Test Coverage
   - Unit tests for all new hooks (`useRealTimeDashboard`, `useTabVisibility`, `useAutoRefreshTabPause`, `useCurrentTime`)
   - Unit tests for auto-refresh reducer actions and state transitions
   - Unit tests for UI components (`AutoRefreshStatus`, `StatusIndicatorDot`, `StatusTooltipContent`, `AutoRefreshControls`)
   - Unit tests for `RefreshFrequencySelect` validation functions
   - Integration tests for FiltersBadge auto-refresh behavior
   - Updated Header component tests for auto-refresh orchestration
   - Tests for lazy-load tab refresh behavior
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #37459:
URL: https://github.com/apache/superset/pull/37459#issuecomment-3857411181

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "msyavuz (via GitHub)" <gi...@apache.org>.
msyavuz commented on PR #37459:
URL: https://github.com/apache/superset/pull/37459#issuecomment-3870430065

   This heavily ties in with the work from @villebro on async task framework right @richardfogaca ?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "msyavuz (via GitHub)" <gi...@apache.org>.
msyavuz commented on PR #37459:
URL: https://github.com/apache/superset/pull/37459#issuecomment-3876115604

   Or do you think a new implementation using GTF would be overkill for this?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #37459:
URL: https://github.com/apache/superset/pull/37459#issuecomment-3804260012

   🎪 [Showtime](https://github.com/mistercrunch/superset-showtime) is building environment on [GHA](https://github.com/apache/superset/actions/runs/21392903422) for [f6f8a6d](https://github.com/apache/superset/commit/f6f8a6d)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "kgabryje (via GitHub)" <gi...@apache.org>.
kgabryje commented on PR #37459:
URL: https://github.com/apache/superset/pull/37459#issuecomment-3945415935

   While testing, page crashed for me with this error:
   ```
   Uncaught TypeError: Cannot read properties of undefined (reading 'getRawIndex')
       at DataFormatMixin.getDataParams (dataFormat.js:72:29)
       at eval (TooltipView.js:411:33)
       at Array.forEach (<anonymous>)
       at each (util.js:262:13)
       at eval (TooltipView.js:408:71)
       at Array.forEach (<anonymous>)
       at each (util.js:262:13)
       at eval (TooltipView.js:394:69)
       at Array.forEach (<anonymous>)
       at each (util.js:262:13)
   ```
   It only happened once though and I'm not sure how to reproduce it. Claude suggests that it might be connected to `notMerge: false` setting for Echarts?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #37459:
URL: https://github.com/apache/superset/pull/37459#issuecomment-3947942310

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #37459:
URL: https://github.com/apache/superset/pull/37459#issuecomment-3951651343

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "richardfogaca (via GitHub)" <gi...@apache.org>.
richardfogaca closed pull request #37459: feat: auto refresh dashboard
URL: https://github.com/apache/superset/pull/37459


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "richardfogaca (via GitHub)" <gi...@apache.org>.
richardfogaca commented on PR #37459:
URL: https://github.com/apache/superset/pull/37459#issuecomment-3951432503

   You're right @kgabryje thanks for catching that. Here's what I updated on the lastest changes:
   
   1. I removed the leftover isRefreshing passthrough from the ECharts transformProps.ts files, and kept only the Tree chart specific usage since that one is still needed to disable tree animation during refresh.
   
   2. Fixed the periodic refresh path to go through onRefresh(..., skipFiltersRefresh=true) instead of calling fetchCharts directly, so ON_REFRESH/ON_REFRESH_SUCCESS are dispatched consistently and dashboardState.isRefreshing stays in sync for ECharts refresh handling.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "kgabryje (via GitHub)" <gi...@apache.org>.
kgabryje commented on code in PR #37459:
URL: https://github.com/apache/superset/pull/37459#discussion_r2841302033


##########
superset-frontend/src/dashboard/components/AutoRefreshStatus/index.tsx:
##########
@@ -0,0 +1,84 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { FC } from 'react';
+import { Tooltip } from '@superset-ui/core/components';
+import { useRealTimeDashboard } from '../../hooks/useRealTimeDashboard';
+import { useCurrentTime } from '../../hooks/useCurrentTime';
+import { StatusIndicatorDot } from './StatusIndicatorDot';
+import { StatusTooltipContent } from './StatusTooltipContent';
+
+export interface AutoRefreshStatusProps {
+  /** Additional CSS class name */
+  className?: string;
+}
+
+/**
+ * Auto-refresh status indicator displayed in the dashboard header.
+ * Only renders when the dashboard has an auto-refresh interval configured.
+ *
+ * Shows different colored dots based on refresh state:
+ * - Green (success): Last refresh was successful
+ * - Blue (fetching): Currently fetching data
+ * - Yellow (delayed): Refresh is taking longer than expected
+ * - Red (error): Refresh failed with an error
+ * - White (paused): Auto-refresh is paused
+ */
+export const AutoRefreshStatus: FC<AutoRefreshStatusProps> = ({

Review Comment:
   This looks like dead code? I think it's not imported anywhere except for its own test file



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "richardfogaca (via GitHub)" <gi...@apache.org>.
richardfogaca commented on code in PR #37459:
URL: https://github.com/apache/superset/pull/37459#discussion_r2796210917


##########
superset-frontend/src/dashboard/hooks/useTabVisibility.ts:
##########
@@ -0,0 +1,102 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { useEffect, useRef, useCallback, useState } from 'react';
+
+export interface UseTabVisibilityOptions {
+  /** Callback when tab becomes visible */
+  onVisible?: () => void;
+  /** Callback when tab becomes hidden */
+  onHidden?: () => void;
+  /** Whether the hook is enabled */
+  enabled?: boolean;
+}
+
+export interface UseTabVisibilityResult {
+  /** Whether the tab is visible */
+  isVisible: boolean;
+}
+
+/**
+ * Hook to track browser tab visibility state.
+ * Uses the Page Visibility API to detect when the user switches tabs.
+ *
+ * @example
+ * ```tsx
+ * const { isVisible } = useTabVisibility({
+ *   onVisible: () => console.log('Tab is visible'),
+ *   onHidden: () => console.log('Tab is hidden'),
+ * });
+ * ```
+ */
+export function useTabVisibility({
+  onVisible,
+  onHidden,
+  enabled = true,
+}: UseTabVisibilityOptions = {}): UseTabVisibilityResult {
+  const [isVisible, setIsVisible] = useState(
+    () => document.visibilityState === 'visible',
+  );

Review Comment:
   Good question. This is using the standard Page Visibility API (`document.visibilityState` + `visibilitychange`), which tracks page/tab visibility, not document body visibility.
   
   Even though it’s accessed through `document`, the semantics are tab/page-level: `visible` for foreground tab, `hidden` for background/minimized states. So this is the intended browser API for this behavior, not a workaround.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "kgabryje (via GitHub)" <gi...@apache.org>.
kgabryje commented on code in PR #37459:
URL: https://github.com/apache/superset/pull/37459#discussion_r2841348791


##########
superset-frontend/src/dashboard/components/Header/index.tsx:
##########
@@ -139,83 +196,68 @@ const StyledUndoRedoButton = styled(Button)`
   }
 `;
 
-const undoRedoStyle = (theme: {
-  colorIcon: string;
-  colorIconHover: string;
-}) => css`
+const undoRedoStyle = (theme: SupersetTheme) => css`
   color: ${theme.colorIcon};
   &:hover {
     color: ${theme.colorIconHover};
   }
 `;
 
-const undoRedoEmphasized = (theme: { colorIcon: string }) => css`
+const undoRedoEmphasized = (theme: SupersetTheme) => css`
   color: ${theme.colorIcon};
 `;
 
-const undoRedoDisabled = (theme: { colorTextDisabled: string }) => css`
+const undoRedoDisabled = (theme: SupersetTheme) => css`
   color: ${theme.colorTextDisabled};
 `;
 
-const saveBtnStyle = (theme: { sizeUnit: number }) => css`
+const saveBtnStyle = (theme: SupersetTheme) => css`
   min-width: ${theme.sizeUnit * 17}px;
   height: ${theme.sizeUnit * 8}px;
   span > :first-of-type {
     margin-right: 0;
   }
 `;
 
-const discardBtnStyle = (theme: { sizeUnit: number }) => css`
+const discardBtnStyle = (theme: SupersetTheme) => css`
   min-width: ${theme.sizeUnit * 22}px;
   height: ${theme.sizeUnit * 8}px;
 `;
 
-const discardChanges = (): void => {
+const discardChanges = () => {
   const url = new URL(window.location.href);
 
   url.searchParams.delete('edit');
   window.location.assign(url);
 };
 
-interface PropertiesChanges {
-  slug?: string;
-  jsonMetadata?: string;
-  certifiedBy?: string;
-  certificationDetails?: string;
-  owners?: DashboardInfo['owners'];
-  roles?: DashboardInfo['roles'];
-  tags?: DashboardInfo['tags'];
-  themeId?: number | null;
-  css?: string;
-  title?: string;
-}
-
-const Header = (): ReactElement => {
+const Header = (): JSX.Element => {
   const dispatch = useDispatch();
+  const store = useStore<HeaderRootState>();
   const [didNotifyMaxUndoHistoryToast, setDidNotifyMaxUndoHistoryToast] =
-    useState<boolean>(false);
-  const [emphasizeUndo, setEmphasizeUndo] = useState<boolean>(false);
-  const [emphasizeRedo, setEmphasizeRedo] = useState<boolean>(false);
-  const [showingPropertiesModal, setShowingPropertiesModal] =
-    useState<boolean>(false);
-  const [showingRefreshModal, setShowingRefreshModal] =
-    useState<boolean>(false);
-  const [showingEmbedModal, setShowingEmbedModal] = useState<boolean>(false);
-  const [showingReportModal, setShowingReportModal] = useState<boolean>(false);
+    useState(false);
+  const [emphasizeUndo, setEmphasizeUndo] = useState(false);
+  const [emphasizeRedo, setEmphasizeRedo] = useState(false);
+  const [showingPropertiesModal, setShowingPropertiesModal] = useState(false);
+  const [showingRefreshModal, setShowingRefreshModal] = useState(false);
+  const [showingEmbedModal, setShowingEmbedModal] = useState(false);
+  const [showingReportModal, setShowingReportModal] = useState(false);
   const [currentReportDeleting, setCurrentReportDeleting] =
-    useState<ReportObject | null>(null);
-  const dashboardInfo = useSelector((state: RootState) => state.dashboardInfo);
+    useState<AlertObject | null>(null);
+  const dashboardInfo = useSelector(
+    (state: HeaderRootState) => state.dashboardInfo,
+  );
   const layout = useSelector(
-    (state: RootState) => state.dashboardLayout.present,
+    (state: HeaderRootState) => state.dashboardLayout.present,
   );
   const undoLength = useSelector(
-    (state: RootState) => state.dashboardLayout.past.length,
+    (state: HeaderRootState) => state.dashboardLayout.past.length,
   );
   const redoLength = useSelector(
-    (state: RootState) => state.dashboardLayout.future.length,
+    (state: HeaderRootState) => state.dashboardLayout.future.length,
   );
-  const dataMask = useSelector((state: RootState) => state.dataMask);
-  const user = useSelector((state: RootState) => state.user);
+  const dataMask = useSelector((state: HeaderRootState) => state.dataMask);

Review Comment:
   I think datamask is not actually used? Like its passed to useHeaderActionsDropdownMenu, but its not really using datamask



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #37459:
URL: https://github.com/apache/superset/pull/37459#issuecomment-3804344821

   🎪 [Showtime](https://github.com/mistercrunch/superset-showtime) deployed environment on [GHA](https://github.com/apache/superset/actions/runs/21392903422) for [f6f8a6d](https://github.com/apache/superset/commit/f6f8a6d)
   
   • **Environment:** http://35.162.106.105:8080 (admin/admin)
   • **Lifetime:** 48h auto-cleanup
   • **Updates:** New commits create fresh environments automatically


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "kgabryje (via GitHub)" <gi...@apache.org>.
kgabryje commented on code in PR #37459:
URL: https://github.com/apache/superset/pull/37459#discussion_r2841120804


##########
superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx:
##########
@@ -132,6 +132,7 @@ function Echart(
     selectedValues = {},
     refs,
     vizType,

Review Comment:
   Could we just get isRefreshing value from Redux instead of propagating it through props? That would reduce the boilerplate a lot if we didn't need to adjust ~20 charts for that



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "kgabryje (via GitHub)" <gi...@apache.org>.
kgabryje commented on PR #37459:
URL: https://github.com/apache/superset/pull/37459#issuecomment-3949608691

   > great thanks for reviewing this PR @kgabryje , I applied your suggestions
   
   I think we don’t need those transformProps.ts changes anymore right?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "richardfogaca (via GitHub)" <gi...@apache.org>.
richardfogaca commented on code in PR #37459:
URL: https://github.com/apache/superset/pull/37459#discussion_r2796217222


##########
tests/unit_tests/extensions/test_types.py:
##########
@@ -65,9 +65,11 @@ def test_extension_config_full():
             "frontend": {
                 "contributions": {
                     "views": {
-                        "sqllab.panels": [
-                            {"id": "query_insights.main", "name": "Query Insights"}
-                        ]
+                        "sqllab": {
+                            "panels": [
+                                {"id": "query_insights.main", "name": "Query Insights"}
+                            ]
+                        }

Review Comment:
   You're right @msyavuz  thanks for catching it, I'll rebase the branch properly



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "richardfogaca (via GitHub)" <gi...@apache.org>.
richardfogaca commented on PR #37459:
URL: https://github.com/apache/superset/pull/37459#issuecomment-3877136197

   Thanks for the detailed review, @msyavuz!
   
   You raise a good point about GTF—it's definitely powerful for server-side task orchestration. However, auto-refresh here is a client-side feature: we're just re-querying existing dashboards on a timer. GTF's overhead (task scheduling, coordination, state management) is optimized for complex dependencies and workflows, not for repetitive client-initiated queries. That's why we kept it simple with native browser intervals.
   
   That said, your suggestions on code quality are spot-on—we'll tidy up the sort ordering and extract that repeated buildQuery pattern into a helper. Thanks for the thorough review! 🙏


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "richardfogaca (via GitHub)" <gi...@apache.org>.
richardfogaca commented on code in PR #37459:
URL: https://github.com/apache/superset/pull/37459#discussion_r2796214744


##########
superset-frontend/src/dashboard/hooks/useCurrentTime.ts:
##########
@@ -0,0 +1,67 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { useState, useEffect, useRef, useCallback } from 'react';
+
+/**
+ * Hook that provides the current time, updating every second.
+ *
+ * @param enabled - Whether the timer should be running
+ * @param syncTrigger - When this value changes, the timer restarts in phase
+ *                      with the new value. This ensures the display timer is
+ *                      synchronized with refresh cycles.
+ * @returns The current timestamp in milliseconds
+ */
+export const useCurrentTime = (
+  enabled = true,
+  syncTrigger?: number | null,
+): number => {
+  const [currentTime, setCurrentTime] = useState(() => Date.now());
+  const intervalRef = useRef<ReturnType<typeof setInterval> | null>(null);
+
+  const clearExistingInterval = useCallback(() => {
+    if (intervalRef.current !== null) {
+      clearInterval(intervalRef.current);
+      intervalRef.current = null;
+    }
+  }, []);
+
+  // When syncTrigger changes (refresh completes), restart the interval
+  // This keeps the display timer aligned with the refresh cycle
+  useEffect(() => {
+    if (!enabled) {
+      clearExistingInterval();
+      return undefined;
+    }
+
+    if (syncTrigger != null) {
+      setCurrentTime(Date.now());
+    }
+
+    clearExistingInterval();
+    intervalRef.current = setInterval(() => {
+      setCurrentTime(Date.now());
+    }, 1000);
+
+    return clearExistingInterval;
+  }, [enabled, syncTrigger, clearExistingInterval]);
+
+  return currentTime;
+};
+

Review Comment:
   You’re right that the return value is a timestamp number, but the key is that it’s reactive state, not a one-
     off `Date.now()` call.
   
   The hook updates `currentTime` every second (`setInterval`), which triggers re-renders for live relative-time text in the status tooltip (e.g. “updated Xs ago”, “retry in Ys”).
   
   Also, `syncTrigger` is used to restart the ticker when `lastAutoRefreshTime` changes, so the countdown stays aligned with refresh cycles.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] Feat/auto refresh dashboard [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #37459:
URL: https://github.com/apache/superset/pull/37459#issuecomment-3802440664

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] Feat/auto refresh dashboard [superset]

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #37459:
URL: https://github.com/apache/superset/pull/37459#issuecomment-3802528404

   ## [Codecov](https://app.codecov.io/gh/apache/superset/pull/37459?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :white_check_mark: All modified and coverable lines are covered by tests.
   :white_check_mark: Project coverage is 42.29%. Comparing base ([`290bcc1`](https://app.codecov.io/gh/apache/superset/commit/290bcc1dbbbd2824639d8d2a405028c339104a6b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`a65a44f`](https://app.codecov.io/gh/apache/superset/commit/a65a44fc28a9c2d05e1eed2b3f752c8fc6d8417c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   :warning: Report is 3 commits behind head on master.
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #37459       +/-   ##
   ===========================================
   + Coverage        0   42.29%   +42.29%     
   ===========================================
     Files           0      646      +646     
     Lines           0    49352    +49352     
     Branches        0     5495     +5495     
   ===========================================
   + Hits            0    20874    +20874     
   - Misses          0    28170    +28170     
   - Partials        0      308      +308     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/superset/pull/37459/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [presto](https://app.codecov.io/gh/apache/superset/pull/37459/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `41.94% <ø> (?)` | |
   | [python](https://app.codecov.io/gh/apache/superset/pull/37459/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `41.94% <ø> (?)` | |
   | [superset-extensions-cli](https://app.codecov.io/gh/apache/superset/pull/37459/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `96.49% <ø> (?)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   </details>
   
   [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/superset/pull/37459?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #37459:
URL: https://github.com/apache/superset/pull/37459#issuecomment-3936276460

   🎪 [Showtime](https://github.com/mistercrunch/superset-showtime) is building environment on [GHA](https://github.com/apache/superset/actions/runs/22234758150) for [201b34d](https://github.com/apache/superset/commit/201b34d)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #37459:
URL: https://github.com/apache/superset/pull/37459#issuecomment-3802572354

   ### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
   
   
   |  Name | Link |
   |:-:|------------------------|
   |<span aria-hidden="true">🔨</span> Latest commit | d1a454f9a2dd82ebbbff9568de0a9e8bdf95b6e4 |
   |<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/697810789e7ece000811b07f |
   |<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-37459--superset-docs-preview.netlify.app](https://deploy-preview-37459--superset-docs-preview.netlify.app) |
   |<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br />![QR Code](https://app.netlify.com/qr-code/eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJ1cmwiOiJodHRwczovL2RlcGxveS1wcmV2aWV3LTM3NDU5LS1zdXBlcnNldC1kb2NzLXByZXZpZXcubmV0bGlmeS5hcHAifQ.FLvZww9lqGJwX0e9dYfQhF82Jz65vsZoLg8acJjHcBU)<br /><br />_Use your smartphone camera to open QR code link._</details> |
   ---
   <!-- [superset-docs-preview Preview](https://deploy-preview-37459--superset-docs-preview.netlify.app) -->
   _To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "richardfogaca (via GitHub)" <gi...@apache.org>.
richardfogaca commented on code in PR #37459:
URL: https://github.com/apache/superset/pull/37459#discussion_r2790734001


##########
superset-frontend/plugins/plugin-chart-echarts/src/Graph/buildQuery.ts:
##########
@@ -16,15 +16,34 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { buildQueryContext, QueryFormData } from '@superset-ui/core';
+import { buildQueryContext, QueryFormOrderBy } from '@superset-ui/core';
+import { EchartsGraphFormData } from './types';
+
+export default function buildQuery(formData: EchartsGraphFormData) {
+  const { source, target, source_category, target_category, row_limit } =
+    formData;
+  const orderby: QueryFormOrderBy[] = [];
+  const shouldApplyOrderBy =
+    row_limit !== undefined && row_limit !== null && row_limit !== 0;
+
+  [source, target, source_category, target_category].forEach(column => {
+    if (column) {
+      orderby.push([column, true]);
+    }
+  });

Review Comment:
   yes these orderbys are relevant, please read my comment below 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "richardfogaca (via GitHub)" <gi...@apache.org>.
richardfogaca commented on code in PR #37459:
URL: https://github.com/apache/superset/pull/37459#discussion_r2790717914


##########
superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberViz.tsx:
##########
@@ -52,14 +51,11 @@ function BigNumberVis({
   kickerFontSize = PROPORTION.KICKER,
   metricNameFontSize = PROPORTION.METRIC_NAME,
   showMetricName = true,
-  mainColor = BRAND_COLOR,

Review Comment:
   mainColor was unused in the file



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "kgabryje (via GitHub)" <gi...@apache.org>.
kgabryje commented on code in PR #37459:
URL: https://github.com/apache/superset/pull/37459#discussion_r2841312043


##########
superset-frontend/src/dashboard/components/AutoRefreshControls/index.tsx:
##########
@@ -0,0 +1,105 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { FC } from 'react';
+import { css, useTheme, t } from '@apache-superset/core/ui';
+import { Tooltip, Button } from '@superset-ui/core/components';
+import { Icons } from '@superset-ui/core/components/Icons';
+import { useRealTimeDashboard } from '../../hooks/useRealTimeDashboard';
+
+export interface AutoRefreshControlsProps {
+  onTogglePause: () => void;
+  onRefresh?: () => void;
+  isLoading?: boolean;
+  showRefreshButton?: boolean;
+}
+
+/**
+ * Pause/Resume and Refresh buttons for real-time dashboards.
+ * Only renders when auto-refresh is enabled.
+ */
+export const AutoRefreshControls: FC<AutoRefreshControlsProps> = ({

Review Comment:
   This looks like dead code? I think it's not imported anywhere except for its own test file



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "kgabryje (via GitHub)" <gi...@apache.org>.
kgabryje commented on code in PR #37459:
URL: https://github.com/apache/superset/pull/37459#discussion_r2841120804


##########
superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx:
##########
@@ -132,6 +132,7 @@ function Echart(
     selectedValues = {},
     refs,
     vizType,

Review Comment:
   Could we just get isRefreshing value from context instead of propagating it through props? That would reduce the boilerplate a lot if we didn't need to adjust ~20 charts for that



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "richardfogaca (via GitHub)" <gi...@apache.org>.
richardfogaca commented on code in PR #37459:
URL: https://github.com/apache/superset/pull/37459#discussion_r2790717914


##########
superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberViz.tsx:
##########
@@ -52,14 +51,11 @@ function BigNumberVis({
   kickerFontSize = PROPORTION.KICKER,
   metricNameFontSize = PROPORTION.METRIC_NAME,
   showMetricName = true,
-  mainColor = BRAND_COLOR,

Review Comment:
   mainColor, startYAxisAtZero, and timeRangeFixed were unused props that were cleaned up



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #37459:
URL: https://github.com/apache/superset/pull/37459#issuecomment-3881459260

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #37459:
URL: https://github.com/apache/superset/pull/37459#issuecomment-3861664531

   🎪 [Showtime](https://github.com/mistercrunch/superset-showtime) is building environment on [GHA](https://github.com/apache/superset/actions/runs/21759373621) for [c7cbd49](https://github.com/apache/superset/commit/c7cbd49)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "msyavuz (via GitHub)" <gi...@apache.org>.
msyavuz commented on code in PR #37459:
URL: https://github.com/apache/superset/pull/37459#discussion_r2786606344


##########
superset-frontend/src/dashboard/components/FiltersBadge/index.tsx:
##########
@@ -106,10 +107,14 @@ const indicatorsInitialState: Indicator[] = [];
 
 export const FiltersBadge = ({ chartId }: FiltersBadgeProps) => {
   const dispatch = useDispatch();
-  const datasources = useSelector<RootState, any>(state => state.datasources);
-  const dashboardFilters = useSelector<RootState, any>(
-    state => state.dashboardFilters,
+  const isAutoRefreshing = useIsAutoRefreshing();
+  const datasources = useSelector<RootState, RootState['datasources']>(

Review Comment:
   Really nice to see proper types on these!



##########
superset-frontend/packages/superset-ui-core/src/components/Icons/AntdEnhanced.tsx:
##########
@@ -40,6 +40,9 @@ import {
   CaretLeftOutlined,
   CaretRightOutlined,
   CaretRightFilled,
+  PauseOutlined,
+  PauseCircleOutlined,
+  PlayCircleOutlined,

Review Comment:
   Can we add these in alphabetical order?



##########
superset-frontend/plugins/plugin-chart-echarts/src/Tree/buildQuery.ts:
##########
@@ -16,14 +16,36 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { buildQueryContext, QueryFormData } from '@superset-ui/core';
+import { buildQueryContext, QueryFormOrderBy } from '@superset-ui/core';
+import { EchartsTreeFormData } from './types';
+
+export default function buildQuery(formData: EchartsTreeFormData) {
+  const { id, parent, name, row_limit } = formData;
+  const orderby: QueryFormOrderBy[] = [];
+  const shouldApplyOrderBy =
+    row_limit !== undefined && row_limit !== null && row_limit !== 0;
+  const orderbyColumns: string[] = [];
+
+  [parent, id, name].forEach(column => {
+    if (column) {
+      orderbyColumns.push(column);
+    }
+  });
+  orderbyColumns.forEach(column => {
+    orderby.push([column, true]);
+  });

Review Comment:
   Can you expand a bit more on these order by changes?



##########
superset-frontend/plugins/plugin-chart-echarts/src/Treemap/buildQuery.ts:
##########
@@ -16,15 +16,36 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { buildQueryContext, QueryFormData } from '@superset-ui/core';
+import {
+  buildQueryContext,
+  QueryFormData,
+  QueryFormOrderBy,
+} from '@superset-ui/core';
 
 export default function buildQuery(formData: QueryFormData) {
-  const { metric, sort_by_metric } = formData;
+  const { metric, sort_by_metric, groupby = [], row_limit } = formData;
+  const orderby: QueryFormOrderBy[] = [];
+  const shouldApplyOrderBy =
+    row_limit !== undefined && row_limit !== null && row_limit !== 0;
+
+  if (sort_by_metric && metric) {
+    orderby.push([metric, false]);
+  }
+  if (!sort_by_metric && groupby.length > 0) {
+    groupby.forEach(column => {
+      orderby.push([column, true]);
+    });
+  }
+  if (sort_by_metric && groupby.length > 0) {
+    groupby.forEach(column => {
+      orderby.push([column, true]);
+    });
+  }

Review Comment:
   Here too. Not sure about the reason for these but we can probably extract a generic helper for this right?



##########
superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberViz.tsx:
##########
@@ -52,14 +51,11 @@ function BigNumberVis({
   kickerFontSize = PROPORTION.KICKER,
   metricNameFontSize = PROPORTION.METRIC_NAME,
   showMetricName = true,
-  mainColor = BRAND_COLOR,

Review Comment:
   These were unused?



##########
superset-frontend/plugins/plugin-chart-echarts/src/Graph/buildQuery.ts:
##########
@@ -16,15 +16,34 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { buildQueryContext, QueryFormData } from '@superset-ui/core';
+import { buildQueryContext, QueryFormOrderBy } from '@superset-ui/core';
+import { EchartsGraphFormData } from './types';
+
+export default function buildQuery(formData: EchartsGraphFormData) {
+  const { source, target, source_category, target_category, row_limit } =
+    formData;
+  const orderby: QueryFormOrderBy[] = [];
+  const shouldApplyOrderBy =
+    row_limit !== undefined && row_limit !== null && row_limit !== 0;
+
+  [source, target, source_category, target_category].forEach(column => {
+    if (column) {
+      orderby.push([column, true]);
+    }
+  });

Review Comment:
   Are these changes related to the PR?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #37459:
URL: https://github.com/apache/superset/pull/37459#issuecomment-3874661838

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "richardfogaca (via GitHub)" <gi...@apache.org>.
richardfogaca commented on PR #37459:
URL: https://github.com/apache/superset/pull/37459#issuecomment-3947807653

   great thanks for reviewing this PR @kgabryje , I applied your suggestions


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #37459:
URL: https://github.com/apache/superset/pull/37459#issuecomment-3802832755

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "richardfogaca (via GitHub)" <gi...@apache.org>.
richardfogaca commented on PR #37459:
URL: https://github.com/apache/superset/pull/37459#issuecomment-3814682899

   > <img alt="Screenshot 2026-01-28 150341" width="235" height="313" src="https://private-user-images.githubusercontent.com/52219496/541644986-a833bb2a-69fa-482e-9191-f784b4059150.png?jwt=eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Njk2NDc0NzEsIm5iZiI6MTc2OTY0NzE3MSwicGF0aCI6Ii81MjIxOTQ5Ni81NDE2NDQ5ODYtYTgzM2JiMmEtNjlmYS00ODJlLTkxOTEtZjc4NGI0MDU5MTUwLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNjAxMjklMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjYwMTI5VDAwMzkzMVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWZmY2MxYzY4YmYwYWMyZTNlYTVhODNkZGQwODU1YjUzOTY3NzQ0YmMyMmQzYTIxZTc4OTc5YzU4M2NlMzc3NmMmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.LVYZb0yZ53HtA5GojEnO8b4_C7nEsEQg4RKn1hcJRzQ">
   > Should the label “Set auto-refresh” be updated to “Update auto-refresh” when a refresh frequency is already configured?
   
   indeed, good catch @EnxDev thanks


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] Feat/auto refresh dashboard [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #37459:
URL: https://github.com/apache/superset/pull/37459#discussion_r2729821163


##########
superset-frontend/src/dashboard/hooks/useCurrentTime.test.ts:
##########
@@ -0,0 +1,182 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { renderHook, act } from '@testing-library/react-hooks';

Review Comment:
   <div>
   
   
   <div id="suggestion">
   <div id="issue"><b>Deprecated Testing Library</b></div>
   <div id="fix">
   
   The import uses @testing-library/react-hooks, which is deprecated since May 2022. Its renderHook API was merged into @testing-library/react v13+, and the separate package lacks React 18+ support. Migrate to the main library for future compatibility.
   </div>
   
   
   </div>
   
   <details>
   <summary><b>Citations</b></summary>
   <ul>
   
   <li>
   Rule Violated: <a href="https://github.com/apache/superset/blob/196e592/.cursor/rules/dev-standard.mdc#L25">dev-standard.mdc:25</a>
   </li>
   
   <li>
   Rule Violated: <a href="https://github.com/apache/superset/blob/196e592/AGENTS.md#L21">AGENTS.md:21</a>
   </li>
   
   </ul>
   </details>
   
   
   
   
   <small><i>Code Review Run #b4bb6c</i></small>
   </div>
   
   ---
   Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
   - [ ] Yes, avoid them



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #37459:
URL: https://github.com/apache/superset/pull/37459#issuecomment-3881773316

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #37459:
URL: https://github.com/apache/superset/pull/37459#issuecomment-3861531665

   🎪 [Showtime](https://github.com/mistercrunch/superset-showtime) is building environment on [GHA](https://github.com/apache/superset/actions/runs/21758714790) for [c7cbd49](https://github.com/apache/superset/commit/c7cbd49)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #37459:
URL: https://github.com/apache/superset/pull/37459#issuecomment-3892413012

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "richardfogaca (via GitHub)" <gi...@apache.org>.
richardfogaca merged PR #37459:
URL: https://github.com/apache/superset/pull/37459


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "kgabryje (via GitHub)" <gi...@apache.org>.
kgabryje commented on code in PR #37459:
URL: https://github.com/apache/superset/pull/37459#discussion_r2841200201


##########
superset-frontend/src/dashboard/components/Header/index.tsx:
##########
@@ -302,72 +363,249 @@ const Header = (): ReactElement => {
     [dispatch],
   );
 
-  const startPeriodicRender = useCallback(
-    (interval: number) => {
-      let intervalMessage: string | undefined;
-
-      if (interval) {
-        const periodicRefreshOptions =
-          dashboardInfo.common?.conf?.DASHBOARD_AUTO_REFRESH_INTERVALS;
-        const predefinedValue = periodicRefreshOptions.find(
-          (option: [string, string]) => Number(option[0]) === interval / 1000,
-        );
+  const executeRefresh = useCallback(

Review Comment:
   Could we extract all that logic to a separate hook to make this file easier to read and maintain?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #37459:
URL: https://github.com/apache/superset/pull/37459#issuecomment-3952354268

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #37459:
URL: https://github.com/apache/superset/pull/37459#issuecomment-3802665536

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] Feat/auto refresh dashboard [superset]

Posted by "richardfogaca (via GitHub)" <gi...@apache.org>.
richardfogaca commented on PR #37459:
URL: https://github.com/apache/superset/pull/37459#issuecomment-3802463034

   /review


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "richardfogaca (via GitHub)" <gi...@apache.org>.
richardfogaca commented on PR #37459:
URL: https://github.com/apache/superset/pull/37459#issuecomment-3873967811

   hey @msyavuz thank you for raising this.
   
   I reviewed both PRs in detail (#37459 and #36368). There’s no direct dependency between them right now:
   - no overlapping files changed
   - no shared commit ancestry
   - the recent merge conflicts on #37459 are in dashboard/chart files not touched by #36368
   
   so #36368 doesn’t directly affect this PR’s implementation


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "richardfogaca (via GitHub)" <gi...@apache.org>.
richardfogaca commented on code in PR #37459:
URL: https://github.com/apache/superset/pull/37459#discussion_r2790708448


##########
superset-frontend/plugins/plugin-chart-echarts/src/Tree/buildQuery.ts:
##########
@@ -16,14 +16,36 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { buildQueryContext, QueryFormData } from '@superset-ui/core';
+import { buildQueryContext, QueryFormOrderBy } from '@superset-ui/core';
+import { EchartsTreeFormData } from './types';
+
+export default function buildQuery(formData: EchartsTreeFormData) {
+  const { id, parent, name, row_limit } = formData;
+  const orderby: QueryFormOrderBy[] = [];
+  const shouldApplyOrderBy =
+    row_limit !== undefined && row_limit !== null && row_limit !== 0;
+  const orderbyColumns: string[] = [];
+
+  [parent, id, name].forEach(column => {
+    if (column) {
+      orderbyColumns.push(column);
+    }
+  });
+  orderbyColumns.forEach(column => {
+    orderby.push([column, true]);
+  });

Review Comment:
   The orderbys ensure deterministic row ordering so the chart elements maintain stable positions across auto-refreshes



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "richardfogaca (via GitHub)" <gi...@apache.org>.
richardfogaca commented on code in PR #37459:
URL: https://github.com/apache/superset/pull/37459#discussion_r2790708448


##########
superset-frontend/plugins/plugin-chart-echarts/src/Tree/buildQuery.ts:
##########
@@ -16,14 +16,36 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { buildQueryContext, QueryFormData } from '@superset-ui/core';
+import { buildQueryContext, QueryFormOrderBy } from '@superset-ui/core';
+import { EchartsTreeFormData } from './types';
+
+export default function buildQuery(formData: EchartsTreeFormData) {
+  const { id, parent, name, row_limit } = formData;
+  const orderby: QueryFormOrderBy[] = [];
+  const shouldApplyOrderBy =
+    row_limit !== undefined && row_limit !== null && row_limit !== 0;
+  const orderbyColumns: string[] = [];
+
+  [parent, id, name].forEach(column => {
+    if (column) {
+      orderbyColumns.push(column);
+    }
+  });
+  orderbyColumns.forEach(column => {
+    orderby.push([column, true]);
+  });

Review Comment:
   the orderby ensures deterministic row ordering so chart elements maintain stable positions across dashboard auto-refreshes — without explicit ordering, the database can return rows in arbitrary order causing visual jumps



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #37459:
URL: https://github.com/apache/superset/pull/37459#issuecomment-3891924418

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "EnxDev (via GitHub)" <gi...@apache.org>.
EnxDev commented on PR #37459:
URL: https://github.com/apache/superset/pull/37459#issuecomment-3811519530

   <img width="235" height="313" alt="Screenshot 2026-01-28 150341" src="https://github.com/user-attachments/assets/a833bb2a-69fa-482e-9191-f784b4059150" />
   
   Should the label “Set auto-refresh” be updated to “Update auto-refresh” when a refresh frequency is already configured?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] Feat/auto refresh dashboard [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #37459:
URL: https://github.com/apache/superset/pull/37459#issuecomment-3802463373

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #37459:
URL: https://github.com/apache/superset/pull/37459#discussion_r2739268605


##########
superset-frontend/src/dashboard/components/AutoRefreshStatus/StatusIndicatorDot.tsx:
##########
@@ -0,0 +1,168 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { FC, useMemo, useRef, useEffect, useState } from 'react';
+import { css, useTheme } from '@apache-superset/core/ui';
+import { AutoRefreshStatus } from '../../types/autoRefresh';
+
+export interface StatusIndicatorDotProps {
+  /** Current status to display */
+  status: AutoRefreshStatus;
+  /** Size of the dot in pixels */
+  size?: number;
+}
+
+/**
+ * Status indicator configuration mapping.
+ *
+ * - Green dot: Refreshed on schedule
+ * - Blue dot: Fetching data or waiting for first refresh
+ * - Yellow/warning dot: Delayed
+ * - Red dot: Error
+ * - White dot: Paused
+ */
+interface StatusConfig {
+  color: string;
+  needsBorder: boolean;
+}
+
+const getStatusConfig = (
+  theme: ReturnType<typeof useTheme>,
+  status: AutoRefreshStatus,
+): StatusConfig => {
+  switch (status) {
+    case AutoRefreshStatus.Success:
+      return {
+        color: theme.colorSuccess,
+        needsBorder: false,
+      };
+    case AutoRefreshStatus.Idle:
+      return {
+        color: theme.colorInfo,
+        needsBorder: false,
+      };
+    case AutoRefreshStatus.Fetching:
+      return {
+        color: theme.colorInfo,
+        needsBorder: false,
+      };
+    case AutoRefreshStatus.Delayed:
+      return {
+        color: theme.colorWarning,
+        needsBorder: false,
+      };
+    case AutoRefreshStatus.Error:
+      return {
+        color: theme.colorError,
+        needsBorder: false,
+      };
+    case AutoRefreshStatus.Paused:
+      return {
+        color: theme.colorBgContainer,
+        needsBorder: true,
+      };
+    default:
+      return {
+        color: theme.colorTextSecondary,
+        needsBorder: false,
+      };
+  }
+};
+
+/**
+ * A colored dot indicator that shows the auto-refresh status.
+ *
+ * Uses CSS transitions to prevent flickering between states.
+ * The color change is animated smoothly rather than instantly.
+ */
+export const StatusIndicatorDot: FC<StatusIndicatorDotProps> = ({
+  status,
+  size = 10,
+}) => {
+  const theme = useTheme();
+
+  // Debounce rapid status changes to prevent flickering
+  const [displayStatus, setDisplayStatus] = useState(status);
+  const timerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
+
+  useEffect(() => {
+    // Clear any pending timer
+    if (timerRef.current) {
+      clearTimeout(timerRef.current);
+    }
+
+    // For fetching state, update immediately to show user something is happening
+    if (status === AutoRefreshStatus.Fetching) {
+      setDisplayStatus(status);
+    } else {
+      // For other states, debounce to prevent flickering
+      timerRef.current = setTimeout(() => {
+        setDisplayStatus(status);
+      }, 100);
+    }
+
+    return () => {
+      if (timerRef.current) {
+        clearTimeout(timerRef.current);
+      }
+    };
+  }, [status]);
+
+  const statusConfig = useMemo(
+    () => getStatusConfig(theme, displayStatus),
+    [theme, displayStatus],
+  );
+
+  const dotStyles = useMemo(
+    () => css`
+      display: inline-flex;
+      align-items: center;
+      justify-content: center;
+      width: ${size}px;
+      height: ${size}px;
+      border-radius: 50%;
+      background-color: ${statusConfig.color};
+      transition:
+        background-color ${theme.motionDurationMid} ease-in-out,
+        border-color ${theme.motionDurationMid} ease-in-out;
+      border: ${statusConfig.needsBorder
+        ? `1px solid ${theme.colorBorder}`
+        : 'none'};
+      box-shadow: ${statusConfig.needsBorder
+        ? 'none'
+        : `0 0 0 2px ${theme.colorBgContainer}`};
+      margin-left: ${theme.marginXS}px;
+      margin-right: ${theme.marginXS}px;
+      cursor: help;
+    `,
+    [statusConfig, size, theme],
+  );
+
+  return (
+    <span
+      css={dotStyles}
+      role="status"
+      aria-label={`Auto-refresh status: ${displayStatus}`}
+      data-test="status-indicator-dot"

Review Comment:
   **Suggestion:** `aria-label` and `data-status` interpolate `displayStatus` directly; if `AutoRefreshStatus` is a numeric enum this will produce numbers instead of readable labels for accessibility and debugging — map statuses to human-readable strings and use that label in `aria-label` and `data-status`. [possible bug]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ❌ Screen readers announce numeric enum values.
   - ⚠️ DOM data-status shows unreadable numeric values.
   ```
   </details>
   
   ```suggestion
     const statusLabel = useMemo(() => {
       const labels: Record<AutoRefreshStatus, string> = {
         [AutoRefreshStatus.Success]: 'Success',
         [AutoRefreshStatus.Idle]: 'Idle',
         [AutoRefreshStatus.Fetching]: 'Fetching',
         [AutoRefreshStatus.Delayed]: 'Delayed',
         [AutoRefreshStatus.Error]: 'Error',
         [AutoRefreshStatus.Paused]: 'Paused',
       };
       return labels[displayStatus] ?? String(displayStatus);
     }, [displayStatus]);
   
     return (
       <span
         css={dotStyles}
         role="status"
         aria-label={`Auto-refresh status: ${statusLabel}`}
         data-test="status-indicator-dot"
         data-status={statusLabel}
   ```
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Open
   superset-frontend/src/dashboard/components/AutoRefreshStatus/StatusIndicatorDot.tsx and
   locate the render block at lines 156-164 where `aria-label` and `data-status` interpolate
   `displayStatus`.
   
   2. Start the frontend and open a dashboard with auto-refresh enabled (the PR adds this
   indicator to the dashboard header). Trigger different statuses by causing fetches to run
   and fail/succeed (the UI sets AutoRefreshStatus values based on refresh outcomes).
   
   3. Inspect the DOM for the status indicator element (`data-test="status-indicator-dot"`)
   and observe the `aria-label` and `data-status` attributes. If `AutoRefreshStatus` in
   types/autoRefresh is a numeric enum, these attributes will show numbers (e.g.,
   "Auto-refresh status: 2") rather than readable text.
   
   4. Result: screen readers and DOM debugging tools see numeric values instead of
   human-friendly labels. Mapping enum values to strings (as proposed) produces accessible,
   debuggable labels.
   ```
   </details>
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset-frontend/src/dashboard/components/AutoRefreshStatus/StatusIndicatorDot.tsx
   **Line:** 156:161
   **Comment:**
   	*Possible Bug: `aria-label` and `data-status` interpolate `displayStatus` directly; if `AutoRefreshStatus` is a numeric enum this will produce numbers instead of readable labels for accessibility and debugging — map statuses to human-readable strings and use that label in `aria-label` and `data-status`.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
   ```
   </details>



##########
superset-frontend/src/dashboard/components/AutoRefreshStatus/StatusIndicatorDot.test.tsx:
##########
@@ -0,0 +1,113 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { render, screen, act } from 'spec/helpers/testing-library';
+import { StatusIndicatorDot } from './StatusIndicatorDot';
+import { AutoRefreshStatus } from '../../types/autoRefresh';
+
+test('renders with success status', () => {
+  render(<StatusIndicatorDot status={AutoRefreshStatus.Success} />);
+  const dot = screen.getByTestId('status-indicator-dot');
+  expect(dot).toBeInTheDocument();
+  expect(dot).toHaveAttribute('data-status', AutoRefreshStatus.Success);
+});
+
+test('renders with fetching status', () => {
+  render(<StatusIndicatorDot status={AutoRefreshStatus.Fetching} />);
+  const dot = screen.getByTestId('status-indicator-dot');
+  expect(dot).toHaveAttribute('data-status', AutoRefreshStatus.Fetching);
+});
+
+test('renders with delayed status', () => {
+  render(<StatusIndicatorDot status={AutoRefreshStatus.Delayed} />);
+  const dot = screen.getByTestId('status-indicator-dot');
+  expect(dot).toHaveAttribute('data-status', AutoRefreshStatus.Delayed);
+});
+
+test('renders with idle status', () => {
+  render(<StatusIndicatorDot status={AutoRefreshStatus.Idle} />);
+  const dot = screen.getByTestId('status-indicator-dot');
+  expect(dot).toHaveAttribute('data-status', AutoRefreshStatus.Idle);
+});
+
+test('renders with error status', () => {
+  render(<StatusIndicatorDot status={AutoRefreshStatus.Error} />);
+  const dot = screen.getByTestId('status-indicator-dot');
+  expect(dot).toHaveAttribute('data-status', AutoRefreshStatus.Error);
+});
+
+test('renders with paused status', () => {
+  render(<StatusIndicatorDot status={AutoRefreshStatus.Paused} />);
+  const dot = screen.getByTestId('status-indicator-dot');
+  expect(dot).toHaveAttribute('data-status', AutoRefreshStatus.Paused);
+});
+
+test('has correct accessibility attributes', () => {
+  render(<StatusIndicatorDot status={AutoRefreshStatus.Success} />);
+  const dot = screen.getByTestId('status-indicator-dot');
+  expect(dot).toHaveAttribute('role', 'status');
+  expect(dot).toHaveAttribute('aria-label', 'Auto-refresh status: success');
+});
+
+test('fetching status updates immediately without debounce', async () => {
+  jest.useFakeTimers();
+
+  const { rerender } = render(
+    <StatusIndicatorDot status={AutoRefreshStatus.Success} />,
+  );
+
+  // Change to fetching - should be immediate
+  rerender(<StatusIndicatorDot status={AutoRefreshStatus.Fetching} />);
+
+  const dot = screen.getByTestId('status-indicator-dot');
+  expect(dot).toHaveAttribute('data-status', AutoRefreshStatus.Fetching);
+
+  jest.useRealTimers();
+});
+
+test('debounces non-fetching status changes to prevent flickering', async () => {
+  jest.useFakeTimers();
+
+  const { rerender } = render(
+    <StatusIndicatorDot status={AutoRefreshStatus.Success} />,
+  );
+
+  // Change to error - should be debounced
+  rerender(<StatusIndicatorDot status={AutoRefreshStatus.Error} />);
+
+  // Status should still show success (debounced)
+  let dot = screen.getByTestId('status-indicator-dot');
+  expect(dot).toHaveAttribute('data-status', AutoRefreshStatus.Success);
+
+  // Fast forward past debounce time (100ms)
+  act(() => {
+    jest.advanceTimersByTime(150);
+  });
+
+  // Now should be error
+  dot = screen.getByTestId('status-indicator-dot');
+  expect(dot).toHaveAttribute('data-status', AutoRefreshStatus.Error);
+
+  jest.useRealTimers();

Review Comment:
   **Suggestion:** Another fake-timer test also calls `jest.useRealTimers()` only at the end; if an assertion throws the timers aren't restored. Wrap the test body in try/finally and restore timers in finally. Also remove the unnecessary `async` declaration since the test doesn't await anything. [possible bug]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ❌ AutoRefreshStatus debounce test becomes flaky.
   - ❌ Neighboring timer tests can fail unexpectedly.
   - ⚠️ Test authoring may hide real failures.
   ```
   </details>
   
   ```suggestion
   test('debounces non-fetching status changes to prevent flickering', () => {
     jest.useFakeTimers();
     try {
       const { rerender } = render(
         <StatusIndicatorDot status={AutoRefreshStatus.Success} />,
       );
   
       // Change to error - should be debounced
       rerender(<StatusIndicatorDot status={AutoRefreshStatus.Error} />);
   
       // Status should still show success (debounced)
       let dot = screen.getByTestId('status-indicator-dot');
       expect(dot).toHaveAttribute('data-status', AutoRefreshStatus.Success);
   
       // Fast forward past debounce time (100ms)
       act(() => {
         jest.advanceTimersByTime(150);
       });
   
       // Now should be error
       dot = screen.getByTestId('status-indicator-dot');
       expect(dot).toHaveAttribute('data-status', AutoRefreshStatus.Error);
     } finally {
       jest.useRealTimers();
     }
   ```
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Open
   superset-frontend/src/dashboard/components/AutoRefreshStatus/StatusIndicatorDot.test.tsx
   and locate the debounce test starting at line 83 which calls jest.useFakeTimers() (line
   84) and restores real timers only at line 106.
   
   2. Run the test file with Jest (e.g. yarn test path/to/StatusIndicatorDot.test.tsx). The
   test renders the component (lines 86-89), rerenders to Error (line 91), asserts the
   intermediate state (lines 94-95), advances fake timers (lines 98-101), then asserts the
   final state (lines 103-104).
   
   3. If any assertion (for example the initial expectation at line 95 or final expectation
   at line 104) throws, execution exits before reaching jest.useRealTimers() at line 106,
   leaving fake timers active in the Jest process.
   
   4. Observe other tests running after this file failing or behaving differently due to fake
   timers still active (setTimeout/setInterval controlled by Jest fake timers), reproducing
   the leak.
   ```
   </details>
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset-frontend/src/dashboard/components/AutoRefreshStatus/StatusIndicatorDot.test.tsx
   **Line:** 83:106
   **Comment:**
   	*Possible Bug: Another fake-timer test also calls `jest.useRealTimers()` only at the end; if an assertion throws the timers aren't restored. Wrap the test body in try/finally and restore timers in finally. Also remove the unnecessary `async` declaration since the test doesn't await anything.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
   ```
   </details>



##########
superset-frontend/src/dashboard/hooks/useRealTimeDashboard.ts:
##########
@@ -0,0 +1,245 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { useCallback, useMemo } from 'react';
+import { useSelector, useDispatch } from 'react-redux';
+import { AutoRefreshStatus } from '../types/autoRefresh';
+import { DashboardState, RootState } from '../types';
+import {
+  setAutoRefreshStatus,
+  setAutoRefreshPaused,
+  setAutoRefreshPausedByTab,
+  recordAutoRefreshSuccess,
+  recordAutoRefreshError,
+  setAutoRefreshFetchStartTime,
+  setAutoRefreshPauseOnInactiveTab,
+} from '../actions/autoRefresh';
+
+type DashboardStateRoot = {
+  dashboardState: Partial<DashboardState>;
+};
+
+/**
+ * Selector: Determines if this is a "real-time" dashboard.
+ * A dashboard is real-time if it has an auto-refresh frequency > 0.
+ */
+export const selectIsRealTimeDashboard = (state: DashboardStateRoot): boolean =>
+  (state.dashboardState?.refreshFrequency ?? 0) > 0;
+
+/**
+ * Selector: Determines if auto-refresh is manually paused (by user action).
+ * Does NOT include tab visibility pause.
+ */
+export const selectIsManuallyPaused = (state: DashboardStateRoot): boolean =>

Review Comment:
   **Suggestion:** The `selectIsManuallyPaused` selector expects `DashboardStateRoot` while `useSelector` will call it with the full `RootState`; this mismatched parameter type can lead to incorrect type checking and maintenance issues—use `RootState` to reflect the actual shape provided by `useSelector`. [type error]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ⚠️ useSelector calls in hook show type mismatch.
   - ⚠️ IDE highlights selector declaration errors.
   - ❌ CI typecheck can fail on PRs.
   ```
   </details>
   
   ```suggestion
   export const selectIsManuallyPaused = (state: RootState): boolean =>
   ```
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Inspect superset-frontend/src/dashboard/hooks/useRealTimeDashboard.ts: selector
   `selectIsManuallyPaused` is declared at lines 48-49 with parameter `DashboardStateRoot`.
   
   2. The selector is consumed in this same hook at line 112: `const isManuallyPaused =
   useSelector(selectIsManuallyPaused);` where `useSelector` provides the global `RootState`.
   
   3. Run the project's TypeScript type-check (or open in IDE): the selector's parameter type
   (`DashboardStateRoot`) is narrower than the `RootState` provided by `useSelector`, causing
   a type incompatibility reported at the call site (line 112) and/or declaration (lines
   48-49).
   
   4. Updating the selector signature to accept `RootState` (as proposed) resolves the
   mismatch and clears the type-checker/IDE warning.
   ```
   </details>
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset-frontend/src/dashboard/hooks/useRealTimeDashboard.ts
   **Line:** 48:48
   **Comment:**
   	*Type Error: The `selectIsManuallyPaused` selector expects `DashboardStateRoot` while `useSelector` will call it with the full `RootState`; this mismatched parameter type can lead to incorrect type checking and maintenance issues—use `RootState` to reflect the actual shape provided by `useSelector`.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
   ```
   </details>



##########
superset-frontend/src/dashboard/components/FiltersBadge/index.tsx:
##########
@@ -106,10 +107,14 @@ const indicatorsInitialState: Indicator[] = [];
 
 export const FiltersBadge = ({ chartId }: FiltersBadgeProps) => {
   const dispatch = useDispatch();
-  const datasources = useSelector<RootState, any>(state => state.datasources);
-  const dashboardFilters = useSelector<RootState, any>(
-    state => state.dashboardFilters,
+  const isAutoRefreshing = useIsAutoRefreshing();
+  const datasources = useSelector<RootState, RootState['datasources']>(
+    state => state.datasources,
   );
+  const dashboardFilters = useSelector<
+    RootState,
+    RootState['dashboardFilters']
+  >(state => state.dashboardFilters);

Review Comment:
   **Suggestion:** `useSelector` is selecting entire `datasources` and `dashboardFilters` objects which will change reference often and trigger unnecessary recomputations; pass `shallowEqual` as the equality function to `useSelector` (or select only the minimal fields needed) to avoid frequent re-renders and expensive recalculations. [performance]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ⚠️ Extra re-renders during dashboard state updates.
   - ❌ Slower dashboard filter indicator recalculations.
   ```
   </details>
   
   ```suggestion
       shallowEqual,
     );
     const dashboardFilters = useSelector<
       RootState,
       RootState['dashboardFilters']
     >(state => state.dashboardFilters,
     shallowEqual);
   ```
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Open a dashboard which mounts FiltersBadge (file:
   superset-frontend/src/dashboard/components/FiltersBadge/index.tsx). FiltersBadge selects
   large slices datasources and dashboardFilters at lines ~111-117.
   
   2. Trigger frequent dashboard state updates (for example auto-refresh / chart state
   updates). FiltersBadge's effect depends on datasources (effect dependency list includes
   datasources near the setDashboardIndicators effect at index.tsx:168-176).
   
   3. Because useSelector currently returns the entire datasources and dashboardFilters
   objects without shallowEqual, any update that replaces those objects by reference (even
   when contents unchanged) will cause FiltersBadge to re-run effects and recompute
   indicators via selectIndicatorsForChart at the same effect (setDashboardIndicators is
   called there).
   
   4. Observe increased recalculation: selectIndicatorsForChart is invoked more often
   (index.tsx where setDashboardIndicators is called), causing extra CPU work and re-renders
   of the FiltersBadge UI during normal dashboard refresh cycles.
   ```
   </details>
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset-frontend/src/dashboard/components/FiltersBadge/index.tsx
   **Line:** 113:117
   **Comment:**
   	*Performance: `useSelector` is selecting entire `datasources` and `dashboardFilters` objects which will change reference often and trigger unnecessary recomputations; pass `shallowEqual` as the equality function to `useSelector` (or select only the minimal fields needed) to avoid frequent re-renders and expensive recalculations.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
   ```
   </details>



##########
superset-frontend/src/dashboard/hooks/useRealTimeDashboard.ts:
##########
@@ -0,0 +1,245 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { useCallback, useMemo } from 'react';
+import { useSelector, useDispatch } from 'react-redux';
+import { AutoRefreshStatus } from '../types/autoRefresh';
+import { DashboardState, RootState } from '../types';
+import {
+  setAutoRefreshStatus,
+  setAutoRefreshPaused,
+  setAutoRefreshPausedByTab,
+  recordAutoRefreshSuccess,
+  recordAutoRefreshError,
+  setAutoRefreshFetchStartTime,
+  setAutoRefreshPauseOnInactiveTab,
+} from '../actions/autoRefresh';
+
+type DashboardStateRoot = {
+  dashboardState: Partial<DashboardState>;
+};
+
+/**
+ * Selector: Determines if this is a "real-time" dashboard.
+ * A dashboard is real-time if it has an auto-refresh frequency > 0.
+ */
+export const selectIsRealTimeDashboard = (state: DashboardStateRoot): boolean =>
+  (state.dashboardState?.refreshFrequency ?? 0) > 0;
+
+/**
+ * Selector: Determines if auto-refresh is manually paused (by user action).
+ * Does NOT include tab visibility pause.
+ */
+export const selectIsManuallyPaused = (state: DashboardStateRoot): boolean =>
+  state.dashboardState?.autoRefreshPaused === true;
+
+/**
+ * Selector: Determines if auto-refresh is paused.
+ * Paused can be due to manual pause or tab visibility.
+ */
+export const selectIsPaused = (state: DashboardStateRoot): boolean =>

Review Comment:
   **Suggestion:** The `selectIsPaused` selector is declared to accept `DashboardStateRoot` but is invoked via `useSelector` with the full `RootState`; change the parameter to `RootState` to ensure correct typing and prevent incorrect assumptions about state shape. [type error]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ⚠️ useSelector type mismatch in dashboard hook.
   - ⚠️ Developer workflow interrupted by IDE warnings.
   - ❌ Potential CI pipeline typecheck failure.
   ```
   </details>
   
   ```suggestion
   export const selectIsPaused = (state: RootState): boolean =>
   ```
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Open superset-frontend/src/dashboard/hooks/useRealTimeDashboard.ts and find
   `selectIsPaused` at lines 55-57.
   
   2. Within the same file the selector is invoked via `useSelector` at line 113: `const
   isPaused = useSelector(selectIsPaused);` which supplies the full `RootState`.
   
   3. Execute TypeScript checking (tsc) or view the file in an editor: the selector signature
   expecting `DashboardStateRoot` conflicts with `RootState` passed by `useSelector`,
   generating a type error at the call site (line 113) or the selector declaration (lines
   55-57).
   
   4. Change the selector parameter to `RootState` as suggested to align types and eliminate
   the type-checker errors.
   ```
   </details>
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset-frontend/src/dashboard/hooks/useRealTimeDashboard.ts
   **Line:** 55:55
   **Comment:**
   	*Type Error: The `selectIsPaused` selector is declared to accept `DashboardStateRoot` but is invoked via `useSelector` with the full `RootState`; change the parameter to `RootState` to ensure correct typing and prevent incorrect assumptions about state shape.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
   ```
   </details>



##########
superset-frontend/src/dashboard/components/AutoRefreshStatus/StatusIndicatorDot.test.tsx:
##########
@@ -0,0 +1,113 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { render, screen, act } from 'spec/helpers/testing-library';
+import { StatusIndicatorDot } from './StatusIndicatorDot';
+import { AutoRefreshStatus } from '../../types/autoRefresh';
+
+test('renders with success status', () => {
+  render(<StatusIndicatorDot status={AutoRefreshStatus.Success} />);
+  const dot = screen.getByTestId('status-indicator-dot');
+  expect(dot).toBeInTheDocument();
+  expect(dot).toHaveAttribute('data-status', AutoRefreshStatus.Success);
+});
+
+test('renders with fetching status', () => {
+  render(<StatusIndicatorDot status={AutoRefreshStatus.Fetching} />);
+  const dot = screen.getByTestId('status-indicator-dot');
+  expect(dot).toHaveAttribute('data-status', AutoRefreshStatus.Fetching);
+});
+
+test('renders with delayed status', () => {
+  render(<StatusIndicatorDot status={AutoRefreshStatus.Delayed} />);
+  const dot = screen.getByTestId('status-indicator-dot');
+  expect(dot).toHaveAttribute('data-status', AutoRefreshStatus.Delayed);
+});
+
+test('renders with idle status', () => {
+  render(<StatusIndicatorDot status={AutoRefreshStatus.Idle} />);
+  const dot = screen.getByTestId('status-indicator-dot');
+  expect(dot).toHaveAttribute('data-status', AutoRefreshStatus.Idle);
+});
+
+test('renders with error status', () => {
+  render(<StatusIndicatorDot status={AutoRefreshStatus.Error} />);
+  const dot = screen.getByTestId('status-indicator-dot');
+  expect(dot).toHaveAttribute('data-status', AutoRefreshStatus.Error);
+});
+
+test('renders with paused status', () => {
+  render(<StatusIndicatorDot status={AutoRefreshStatus.Paused} />);
+  const dot = screen.getByTestId('status-indicator-dot');
+  expect(dot).toHaveAttribute('data-status', AutoRefreshStatus.Paused);
+});
+
+test('has correct accessibility attributes', () => {
+  render(<StatusIndicatorDot status={AutoRefreshStatus.Success} />);
+  const dot = screen.getByTestId('status-indicator-dot');
+  expect(dot).toHaveAttribute('role', 'status');
+  expect(dot).toHaveAttribute('aria-label', 'Auto-refresh status: success');
+});
+
+test('fetching status updates immediately without debounce', async () => {
+  jest.useFakeTimers();
+
+  const { rerender } = render(
+    <StatusIndicatorDot status={AutoRefreshStatus.Success} />,
+  );
+
+  // Change to fetching - should be immediate
+  rerender(<StatusIndicatorDot status={AutoRefreshStatus.Fetching} />);
+
+  const dot = screen.getByTestId('status-indicator-dot');
+  expect(dot).toHaveAttribute('data-status', AutoRefreshStatus.Fetching);
+
+  jest.useRealTimers();

Review Comment:
   **Suggestion:** Tests use fake timers but call `jest.useRealTimers()` only at the end of the test body; if the test throws before that call the real timers won't be restored and can leak to other tests. Wrap fake-timer tests in try/finally and ensure timers are always restored. [possible bug]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ❌ AutoRefreshStatus unit tests become flaky.
   - ❌ Timer-dependent tests fail unpredictably.
   - ⚠️ CI test runs may intermittently fail.
   ```
   </details>
   
   ```suggestion
   test('fetching status updates immediately without debounce', () => {
     jest.useFakeTimers();
     try {
       const { rerender } = render(
         <StatusIndicatorDot status={AutoRefreshStatus.Success} />,
       );
   
       // Change to fetching - should be immediate
       rerender(<StatusIndicatorDot status={AutoRefreshStatus.Fetching} />);
   
       const dot = screen.getByTestId('status-indicator-dot');
       expect(dot).toHaveAttribute('data-status', AutoRefreshStatus.Fetching);
     } finally {
       jest.useRealTimers();
     }
   ```
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Open file
   superset-frontend/src/dashboard/components/AutoRefreshStatus/StatusIndicatorDot.test.tsx
   and locate the test beginning at line 67 which calls jest.useFakeTimers() (line 67) and
   only calls jest.useRealTimers() at line 80.
   
   2. Run the specific test file with Jest (e.g. yarn test
   superset-frontend/src/dashboard/components/AutoRefreshStatus/StatusIndicatorDot.test.tsx).
   The test executes the code path at lines 70-76: render, rerender to Fetching (lines
   71-75), then calls screen.getByTestId() at line 77 and an assertion at line 78.
   
   3. If the assertion at line 78 throws (for example the element is not found or the
   attribute differs), the test will exit before reaching jest.useRealTimers() at line 80,
   leaving Jest running with fake timers enabled.
   
   4. Observe subsequent tests in the same Jest process (other files or later tests)
   misbehave: timer-based APIs (setTimeout, setInterval) will be controlled by fake timers
   causing unexpected timing and flaky failures across the suite.
   ```
   </details>
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset-frontend/src/dashboard/components/AutoRefreshStatus/StatusIndicatorDot.test.tsx
   **Line:** 67:80
   **Comment:**
   	*Possible Bug: Tests use fake timers but call `jest.useRealTimers()` only at the end of the test body; if the test throws before that call the real timers won't be restored and can leak to other tests. Wrap fake-timer tests in try/finally and ensure timers are always restored.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
   ```
   </details>



##########
superset-frontend/src/dashboard/hooks/useRealTimeDashboard.ts:
##########
@@ -0,0 +1,245 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { useCallback, useMemo } from 'react';
+import { useSelector, useDispatch } from 'react-redux';
+import { AutoRefreshStatus } from '../types/autoRefresh';
+import { DashboardState, RootState } from '../types';
+import {
+  setAutoRefreshStatus,
+  setAutoRefreshPaused,
+  setAutoRefreshPausedByTab,
+  recordAutoRefreshSuccess,
+  recordAutoRefreshError,
+  setAutoRefreshFetchStartTime,
+  setAutoRefreshPauseOnInactiveTab,
+} from '../actions/autoRefresh';
+
+type DashboardStateRoot = {
+  dashboardState: Partial<DashboardState>;
+};
+
+/**
+ * Selector: Determines if this is a "real-time" dashboard.
+ * A dashboard is real-time if it has an auto-refresh frequency > 0.
+ */
+export const selectIsRealTimeDashboard = (state: DashboardStateRoot): boolean =>

Review Comment:
   **Suggestion:** The selector is typed to accept `DashboardStateRoot` but is used with `useSelector`, which supplies the full `RootState`; this causes a TypeScript typing mismatch and can hide bugs or cause incorrect assumptions when the actual root state shape differs. Change the selector parameter type to `RootState` to match `useSelector` usage. [type error]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ⚠️ TypeScript errors in useRealTimeDashboard hook.
   - ⚠️ Editor/IDE reports selector type mismatch.
   - ❌ CI typecheck may fail blocking merges.
   ```
   </details>
   
   ```suggestion
   export const selectIsRealTimeDashboard = (state: RootState): boolean =>
   ```
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Open file superset-frontend/src/dashboard/hooks/useRealTimeDashboard.ts and view the
   selector declaration at lines 41-42 (`selectIsRealTimeDashboard`).
   
   2. In the same file, observe the call site at line 111: `const isRealTimeDashboard =
   useSelector(selectIsRealTimeDashboard);` — `useSelector` passes the app RootState (see
   lines 111-114 where other selectors are also passed to `useSelector`).
   
   3. Run TypeScript type-check (e.g., npm run typecheck or tsc) or view in IDE: the selector
   signature expecting `DashboardStateRoot` is incompatible with the `RootState` provided by
   `useSelector`, producing a type error at the call site
   (`useSelector(selectIsRealTimeDashboard)`).
   
   4. Result: editor/CI reports a type-mismatch warning/error pointing to the selector
   declaration (line 41-42) and the call site (line 111). Changing the selector parameter to
   `RootState` (as proposed) aligns the types and removes the type-check error.
   ```
   </details>
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset-frontend/src/dashboard/hooks/useRealTimeDashboard.ts
   **Line:** 41:41
   **Comment:**
   	*Type Error: The selector is typed to accept `DashboardStateRoot` but is used with `useSelector`, which supplies the full `RootState`; this causes a TypeScript typing mismatch and can hide bugs or cause incorrect assumptions when the actual root state shape differs. Change the selector parameter type to `RootState` to match `useSelector` usage.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
   ```
   </details>



##########
superset-frontend/src/dashboard/hooks/useRealTimeDashboard.ts:
##########
@@ -0,0 +1,245 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { useCallback, useMemo } from 'react';
+import { useSelector, useDispatch } from 'react-redux';
+import { AutoRefreshStatus } from '../types/autoRefresh';
+import { DashboardState, RootState } from '../types';
+import {
+  setAutoRefreshStatus,
+  setAutoRefreshPaused,
+  setAutoRefreshPausedByTab,
+  recordAutoRefreshSuccess,
+  recordAutoRefreshError,
+  setAutoRefreshFetchStartTime,
+  setAutoRefreshPauseOnInactiveTab,
+} from '../actions/autoRefresh';
+
+type DashboardStateRoot = {
+  dashboardState: Partial<DashboardState>;
+};
+
+/**
+ * Selector: Determines if this is a "real-time" dashboard.
+ * A dashboard is real-time if it has an auto-refresh frequency > 0.
+ */
+export const selectIsRealTimeDashboard = (state: DashboardStateRoot): boolean =>
+  (state.dashboardState?.refreshFrequency ?? 0) > 0;
+
+/**
+ * Selector: Determines if auto-refresh is manually paused (by user action).
+ * Does NOT include tab visibility pause.
+ */
+export const selectIsManuallyPaused = (state: DashboardStateRoot): boolean =>
+  state.dashboardState?.autoRefreshPaused === true;
+
+/**
+ * Selector: Determines if auto-refresh is paused.
+ * Paused can be due to manual pause or tab visibility.
+ */
+export const selectIsPaused = (state: DashboardStateRoot): boolean =>
+  state.dashboardState?.autoRefreshPaused === true ||
+  state.dashboardState?.autoRefreshPausedByTab === true;
+
+/**
+ * Selector: Computes the effective refresh status for the indicator.
+ *
+ * Priority order:
+ * 1. If not a real-time dashboard → Idle
+ * 2. If paused (manually or by tab) → Paused
+ * 3. If fetching → Fetching
+ * 4. If refreshErrorCount >= 2 → Error
+ * 5. If refreshErrorCount === 1 → Delayed
+ * 6. Otherwise → Current status from state
+ */
+export const selectEffectiveRefreshStatus = (
+  state: DashboardStateRoot,

Review Comment:
   **Suggestion:** `selectEffectiveRefreshStatus` is declared to accept `DashboardStateRoot` but is used with `useSelector`, which passes the full `RootState`; this type mismatch can hide incorrect assumptions about the root state structure—update the selector signature to `RootState`. [type error]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ⚠️ Type-check errors for selectEffectiveRefreshStatus.
   - ⚠️ IDE highlights mismatch at useSelector call (line 114).
   - ❌ Failing CI typecheck blocks merges.
   ```
   </details>
   
   ```suggestion
     state: RootState,
   ```
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Open superset-frontend/src/dashboard/hooks/useRealTimeDashboard.ts and locate
   `selectEffectiveRefreshStatus` declared at lines 70-105.
   
   2. Confirm the selector is consumed in this file at line 114: `const effectiveStatus =
   useSelector(selectEffectiveRefreshStatus);` where `useSelector` provides the global
   `RootState`.
   
   3. Run TypeScript type-check (`npm run typecheck`/tsc) or check in IDE: the selector
   signature expecting `DashboardStateRoot` is not the same type `useSelector` supplies
   (`RootState`), causing a type error flagged at the call site (line 114) and/or declaration
   (lines 70-105).
   
   4. Update the selector signature to accept `RootState` (as proposed) to align types and
   clear type-checker/CI/IDE errors.
   ```
   </details>
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset-frontend/src/dashboard/hooks/useRealTimeDashboard.ts
   **Line:** 71:71
   **Comment:**
   	*Type Error: `selectEffectiveRefreshStatus` is declared to accept `DashboardStateRoot` but is used with `useSelector`, which passes the full `RootState`; this type mismatch can hide incorrect assumptions about the root state structure—update the selector signature to `RootState`.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
   ```
   </details>



##########
superset-frontend/src/dashboard/components/Header/useHeaderActionsDropdownMenu.tsx:
##########
@@ -329,5 +339,19 @@ export const useHeaderActionsMenu = ({
     userCanShare,
   ]);
 
-  return [menu, isDropdownVisible, setIsDropdownVisible];
+  if (!menuRef.current) {
+    menuRef.current = menu;
+  }
+
+  useEffect(() => {
+    if (!isDropdownVisible) {
+      menuRef.current = menu;
+    }
+  }, [isDropdownVisible, menu]);
+
+  return [
+    isDropdownVisible && menuRef.current ? menuRef.current : menu,

Review Comment:
   **Suggestion:** Caching the rendered React element (`menu`) in `menuRef.current` and returning that cached element while the dropdown is visible preserves the element instance (and therefore its props/handlers) across renders; this causes stale event handlers and props to be used when `handleMenuClick` or other dependencies change while the dropdown is open. Return the current `menu` element instead of the cached element to ensure handlers and props are up-to-date. [logic error]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ❌ Header actions menu can invoke outdated click handlers.
   - ⚠️ Menu props updates not reflected while dropdown open.
   - ⚠️ User actions may run stale callbacks.
   ```
   </details>
   
   ```suggestion
       menu,
   ```
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. A component renders and uses `useHeaderActionsMenu` from
   superset-frontend/src/dashboard/components/Header/useHeaderActionsDropdownMenu.tsx; the
   hook computes `menu` in the useMemo block and stores it into `menuRef.current` on first
   render (see lines near 342-344).
   
   2. The dropdown is opened (the consumer sets `isDropdownVisible` true), so the hook's
   return uses the cached element at lines 352-353: `menuRef.current` is returned while
   visible.
   
   3. While the dropdown remains open, one of the hook's dependencies changes (for example
   `isLoading`, `dashboardTitle`, or any dependency listed in the useMemo/useCallback
   dependency lists around lines ~117-129 and the useMemo dependencies). The hook creates a
   new `menu` instance with updated props/handlers.
   
   4. Because the hook continues returning the cached `menuRef.current`, the visible dropdown
   retains the old React element and its old handlers, causing stale handlers/props to be
   used until the dropdown is closed and reopened.
   ```
   </details>
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset-frontend/src/dashboard/components/Header/useHeaderActionsDropdownMenu.tsx
   **Line:** 353:353
   **Comment:**
   	*Logic Error: Caching the rendered React element (`menu`) in `menuRef.current` and returning that cached element while the dropdown is visible preserves the element instance (and therefore its props/handlers) across renders; this causes stale event handlers and props to be used when `handleMenuClick` or other dependencies change while the dropdown is open. Return the current `menu` element instead of the cached element to ensure handlers and props are up-to-date.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
   ```
   </details>



##########
superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.jsx:
##########
@@ -597,18 +608,18 @@ const Chart = props => {
   }, [exportTable]);
 
   const forceRefresh = useCallback(() => {
-    boundActionCreators.logEvent(LOG_ACTIONS_FORCE_REFRESH_CHART, {
+    logEventAction(LOG_ACTIONS_FORCE_REFRESH_CHART, {
       slice_id: slice.slice_id,
       is_cached: isCached,
     });
-    return boundActionCreators.refreshChart(chart.id, true, props.dashboardId);
+    return refreshChartAction(chart.id, true, dashboardId);
   }, [
-    boundActionCreators.refreshChart,
+    refreshChartAction,
     chart.id,

Review Comment:
   **Suggestion:** `forceRefresh` calls `refreshChartAction(chart.id, ...)` while `chart.id` may be undefined when the chart fallback object is used; this can result in dispatching a refresh for an undefined id — use the `chartId` prop to reliably identify the chart and update the dependency array accordingly. [possible bug]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ⚠️ Force-refresh could dispatch wrong chart id.
   - ⚠️ Affects manual refresh in SliceHeader component.
   - ⚠️ Limited scope; chart objects usually include id.
   ```
   </details>
   
   ```suggestion
       return refreshChartAction(chartId, true, dashboardId);
     }, [
       refreshChartAction,
       chartId,
   ```
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Inspect Chart.jsx: the chart selector is `const chart = useSelector(state =>
   state.charts[chartId] || EMPTY_OBJECT);` (hunk around line 196). `chartId` is the stable
   prop passed into the component (props destructured at top of Chart.jsx).
   
   2. The forceRefresh callback is defined at Chart.jsx lines 610-623 (existing snippet). It
   calls `refreshChartAction(chart.id, true, dashboardId)` (line 615 in the hunk).
   
   3. SliceHeader receives `forceRefresh={forceRefresh}` (see SliceHeader prop wiring in the
   render block around line ~647). A user clicking the manual refresh control will execute
   this callback.
   
   4. If the `chart` object is present but missing `id`, `refreshChartAction` will be invoked
   with `undefined` as the chart id (line 615), potentially dispatching an invalid refresh.
   In practice this is unlikely because chart objects normally include id and the component
   returns early when chart is exactly EMPTY_OBJECT; therefore the condition where chart
   exists but lacks `id` is rare.
   ```
   </details>
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.jsx
   **Line:** 615:618
   **Comment:**
   	*Possible Bug: `forceRefresh` calls `refreshChartAction(chart.id, ...)` while `chart.id` may be undefined when the chart fallback object is used; this can result in dispatching a refresh for an undefined id — use the `chartId` prop to reliably identify the chart and update the dependency array accordingly.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
   ```
   </details>



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #37459:
URL: https://github.com/apache/superset/pull/37459#issuecomment-3814710195

   🎪 [Showtime](https://github.com/mistercrunch/superset-showtime) is building environment on [GHA](https://github.com/apache/superset/actions/runs/21461201875) for [97f96a3](https://github.com/apache/superset/commit/97f96a3)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #37459:
URL: https://github.com/apache/superset/pull/37459#issuecomment-3814895153

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #37459:
URL: https://github.com/apache/superset/pull/37459#issuecomment-3936366182

   🎪 [Showtime](https://github.com/mistercrunch/superset-showtime) deployed environment on [GHA](https://github.com/apache/superset/actions/runs/22234758150) for [201b34d](https://github.com/apache/superset/commit/201b34d)
   
   • **Environment:** http://44.243.25.92:8080 (admin/admin)
   • **Lifetime:** 48h auto-cleanup
   • **Updates:** New commits create fresh environments automatically


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #37459:
URL: https://github.com/apache/superset/pull/37459#discussion_r2843448596


##########
superset-frontend/src/components/Chart/ChartRenderer.tsx:
##########
@@ -548,6 +558,7 @@ class ChartRenderer extends Component<ChartRendererProps, ChartRendererState> {
             legendState={this.state.legendState}
             enableNoResults={bypassNoResult}
             legendIndex={this.state.legendIndex}
+            isRefreshing={this.props.suppressLoadingSpinner}
             {...drillToDetailProps}

Review Comment:
   **Suggestion:** The `isRefreshing` prop passed to `SuperChart` is currently tied only to `suppressLoadingSpinner`, so the chart is marked as refreshing on every render when spinner suppression is enabled, even when no refresh is in progress, which can cause incorrect behavior in plugins relying on this flag; it should instead depend on the chart actually being in a loading state. [logic error]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ⚠️ SuperChart children see refreshing state after load completes.
   - ⚠️ Auto-refresh dashboards may show stale refreshing indicators.
   - ⚠️ Flicker-prevention logic can stay active unnecessarily.
   ```
   </details>
   
   ```suggestion
               isRefreshing={this.props.suppressLoadingSpinner && chartStatus === 'loading'}
   ```
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Render `ChartRenderer` from `superset-frontend/src/components/Chart/ChartRenderer.tsx`
   with props `chartStatus="loading"`, `suppressLoadingSpinner={true}`, and a non-empty
   `queriesResponse` without errors (see `render()` guard at lines 415–427).
   
   2. After data loads, update props so `chartStatus` becomes `"rendered"` or `"success"`
   while keeping `suppressLoadingSpinner={true}` and the same `queriesResponse` (normal
   post-refresh state for auto-refresh dashboards).
   
   3. On this update, `shouldComponentUpdate` (lines 86–137) returns `true` because
   `resultsReady` is satisfied and props have changed, so `render()` runs with
   `chartStatus!=="loading"`.
   
   4. In `render()` at lines 559–563, `SuperChart` is rendered with
   `isRefreshing={this.props.suppressLoadingSpinner}` while `chartStatus` is no longer
   `"loading"`, so `isRefreshing` stays `true` even though no refresh is in progress, causing
   any SuperChart consumer of `isRefreshing` to see an incorrect persistent "refreshing"
   state.
   ```
   </details>
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset-frontend/src/components/Chart/ChartRenderer.tsx
   **Line:** 562:562
   **Comment:**
   	*Logic Error: The `isRefreshing` prop passed to `SuperChart` is currently tied only to `suppressLoadingSpinner`, so the chart is marked as refreshing on every render when spinner suppression is enabled, even when no refresh is in progress, which can cause incorrect behavior in plugins relying on this flag; it should instead depend on the chart actually being in a loading state.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
   ```
   </details>
   <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37459&comment_hash=3ecb740b9eda4ede62666f60d21816275ed501f6ae2807b000dbaea6cf694bc3&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37459&comment_hash=3ecb740b9eda4ede62666f60d21816275ed501f6ae2807b000dbaea6cf694bc3&reaction=dislike'>👎</a>



##########
superset-frontend/plugins/plugin-chart-echarts/src/utils/orderby.ts:
##########
@@ -0,0 +1,47 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { QueryFormColumn, QueryFormOrderBy } from '@superset-ui/core';
+
+/**
+ * Builds orderby clauses from a list of columns, filtering out any non-string
+ * or nullish values. This ensures deterministic row ordering so that chart
+ * elements maintain stable positions across auto-refreshes.
+ */
+export function buildColumnsOrderBy(
+  columns: (QueryFormColumn | string | undefined | null)[],
+  ascending: boolean = true,
+): QueryFormOrderBy[] {
+  return columns
+    .filter((col): col is string => typeof col === 'string' && col !== '')
+    .map(col => [col, ascending]);
+}
+
+/**
+ * Conditionally applies orderby to a query object spread. Returns the
+ * orderby field only when row_limit is set (non-zero, non-null) and
+ * there are orderby entries to apply.
+ */
+export function applyOrderBy(
+  orderby: QueryFormOrderBy[],
+  rowLimit: string | number | undefined | null,
+): { orderby: QueryFormOrderBy[] } | Record<string, never> {
+  const shouldApply =
+    rowLimit !== undefined && rowLimit !== null && rowLimit !== 0;

Review Comment:
   **Suggestion:** The `applyOrderBy` helper treats a string row limit of `'0'` as a non-zero value, so when `rowLimit` is the string `"0"` it will still apply `orderby`, contradicting the function's intent and potentially changing query semantics compared with the numeric `0` case. To fix this, normalize string row limits to numbers before checking for zero so that both `0` and `"0"` are handled consistently as "no limit". [logic error]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ⚠️ ECharts query utils mishandle string "0" row limits.
   - ⚠️ Inconsistent behavior between numeric and string row_limit values.
   - ⚠️ Potentially unexpected ORDER BY on charts with disabled limit.
   ```
   </details>
   
   ```suggestion
     const numericRowLimit =
       typeof rowLimit === 'string' ? Number(rowLimit) : rowLimit;
     const shouldApply =
       numericRowLimit !== undefined &&
       numericRowLimit !== null &&
       numericRowLimit !== 0;
   ```
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Open `superset-frontend/plugins/plugin-chart-echarts/src/utils/orderby.ts` and locate
   `applyOrderBy` at lines 40–47.
   
   2. In any TypeScript test or caller, import this helper: `import { applyOrderBy } from
   'superset-frontend/plugins/plugin-chart-echarts/src/utils/orderby';`.
   
   3. Invoke it with a string row limit of `"0"` and a non-empty orderby:
   
      `applyOrderBy([['my_col', true]], '0');`.
   
   4. Observe that inside `applyOrderBy` the `shouldApply` expression is `true` (because `'0'
   !== 0`), so the function returns `{ orderby: [['my_col', true]] }`, whereas calling
   `applyOrderBy([['my_col', true]], 0)` returns `{}`. This inconsistent treatment of `"0"`
   vs `0` contradicts the documented intent ("only when row_limit is set (non-zero,
   non-null)") for the same effective row limit.
   ```
   </details>
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset-frontend/plugins/plugin-chart-echarts/src/utils/orderby.ts
   **Line:** 44:45
   **Comment:**
   	*Logic Error: The `applyOrderBy` helper treats a string row limit of `'0'` as a non-zero value, so when `rowLimit` is the string `"0"` it will still apply `orderby`, contradicting the function's intent and potentially changing query semantics compared with the numeric `0` case. To fix this, normalize string row limits to numbers before checking for zero so that both `0` and `"0"` are handled consistently as "no limit".
   
   Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
   ```
   </details>
   <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37459&comment_hash=778128e1b1f59dd71bbea64d190a8bf6d7a6b1d2fc84966b5c460a835e402096&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37459&comment_hash=778128e1b1f59dd71bbea64d190a8bf6d7a6b1d2fc84966b5c460a835e402096&reaction=dislike'>👎</a>



##########
superset-frontend/src/dashboard/actions/dashboardState.ts:
##########
@@ -770,12 +771,20 @@ export function fetchCharts(
       staggerRefresh && chartList.length > 1
         ? refreshTime / (chartList.length - 1)
         : 0;
-    chartList.forEach((chartKey: number, i: number) => {
-      setTimeout(
-        () => dispatch(refreshChart(chartKey, force, dashboardId)),
-        delay * i,
-      );
-    });
+    return Promise.all(
+      chartList.map(
+        (chartKey: number, i: number) =>
+          new Promise<void>(resolve => {
+            setTimeout(() => {
+              Promise.resolve(
+                dispatch(refreshChart(chartKey, force, dashboardId)),
+              )
+                .then(() => resolve())
+                .catch(() => resolve());

Review Comment:
   **Suggestion:** In the staggered refresh path, errors from individual chart refreshes are caught and ignored, so the returned Promise always resolves and callers like the dashboard refresh handler will dispatch a "success" action even when some chart refreshes failed; instead, errors should propagate so that overall refresh can be marked as failed consistently with the non-staggered path. [logic error]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ⚠️ Dashboard-level refresh reports success despite chart failures.
   - ⚠️ Auto-refresh status indicator may show green incorrectly.
   - ⚠️ Filter refresh actions run after partially failed refresh.
   ```
   </details>
   
   ```suggestion
             new Promise<void>((resolve, reject) => {
               setTimeout(() => {
                 Promise.resolve(
                   dispatch(refreshChart(chartKey, force, dashboardId)),
                 )
                   .then(() => resolve())
                   .catch(reject);
   ```
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Open any dashboard with multiple charts and auto-refresh enabled so that `interval > 0`
   and `chartList.length > 1` when `onRefresh` is dispatched (see `onRefresh` in
   `src/dashboard/actions/dashboardState.ts`, around lines 832–860, which calls
   `refreshCharts`, which calls `fetchCharts`).
   
   2. Ensure dashboard metadata has `stagger_refresh` enabled or undefined (defaulted to
   `true` in `fetchCharts` at `dashboardState.ts`, lines 753–768) so the staggered branch
   with computed `delay` is taken.
   
   3. Trigger a dashboard-level refresh (manual refresh button or timer-driven auto-refresh)
   so that `onRefresh(...)` dispatches `refreshCharts(chartList, force, interval,
   dashboardId, dispatch)`, which in turn calls `fetchCharts` and executes the staggered
   block shown at `dashboardState.ts`, lines 770–787.
   
   4. Cause one of the per-chart refresh thunks `refreshChart(chartKey, force, dashboardId)`
   (imported from `src/components/Chart/chartAction`) to reject (for example, by simulating a
   failing network request in that thunk); observe that the `.catch(() => resolve())` in
   `fetchCharts` swallows the rejection, `Promise.all` still resolves, and
   `onRefresh(...).then(...)` proceeds to dispatch `onRefreshSuccess()` and
   `onFiltersRefresh()` even though at least one chart refresh failed, whereas in the
   non-staggered path (no interval) such a rejection would cause the overall promise to
   reject and skip the "success" actions.
   ```
   </details>
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset-frontend/src/dashboard/actions/dashboardState.ts
   **Line:** 777:783
   **Comment:**
   	*Logic Error: In the staggered refresh path, errors from individual chart refreshes are caught and ignored, so the returned Promise always resolves and callers like the dashboard refresh handler will dispatch a "success" action even when some chart refreshes failed; instead, errors should propagate so that overall refresh can be marked as failed consistently with the non-staggered path.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
   ```
   </details>
   <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37459&comment_hash=dc6e601c9a8e46750f06a07ca20b994b2cb94b9cbf42c70612f3f9dd8027d706&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37459&comment_hash=dc6e601c9a8e46750f06a07ca20b994b2cb94b9cbf42c70612f3f9dd8027d706&reaction=dislike'>👎</a>



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "msyavuz (via GitHub)" <gi...@apache.org>.
msyavuz commented on code in PR #37459:
URL: https://github.com/apache/superset/pull/37459#discussion_r2792356696


##########
superset-frontend/src/dashboard/hooks/useTabVisibility.ts:
##########
@@ -0,0 +1,102 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { useEffect, useRef, useCallback, useState } from 'react';
+
+export interface UseTabVisibilityOptions {
+  /** Callback when tab becomes visible */
+  onVisible?: () => void;
+  /** Callback when tab becomes hidden */
+  onHidden?: () => void;
+  /** Whether the hook is enabled */
+  enabled?: boolean;
+}
+
+export interface UseTabVisibilityResult {
+  /** Whether the tab is visible */
+  isVisible: boolean;
+}
+
+/**
+ * Hook to track browser tab visibility state.
+ * Uses the Page Visibility API to detect when the user switches tabs.
+ *
+ * @example
+ * ```tsx
+ * const { isVisible } = useTabVisibility({
+ *   onVisible: () => console.log('Tab is visible'),
+ *   onHidden: () => console.log('Tab is hidden'),
+ * });
+ * ```
+ */
+export function useTabVisibility({
+  onVisible,
+  onHidden,
+  enabled = true,
+}: UseTabVisibilityOptions = {}): UseTabVisibilityResult {
+  const [isVisible, setIsVisible] = useState(
+    () => document.visibilityState === 'visible',
+  );

Review Comment:
   Does this check the document body or the tab? Seems a bit hacky



##########
superset-frontend/src/dashboard/hooks/useCurrentTime.ts:
##########
@@ -0,0 +1,67 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { useState, useEffect, useRef, useCallback } from 'react';
+
+/**
+ * Hook that provides the current time, updating every second.
+ *
+ * @param enabled - Whether the timer should be running
+ * @param syncTrigger - When this value changes, the timer restarts in phase
+ *                      with the new value. This ensures the display timer is
+ *                      synchronized with refresh cycles.
+ * @returns The current timestamp in milliseconds
+ */
+export const useCurrentTime = (
+  enabled = true,
+  syncTrigger?: number | null,
+): number => {
+  const [currentTime, setCurrentTime] = useState(() => Date.now());
+  const intervalRef = useRef<ReturnType<typeof setInterval> | null>(null);
+
+  const clearExistingInterval = useCallback(() => {
+    if (intervalRef.current !== null) {
+      clearInterval(intervalRef.current);
+      intervalRef.current = null;
+    }
+  }, []);
+
+  // When syncTrigger changes (refresh completes), restart the interval
+  // This keeps the display timer aligned with the refresh cycle
+  useEffect(() => {
+    if (!enabled) {
+      clearExistingInterval();
+      return undefined;
+    }
+
+    if (syncTrigger != null) {
+      setCurrentTime(Date.now());
+    }
+
+    clearExistingInterval();
+    intervalRef.current = setInterval(() => {
+      setCurrentTime(Date.now());
+    }, 1000);
+
+    return clearExistingInterval;
+  }, [enabled, syncTrigger, clearExistingInterval]);
+
+  return currentTime;
+};
+

Review Comment:
   Am i missing something here? Can you explain this hook a bit more? It looks like it just returns Date.now()?



##########
tests/unit_tests/extensions/test_types.py:
##########
@@ -65,9 +65,11 @@ def test_extension_config_full():
             "frontend": {
                 "contributions": {
                     "views": {
-                        "sqllab.panels": [
-                            {"id": "query_insights.main", "name": "Query Insights"}
-                        ]
+                        "sqllab": {
+                            "panels": [
+                                {"id": "query_insights.main", "name": "Query Insights"}
+                            ]
+                        }

Review Comment:
   Are these some rebase leftovers?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #37459:
URL: https://github.com/apache/superset/pull/37459#issuecomment-3861705223

   @geido Ephemeral environment spinning up at http://35.163.72.181:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #37459:
URL: https://github.com/apache/superset/pull/37459#issuecomment-3861660100

   ⚠️ **DEPRECATED WORKFLOW** ⚠️
   
   @geido This workflow is deprecated! Please use the new **Superset Showtime** system instead:
   
   - Replace "testenv-up" label with "🎪 trigger-start"
   - Better reliability and easier management
   - See https://github.com/mistercrunch/superset-showtime for details
   
   Processing your ephemeral environment request [here](https://github.com/apache/superset/actions/runs/21759373574). Action: **up**. More information on [how to use or configure ephemeral environments](https://superset.apache.org/docs/contributing/howtos/#github-ephemeral-environments)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #37459:
URL: https://github.com/apache/superset/pull/37459#issuecomment-3874202629

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] feat: auto refresh dashboard [superset]

Posted by "msyavuz (via GitHub)" <gi...@apache.org>.
msyavuz commented on PR #37459:
URL: https://github.com/apache/superset/pull/37459#issuecomment-3876085093

   @richardfogaca Can't you use the new async task framework to track these dashboard refreshes?
   
   The connection would be:
   - Dashboard refresh operations could be tracked as async tasks in the GTF
   - Users could monitor refresh status, progress, and errors in the unified task list UI
   - Failed refreshes could be retried/cancelled through the task management interface
   - The GTF's deduplication would prevent redundant refresh requests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org