🧹 [Refactor applyBatchUpdate to improve maintainability]#360
🧹 [Refactor applyBatchUpdate to improve maintainability]#360google-labs-jules[bot] wants to merge 1 commit into
Conversation
Extracts validation, update preparation, and local grid update logic from the applyBatchUpdate function in core/ui/modules/sidebar.js into separate helper functions to improve maintainability and readability.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Closing — refactors |
Understood. Acknowledging that this work is now obsolete and stopping work on this task. |
* refactor(ui): extract testable batch-update logic from sidebar.js Reworks the closed #360/#361 (which couldn't be unit-tested as in-file helpers — sidebar.js's import chain requires a DOM). Extracts the pure value-processing logic into a DOM-free module so it can be unit-tested: - core/ui/modules/batch-update-logic.js (+ .d.ts): groupSelectedCellsByColumn, summarizeColumnValue, prepareBatchUpdates (type coercion / NULL / json_patch / skip-empty rules) - sidebar.js: updateBatchSidebar + applyBatchUpdate delegate to these (behavior unchanged); removed a now-unused escapeHtml import - tests/unit/batch-update-logic.test.ts: 13 cases covering the rules Verified: 324 unit tests pass (was 311); tsc --noEmit clean; full batch-update flow runtime-checked in the web demo (select 2 cells -> panel shows the grouped column with "(mixed values)" -> apply -> both cells update; zero console errors). Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(ui): harden batch-update logic against edge cases Applies the defensive findings from Gemini's review of this PR: - groupSelectedCellsByColumn / prepareBatchUpdates: skip cells whose colDef is missing (stale/out-of-bounds selection, e.g. after a column drop) rather than throwing - summarizeColumnValue: return '' for an empty value set - prepareBatchUpdates: tolerate an input without a `dataset` - `dataset` made optional in the .d.ts + 4 edge-case unit tests (suite: 328 passing). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(ui): address remaining batch-update review nits - batch-update-logic.js: normalize column type to upper-case before numeric coercion (Gemini — SQLite reports declared types verbatim, e.g. 'integer', so the case-sensitive check could miss lowercase numeric columns) - sidebar.js: guard colDef?.name in the invalid-JSON validation path against a stale/out-of-bounds colIdx (Codex non-blocking caveat) - tests: add NUMERIC + lowercase-type coercion cases (suite: 330 passing) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
🎯 What: Extracted validation, preparation, and local update logic from
applyBatchUpdateinto smaller helper functions.💡 Why:
applyBatchUpdatewas too long and complex. Breaking it into cohesive pieces makes the logic easier to reason about, test, and maintain.✅ Verification: Verified by checking that no syntax errors were introduced (
bun build), testing the project functionality vianpm test, and receiving a positive code review that confirmed the safety and completeness of the patch.✨ Result: Improved codebase maintainability and readability without altering behavior.
PR created automatically by Jules for task 13925686891959446430 started by @zknpr