Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fixed widgets not visible after saving in the dashboard detail #7106

Merged
merged 4 commits into from
Feb 13, 2025

Conversation

SagarRajput-7
Copy link
Contributor

@SagarRajput-7 SagarRajput-7 commented Feb 13, 2025

Summary

Here we had a race condition in maintaining 2 interdependent variables dashboardLayout and layouts caused by the parallel execution of the get and put calls triggered from different sources.

The get API gets called with the updated values and returns the correct result. Meanwhile, the put call doesn't wait for the get call to finish and update the variables; rather, it continues with the older value. In those cases when the put call finishes after the get call, it overrides the updated value. Hence, we see the widget coming and then going away.

Related Issues / PR's

Solves following customer issues:

Screenshots

Before:

Screen.Recording.2025-02-13.at.8.01.23.AM.mov

After:

Solved-Dashboard-not-visible.mov

Affected Areas and Manually Tested Areas


Important

Fix widgets visibility issue by adding isDashboardFetching check in GridCardLayout.tsx and updating dashboard context in Dashboard.tsx.

  • Behavior:
    • Add isDashboardFetching check in GraphLayout in GridCardLayout.tsx to prevent saving layout during fetch.
    • Modify useDashboard in Dashboard.tsx to set isDashboardFetching state during dashboard data fetch.
  • Context:
    • Add isDashboardFetching to IDashboardContext in types.ts and Dashboard.tsx.
  • Misc:
    • Update DashboardProvider in Dashboard.tsx to manage isDashboardFetching state.

This description was created by Ellipsis for 98ec616. It will automatically update as commits are pushed.

@github-actions github-actions bot added the bug Something isn't working label Feb 13, 2025
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks good to me! Reviewed everything up to 98ec616 in 1 minute and 3 seconds

More details
  • Looked at 90 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 5 drafted comments based on config settings.
1. frontend/src/container/GridCardLayout/GridCardLayout.tsx:487
  • Draft comment:
    Avoid inline styles in React. Consider moving the background color style to a CSS class or styled component.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. frontend/src/container/GridCardLayout/GridCardLayout.tsx:232
  • Draft comment:
    Good addition of '!isDashboardFetching' in the condition to prevent premature layout saves during an active fetch. Consider adding isDashboardFetching (and updateDashboardMutation.isLoading) to the dependency array or a comment justifying their exclusion to avoid stale closures.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. frontend/src/providers/Dashboard/Dashboard.tsx:262
  • Draft comment:
    Nice use of a try-finally block in the async queryFn to manage the isDashboardFetching state. This pattern ensures the flag resets even if an error occurs.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
4. frontend/src/providers/Dashboard/Dashboard.tsx:436
  • Draft comment:
    The context value now passes isDashboardFetching, ensuring components consuming the Dashboard context have access to the updated fetch status.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
5. frontend/src/providers/Dashboard/types.ts:50
  • Draft comment:
    The IDashboardContext interface now includes isDashboardFetching, which aligns well with the context changes in DashboardProvider.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None

Workflow ID: wflow_nS3AWvtjE5aeOGA8


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@SagarRajput-7 SagarRajput-7 merged commit 50ecf76 into main Feb 13, 2025
14 of 16 checks passed
@SagarRajput-7 SagarRajput-7 deleted the SIG-6947 branch February 13, 2025 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants