Skip to content

Conversation

mtrezza
Copy link
Member

@mtrezza mtrezza commented Oct 14, 2025

New Pull Request Checklist

Issue Description

When editing a View that is different from the currently displaying view and then saving, the currently displaying view's table data was being reloaded unnecessarily.

Approach

The onConfirm callback in the EditViewDialog was always calling loadViews() without any condition, which internally called loadData(this.props.params.name) to reload the currently displayed view's data. Added a skipDataReload parameter to the loadViews() method. Modified the edit view confirm handler to check if the edited view is the currently displayed view. Only reload data when editing the currently displayed view; otherwise, skip the data reload.

Summary by CodeRabbit

  • Bug Fixes

    • Editing a non-active view no longer triggers an unnecessary data reload, preventing UI flicker and preserving your current context.
    • Data reloads now respect edit context and local-cache fallbacks, reducing unexpected refreshes during save or error recovery.
    • Initial view loading avoids redundant reloads, improving stability when navigating and editing views.
  • Refactor

    • Improved view-loading control to selectively skip data reloads, resulting in smoother interactions when managing views.

Copy link

parse-github-assistant bot commented Oct 14, 2025

🚀 Thanks for opening this pull request! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review.

Copy link

coderabbitai bot commented Oct 14, 2025

📝 Walkthrough

Walkthrough

Adds an optional skipDataReload parameter to loadViews and updates Edit View and local-storage fallback paths to pass and respect this flag, allowing conditional suppression of the post-view-load data reload.

Changes

Cohort / File(s) Summary
Views loading and edit-flow control
src/dashboard/Data/Views/Views.react.js
- Updated method signature: loadViews(app, skipDataReload = false)
- Data reload after building view counts only runs when skipDataReload is false
- Local-storage fallback path also respects skipDataReload
- Edit View flow computes isEditingCurrentView (using original view name) and passes skipDataReload to loadViews, so saving an edit reloads data only when the edited view is the active view

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor U as User
  participant E as EditViewModal
  participant V as Views (Views.react.js)
  participant D as Data Layer

  U->>E: Save edited view
  E->>V: onSave(editedView)
  V->>V: determine isEditingCurrentView (compare original/current view)
  V->>V: set skipDataReload = !isEditingCurrentView
  V->>V: loadViews(app, skipDataReload)

  alt Load from server
    V->>D: request views
    D-->>V: views list
    V->>V: build view counts
    opt when not skipDataReload
      V->>D: reload data for active view
      D-->>V: data refreshed
    end
  else Fallback to local storage
    V->>V: load views from localStorage
    opt when not skipDataReload
      V->>D: reload data for active view
      D-->>V: data refreshed
    end
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The PR description includes the checklist, issue description, and approach but is missing the required “Closes:” reference for the linked issue and lacks the “TODOs before merging” section with tasks for adding tests and documentation. These omissions mean the template is not fully followed. The missing sections are essential for tracking issue closure and ensuring tests and docs are addressed before merging. Please add a “Closes: ” line under the Issue Description to link the related issue and include the “TODOs before merging” section from the template with checklist items for adding tests and updating documentation.
✅ Passed checks (2 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
Title Check ✅ Passed The title concisely states the bug being addressed—the unintended reload of the currently displayed view when editing and saving a different view—and uses a “fix:” prefix to denote a bug fix. It directly relates to the main change and conveys the primary issue without extraneous detail. Though it describes the problem more than the implementation, it remains specific and clear.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@parseplatformorg
Copy link
Contributor

parseplatformorg commented Oct 14, 2025

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a3c6a83 and a4c1bb8.

📒 Files selected for processing (1)
  • src/dashboard/Data/Views/Views.react.js (5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Docker linux/amd64
🔇 Additional comments (4)
src/dashboard/Data/Views/Views.react.js (4)

90-90: LGTM! Good backward-compatible API design.

The addition of the skipDataReload parameter with a default value of false maintains backward compatibility while enabling the new behavior.


135-137: LGTM! Correct implementation.

The condition properly gates the data reload while maintaining the existing safety check for component mount status.


144-146: LGTM! Consistent error handling.

The fallback path correctly respects the skipDataReload parameter, ensuring consistent behavior whether views are loaded from the server or local storage.


776-779: LGTM! Clear implementation with helpful comments.

The logic correctly computes skipDataReload based on whether the edited view is the currently displayed one. The comments clearly explain the intent, making the code maintainable.

Note: This assessment assumes the isEditingCurrentView check at Line 760 is correct (see comment above for verification).

@mtrezza mtrezza changed the title fix: View reload fix: Currently displayed view reloads when editing and saving a different view Oct 14, 2025
@mtrezza mtrezza merged commit 794a35a into parse-community:alpha Oct 14, 2025
11 checks passed
@mtrezza mtrezza deleted the fix/view-reload branch October 14, 2025 22:35
parseplatformorg pushed a commit that referenced this pull request Oct 14, 2025
# [7.6.0-alpha.11](7.6.0-alpha.10...7.6.0-alpha.11) (2025-10-14)

### Bug Fixes

* Currently displayed view reloads when editing and saving a different view ([#3002](#3002)) ([794a35a](794a35a))
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 7.6.0-alpha.11

@parseplatformorg parseplatformorg added the state:released-alpha Released as alpha version label Oct 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state:released-alpha Released as alpha version

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants