Skip to content

feat(bulk-entry): add bulk entry workflow for snapshot creation#26

Merged
Jench2103 merged 25 commits intomainfrom
feat/bulk-snapshot-entry
Mar 23, 2026
Merged

feat(bulk-entry): add bulk entry workflow for snapshot creation#26
Jench2103 merged 25 commits intomainfrom
feat/bulk-snapshot-entry

Conversation

@Jench2103
Copy link
Owner

@Jench2103 Jench2103 commented Mar 23, 2026

Summary

Adds a new "Bulk Entry" mode as the default snapshot creation workflow. Instead of creating empty snapshots or copying from the latest, users now get a full-screen editable table with all assets from the prior snapshot grouped by platform, where they can enter new market values directly or import them from per-platform CSV files. This replaces the previous "Copy from latest" option and provides a significantly more efficient data entry experience.

Changes

  • Bulk Entry ViewModel & Types

    • BulkEntryViewModel with platform-grouped rows, save logic (zero-value fallback for pending rows), and per-platform CSV import with match/append semantics
    • BulkEntryRow value type with rich state tracking (updated, pending, excluded, validation errors, zero-value detection, duplicate name detection)
    • CSVImportResult with formatted user-facing messages for match counts, errors, and warnings
  • Bulk Entry View

    • BulkEntryView with platform-grouped table, keyboard navigation (Tab/Enter between fields), and toolbar with progress stats, validation warnings, and save button
    • BulkEntrySubviews extracted as structs: progress stats, validation warnings, platform section headers, asset rows, empty state, and glass-style toolbar
    • Inline asset creation with name, currency, and category picker per platform group
    • "Add Platform" toolbar popover for creating new platform groups
    • Per-platform CSV import via file picker
  • Snapshot List & Navigation

    • Updated NewSnapshotSheet to replace "Copy from latest" toggle with a creation mode picker (Empty / Bulk Entry)
    • ContentView updated to navigate to BulkEntryView when bulk entry mode is selected
    • Zero-value warning icons on snapshot list rows and snapshot detail asset rows via new HoverWarningIcon component
  • CSV Import Enhancement

    • ImportViewModel+Helpers now allows CSV import to overwrite zero-value assets in existing snapshots (deferred from bulk entry pending rows)
  • Utilities

    • WhenUnlockedModifiers extended with helpWhenUnlocked(_ text: String) overload; HelpWhenUnlockedModifier stabilized to always apply .help() (avoiding @FocusState resets from view tree changes)
  • Localization

    • Full Traditional Chinese (zh-Hant) translations for all bulk entry strings in Localizable.xcstrings and Snapshot.xcstrings
  • Documentation

    • Updated SPEC.md, BusinessLogic.md, and UserInterfaceDesign.md with bulk entry feature spec, validation rules, save semantics, and implementation status
    • Added user-facing bulk entry guide for MkDocs (English and zh-TW) with screenshots
  • Tests

    • 751-line BulkEntryViewModelTests covering row state logic, platform grouping, counts, save semantics (including zero-value fallback and new asset creation), CSV import matching/appending/re-import, manual row/platform CRUD, duplicate name detection, and category resolution
    • SnapshotListViewModelTests extended with zero-value asset detection tests

Testing

  • Unit tests: BulkEntryViewModelTests (25 tests) and SnapshotListViewModelTests (2 new tests) using Swift Testing with in-memory SwiftData containers
  • Manual testing: snapshot creation via bulk entry (empty state, with prior data), per-platform CSV import, inline asset/platform creation, keyboard navigation, zero-value warnings, cancel/discard flows

Notes

  • The "Copy from latest" creation mode has been replaced by "Bulk Entry" as the default. "Start empty" remains available.
  • Pending rows (included but no value entered) are saved with market value 0, triggering zero-value warnings in the snapshot detail and list views to prompt the user to update them later.
  • New categories typed during bulk entry are created in the database only on save (deferred creation).

Jench2103 and others added 24 commits March 23, 2026 02:26
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…, and state tracking

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ng rows

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…port logic

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… list views

Add hasZeroValueAssets to SnapshotRowData and display warning triangle
icons for assets with marketValue == 0. Also add String overload for
helpWhenUnlocked to support table-based localized strings.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…picker in NewSnapshotSheet

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…import, and keyboard navigation

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add bulkEntry case to SidebarSection enum and wire up BulkEntryView in
ContentView's detail pane. Connect NewSnapshotSheet's onBulkEntry callback
through SnapshotListView to ContentView for proper navigation flow.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…bulk entry

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…CLAUDE.md

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…pshotDetailView

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ls to BulkEntryView

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Save dialog now mentions values can be filled later
- Prohibit entering 0 (zero-value assets should be excluded instead)
- Warning icons use hover popovers instead of system tooltips
- Sidebar "New Snapshot" shows date picker sheet; cancel navigates back
- CSV import surfaces errors, warnings, and success count to users
- NewSnapshotSheet cancel from sidebar navigates back to previous section

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Address multiple UX issues in the bulk entry workflow:

- Remove Cancel button from BulkEntryView; use Go back with discard
  confirmation instead (matching ImportView pattern)
- Detect platform/currency mismatches during CSV import and skip
  conflicting rows with grouped warning summaries
- Fix CSV import race condition where fileImporter binding cleared
  the target platform before the result handler ran
- Fix sidebar "New Snapshot" to show date picker sheet without
  navigating away, so Cancel is a clean no-op
- Restructure CSVImportResult with typed mismatch data for cleaner
  alert formatting
- Improve import result dialog with severity-based titles and
  grouped mismatch summaries
- Add 15 missing zh-Hant translations for new UI strings

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ing snapshots

Assets deferred during bulk snapshot creation are saved with
marketValue=0 as placeholders. Previously, ImportView flagged these
as duplicate errors, blocking users from updating them via CSV.

- Skip snapshotDuplicateError when existing asset has marketValue == 0
- Update existing zero-value SAV in place instead of creating a new one
- Non-zero existing values still trigger the duplicate error to prevent
  accidental double-counting

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Position the warning icon to the left of the total value so it
visually signals that the displayed value needs attention, rather
than appearing as an afterthought after the asset count badge.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…d design comments

Address code review findings:

- Add date conflict check in saveSnapshot() using predicate-based
  fetch with fetchLimit=1 to prevent duplicate snapshots from race
  conditions
- Show inline error label in toolbar when assets have zero values,
  explaining why Save is disabled
- Add comments documenting intentional design decisions: stable row
  IDs via asset.id and csvCategory being nil due to AssetCSVRow
  lacking a category field

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Allow users to add new assets and platforms directly in BulkEntryView,
making it self-sufficient even without prior snapshots. Previously, the
empty state was a dead end requiring CSV import.

- Add per-platform "Add Asset" button for inline row creation with
  editable name, currency picker, and category picker
- Add "Add Platform" toolbar button with popover for creating new
  platform groups
- Redesign empty state with "Add Platform" action instead of dead-end
  message
- Add CategoryNamePicker for new-asset rows (defers DB creation to save)
- Add duplicate asset name validation within platform (case-insensitive)
- Extend ValueSource with .manualNew; rename csvCategory to categoryName
- Update canSave/hasUnsavedChanges for new row validation
- Add 15 tests covering new ViewModel methods and edge cases
- Add zh-Hant translations for all new UI strings
- Update SPEC.md, BusinessLogic.md, and UserInterfaceDesign.md

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Improve SwiftUI view composition for better diffing performance and
code organization. The main BulkEntryView body previously used
@ViewBuilder functions for all subviews, causing unnecessary
re-evaluation on every parent state change.

- Extract AssetEntryRow, ProgressStats, ColumnHeaders, ValidationWarnings,
  and CategoryPicker into separate struct views in BulkEntrySubviews.swift
- Make @State viewModel private with explicit initializer
- Compute duplicateNameRowIDs once at body level instead of per platform
  section, avoiding repeated O(n) traversals
- Move formatImportResult to CSVImportResult.formattedResult() method
- Add .ultraThinMaterial toolbar background for Liquid Glass treatment

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The bulk entry feature and related UX changes need corresponding
user-facing documentation so users can discover and learn the new
snapshot creation workflow.

- Add dedicated Bulk Entry guide page (EN + zh-TW) covering the
  full-screen workflow, table layout, inline asset/platform creation,
  per-platform CSV import, validation, and zero-value warnings
- Update snapshots page to replace "Copy from Latest" with creation
  mode picker (Bulk Entry / Empty Snapshot) and mention zero-value
  warning icons
- Update quick-start page to reflect the new snapshot creation flow
- Update import-csv page with per-platform CSV import tip and
  zero-value overwrite behavior note
- Update index pages to include Bulk Entry in the guide topic table
- Update keyboard shortcuts with Enter key navigation in Bulk Entry
- Add 3 new screenshots (bulk-entry, add-platform, csv-import) and
  update 3 existing ones (snapshot-create, snapshot-list, snapshot-detail)
- Add Bulk Entry to mkdocs.yml nav under Guide

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…changes

HelpWhenUnlockedModifier used if/else branching between content.help(key)
and bare content, changing the view tree structure on the focused TextField
whenever hasZeroValueError toggled. This caused SwiftUI to reset
@focusstate, forcing users to re-click the field mid-typing.

- Make HelpWhenUnlockedModifier always return content.help(...) with an
  empty string fallback, keeping the view tree stable

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The "No Assets" empty state was pinned to the top-left because it lived
inside the ScrollView's LazyVStack. Move it outside the ScrollView so it
can fill the full area and center both horizontally and vertically.

Pin the add-platform popover arrow edge to `.bottom` so it always
appears above the toolbar button regardless of screen position.

- Move ContentUnavailableView out of ScrollView into a conditional branch
- Add .frame(maxWidth: .infinity, maxHeight: .infinity) for centering
- Set arrowEdge: .bottom on the toolbar popover
- Update bulk-entry-add-platform screenshot

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ange

Replace inline conditional warning labels with a single stable icon that
uses opacity to show/hide without affecting layout. Warning details are
displayed in a hover popover following the existing HoverWarningIcon
pattern with onHoverWhenUnlocked for lock-aware behavior.

- Collapse 0–3 conditional Label views into one fixed-size Image icon
- Use opacity(0/1) instead of if/else to reserve layout space
- Show all active warnings in a hover popover with lock-aware binding
- Reuse existing onHoverWhenUnlocked + popover pattern from HoverWarningIcon

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Refresh the app overview image to reflect the current UI after
bulk snapshot entry feature additions.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Jench2103 Jench2103 self-assigned this Mar 23, 2026
@Jench2103 Jench2103 added documentation Improvements or additions to documentation enhancement New feature or request ui / ux labels Mar 23, 2026
@Jench2103
Copy link
Owner Author

Code review

Found 4 issues:

  1. canSave does not guard against rows with validation errors -- invalid text silently saved as 0. A row with non-parseable text (e.g. "abc") has newValue == nil and shows a red border, but the user can still click Save. In saveSnapshot(), row.newValue ?? Decimal(0) silently converts the invalid input to 0. Fix: add && !rows.contains(where: { $0.isIncluded && $0.hasValidationError }) to canSave.

var hasDuplicateNames: Bool { !duplicateNameRowIDs.isEmpty }
var canSave: Bool {
includedCount > 0 && zeroValueCount == 0 && !hasInvalidNewRows && !hasDuplicateNames
}

  1. "Empty Snapshot" option always fails when triggered from sidebar. The sidebar "New Snapshot" click shows NewSnapshotSheet without passing a viewModel. The sheet still presents both "Empty Snapshot" and "Bulk Entry" radio buttons. Selecting "Empty Snapshot" hits guard let viewModel and shows an error alert. The option should be hidden or disabled when viewModel is nil.

}
guard let viewModel else {
errorMessage = String(
localized: "Cannot create an empty snapshot from this context.",
table: "Snapshot")
showError = true
return
}

.sheet(isPresented: $showBulkEntrySheet) {
NewSnapshotSheet(
onBulkEntry: { date in
bulkEntryViewModel = BulkEntryViewModel(
modelContext: modelContext, date: date)
pushHistory(.bulkEntry)
}
)
}

  1. Sidebar "New Snapshot" bypasses importCSV unsaved-changes guard. In sidebarBinding, the .bulkEntry check at line 259 returns early before the unsaved-changes guard at line 265. A user on the Import CSV screen with unsaved changes can click "New Snapshot" and silently lose their import data with no confirmation dialog.

// Bulk entry: show date picker sheet instead of navigating
if newSection == .bulkEntry {
showBulkEntrySheet = true
return
}
// Check if navigating away from section with unsaved changes
if selectedSection == .importCSV,
importViewModel?.hasUnsavedChanges == true
{
pendingSection = newSection
showDiscardConfirmation = true
} else if selectedSection == .bulkEntry,
bulkEntryViewModel?.hasUnsavedChanges == true
{
pendingSection = newSection
showDiscardConfirmation = true

  1. Stale documentation -- "Copy from latest" still referenced in UserInterfaceDesign.md (CLAUDE.md says "Before completing any task, review and update affected docs. UI/screen -> UserInterfaceDesign.md"). The New Snapshot Sheet section still documents the removed "Copy from latest" toggle. Both SPEC.md and BusinessLogic.md were correctly updated, but UserInterfaceDesign.md was not.

1. Starting point selector:
- **Start empty**: Creates snapshot with no asset entries
- **Copy from latest**: Pre-populates with all direct SnapshotAssetValues from the most recent prior snapshot. Disabled (grayed out with explanatory text) when no snapshots exist before the selected date.
- **Bulk Entry**: Opens `BulkEntryView` for full-screen bulk value entry. Disabled when no assets exist.
1. On creation, user is taken to the snapshot detail view for editing

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

Fix bugs, documentation gaps, and inconsistencies identified during
code review of the bulk snapshot entry feature.

- Add hasValidationError guard to canSave so rows with unparseable
  text (e.g. "abc") cannot be silently saved as Decimal(0)
- Fix HelpWhenUnlockedStringModifier to always emit .help() for
  stable view tree, matching the LocalizedStringKey overload
- Reorder sidebarBinding checks so unsaved-changes guards run
  before the .bulkEntry sheet trigger
- Enable empty snapshot creation from sidebar by using modelContext
  directly when SnapshotListViewModel is unavailable
- Truncate forward history on bulk entry discard to prevent
  goForward() from navigating to nil ViewModel
- Use normalizedForIdentity instead of lowercased() in addPlatform()
  for consistent identity comparison
- Anchor empty-state "Add Platform" popover to its own button
  instead of the toolbar button
- Add "Import Error" key to Snapshot.xcstrings with zh-Hant
  translation for consistency with sibling keys
- Remove unused "Cannot create an empty snapshot" localization string
- Document zero-value SAV placeholder design invariant in
  BusinessLogic.md and add explanatory code comments
- Update UserInterfaceDesign.md to remove stale "Copy from latest"
  and fix Bulk Entry description
- Update ContentView doc comment to reflect 8-section sidebar

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Jench2103
Copy link
Owner Author

Original review

All 4 reported issues (plus 8 additional issues found below the reporting threshold) have been addressed in be6f175.

Reported issues fixed:

  1. canSave now guards against hasValidationError rows
  2. "Empty Snapshot" works from sidebar (creates snapshot via modelContext directly)
  3. Sidebar unsaved-changes guard now runs before .bulkEntry sheet trigger
  4. UserInterfaceDesign.md updated — removed stale "Copy from latest", fixed Bulk Entry description

Additional fixes:

  • HelpWhenUnlockedStringModifier uses stable always-emit .help() pattern
  • addPlatform() uses normalizedForIdentity instead of .lowercased()
  • Added "Import Error" to Snapshot.xcstrings with zh-Hant translation
  • Documented zero-value SAV placeholder invariant in code comments and BusinessLogic.md
  • Forward history truncated on bulk entry discard to prevent nil ViewModel navigation
  • Empty-state "Add Platform" popover anchored to its own button
  • ContentView doc comment updated to 8-section sidebar
  • Removed unused localization string

🤖 Generated with Claude Code

@Jench2103 Jench2103 merged commit 50b9c8e into main Mar 23, 2026
1 check passed
@Jench2103 Jench2103 deleted the feat/bulk-snapshot-entry branch March 23, 2026 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation enhancement New feature or request ui / ux

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant