Skip to content

Remove the dead tree 'loadfail' handler from browser.js. #10601 - #10620

Closed
dpage wants to merge 1 commit into
pgadmin-org:masterfrom
dpage:fix/issue-10601-remove-loadfail-handler
Closed

dpage wants to merge 1 commit into
pgadmin-org:masterfrom
dpage:fix/issue-10601-remove-loadfail-handler

Conversation

@dpage

@dpage dpage commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Removes onLoadFailNode and its pgadmin-browser:tree:loadfail registration from browser.js. Nothing has emitted that event since the move away from aciTree (load failures are handled in tree_nodes.ts), and the handler called tree.isSelected(), which the current Tree class does not have.
  • No user-visible change; the master password prompt is still raised when the UI loads (uiloaded).

Test plan

  • eslint on browser.js
  • Jest regression/javascript/browser suites pass

Closes #10601

Summary by CodeRabbit

  • Behavior Changes
    • The browser no longer prompts you to set a master password when a tree node fails to load.

…10601

Nothing has emitted the 'pgadmin-browser:tree:loadfail' event since the
move away from aciTree, and the handler called tree.isSelected(), which
the current Tree class does not provide.

Closes pgadmin-org#10601
@coderabbitai

coderabbitai Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: pgadmin-org/pgadmin4/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 63417ec5-e206-4c2c-94e3-d08b74675e40
📥 Commits

Reviewing files that changed from the base of the PR and between b98470f and 3417b6b.

📒 Files selected for processing: 1
  • web/pgadmin/browser/static/js/browser.js
💤 Files with no reviewable changes: 1
  • web/pgadmin/browser/static/js/browser.js

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 5 remain after this review.

Walkthrough

The browser no longer registers the pgadmin-browser:tree:loadfail event or defines its handler, which previously checked master-password status and could reopen a selected node.

Changes

Tree load-failure handler removal

Layer / File(s) Summary
Remove load-failure handling
web/pgadmin/browser/static/js/browser.js
The browser no longer registers the pgadmin-browser:tree:loadfail event or defines onLoadFailNode.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Refactor

Merge Risk: ⚪ Minimal · up to 3417b

The handler removal appears to leave tree error handling and master-password prompts intact. No actionable merge risk remains after normal checks.

Pre-merge checks: Passed 5
Check name Status Explanation
Linked Issues check Passed Issue #10601 requires cleanup of the unreachable loadfail handler and its event registration. The whole-PR diff removes both pgadmin-browser:tree:loadfail registration and onLoadFailNode from `w…
Out of Scope Changes check Passed The whole-PR diff contains only the removal of the dead handler and its matching event registration in browser.js. These changes directly implement issue #10601. No unrelated code changes are presen…
Docstring Coverage Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: removing the obsolete tree load-failure handler from browser.js.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@dpage

dpage commented Oct 15, 2026

Copy link
Copy Markdown
Contributor Author

Squashed and merged as cc1e1ef.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove the dead tree 'loadfail' handler in browser.js, which calls a non-existent tree.isSelected()

1 participant