fix(queries): skip missing tree ranks - #8510
grantfitzsimmons wants to merge 1 commit into
Conversation
If a query, data export, or data view is executed in a collection that doesn't have matching ranks, this prevents it from crashing
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughStored query handling now tolerates unmapped fields and missing tree ranks. Tree joins can be reverted, missing ORM fields skip filter processing, and displayed fields receive ChangesStored query resilience
Suggested reviewers: Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to Stored queries with unmapped fields or missing tree ranks now avoid execution errors while preserving output alignment and skipping unavailable filters and sorts. No concrete merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (4 passed)
Full details: Automatic TestsExplanation Automatic tests are necessary for this behavior change. The authoritative PR diff changes only Resolution Add automatic regression tests in Full details: Testing InstructionsExplanation The instructions cover table-view execution, filter/sort handling, and export behavior, but they omit the Statistics component. The changed Resolution Update the instructions to include importing or opening the affected query in Statistics and confirm that the statistic loads without an error. State how to obtain or import the invalid query, because the normal query builder may not allow creation of an unavailable rank. Specify CSV export, and test other supported export modes if they are in scope. Verify that a missing displayed rank produces an empty placeholder and preserves later columns, while a missing filtered or sorted rank does not change the result set. Add the claimed automated tests or remove the claim about automated coverage.
✨ 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 |
g1rly-c0d3r
left a comment
There was a problem hiding this comment.
Testing instructions
- Open or create a stored query containing a tree-rank field that is not present in the active collection. (If you are having a hard time finding one, I guess you can use this: FakeRankQuery.json)
- Execute the query in the table view and confirm that it does not crash and that the remaining result columns stay aligned with their headers.
- Apply a filter or sort using the unavailable rank and confirm that the query still executes while ignoring that field.
- Export the same query and confirm that the export completes and the unavailable field is omitted or empty without shifting other columns.
Everything looked good, none of the columns were shifted and no errors happened. I looked at the re-exported query and the fake rank was still in the export, but the query results were empty. I'm not sure if you meant that in the exported JSON the fake rank should be omitted.
There was a problem hiding this comment.
Testing instructions
- Open or create a stored query containing a tree-rank field that is not present in the active collection. (If you are having a hard time finding one, I guess you can use this: FakeRankQuery.json)
- Execute the query in the table view and confirm that it does not crash and that the remaining result columns stay aligned with their headers.
- Apply a filter or sort using the unavailable rank and confirm that the query still executes while ignoring that field.
- Export the same query and confirm that the export completes and the unavailable field is omitted or empty without shifting other columns.
Seems good to me. This may also fix #3369 but I'm not sure.
Fixes #3351
If a query, data export, (or soon, data view query) is executed in a collection that doesn't have matching ranks, this prevents it from crashing. This is a common problem for databases that share queries between collections, especially when those collections exist in other disciplines where some tree ranks might not exist. It also allows us to ship default queries or export mappings that function even if the ranks included in the defaults are not present in the collection(s).
Checklist
self-explanatory (or properly documented)
Testing instructions
There is automated coverage should verify both a missing displayed rank (preserving the result-column position) and a missing filtered/sorted rank (ignoring the field without changing the result set).
Summary by CodeRabbit