Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

fix: dashboard refresh warning when no variables present#8191

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

@ktx-vaidehi
Copy link
Collaborator

@ktx-vaidehiktx-vaidehi commentedAug 29, 2025
edited by github-actionsbot
Loading

PR Type

Bug fix


Description

  • Prevent refresh warning without dashboard variables

  • Reset refreshed variables state when none exist


Diagram Walkthrough

flowchart LR  A["Dashboard load"] -- "No variables present" --> B["Set variablesData to idle"]  A -- "No variables present" --> C["Set refreshedVariablesData to idle"]  B -- "Prevents spurious warnings" --> D["Clean UX"]  C -- "Avoid stale refreshed state" --> D
Loading

File Walkthrough

Relevant files
Bug fix
ViewDashboard.vue
Reset refreshed variables state when none exist                   

web/src/views/Dashboards/ViewDashboard.vue

  • When no variables exist, also resetrefreshedVariablesData.
  • SetrefreshedVariablesData.isVariablesLoading to false.
  • ClearrefreshedVariablesData.values array.
+2/-0     

greptile-apps[bot] reacted with thumbs up emoji
@ktx-vaidehiktx-vaidehi marked this pull request as ready for reviewAugust 29, 2025 05:49
@github-actionsgithub-actionsbot added ☢️ BugSomething isn't working Review effort 2/5 labelsAug 29, 2025
@github-actions
Copy link
Contributor

github-actionsbot commentedAug 29, 2025
edited
Loading

PR Reviewer Guide 🔍

(Review updated until commitb9667ec)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

State Consistency

Ensure resetting bothvariablesData andrefreshedVariablesData to non-loading empty states is consistent with downstream consumers (watchers/computed/conditions) that may expect null/undefined or prior shape; verify no components rely on previous values or loading transitions.

  variablesData.isVariablesLoading = false;  variablesData.values = [];  refreshedVariablesData.isVariablesLoading = false;  refreshedVariablesData.values = [];}
UX Edge Case

Confirm that settingisVariablesLoading to false immediately when no variables exist does not skip any intended skeleton/loading cues or trigger conditions that assume a prior loading phase.

  variablesData.isVariablesLoading = false;  variablesData.values = [];  refreshedVariablesData.isVariablesLoading = false;  refreshedVariablesData.values = [];}

@github-actions
Copy link
Contributor

github-actionsbot commentedAug 29, 2025
edited
Loading

PR Code Suggestions ✨

Latest suggestions up tob9667ec
Explore these optional code suggestions:

CategorySuggestion                                                                                                                                   Impact
Possible issue
Add null guard on assignment

Guard againstrefreshedVariablesData being undefined to avoid runtime errors in edge
cases when the object isn't initialized. Add optional chaining or a null check
before assigning.

web/src/views/Dashboards/ViewDashboard.vue [689-690]

-refreshedVariablesData.isVariablesLoading = false;-refreshedVariablesData.values = [];+if (refreshedVariablesData) {+  refreshedVariablesData.isVariablesLoading = false;+  refreshedVariablesData.values = [];+}
Suggestion importance[1-10]: 4

__

Why: The existing lines at 689-690 match and adding a guard could prevent a runtime error ifrefreshedVariablesData is ever undefined, but the PR context likely implies it exists; impact is minor and situational.

Low

Previous suggestions

Suggestions up to commitb9667ec
CategorySuggestion                                                                                                                                   Impact
General
Ensure reactive array updates

Ensure reactive updates by replacing the arrays instead of mutating them directly.
In Vue reactivity, reassigning to a new array helps avoid stale references and
ensures watchers/computed values update correctly.

web/src/views/Dashboards/ViewDashboard.vue [687-690]

 variablesData.isVariablesLoading = false; variablesData.values = []; refreshedVariablesData.isVariablesLoading = false;-refreshedVariablesData.values = [];+refreshedVariablesData.values = [...[]];
Suggestion importance[1-10]: 2

__

Why: The proposed change is effectively a no-op:[] and[...[]] both create new arrays, offering no improvement in Vue reactivity. The existing code already replaces arrays, so the suggestion adds little value and does not address an actual issue.

Low

@github-actions
Copy link
Contributor

Persistent review updated to latest commitb9667ec

Copy link
Contributor

@greptile-appsgreptile-appsbot left a comment

Choose a reason for hiding this comment

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

Greptile Summary

This PR fixes a UX issue in the dashboard interface where the refresh button would incorrectly display a warning state (orange color with warning message) when no variables are present on the dashboard. The problem occurred because of inconsistent initialization of two reactive objects used to track variable states.

The change is inViewDashboard.vue and addresses the root cause by ensuring bothvariablesData andrefreshedVariablesData reactive objects are properly initialized with identical empty states when a dashboard has no variables configured. Specifically, when no variables are present, both objects now get:

  • isVariablesLoading set tofalse
  • values set to an empty array[]

This fix integrates with the existingisVariablesChanged computed property that performs deep equality comparison between these two objects to determine whether to show the warning state. Previously, onlyvariablesData was being initialized whilerefreshedVariablesData remained uninitialized (empty object{}), causing the comparison to always returntrue for "variables changed" even when no variables existed. The solution ensures consistent initialization, making the comparison accurate and eliminating false positive warnings.

The change fits well within the component's architecture, which already handles various dashboard states and uses these reactive objects throughout the variable management system.

Confidence score: 5/5

  • This PR is safe to merge with minimal risk
  • Score reflects a targeted bug fix with clear logic and no side effects
  • No files require special attention

1 file reviewed, no comments

Edit Code Review Bot Settings |Greptile

@ktx-vaidehiktx-vaidehiforce-pushed thefix/dashboard/refresh-warning-when-no-variables-present branch fromb9667ec to352bc06CompareAugust 29, 2025 07:58
@ktx-vaidehiktx-vaidehi merged commit4ce6a16 intomainAug 29, 2025
29 of 30 checks passed
@ktx-vaidehiktx-vaidehi deleted the fix/dashboard/refresh-warning-when-no-variables-present branchAugust 29, 2025 09:32
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@ktx-kirtanktx-kirtanktx-kirtan approved these changes

+1 more reviewer

@greptile-appsgreptile-apps[bot]greptile-apps[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

☢️ BugSomething isn't workingReview effort 2/5

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ktx-vaidehi@ktx-kirtan

[8]ページ先頭

©2009-2025 Movatter.jp