Keep query field order when using Batch Edit#8023
Conversation
github-project-automation
Bot
moved this from 📋Back Log
to ✅Done
in General Tester Board
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughCompute of batch-edit column visual ordering changed from a single fallback to a bucketed, position-aware algorithm: enumerated headers are grouped by original query field index, duplicates placed by their duplicate offset, out-of-range headers appended as new buckets, then flattened to produce Changes
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@specifyweb/backend/stored_queries/batch_edit.py`:
- Around line 1100-1107: The loop that assigns original_place using only key[0]
misplaces repeated QueryField occurrences; modify the logic in the
headers_enumerated loop to make placement occurrence-based by tracking per-key
occurrence counts (e.g., a dict seen_counts keyed by key or QueryField id),
compute original_place = key[0] + seen_counts[key] (or otherwise offset the base
index by the number of prior occurrences), then increment seen_counts[key]; use
that computed original_place when checking visual_order and appending to
new_columns so duplicate fields map to their nth original slot rather than being
treated as new.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 55bd6724-bccf-4ddc-a546-90f2bfdf07eb
📒 Files selected for processing (1)
specifyweb/backend/stored_queries/batch_edit.py
github-project-automation
Bot
moved this from ✅Done
to Dev Attention Needed
in General Tester Board
|
@specify/ux-testing Should be good to test. If the PR is found to work correctly I'll update the tests. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
specifyweb/backend/stored_queries/batch_edit.py (1)
1099-1112:⚠️ Potential issue | 🟠 MajorPreserve repeated query-field occurrences by occurrence, not first match.
This bucketing still collapses repeated query fields onto the first matching visible-field slot. If the query is
[A, B, A], bothAcolumns share the samekey[0], so flattening these groups can produce[A, A, B]instead of the original order. Please carry the exact visible-field occurrence into the key, rather than the firststring_idmatch, before buildingvisual_order_groups.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@specifyweb/backend/stored_queries/batch_edit.py` around lines 1099 - 1112, The current bucketing in visual_order_groups uses key[0] which is the first visible_fields match for a string_id, collapsing repeated fields; fix by making the header key carry the exact visible-field occurrence (i.e., which duplicate instance of that string_id it maps to) when headers_enumerated is built so that key[0] is the occurrence index rather than the first-match index—update the key construction logic that produces headers_enumerated (used by the loop that references headers_enumerated, key, visible_fields, existing_column) to compute the n-th occurrence by counting prior matches in visible_fields and include that occurrence index in the key so visual_order_groups.insert/append preserves original repeated-field order before flattening into visual_order.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@specifyweb/backend/stored_queries/batch_edit.py`:
- Around line 1105-1108: The code references an undefined variable
existing_column when trying to append new generated columns to
visual_order_groups, causing a NameError; fix by removing the nonexistent guard
and always appending the new field index to visual_order_groups (i.e., replace
the conditional block using existing_column with a direct
visual_order_groups.append([index]) in the function manipulating
visual_order_groups and index).
---
Duplicate comments:
In `@specifyweb/backend/stored_queries/batch_edit.py`:
- Around line 1099-1112: The current bucketing in visual_order_groups uses
key[0] which is the first visible_fields match for a string_id, collapsing
repeated fields; fix by making the header key carry the exact visible-field
occurrence (i.e., which duplicate instance of that string_id it maps to) when
headers_enumerated is built so that key[0] is the occurrence index rather than
the first-match index—update the key construction logic that produces
headers_enumerated (used by the loop that references headers_enumerated, key,
visible_fields, existing_column) to compute the n-th occurrence by counting
prior matches in visible_fields and include that occurrence index in the key so
visual_order_groups.insert/append preserves original repeated-field order before
flattening into visual_order.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: edcd8096-98a6-4781-980c-451f47375b94
📒 Files selected for processing (1)
specifyweb/backend/stored_queries/batch_edit.py
emenslin
left a comment
emenslin
left a comment
There was a problem hiding this comment.
- See that the order stayed the same as in the original query.
- Make sure there are no glitches when sorting by column, looking at the mapping, editing cells, and searching
- See that the order is the same, and all new columns were added to the end.
Looks good, I didn't run into any new issues
Iwantexpresso
left a comment
Iwantexpresso
left a comment
There was a problem hiding this comment.
- See that the order stayed the same as in the original query.
- Make sure there are no glitches when sorting by column, looking at the mapping, editing cells, and searching
- See that the order is the same, and all new columns were added to the end.
Nice work!
|
NOTE: Old: New: |
bhumikaguptaa
left a comment
bhumikaguptaa
left a comment
There was a problem hiding this comment.
- See that the order stayed the same as in the original query.
- Make sure there are no glitches when sorting by column, looking at the mapping, editing cells, and searching
- See that the order is the same, and all new columns were added to the end.
Works as expected!
|
@alesan99 My feeling is that the query field order should be preserved.
Since the query lines show |
|
@alesan99 @grantfitzsimmons The order Grant describes is definitely what the user will expect and how the user will envision their data. It will be seen as a bug if it orders by field (integer1, integer1 #2, etc.) rather than by record. |
|
I will order the columns by record whenever there are multiple columns for a field 👍 Going to set the PR to a draft in the meanwhile |
|
@specify/ux-testing I have updated this PR, its ready for review again 👍 This is the new behavior for fields with duplicate columns. All other fields should still be in the query order |
emenslin
left a comment
emenslin
left a comment
There was a problem hiding this comment.
- See that the order stayed the same as in the original query.
- Make sure there are no glitches when sorting by column, looking at the mapping, editing cells, and searching
- See that the order is the same, and all new columns were added to the end.
When tree fields are included in the query, the batch edit results are displayed incorrectly.
With trees:
05-13_09.44.mp4
Query: https://ojsmnh20260326-issue-6597.test.specifysystems.org/specify/query/200/
Without trees:
05-13_09.45.mp4
Query: https://ojsmnh20260326-issue-6597.test.specifysystems.org/specify/query/328/
bhumikaguptaa
left a comment
bhumikaguptaa
left a comment
There was a problem hiding this comment.
- See that the order stayed the same as in the original query.
- Make sure there are no glitches when sorting by column, looking at the mapping, editing cells, and searching
- See that the order is the same, and all new columns were added to the end.
While i was creating the query, if I map any field from the Project table I run into an error. And the query never loads.
Crash report: Specify 7 Crash Report - 2026-05-13T16_06_03.003Z.txt
Link to query: https://ojsmnh20260326-issue-6597-2.test.specifysystems.org/specify/query/330/
7 participants
Fixes #6597
Preserves order from query to batch edit.
Query:
New:
Old:
What this PR does not do:
Checklist
self-explanatory (or properly documented)
Testing instructions
Summary by CodeRabbit