From 759ec1d96b48b103f907c90eb5e002d376f96611 Mon Sep 17 00:00:00 2001 From: Eoic Date: Sat, 1 Aug 2026 23:46:26 +0300 Subject: [PATCH 01/17] PPR-26: Define book import workflow rework --- ...8-01-book-import-workflow-rework-design.md | 127 ++++++++++++++++++ 1 file changed, 127 insertions(+) create mode 100644 docs/superpowers/specs/2026-08-01-book-import-workflow-rework-design.md diff --git a/docs/superpowers/specs/2026-08-01-book-import-workflow-rework-design.md b/docs/superpowers/specs/2026-08-01-book-import-workflow-rework-design.md new file mode 100644 index 0000000..62ac693 --- /dev/null +++ b/docs/superpowers/specs/2026-08-01-book-import-workflow-rework-design.md @@ -0,0 +1,127 @@ +# Book Import Workflow Rework Design + +## Overview + +Rework book import into distinct, consistently styled bottom sheets. Digital import becomes a confirmed multi-file workflow with per-book results, retry and removal controls, and one final library commit. Physical import adopts the same fixed-header and fixed-footer structure. + +## Problem + +The add-book method sheet currently replaces its own content with the digital import widget. This keeps the same modal route and mixes method selection, file selection, processing, preview, and commit state in one component. Digital import accepts only one file, and its actions scroll with the content. The physical form keeps its save action in the header rather than using the fixed footer established by Advanced filters. + +## Goals + +- Open digital and physical import as independent modal routes after the method sheet closes. +- Let users select and confirm multiple digital files in one operation. +- Present processing and commit results as removable book rows with explicit statuses. +- Support retrying failed files without reopening the workflow. +- Give digital selection, import results, and physical entry fixed headers and footers. +- Preserve the existing import, metadata extraction, storage, and commit services. + +## Non-goals + +- Background imports that survive navigation or application restarts. +- Import history or persistent import queues. +- Editing extracted digital-book metadata before commit. +- Changing supported file formats or the underlying metadata parsers. +- Adding online acquisition behavior to this workflow. + +## Component Design + +### AddBookChoiceSheet + +The method sheet remains selection-only. Its result enum gains a digital-import choice. After the choice route has fully completed, the caller opens either `DigitalBookImportSheet`, `AddPhysicalBookSheet`, or online search. The method sheet never swaps its own body. + +### AddBookSheetScaffold + +The three add-book sheets share a small layout component that renders: + +- a fixed drag handle and title/close header; +- a divider; +- one expanded scrollable body supplied by the sheet; +- a fixed footer with a top border and safe-area handling. + +Spacing, surface color, border treatment, and action placement match `LibraryAdvancedFilterSheet`. The shared component defines layout only; each sheet owns its actions and state. + +### DigitalBookImportSheet + +This sheet owns only file selection and confirmation. + +- The file picker uses multi-selection and reads file bytes. +- Supported extensions remain platform-specific: EPUB on web and the existing native format list elsewhere. +- The body initially presents a browse action, then a scrollable filename list. +- Every selected row can be removed before processing. +- Reopening the picker replaces the current selection. +- The footer contains Cancel and `Import N books`. +- The primary action is disabled until at least one readable file remains. + +Confirming closes this sheet and immediately opens `BookImportResultsSheet` with the selected files. + +### BookImportResultsSheet + +The results sheet starts processing when it opens. Its body is a scrollable list of one row per selected file. Each row includes the filename, extracted title and author when available, status, and contextual actions. + +Processing statuses are: + +- queued; +- processing; +- ready; +- failed. + +Ready rows may be removed before the final action. Failed rows provide Retry and Remove. A processing retry uses the original in-memory bytes and replaces the row’s prior error state. + +The fixed footer contains Cancel and `Add N to library`. The primary action is enabled only when processing has settled and at least one ready row remains. Its count includes only ready rows. + +During final commit, row actions and sheet dismissal are disabled. Commit states are adding, added, and failed. Ready rows are committed individually through `BookImportCommitService`. If every retained row is added, the sheet closes and reports the total added. If a commit fails, successfully added rows remain final, failed rows remain visible, and the sheet stays open so the user can retry that row’s commit or remove it without duplicating successful books. + +### AddPhysicalBookSheet + +The existing form and validation remain unchanged. The form becomes the scrollable body of the shared sheet scaffold. The fixed header contains the handle, title, and close action. The fixed footer contains Cancel and Add; Add uses the existing validation and save behavior. + +## Batch State + +Each selected file becomes a workflow-local immutable batch item containing: + +- a stable item ID; +- filename and optional bytes, allowing unreadable picker results to become failed rows; +- current processing or commit status; +- optional `BookImportResult`; +- optional user-safe error message. + +The workflow remains local to the results sheet and does not introduce provider-level or application-global state. Items process independently so the UI updates as each file finishes. + +## Cleanup and Dismissal + +- Removing a ready row deletes the temporary imported book file created by `BookImportService`. +- Retrying metadata processing starts from the original bytes; retrying a commit reuses its existing parsed result and temporary file. +- Cancelling, closing, or dismissing the results sheet deletes every successful-but-uncommitted temporary file. +- Added rows are never cleaned by sheet dismissal. +- Dismissal is blocked only while final commits are running. +- Failed parsing rows have no committed library record and retain their source bytes only until the sheet closes. + +## Error Handling + +File-read failures appear as failed rows rather than aborting the batch. Processing and commit errors are isolated to their rows. One failure never prevents other files from becoming ready or being added. Raw internal exceptions are converted to concise user-facing messages while remaining available to existing logging where applicable. + +## Verification + +Widget and model tests will verify: + +- digital selection opens on a new modal route after the method sheet dismisses; +- multi-file selection, confirmation counts, and pre-processing removal; +- fixed header, scrolling body, and fixed footer structure for all three sheets; +- independent queued, processing, ready, and failed states; +- retry and removal behavior; +- cleanup of successful-but-uncommitted temporary files; +- final commit of only retained ready rows; +- partial commit failure without duplicate additions; +- physical form validation and Add behavior from the footer. + +Run targeted Flutter tests during implementation, followed by `flutter analyze` and the complete Flutter test suite. + +## Assumptions + +- Selected file bytes may remain in memory for the lifetime of the results sheet. +- Processing can run concurrently through the existing import service. +- A fresh multi-file picker result replaces the digital selection draft. +- The final action adds every retained ready row; per-row inclusion is controlled through Remove. +- The current supported-format lists remain the source of truth. From 3eeddef2e2e2af915081304051f3a19da991f1d7 Mon Sep 17 00:00:00 2001 From: Eoic Date: Sat, 1 Aug 2026 23:53:50 +0300 Subject: [PATCH 02/17] PPR-26: Plan book import workflow rework --- .../2026-08-01-book-import-workflow-rework.md | 805 ++++++++++++++++++ 1 file changed, 805 insertions(+) create mode 100644 docs/superpowers/plans/2026-08-01-book-import-workflow-rework.md diff --git a/docs/superpowers/plans/2026-08-01-book-import-workflow-rework.md b/docs/superpowers/plans/2026-08-01-book-import-workflow-rework.md new file mode 100644 index 0000000..cd59901 --- /dev/null +++ b/docs/superpowers/plans/2026-08-01-book-import-workflow-rework.md @@ -0,0 +1,805 @@ +# Book Import Workflow Rework Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Replace the single-file, in-place digital import flow with independent fixed-layout selection and results sheets that support confirmed batch imports, retry, removal, cleanup, and partial commit failures, while moving physical import actions into a fixed footer. + +**Architecture:** `AddBookChoiceSheet` returns a method choice and opens a new route only after the choice route completes. A workflow-local immutable batch item models every selected file and its processing or commit state. Three add-book sheets share a layout-only scaffold with a fixed header, expanded body, and fixed footer; the results sheet injects processing, deletion, and commit callbacks for deterministic widget tests while production callbacks use the existing import and commit services. + +**Tech Stack:** Flutter, Dart, Provider, `file_picker`, existing `BookImportService` and `BookImportCommitService`, `flutter_test`. + +--- + +## File Structure + +- Create `app/lib/widgets/add_book/book_import_batch_item.dart` — selected-file value and immutable per-row state transitions. +- Create `app/lib/widgets/add_book/add_book_sheet_scaffold.dart` — fixed handle/header, expanded body, and fixed safe-area footer. +- Create `app/lib/widgets/add_book/digital_book_import_sheet.dart` — multi-file selection, confirmation, and pre-processing removal. +- Create `app/lib/widgets/add_book/book_import_results_sheet.dart` — processing, retry, removal, cleanup, commit, and results UI. +- Modify `app/lib/widgets/add_book/add_book_choice_sheet.dart` — selection-only routing. +- Modify `app/lib/widgets/add_book/add_physical_book_sheet.dart` — shared fixed layout and footer actions. +- Delete `app/lib/widgets/add_book/import_book_sheet.dart` after all production references move. +- Create `app/test/widgets/add_book/book_import_batch_item_test.dart`. +- Create `app/test/widgets/add_book/add_book_sheet_scaffold_test.dart`. +- Create `app/test/widgets/add_book/digital_book_import_sheet_test.dart`. +- Create `app/test/widgets/add_book/book_import_results_sheet_test.dart`. +- Modify `app/test/widgets/add_book/add_book_sheets_test.dart` — method routing and physical-sheet integration. +- Modify `app/test/media/media_profile_switch_contract_test.dart` — point commit-boundary contracts at the results sheet. + +### Task 1: Model Batch Files and Row State + +**Files:** +- Create: `app/lib/widgets/add_book/book_import_batch_item.dart` +- Test: `app/test/widgets/add_book/book_import_batch_item_test.dart` + +- [ ] **Step 1: Write failing transition tests** + +```dart +import 'dart:typed_data'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:papyrus/services/book_import_result.dart'; +import 'package:papyrus/widgets/add_book/book_import_batch_item.dart'; + +void main() { + const result = BookImportResult( + bookId: 'book-1', + title: 'Frankenstein', + author: 'Mary Shelley', + fileSize: 4, + fileHash: 'hash', + fileExtension: 'epub', + ); + + test('processing transitions preserve identity and clear stale errors', () { + final file = SelectedBookFile(name: 'book.epub', bytes: Uint8List.fromList([1, 2, 3, 4])); + final failed = BookImportBatchItem.queued(id: 'row-1', file: file) + .startProcessing() + .processingFailed('Could not process this file.'); + final ready = failed.startProcessing().processingSucceeded(result); + + expect(ready.id, 'row-1'); + expect(ready.status, BookImportBatchStatus.ready); + expect(ready.result, same(result)); + expect(ready.errorMessage, isNull); + }); + + test('commit failure keeps the parsed result for retry', () { + final file = SelectedBookFile(name: 'book.epub', bytes: Uint8List.fromList([1])); + final failed = BookImportBatchItem.queued(id: 'row-1', file: file) + .startProcessing() + .processingSucceeded(result) + .startAdding() + .commitFailed('Could not add this book.'); + + expect(failed.status, BookImportBatchStatus.commitFailed); + expect(failed.result, same(result)); + expect(failed.canRetry, isTrue); + }); +} +``` + +- [ ] **Step 2: Run the model test and verify RED** + +Run: `cd app && flutter test test/widgets/add_book/book_import_batch_item_test.dart` + +Expected: compilation fails because `book_import_batch_item.dart` and its types do not exist. + +- [ ] **Step 3: Implement the immutable batch types** + +```dart +import 'dart:typed_data'; + +import 'package:flutter/foundation.dart'; +import 'package:papyrus/services/book_import_result.dart'; + +@immutable +class SelectedBookFile { + const SelectedBookFile({required this.name, required this.bytes}); + + final String name; + final Uint8List? bytes; +} + +enum BookImportBatchStatus { + queued, + processing, + ready, + processingFailed, + adding, + added, + commitFailed, +} + +@immutable +class BookImportBatchItem { + const BookImportBatchItem._({ + required this.id, + required this.file, + required this.status, + this.result, + this.errorMessage, + }); + + factory BookImportBatchItem.queued({required String id, required SelectedBookFile file}) { + return BookImportBatchItem._(id: id, file: file, status: BookImportBatchStatus.queued); + } + + final String id; + final SelectedBookFile file; + final BookImportBatchStatus status; + final BookImportResult? result; + final String? errorMessage; + + bool get canRetry => + status == BookImportBatchStatus.processingFailed || status == BookImportBatchStatus.commitFailed; + bool get isSettled => status != BookImportBatchStatus.queued && status != BookImportBatchStatus.processing; + bool get hasTemporaryFile => result != null && status != BookImportBatchStatus.added; + + BookImportBatchItem startProcessing() => BookImportBatchItem._( + id: id, + file: file, + status: BookImportBatchStatus.processing, + ); + + BookImportBatchItem processingSucceeded(BookImportResult value) => BookImportBatchItem._( + id: id, + file: file, + status: BookImportBatchStatus.ready, + result: value, + ); + + BookImportBatchItem processingFailed(String message) => BookImportBatchItem._( + id: id, + file: file, + status: BookImportBatchStatus.processingFailed, + errorMessage: message, + ); + + BookImportBatchItem startAdding() => BookImportBatchItem._( + id: id, + file: file, + status: BookImportBatchStatus.adding, + result: result, + ); + + BookImportBatchItem added() => BookImportBatchItem._( + id: id, + file: file, + status: BookImportBatchStatus.added, + result: result, + ); + + BookImportBatchItem commitFailed(String message) => BookImportBatchItem._( + id: id, + file: file, + status: BookImportBatchStatus.commitFailed, + result: result, + errorMessage: message, + ); +} +``` + +- [ ] **Step 4: Run the model test and verify GREEN** + +Run: `cd app && flutter test test/widgets/add_book/book_import_batch_item_test.dart` + +Expected: all batch-item tests pass. + +- [ ] **Step 5: Commit the model** + +```bash +git add app/lib/widgets/add_book/book_import_batch_item.dart app/test/widgets/add_book/book_import_batch_item_test.dart +git commit -m "PPR-26: Model batch book imports" +``` + +### Task 2: Add the Fixed Add-Book Sheet Layout and Migrate Physical Entry + +**Files:** +- Create: `app/lib/widgets/add_book/add_book_sheet_scaffold.dart` +- Create: `app/test/widgets/add_book/add_book_sheet_scaffold_test.dart` +- Modify: `app/lib/widgets/add_book/add_physical_book_sheet.dart` +- Modify: `app/test/widgets/add_book/add_book_sheets_test.dart` + +- [ ] **Step 1: Write failing fixed-layout tests** + +```dart +Future openPhysicalBookSheet(WidgetTester tester) async { + await tester.pumpWidget( + MaterialApp( + home: Builder( + builder: (context) => Scaffold( + body: FilledButton( + onPressed: () => AddPhysicalBookSheet.show(context), + child: const Text('Open physical import'), + ), + ), + ), + ), + ); + await tester.tap(find.text('Open physical import')); + await tester.pumpAndSettle(); +} + +testWidgets('header and footer remain fixed while the body scrolls', (tester) async { + final controller = ScrollController(); + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: SizedBox( + height: 500, + child: AddBookSheetScaffold( + title: 'Import books', + onClose: () {}, + body: ListView( + controller: controller, + children: List.generate(40, (index) => Text('Row $index')), + ), + footer: const Text('Fixed footer', key: Key('fixed-footer')), + ), + ), + ), + ), + ); + + final headerTop = tester.getTopLeft(find.text('Import books')); + final footerTop = tester.getTopLeft(find.byKey(const Key('fixed-footer'))); + await tester.drag(find.byType(ListView), const Offset(0, -600)); + await tester.pump(); + + expect(tester.getTopLeft(find.text('Import books')), headerTop); + expect(tester.getTopLeft(find.byKey(const Key('fixed-footer'))), footerTop); +}); + +testWidgets('physical Add action is rendered in the footer', (tester) async { + await openPhysicalBookSheet(tester); + + expect(find.byKey(const Key('add-book-sheet-header')), findsOneWidget); + expect(find.byKey(const Key('add-book-sheet-footer')), findsOneWidget); + expect( + find.descendant( + of: find.byKey(const Key('add-book-sheet-footer')), + matching: find.widgetWithText(FilledButton, 'Add'), + ), + findsOneWidget, + ); +}); +``` + +- [ ] **Step 2: Run the scaffold and add-book sheet tests and verify RED** + +Run: `cd app && flutter test test/widgets/add_book/add_book_sheet_scaffold_test.dart test/widgets/add_book/add_book_sheets_test.dart` + +Expected: the scaffold type and fixed-footer keys are missing, and the physical Add action is still in `BottomSheetHeader`. + +- [ ] **Step 3: Implement the shared layout** + +Create `AddBookSheetScaffold` with this public interface and structure: + +```dart +class AddBookSheetScaffold extends StatelessWidget { + const AddBookSheetScaffold({ + super.key, + required this.title, + required this.onClose, + required this.body, + required this.footer, + this.canClose = true, + }); + + final String title; + final VoidCallback onClose; + final Widget body; + final Widget footer; + final bool canClose; + + @override + Widget build(BuildContext context) { + final colorScheme = Theme.of(context).colorScheme; + return Column( + children: [ + Padding( + key: const Key('add-book-sheet-header'), + padding: const EdgeInsets.fromLTRB(Spacing.lg, Spacing.md, Spacing.lg, Spacing.md), + child: Column( + children: [ + const BottomSheetHandle(), + const SizedBox(height: Spacing.lg), + Row( + children: [ + Expanded(child: Text(title, style: Theme.of(context).textTheme.headlineSmall)), + IconButton( + icon: const Icon(Icons.close), + tooltip: 'Close', + onPressed: canClose ? onClose : null, + ), + ], + ), + ], + ), + ), + const Divider(height: 1), + Expanded(child: body), + Container( + key: const Key('add-book-sheet-footer'), + padding: const EdgeInsets.symmetric(horizontal: Spacing.lg, vertical: Spacing.md), + decoration: BoxDecoration( + color: colorScheme.surface, + border: Border(top: BorderSide(color: colorScheme.outlineVariant)), + ), + child: SafeArea(top: false, child: footer), + ), + ], + ); + } +} +``` + +- [ ] **Step 4: Move physical actions into the footer** + +Replace the physical sheet’s top `BottomSheetHeader` and trailing body structure with `AddBookSheetScaffold`. Supply the existing form `ListView` as `body` and this footer: + +```dart +Row( + children: [ + const Spacer(), + TextButton(onPressed: () => Navigator.of(context).pop(), child: const Text('Cancel')), + const SizedBox(width: Spacing.sm), + FilledButton(onPressed: _canSave ? _onSave : null, child: const Text('Add')), + ], +) +``` + +Keep `MediaQuery.viewInsets.bottom` around the scaffold so the keyboard does not cover the footer. + +- [ ] **Step 5: Run the focused tests and verify GREEN** + +Run: `cd app && flutter test test/widgets/add_book/add_book_sheet_scaffold_test.dart test/widgets/add_book/add_book_sheets_test.dart` + +Expected: fixed-layout and physical-footer tests pass. + +- [ ] **Step 6: Commit the shared layout and physical migration** + +```bash +git add app/lib/widgets/add_book/add_book_sheet_scaffold.dart app/lib/widgets/add_book/add_physical_book_sheet.dart app/test/widgets/add_book/add_book_sheet_scaffold_test.dart app/test/widgets/add_book/add_book_sheets_test.dart +git commit -m "PPR-26: Fix physical import sheet actions" +``` + +### Task 3: Build the Confirmed Multi-File Selection Sheet + +**Files:** +- Create: `app/lib/widgets/add_book/digital_book_import_sheet.dart` +- Test: `app/test/widgets/add_book/digital_book_import_sheet_test.dart` + +- [ ] **Step 1: Write failing selection and removal tests** + +```dart +testWidgets('confirms multiple selected files and removes accidental selections', (tester) async { + final files = [ + SelectedBookFile(name: 'one.epub', bytes: Uint8List.fromList([1])), + SelectedBookFile(name: 'two.epub', bytes: Uint8List.fromList([2])), + ]; + List? confirmed; + + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: SizedBox( + height: 600, + child: DigitalBookImportSheet( + pickFiles: () async => files, + onConfirm: (value) => confirmed = value, + onCancel: () {}, + ), + ), + ), + ), + ); + + await tester.tap(find.text('Browse files')); + await tester.pump(); + expect(find.text('one.epub'), findsOneWidget); + expect(find.text('two.epub'), findsOneWidget); + expect(find.text('Import 2 books'), findsOneWidget); + + await tester.tap(find.byKey(const ValueKey('remove-two.epub'))); + await tester.pump(); + await tester.tap(find.text('Import 1 book')); + + expect(confirmed!.map((file) => file.name), ['one.epub']); + expect(find.byKey(const Key('add-book-sheet-header')), findsOneWidget); + expect(find.byKey(const Key('add-book-sheet-footer')), findsOneWidget); +}); + +testWidgets('a fresh picker result replaces the selection and unreadable files cannot confirm alone', (tester) async { + var pickCount = 0; + List? confirmed; + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: SizedBox( + height: 600, + child: DigitalBookImportSheet( + pickFiles: () async { + pickCount++; + return pickCount == 1 + ? [SelectedBookFile(name: 'first.epub', bytes: Uint8List.fromList([1]))] + : const [SelectedBookFile(name: 'unreadable.epub', bytes: null)]; + }, + onConfirm: (value) => confirmed = value, + onCancel: () {}, + ), + ), + ), + ), + ); + + await tester.tap(find.text('Browse files')); + await tester.pump(); + await tester.tap(find.text('Browse files')); + await tester.pump(); + + expect(find.text('first.epub'), findsNothing); + expect(find.text('unreadable.epub'), findsOneWidget); + final button = tester.widget(find.widgetWithText(FilledButton, 'Import 1 book')); + expect(button.onPressed, isNull); + expect(confirmed, isNull); +}); +``` + +- [ ] **Step 2: Run the digital sheet test and verify RED** + +Run: `cd app && flutter test test/widgets/add_book/digital_book_import_sheet_test.dart` + +Expected: compilation fails because `DigitalBookImportSheet` does not exist. + +- [ ] **Step 3: Implement the picker adapter and sheet** + +Define: + +```dart +typedef DigitalBookFilePicker = Future> Function(); + +Future> pickDigitalBookFiles() async { + final extensions = kIsWeb ? const ['epub'] : const ['epub', 'pdf', 'mobi', 'azw3', 'txt', 'cbr', 'cbz']; + final result = await FilePicker.platform.pickFiles( + type: FileType.custom, + allowedExtensions: extensions, + allowMultiple: true, + withData: true, + ); + if (result == null) return const []; + return [for (final file in result.files) SelectedBookFile(name: file.name, bytes: file.bytes)]; +} +``` + +Give `DigitalBookImportSheet` the testable constructor used above and a production `show` method that wraps it in a `DraggableScrollableSheet`. Use `AddBookSheetScaffold`, a `ListView` body, keyed remove buttons, and a footer containing Cancel plus a pluralized `Import N book(s)` button. Replacing the selection after each non-empty picker result must be one `setState` call. + +- [ ] **Step 4: Run the selection tests and verify GREEN** + +Run: `cd app && flutter test test/widgets/add_book/digital_book_import_sheet_test.dart` + +Expected: multi-selection, replacement, removal, unreadable-file display, and confirmation tests pass. + +- [ ] **Step 5: Commit the digital selection sheet** + +```bash +git add app/lib/widgets/add_book/digital_book_import_sheet.dart app/test/widgets/add_book/digital_book_import_sheet_test.dart +git commit -m "PPR-26: Add batch digital import selection" +``` + +### Task 4: Process, Route, Retry, Remove, and Clean Batch Results + +**Files:** +- Create: `app/lib/widgets/add_book/book_import_results_sheet.dart` +- Create: `app/test/widgets/add_book/book_import_results_sheet_test.dart` +- Modify: `app/lib/widgets/add_book/add_book_choice_sheet.dart` +- Modify: `app/test/widgets/add_book/add_book_sheets_test.dart` + +- [ ] **Step 1: Write failing independent-result tests** + +```dart +BookImportResult importResult(String filename, {required String bookId}) { + return BookImportResult( + bookId: bookId, + title: filename, + author: 'Author', + fileSize: 1, + fileHash: 'hash-$bookId', + fileExtension: 'epub', + ); +} + +Future pumpResultsSheet( + WidgetTester tester, { + required List files, + required BookImportProcessor processBook, + required ImportedBookFileDeleter deleteBookFile, + ImportedBookCommitter? commitBook, +}) async { + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: SizedBox( + height: 700, + child: BookImportResultsSheet( + files: files, + processBook: processBook, + deleteBookFile: deleteBookFile, + commitBook: commitBook ?? + (result, _) async => Book( + id: result.bookId, + title: result.title, + author: result.author, + addedAt: DateTime(2026), + ), + onClose: () {}, + onCompleted: (_) {}, + ), + ), + ), + ), + ); +} + +testWidgets('processes rows independently and retries only the failed row', (tester) async { + var failingAttempts = 0; + final files = [ + SelectedBookFile(name: 'good.epub', bytes: Uint8List.fromList([1])), + SelectedBookFile(name: 'bad.epub', bytes: Uint8List.fromList([2])), + ]; + + Future process(Uint8List bytes, String filename) async { + if (filename == 'bad.epub' && failingAttempts++ == 0) throw StateError('broken'); + return importResult(filename, bookId: filename); + } + + final deleted = []; + await pumpResultsSheet( + tester, + files: files, + processBook: process, + deleteBookFile: (bookId) async => deleted.add(bookId), + ); + await tester.pumpAndSettle(); + + expect(find.text('Ready'), findsOneWidget); + expect(find.text('Failed'), findsOneWidget); + expect(find.byKey(const Key('add-book-sheet-header')), findsOneWidget); + expect(find.byKey(const Key('add-book-sheet-footer')), findsOneWidget); + await tester.tap(find.byKey(const ValueKey('retry-bad.epub'))); + await tester.pumpAndSettle(); + expect(find.text('Ready'), findsNWidgets(2)); + + await tester.tap(find.byKey(const ValueKey('remove-good.epub'))); + await tester.pump(); + expect(deleted, contains('good.epub')); +}); + +testWidgets('digital import dismisses the method sheet before opening its own route', (tester) async { + final observer = CountingNavigatorObserver(); + await pumpLauncher(tester, (context) => () => AddBookChoiceSheet.show(context), navigatorObservers: [observer]); + await tester.tap(find.text('Open')); + await tester.pumpAndSettle(); + + final barrier = find.byWidgetPredicate((widget) => widget is ModalBarrier && widget.color != null); + final firstBarrier = tester.element(barrier); + final pushesBeforeChoice = observer.pushCount; + + await tester.tap(find.text('Import digital books')); + await tester.pumpAndSettle(); + + expect(find.text('Import digital books'), findsWidgets); + expect(observer.pushCount, pushesBeforeChoice + 1); + expect(tester.element(barrier), isNot(same(firstBarrier))); +}); +``` + +- [ ] **Step 2: Run the results test and verify RED** + +Run: `cd app && flutter test test/widgets/add_book/book_import_results_sheet_test.dart` + +Expected: the results sheet, callback typedefs, and status rows are missing. + +- [ ] **Step 3: Implement processing and row actions** + +Define these injectable callbacks: + +```dart +typedef BookImportProcessor = Future Function(Uint8List bytes, String filename); +typedef ImportedBookFileDeleter = Future Function(String bookId); +typedef ImportedBookCommitter = Future Function(BookImportResult result, String sourceFilename); +``` + +`BookImportResultsSheet.show` must resolve `BookImportService` from the caller’s provider and pass `importBook` and `deleteBookFile` into the sheet. In `initState`, create queued items with stable IDs and schedule `_processAll`. `_processItem` must: + +1. mark only that row processing; +2. turn null bytes into a user-safe processing failure; +3. await the injected processor; +4. delete a late successful result immediately when the sheet has started closing; +5. otherwise mark the row ready or failed. + +Use `AddBookSheetScaffold`, `PopScope`, a `ListView.separated`, and keyed Retry/Remove controls. `_removeItem` must await temporary-file deletion before removing a ready or commit-failed row. `_requestClose` must mark the sheet closing, clean every uncommitted result, allow pop, and then pop exactly once. + +Remove `_showImport` from `AddBookChoiceSheet`, add `_AddBookChoice.importDigital`, and route after the method sheet has completed: + +```dart +case _AddBookChoice.importDigital: + final files = await DigitalBookImportSheet.show(context); + if (!context.mounted || files == null || files.isEmpty) return; + await BookImportResultsSheet.show(context, files: files); +case _AddBookChoice.addPhysical: + await AddPhysicalBookSheet.show(context); +case _AddBookChoice.findOnline: + onFindOnline?.call(); +``` + +- [ ] **Step 4: Run processing, retry, removal, and cleanup tests and verify GREEN** + +Run: `cd app && flutter test test/widgets/add_book/book_import_results_sheet_test.dart` + +Expected: independent state, processing retry, row removal, late-result cleanup, and close cleanup tests pass. + +- [ ] **Step 5: Run the method-routing test and verify GREEN** + +Run: `cd app && flutter test test/widgets/add_book/add_book_sheets_test.dart` + +Expected: digital and physical options both dismiss the method sheet and open distinct routes. + +- [ ] **Step 6: Commit routing and result processing** + +```bash +git add app/lib/widgets/add_book/add_book_choice_sheet.dart app/lib/widgets/add_book/book_import_results_sheet.dart app/test/widgets/add_book/add_book_sheets_test.dart app/test/widgets/add_book/book_import_results_sheet_test.dart +git commit -m "PPR-26: Add batch import results" +``` + +### Task 5: Commit Ready Books and Preserve Partial Failures + +**Files:** +- Modify: `app/lib/widgets/add_book/book_import_results_sheet.dart` +- Modify: `app/test/widgets/add_book/book_import_results_sheet_test.dart` +- Modify: `app/test/media/media_profile_switch_contract_test.dart` + +- [ ] **Step 1: Write failing batch-commit tests** + +```dart +testWidgets('partial commit failure never recommits successful rows', (tester) async { + final commits = []; + var secondAttempts = 0; + final results = { + 'one.epub': importResult('One', bookId: 'one'), + 'two.epub': importResult('Two', bookId: 'two'), + }; + await pumpResultsSheet( + tester, + files: [ + SelectedBookFile(name: 'one.epub', bytes: Uint8List.fromList([1])), + SelectedBookFile(name: 'two.epub', bytes: Uint8List.fromList([2])), + ], + processBook: (_, filename) async => results[filename]!, + deleteBookFile: (_) async {}, + commitBook: (result, _) async { + commits.add(result.bookId); + if (result.bookId == 'two' && secondAttempts++ == 0) throw StateError('commit failed'); + return Book(id: result.bookId, title: result.title, author: result.author, addedAt: DateTime(2026)); + }, + ); + + await tester.tap(find.text('Add 2 to library')); + await tester.pumpAndSettle(); + expect(commits, ['one', 'two']); + expect(find.text('Added'), findsOneWidget); + expect(find.text('Failed'), findsOneWidget); + + await tester.tap(find.byKey(const ValueKey('retry-two.epub'))); + await tester.pumpAndSettle(); + expect(commits, ['one', 'two', 'two']); +}); +``` + +- [ ] **Step 2: Run the commit tests and verify RED** + +Run: `cd app && flutter test test/widgets/add_book/book_import_results_sheet_test.dart --plain-name "partial commit failure never recommits successful rows"` + +Expected: the footer does not commit multiple ready rows or preserve per-row commit state. + +- [ ] **Step 3: Move the production commit boundary into the results sheet** + +Create `_commitResult(BookImportResult result, String sourceFilename)` by moving the current dependency resolution and `BookImportCommitService.commit` setup out of `ImportBookSheet._addToLibrary`. Preserve: + +- repository capture through `requireBookRepository()`; +- account scope validation; +- pending and guest cover callbacks; +- repository add/delete compensation callbacks; +- upload queue callback; +- library-context validation; +- web OPFS and native local-file path behavior. + +Implement `_addReadyBooks` so it snapshots only ready row IDs, marks them adding, commits each once, and updates each row to added or commit-failed. Disable close, remove, retry, and footer actions while any row is adding. A commit retry calls the committer only for that row. Close with an `Added N books to library` snackbar only when no retained ready, processing-failed, or commit-failed rows remain. + +- [ ] **Step 4: Update the source contract to the new commit boundary** + +Change `media_profile_switch_contract_test.dart` to read `lib/widgets/add_book/book_import_results_sheet.dart`, extract `_commitResult`, and retain its existing assertions for account scope, cover persistence, queueing, repository identity, and context validation. Replace the old single `_committing` source assertions with widget tests that prove actions are disabled during injected commit futures. + +- [ ] **Step 5: Run commit, service, and contract tests and verify GREEN** + +Run: + +```bash +cd app +flutter test test/widgets/add_book/book_import_results_sheet_test.dart test/services/book_import_commit_service_test.dart test/media/media_profile_switch_contract_test.dart +``` + +Expected: batch commits, partial failure retry, no duplicate additions, and the existing media-profile safety contracts pass. + +- [ ] **Step 6: Commit batch finalization** + +```bash +git add app/lib/widgets/add_book/book_import_results_sheet.dart app/test/widgets/add_book/book_import_results_sheet_test.dart app/test/media/media_profile_switch_contract_test.dart +git commit -m "PPR-26: Commit batch book imports" +``` + +### Task 6: Remove the Legacy Combined Sheet and Verify the Feature + +**Files:** +- Delete: `app/lib/widgets/add_book/import_book_sheet.dart` +- Modify: `app/test/widgets/add_book/add_book_sheets_test.dart` +- Inspect: all `app/lib` and `app/test` Dart files for stale imports and symbols. + +- [ ] **Step 1: Run the focused tests before deleting the legacy sheet** + +Run: + +```bash +cd app +flutter test test/widgets/add_book/add_book_sheet_scaffold_test.dart test/widgets/add_book/digital_book_import_sheet_test.dart test/widgets/add_book/book_import_results_sheet_test.dart test/widgets/add_book/add_book_sheets_test.dart +``` + +Expected: the new workflow passes while the unused legacy file still exists. + +- [ ] **Step 2: Delete the old sheet and remove stale references** + +Delete `app/lib/widgets/add_book/import_book_sheet.dart`. Run: + +```bash +rg -n "ImportBookSheet|import_book_sheet|_showImport" app/lib app/test +``` + +Expected: no matches. Update any remaining import or source-contract path to the new digital or results component rather than retaining compatibility aliases. + +- [ ] **Step 3: Format and analyze** + +Run: + +```bash +cd app +dart format --set-exit-if-changed lib test +flutter analyze --no-fatal-warnings --no-fatal-infos +``` + +Expected: formatting makes no changes and analysis reports no issues. + +- [ ] **Step 4: Run the complete test suite** + +Run: `cd app && flutter test --reporter expanded` + +Expected: every test passes; intentional skips remain skipped. + +- [ ] **Step 5: Review the final diff** + +Run: + +```bash +git diff --check +git status --short +git diff --stat HEAD~6..HEAD +``` + +Expected: no whitespace errors, only PPR-26 import workflow files are changed, and no generated file is present. + +- [ ] **Step 6: Commit final cleanup** + +```bash +git add -A app/lib/widgets/add_book app/test/widgets/add_book app/test/media/media_profile_switch_contract_test.dart +git commit -m "PPR-26: Remove legacy book import sheet" +``` From 927d5b617625c82eeac58d5469aa527d42afc8b6 Mon Sep 17 00:00:00 2001 From: Eoic Date: Sat, 1 Aug 2026 23:57:39 +0300 Subject: [PATCH 03/17] PPR-26: Model batch book imports --- .../add_book/book_import_batch_item.dart | 61 ++++++++++++++++ .../add_book/book_import_batch_item_test.dart | 73 +++++++++++++++++++ 2 files changed, 134 insertions(+) create mode 100644 app/lib/widgets/add_book/book_import_batch_item.dart create mode 100644 app/test/widgets/add_book/book_import_batch_item_test.dart diff --git a/app/lib/widgets/add_book/book_import_batch_item.dart b/app/lib/widgets/add_book/book_import_batch_item.dart new file mode 100644 index 0000000..bbb69fa --- /dev/null +++ b/app/lib/widgets/add_book/book_import_batch_item.dart @@ -0,0 +1,61 @@ +import 'dart:typed_data'; + +import 'package:papyrus/services/book_import_result.dart'; + +class SelectedBookFile { + const SelectedBookFile({required this.name, required this.bytes}); + + final String name; + final Uint8List? bytes; +} + +enum BookImportBatchStatus { queued, processing, ready, processingFailed, adding, added, commitFailed } + +class BookImportBatchItem { + const BookImportBatchItem._({ + required this.id, + required this.file, + required this.status, + this.result, + this.errorMessage, + }); + + factory BookImportBatchItem.queued({required String id, required SelectedBookFile file}) { + return BookImportBatchItem._(id: id, file: file, status: BookImportBatchStatus.queued); + } + + final String id; + final SelectedBookFile file; + final BookImportBatchStatus status; + final BookImportResult? result; + final String? errorMessage; + + bool get canRetry => status == BookImportBatchStatus.processingFailed || status == BookImportBatchStatus.commitFailed; + + bool get isSettled => status != BookImportBatchStatus.queued && status != BookImportBatchStatus.processing; + + bool get hasTemporaryFile => result != null && status != BookImportBatchStatus.added; + + BookImportBatchItem startProcessing() => + BookImportBatchItem._(id: id, file: file, status: BookImportBatchStatus.processing); + + BookImportBatchItem processingSucceeded(BookImportResult value) => + BookImportBatchItem._(id: id, file: file, status: BookImportBatchStatus.ready, result: value); + + BookImportBatchItem processingFailed(String message) => + BookImportBatchItem._(id: id, file: file, status: BookImportBatchStatus.processingFailed, errorMessage: message); + + BookImportBatchItem startAdding() => + BookImportBatchItem._(id: id, file: file, status: BookImportBatchStatus.adding, result: result); + + BookImportBatchItem added() => + BookImportBatchItem._(id: id, file: file, status: BookImportBatchStatus.added, result: result); + + BookImportBatchItem commitFailed(String message) => BookImportBatchItem._( + id: id, + file: file, + status: BookImportBatchStatus.commitFailed, + result: result, + errorMessage: message, + ); +} diff --git a/app/test/widgets/add_book/book_import_batch_item_test.dart b/app/test/widgets/add_book/book_import_batch_item_test.dart new file mode 100644 index 0000000..5b151c6 --- /dev/null +++ b/app/test/widgets/add_book/book_import_batch_item_test.dart @@ -0,0 +1,73 @@ +import 'dart:typed_data'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:papyrus/services/book_import_result.dart'; +import 'package:papyrus/widgets/add_book/book_import_batch_item.dart'; + +void main() { + final file = SelectedBookFile(name: 'book.epub', bytes: Uint8List.fromList([1, 2, 3])); + const result = BookImportResult( + bookId: 'book-1', + title: 'Test Book', + author: 'Test Author', + fileSize: 3, + fileHash: 'hash', + fileExtension: 'epub', + ); + + group('BookImportBatchItem', () { + test('processing failure retries to ready while preserving identity', () { + final failed = BookImportBatchItem.queued( + id: 'row-1', + file: file, + ).startProcessing().processingFailed('Could not read the file.'); + + expect(failed.status, BookImportBatchStatus.processingFailed); + expect(failed.errorMessage, 'Could not read the file.'); + expect(failed.canRetry, isTrue); + expect(failed.isSettled, isTrue); + expect(failed.hasTemporaryFile, isFalse); + + final processing = failed.startProcessing(); + expect(processing.id, 'row-1'); + expect(processing.status, BookImportBatchStatus.processing); + expect(processing.result, isNull); + expect(processing.errorMessage, isNull); + + final ready = processing.processingSucceeded(result); + expect(ready.id, 'row-1'); + expect(ready.status, BookImportBatchStatus.ready); + expect(ready.result, same(result)); + expect(ready.errorMessage, isNull); + expect(ready.canRetry, isFalse); + expect(ready.isSettled, isTrue); + expect(ready.hasTemporaryFile, isTrue); + }); + + test('commit failure retains result and can retry adding', () { + final ready = BookImportBatchItem.queued(id: 'row-2', file: file).startProcessing().processingSucceeded(result); + + final failed = ready.startAdding().commitFailed('Could not add the book.'); + expect(failed.id, 'row-2'); + expect(failed.status, BookImportBatchStatus.commitFailed); + expect(failed.result, same(result)); + expect(failed.errorMessage, 'Could not add the book.'); + expect(failed.canRetry, isTrue); + expect(failed.isSettled, isTrue); + expect(failed.hasTemporaryFile, isTrue); + + final adding = failed.startAdding(); + expect(adding.id, 'row-2'); + expect(adding.status, BookImportBatchStatus.adding); + expect(adding.result, same(result)); + expect(adding.errorMessage, isNull); + + final added = adding.added(); + expect(added.id, 'row-2'); + expect(added.status, BookImportBatchStatus.added); + expect(added.result, same(result)); + expect(added.hasTemporaryFile, isFalse); + expect(added.isSettled, isTrue); + }); + }); +} From 42d3f8da39f4351bf986a8c8753329a1fa215413 Mon Sep 17 00:00:00 2001 From: Eoic Date: Sun, 2 Aug 2026 00:00:05 +0300 Subject: [PATCH 04/17] PPR-26: Mark import batch models immutable --- app/lib/widgets/add_book/book_import_batch_item.dart | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/lib/widgets/add_book/book_import_batch_item.dart b/app/lib/widgets/add_book/book_import_batch_item.dart index bbb69fa..5266820 100644 --- a/app/lib/widgets/add_book/book_import_batch_item.dart +++ b/app/lib/widgets/add_book/book_import_batch_item.dart @@ -1,7 +1,9 @@ import 'dart:typed_data'; +import 'package:flutter/foundation.dart'; import 'package:papyrus/services/book_import_result.dart'; +@immutable class SelectedBookFile { const SelectedBookFile({required this.name, required this.bytes}); @@ -11,6 +13,7 @@ class SelectedBookFile { enum BookImportBatchStatus { queued, processing, ready, processingFailed, adding, added, commitFailed } +@immutable class BookImportBatchItem { const BookImportBatchItem._({ required this.id, From f0279cea1d2598f2a5e3163a4b3bae6d4bad05c1 Mon Sep 17 00:00:00 2001 From: Eoic Date: Sun, 2 Aug 2026 00:05:37 +0300 Subject: [PATCH 05/17] PPR-26: Guard batch import transitions --- .../add_book/book_import_batch_item.dart | 87 +++++++++++++++---- .../add_book/book_import_batch_item_test.dart | 32 +++++++ 2 files changed, 100 insertions(+), 19 deletions(-) diff --git a/app/lib/widgets/add_book/book_import_batch_item.dart b/app/lib/widgets/add_book/book_import_batch_item.dart index 5266820..a196d00 100644 --- a/app/lib/widgets/add_book/book_import_batch_item.dart +++ b/app/lib/widgets/add_book/book_import_batch_item.dart @@ -1,5 +1,3 @@ -import 'dart:typed_data'; - import 'package:flutter/foundation.dart'; import 'package:papyrus/services/book_import_result.dart'; @@ -8,6 +6,9 @@ class SelectedBookFile { const SelectedBookFile({required this.name, required this.bytes}); final String name; + + /// The import workflow takes ownership of these bytes; callers must not + /// mutate them after creating this file selection. final Uint8List? bytes; } @@ -39,26 +40,74 @@ class BookImportBatchItem { bool get hasTemporaryFile => result != null && status != BookImportBatchStatus.added; - BookImportBatchItem startProcessing() => - BookImportBatchItem._(id: id, file: file, status: BookImportBatchStatus.processing); + BookImportBatchItem startProcessing() { + if (status != BookImportBatchStatus.queued && status != BookImportBatchStatus.processingFailed) { + throw StateError('Cannot start processing an item with status $status.'); + } + + return BookImportBatchItem._(id: id, file: file, status: BookImportBatchStatus.processing); + } + + BookImportBatchItem processingSucceeded(BookImportResult value) { + if (status != BookImportBatchStatus.processing) { + throw StateError('Cannot complete processing for an item with status $status.'); + } + + return BookImportBatchItem._(id: id, file: file, status: BookImportBatchStatus.ready, result: value); + } + + BookImportBatchItem processingFailed(String message) { + if (status != BookImportBatchStatus.processing) { + throw StateError('Cannot fail processing for an item with status $status.'); + } + + return BookImportBatchItem._( + id: id, + file: file, + status: BookImportBatchStatus.processingFailed, + errorMessage: message, + ); + } - BookImportBatchItem processingSucceeded(BookImportResult value) => - BookImportBatchItem._(id: id, file: file, status: BookImportBatchStatus.ready, result: value); + BookImportBatchItem startAdding() { + if (status != BookImportBatchStatus.ready && status != BookImportBatchStatus.commitFailed) { + throw StateError('Cannot start adding an item with status $status.'); + } + final value = result; + if (value == null) { + throw StateError('Cannot add an item without a processed result.'); + } - BookImportBatchItem processingFailed(String message) => - BookImportBatchItem._(id: id, file: file, status: BookImportBatchStatus.processingFailed, errorMessage: message); + return BookImportBatchItem._(id: id, file: file, status: BookImportBatchStatus.adding, result: value); + } - BookImportBatchItem startAdding() => - BookImportBatchItem._(id: id, file: file, status: BookImportBatchStatus.adding, result: result); + BookImportBatchItem added() { + if (status != BookImportBatchStatus.adding) { + throw StateError('Cannot finish adding an item with status $status.'); + } + final value = result; + if (value == null) { + throw StateError('Cannot finish adding an item without a processed result.'); + } - BookImportBatchItem added() => - BookImportBatchItem._(id: id, file: file, status: BookImportBatchStatus.added, result: result); + return BookImportBatchItem._(id: id, file: file, status: BookImportBatchStatus.added, result: value); + } - BookImportBatchItem commitFailed(String message) => BookImportBatchItem._( - id: id, - file: file, - status: BookImportBatchStatus.commitFailed, - result: result, - errorMessage: message, - ); + BookImportBatchItem commitFailed(String message) { + if (status != BookImportBatchStatus.adding) { + throw StateError('Cannot fail adding an item with status $status.'); + } + final value = result; + if (value == null) { + throw StateError('Cannot fail adding an item without a processed result.'); + } + + return BookImportBatchItem._( + id: id, + file: file, + status: BookImportBatchStatus.commitFailed, + result: value, + errorMessage: message, + ); + } } diff --git a/app/test/widgets/add_book/book_import_batch_item_test.dart b/app/test/widgets/add_book/book_import_batch_item_test.dart index 5b151c6..25d582d 100644 --- a/app/test/widgets/add_book/book_import_batch_item_test.dart +++ b/app/test/widgets/add_book/book_import_batch_item_test.dart @@ -69,5 +69,37 @@ void main() { expect(added.hasTemporaryFile, isFalse); expect(added.isSettled, isTrue); }); + + test('rejects invalid state transitions', () { + final queued = BookImportBatchItem.queued(id: 'row-3', file: file); + final processing = queued.startProcessing(); + final ready = processing.processingSucceeded(result); + final adding = ready.startAdding(); + + expect(() => queued.processingSucceeded(result), throwsStateError); + expect(() => queued.processingFailed('Could not read the file.'), throwsStateError); + expect(() => ready.startProcessing(), throwsStateError); + expect(() => processing.startAdding(), throwsStateError); + expect(() => ready.added(), throwsStateError); + expect(() => adding.processingFailed('Could not read the file.'), throwsStateError); + }); + + test('reports settlement for every status', () { + final queued = BookImportBatchItem.queued(id: 'row-4', file: file); + final processing = queued.startProcessing(); + final ready = processing.processingSucceeded(result); + final processingFailed = processing.processingFailed('Could not read the file.'); + final adding = ready.startAdding(); + final added = adding.added(); + final commitFailed = adding.commitFailed('Could not add the book.'); + + expect(queued.isSettled, isFalse); + expect(processing.isSettled, isFalse); + expect(ready.isSettled, isTrue); + expect(processingFailed.isSettled, isTrue); + expect(adding.isSettled, isTrue); + expect(added.isSettled, isTrue); + expect(commitFailed.isSettled, isTrue); + }); }); } From b44fd5212b23f76c4181a0122ef46cbb74fcbf94 Mon Sep 17 00:00:00 2001 From: Eoic Date: Sun, 2 Aug 2026 00:09:48 +0300 Subject: [PATCH 06/17] PPR-26: Fix physical import sheet actions --- .../add_book/add_book_sheet_scaffold.dart | 60 ++++++++++++ .../add_book/add_physical_book_sheet.dart | 98 ++++++++----------- .../add_book_sheet_scaffold_test.dart | 36 +++++++ .../add_book/add_book_sheets_test.dart | 26 +++++ 4 files changed, 163 insertions(+), 57 deletions(-) create mode 100644 app/lib/widgets/add_book/add_book_sheet_scaffold.dart create mode 100644 app/test/widgets/add_book/add_book_sheet_scaffold_test.dart diff --git a/app/lib/widgets/add_book/add_book_sheet_scaffold.dart b/app/lib/widgets/add_book/add_book_sheet_scaffold.dart new file mode 100644 index 0000000..811c46b --- /dev/null +++ b/app/lib/widgets/add_book/add_book_sheet_scaffold.dart @@ -0,0 +1,60 @@ +import 'package:flutter/material.dart'; +import 'package:papyrus/themes/design_tokens.dart'; +import 'package:papyrus/widgets/shared/bottom_sheet_handle.dart'; + +/// Lays out an add-book sheet with fixed header and footer regions. +class AddBookSheetScaffold extends StatelessWidget { + final String title; + final VoidCallback onClose; + final Widget body; + final Widget footer; + final bool canClose; + + const AddBookSheetScaffold({ + super.key, + required this.title, + required this.onClose, + required this.body, + required this.footer, + this.canClose = true, + }); + + @override + Widget build(BuildContext context) { + final colorScheme = Theme.of(context).colorScheme; + + return Column( + children: [ + Container( + key: const Key('add-book-sheet-header'), + padding: const EdgeInsets.fromLTRB(Spacing.lg, Spacing.md, Spacing.lg, Spacing.md), + child: Column( + crossAxisAlignment: CrossAxisAlignment.stretch, + children: [ + const BottomSheetHandle(), + const SizedBox(height: Spacing.lg), + Row( + children: [ + Text(title, style: Theme.of(context).textTheme.headlineSmall), + const Spacer(), + IconButton(icon: const Icon(Icons.close), tooltip: 'Close', onPressed: canClose ? onClose : null), + ], + ), + ], + ), + ), + const Divider(height: 1), + Expanded(child: body), + Container( + key: const Key('add-book-sheet-footer'), + padding: const EdgeInsets.symmetric(horizontal: Spacing.lg, vertical: Spacing.md), + decoration: BoxDecoration( + color: colorScheme.surface, + border: Border(top: BorderSide(color: colorScheme.outlineVariant)), + ), + child: SafeArea(top: false, child: footer), + ), + ], + ); + } +} diff --git a/app/lib/widgets/add_book/add_physical_book_sheet.dart b/app/lib/widgets/add_book/add_physical_book_sheet.dart index e8fce5f..840cd40 100644 --- a/app/lib/widgets/add_book/add_physical_book_sheet.dart +++ b/app/lib/widgets/add_book/add_physical_book_sheet.dart @@ -7,13 +7,12 @@ import 'package:papyrus/models/book.dart'; import 'package:papyrus/services/metadata_service.dart'; import 'package:papyrus/themes/design_tokens.dart'; import 'package:papyrus/utils/image_utils.dart'; +import 'package:papyrus/widgets/add_book/add_book_sheet_scaffold.dart'; import 'package:papyrus/widgets/add_book/isbn_scanner_dialog.dart'; import 'package:papyrus/widgets/book_edit/cover_image_picker.dart'; import 'package:papyrus/widgets/book_form/book_date_field.dart'; import 'package:papyrus/widgets/book_form/book_text_field.dart'; import 'package:papyrus/widgets/book_form/co_author_editor.dart'; -import 'package:papyrus/widgets/shared/bottom_sheet_handle.dart'; -import 'package:papyrus/widgets/shared/bottom_sheet_header.dart'; import 'package:provider/provider.dart'; import 'package:uuid/uuid.dart'; @@ -269,66 +268,51 @@ class _PhysicalBookContentState extends State<_PhysicalBookContent> { padding: EdgeInsets.only(bottom: MediaQuery.of(context).viewInsets.bottom), child: Form( key: _formKey, - child: Column( - children: [ - // Fixed header - Padding( - padding: const EdgeInsets.fromLTRB(Spacing.md, Spacing.md, Spacing.md, 0), - child: Column( - children: [ - const BottomSheetHandle(), - const SizedBox(height: Spacing.md), - BottomSheetHeader( - title: 'Add physical book', - onCancel: () => Navigator.pop(context), - onSave: _onSave, - saveLabel: 'Add', - canSave: _canSave, - ), - ], - ), - ), - const SizedBox(height: Spacing.md), - const Divider(height: 1), - - // Scrollable form content - Expanded( - child: ListView( - controller: widget.scrollController, - padding: const EdgeInsets.symmetric(horizontal: Spacing.lg, vertical: Spacing.md), + child: AddBookSheetScaffold( + title: 'Add physical book', + onClose: () => Navigator.of(context).pop(), + body: ListView( + controller: widget.scrollController, + padding: const EdgeInsets.symmetric(horizontal: Spacing.lg, vertical: Spacing.md), + children: [ + Column( + crossAxisAlignment: CrossAxisAlignment.stretch, children: [ - Column( - crossAxisAlignment: CrossAxisAlignment.stretch, - children: [ - Card( - margin: const EdgeInsets.only(bottom: Spacing.xs), - child: Padding( - padding: const EdgeInsets.all(Spacing.md), - child: CoverImagePicker( - initialUrl: _coverUrl, - initialBytes: _coverImageBytes, - onUrlChanged: (url) => setState(() => _coverUrl = url), - onFileChanged: (bytes) => setState(() { - _coverImageBytes = bytes; - if (bytes != null) _coverUrl = null; - }), - coverWidth: 240, - ), - ), + Card( + margin: const EdgeInsets.only(bottom: Spacing.xs), + child: Padding( + padding: const EdgeInsets.all(Spacing.md), + child: CoverImagePicker( + initialUrl: _coverUrl, + initialBytes: _coverImageBytes, + onUrlChanged: (url) => setState(() => _coverUrl = url), + onFileChanged: (bytes) => setState(() { + _coverImageBytes = bytes; + if (bytes != null) _coverUrl = null; + }), + coverWidth: 240, ), - - _buildIsbnSection(), - _buildBasicInfoSection(), - _buildPublicationSection(), - _buildIdentifiersSection(), - _buildSeriesSection(), - _buildPhysicalBookSection(), - ], + ), ), + + _buildIsbnSection(), + _buildBasicInfoSection(), + _buildPublicationSection(), + _buildIdentifiersSection(), + _buildSeriesSection(), + _buildPhysicalBookSection(), ], ), - ), - ], + ], + ), + footer: Row( + mainAxisAlignment: MainAxisAlignment.end, + children: [ + TextButton(onPressed: () => Navigator.of(context).pop(), child: const Text('Cancel')), + const SizedBox(width: Spacing.sm), + FilledButton(onPressed: _canSave ? _onSave : null, child: const Text('Add')), + ], + ), ), ), ); diff --git a/app/test/widgets/add_book/add_book_sheet_scaffold_test.dart b/app/test/widgets/add_book/add_book_sheet_scaffold_test.dart new file mode 100644 index 0000000..9014a16 --- /dev/null +++ b/app/test/widgets/add_book/add_book_sheet_scaffold_test.dart @@ -0,0 +1,36 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:papyrus/widgets/add_book/add_book_sheet_scaffold.dart'; + +void main() { + testWidgets('keeps its header and footer fixed while the body scrolls', (tester) async { + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: AddBookSheetScaffold( + title: 'Add book', + onClose: () {}, + body: ListView.builder( + key: const Key('scrolling-body'), + itemCount: 30, + itemBuilder: (_, index) => SizedBox(height: 64, child: Text('Item $index')), + ), + footer: const Text('Actions'), + ), + ), + ), + ); + + expect(find.byKey(const Key('add-book-sheet-header')), findsOneWidget); + expect(find.byKey(const Key('add-book-sheet-footer')), findsOneWidget); + + final headerTopBeforeScroll = tester.getTopLeft(find.byKey(const Key('add-book-sheet-header'))).dy; + final footerTopBeforeScroll = tester.getTopLeft(find.byKey(const Key('add-book-sheet-footer'))).dy; + + await tester.drag(find.byKey(const Key('scrolling-body')), const Offset(0, -300)); + await tester.pumpAndSettle(); + + expect(tester.getTopLeft(find.byKey(const Key('add-book-sheet-header'))).dy, headerTopBeforeScroll); + expect(tester.getTopLeft(find.byKey(const Key('add-book-sheet-footer'))).dy, footerTopBeforeScroll); + }); +} diff --git a/app/test/widgets/add_book/add_book_sheets_test.dart b/app/test/widgets/add_book/add_book_sheets_test.dart index ca59c1d..55f476d 100644 --- a/app/test/widgets/add_book/add_book_sheets_test.dart +++ b/app/test/widgets/add_book/add_book_sheets_test.dart @@ -3,6 +3,7 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:papyrus/services/book_import_result.dart'; import 'package:papyrus/themes/app_theme.dart'; import 'package:papyrus/widgets/add_book/add_book_choice_sheet.dart'; +import 'package:papyrus/widgets/add_book/add_physical_book_sheet.dart'; import 'package:papyrus/widgets/add_book/import_book_sheet.dart'; import 'package:papyrus/widgets/shared/bottom_sheet_handle.dart'; import 'package:papyrus/widgets/shared/bottom_sheet_header.dart'; @@ -236,4 +237,29 @@ void main() { expect(addButton.style?.backgroundColor?.resolve(disabled), colorScheme.primary); expect(addButton.style?.foregroundColor?.resolve(disabled), colorScheme.onPrimary); }); + + testWidgets('physical import places Add in the fixed footer', (tester) async { + await pumpLauncher( + tester, + (context) => + () => AddPhysicalBookSheet.show(context), + ); + await tester.tap(find.text('Open')); + await tester.pumpAndSettle(); + + expect( + find.ancestor( + of: find.widgetWithText(FilledButton, 'Add'), + matching: find.byKey(const Key('add-book-sheet-footer')), + ), + findsOneWidget, + ); + expect( + find.ancestor( + of: find.widgetWithText(FilledButton, 'Add'), + matching: find.byKey(const Key('add-book-sheet-header')), + ), + findsNothing, + ); + }); } From 2442656d43d99b24520138d1de5abe160195e7ca Mon Sep 17 00:00:00 2001 From: Eoic Date: Sun, 2 Aug 2026 00:22:02 +0300 Subject: [PATCH 07/17] PPR-26: Harden add-book sheet layout --- .../add_book/add_book_sheet_scaffold.dart | 78 ++++++++++++------- .../add_book_sheet_scaffold_test.dart | 30 +++++++ .../add_book/add_book_sheets_test.dart | 33 ++++++++ 3 files changed, 112 insertions(+), 29 deletions(-) diff --git a/app/lib/widgets/add_book/add_book_sheet_scaffold.dart b/app/lib/widgets/add_book/add_book_sheet_scaffold.dart index 811c46b..825c7a7 100644 --- a/app/lib/widgets/add_book/add_book_sheet_scaffold.dart +++ b/app/lib/widgets/add_book/add_book_sheet_scaffold.dart @@ -23,38 +23,58 @@ class AddBookSheetScaffold extends StatelessWidget { Widget build(BuildContext context) { final colorScheme = Theme.of(context).colorScheme; - return Column( - children: [ - Container( - key: const Key('add-book-sheet-header'), - padding: const EdgeInsets.fromLTRB(Spacing.lg, Spacing.md, Spacing.lg, Spacing.md), - child: Column( - crossAxisAlignment: CrossAxisAlignment.stretch, - children: [ - const BottomSheetHandle(), - const SizedBox(height: Spacing.lg), - Row( + return LayoutBuilder( + builder: (context, constraints) { + final isCompactHeight = constraints.maxHeight < 280; + final verticalPadding = isCompactHeight ? 0.0 : Spacing.md; + final handleSpacing = isCompactHeight ? Spacing.xs : Spacing.lg; + + return Column( + children: [ + Container( + key: const Key('add-book-sheet-header'), + padding: EdgeInsets.fromLTRB(Spacing.lg, verticalPadding, Spacing.lg, verticalPadding), + child: Column( + crossAxisAlignment: CrossAxisAlignment.stretch, children: [ - Text(title, style: Theme.of(context).textTheme.headlineSmall), - const Spacer(), - IconButton(icon: const Icon(Icons.close), tooltip: 'Close', onPressed: canClose ? onClose : null), + const BottomSheetHandle(), + SizedBox(height: handleSpacing), + Row( + children: [ + Expanded( + child: Text( + title, + maxLines: isCompactHeight ? 1 : 2, + overflow: TextOverflow.ellipsis, + style: Theme.of(context).textTheme.headlineSmall, + ), + ), + IconButton( + constraints: isCompactHeight ? const BoxConstraints.tightFor(width: 40, height: 40) : null, + icon: const Icon(Icons.close), + padding: isCompactHeight ? EdgeInsets.zero : null, + tooltip: 'Close', + onPressed: canClose ? onClose : null, + ), + ], + ), ], ), - ], - ), - ), - const Divider(height: 1), - Expanded(child: body), - Container( - key: const Key('add-book-sheet-footer'), - padding: const EdgeInsets.symmetric(horizontal: Spacing.lg, vertical: Spacing.md), - decoration: BoxDecoration( - color: colorScheme.surface, - border: Border(top: BorderSide(color: colorScheme.outlineVariant)), - ), - child: SafeArea(top: false, child: footer), - ), - ], + ), + const Divider(height: 1), + Expanded(child: body), + Container( + key: const Key('add-book-sheet-footer'), + padding: EdgeInsets.symmetric(horizontal: Spacing.lg, vertical: verticalPadding), + decoration: BoxDecoration( + color: colorScheme.surface, + border: Border(top: BorderSide(color: colorScheme.outlineVariant)), + ), + child: SafeArea(top: false, child: footer), + ), + ], + ); + }, ); } } diff --git a/app/test/widgets/add_book/add_book_sheet_scaffold_test.dart b/app/test/widgets/add_book/add_book_sheet_scaffold_test.dart index 9014a16..78ff8e3 100644 --- a/app/test/widgets/add_book/add_book_sheet_scaffold_test.dart +++ b/app/test/widgets/add_book/add_book_sheet_scaffold_test.dart @@ -4,6 +4,9 @@ import 'package:papyrus/widgets/add_book/add_book_sheet_scaffold.dart'; void main() { testWidgets('keeps its header and footer fixed while the body scrolls', (tester) async { + final scrollController = ScrollController(); + addTearDown(scrollController.dispose); + await tester.pumpWidget( MaterialApp( home: Scaffold( @@ -12,6 +15,7 @@ void main() { onClose: () {}, body: ListView.builder( key: const Key('scrolling-body'), + controller: scrollController, itemCount: 30, itemBuilder: (_, index) => SizedBox(height: 64, child: Text('Item $index')), ), @@ -30,7 +34,33 @@ void main() { await tester.drag(find.byKey(const Key('scrolling-body')), const Offset(0, -300)); await tester.pumpAndSettle(); + expect(scrollController.offset, greaterThan(0)); expect(tester.getTopLeft(find.byKey(const Key('add-book-sheet-header'))).dy, headerTopBeforeScroll); expect(tester.getTopLeft(find.byKey(const Key('add-book-sheet-footer'))).dy, footerTopBeforeScroll); }); + + testWidgets('constrains a large title beside the close button on narrow screens', (tester) async { + tester.view.devicePixelRatio = 2; + tester.view.physicalSize = const Size(640, 1200); + addTearDown(tester.view.reset); + + await tester.pumpWidget( + MaterialApp( + home: MediaQuery( + data: const MediaQueryData(textScaler: TextScaler.linear(2)), + child: Scaffold( + body: AddBookSheetScaffold( + title: 'A very long physical book title that must remain readable', + onClose: () {}, + body: const SizedBox(), + footer: const SizedBox(), + ), + ), + ), + ), + ); + + expect(tester.takeException(), isNull); + expect(find.byIcon(Icons.close), findsOneWidget); + }); } diff --git a/app/test/widgets/add_book/add_book_sheets_test.dart b/app/test/widgets/add_book/add_book_sheets_test.dart index 55f476d..10f0d9c 100644 --- a/app/test/widgets/add_book/add_book_sheets_test.dart +++ b/app/test/widgets/add_book/add_book_sheets_test.dart @@ -262,4 +262,37 @@ void main() { findsNothing, ); }); + + testWidgets('physical import keeps its footer above the landscape keyboard', (tester) async { + tester.view.devicePixelRatio = 1; + tester.view.physicalSize = const Size(800, 400); + tester.view.viewInsets = const FakeViewPadding(bottom: 250); + addTearDown(tester.view.reset); + + await tester.pumpWidget( + MaterialApp( + home: Builder( + builder: (context) => Scaffold( + body: FilledButton(onPressed: () => AddPhysicalBookSheet.show(context), child: const Text('Open')), + ), + ), + ), + ); + await tester.tap(find.text('Open')); + await tester.pumpAndSettle(); + + expect(tester.takeException(), isNull); + expect(find.byKey(const Key('add-book-sheet-footer')), findsOneWidget); + expect(tester.getRect(find.byKey(const Key('add-book-sheet-footer'))).bottom, lessThanOrEqualTo(150)); + + final listView = find.byType(ListView); + final scrollable = tester.state(find.byType(Scrollable)); + expect(tester.getSize(listView).height, greaterThan(0)); + expect(scrollable.position.maxScrollExtent, greaterThan(0)); + + scrollable.position.jumpTo(1); + await tester.pump(); + + expect(scrollable.position.pixels, 1); + }); } From e5c7090cf564fe09127638a49de7a6c97b859a69 Mon Sep 17 00:00:00 2001 From: Eoic Date: Sun, 2 Aug 2026 00:25:42 +0300 Subject: [PATCH 08/17] PPR-26: Guard compact sheet text scaling --- .../add_book/add_book_sheet_scaffold.dart | 5 +-- .../add_book/add_book_sheets_test.dart | 35 +++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/app/lib/widgets/add_book/add_book_sheet_scaffold.dart b/app/lib/widgets/add_book/add_book_sheet_scaffold.dart index 825c7a7..3c56374 100644 --- a/app/lib/widgets/add_book/add_book_sheet_scaffold.dart +++ b/app/lib/widgets/add_book/add_book_sheet_scaffold.dart @@ -27,7 +27,7 @@ class AddBookSheetScaffold extends StatelessWidget { builder: (context, constraints) { final isCompactHeight = constraints.maxHeight < 280; final verticalPadding = isCompactHeight ? 0.0 : Spacing.md; - final handleSpacing = isCompactHeight ? Spacing.xs : Spacing.lg; + final handleSpacing = isCompactHeight ? 0.0 : Spacing.lg; return Column( children: [ @@ -47,10 +47,11 @@ class AddBookSheetScaffold extends StatelessWidget { maxLines: isCompactHeight ? 1 : 2, overflow: TextOverflow.ellipsis, style: Theme.of(context).textTheme.headlineSmall, + textScaler: isCompactHeight ? TextScaler.noScaling : null, ), ), IconButton( - constraints: isCompactHeight ? const BoxConstraints.tightFor(width: 40, height: 40) : null, + constraints: isCompactHeight ? const BoxConstraints.tightFor(width: 44, height: 44) : null, icon: const Icon(Icons.close), padding: isCompactHeight ? EdgeInsets.zero : null, tooltip: 'Close', diff --git a/app/test/widgets/add_book/add_book_sheets_test.dart b/app/test/widgets/add_book/add_book_sheets_test.dart index 10f0d9c..7bad3a2 100644 --- a/app/test/widgets/add_book/add_book_sheets_test.dart +++ b/app/test/widgets/add_book/add_book_sheets_test.dart @@ -295,4 +295,39 @@ void main() { expect(scrollable.position.pixels, 1); }); + + testWidgets('physical import keeps scaled compact controls above the landscape keyboard', (tester) async { + tester.view.devicePixelRatio = 1; + tester.view.physicalSize = const Size(800, 400); + tester.view.viewInsets = const FakeViewPadding(bottom: 250); + tester.platformDispatcher.textScaleFactorTestValue = 2; + addTearDown(tester.view.reset); + addTearDown(tester.platformDispatcher.clearTextScaleFactorTestValue); + + await tester.pumpWidget( + MaterialApp( + home: Builder( + builder: (context) => Scaffold( + body: FilledButton(onPressed: () => AddPhysicalBookSheet.show(context), child: const Text('Open')), + ), + ), + ), + ); + await tester.tap(find.text('Open')); + await tester.pumpAndSettle(); + + expect(tester.takeException(), isNull); + expect(tester.getRect(find.byKey(const Key('add-book-sheet-footer'))).bottom, lessThanOrEqualTo(150)); + + final closeButton = find.descendant( + of: find.byKey(const Key('add-book-sheet-header')), + matching: find.byType(IconButton), + ); + expect(tester.getSize(closeButton).width, greaterThanOrEqualTo(44)); + expect(tester.getSize(closeButton).height, greaterThanOrEqualTo(44)); + + final scrollable = tester.state(find.byType(Scrollable)); + expect(tester.getSize(find.byType(ListView)).height, greaterThan(0)); + expect(scrollable.position.maxScrollExtent, greaterThan(0)); + }); } From db253c5fc977d1ad74011e1cabe41ab3df8e884d Mon Sep 17 00:00:00 2001 From: Eoic Date: Sun, 2 Aug 2026 00:29:52 +0300 Subject: [PATCH 09/17] PPR-26: Add batch digital import selection --- .../add_book/digital_book_import_sheet.dart | 167 ++++++++++++++++++ .../digital_book_import_sheet_test.dart | 154 ++++++++++++++++ 2 files changed, 321 insertions(+) create mode 100644 app/lib/widgets/add_book/digital_book_import_sheet.dart create mode 100644 app/test/widgets/add_book/digital_book_import_sheet_test.dart diff --git a/app/lib/widgets/add_book/digital_book_import_sheet.dart b/app/lib/widgets/add_book/digital_book_import_sheet.dart new file mode 100644 index 0000000..b36e611 --- /dev/null +++ b/app/lib/widgets/add_book/digital_book_import_sheet.dart @@ -0,0 +1,167 @@ +import 'package:file_picker/file_picker.dart'; +import 'package:flutter/foundation.dart'; +import 'package:flutter/material.dart'; +import 'package:papyrus/themes/design_tokens.dart'; +import 'package:papyrus/widgets/add_book/add_book_sheet_scaffold.dart'; +import 'package:papyrus/widgets/add_book/book_import_batch_item.dart'; + +typedef DigitalBookFilePicker = Future> Function(); + +/// Selects one or more digital books before they are processed for import. +class DigitalBookImportSheet extends StatefulWidget { + const DigitalBookImportSheet({ + super.key, + required this.pickFiles, + required this.onConfirm, + required this.onCancel, + this.scrollController, + }); + + final DigitalBookFilePicker pickFiles; + final ValueChanged> onConfirm; + final VoidCallback onCancel; + final ScrollController? scrollController; + + static const _webExtensions = ['epub']; + static const _nativeExtensions = ['epub', 'pdf', 'mobi', 'azw3', 'txt', 'cbr', 'cbz']; + + /// Opens the file-selection step as its own root-level modal sheet. + static Future?> show(BuildContext context, {DigitalBookFilePicker? pickFiles}) { + return showModalBottomSheet>( + context: context, + isScrollControlled: true, + useRootNavigator: true, + useSafeArea: true, + shape: const RoundedRectangleBorder(borderRadius: BorderRadius.vertical(top: Radius.circular(AppRadius.xl))), + builder: (sheetContext) => DraggableScrollableSheet( + initialChildSize: 0.9, + minChildSize: 0.5, + maxChildSize: 0.95, + expand: false, + builder: (context, scrollController) => DigitalBookImportSheet( + pickFiles: pickFiles ?? _pickFiles, + scrollController: scrollController, + onCancel: () => Navigator.of(sheetContext).pop(), + onConfirm: (files) => Navigator.of(sheetContext).pop(files), + ), + ), + ); + } + + static Future> _pickFiles() async { + final result = await FilePicker.platform.pickFiles( + type: FileType.custom, + allowedExtensions: kIsWeb ? _webExtensions : _nativeExtensions, + allowMultiple: true, + withData: true, + ); + + if (result == null) return const []; + + return result.files.map((file) => SelectedBookFile(name: file.name, bytes: file.bytes)).toList(); + } + + @override + State createState() => _DigitalBookImportSheetState(); +} + +class _DigitalBookImportSheetState extends State { + List _files = const []; + bool _isPicking = false; + + List get _readableFiles => _files.where((file) => file.bytes != null).toList(growable: false); + + Future _browse() async { + if (_isPicking) return; + setState(() => _isPicking = true); + + try { + final files = await widget.pickFiles(); + if (!mounted || files.isEmpty) return; + setState(() => _files = List.unmodifiable(files)); + } finally { + if (mounted) setState(() => _isPicking = false); + } + } + + void _remove(SelectedBookFile file) { + setState(() => _files = List.unmodifiable(_files.where((candidate) => !identical(candidate, file)))); + } + + @override + Widget build(BuildContext context) { + final readableFiles = _readableFiles; + final readableCount = readableFiles.length; + final importLabel = 'Import $readableCount ${readableCount == 1 ? 'book' : 'books'}'; + + return AddBookSheetScaffold( + title: 'Import digital books', + onClose: widget.onCancel, + body: ListView( + key: const Key('digital-import-selection-list'), + controller: widget.scrollController, + padding: const EdgeInsets.symmetric(horizontal: Spacing.lg, vertical: Spacing.md), + children: [ + Text('Select one or more digital book files to import.', style: Theme.of(context).textTheme.bodyLarge), + const SizedBox(height: Spacing.md), + Align( + alignment: Alignment.centerLeft, + child: OutlinedButton.icon( + onPressed: _isPicking ? null : _browse, + icon: _isPicking + ? const SizedBox.square(dimension: 18, child: CircularProgressIndicator(strokeWidth: 2)) + : const Icon(Icons.upload_file), + label: const Text('Browse files'), + ), + ), + if (_files.isNotEmpty) ...[ + const SizedBox(height: Spacing.lg), + Text('${_files.length} selected', style: Theme.of(context).textTheme.titleSmall), + const SizedBox(height: Spacing.sm), + ..._files.map((file) => _SelectedFileRow(file: file, onRemove: () => _remove(file))), + ], + ], + ), + footer: Row( + mainAxisAlignment: MainAxisAlignment.end, + children: [ + TextButton(onPressed: widget.onCancel, child: const Text('Cancel')), + const SizedBox(width: Spacing.sm), + FilledButton( + onPressed: readableFiles.isEmpty ? null : () => widget.onConfirm(readableFiles), + child: Text(importLabel), + ), + ], + ), + ); + } +} + +class _SelectedFileRow extends StatelessWidget { + const _SelectedFileRow({required this.file, required this.onRemove}); + + final SelectedBookFile file; + final VoidCallback onRemove; + + @override + Widget build(BuildContext context) { + final readable = file.bytes != null; + final colorScheme = Theme.of(context).colorScheme; + + return Container( + decoration: BoxDecoration( + border: Border(bottom: BorderSide(color: colorScheme.outlineVariant)), + ), + child: ListTile( + contentPadding: EdgeInsets.zero, + leading: Icon( + readable ? Icons.description_outlined : Icons.error_outline, + color: readable ? null : colorScheme.error, + ), + title: Text(file.name, maxLines: 1, overflow: TextOverflow.ellipsis), + subtitle: readable ? null : Text('Unreadable', style: TextStyle(color: colorScheme.error)), + trailing: IconButton(tooltip: 'Remove ${file.name}', onPressed: onRemove, icon: const Icon(Icons.close)), + ), + ); + } +} diff --git a/app/test/widgets/add_book/digital_book_import_sheet_test.dart b/app/test/widgets/add_book/digital_book_import_sheet_test.dart new file mode 100644 index 0000000..d494a64 --- /dev/null +++ b/app/test/widgets/add_book/digital_book_import_sheet_test.dart @@ -0,0 +1,154 @@ +import 'dart:typed_data'; + +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:papyrus/widgets/add_book/book_import_batch_item.dart'; +import 'package:papyrus/widgets/add_book/digital_book_import_sheet.dart'; + +class _CountingNavigatorObserver extends NavigatorObserver { + int pushes = 0; + + @override + void didPush(Route route, Route? previousRoute) { + pushes++; + super.didPush(route, previousRoute); + } +} + +void main() { + Future pumpSheet( + WidgetTester tester, { + required DigitalBookFilePicker pickFiles, + required ValueChanged> onConfirm, + VoidCallback? onCancel, + }) async { + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: SizedBox( + height: 700, + child: DigitalBookImportSheet(pickFiles: pickFiles, onConfirm: onConfirm, onCancel: onCancel ?? () {}), + ), + ), + ), + ); + } + + testWidgets('confirms the readable files left after removing a selection', (tester) async { + List? confirmedFiles; + final first = SelectedBookFile(name: 'first.epub', bytes: Uint8List.fromList([1])); + final second = SelectedBookFile(name: 'second.pdf', bytes: Uint8List.fromList([2])); + + await pumpSheet(tester, pickFiles: () async => [first, second], onConfirm: (files) => confirmedFiles = files); + + await tester.tap(find.widgetWithText(OutlinedButton, 'Browse files')); + await tester.pump(); + + expect(find.text('first.epub'), findsOneWidget); + expect(find.text('second.pdf'), findsOneWidget); + expect(find.widgetWithText(FilledButton, 'Import 2 books'), findsOneWidget); + + await tester.tap(find.byTooltip('Remove first.epub')); + await tester.pump(); + await tester.tap(find.widgetWithText(FilledButton, 'Import 1 book')); + + expect(confirmedFiles, [same(second)]); + }); + + testWidgets('places selection content between the fixed header and footer', (tester) async { + await pumpSheet(tester, pickFiles: () async => [], onConfirm: (_) {}); + + expect(find.byKey(const Key('add-book-sheet-header')), findsOneWidget); + expect(find.byKey(const Key('add-book-sheet-footer')), findsOneWidget); + expect( + find.ancestor( + of: find.widgetWithText(FilledButton, 'Import 0 books'), + matching: find.byKey(const Key('add-book-sheet-footer')), + ), + findsOneWidget, + ); + expect( + find.ancestor( + of: find.widgetWithText(OutlinedButton, 'Browse files'), + matching: find.byKey(const Key('add-book-sheet-footer')), + ), + findsNothing, + ); + }); + + testWidgets('a fresh non-empty pick replaces the previous selection', (tester) async { + var pickCount = 0; + final oldFile = SelectedBookFile(name: 'old.epub', bytes: Uint8List.fromList([1])); + final newFile = SelectedBookFile(name: 'new.pdf', bytes: Uint8List.fromList([2])); + + await pumpSheet( + tester, + pickFiles: () async { + pickCount++; + return pickCount == 1 ? [oldFile] : [newFile]; + }, + onConfirm: (_) {}, + ); + + await tester.tap(find.widgetWithText(OutlinedButton, 'Browse files')); + await tester.pump(); + expect(find.text('old.epub'), findsOneWidget); + + await tester.tap(find.widgetWithText(OutlinedButton, 'Browse files')); + await tester.pump(); + + expect(find.text('old.epub'), findsNothing); + expect(find.text('new.pdf'), findsOneWidget); + }); + + testWidgets('marks unreadable files and disables confirmation when none are readable', (tester) async { + await pumpSheet( + tester, + pickFiles: () async => const [SelectedBookFile(name: 'broken.epub', bytes: null)], + onConfirm: (_) => fail('Unreadable files must not be confirmed'), + ); + + await tester.tap(find.widgetWithText(OutlinedButton, 'Browse files')); + await tester.pump(); + + expect(find.text('broken.epub'), findsOneWidget); + expect(find.text('Unreadable'), findsOneWidget); + expect(tester.widget(find.widgetWithText(FilledButton, 'Import 0 books')).onPressed, isNull); + }); + + testWidgets('show pushes the draggable sheet on the root navigator', (tester) async { + final rootObserver = _CountingNavigatorObserver(); + final nestedObserver = _CountingNavigatorObserver(); + + await tester.pumpWidget( + MaterialApp( + navigatorObservers: [rootObserver], + home: Navigator( + observers: [nestedObserver], + onGenerateRoute: (_) => MaterialPageRoute( + builder: (nestedContext) => Scaffold( + body: Builder( + builder: (context) => FilledButton( + onPressed: () => DigitalBookImportSheet.show(context, pickFiles: () async => []), + child: const Text('Open'), + ), + ), + ), + ), + ), + ), + ); + await tester.pumpAndSettle(); + final rootPushes = rootObserver.pushes; + final nestedPushes = nestedObserver.pushes; + + await tester.tap(find.text('Open')); + await tester.pumpAndSettle(); + + expect(rootObserver.pushes, rootPushes + 1); + expect(nestedObserver.pushes, nestedPushes); + expect(find.byType(BottomSheet), findsOneWidget); + expect(find.byType(DraggableScrollableSheet), findsOneWidget); + expect(find.byWidgetPredicate((widget) => widget is ModalBarrier && widget.color != null), findsOneWidget); + }); +} From 370d800e3f40231513646e5b6ecef726a6d66e69 Mon Sep 17 00:00:00 2001 From: Eoic Date: Sun, 2 Aug 2026 00:37:33 +0300 Subject: [PATCH 10/17] PPR-26: Harden digital import selection --- .../add_book/digital_book_import_sheet.dart | 27 +++++-- .../digital_book_import_sheet_test.dart | 72 ++++++++++++++++++- 2 files changed, 91 insertions(+), 8 deletions(-) diff --git a/app/lib/widgets/add_book/digital_book_import_sheet.dart b/app/lib/widgets/add_book/digital_book_import_sheet.dart index b36e611..de16257 100644 --- a/app/lib/widgets/add_book/digital_book_import_sheet.dart +++ b/app/lib/widgets/add_book/digital_book_import_sheet.dart @@ -68,17 +68,24 @@ class DigitalBookImportSheet extends StatefulWidget { class _DigitalBookImportSheetState extends State { List _files = const []; bool _isPicking = false; + String? _pickerError; List get _readableFiles => _files.where((file) => file.bytes != null).toList(growable: false); Future _browse() async { if (_isPicking) return; - setState(() => _isPicking = true); + setState(() { + _isPicking = true; + _pickerError = null; + }); try { final files = await widget.pickFiles(); if (!mounted || files.isEmpty) return; setState(() => _files = List.unmodifiable(files)); + } catch (_) { + if (!mounted) return; + setState(() => _pickerError = 'Could not open the selected files. Please try again.'); } finally { if (mounted) setState(() => _isPicking = false); } @@ -91,8 +98,8 @@ class _DigitalBookImportSheetState extends State { @override Widget build(BuildContext context) { final readableFiles = _readableFiles; - final readableCount = readableFiles.length; - final importLabel = 'Import $readableCount ${readableCount == 1 ? 'book' : 'books'}'; + final fileCount = _files.length; + final importLabel = 'Import $fileCount ${fileCount == 1 ? 'book' : 'books'}'; return AddBookSheetScaffold( title: 'Import digital books', @@ -114,6 +121,10 @@ class _DigitalBookImportSheetState extends State { label: const Text('Browse files'), ), ), + if (_pickerError case final message?) ...[ + const SizedBox(height: Spacing.sm), + Text(message, style: TextStyle(color: Theme.of(context).colorScheme.error)), + ], if (_files.isNotEmpty) ...[ const SizedBox(height: Spacing.lg), Text('${_files.length} selected', style: Theme.of(context).textTheme.titleSmall), @@ -122,13 +133,15 @@ class _DigitalBookImportSheetState extends State { ], ], ), - footer: Row( - mainAxisAlignment: MainAxisAlignment.end, + footer: OverflowBar( + alignment: MainAxisAlignment.end, + overflowAlignment: OverflowBarAlignment.end, + spacing: Spacing.sm, + overflowSpacing: Spacing.sm, children: [ TextButton(onPressed: widget.onCancel, child: const Text('Cancel')), - const SizedBox(width: Spacing.sm), FilledButton( - onPressed: readableFiles.isEmpty ? null : () => widget.onConfirm(readableFiles), + onPressed: readableFiles.isEmpty ? null : () => widget.onConfirm(_files), child: Text(importLabel), ), ], diff --git a/app/test/widgets/add_book/digital_book_import_sheet_test.dart b/app/test/widgets/add_book/digital_book_import_sheet_test.dart index d494a64..70f0cd5 100644 --- a/app/test/widgets/add_book/digital_book_import_sheet_test.dart +++ b/app/test/widgets/add_book/digital_book_import_sheet_test.dart @@ -113,7 +113,77 @@ void main() { expect(find.text('broken.epub'), findsOneWidget); expect(find.text('Unreadable'), findsOneWidget); - expect(tester.widget(find.widgetWithText(FilledButton, 'Import 0 books')).onPressed, isNull); + expect(tester.widget(find.widgetWithText(FilledButton, 'Import 1 book')).onPressed, isNull); + }); + + testWidgets('confirms readable and unreadable retained files as one batch', (tester) async { + List? confirmedFiles; + final readable = SelectedBookFile(name: 'readable.epub', bytes: Uint8List.fromList([1])); + const unreadable = SelectedBookFile(name: 'unreadable.pdf', bytes: null); + + await pumpSheet( + tester, + pickFiles: () async => [readable, unreadable], + onConfirm: (files) => confirmedFiles = files, + ); + + await tester.tap(find.widgetWithText(OutlinedButton, 'Browse files')); + await tester.pump(); + + expect(find.widgetWithText(FilledButton, 'Import 2 books'), findsOneWidget); + await tester.tap(find.widgetWithText(FilledButton, 'Import 2 books')); + + expect(confirmedFiles, [same(readable), same(unreadable)]); + }); + + testWidgets('preserves the selection and shows safe feedback when picking fails', (tester) async { + var pickCount = 0; + final selected = SelectedBookFile(name: 'selected.epub', bytes: Uint8List.fromList([1])); + + await pumpSheet( + tester, + pickFiles: () async { + pickCount++; + if (pickCount == 1) return [selected]; + throw Exception('private platform path'); + }, + onConfirm: (_) {}, + ); + + await tester.tap(find.widgetWithText(OutlinedButton, 'Browse files')); + await tester.pump(); + await tester.tap(find.widgetWithText(OutlinedButton, 'Browse files')); + await tester.pump(); + + expect(find.text('selected.epub'), findsOneWidget); + expect(find.text('Could not open the selected files. Please try again.'), findsOneWidget); + expect(tester.takeException(), isNull); + }); + + testWidgets('keeps footer actions usable at narrow width with scaled text', (tester) async { + tester.view.devicePixelRatio = 2; + tester.view.physicalSize = const Size(640, 1200); + tester.platformDispatcher.textScaleFactorTestValue = 2; + addTearDown(tester.view.reset); + addTearDown(tester.platformDispatcher.clearTextScaleFactorTestValue); + + await pumpSheet(tester, pickFiles: () async => [], onConfirm: (_) {}); + + expect(tester.takeException(), isNull); + expect( + find.ancestor( + of: find.widgetWithText(TextButton, 'Cancel'), + matching: find.byKey(const Key('add-book-sheet-footer')), + ), + findsOneWidget, + ); + expect( + find.ancestor( + of: find.widgetWithText(FilledButton, 'Import 0 books'), + matching: find.byKey(const Key('add-book-sheet-footer')), + ), + findsOneWidget, + ); }); testWidgets('show pushes the draggable sheet on the root navigator', (tester) async { From 893041878badbfaa2b2336a44ee9af0c42df7791 Mon Sep 17 00:00:00 2001 From: Eoic Date: Sun, 2 Aug 2026 00:50:07 +0300 Subject: [PATCH 11/17] PPR-26: Add batch import results --- .../add_book/add_book_choice_sheet.dart | 33 +- .../add_book/book_import_results_sheet.dart | 326 ++++++++++++++++++ .../add_book/digital_book_import_sheet.dart | 34 +- .../add_book/add_book_sheets_test.dart | 68 +++- .../book_import_results_sheet_test.dart | 267 ++++++++++++++ 5 files changed, 696 insertions(+), 32 deletions(-) create mode 100644 app/lib/widgets/add_book/book_import_results_sheet.dart create mode 100644 app/test/widgets/add_book/book_import_results_sheet_test.dart diff --git a/app/lib/widgets/add_book/add_book_choice_sheet.dart b/app/lib/widgets/add_book/add_book_choice_sheet.dart index 765d642..04e2631 100644 --- a/app/lib/widgets/add_book/add_book_choice_sheet.dart +++ b/app/lib/widgets/add_book/add_book_choice_sheet.dart @@ -1,7 +1,8 @@ import 'package:flutter/material.dart'; import 'package:papyrus/themes/design_tokens.dart'; import 'package:papyrus/widgets/add_book/add_physical_book_sheet.dart'; -import 'package:papyrus/widgets/add_book/import_book_sheet.dart'; +import 'package:papyrus/widgets/add_book/book_import_results_sheet.dart'; +import 'package:papyrus/widgets/add_book/digital_book_import_sheet.dart'; import 'package:papyrus/widgets/shared/bottom_sheet_handle.dart'; /// Choice sheet for selecting digital import, physical entry, or optional online search. @@ -15,7 +16,13 @@ class AddBookChoiceSheet extends StatefulWidget { final VoidCallback? onFindOnline; /// Show the choice sheet as a modal bottom sheet. - static Future show(BuildContext context, {VoidCallback? onFindOnline}) async { + static Future show( + BuildContext context, { + VoidCallback? onFindOnline, + DigitalBookFilePicker? digitalFilePicker, + BookImportProcessor? bookImportProcessor, + ImportedBookFileDeleter? deleteImportedBookFile, + }) async { Future<_AddBookChoice?>? sheetCompleted; final choice = await showModalBottomSheet<_AddBookChoice>( context: context, @@ -40,8 +47,19 @@ class AddBookChoiceSheet extends StatefulWidget { } switch (choice) { + case _AddBookChoice.importDigital: + final files = await DigitalBookImportSheet.show(context, pickFiles: digitalFilePicker); + if (!context.mounted || files == null || files.isEmpty) { + return; + } + await BookImportResultsSheet.show( + context, + files: files, + processor: bookImportProcessor, + deleteBookFile: deleteImportedBookFile, + ); case _AddBookChoice.addPhysical: - AddPhysicalBookSheet.show(context); + await AddPhysicalBookSheet.show(context); case _AddBookChoice.findOnline: onFindOnline?.call(); } @@ -53,7 +71,6 @@ class AddBookChoiceSheet extends StatefulWidget { class _AddBookChoiceSheetState extends State { bool _isSelecting = false; - bool _showImport = false; void _select(_AddBookChoice choice) { if (_isSelecting) { @@ -66,10 +83,6 @@ class _AddBookChoiceSheetState extends State { @override Widget build(BuildContext context) { - if (_showImport) { - return const ImportBookSheet(); - } - final textTheme = Theme.of(context).textTheme; return Column( @@ -84,7 +97,7 @@ class _AddBookChoiceSheetState extends State { icon: Icons.upload_file, title: 'Import digital books', subtitle: 'EPUB, PDF, AZW3, MOBI, CBZ/CBR', - onTap: () => setState(() => _showImport = true), + onTap: () => _select(_AddBookChoice.importDigital), ), const SizedBox(height: Spacing.sm), _ChoiceOption( @@ -107,7 +120,7 @@ class _AddBookChoiceSheetState extends State { } } -enum _AddBookChoice { addPhysical, findOnline } +enum _AddBookChoice { importDigital, addPhysical, findOnline } class _ChoiceOption extends StatelessWidget { final IconData icon; diff --git a/app/lib/widgets/add_book/book_import_results_sheet.dart b/app/lib/widgets/add_book/book_import_results_sheet.dart new file mode 100644 index 0000000..a8971a5 --- /dev/null +++ b/app/lib/widgets/add_book/book_import_results_sheet.dart @@ -0,0 +1,326 @@ +import 'dart:async'; + +import 'package:flutter/foundation.dart'; +import 'package:flutter/material.dart'; +import 'package:papyrus/models/book.dart'; +import 'package:papyrus/services/book_import_service_stub.dart' + if (dart.library.js_interop) 'package:papyrus/services/book_import_service.dart'; +import 'package:papyrus/themes/design_tokens.dart'; +import 'package:papyrus/widgets/add_book/add_book_sheet_scaffold.dart'; +import 'package:papyrus/widgets/add_book/book_import_batch_item.dart'; +import 'package:provider/provider.dart'; + +typedef BookImportProcessor = Future Function(Uint8List bytes, String filename); +typedef ImportedBookFileDeleter = Future Function(String bookId); +typedef ImportedBookCommitter = Future Function(BookImportResult result, String sourceFilename); + +/// Processes a selected batch and keeps each file's result independently actionable. +class BookImportResultsSheet extends StatefulWidget { + const BookImportResultsSheet({ + super.key, + required this.files, + required this.processor, + required this.deleteBookFile, + required this.onClose, + this.scrollController, + }); + + final List files; + final BookImportProcessor processor; + final ImportedBookFileDeleter deleteBookFile; + final VoidCallback onClose; + final ScrollController? scrollController; + + /// Opens the processing step as its own root-level modal sheet. + static Future show( + BuildContext context, { + required List files, + BookImportProcessor? processor, + ImportedBookFileDeleter? deleteBookFile, + }) { + final importService = processor == null || deleteBookFile == null ? context.read() : null; + final effectiveProcessor = processor ?? importService!.importBook; + final effectiveDeleter = deleteBookFile ?? importService!.deleteBookFile; + + return showModalBottomSheet( + context: context, + isScrollControlled: true, + useRootNavigator: true, + useSafeArea: true, + shape: const RoundedRectangleBorder(borderRadius: BorderRadius.vertical(top: Radius.circular(AppRadius.xl))), + builder: (sheetContext) => DraggableScrollableSheet( + initialChildSize: 0.9, + minChildSize: 0.5, + maxChildSize: 0.95, + expand: false, + builder: (context, scrollController) => BookImportResultsSheet( + files: files, + processor: effectiveProcessor, + deleteBookFile: effectiveDeleter, + scrollController: scrollController, + onClose: () => Navigator.of(sheetContext).pop(), + ), + ), + ); + } + + @override + State createState() => _BookImportResultsSheetState(); +} + +class _BookImportResultsSheetState extends State { + late List _items; + final Map _processingTokens = {}; + final Set _removingIds = {}; + final Set _cleanedBookIds = {}; + bool _isClosing = false; + Future? _closeFuture; + + @override + void initState() { + super.initState(); + _items = List.generate( + widget.files.length, + (index) => BookImportBatchItem.queued(id: 'import-$index', file: widget.files[index]), + growable: true, + ); + WidgetsBinding.instance.addPostFrameCallback((_) { + for (final item in List.of(_items)) { + unawaited(_process(item.id)); + } + }); + } + + int _indexOf(String id) => _items.indexWhere((item) => item.id == id); + + Future _process(String id) async { + if (_isClosing || !mounted) return; + final index = _indexOf(id); + if (index < 0) return; + + final token = (_processingTokens[id] ?? 0) + 1; + _processingTokens[id] = token; + final processingItem = _items[index].startProcessing(); + setState(() => _items[index] = processingItem); + + final bytes = processingItem.file.bytes; + if (bytes == null) { + if (!_isCurrentProcessing(id, token)) return; + setState(() { + final currentIndex = _indexOf(id); + _items[currentIndex] = _items[currentIndex].processingFailed('Could not read this file.'); + }); + return; + } + + try { + final result = await widget.processor(bytes, processingItem.file.name); + if (_isClosing || !mounted || !_isCurrentProcessing(id, token)) { + await _deleteTemporary(result.bookId); + return; + } + setState(() { + final currentIndex = _indexOf(id); + _items[currentIndex] = _items[currentIndex].processingSucceeded(result); + }); + } catch (error) { + if (!_isCurrentProcessing(id, token)) return; + setState(() { + final currentIndex = _indexOf(id); + _items[currentIndex] = _items[currentIndex].processingFailed(_safeErrorMessage(error)); + }); + } + } + + bool _isCurrentProcessing(String id, int token) { + if (_isClosing || !mounted || _processingTokens[id] != token) return false; + final index = _indexOf(id); + return index >= 0 && _items[index].status == BookImportBatchStatus.processing; + } + + String _safeErrorMessage(Object error) { + final message = error.toString().replaceFirst(RegExp(r'^(Exception|Error):\s*'), '').trim(); + return message.isEmpty ? 'Could not import this file.' : message; + } + + Future _remove(String id) async { + if (_isClosing || _removingIds.contains(id)) return; + final index = _indexOf(id); + if (index < 0) return; + final item = _items[index]; + if (item.status != BookImportBatchStatus.processingFailed && item.status != BookImportBatchStatus.ready) return; + + _removingIds.add(id); + if (mounted) setState(() {}); + var deleted = true; + final result = item.result; + if (result != null) { + deleted = await _deleteTemporary(result.bookId); + } + if (!mounted || _isClosing) return; + _removingIds.remove(id); + if (!deleted) { + setState(() {}); + ScaffoldMessenger.maybeOf( + context, + )?.showSnackBar(const SnackBar(content: Text('Could not remove the imported file. Please try again.'))); + return; + } + final currentIndex = _indexOf(id); + if (currentIndex >= 0) { + setState(() => _items.removeAt(currentIndex)); + } + } + + Future _deleteTemporary(String bookId) async { + if (!_cleanedBookIds.add(bookId)) return true; + try { + await widget.deleteBookFile(bookId); + return true; + } catch (error, stackTrace) { + _cleanedBookIds.remove(bookId); + FlutterError.reportError( + FlutterErrorDetails( + exception: error, + stack: stackTrace, + library: 'book import results cleanup', + context: ErrorDescription('while deleting a temporary imported book file'), + ), + ); + return false; + } + } + + Future _requestClose() => _closeFuture ??= _close(); + + Future _close() async { + if (_isClosing) return; + if (mounted) { + setState(() => _isClosing = true); + } else { + _isClosing = true; + } + + final bookIds = _items.where((item) => item.hasTemporaryFile).map((item) => item.result!.bookId).toSet(); + await Future.wait(bookIds.map(_deleteTemporary)); + widget.onClose(); + } + + @override + Widget build(BuildContext context) { + return PopScope( + canPop: false, + onPopInvokedWithResult: (didPop, _) { + if (!didPop) unawaited(_requestClose()); + }, + child: AddBookSheetScaffold( + title: 'Import results', + canClose: !_isClosing, + onClose: () => unawaited(_requestClose()), + body: ListView.separated( + key: const Key('book-import-results-list'), + controller: widget.scrollController, + padding: const EdgeInsets.symmetric(horizontal: Spacing.lg, vertical: Spacing.md), + itemCount: _items.length, + separatorBuilder: (_, _) => const Divider(height: 1), + itemBuilder: (context, index) { + final item = _items[index]; + return _ImportResultRow( + key: ValueKey(item.id), + item: item, + isRemoving: _removingIds.contains(item.id), + onRetry: _isClosing ? null : () => unawaited(_process(item.id)), + onRemove: _isClosing ? null : () => unawaited(_remove(item.id)), + ); + }, + ), + footer: Align( + alignment: Alignment.centerRight, + child: TextButton( + onPressed: _isClosing ? null : () => unawaited(_requestClose()), + child: const Text('Close'), + ), + ), + ), + ); + } +} + +class _ImportResultRow extends StatelessWidget { + const _ImportResultRow({ + super.key, + required this.item, + required this.isRemoving, + required this.onRetry, + required this.onRemove, + }); + + final BookImportBatchItem item; + final bool isRemoving; + final VoidCallback? onRetry; + final VoidCallback? onRemove; + + @override + Widget build(BuildContext context) { + final colorScheme = Theme.of(context).colorScheme; + final failed = item.status == BookImportBatchStatus.processingFailed; + final ready = item.status == BookImportBatchStatus.ready; + + return ListTile( + contentPadding: EdgeInsets.zero, + leading: _statusIcon(colorScheme), + title: Text(item.file.name, maxLines: 1, overflow: TextOverflow.ellipsis), + subtitle: Text( + _statusLabel, + style: failed ? TextStyle(color: colorScheme.error) : null, + maxLines: 2, + overflow: TextOverflow.ellipsis, + ), + trailing: failed || ready + ? Row( + mainAxisSize: MainAxisSize.min, + children: [ + if (failed) TextButton(onPressed: isRemoving ? null : onRetry, child: const Text('Retry')), + if (isRemoving) + const Padding( + padding: EdgeInsets.all(Spacing.sm), + child: SizedBox.square(dimension: 20, child: CircularProgressIndicator(strokeWidth: 2)), + ) + else + IconButton(tooltip: 'Remove ${item.file.name}', onPressed: onRemove, icon: const Icon(Icons.close)), + ], + ) + : null, + ); + } + + Widget _statusIcon(ColorScheme colorScheme) { + return switch (item.status) { + BookImportBatchStatus.queued => const Icon(Icons.schedule_outlined), + BookImportBatchStatus.processing => const SizedBox.square( + dimension: 24, + child: CircularProgressIndicator(strokeWidth: 2), + ), + BookImportBatchStatus.ready => Icon(Icons.check_circle_outline, color: colorScheme.primary), + BookImportBatchStatus.processingFailed => Icon(Icons.error_outline, color: colorScheme.error), + BookImportBatchStatus.adding => const SizedBox.square( + dimension: 24, + child: CircularProgressIndicator(strokeWidth: 2), + ), + BookImportBatchStatus.added => Icon(Icons.check_circle, color: colorScheme.primary), + BookImportBatchStatus.commitFailed => Icon(Icons.error_outline, color: colorScheme.error), + }; + } + + String get _statusLabel { + return switch (item.status) { + BookImportBatchStatus.queued => 'Queued', + BookImportBatchStatus.processing => 'Processing', + BookImportBatchStatus.ready => 'Ready', + BookImportBatchStatus.processingFailed => item.errorMessage ?? 'Could not import this file.', + BookImportBatchStatus.adding => 'Adding', + BookImportBatchStatus.added => 'Added', + BookImportBatchStatus.commitFailed => item.errorMessage ?? 'Could not add this book.', + }; + } +} diff --git a/app/lib/widgets/add_book/digital_book_import_sheet.dart b/app/lib/widgets/add_book/digital_book_import_sheet.dart index de16257..f9892c4 100644 --- a/app/lib/widgets/add_book/digital_book_import_sheet.dart +++ b/app/lib/widgets/add_book/digital_book_import_sheet.dart @@ -26,26 +26,32 @@ class DigitalBookImportSheet extends StatefulWidget { static const _nativeExtensions = ['epub', 'pdf', 'mobi', 'azw3', 'txt', 'cbr', 'cbz']; /// Opens the file-selection step as its own root-level modal sheet. - static Future?> show(BuildContext context, {DigitalBookFilePicker? pickFiles}) { - return showModalBottomSheet>( + static Future?> show(BuildContext context, {DigitalBookFilePicker? pickFiles}) async { + Future?>? sheetCompleted; + final files = await showModalBottomSheet>( context: context, isScrollControlled: true, useRootNavigator: true, useSafeArea: true, shape: const RoundedRectangleBorder(borderRadius: BorderRadius.vertical(top: Radius.circular(AppRadius.xl))), - builder: (sheetContext) => DraggableScrollableSheet( - initialChildSize: 0.9, - minChildSize: 0.5, - maxChildSize: 0.95, - expand: false, - builder: (context, scrollController) => DigitalBookImportSheet( - pickFiles: pickFiles ?? _pickFiles, - scrollController: scrollController, - onCancel: () => Navigator.of(sheetContext).pop(), - onConfirm: (files) => Navigator.of(sheetContext).pop(files), - ), - ), + builder: (sheetContext) { + sheetCompleted = ModalRoute.of>(sheetContext)?.completed; + return DraggableScrollableSheet( + initialChildSize: 0.9, + minChildSize: 0.5, + maxChildSize: 0.95, + expand: false, + builder: (context, scrollController) => DigitalBookImportSheet( + pickFiles: pickFiles ?? _pickFiles, + scrollController: scrollController, + onCancel: () => Navigator.of(sheetContext).pop(), + onConfirm: (files) => Navigator.of(sheetContext).pop(files), + ), + ); + }, ); + await sheetCompleted; + return files; } static Future> _pickFiles() async { diff --git a/app/test/widgets/add_book/add_book_sheets_test.dart b/app/test/widgets/add_book/add_book_sheets_test.dart index 7bad3a2..d446866 100644 --- a/app/test/widgets/add_book/add_book_sheets_test.dart +++ b/app/test/widgets/add_book/add_book_sheets_test.dart @@ -1,9 +1,12 @@ +import 'dart:typed_data'; + import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:papyrus/services/book_import_result.dart'; import 'package:papyrus/themes/app_theme.dart'; import 'package:papyrus/widgets/add_book/add_book_choice_sheet.dart'; import 'package:papyrus/widgets/add_book/add_physical_book_sheet.dart'; +import 'package:papyrus/widgets/add_book/book_import_batch_item.dart'; import 'package:papyrus/widgets/add_book/import_book_sheet.dart'; import 'package:papyrus/widgets/shared/bottom_sheet_handle.dart'; import 'package:papyrus/widgets/shared/bottom_sheet_header.dart'; @@ -175,28 +178,77 @@ void main() { ); }); - testWidgets('digital import transition preserves the modal backdrop', (tester) async { + testWidgets('digital selection and results use distinct modal routes', (tester) async { final observer = _CountingNavigatorObserver(); + final selected = SelectedBookFile(name: 'selected.epub', bytes: Uint8List.fromList([1])); await pumpLauncher( tester, (context) => - () => AddBookChoiceSheet.show(context), + () => AddBookChoiceSheet.show( + context, + digitalFilePicker: () async => [selected], + bookImportProcessor: (_, filename) async => BookImportResult( + bookId: 'temporary-book', + title: filename, + author: 'Author', + fileSize: 1, + fileHash: 'hash', + fileExtension: 'epub', + ), + deleteImportedBookFile: (_) async {}, + ), navigatorObservers: [observer], ); await tester.tap(find.text('Open')); await tester.pumpAndSettle(); - final dimmingBarrier = find.byWidgetPredicate((widget) => widget is ModalBarrier && widget.color != null); final initialPushCount = observer.pushCount; - final initialBarrier = tester.element(dimmingBarrier); await tester.tap(find.text('Import digital books')); await tester.pumpAndSettle(); - expect(find.text('Import book'), findsOneWidget); - expect(dimmingBarrier, findsOneWidget); - expect(tester.element(dimmingBarrier), same(initialBarrier)); - expect(observer.pushCount, initialPushCount); + expect(find.widgetWithText(OutlinedButton, 'Browse files'), findsOneWidget); + expect(find.text('Add book'), findsNothing); + expect(observer.pushCount, initialPushCount + 1); + + await tester.tap(find.widgetWithText(OutlinedButton, 'Browse files')); + await tester.pump(); + await tester.tap(find.widgetWithText(FilledButton, 'Import 1 book')); + await tester.pump(); + + // The results route is not pushed until the selection route has finished + // its dismissal animation. + expect(find.text('Import results'), findsNothing); + + await tester.pumpAndSettle(); + + expect(find.text('Import results'), findsOneWidget); + expect(find.text('selected.epub'), findsOneWidget); + expect(find.text('Ready'), findsOneWidget); + expect(find.widgetWithText(OutlinedButton, 'Browse files'), findsNothing); + expect(observer.pushCount, initialPushCount + 2); + }); + + testWidgets('physical choice dismisses before opening its separate sheet', (tester) async { + final observer = _CountingNavigatorObserver(); + await pumpLauncher( + tester, + (context) => + () => AddBookChoiceSheet.show(context), + navigatorObservers: [observer], + ); + await tester.tap(find.text('Open')); + await tester.pumpAndSettle(); + + final initialPushCount = observer.pushCount; + + await tester.tap(find.text('Add physical book')); + await tester.pumpAndSettle(); + + expect(find.text('Add physical book'), findsOneWidget); + expect(find.byKey(const Key('add-book-sheet-header')), findsOneWidget); + expect(find.text('Add book'), findsNothing); + expect(observer.pushCount, initialPushCount + 1); }); testWidgets('successful import actions can be rendered for widget verification', (tester) async { diff --git a/app/test/widgets/add_book/book_import_results_sheet_test.dart b/app/test/widgets/add_book/book_import_results_sheet_test.dart new file mode 100644 index 0000000..2f0fb5e --- /dev/null +++ b/app/test/widgets/add_book/book_import_results_sheet_test.dart @@ -0,0 +1,267 @@ +import 'dart:async'; +import 'dart:typed_data'; + +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:papyrus/services/book_import_result.dart'; +import 'package:papyrus/widgets/add_book/book_import_batch_item.dart'; +import 'package:papyrus/widgets/add_book/book_import_results_sheet.dart'; + +class _PopCountingObserver extends NavigatorObserver { + int pops = 0; + + @override + void didPop(Route route, Route? previousRoute) { + pops++; + super.didPop(route, previousRoute); + } +} + +BookImportResult resultFor(String filename, {String? bookId}) { + return BookImportResult( + bookId: bookId ?? 'book-$filename', + title: filename, + author: 'Author', + fileSize: 1, + fileHash: 'hash-$filename', + fileExtension: filename.split('.').last, + ); +} + +void main() { + Future pumpResults( + WidgetTester tester, { + required List files, + required BookImportProcessor processor, + ImportedBookFileDeleter? deleter, + VoidCallback? onClose, + }) async { + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: SizedBox( + height: 700, + child: BookImportResultsSheet( + files: files, + processor: processor, + deleteBookFile: deleter ?? (_) async {}, + onClose: onClose ?? () {}, + ), + ), + ), + ), + ); + await tester.pump(); + } + + testWidgets('processes batch rows independently into ready and failed states', (tester) async { + final files = [ + SelectedBookFile(name: 'good.epub', bytes: Uint8List.fromList([1])), + SelectedBookFile(name: 'bad.epub', bytes: Uint8List.fromList([2])), + ]; + + await pumpResults( + tester, + files: files, + processor: (_, filename) async { + if (filename == 'bad.epub') throw Exception('Broken file'); + return resultFor(filename); + }, + ); + await tester.pump(); + + expect(find.text('good.epub'), findsOneWidget); + expect(find.text('Ready'), findsOneWidget); + expect(find.text('bad.epub'), findsOneWidget); + expect(find.text('Broken file'), findsOneWidget); + expect(find.widgetWithText(TextButton, 'Retry'), findsOneWidget); + }); + + testWidgets('retries only the failed row', (tester) async { + var goodCalls = 0; + var badCalls = 0; + final files = [ + SelectedBookFile(name: 'good.epub', bytes: Uint8List.fromList([1])), + SelectedBookFile(name: 'bad.epub', bytes: Uint8List.fromList([2])), + ]; + + await pumpResults( + tester, + files: files, + processor: (_, filename) async { + if (filename == 'good.epub') { + goodCalls++; + return resultFor(filename); + } + badCalls++; + if (badCalls == 1) throw Exception('Try again'); + return resultFor(filename); + }, + ); + await tester.pump(); + + await tester.tap(find.widgetWithText(TextButton, 'Retry')); + await tester.pump(); + await tester.pump(); + + expect(goodCalls, 1); + expect(badCalls, 2); + expect(find.text('Ready'), findsNWidgets(2)); + expect(find.text('Try again'), findsNothing); + }); + + testWidgets('removes failed rows and cleans ready files before removing them', (tester) async { + final deletionGate = Completer(); + final deleted = []; + final files = [ + SelectedBookFile(name: 'ready.epub', bytes: Uint8List.fromList([1])), + SelectedBookFile(name: 'failed.epub', bytes: Uint8List.fromList([2])), + ]; + + await pumpResults( + tester, + files: files, + processor: (_, filename) async { + if (filename == 'failed.epub') throw Exception('Failed'); + return resultFor(filename, bookId: 'temporary-ready'); + }, + deleter: (bookId) async { + deleted.add(bookId); + await deletionGate.future; + }, + ); + await tester.pump(); + + await tester.tap(find.byTooltip('Remove failed.epub')); + await tester.pump(); + expect(find.text('failed.epub'), findsNothing); + + await tester.tap(find.byTooltip('Remove ready.epub')); + await tester.pump(); + expect(deleted, ['temporary-ready']); + expect(find.text('ready.epub'), findsOneWidget); + + deletionGate.complete(); + await tester.pump(); + expect(find.text('ready.epub'), findsNothing); + }); + + testWidgets('marks unreadable files failed without calling the processor', (tester) async { + var processorCalls = 0; + + await pumpResults( + tester, + files: const [SelectedBookFile(name: 'missing.epub', bytes: null)], + processor: (_, _) async { + processorCalls++; + return resultFor('unexpected.epub'); + }, + ); + await tester.pump(); + + expect(processorCalls, 0); + expect(find.text('missing.epub'), findsOneWidget); + expect(find.text('Could not read this file.'), findsOneWidget); + }); + + testWidgets('close cleans every ready result and completes once', (tester) async { + final deleted = []; + final observer = _PopCountingObserver(); + await tester.pumpWidget( + MaterialApp( + navigatorObservers: [observer], + home: Builder( + builder: (context) => Scaffold( + body: FilledButton( + onPressed: () => BookImportResultsSheet.show( + context, + files: [ + SelectedBookFile(name: 'one.epub', bytes: Uint8List.fromList([1])), + SelectedBookFile(name: 'two.epub', bytes: Uint8List.fromList([2])), + ], + processor: (_, filename) async => resultFor(filename), + deleteBookFile: (bookId) async => deleted.add(bookId), + ), + child: const Text('Open'), + ), + ), + ), + ), + ); + await tester.tap(find.text('Open')); + await tester.pump(); + await tester.pump(const Duration(seconds: 1)); + + await tester.tap(find.widgetWithText(TextButton, 'Close')); + await tester.pump(); + await tester.pump(const Duration(seconds: 1)); + + expect(deleted.toSet(), {'book-one.epub', 'book-two.epub'}); + expect(observer.pops, 1); + expect(find.byType(BookImportResultsSheet), findsNothing); + }); + + testWidgets('a success arriving after close is deleted without updating disposed state', (tester) async { + final processing = Completer(); + final deleted = []; + await tester.pumpWidget( + MaterialApp( + home: Builder( + builder: (context) => Scaffold( + body: FilledButton( + onPressed: () => BookImportResultsSheet.show( + context, + files: [ + SelectedBookFile(name: 'slow.epub', bytes: Uint8List.fromList([1])), + ], + processor: (_, _) => processing.future, + deleteBookFile: (bookId) async => deleted.add(bookId), + ), + child: const Text('Open'), + ), + ), + ), + ), + ); + await tester.tap(find.text('Open')); + await tester.pump(); + await tester.pump(const Duration(seconds: 1)); + + await tester.tap(find.widgetWithText(TextButton, 'Close')); + await tester.pump(); + await tester.pump(const Duration(seconds: 1)); + expect(find.byType(BookImportResultsSheet), findsNothing); + + processing.complete(resultFor('slow.epub', bookId: 'late-book')); + await tester.pump(); + await tester.pump(); + + expect(deleted, ['late-book']); + expect(tester.takeException(), isNull); + }); + + testWidgets('places results between the fixed header and footer', (tester) async { + await pumpResults( + tester, + files: const [SelectedBookFile(name: 'missing.epub', bytes: null)], + processor: (_, _) async => resultFor('unused.epub'), + ); + + expect(find.byKey(const Key('add-book-sheet-header')), findsOneWidget); + expect(find.byKey(const Key('add-book-sheet-footer')), findsOneWidget); + expect( + find.ancestor( + of: find.widgetWithText(TextButton, 'Close'), + matching: find.byKey(const Key('add-book-sheet-footer')), + ), + findsOneWidget, + ); + expect( + find.ancestor( + of: find.byKey(const Key('book-import-results-list')), + matching: find.byKey(const Key('add-book-sheet-footer')), + ), + findsNothing, + ); + }); +} From 82925a16790efac5190bdda4adb84db0c2fc0657 Mon Sep 17 00:00:00 2001 From: Eoic Date: Sun, 2 Aug 2026 01:01:20 +0300 Subject: [PATCH 12/17] PPR-26: Harden import result cleanup --- .../add_book/book_import_results_sheet.dart | 69 +++++++--- .../book_import_results_sheet_test.dart | 123 ++++++++++++++++++ 2 files changed, 177 insertions(+), 15 deletions(-) diff --git a/app/lib/widgets/add_book/book_import_results_sheet.dart b/app/lib/widgets/add_book/book_import_results_sheet.dart index a8971a5..2caa6a1 100644 --- a/app/lib/widgets/add_book/book_import_results_sheet.dart +++ b/app/lib/widgets/add_book/book_import_results_sheet.dart @@ -47,6 +47,7 @@ class BookImportResultsSheet extends StatefulWidget { isScrollControlled: true, useRootNavigator: true, useSafeArea: true, + enableDrag: false, shape: const RoundedRectangleBorder(borderRadius: BorderRadius.vertical(top: Radius.circular(AppRadius.xl))), builder: (sheetContext) => DraggableScrollableSheet( initialChildSize: 0.9, @@ -73,6 +74,7 @@ class _BookImportResultsSheetState extends State { final Map _processingTokens = {}; final Set _removingIds = {}; final Set _cleanedBookIds = {}; + final Map> _cleanupFutures = {}; bool _isClosing = false; Future? _closeFuture; @@ -157,8 +159,12 @@ class _BookImportResultsSheetState extends State { if (result != null) { deleted = await _deleteTemporary(result.bookId); } - if (!mounted || _isClosing) return; + if (!mounted) return; _removingIds.remove(id); + if (_isClosing) { + setState(() {}); + return; + } if (!deleted) { setState(() {}); ScaffoldMessenger.maybeOf( @@ -172,26 +178,49 @@ class _BookImportResultsSheetState extends State { } } - Future _deleteTemporary(String bookId) async { - if (!_cleanedBookIds.add(bookId)) return true; + Future _deleteTemporary(String bookId) { + if (_cleanedBookIds.contains(bookId)) return Future.value(true); + final inFlight = _cleanupFutures[bookId]; + if (inFlight != null) return inFlight; + + final cleanup = _performDelete(bookId); + _cleanupFutures[bookId] = cleanup; + unawaited( + cleanup.whenComplete(() { + if (identical(_cleanupFutures[bookId], cleanup)) { + _cleanupFutures.remove(bookId); + } + }), + ); + return cleanup; + } + + Future _performDelete(String bookId) async { try { await widget.deleteBookFile(bookId); + _cleanedBookIds.add(bookId); return true; - } catch (error, stackTrace) { - _cleanedBookIds.remove(bookId); - FlutterError.reportError( - FlutterErrorDetails( - exception: error, - stack: stackTrace, - library: 'book import results cleanup', - context: ErrorDescription('while deleting a temporary imported book file'), - ), - ); + } catch (_) { + debugPrint('Book import temporary-file cleanup failed.'); return false; } } - Future _requestClose() => _closeFuture ??= _close(); + Future _requestClose() { + final inFlight = _closeFuture; + if (inFlight != null) return inFlight; + + final close = _close(); + _closeFuture = close; + unawaited( + close.whenComplete(() { + if (identical(_closeFuture, close)) { + _closeFuture = null; + } + }), + ); + return close; + } Future _close() async { if (_isClosing) return; @@ -202,7 +231,17 @@ class _BookImportResultsSheetState extends State { } final bookIds = _items.where((item) => item.hasTemporaryFile).map((item) => item.result!.bookId).toSet(); - await Future.wait(bookIds.map(_deleteTemporary)); + final cleanupResults = await Future.wait(bookIds.map(_deleteTemporary)); + if (cleanupResults.any((deleted) => !deleted)) { + if (mounted) { + setState(() => _isClosing = false); + ScaffoldMessenger.maybeOf( + context, + )?.showSnackBar(const SnackBar(content: Text('Could not remove temporary files. Please try again.'))); + } + return; + } + if (!mounted) return; widget.onClose(); } diff --git a/app/test/widgets/add_book/book_import_results_sheet_test.dart b/app/test/widgets/add_book/book_import_results_sheet_test.dart index 2f0fb5e..d456f47 100644 --- a/app/test/widgets/add_book/book_import_results_sheet_test.dart +++ b/app/test/widgets/add_book/book_import_results_sheet_test.dart @@ -201,6 +201,129 @@ void main() { expect(find.byType(BookImportResultsSheet), findsNothing); }); + testWidgets('dragging the route downward cannot dismiss and bypass cleanup', (tester) async { + final deleted = []; + await tester.pumpWidget( + MaterialApp( + home: Builder( + builder: (context) => Scaffold( + body: FilledButton( + onPressed: () => BookImportResultsSheet.show( + context, + files: [ + SelectedBookFile(name: 'ready.epub', bytes: Uint8List.fromList([1])), + ], + processor: (_, filename) async => resultFor(filename, bookId: 'temporary-ready'), + deleteBookFile: (bookId) async => deleted.add(bookId), + ), + child: const Text('Open'), + ), + ), + ), + ), + ); + await tester.tap(find.text('Open')); + await tester.pumpAndSettle(); + + await tester.drag(find.byKey(const Key('add-book-sheet-header')), const Offset(0, 700)); + await tester.pumpAndSettle(); + + expect(find.byType(BookImportResultsSheet), findsOneWidget); + expect(find.text('ready.epub'), findsOneWidget); + expect(deleted, isEmpty); + }); + + testWidgets('cleanup failure keeps the sheet open and close can retry', (tester) async { + var deleteAttempts = 0; + await tester.pumpWidget( + MaterialApp( + home: Builder( + builder: (context) => Scaffold( + body: FilledButton( + onPressed: () => BookImportResultsSheet.show( + context, + files: [ + SelectedBookFile(name: 'ready.epub', bytes: Uint8List.fromList([1])), + ], + processor: (_, filename) async => resultFor(filename, bookId: 'temporary-ready'), + deleteBookFile: (_) async { + deleteAttempts++; + if (deleteAttempts == 1) throw Exception('private cleanup detail'); + }, + ), + child: const Text('Open'), + ), + ), + ), + ), + ); + await tester.tap(find.text('Open')); + await tester.pumpAndSettle(); + + await tester.tap(find.widgetWithText(TextButton, 'Close')); + await tester.pump(); + await tester.pump(const Duration(seconds: 1)); + + expect(deleteAttempts, 1); + expect(find.byType(BookImportResultsSheet), findsOneWidget); + expect(find.text('Could not remove temporary files. Please try again.'), findsOneWidget); + expect(tester.widget(find.widgetWithText(TextButton, 'Close')).onPressed, isNotNull); + + await tester.tap(find.widgetWithText(TextButton, 'Close')); + await tester.pump(); + await tester.pump(const Duration(seconds: 1)); + + expect(deleteAttempts, 2); + expect(find.byType(BookImportResultsSheet), findsNothing); + }); + + testWidgets('close awaits and deduplicates cleanup already started by remove', (tester) async { + final deletionGate = Completer(); + var deleteCalls = 0; + await tester.pumpWidget( + MaterialApp( + home: Builder( + builder: (context) => Scaffold( + body: FilledButton( + onPressed: () => BookImportResultsSheet.show( + context, + files: [ + SelectedBookFile(name: 'ready.epub', bytes: Uint8List.fromList([1])), + ], + processor: (_, filename) async => resultFor(filename, bookId: 'temporary-ready'), + deleteBookFile: (_) async { + deleteCalls++; + await deletionGate.future; + }, + ), + child: const Text('Open'), + ), + ), + ), + ), + ); + await tester.tap(find.text('Open')); + await tester.pumpAndSettle(); + + await tester.tap(find.byTooltip('Remove ready.epub')); + await tester.pump(); + expect(deleteCalls, 1); + + await tester.tap(find.widgetWithText(TextButton, 'Close')); + await tester.pump(); + await tester.pump(const Duration(seconds: 1)); + + expect(deleteCalls, 1); + expect(find.byType(BookImportResultsSheet), findsOneWidget); + + deletionGate.complete(); + await tester.pump(); + await tester.pump(const Duration(seconds: 1)); + + expect(deleteCalls, 1); + expect(find.byType(BookImportResultsSheet), findsNothing); + }); + testWidgets('a success arriving after close is deleted without updating disposed state', (tester) async { final processing = Completer(); final deleted = []; From 0dbf9c5b8216a4e2861b586b3ffcca101412dfd5 Mon Sep 17 00:00:00 2001 From: Eoic Date: Sun, 2 Aug 2026 01:10:48 +0300 Subject: [PATCH 13/17] PPR-26: Commit batch book imports --- .../add_book/book_import_results_sheet.dart | 194 ++++++++++++++- .../media_profile_switch_contract_test.dart | 27 ++- .../book_import_results_sheet_test.dart | 222 ++++++++++++++++++ 3 files changed, 419 insertions(+), 24 deletions(-) diff --git a/app/lib/widgets/add_book/book_import_results_sheet.dart b/app/lib/widgets/add_book/book_import_results_sheet.dart index 2caa6a1..2e6805d 100644 --- a/app/lib/widgets/add_book/book_import_results_sheet.dart +++ b/app/lib/widgets/add_book/book_import_results_sheet.dart @@ -2,7 +2,13 @@ import 'dart:async'; import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; +import 'package:papyrus/data/data_store.dart'; +import 'package:papyrus/media/media_upload_queue.dart'; import 'package:papyrus/models/book.dart'; +import 'package:papyrus/powersync/powersync_service.dart'; +import 'package:papyrus/powersync/sync_state.dart'; +import 'package:papyrus/providers/auth_provider.dart'; +import 'package:papyrus/services/book_import_commit_service.dart'; import 'package:papyrus/services/book_import_service_stub.dart' if (dart.library.js_interop) 'package:papyrus/services/book_import_service.dart'; import 'package:papyrus/themes/design_tokens.dart'; @@ -22,25 +28,74 @@ class BookImportResultsSheet extends StatefulWidget { required this.processor, required this.deleteBookFile, required this.onClose, + this.committer, + this.onCompleted, this.scrollController, }); final List files; final BookImportProcessor processor; final ImportedBookFileDeleter deleteBookFile; + final ImportedBookCommitter? committer; final VoidCallback onClose; + final ValueChanged>? onCompleted; final ScrollController? scrollController; + static Future _commitResult(BuildContext context, BookImportResult result, String sourceFilename) async { + final dataStore = context.read(); + final bookRepository = dataStore.requireBookRepository(); + final queue = context.read(); + final importService = context.read(); + final authProvider = context.read(); + final powerSyncService = context.read(); + final isOnlineAccount = authProvider.isSignedIn && powerSyncService.mode == LibraryDatabaseMode.authenticated; + final accountScope = isOnlineAccount ? queue.activeScope : null; + if (isOnlineAccount && accountScope == null) { + throw StateError('Cannot import account media without an active media storage scope'); + } + + final extension = result.fileExtension; + final filePath = kIsWeb + ? 'opfs://books/${result.bookId}.$extension' + : result.bookId; // Native resolves via BookImportService.getBookFile. + final commitService = BookImportCommitService( + storePendingCover: importService.storePendingCoverFile, + storeGuestCover: importService.storeGuestCoverFile, + deletePendingCover: importService.deletePendingCoverFile, + deleteGuestCover: importService.deleteGuestCoverFile, + addBook: (book) => dataStore.addBookToRepositoryAndWait(bookRepository, book), + deleteBook: (bookId) => dataStore.deleteBookFromRepositoryAndWait(bookRepository, bookId), + enqueueImportedBookMedia: queue.enqueueImportedBookMedia, + isLibraryContextCurrent: () { + final currentIsOnlineAccount = + authProvider.isSignedIn && powerSyncService.mode == LibraryDatabaseMode.authenticated; + return dataStore.isBookRepositoryCurrent(bookRepository) && + currentIsOnlineAccount == isOnlineAccount && + queue.activeScope == accountScope; + }, + ); + return commitService.commit( + result: result, + sourceFilename: sourceFilename, + addedAt: DateTime.now(), + localFilePath: filePath, + accountScope: accountScope, + ); + } + /// Opens the processing step as its own root-level modal sheet. static Future show( BuildContext context, { required List files, BookImportProcessor? processor, ImportedBookFileDeleter? deleteBookFile, + ImportedBookCommitter? committer, }) { final importService = processor == null || deleteBookFile == null ? context.read() : null; final effectiveProcessor = processor ?? importService!.importBook; final effectiveDeleter = deleteBookFile ?? importService!.deleteBookFile; + final effectiveCommitter = committer ?? (result, filename) => _commitResult(context, result, filename); + final messenger = ScaffoldMessenger.maybeOf(context); return showModalBottomSheet( context: context, @@ -58,8 +113,15 @@ class BookImportResultsSheet extends StatefulWidget { files: files, processor: effectiveProcessor, deleteBookFile: effectiveDeleter, + committer: effectiveCommitter, scrollController: scrollController, onClose: () => Navigator.of(sheetContext).pop(), + onCompleted: (books) { + final count = books.length; + messenger?.showSnackBar( + SnackBar(content: Text('Added $count ${count == 1 ? 'book' : 'books'} to library')), + ); + }, ), ), ); @@ -75,7 +137,10 @@ class _BookImportResultsSheetState extends State { final Set _removingIds = {}; final Set _cleanedBookIds = {}; final Map> _cleanupFutures = {}; + final List _addedBooks = []; bool _isClosing = false; + bool _isAdding = false; + bool _didComplete = false; Future? _closeFuture; @override @@ -145,12 +210,103 @@ class _BookImportResultsSheetState extends State { return message.isEmpty ? 'Could not import this file.' : message; } + int get _readyCount => _items.where((item) => item.status == BookImportBatchStatus.ready).length; + + bool get _processingSettled => _items.every((item) => item.isSettled); + + bool get _hasCleanupAction => _isClosing || _removingIds.isNotEmpty; + + bool get _canAdd => _processingSettled && _readyCount > 0 && !_isAdding && !_hasCleanupAction; + + Future _addReadyBooks() async { + if (!_canAdd) return; + final readyIds = _items + .where((item) => item.status == BookImportBatchStatus.ready) + .map((item) => item.id) + .toList(growable: false); + if (readyIds.isEmpty) return; + + setState(() { + _isAdding = true; + for (final id in readyIds) { + final index = _indexOf(id); + if (index >= 0 && _items[index].status == BookImportBatchStatus.ready) { + _items[index] = _items[index].startAdding(); + } + } + }); + + for (final id in readyIds) { + await _commitAddingItem(id); + } + if (!mounted) return; + setState(() => _isAdding = false); + _completeIfAllAdded(); + } + + Future _retryCommit(String id) async { + if (_isAdding || _hasCleanupAction || !mounted) return; + final index = _indexOf(id); + if (index < 0 || _items[index].status != BookImportBatchStatus.commitFailed) return; + + setState(() { + _isAdding = true; + _items[index] = _items[index].startAdding(); + }); + await _commitAddingItem(id); + if (!mounted) return; + setState(() => _isAdding = false); + _completeIfAllAdded(); + } + + Future _commitAddingItem(String id) async { + final index = _indexOf(id); + if (index < 0) return; + final item = _items[index]; + if (item.status != BookImportBatchStatus.adding || item.result == null) return; + + try { + final book = + await (widget.committer ?? + (result, filename) => + BookImportResultsSheet._commitResult(context, result, filename))(item.result!, item.file.name); + if (!mounted) return; + final currentIndex = _indexOf(id); + if (currentIndex < 0 || _items[currentIndex].status != BookImportBatchStatus.adding) return; + setState(() { + _items[currentIndex] = _items[currentIndex].added(); + _addedBooks.add(book); + }); + } catch (error) { + if (!mounted) return; + final currentIndex = _indexOf(id); + if (currentIndex < 0 || _items[currentIndex].status != BookImportBatchStatus.adding) return; + setState(() { + _items[currentIndex] = _items[currentIndex].commitFailed(_safeErrorMessage(error)); + }); + } + } + + void _completeIfAllAdded() { + if (_didComplete || _items.isEmpty || !_items.every((item) => item.status == BookImportBatchStatus.added)) { + return; + } + _didComplete = true; + final books = List.unmodifiable(_addedBooks); + widget.onClose(); + widget.onCompleted?.call(books); + } + Future _remove(String id) async { - if (_isClosing || _removingIds.contains(id)) return; + if (_isClosing || _isAdding || _removingIds.contains(id)) return; final index = _indexOf(id); if (index < 0) return; final item = _items[index]; - if (item.status != BookImportBatchStatus.processingFailed && item.status != BookImportBatchStatus.ready) return; + if (item.status != BookImportBatchStatus.processingFailed && + item.status != BookImportBatchStatus.ready && + item.status != BookImportBatchStatus.commitFailed) { + return; + } _removingIds.add(id); if (mounted) setState(() {}); @@ -175,6 +331,7 @@ class _BookImportResultsSheetState extends State { final currentIndex = _indexOf(id); if (currentIndex >= 0) { setState(() => _items.removeAt(currentIndex)); + _completeIfAllAdded(); } } @@ -207,6 +364,7 @@ class _BookImportResultsSheetState extends State { } Future _requestClose() { + if (_isAdding) return Future.value(); final inFlight = _closeFuture; if (inFlight != null) return inFlight; @@ -254,7 +412,7 @@ class _BookImportResultsSheetState extends State { }, child: AddBookSheetScaffold( title: 'Import results', - canClose: !_isClosing, + canClose: !_isClosing && !_isAdding, onClose: () => unawaited(_requestClose()), body: ListView.separated( key: const Key('book-import-results-list'), @@ -268,17 +426,28 @@ class _BookImportResultsSheetState extends State { key: ValueKey(item.id), item: item, isRemoving: _removingIds.contains(item.id), - onRetry: _isClosing ? null : () => unawaited(_process(item.id)), - onRemove: _isClosing ? null : () => unawaited(_remove(item.id)), + onRetry: _isClosing || _isAdding + ? null + : () => unawaited( + item.status == BookImportBatchStatus.commitFailed ? _retryCommit(item.id) : _process(item.id), + ), + onRemove: _isClosing || _isAdding ? null : () => unawaited(_remove(item.id)), ); }, ), - footer: Align( - alignment: Alignment.centerRight, - child: TextButton( - onPressed: _isClosing ? null : () => unawaited(_requestClose()), - child: const Text('Close'), - ), + footer: Row( + mainAxisAlignment: MainAxisAlignment.end, + children: [ + TextButton( + onPressed: _isClosing || _isAdding ? null : () => unawaited(_requestClose()), + child: const Text('Close'), + ), + const SizedBox(width: Spacing.sm), + FilledButton( + onPressed: _canAdd ? () => unawaited(_addReadyBooks()) : null, + child: Text('Add $_readyCount to library'), + ), + ], ), ), ); @@ -302,7 +471,8 @@ class _ImportResultRow extends StatelessWidget { @override Widget build(BuildContext context) { final colorScheme = Theme.of(context).colorScheme; - final failed = item.status == BookImportBatchStatus.processingFailed; + final failed = + item.status == BookImportBatchStatus.processingFailed || item.status == BookImportBatchStatus.commitFailed; final ready = item.status == BookImportBatchStatus.ready; return ListTile( diff --git a/app/test/media/media_profile_switch_contract_test.dart b/app/test/media/media_profile_switch_contract_test.dart index 51e7878..9a89d23 100644 --- a/app/test/media/media_profile_switch_contract_test.dart +++ b/app/test/media/media_profile_switch_contract_test.dart @@ -46,9 +46,12 @@ void main() { ); expect(processor, contains('readPendingCover: _bookImportService.getPendingCoverFile')); - final importSource = File('lib/widgets/add_book/import_book_sheet.dart').readAsStringSync(); - final commitStart = importSource.indexOf('Future _addToLibrary()'); - final commit = importSource.substring(commitStart, importSource.indexOf('@override\n Widget build', commitStart)); + final importSource = File('lib/widgets/add_book/book_import_results_sheet.dart').readAsStringSync(); + final commitStart = importSource.indexOf('static Future _commitResult('); + final commit = importSource.substring( + commitStart, + importSource.indexOf('/// Opens the processing step', commitStart), + ); expect(commit, contains('final accountScope = isOnlineAccount ? queue.activeScope : null;')); expect(commit, contains("throw StateError('Cannot import account media without an active media storage scope')")); expect(commit, contains('storePendingCover: importService.storePendingCoverFile')); @@ -67,15 +70,15 @@ void main() { }); test('import commit guard prevents repeat commits and disables mutable actions', () { - final source = File('lib/widgets/add_book/import_book_sheet.dart').readAsStringSync(); - final commitStart = source.indexOf('Future _addToLibrary()'); - final commit = source.substring(commitStart, source.indexOf('@override\n Widget build', commitStart)); + final source = File('lib/widgets/add_book/book_import_results_sheet.dart').readAsStringSync(); + final addStart = source.indexOf('Future _addReadyBooks()'); + final add = source.substring(addStart, source.indexOf('Future _retryCommit', addStart)); - expect(commit, contains('if (_committing) return;')); - expect(commit, contains('_committing = true')); - expect(commit, contains('_committing = false')); - expect(source, contains('onPressed: _committing ? null : _addToLibrary')); - expect(RegExp(r'onPressed: _committing \? null : _pickAndProcess').allMatches(source), hasLength(2)); - expect(source, contains('onPressed: _committing\n ? null')); + expect(add, contains('if (!_canAdd) return;')); + expect(add, contains('_isAdding = true')); + expect(add, contains('_isAdding = false')); + expect(source, contains('canClose: !_isClosing && !_isAdding')); + expect(source, contains('onPressed: _canAdd ? () => unawaited(_addReadyBooks()) : null')); + expect(source, contains('onRemove: _isClosing || _isAdding ? null')); }); } diff --git a/app/test/widgets/add_book/book_import_results_sheet_test.dart b/app/test/widgets/add_book/book_import_results_sheet_test.dart index d456f47..4e17133 100644 --- a/app/test/widgets/add_book/book_import_results_sheet_test.dart +++ b/app/test/widgets/add_book/book_import_results_sheet_test.dart @@ -3,6 +3,7 @@ import 'dart:typed_data'; import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:papyrus/models/book.dart'; import 'package:papyrus/services/book_import_result.dart'; import 'package:papyrus/widgets/add_book/book_import_batch_item.dart'; import 'package:papyrus/widgets/add_book/book_import_results_sheet.dart'; @@ -28,13 +29,19 @@ BookImportResult resultFor(String filename, {String? bookId}) { ); } +Book bookFor(BookImportResult result) { + return Book(id: result.bookId, title: result.title, author: result.author, addedAt: DateTime(2026)); +} + void main() { Future pumpResults( WidgetTester tester, { required List files, required BookImportProcessor processor, ImportedBookFileDeleter? deleter, + ImportedBookCommitter? committer, VoidCallback? onClose, + ValueChanged>? onCompleted, }) async { await tester.pumpWidget( MaterialApp( @@ -45,7 +52,9 @@ void main() { files: files, processor: processor, deleteBookFile: deleter ?? (_) async {}, + committer: committer, onClose: onClose ?? () {}, + onCompleted: onCompleted, ), ), ), @@ -387,4 +396,217 @@ void main() { findsNothing, ); }); + + testWidgets('enables the counted add action only after processing settles', (tester) async { + final processing = Completer(); + await pumpResults( + tester, + files: [ + SelectedBookFile(name: 'one.epub', bytes: Uint8List.fromList([1])), + ], + processor: (_, _) => processing.future, + committer: (result, _) async => bookFor(result), + ); + + final addButton = find.widgetWithText(FilledButton, 'Add 0 to library'); + expect(addButton, findsOneWidget); + expect(tester.widget(addButton).onPressed, isNull); + + processing.complete(resultFor('one.epub')); + await tester.pump(); + + final enabledButton = find.widgetWithText(FilledButton, 'Add 1 to library'); + expect(enabledButton, findsOneWidget); + expect(tester.widget(enabledButton).onPressed, isNotNull); + }); + + testWidgets('commits every ready book and reports the completed batch', (tester) async { + final committed = []; + List? completed; + var closed = false; + await pumpResults( + tester, + files: [ + SelectedBookFile(name: 'one.epub', bytes: Uint8List.fromList([1])), + SelectedBookFile(name: 'two.epub', bytes: Uint8List.fromList([2])), + ], + processor: (_, filename) async => resultFor(filename), + committer: (result, filename) async { + committed.add(filename); + return bookFor(result); + }, + onClose: () => closed = true, + onCompleted: (books) => completed = books, + ); + await tester.pump(); + + await tester.tap(find.widgetWithText(FilledButton, 'Add 2 to library')); + await tester.pump(); + await tester.pump(); + + expect(committed, ['one.epub', 'two.epub']); + expect(completed?.map((book) => book.id), ['book-one.epub', 'book-two.epub']); + expect(closed, isTrue); + }); + + testWidgets('partial commit retries only the failed final commit without duplicating successes', (tester) async { + final processCalls = {}; + final commitCalls = {}; + List? completed; + await pumpResults( + tester, + files: [ + SelectedBookFile(name: 'a.epub', bytes: Uint8List.fromList([1])), + SelectedBookFile(name: 'b.epub', bytes: Uint8List.fromList([2])), + ], + processor: (_, filename) async { + processCalls.update(filename, (count) => count + 1, ifAbsent: () => 1); + return resultFor(filename); + }, + committer: (result, filename) async { + final attempt = commitCalls.update(filename, (count) => count + 1, ifAbsent: () => 1); + if (filename == 'b.epub' && attempt == 1) throw Exception('Could not save'); + return bookFor(result); + }, + onCompleted: (books) => completed = books, + ); + await tester.pump(); + + await tester.tap(find.widgetWithText(FilledButton, 'Add 2 to library')); + await tester.pump(); + await tester.pump(); + + expect(find.text('Added'), findsOneWidget); + expect(find.text('Could not save'), findsOneWidget); + expect(completed, isNull); + + await tester.tap(find.widgetWithText(TextButton, 'Retry')); + await tester.pump(); + await tester.pump(); + + expect(processCalls, {'a.epub': 1, 'b.epub': 1}); + expect(commitCalls, {'a.epub': 1, 'b.epub': 2}); + expect(completed, isNotNull); + }); + + testWidgets('removing a commit failure cleans only its temporary result', (tester) async { + final deleted = []; + await pumpResults( + tester, + files: [ + SelectedBookFile(name: 'added.epub', bytes: Uint8List.fromList([1])), + SelectedBookFile(name: 'failed.epub', bytes: Uint8List.fromList([2])), + ], + processor: (_, filename) async => resultFor(filename, bookId: 'temporary-$filename'), + committer: (result, filename) async { + if (filename == 'failed.epub') throw Exception('Commit failed'); + return bookFor(result); + }, + deleter: (bookId) async => deleted.add(bookId), + ); + await tester.pump(); + + await tester.tap(find.widgetWithText(FilledButton, 'Add 2 to library')); + await tester.pump(); + await tester.pump(); + + expect(find.byTooltip('Remove added.epub'), findsNothing); + await tester.tap(find.byTooltip('Remove failed.epub')); + await tester.pump(); + + expect(deleted, ['temporary-failed.epub']); + expect(find.text('failed.epub'), findsNothing); + }); + + testWidgets('disables dismissal and mutable actions while a commit is active', (tester) async { + final commit = Completer(); + var closed = false; + await pumpResults( + tester, + files: [ + SelectedBookFile(name: 'slow.epub', bytes: Uint8List.fromList([1])), + ], + processor: (_, filename) async => resultFor(filename), + committer: (_, _) => commit.future, + onClose: () => closed = true, + ); + await tester.pump(); + + await tester.tap(find.widgetWithText(FilledButton, 'Add 1 to library')); + await tester.pump(); + + final closeButton = find.descendant( + of: find.byKey(const Key('add-book-sheet-header')), + matching: find.byType(IconButton), + ); + expect(tester.widget(closeButton).onPressed, isNull); + expect(tester.widget(find.widgetWithText(TextButton, 'Close')).onPressed, isNull); + expect(tester.widget(find.widgetWithText(FilledButton, 'Add 0 to library')).onPressed, isNull); + await tester.binding.handlePopRoute(); + await tester.pump(); + expect(closed, isFalse); + + commit.complete(bookFor(resultFor('slow.epub'))); + await tester.pump(); + await tester.pump(); + }); + + testWidgets('closing after a partial commit deletes only uncommitted temporary files', (tester) async { + final deleted = []; + await pumpResults( + tester, + files: [ + SelectedBookFile(name: 'added.epub', bytes: Uint8List.fromList([1])), + SelectedBookFile(name: 'failed.epub', bytes: Uint8List.fromList([2])), + ], + processor: (_, filename) async => resultFor(filename, bookId: filename), + committer: (result, filename) async { + if (filename == 'failed.epub') throw Exception('Commit failed'); + return bookFor(result); + }, + deleter: (bookId) async => deleted.add(bookId), + ); + await tester.pump(); + + await tester.tap(find.widgetWithText(FilledButton, 'Add 2 to library')); + await tester.pump(); + await tester.pump(); + await tester.tap(find.widgetWithText(TextButton, 'Close')); + await tester.pump(); + await tester.pump(); + + expect(deleted, ['failed.epub']); + }); + + testWidgets('production route closes and reports the completed book count', (tester) async { + await tester.pumpWidget( + MaterialApp( + home: Builder( + builder: (context) => Scaffold( + body: FilledButton( + onPressed: () => BookImportResultsSheet.show( + context, + files: [ + SelectedBookFile(name: 'one.epub', bytes: Uint8List.fromList([1])), + SelectedBookFile(name: 'two.epub', bytes: Uint8List.fromList([2])), + ], + processor: (_, filename) async => resultFor(filename), + deleteBookFile: (_) async {}, + committer: (result, _) async => bookFor(result), + ), + child: const Text('Open'), + ), + ), + ), + ), + ); + + await tester.tap(find.text('Open')); + await tester.pumpAndSettle(); + await tester.tap(find.widgetWithText(FilledButton, 'Add 2 to library')); + await tester.pumpAndSettle(); + + expect(find.byType(BookImportResultsSheet), findsNothing); + expect(find.text('Added 2 books to library'), findsOneWidget); + }); } From 5044c17e7015aea5a6e982b8cc0a84ae5edaad4d Mon Sep 17 00:00:00 2001 From: Eoic Date: Sun, 2 Aug 2026 01:20:30 +0300 Subject: [PATCH 14/17] PPR-26: Make import footer responsive --- .../add_book/book_import_results_sheet.dart | 17 +++++---- .../book_import_results_sheet_test.dart | 36 +++++++++++++++++++ 2 files changed, 46 insertions(+), 7 deletions(-) diff --git a/app/lib/widgets/add_book/book_import_results_sheet.dart b/app/lib/widgets/add_book/book_import_results_sheet.dart index 2e6805d..8901483 100644 --- a/app/lib/widgets/add_book/book_import_results_sheet.dart +++ b/app/lib/widgets/add_book/book_import_results_sheet.dart @@ -435,18 +435,21 @@ class _BookImportResultsSheetState extends State { ); }, ), - footer: Row( - mainAxisAlignment: MainAxisAlignment.end, + footer: OverflowBar( + alignment: MainAxisAlignment.end, + overflowAlignment: OverflowBarAlignment.end, + spacing: Spacing.sm, + overflowSpacing: Spacing.sm, children: [ TextButton( onPressed: _isClosing || _isAdding ? null : () => unawaited(_requestClose()), child: const Text('Close'), ), - const SizedBox(width: Spacing.sm), - FilledButton( - onPressed: _canAdd ? () => unawaited(_addReadyBooks()) : null, - child: Text('Add $_readyCount to library'), - ), + if (_items.isNotEmpty) + FilledButton( + onPressed: _canAdd ? () => unawaited(_addReadyBooks()) : null, + child: Text('Add $_readyCount to library'), + ), ], ), ), diff --git a/app/test/widgets/add_book/book_import_results_sheet_test.dart b/app/test/widgets/add_book/book_import_results_sheet_test.dart index 4e17133..1d6f661 100644 --- a/app/test/widgets/add_book/book_import_results_sheet_test.dart +++ b/app/test/widgets/add_book/book_import_results_sheet_test.dart @@ -609,4 +609,40 @@ void main() { expect(find.byType(BookImportResultsSheet), findsNothing); expect(find.text('Added 2 books to library'), findsOneWidget); }); + + testWidgets('empty results keep a Close-only footer usable at narrow scaled width', (tester) async { + tester.view.devicePixelRatio = 1; + tester.view.physicalSize = const Size(320, 700); + tester.platformDispatcher.textScaleFactorTestValue = 2; + addTearDown(tester.view.reset); + addTearDown(tester.platformDispatcher.clearTextScaleFactorTestValue); + + await pumpResults(tester, files: const [], processor: (_, filename) async => resultFor(filename)); + + expect(tester.takeException(), isNull); + expect(find.widgetWithText(TextButton, 'Close').hitTestable(), findsOneWidget); + expect(find.byType(FilledButton), findsNothing); + }); + + testWidgets('ready results keep Close and counted Add usable at narrow scaled width', (tester) async { + tester.view.devicePixelRatio = 1; + tester.view.physicalSize = const Size(320, 700); + tester.platformDispatcher.textScaleFactorTestValue = 2; + addTearDown(tester.view.reset); + addTearDown(tester.platformDispatcher.clearTextScaleFactorTestValue); + + await pumpResults( + tester, + files: [ + SelectedBookFile(name: 'ready.epub', bytes: Uint8List.fromList([1])), + ], + processor: (_, filename) async => resultFor(filename), + committer: (result, _) async => bookFor(result), + ); + await tester.pump(); + + expect(tester.takeException(), isNull); + expect(find.widgetWithText(TextButton, 'Close').hitTestable(), findsOneWidget); + expect(find.widgetWithText(FilledButton, 'Add 1 to library').hitTestable(), findsOneWidget); + }); } From db67bf7d4eff983e521c65658ca1ad1775248987 Mon Sep 17 00:00:00 2001 From: Eoic Date: Sun, 2 Aug 2026 01:24:47 +0300 Subject: [PATCH 15/17] PPR-26: Remove legacy book import sheet --- .../widgets/add_book/import_book_sheet.dart | 428 ------------------ .../add_book/add_book_sheets_test.dart | 87 ---- 2 files changed, 515 deletions(-) delete mode 100644 app/lib/widgets/add_book/import_book_sheet.dart diff --git a/app/lib/widgets/add_book/import_book_sheet.dart b/app/lib/widgets/add_book/import_book_sheet.dart deleted file mode 100644 index f6a0349..0000000 --- a/app/lib/widgets/add_book/import_book_sheet.dart +++ /dev/null @@ -1,428 +0,0 @@ -import 'dart:async'; - -import 'package:file_picker/file_picker.dart'; -import 'package:flutter/foundation.dart'; -import 'package:flutter/material.dart'; -import 'package:papyrus/data/data_store.dart'; -import 'package:papyrus/media/media_upload_queue.dart'; -import 'package:papyrus/models/book.dart'; -import 'package:papyrus/powersync/powersync_service.dart'; -import 'package:papyrus/powersync/sync_state.dart'; -import 'package:papyrus/providers/auth_provider.dart'; -import 'package:papyrus/services/book_import_commit_service.dart'; -import 'package:papyrus/services/book_import_service_stub.dart' - if (dart.library.js_interop) 'package:papyrus/services/book_import_service.dart'; -import 'package:papyrus/themes/design_tokens.dart'; -import 'package:papyrus/widgets/shared/bottom_sheet_handle.dart'; -import 'package:papyrus/widgets/shared/bottom_sheet_header.dart'; -import 'package:provider/provider.dart'; - -enum _ImportState { idle, processing, success, error } - -/// Sheet for importing a digital book file. -/// -/// Opens a file picker, processes the file in a Web Worker, -/// previews the extracted metadata, and adds the book to the library. -class ImportBookSheet extends StatelessWidget { - const ImportBookSheet({super.key}) : initialResult = null, initialCommitting = false; - - @visibleForTesting - const ImportBookSheet.withInitialResult(this.initialResult, {this.initialCommitting = false, super.key}); - - final BookImportResult? initialResult; - final bool initialCommitting; - - /// Show the import sheet as a scrollable, content-sized bottom sheet. - static Future show(BuildContext context) { - return showModalBottomSheet( - context: context, - isScrollControlled: true, - useRootNavigator: true, - useSafeArea: true, - shape: const RoundedRectangleBorder(borderRadius: BorderRadius.vertical(top: Radius.circular(AppRadius.xl))), - builder: (_) => const ImportBookSheet(), - ); - } - - @override - Widget build(BuildContext context) { - return SingleChildScrollView( - child: _ImportContent(initialResult: initialResult, initialCommitting: initialCommitting), - ); - } -} - -class _ImportContent extends StatefulWidget { - const _ImportContent({this.initialResult, this.initialCommitting = false}); - - final BookImportResult? initialResult; - final bool initialCommitting; - - @override - State<_ImportContent> createState() => _ImportContentState(); -} - -class _ImportContentState extends State<_ImportContent> { - static const double _pendingContentHeight = 232; - - final _importService = BookImportService(); - late _ImportState _state; - String? _filename; - String? _errorMessage; - BookImportResult? _result; - - @override - void initState() { - super.initState(); - _result = widget.initialResult; - _state = _result == null ? _ImportState.idle : _ImportState.success; - _committing = widget.initialCommitting; - } - - @override - void dispose() { - _importService.dispose(); - super.dispose(); - } - - /// Allowed file extensions per platform. - static const _webExtensions = ['epub']; - static const _nativeExtensions = ['epub', 'pdf', 'mobi', 'azw3', 'txt', 'cbr', 'cbz']; - - /// Schedule a setState that is guaranteed to trigger a frame. - /// - /// On web, setState called from a continuation resumed by a JS callback - /// (e.g. Web Worker message) may not trigger frame scheduling because the - /// callback runs outside Flutter's animation frame context. Wrapping in - /// [Timer.run] pushes the call into the event loop proper. - void _safeSetState(VoidCallback fn) { - Timer.run(() { - if (!mounted) return; - setState(fn); - }); - } - - bool _picking = false; - late bool _committing; - - Future _pickAndProcess() async { - if (_picking || _committing) return; - _picking = true; - - try { - final extensions = kIsWeb ? _webExtensions : _nativeExtensions; - final pickerResult = await FilePicker.platform.pickFiles( - type: FileType.custom, - allowedExtensions: extensions, - withData: true, - ); - - if (!mounted) return; - if (pickerResult == null || pickerResult.files.isEmpty) return; - - final file = pickerResult.files.first; - final bytes = file.bytes; - if (bytes == null) { - _safeSetState(() { - _state = _ImportState.error; - _errorMessage = 'Could not read the selected file.'; - }); - return; - } - - _safeSetState(() { - _state = _ImportState.processing; - _filename = file.name; - _errorMessage = null; - }); - - final results = await Future.wait([ - _importService.importBook(bytes, file.name), - Future.delayed(const Duration(milliseconds: 800)), - ]); - final result = results[0] as BookImportResult; - _safeSetState(() { - _state = _ImportState.success; - _result = result; - }); - } catch (e) { - _safeSetState(() { - _state = _ImportState.error; - _errorMessage = e.toString(); - }); - } finally { - _picking = false; - } - } - - Future _addToLibrary() async { - if (_committing) return; - final result = _result; - if (result == null) return; - setState(() => _committing = true); - - Book? committedBook; - try { - final dataStore = context.read(); - final bookRepository = dataStore.requireBookRepository(); - final queue = context.read(); - final importService = context.read(); - final authProvider = context.read(); - final powerSyncService = context.read(); - final isOnlineAccount = authProvider.isSignedIn && powerSyncService.mode == LibraryDatabaseMode.authenticated; - final accountScope = isOnlineAccount ? queue.activeScope : null; - if (isOnlineAccount && accountScope == null) { - throw StateError('Cannot import account media without an active media storage scope'); - } - - final ext = result.fileExtension; - final filePath = kIsWeb - ? 'opfs://books/${result.bookId}.$ext' - : result.bookId; // Native resolves via BookImportService.getBookFile - final commitService = BookImportCommitService( - storePendingCover: importService.storePendingCoverFile, - storeGuestCover: importService.storeGuestCoverFile, - deletePendingCover: importService.deletePendingCoverFile, - deleteGuestCover: importService.deleteGuestCoverFile, - addBook: (book) => dataStore.addBookToRepositoryAndWait(bookRepository, book), - deleteBook: (bookId) => dataStore.deleteBookFromRepositoryAndWait(bookRepository, bookId), - enqueueImportedBookMedia: queue.enqueueImportedBookMedia, - isLibraryContextCurrent: () { - final currentIsOnlineAccount = - authProvider.isSignedIn && powerSyncService.mode == LibraryDatabaseMode.authenticated; - return dataStore.isBookRepositoryCurrent(bookRepository) && - currentIsOnlineAccount == isOnlineAccount && - queue.activeScope == accountScope; - }, - ); - committedBook = await commitService.commit( - result: result, - sourceFilename: _filename ?? '${result.bookId}.$ext', - addedAt: DateTime.now(), - localFilePath: filePath, - accountScope: accountScope, - ); - } catch (error) { - if (!mounted) return; - setState(() { - _committing = false; - _state = _ImportState.error; - _errorMessage = error.toString(); - }); - } - - if (!mounted || committedBook == null) return; - final messenger = ScaffoldMessenger.of(context); - Navigator.of(context).pop(); - messenger.showSnackBar(SnackBar(content: Text('Added "${committedBook.title}" to library'))); - } - - @override - Widget build(BuildContext context) { - return Column( - mainAxisSize: MainAxisSize.min, - children: [ - Padding( - padding: const EdgeInsets.fromLTRB(Spacing.md, Spacing.md, Spacing.md, 0), - child: Column( - children: [ - const BottomSheetHandle(), - const SizedBox(height: Spacing.md), - BottomSheetHeader(title: 'Import book', onCancel: () => Navigator.of(context).pop()), - ], - ), - ), - const SizedBox(height: Spacing.md), - const Divider(height: 1), - Padding( - padding: const EdgeInsets.symmetric(horizontal: Spacing.lg, vertical: Spacing.md), - child: switch (_state) { - _ImportState.idle => _buildIdleState(context), - _ImportState.processing => _buildProcessingState(context), - _ImportState.success => _buildSuccessState(context), - _ImportState.error => _buildErrorState(context), - }, - ), - ], - ); - } - - Widget _buildIdleState(BuildContext context) { - final colorScheme = Theme.of(context).colorScheme; - final textTheme = Theme.of(context).textTheme; - - return _buildPendingContent( - Column( - mainAxisSize: MainAxisSize.min, - children: [ - Icon(Icons.upload_file, size: 48, color: colorScheme.primary), - const SizedBox(height: Spacing.md), - Text('Select a digital book file', style: textTheme.titleMedium), - const SizedBox(height: Spacing.xs), - Text( - 'EPUB, PDF, AZW3, MOBI, CBZ/CBR', - style: textTheme.bodySmall?.copyWith(color: colorScheme.onSurfaceVariant), - ), - const SizedBox(height: Spacing.lg), - FilledButton.icon( - onPressed: _committing ? null : _pickAndProcess, - icon: const Icon(Icons.folder_open), - label: const Text('Browse files'), - ), - ], - ), - ); - } - - Widget _buildProcessingState(BuildContext context) { - final colorScheme = Theme.of(context).colorScheme; - final textTheme = Theme.of(context).textTheme; - - return _buildPendingContent( - Column( - mainAxisSize: MainAxisSize.min, - children: [ - const SizedBox(width: 48, height: 48, child: CircularProgressIndicator()), - const SizedBox(height: Spacing.lg), - Text('Processing...', style: textTheme.titleMedium), - if (_filename != null) ...[ - const SizedBox(height: Spacing.xs), - Text(_filename!, style: textTheme.bodySmall?.copyWith(color: colorScheme.onSurfaceVariant)), - ], - ], - ), - ); - } - - Widget _buildPendingContent(Widget child) { - return SizedBox( - key: const Key('import-pending-content'), - width: double.infinity, - height: _pendingContentHeight, - child: Center(child: child), - ); - } - - Widget _buildSuccessState(BuildContext context) { - final colorScheme = Theme.of(context).colorScheme; - final textTheme = Theme.of(context).textTheme; - final result = _result!; - - return Column( - crossAxisAlignment: CrossAxisAlignment.start, - children: [ - Row( - crossAxisAlignment: CrossAxisAlignment.start, - children: [ - // Cover preview - Container( - width: 80, - height: 120, - decoration: BoxDecoration( - color: colorScheme.surfaceContainerHighest, - borderRadius: BorderRadius.circular(AppRadius.sm), - ), - clipBehavior: Clip.antiAlias, - child: result.coverImage != null - ? Image.memory(result.coverImage!, fit: BoxFit.cover) - : Center(child: Icon(Icons.menu_book, size: 32, color: colorScheme.onSurfaceVariant)), - ), - const SizedBox(width: Spacing.md), - Expanded( - child: Column( - crossAxisAlignment: CrossAxisAlignment.start, - children: [ - Text(result.title, style: textTheme.titleMedium), - const SizedBox(height: Spacing.xs), - Text(result.author, style: textTheme.bodyMedium?.copyWith(color: colorScheme.onSurfaceVariant)), - if (result.pageCount != null) ...[ - const SizedBox(height: Spacing.xs), - Text( - '~${result.pageCount} pages', - style: textTheme.bodySmall?.copyWith(color: colorScheme.onSurfaceVariant), - ), - ], - ], - ), - ), - ], - ), - const SizedBox(height: Spacing.lg), - Row( - children: [ - Expanded( - child: OutlinedButton( - onPressed: _committing - ? null - : () { - setState(() { - _state = _ImportState.idle; - _result = null; - _filename = null; - }); - }, - style: OutlinedButton.styleFrom( - shape: const StadiumBorder(), - disabledForegroundColor: colorScheme.primary, - side: BorderSide(color: colorScheme.outline, width: BorderWidths.thin), - ), - child: const Text('Pick different file'), - ), - ), - const SizedBox(width: Spacing.md), - Expanded( - child: FilledButton( - onPressed: _committing ? null : _addToLibrary, - style: FilledButton.styleFrom( - shape: const StadiumBorder(), - disabledBackgroundColor: colorScheme.primary, - disabledForegroundColor: colorScheme.onPrimary, - ), - child: _committing - ? Row( - mainAxisSize: MainAxisSize.min, - children: [ - SizedBox.square( - dimension: 16, - child: CircularProgressIndicator(strokeWidth: 2, color: colorScheme.onPrimary), - ), - const SizedBox(width: Spacing.sm), - const Text('Adding...'), - ], - ) - : const Text('Add to library'), - ), - ), - ], - ), - ], - ); - } - - Widget _buildErrorState(BuildContext context) { - final colorScheme = Theme.of(context).colorScheme; - final textTheme = Theme.of(context).textTheme; - - return Container( - width: double.infinity, - padding: const EdgeInsets.all(Spacing.lg), - decoration: BoxDecoration( - border: Border.all(color: colorScheme.error.withValues(alpha: 0.5)), - borderRadius: BorderRadius.circular(AppRadius.lg), - ), - child: Column( - children: [ - Icon(Icons.error_outline, size: 48, color: colorScheme.error), - const SizedBox(height: Spacing.md), - Text( - _errorMessage ?? 'Something went wrong', - style: textTheme.bodyMedium?.copyWith(color: colorScheme.error), - textAlign: TextAlign.center, - ), - const SizedBox(height: Spacing.lg), - FilledButton(onPressed: _committing ? null : _pickAndProcess, child: const Text('Try again')), - ], - ), - ); - } -} diff --git a/app/test/widgets/add_book/add_book_sheets_test.dart b/app/test/widgets/add_book/add_book_sheets_test.dart index d446866..6a8a823 100644 --- a/app/test/widgets/add_book/add_book_sheets_test.dart +++ b/app/test/widgets/add_book/add_book_sheets_test.dart @@ -3,13 +3,10 @@ import 'dart:typed_data'; import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:papyrus/services/book_import_result.dart'; -import 'package:papyrus/themes/app_theme.dart'; import 'package:papyrus/widgets/add_book/add_book_choice_sheet.dart'; import 'package:papyrus/widgets/add_book/add_physical_book_sheet.dart'; import 'package:papyrus/widgets/add_book/book_import_batch_item.dart'; -import 'package:papyrus/widgets/add_book/import_book_sheet.dart'; import 'package:papyrus/widgets/shared/bottom_sheet_handle.dart'; -import 'package:papyrus/widgets/shared/bottom_sheet_header.dart'; class _CountingNavigatorObserver extends NavigatorObserver { int pushCount = 0; @@ -21,16 +18,6 @@ class _CountingNavigatorObserver extends NavigatorObserver { } } -const importedBook = BookImportResult( - bookId: 'book-1', - title: 'Frankenstein', - author: 'Mary Wollstonecraft Shelley', - pageCount: 239, - fileSize: 1024, - fileHash: 'hash', - fileExtension: 'epub', -); - void main() { Future pumpLauncher( WidgetTester tester, @@ -143,41 +130,6 @@ void main() { expect(find.text('Open page'), findsNothing); }); - testWidgets('import book opens as a format-neutral bottom sheet on desktop', (tester) async { - await pumpLauncher( - tester, - (context) => - () => ImportBookSheet.show(context), - ); - await tester.tap(find.text('Open')); - await tester.pumpAndSettle(); - - expect(find.byType(BottomSheet), findsOneWidget); - expect(find.byType(Dialog), findsNothing); - expect(find.byType(BottomSheetHandle), findsOneWidget); - expect(find.byType(BottomSheetHeader), findsOneWidget); - expect(find.text('Cancel'), findsOneWidget); - expect(find.byIcon(Icons.close), findsNothing); - expect(find.text('Select a digital book file'), findsOneWidget); - expect(find.text('EPUB, PDF, AZW3, MOBI, CBZ/CBR'), findsOneWidget); - expect(find.text('Select an EPUB file'), findsNothing); - expect(find.text('The file will be stored offline on this device'), findsNothing); - expect(find.byKey(const Key('import-pending-content')), findsOneWidget); - expect(tester.getSize(find.byKey(const Key('import-pending-content'))).height, 232); - expect( - find.ancestor( - of: find.text('Select a digital book file'), - matching: find.byWidgetPredicate( - (widget) => - widget is Container && - widget.decoration is BoxDecoration && - (widget.decoration! as BoxDecoration).border != null, - ), - ), - findsNothing, - ); - }); - testWidgets('digital selection and results use distinct modal routes', (tester) async { final observer = _CountingNavigatorObserver(); final selected = SelectedBookFile(name: 'selected.epub', bytes: Uint8List.fromList([1])); @@ -251,45 +203,6 @@ void main() { expect(observer.pushCount, initialPushCount + 1); }); - testWidgets('successful import actions can be rendered for widget verification', (tester) async { - await tester.pumpWidget(const MaterialApp(home: Scaffold(body: ImportBookSheet.withInitialResult(importedBook)))); - - expect(find.text('Pick different file'), findsOneWidget); - expect(find.text('Add to library'), findsOneWidget); - - final pickButton = tester.widget(find.widgetWithText(OutlinedButton, 'Pick different file')); - final addButton = tester.widget(find.widgetWithText(FilledButton, 'Add to library')); - - expect(pickButton.style?.shape?.resolve({}), isA()); - expect(addButton.style?.shape?.resolve({}), isA()); - }); - - testWidgets('committing import actions stay visually stable and show progress', (tester) async { - await tester.pumpWidget( - MaterialApp( - theme: AppTheme.dark, - home: const Scaffold(body: ImportBookSheet.withInitialResult(importedBook, initialCommitting: true)), - ), - ); - - final pickButton = tester.widget(find.widgetWithText(OutlinedButton, 'Pick different file')); - final addButton = tester.widget(find.byType(FilledButton)); - - expect(pickButton.onPressed, isNull); - expect(addButton.onPressed, isNull); - expect(find.text('Adding...'), findsOneWidget); - expect(find.text('Add to library'), findsNothing); - expect(find.byType(CircularProgressIndicator), findsOneWidget); - - final colorScheme = Theme.of(tester.element(find.text('Adding...'))).colorScheme; - const disabled = {WidgetState.disabled}; - - expect(pickButton.style?.foregroundColor?.resolve(disabled), colorScheme.primary); - expect(pickButton.style?.side?.resolve(disabled)?.color, colorScheme.outline); - expect(addButton.style?.backgroundColor?.resolve(disabled), colorScheme.primary); - expect(addButton.style?.foregroundColor?.resolve(disabled), colorScheme.onPrimary); - }); - testWidgets('physical import places Add in the fixed footer', (tester) async { await pumpLauncher( tester, From 0d0723d5606f35000b4b361b13a076ca60cebcbd Mon Sep 17 00:00:00 2001 From: Eoic Date: Sun, 2 Aug 2026 01:41:42 +0300 Subject: [PATCH 16/17] PPR-26: Await late import cleanup --- .../add_book/book_import_results_sheet.dart | 79 ++++++++++++++----- .../book_import_results_sheet_test.dart | 67 +++++++++++++++- 2 files changed, 126 insertions(+), 20 deletions(-) diff --git a/app/lib/widgets/add_book/book_import_results_sheet.dart b/app/lib/widgets/add_book/book_import_results_sheet.dart index 8901483..e362c2c 100644 --- a/app/lib/widgets/add_book/book_import_results_sheet.dart +++ b/app/lib/widgets/add_book/book_import_results_sheet.dart @@ -134,6 +134,7 @@ class BookImportResultsSheet extends StatefulWidget { class _BookImportResultsSheetState extends State { late List _items; final Map _processingTokens = {}; + final Map> _processingFutures = {}; final Set _removingIds = {}; final Set _cleanedBookIds = {}; final Map> _cleanupFutures = {}; @@ -153,17 +154,33 @@ class _BookImportResultsSheetState extends State { ); WidgetsBinding.instance.addPostFrameCallback((_) { for (final item in List.of(_items)) { - unawaited(_process(item.id)); + unawaited(_startProcessing(item.id)); } }); } int _indexOf(String id) => _items.indexWhere((item) => item.id == id); - Future _process(String id) async { - if (_isClosing || !mounted) return; + Future _startProcessing(String id) { + final inFlight = _processingFutures[id]; + if (inFlight != null) return inFlight; + + final processing = _process(id); + _processingFutures[id] = processing; + unawaited( + processing.whenComplete(() { + if (identical(_processingFutures[id], processing)) { + _processingFutures.remove(id); + } + }), + ); + return processing; + } + + Future _process(String id) async { + if (_isClosing || !mounted) return true; final index = _indexOf(id); - if (index < 0) return; + if (index < 0) return true; final token = (_processingTokens[id] ?? 0) + 1; _processingTokens[id] = token; @@ -172,35 +189,50 @@ class _BookImportResultsSheetState extends State { final bytes = processingItem.file.bytes; if (bytes == null) { - if (!_isCurrentProcessing(id, token)) return; + if (!_isCurrentProcessing(id, token)) return true; setState(() { final currentIndex = _indexOf(id); _items[currentIndex] = _items[currentIndex].processingFailed('Could not read this file.'); }); - return; + return true; } try { final result = await widget.processor(bytes, processingItem.file.name); - if (_isClosing || !mounted || !_isCurrentProcessing(id, token)) { - await _deleteTemporary(result.bookId); - return; + if (_isClosing) { + final deleted = await _deleteTemporary(result.bookId); + if (!deleted && _isMatchingProcessing(id, token)) { + setState(() { + final currentIndex = _indexOf(id); + _items[currentIndex] = _items[currentIndex].processingSucceeded(result); + }); + } + return deleted; + } + if (!mounted || !_isMatchingProcessing(id, token)) { + return _deleteTemporary(result.bookId); } setState(() { final currentIndex = _indexOf(id); _items[currentIndex] = _items[currentIndex].processingSucceeded(result); }); + return true; } catch (error) { - if (!_isCurrentProcessing(id, token)) return; + if (!_isCurrentProcessing(id, token)) return true; setState(() { final currentIndex = _indexOf(id); _items[currentIndex] = _items[currentIndex].processingFailed(_safeErrorMessage(error)); }); + return true; } } bool _isCurrentProcessing(String id, int token) { - if (_isClosing || !mounted || _processingTokens[id] != token) return false; + return !_isClosing && _isMatchingProcessing(id, token); + } + + bool _isMatchingProcessing(String id, int token) { + if (!mounted || _processingTokens[id] != token) return false; final index = _indexOf(id); return index >= 0 && _items[index].status == BookImportBatchStatus.processing; } @@ -388,21 +420,30 @@ class _BookImportResultsSheetState extends State { _isClosing = true; } + final processingResults = await Future.wait(List>.of(_processingFutures.values)); + if (processingResults.any((cleaned) => !cleaned)) { + _restoreAfterCloseFailure(); + return; + } + final bookIds = _items.where((item) => item.hasTemporaryFile).map((item) => item.result!.bookId).toSet(); final cleanupResults = await Future.wait(bookIds.map(_deleteTemporary)); if (cleanupResults.any((deleted) => !deleted)) { - if (mounted) { - setState(() => _isClosing = false); - ScaffoldMessenger.maybeOf( - context, - )?.showSnackBar(const SnackBar(content: Text('Could not remove temporary files. Please try again.'))); - } + _restoreAfterCloseFailure(); return; } if (!mounted) return; widget.onClose(); } + void _restoreAfterCloseFailure() { + if (!mounted) return; + setState(() => _isClosing = false); + ScaffoldMessenger.maybeOf( + context, + )?.showSnackBar(const SnackBar(content: Text('Could not remove temporary files. Please try again.'))); + } + @override Widget build(BuildContext context) { return PopScope( @@ -429,7 +470,9 @@ class _BookImportResultsSheetState extends State { onRetry: _isClosing || _isAdding ? null : () => unawaited( - item.status == BookImportBatchStatus.commitFailed ? _retryCommit(item.id) : _process(item.id), + item.status == BookImportBatchStatus.commitFailed + ? _retryCommit(item.id) + : _startProcessing(item.id), ), onRemove: _isClosing || _isAdding ? null : () => unawaited(_remove(item.id)), ); diff --git a/app/test/widgets/add_book/book_import_results_sheet_test.dart b/app/test/widgets/add_book/book_import_results_sheet_test.dart index 1d6f661..e34acd7 100644 --- a/app/test/widgets/add_book/book_import_results_sheet_test.dart +++ b/app/test/widgets/add_book/book_import_results_sheet_test.dart @@ -333,7 +333,7 @@ void main() { expect(find.byType(BookImportResultsSheet), findsNothing); }); - testWidgets('a success arriving after close is deleted without updating disposed state', (tester) async { + testWidgets('close waits for a late success to be cleaned before disposing state', (tester) async { final processing = Completer(); final deleted = []; await tester.pumpWidget( @@ -362,13 +362,76 @@ void main() { await tester.tap(find.widgetWithText(TextButton, 'Close')); await tester.pump(); await tester.pump(const Duration(seconds: 1)); + expect(find.byType(BookImportResultsSheet), findsOneWidget); + + processing.complete(resultFor('slow.epub', bookId: 'late-book')); + await tester.pump(); + await tester.pump(const Duration(seconds: 1)); + + expect(deleted, ['late-book']); expect(find.byType(BookImportResultsSheet), findsNothing); + expect(tester.takeException(), isNull); + }); + + testWidgets('failed late-success cleanup keeps close retryable without reprocessing', (tester) async { + final processing = Completer(); + final observer = _PopCountingObserver(); + var processorCalls = 0; + var deleteAttempts = 0; + + await tester.pumpWidget( + MaterialApp( + navigatorObservers: [observer], + home: Builder( + builder: (context) => Scaffold( + body: FilledButton( + onPressed: () => BookImportResultsSheet.show( + context, + files: [ + SelectedBookFile(name: 'slow.epub', bytes: Uint8List.fromList([1])), + ], + processor: (_, _) { + processorCalls++; + return processing.future; + }, + deleteBookFile: (_) async { + deleteAttempts++; + if (deleteAttempts == 1) throw Exception('private cleanup detail'); + }, + ), + child: const Text('Open'), + ), + ), + ), + ), + ); + await tester.tap(find.text('Open')); + await tester.pump(); + await tester.pump(const Duration(seconds: 1)); + + await tester.tap(find.widgetWithText(TextButton, 'Close')); + await tester.pump(); + expect(find.byType(BookImportResultsSheet), findsOneWidget); processing.complete(resultFor('slow.epub', bookId: 'late-book')); await tester.pump(); + await tester.pump(const Duration(seconds: 1)); + + expect(processorCalls, 1); + expect(deleteAttempts, 1); + expect(observer.pops, 0); + expect(find.byType(BookImportResultsSheet), findsOneWidget); + expect(find.text('Could not remove temporary files. Please try again.'), findsOneWidget); + expect(tester.widget(find.widgetWithText(TextButton, 'Close')).onPressed, isNotNull); + + await tester.tap(find.widgetWithText(TextButton, 'Close')); await tester.pump(); + await tester.pump(const Duration(seconds: 1)); - expect(deleted, ['late-book']); + expect(processorCalls, 1); + expect(deleteAttempts, 2); + expect(observer.pops, 1); + expect(find.byType(BookImportResultsSheet), findsNothing); expect(tester.takeException(), isNull); }); From 183a1b34675b3fa8ceae01374aeb0cebfbfaed80 Mon Sep 17 00:00:00 2001 From: Eoic Date: Sun, 2 Aug 2026 18:29:28 +0300 Subject: [PATCH 17/17] PPR-26: Refactor book import functionality to unify import sheet and streamline processing --- .../add_book/add_book_choice_sheet.dart | 11 +- .../widgets/add_book/book_import_sheet.dart | 1151 +++++++++++++++++ 2 files changed, 1154 insertions(+), 8 deletions(-) create mode 100644 app/lib/widgets/add_book/book_import_sheet.dart diff --git a/app/lib/widgets/add_book/add_book_choice_sheet.dart b/app/lib/widgets/add_book/add_book_choice_sheet.dart index 04e2631..6a14503 100644 --- a/app/lib/widgets/add_book/add_book_choice_sheet.dart +++ b/app/lib/widgets/add_book/add_book_choice_sheet.dart @@ -1,8 +1,7 @@ import 'package:flutter/material.dart'; import 'package:papyrus/themes/design_tokens.dart'; import 'package:papyrus/widgets/add_book/add_physical_book_sheet.dart'; -import 'package:papyrus/widgets/add_book/book_import_results_sheet.dart'; -import 'package:papyrus/widgets/add_book/digital_book_import_sheet.dart'; +import 'package:papyrus/widgets/add_book/book_import_sheet.dart'; import 'package:papyrus/widgets/shared/bottom_sheet_handle.dart'; /// Choice sheet for selecting digital import, physical entry, or optional online search. @@ -48,13 +47,9 @@ class AddBookChoiceSheet extends StatefulWidget { switch (choice) { case _AddBookChoice.importDigital: - final files = await DigitalBookImportSheet.show(context, pickFiles: digitalFilePicker); - if (!context.mounted || files == null || files.isEmpty) { - return; - } - await BookImportResultsSheet.show( + await BookImportSheet.show( context, - files: files, + pickFiles: digitalFilePicker, processor: bookImportProcessor, deleteBookFile: deleteImportedBookFile, ); diff --git a/app/lib/widgets/add_book/book_import_sheet.dart b/app/lib/widgets/add_book/book_import_sheet.dart new file mode 100644 index 0000000..81673c6 --- /dev/null +++ b/app/lib/widgets/add_book/book_import_sheet.dart @@ -0,0 +1,1151 @@ +import 'dart:async'; + +import 'package:file_picker/file_picker.dart'; +import 'package:flutter/foundation.dart'; +import 'package:flutter/material.dart'; +import 'package:papyrus/data/data_store.dart'; +import 'package:papyrus/media/media_upload_queue.dart'; +import 'package:papyrus/models/book.dart'; +import 'package:papyrus/powersync/powersync_service.dart'; +import 'package:papyrus/powersync/sync_state.dart'; +import 'package:papyrus/providers/auth_provider.dart'; +import 'package:papyrus/services/book_import_commit_service.dart'; +import 'package:papyrus/services/book_import_service_stub.dart' + if (dart.library.js_interop) 'package:papyrus/services/book_import_service.dart'; +import 'package:papyrus/themes/design_tokens.dart'; +import 'package:papyrus/widgets/add_book/add_book_sheet_scaffold.dart'; +import 'package:papyrus/widgets/add_book/book_import_batch_item.dart'; +import 'package:provider/provider.dart'; + +// --------------------------------------------------------------------------- +// Type aliases (re-exported from the existing two sheets for compatibility) +// --------------------------------------------------------------------------- + +typedef DigitalBookFilePicker = Future> Function(); +typedef BookImportProcessor = Future Function(Uint8List bytes, String filename); +typedef ImportedBookFileDeleter = Future Function(String bookId); +typedef ImportedBookCommitter = Future Function(BookImportResult result, String sourceFilename); + +// --------------------------------------------------------------------------- +// Helper: human-readable file size +// --------------------------------------------------------------------------- + +String _formatSize(int bytes) { + if (bytes < 1024) return '$bytes B'; + if (bytes < 1024 * 1024) return '${(bytes / 1024).toStringAsFixed(1)} KB'; + return '${(bytes / (1024 * 1024)).toStringAsFixed(1)} MB'; +} + +// --------------------------------------------------------------------------- +// Helper: icon per extension +// --------------------------------------------------------------------------- + +IconData _iconForExtension(String name) { + final ext = name.toLowerCase().split('.').last; + return switch (ext) { + 'pdf' => Icons.picture_as_pdf, + 'epub' || 'mobi' || 'azw3' => Icons.menu_book, + 'cbz' || 'cbr' => Icons.folder_zip, + 'txt' => Icons.text_snippet, + _ => Icons.insert_drive_file, + }; +} + +// --------------------------------------------------------------------------- +// Internal phases +// --------------------------------------------------------------------------- + +enum _ImportPhase { selecting, processing, summary } + +// --------------------------------------------------------------------------- +// BookImportSheet +// --------------------------------------------------------------------------- + +/// A unified import sheet that handles the full journey: +/// 1. File selection +/// 2. Automatic parse + save pipeline (no manual "Add to library" step) +/// 3. Inline summary showing successes and failures +class BookImportSheet extends StatefulWidget { + const BookImportSheet({ + super.key, + required this.pickFiles, + required this.processor, + required this.deleteBookFile, + required this.committer, + required this.onClose, + this.onCompleted, + this.scrollController, + }); + + final DigitalBookFilePicker pickFiles; + final BookImportProcessor processor; + final ImportedBookFileDeleter deleteBookFile; + final ImportedBookCommitter committer; + final VoidCallback onClose; + final ValueChanged>? onCompleted; + final ScrollController? scrollController; + + // -- Standard FilePicker launchers ----------------------------------------- + + static const _webExtensions = ['epub']; + static const _nativeExtensions = ['epub', 'pdf', 'mobi', 'azw3', 'txt', 'cbr', 'cbz']; + + static Future> defaultPickFiles() async { + final result = await FilePicker.platform.pickFiles( + type: FileType.custom, + allowedExtensions: kIsWeb ? _webExtensions : _nativeExtensions, + allowMultiple: true, + withData: true, + ); + if (result == null) return const []; + return result.files.map((file) => SelectedBookFile(name: file.name, bytes: file.bytes)).toList(); + } + + // -- Root-level modal entry ------------------------------------------------- + + /// Opens the unified import sheet as a root-level modal bottom sheet. + /// + /// When [processor] / [deleteBookFile] / [committer] are not provided they + /// are resolved from the widget tree via [Provider]. + static Future show( + BuildContext context, { + DigitalBookFilePicker? pickFiles, + BookImportProcessor? processor, + ImportedBookFileDeleter? deleteBookFile, + ImportedBookCommitter? committer, + }) { + final importService = processor == null || deleteBookFile == null ? context.read() : null; + final effectiveProcessor = processor ?? importService!.importBook; + final effectiveDeleter = deleteBookFile ?? importService!.deleteBookFile; + final effectiveCommitter = + committer ?? (BookImportResult result, String filename) => _commitResult(context, result, filename); + + return showModalBottomSheet( + context: context, + isScrollControlled: true, + useRootNavigator: true, + useSafeArea: true, + shape: const RoundedRectangleBorder(borderRadius: BorderRadius.vertical(top: Radius.circular(AppRadius.xl))), + builder: (sheetContext) { + final effectivePickFiles = pickFiles ?? defaultPickFiles; + return DraggableScrollableSheet( + initialChildSize: 0.6, + minChildSize: 0.4, + maxChildSize: 0.95, + expand: false, + builder: (_, scrollController) => BookImportSheet( + pickFiles: effectivePickFiles, + processor: effectiveProcessor, + deleteBookFile: effectiveDeleter, + committer: effectiveCommitter, + scrollController: scrollController, + onClose: () => Navigator.of(sheetContext).pop(), + onCompleted: (books) { + final messenger = ScaffoldMessenger.maybeOf(context); + final count = books.length; + messenger?.showSnackBar( + SnackBar(content: Text('$count ${count == 1 ? 'book' : 'books'} added to library')), + ); + }, + ), + ); + }, + ); + } + + /// Resolves the commit service from the widget tree (identical logic to the + /// old [BookImportResultsSheet._commitResult]). + static Future _commitResult(BuildContext context, BookImportResult result, String sourceFilename) async { + final dataStore = context.read(); + final bookRepository = dataStore.requireBookRepository(); + final queue = context.read(); + final importService = context.read(); + final authProvider = context.read(); + final powerSyncService = context.read(); + final isOnlineAccount = authProvider.isSignedIn && powerSyncService.mode == LibraryDatabaseMode.authenticated; + final accountScope = isOnlineAccount ? queue.activeScope : null; + if (isOnlineAccount && accountScope == null) { + throw StateError('Cannot import account media without an active media storage scope'); + } + + final extension = result.fileExtension; + final filePath = kIsWeb ? 'opfs://books/${result.bookId}.$extension' : result.bookId; + final commitService = BookImportCommitService( + storePendingCover: importService.storePendingCoverFile, + storeGuestCover: importService.storeGuestCoverFile, + deletePendingCover: importService.deletePendingCoverFile, + deleteGuestCover: importService.deleteGuestCoverFile, + addBook: (book) => dataStore.addBookToRepositoryAndWait(bookRepository, book), + deleteBook: (bookId) => dataStore.deleteBookFromRepositoryAndWait(bookRepository, bookId), + enqueueImportedBookMedia: queue.enqueueImportedBookMedia, + isLibraryContextCurrent: () { + final currentIsOnlineAccount = + authProvider.isSignedIn && powerSyncService.mode == LibraryDatabaseMode.authenticated; + return dataStore.isBookRepositoryCurrent(bookRepository) && + currentIsOnlineAccount == isOnlineAccount && + queue.activeScope == accountScope; + }, + ); + return commitService.commit( + result: result, + sourceFilename: sourceFilename, + addedAt: DateTime.now(), + localFilePath: filePath, + accountScope: accountScope, + ); + } + + @override + State createState() => _BookImportSheetState(); +} + +class _BookImportSheetState extends State { + // -- State ----------------------------------------------------------------- + + _ImportPhase _phase = _ImportPhase.selecting; + List _files = const []; + List _items = const []; + bool _isPicking = false; + String? _pickerError; + bool _isClosing = false; + bool _didComplete = false; + + // Processing bookkeeping + final Map _processingTokens = {}; + final Map> _processingFutures = {}; + final Set _cleanedBookIds = {}; + final Map> _cleanupFutures = {}; + final List _addedBooks = []; + Future? _closeFuture; + + // -- Convenience getters --------------------------------------------------- + + List get _readableFiles => _files.where((file) => file.bytes != null).toList(growable: false); + + bool get _allSettled => _items.isNotEmpty && _items.every((item) => item.isSettled); + + bool get _anyProcessing => _items.any( + (item) => + item.status == BookImportBatchStatus.queued || + item.status == BookImportBatchStatus.processing || + item.status == BookImportBatchStatus.adding, + ); + + int get _successCount => _items.where((item) => item.status == BookImportBatchStatus.added).length; + + int get _failureCount => _items.length - _successCount; + + // -- File picking ---------------------------------------------------------- + + Future _browse() async { + if (_isPicking) return; + setState(() { + _isPicking = true; + _pickerError = null; + }); + try { + final files = await widget.pickFiles(); + if (!mounted || files.isEmpty) return; + setState(() => _files = List.unmodifiable(files)); + } catch (_) { + if (!mounted) return; + setState(() => _pickerError = 'Could not open the selected files. Please try again.'); + } finally { + if (mounted) setState(() => _isPicking = false); + } + } + + void _removeFile(SelectedBookFile file) { + setState(() => _files = List.unmodifiable(_files.where((f) => !identical(f, file)))); + } + + /// Discards the current selection and returns the sheet to the browse view. + void _clearSelection() { + setState(() { + _files = const []; + _pickerError = null; + }); + } + + // -- Start processing ------------------------------------------------------ + + Future _startImport() async { + if (_readableFiles.isEmpty) return; + setState(() { + _phase = _ImportPhase.processing; + _items = List.generate( + _readableFiles.length, + (i) => BookImportBatchItem.queued(id: 'import-$i', file: _readableFiles[i]), + growable: true, + ); + }); + // Kick off all files concurrently + for (final item in List.of(_items)) { + unawaited(_startProcessing(item.id)); + } + } + + // -- Processing pipeline (parse → commit for each file) -------------------- + + int _indexOf(String id) => _items.indexWhere((item) => item.id == id); + + Future _startProcessing(String id) { + final inFlight = _processingFutures[id]; + if (inFlight != null) return inFlight; + final processing = _process(id); + _processingFutures[id] = processing; + unawaited( + processing.whenComplete(() { + if (identical(_processingFutures[id], processing)) { + _processingFutures.remove(id); + } + }), + ); + return processing; + } + + Future _process(String id) async { + if (_isClosing || !mounted) return true; + final index = _indexOf(id); + if (index < 0) return true; + + final token = (_processingTokens[id] ?? 0) + 1; + _processingTokens[id] = token; + final processingItem = _items[index].startProcessing(); + setState(() => _items[index] = processingItem); + + final bytes = processingItem.file.bytes; + if (bytes == null) { + if (!_isCurrentProcessing(id, token)) return true; + setState(() { + final i = _indexOf(id); + _items[i] = _items[i].processingFailed('Could not read this file.'); + }); + _maybeTransitionToSummary(); + return true; + } + + // Phase A: parse + BookImportResult result; + try { + result = await widget.processor(bytes, processingItem.file.name); + } catch (error) { + if (!_isCurrentProcessing(id, token)) return true; + setState(() { + final i = _indexOf(id); + _items[i] = _items[i].processingFailed(_safeErrorMessage(error)); + }); + _maybeTransitionToSummary(); + return true; + } + + if (_isClosing) { + await _deleteTemporary(result.bookId); + return false; + } + if (!mounted || !_isMatchingProcessing(id, token)) { + return _deleteTemporary(result.bookId); + } + + setState(() { + final i = _indexOf(id); + _items[i] = _items[i].processingSucceeded(result); + }); + + // Phase B: commit (automatic, no user intervention) + setState(() { + final i = _indexOf(id); + if (i >= 0 && _items[i].status == BookImportBatchStatus.ready) { + _items[i] = _items[i].startAdding(); + } + }); + + try { + final book = await widget.committer(result, processingItem.file.name); + if (!mounted) return true; + final i = _indexOf(id); + if (i < 0 || _items[i].status != BookImportBatchStatus.adding) return true; + + setState(() { + _items[i] = _items[i].added(); + _addedBooks.add(book); + }); + } catch (error) { + if (!mounted) return true; + final i = _indexOf(id); + if (i < 0 || _items[i].status != BookImportBatchStatus.adding) return true; + setState(() { + _items[i] = _items[i].commitFailed(_safeErrorMessage(error)); + }); + } + + _maybeTransitionToSummary(); + return true; + } + + void _maybeTransitionToSummary() { + if (_phase != _ImportPhase.processing || !_allSettled) return; + setState(() => _phase = _ImportPhase.summary); + if (_didComplete) return; + _didComplete = true; + final books = List.unmodifiable(_addedBooks); + widget.onCompleted?.call(books); + } + + // -- Retry a failed item --------------------------------------------------- + + Future _retryItem(String id) async { + if (_isClosing || !mounted) { + return; + } + final index = _indexOf(id); + if (index < 0) { + return; + } + final item = _items[index]; + + // Processing failures: re-run the full parse → commit pipeline. + if (item.status == BookImportBatchStatus.processingFailed) { + setState(() { + _phase = _ImportPhase.processing; + _items[index] = item.startProcessing(); + }); + unawaited(_startProcessing(id)); + return; + } + + // Commit failures: only re-run the commit step. + if (item.status == BookImportBatchStatus.commitFailed) { + setState(() { + _phase = _ImportPhase.processing; + _items[index] = item.startAdding(); + }); + unawaited(_commitItem(id)); + return; + } + } + + Future _commitItem(String id) async { + final index = _indexOf(id); + if (index < 0) return; + final item = _items[index]; + if (item.status != BookImportBatchStatus.adding || item.result == null) return; + + try { + final book = await widget.committer(item.result!, item.file.name); + if (!mounted) return; + final i = _indexOf(id); + if (i < 0 || _items[i].status != BookImportBatchStatus.adding) return; + + setState(() { + _items[i] = _items[i].added(); + _addedBooks.add(book); + }); + } catch (error) { + if (!mounted) return; + final i = _indexOf(id); + if (i < 0 || _items[i].status != BookImportBatchStatus.adding) return; + setState(() { + _items[i] = _items[i].commitFailed(_safeErrorMessage(error)); + }); + } + _maybeTransitionToSummary(); + } + + // -- Remove an item -------------------------------------------------------- + + Future _removeItem(String id) async { + if (_isClosing || (_phase == _ImportPhase.processing && _anyProcessing)) { + return; + } + final index = _indexOf(id); + if (index < 0) return; + final item = _items[index]; + + final result = item.result; + if (result != null && item.status != BookImportBatchStatus.added) { + final deleted = await _deleteTemporary(result.bookId); + if (!deleted && mounted) { + ScaffoldMessenger.maybeOf( + context, + )?.showSnackBar(const SnackBar(content: Text('Could not remove the imported file.'))); + return; + } + } + if (!mounted) return; + setState(() => _items.removeAt(index)); + } + + // -- Cleanup helpers ------------------------------------------------------- + + Future _deleteTemporary(String bookId) { + if (_cleanedBookIds.contains(bookId)) return Future.value(true); + final inFlight = _cleanupFutures[bookId]; + if (inFlight != null) return inFlight; + final cleanup = _performDelete(bookId); + _cleanupFutures[bookId] = cleanup; + unawaited( + cleanup.whenComplete(() { + if (identical(_cleanupFutures[bookId], cleanup)) _cleanupFutures.remove(bookId); + }), + ); + return cleanup; + } + + Future _performDelete(String bookId) async { + try { + await widget.deleteBookFile(bookId); + _cleanedBookIds.add(bookId); + return true; + } catch (_) { + debugPrint('Book import temporary-file cleanup failed.'); + return false; + } + } + + // -- Close flow ------------------------------------------------------------ + + Future _requestClose() { + final inFlight = _closeFuture; + if (inFlight != null) return inFlight; + final close = _close(); + _closeFuture = close; + unawaited( + close.whenComplete(() { + if (identical(_closeFuture, close)) _closeFuture = null; + }), + ); + return close; + } + + Future _close() async { + if (_isClosing) return; + setState(() => _isClosing = true); + // Wait for in-flight processing + final results = await Future.wait(List>.of(_processingFutures.values)); + if (results.any((ok) => !ok)) { + if (mounted) setState(() => _isClosing = false); + return; + } + // Clean up temporary files for items that haven't been added + final bookIds = _items.where((item) => item.hasTemporaryFile).map((item) => item.result!.bookId).toSet(); + final cleanupResults = await Future.wait(bookIds.map(_deleteTemporary)); + if (cleanupResults.any((deleted) => !deleted)) { + if (mounted) { + setState(() => _isClosing = false); + ScaffoldMessenger.maybeOf( + context, + )?.showSnackBar(const SnackBar(content: Text('Could not remove temporary files. Please try again.'))); + } + return; + } + if (!mounted) return; + widget.onClose(); + } + + // -- Helpers --------------------------------------------------------------- + + bool _isCurrentProcessing(String id, int token) => !_isClosing && _isMatchingProcessing(id, token); + + bool _isMatchingProcessing(String id, int token) { + if (!mounted || _processingTokens[id] != token) return false; + final index = _indexOf(id); + return index >= 0 && _items[index].status == BookImportBatchStatus.processing; + } + + String _safeErrorMessage(Object error) { + final message = error.toString().replaceFirst(RegExp(r'^(Exception|Error):\s*'), '').trim(); + return message.isEmpty ? 'Could not import this file.' : message; + } + + // -- Build ----------------------------------------------------------------- + + @override + Widget build(BuildContext context) { + return PopScope( + canPop: false, + onPopInvokedWithResult: (didPop, _) { + if (!didPop) unawaited(_requestClose()); + }, + child: switch (_phase) { + _ImportPhase.selecting => _buildSelectingPhase(), + _ImportPhase.processing => _buildProcessingPhase(), + _ImportPhase.summary => _buildSummaryPhase(), + }, + ); + } + + // ========================================================================== + // Phase 1: File Selection + // ========================================================================== + + Widget _buildSelectingPhase() { + final hasFiles = _files.isNotEmpty; + final hasReadable = _readableFiles.isNotEmpty; + return AddBookSheetScaffold( + title: 'Import books', + canClose: true, + onClose: widget.onClose, + body: hasFiles ? _buildFileList() : _buildBrowseOnly(), + footer: Row( + children: [ + if (hasFiles) FilledButton(onPressed: _isPicking ? null : _clearSelection, child: const Text('Reset')), + const Spacer(), + TextButton(onPressed: widget.onClose, child: const Text('Cancel')), + const SizedBox(width: Spacing.sm), + FilledButton( + onPressed: hasReadable ? _startImport : null, + child: Text('Import ${_readableFiles.length} ${_readableFiles.length == 1 ? 'book' : 'books'}'), + ), + ], + ), + ); + } + + /// Large, welcoming file-picker area when no files have been selected yet. + /// Supported formats appear only inside [_BrowseArea] — not duplicated here. + Widget _buildBrowseOnly() { + final colorScheme = Theme.of(context).colorScheme; + + return Padding( + padding: const EdgeInsets.symmetric(horizontal: Spacing.lg), + child: Column( + children: [ + const Spacer(flex: 2), + _BrowseArea(isPicking: _isPicking, onTap: _browse), + if (_pickerError case final message?) ...[ + const SizedBox(height: Spacing.sm), + Text(message, style: TextStyle(color: colorScheme.error, fontSize: 13)), + ], + const Spacer(flex: 3), + ], + ), + ); + } + + /// File list — the "Reset" action lives in the footer so it is always visible. + Widget _buildFileList() { + final colorScheme = Theme.of(context).colorScheme; + return ListView( + controller: widget.scrollController, + padding: const EdgeInsets.symmetric(horizontal: Spacing.lg, vertical: Spacing.md), + children: [ + if (_pickerError case final message?) ...[ + Text(message, style: TextStyle(color: colorScheme.error, fontSize: 13)), + const SizedBox(height: Spacing.sm), + ], + Text( + '${_files.length} ${_files.length == 1 ? 'file' : 'files'} selected', + style: Theme.of(context).textTheme.titleSmall, + ), + const SizedBox(height: Spacing.sm), + ..._files.map((file) => _FileSelectCard(file: file, onRemove: () => _removeFile(file))), + ], + ); + } + + // ========================================================================== + // Phase 2: Processing + // ========================================================================== + + Widget _buildProcessingPhase() { + return AddBookSheetScaffold( + title: 'Importing books', + canClose: !_isClosing, + onClose: () => unawaited(_requestClose()), + body: ListView.separated( + controller: widget.scrollController, + padding: const EdgeInsets.symmetric(horizontal: Spacing.lg, vertical: Spacing.md), + itemCount: _items.length, + separatorBuilder: (_, _) => const SizedBox(height: Spacing.xs), + itemBuilder: (context, index) { + final item = _items[index]; + return _ImportProgressCard( + key: ValueKey(item.id), + item: item, + onRetry: _isClosing ? null : () => unawaited(_retryItem(item.id)), + onRemove: _isClosing ? null : () => unawaited(_removeItem(item.id)), + ); + }, + ), + footer: OverflowBar( + alignment: MainAxisAlignment.end, + overflowAlignment: OverflowBarAlignment.end, + spacing: Spacing.sm, + overflowSpacing: Spacing.sm, + children: [ + TextButton( + onPressed: _isClosing || _anyProcessing ? null : () => unawaited(_requestClose()), + child: const Text('Close'), + ), + ], + ), + ); + } + + // ========================================================================== + // Phase 3: Summary + // ========================================================================== + + Widget _buildSummaryPhase() { + final hasFailures = _failureCount > 0; + + return AddBookSheetScaffold( + title: 'Import complete', + canClose: true, + onClose: widget.onClose, + body: ListView( + controller: widget.scrollController, + padding: const EdgeInsets.symmetric(horizontal: Spacing.lg, vertical: Spacing.md), + children: _items + .map( + (item) => _ImportResultCard( + key: ValueKey(item.id), + item: item, + onRetry: hasFailures && !_isClosing + ? () { + setState(() => _phase = _ImportPhase.processing); + unawaited(_retryItem(item.id)); + } + : null, + ), + ) + .toList(), + ), + footer: OverflowBar( + alignment: MainAxisAlignment.end, + overflowAlignment: OverflowBarAlignment.end, + spacing: Spacing.sm, + overflowSpacing: Spacing.sm, + children: [ + FilledButton(onPressed: widget.onClose, child: const Text('Done')), + if (hasFailures) + OutlinedButton( + onPressed: () { + setState(() => _phase = _ImportPhase.processing); + for (final item in List.of(_items)) { + if (item.status == BookImportBatchStatus.processingFailed || + item.status == BookImportBatchStatus.commitFailed) { + unawaited(_retryItem(item.id)); + } + } + }, + child: Text('Retry $_failureCount failed'), + ), + ], + ), + ); + } +} + +// ============================================================================ +// Browse area widget +// ============================================================================ + +/// A large, inviting tappable area that triggers the platform file picker. +/// Supported formats are listed from the [BookImportSheet] extension constants +/// so they stay in one place. +class _BrowseArea extends StatelessWidget { + const _BrowseArea({required this.isPicking, required this.onTap}); + + final bool isPicking; + final VoidCallback onTap; + + static const _allExtensions = BookImportSheet._nativeExtensions; + + @override + Widget build(BuildContext context) { + final colorScheme = Theme.of(context).colorScheme; + final textTheme = Theme.of(context).textTheme; + final formats = _allExtensions.map((e) => e.toUpperCase()).join(', '); + + return Material( + color: colorScheme.primaryContainer.withValues(alpha: 0.3), + borderRadius: BorderRadius.circular(AppRadius.lg), + child: InkWell( + onTap: isPicking ? null : onTap, + borderRadius: BorderRadius.circular(AppRadius.lg), + child: Container( + constraints: const BoxConstraints(minHeight: 200), + padding: const EdgeInsets.symmetric(vertical: Spacing.xxl, horizontal: Spacing.lg), + decoration: BoxDecoration( + border: Border.all( + color: colorScheme.primary.withValues(alpha: 0.4), + strokeAlign: BorderSide.strokeAlignInside, + ), + borderRadius: BorderRadius.circular(AppRadius.lg), + ), + child: Column( + mainAxisAlignment: MainAxisAlignment.center, + mainAxisSize: MainAxisSize.min, + children: [ + if (isPicking) + const SizedBox.square(dimension: 48, child: CircularProgressIndicator(strokeWidth: 3)) + else + Icon(Icons.cloud_upload_outlined, size: 48, color: colorScheme.primary), + const SizedBox(height: Spacing.md), + Text('Browse files', style: textTheme.titleLarge?.copyWith(color: colorScheme.primary)), + const SizedBox(height: Spacing.sm), + Text( + 'Tap to select $formats', + style: textTheme.bodyMedium?.copyWith(color: colorScheme.onSurfaceVariant), + textAlign: TextAlign.center, + ), + ], + ), + ), + ), + ); + } +} + +// ============================================================================ +// File selection card (Phase 1) +// ============================================================================ + +class _FileSelectCard extends StatelessWidget { + const _FileSelectCard({required this.file, required this.onRemove}); + + final SelectedBookFile file; + final VoidCallback onRemove; + + @override + Widget build(BuildContext context) { + final colorScheme = Theme.of(context).colorScheme; + final textTheme = Theme.of(context).textTheme; + final readable = file.bytes != null; + final sizeText = file.bytes != null ? _formatSize(file.bytes!.length) : 'Unknown size'; + + return Container( + margin: const EdgeInsets.only(bottom: Spacing.sm), + decoration: BoxDecoration( + color: colorScheme.surfaceContainerHighest.withValues(alpha: 0.5), + borderRadius: BorderRadius.circular(AppRadius.md), + border: Border.all(color: colorScheme.outlineVariant), + ), + padding: const EdgeInsets.symmetric(horizontal: Spacing.md, vertical: Spacing.sm), + child: Row( + children: [ + Container( + width: 40, + height: 40, + decoration: BoxDecoration( + color: readable ? colorScheme.primaryContainer : colorScheme.errorContainer, + borderRadius: BorderRadius.circular(AppRadius.sm), + ), + child: Icon( + readable ? _iconForExtension(file.name) : Icons.error_outline, + size: 20, + color: readable ? colorScheme.onPrimaryContainer : colorScheme.onErrorContainer, + ), + ), + const SizedBox(width: Spacing.md), + Expanded( + child: Column( + crossAxisAlignment: CrossAxisAlignment.start, + mainAxisSize: MainAxisSize.min, + children: [ + Text(file.name, style: textTheme.bodyMedium, maxLines: 1, overflow: TextOverflow.ellipsis), + Text( + readable ? sizeText : 'Unreadable', + style: textTheme.bodySmall?.copyWith( + color: readable ? colorScheme.onSurfaceVariant : colorScheme.error, + ), + ), + ], + ), + ), + IconButton( + tooltip: 'Remove ${file.name}', + onPressed: onRemove, + icon: const Icon(Icons.close, size: 20), + visualDensity: VisualDensity.compact, + ), + ], + ), + ); + } +} + +// ============================================================================ +// Import progress card (Phase 2) +// ============================================================================ + +class _ImportProgressCard extends StatelessWidget { + const _ImportProgressCard({super.key, required this.item, required this.onRetry, required this.onRemove}); + + final BookImportBatchItem item; + final VoidCallback? onRetry; + final VoidCallback? onRemove; + + @override + Widget build(BuildContext context) { + final colorScheme = Theme.of(context).colorScheme; + final textTheme = Theme.of(context).textTheme; + + final failed = + item.status == BookImportBatchStatus.processingFailed || item.status == BookImportBatchStatus.commitFailed; + final added = item.status == BookImportBatchStatus.added; + + final displayTitle = _displayTitle; + final displaySubtitle = _displaySubtitle; + + return AnimatedContainer( + key: ValueKey('${item.id}-${item.status}'), + duration: const Duration(milliseconds: 250), + margin: const EdgeInsets.only(bottom: Spacing.xs), + decoration: BoxDecoration( + color: added + ? colorScheme.primaryContainer.withValues(alpha: 0.15) + : failed + ? colorScheme.errorContainer.withValues(alpha: 0.15) + : colorScheme.surfaceContainerHighest.withValues(alpha: 0.3), + borderRadius: BorderRadius.circular(AppRadius.md), + border: Border.all( + color: added + ? colorScheme.primary.withValues(alpha: 0.3) + : failed + ? colorScheme.error.withValues(alpha: 0.3) + : colorScheme.outlineVariant, + ), + ), + padding: const EdgeInsets.symmetric(horizontal: Spacing.md, vertical: Spacing.sm), + child: Row( + children: [ + // Leading icon + AnimatedSwitcher(duration: const Duration(milliseconds: 200), child: _statusWidget(colorScheme)), + const SizedBox(width: Spacing.md), + // Title + subtitle + Expanded( + child: Column( + crossAxisAlignment: CrossAxisAlignment.start, + mainAxisSize: MainAxisSize.min, + children: [ + Text(displayTitle, style: textTheme.bodyMedium, maxLines: 1, overflow: TextOverflow.ellipsis), + const SizedBox(height: 2), + Text( + displaySubtitle, + style: textTheme.bodySmall?.copyWith( + color: failed ? colorScheme.error : colorScheme.onSurfaceVariant, + ), + maxLines: 2, + overflow: TextOverflow.ellipsis, + ), + ], + ), + ), + // Trailing actions + if (failed && onRetry != null) + TextButton.icon( + onPressed: onRetry, + icon: const Icon(Icons.refresh, size: 18), + label: const Text('Retry'), + style: TextButton.styleFrom(visualDensity: VisualDensity.compact), + ) + else if (onRemove != null && (failed || item.status == BookImportBatchStatus.ready)) + IconButton( + tooltip: 'Remove ${item.file.name}', + onPressed: onRemove, + icon: const Icon(Icons.close, size: 20), + visualDensity: VisualDensity.compact, + ), + ], + ), + ); + } + + Widget _statusWidget(ColorScheme colorScheme) { + return switch (item.status) { + BookImportBatchStatus.queued => SizedBox( + key: const ValueKey('queued-icon'), + width: 24, + height: 24, + child: CircularProgressIndicator(strokeWidth: 2, color: colorScheme.onSurfaceVariant), + ), + BookImportBatchStatus.processing => SizedBox( + key: const ValueKey('processing-icon'), + width: 24, + height: 24, + child: CircularProgressIndicator(strokeWidth: 2), + ), + BookImportBatchStatus.adding => SizedBox( + key: const ValueKey('adding-icon'), + width: 24, + height: 24, + child: CircularProgressIndicator(strokeWidth: 2), + ), + BookImportBatchStatus.ready => Icon( + Icons.check_circle_outline, + key: const ValueKey('ready-icon'), + color: colorScheme.primary, + size: 24, + ), + BookImportBatchStatus.added => Icon( + Icons.check_circle, + key: const ValueKey('added-icon'), + color: colorScheme.primary, + size: 24, + ), + BookImportBatchStatus.processingFailed => Icon( + Icons.error_outline, + key: const ValueKey('parse-failed-icon'), + color: colorScheme.error, + size: 24, + ), + BookImportBatchStatus.commitFailed => Icon( + Icons.error_outline, + key: const ValueKey('commit-failed-icon'), + color: colorScheme.error, + size: 24, + ), + }; + } + + String get _displayTitle { + final result = item.result; + if (result != null) { + return result.title.isNotEmpty ? result.title : item.file.name; + } + return item.file.name; + } + + String get _displaySubtitle { + return switch (item.status) { + BookImportBatchStatus.queued => 'Preparing…', + BookImportBatchStatus.processing => 'Extracting metadata…', + BookImportBatchStatus.adding => 'Saving to library…', + BookImportBatchStatus.ready => 'Ready', + BookImportBatchStatus.added => item.result?.author.isNotEmpty == true ? 'by ${item.result!.author}' : 'Added', + BookImportBatchStatus.processingFailed => item.errorMessage ?? 'Could not read this file.', + BookImportBatchStatus.commitFailed => item.errorMessage ?? 'Could not save to library.', + }; + } +} + +// ============================================================================ +// Summary banner (Phase 3) +// ============================================================================ +// Import result card (Phase 3: summary individual rows) +// ============================================================================ + +class _ImportResultCard extends StatelessWidget { + const _ImportResultCard({super.key, required this.item, this.onRetry}); + + final BookImportBatchItem item; + final VoidCallback? onRetry; + + @override + Widget build(BuildContext context) { + final colorScheme = Theme.of(context).colorScheme; + final textTheme = Theme.of(context).textTheme; + + final added = item.status == BookImportBatchStatus.added; + final failed = + item.status == BookImportBatchStatus.processingFailed || item.status == BookImportBatchStatus.commitFailed; + final result = item.result; + final hasCover = result?.coverImage != null; + + return Container( + margin: const EdgeInsets.only(bottom: Spacing.sm), + padding: const EdgeInsets.all(Spacing.sm), + decoration: BoxDecoration( + color: colorScheme.surfaceContainerHighest.withValues(alpha: 0.3), + borderRadius: BorderRadius.circular(AppRadius.md), + border: Border.all( + color: added + ? colorScheme.primary.withValues(alpha: 0.2) + : failed + ? colorScheme.error.withValues(alpha: 0.2) + : colorScheme.outlineVariant, + ), + ), + child: Row( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + // Cover or icon + ClipRRect( + borderRadius: BorderRadius.circular(AppRadius.sm), + child: hasCover + ? Image.memory( + result!.coverImage!, + width: 40, + height: 56, + fit: BoxFit.cover, + errorBuilder: (_, _, _) => _fallbackIcon(colorScheme, added, failed), + ) + : _fallbackIcon(colorScheme, added, failed), + ), + const SizedBox(width: Spacing.md), + // Text content + Expanded( + child: Column( + crossAxisAlignment: CrossAxisAlignment.start, + mainAxisSize: MainAxisSize.min, + children: [ + Text( + result?.title.isNotEmpty == true ? result!.title : item.file.name, + style: textTheme.bodyMedium?.copyWith(fontWeight: FontWeight.w500), + maxLines: 1, + overflow: TextOverflow.ellipsis, + ), + if (result?.author.isNotEmpty == true) + Text( + result!.author, + style: textTheme.bodySmall?.copyWith(color: colorScheme.onSurfaceVariant), + maxLines: 1, + overflow: TextOverflow.ellipsis, + ), + const SizedBox(height: 4), + Row( + children: [ + Icon( + added ? Icons.check_circle : Icons.error_outline, + size: 16, + color: added ? colorScheme.primary : colorScheme.error, + ), + const SizedBox(width: 4), + Flexible( + child: Text( + added ? 'Imported' : (item.errorMessage ?? 'Failed'), + style: textTheme.labelSmall?.copyWith(color: added ? colorScheme.primary : colorScheme.error), + maxLines: 1, + overflow: TextOverflow.ellipsis, + ), + ), + ], + ), + ], + ), + ), + // Actions + if (failed && onRetry != null) + TextButton( + onPressed: onRetry, + style: TextButton.styleFrom(visualDensity: VisualDensity.compact), + child: const Text('Retry'), + ), + ], + ), + ); + } + + Widget _fallbackIcon(ColorScheme colorScheme, bool added, bool failed) { + return Container( + width: 40, + height: 56, + decoration: BoxDecoration( + color: added + ? colorScheme.primaryContainer.withValues(alpha: 0.3) + : failed + ? colorScheme.errorContainer.withValues(alpha: 0.3) + : colorScheme.surfaceContainerHighest, + borderRadius: BorderRadius.circular(AppRadius.sm), + ), + child: Icon( + added ? Icons.menu_book : Icons.insert_drive_file, + size: 20, + color: added ? colorScheme.primary : colorScheme.onSurfaceVariant, + ), + ); + } +}