Fix: Response not iterable on trigger expansion#10177
Open
kundansable wants to merge 1 commit into
Open
Conversation
…unction
Expanding a Trigger node under a Table to view its function threw
'Response' object is not iterable, crashing the tree.
get_children_nodes() can legitimately return a Flask Response instead
of a list (e.g. gone("Could not find the specified trigger
function") when the trigger's function has no matching row in the
trigger-function node query, as happens for a genuine internal-
language function such as suppress_redundant_updates_trigger).
NodeView.children() / PGChildNodeView.children() unconditionally
sorted whatever get_children_nodes() returned, crashing on a
Response instead of passing it through.
Separately, TriggerView.get_children_nodes() rendered the trigger-
function node.sql template (which quotes fnid via qtLiteral(conn))
without passing conn into the template context. This call site
predates qtLiteral requiring a connection; after qtLiteral was
hardened in 658bb58 to raise ValueError instead of silently
degrading when conn is missing, this broke node.sql rendering for
every trigger, not just the internal-language case.
Fix both: check isinstance(children, flask.Response) in children()
before sorting/iterating and return it as-is, and pass
conn=self.conn into the node.sql render call, matching every other
render_template call site in the file.
Fixes pgadmin-org#10117
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
WalkthroughTrigger-function SQL rendering now receives the active connection. Browser child-node methods detect Flask responses and return them directly, while preserving sorting and JSON response handling for normal node lists. ChangesTrigger response handling
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #10117 — expanding a Trigger node under a Table to view its trigger function throws
'Response' object is not iterable, crashing the tree instead of showing the function (or a clean error).Root Cause
Two separate bugs on the same code path,
TriggerView.get_children_nodes():get_children_nodes()can legitimately return a FlaskResponseinstead of a list — e.g.gone("Could not find the specified trigger function")when the trigger's function has no matching row in the trigger-function node query (see #2).NodeView.children()/PGChildNodeView.children()unconditionally calledsorted(...)on whateverget_children_nodes()returned, so aResponseblew up with'Response' object is not iterableinstead of being passed through.render_template()call that builds the trigger-functionnode.sqlquery never passedconn=into the template context, even though the template quotesfnidvia{{ fnid|qtLiteral(conn) }}. This call site predatesqtLiteralrequiring a connection; afterqtLiteralwas hardened (in658bb585d) toraise ValueErrorinstead of silently degrading whenconnis missing, this call started raising on every trigger expansion (not just the internal-language case), which is what actually surfaced while verifying fix ISSUE2277-Internal server error displayed if table name contains long character #1 — before that, it looked like every trigger's function node was broken with aqtLiteral requires a connection500 error, masking the narrower, real 'Response' object is not iterable #10117 repro underneath.Fix
NodeView.children()andPGChildNodeView.children()(web/pgadmin/browser/utils.py), checkisinstance(children, flask.Response)before sorting/iterating, and return the Response as-is when it is one.TriggerView.get_children_nodes()(web/pgadmin/browser/server_groups/servers/databases/schemas/tables/triggers/__init__.py), passconn=self.conninto thenode.sqlrender call, matching every otherrender_templatecall site in the file.Test Steps
plpgsqltrigger function:trigger_bug_test > Triggers > trg_suppress_redundant.'Response' object is not iterable.trigger_bug_test > Triggers > trg_normal_plpgsql.conn=self.connfix:qtLiteral requires a connection500 error.trg_normal_plpgsql_fn()function node.Summary by CodeRabbit