From b36e22df557fa10b809c7b8f0086d648a91aeb38 Mon Sep 17 00:00:00 2001 From: rcmerci Date: Sat, 10 Oct 2026 15:34:14 +0800 Subject: [PATCH 1/2] Use shared Capture composer to append Detail children --- app/application.ml | 304 ++++++++++++++++++--------- app/journal_capture.ml | 12 +- app/journal_capture.mli | 3 + app/journal_detail.ml | 37 +++- app/journal_detail.mli | 19 ++ app/journal_header.ml | 77 ++++--- app/journal_header.mli | 3 +- app/journal_routes.ml | 51 +++++ app/journal_routes.mli | 9 + swift/JournalChrome.swift | 7 +- test/application_view_test.ml | 366 ++++++++++++++++++++++++++++++++- test/journal_adaptive_test.ml | 4 +- test/journal_semantics_test.ml | 64 +++++- 13 files changed, 799 insertions(+), 157 deletions(-) diff --git a/app/application.ml b/app/application.ml index 2733b48..a39f5b6 100644 --- a/app/application.ml +++ b/app/application.ml @@ -241,6 +241,10 @@ type capture_import_batch = ; batch_items : Journal_asset_import.staged list } +type composer_owner = + | Timeline_composer + | Detail_composer of string * int64 + type state = { favorites : Journal_routes.Favorites.t ; favorites_media_roots : string Rrbvec.t @@ -265,6 +269,7 @@ type state = ; asset_import_owner : (string * int64) option ; capture_pick_request : int ; capture_picker_armed : bool + ; capture_pick_owner : composer_owner option ; capture_pick_source : Journal_asset_import.source ; capture_imports : capture_import_batch option ; capture_error : capture_failure option @@ -352,6 +357,7 @@ let initial_state = ; asset_import_owner = None ; capture_pick_request = 0 ; capture_picker_armed = false + ; capture_pick_owner = None ; capture_pick_source = Journal_asset_import.Files ; capture_imports = None ; capture_error = None @@ -379,6 +385,71 @@ let initial_state = } ;; +let current_composer_owner state = + match state.modal with + | Append_sheet -> + Option.map + (fun entry -> + Detail_composer (entry, Journal_routes.detail_request_generation state.routes)) + (Journal_routes.active_entry_id state.routes) + | Capture_sheet -> Some Timeline_composer + | _ -> None +;; + +let composer_routes state = function + | Timeline_composer -> None + | Detail_composer (entry_id, generation) -> + Option.bind (Journal_routes.at_entry state.routes ~entry_id) (fun routes -> + if Journal_routes.detail_request_generation routes = generation + then Some routes + else None) +;; + +let composer_capture state = function + | Timeline_composer -> state.direct_capture + | Detail_composer _ as owner -> + Option.bind (composer_routes state owner) (fun routes -> + Option.bind (Journal_routes.detail routes) Journal_detail.child_capture) +;; + +let composer_saving state = function + | Timeline_composer -> + Option.fold + ~none:false + ~some:(fun capture -> Journal_capture.phase capture = Saving) + state.direct_capture + | Detail_composer _ as owner -> + Option.fold + ~none:false + ~some:(fun detail -> Journal_detail.mode detail = Saving_child) + (Option.bind (composer_routes state owner) Journal_routes.detail) +;; + +let map_composer_capture state owner ~f = + match owner with + | Timeline_composer -> { state with direct_capture = Option.map f state.direct_capture } + | Detail_composer (entry_id, _) -> + (match Option.bind (composer_routes state owner) Journal_routes.detail with + | None -> state + | Some detail -> + let next = Journal_detail.map_child_capture detail ~f in + if next == detail + then state + else + { state with + routes = Journal_routes.update_detail_at state.routes ~entry_id next + }) +;; + +let picker_owner state = Option.value state.capture_pick_owner ~default:Timeline_composer + +let picker_owner_is_current state = function + | Timeline_composer -> true + | Detail_composer (entry_id, generation) -> + Journal_routes.active_entry_id state.routes = Some entry_id + && Journal_routes.detail_request_generation state.routes = generation +;; + let feed_projection_context (calendar : Journal_calendar.t) = { local_day = Journal_calendar.local_day calendar ; projection_fingerprint = Journal_calendar.projection_fingerprint calendar @@ -426,6 +497,11 @@ let track_capture_session state = ;; let clear_graph_surface ?(discard_capture = true) state = + if discard_capture + then + List.iter + Journal_asset_import.discard_staged_file + (Journal_routes.pending_attachments state.routes); (match state.direct_capture with | Some capture when discard_capture -> List.iter @@ -434,6 +510,7 @@ let clear_graph_surface ?(discard_capture = true) state = | None | Some _ -> ()); { state with capture_picker_armed = false + ; capture_pick_owner = None ; capture_pick_request = (state.capture_pick_request + @@ -484,7 +561,10 @@ let clear_account_drafts state = List.iter Journal_asset_import.discard_staged_file (Journal_capture.pending_attachments capture)) - draft.capture_draft) + draft.capture_draft; + List.iter + Journal_asset_import.discard_staged_file + (Journal_routes.retained_attachments draft.append_drafts)) state.graph_drafts; { (clear_graph_surface state) with draft_graph = None @@ -1130,6 +1210,7 @@ let apply_worker_response_unstaged state (response : Journal_graph_runtime.respo ~message:(failure_source_message failure) } | Child_created { child; parent; timeline_entry_update } -> + let imports = Journal_routes.child_attachment_imports state.routes ~child ~parent in let was_editing = Option.bind (Journal_routes.detail state.routes) Journal_detail.child_capture <> None @@ -1141,6 +1222,15 @@ let apply_worker_response_unstaged state (response : Journal_graph_runtime.respo in { state with routes + ; capture_imports = + (match imports with + | None -> state.capture_imports + | Some (batch_target, batch_items) -> + Some + { batch_generation = state.graph_state.generation + ; batch_target + ; batch_items + }) ; modal = (if state.modal = Append_sheet && completed_active then No_modal else state.modal) ; timeline = @@ -1336,6 +1426,7 @@ module Root_navigation = struct | Capture_picker_requested source -> { state with capture_picker_armed = true + ; capture_pick_owner = current_composer_owner state ; capture_pick_source = source ; capture_pick_request = state.capture_pick_request + 1 } @@ -1347,11 +1438,12 @@ module Root_navigation = struct | Capture_assets_picked (items, request_id, error) -> items, request_id, error | _ -> assert false in + let owner = picker_owner state in let owned = Option.fold ~none:[] ~some:Journal_capture.pending_attachments - state.direct_capture + (composer_capture state owner) in let discard_from owned item = if @@ -1364,9 +1456,11 @@ module Root_navigation = struct owned) then Journal_asset_import.discard_staged_file item in - (match state.direct_capture with + (match composer_capture state owner with | Some capture when request_id = Some state.capture_pick_request + && picker_owner_is_current state owner + && (not (composer_saving state owner)) && (state.capture_picker_armed || match event with @@ -1391,9 +1485,9 @@ module Root_navigation = struct Some "Some selections could not be added to this draft." | None -> None in + let state = map_composer_capture state owner ~f:(fun _ -> capture) in { state with - direct_capture = Some capture - ; capture_picker_armed = + capture_picker_armed = (match event with | Capture_assets_picked _ -> false | _ -> state.capture_picker_armed) @@ -1472,7 +1566,6 @@ type command = | Select_journals | Select_favorites | Favorites_retry - | Open_asset_import | Refresh_catalog | Begin_online_recovery | Open_capture @@ -2889,22 +2982,10 @@ let detail_page |> V.Body.static in let actions_enabled = enabled && Option.is_some detail && not saving in - let action ~label ~icon command = - V.buttons_action - ~label - ~icon - ~on_press: - (Ui.Event.Handler.create (fun _ -> - if actions_enabled - then Ui.Event.Handler.Private.invoke (on_action command) Ui.Event.Payload.Unit)) - () - in content |> Journal_header.detail - ~actions: - [ action ~label:"Append" ~icon:"plus" Open_append - ; action ~label:"Attach file" ~icon:"paperclip" Open_asset_import - ] + ~capture_enabled:actions_enabled + ~on_capture:(on_action Open_append) |> Journal_asset_import.view ~key:(Ui.Key.string (scope ^ "import")) ~enabled:actions_enabled @@ -5311,25 +5392,6 @@ let start ~asset:event.asset | Next -> Journal_media_runtime.next media_runtime ~root:event.root))) ~f:(fun () -> flush_media set_state) - | Open_asset_import -> - update (fun state -> - if - state.write_enabled - && Option.is_none state.pending_delete - && Option.is_none state.pending_status - && Option.fold - ~none:false - ~some:(fun detail -> Journal_detail.mode detail <> Saving_child) - (Journal_routes.detail state.routes) - then - { state with - asset_import_request = state.asset_import_request + 1 - ; asset_import_owner = - Option.map - (fun id -> id, Journal_routes.detail_request_generation state.routes) - (Journal_routes.active_entry_id state.routes) - } - else state) | Import_asset value -> let import_payload = value in if Journal_asset_import.is_dismissal import_payload @@ -5400,12 +5462,16 @@ let start update (fun state -> Root_navigation.step state Capture_discarded) | Capture_attach value -> update (fun state -> - match state.direct_capture with - | Some capture + match current_composer_owner state with + | Some owner when state.write_enabled && Option.is_none state.pending_delete && Option.is_none state.pending_status - && Journal_capture.can_attach capture -> + && (not (composer_saving state owner)) + && Option.fold + ~none:false + ~some:Journal_capture.can_attach + (composer_capture state owner) -> Root_navigation.step state (Capture_picker_requested value) | _ -> state) | Capture_asset value -> @@ -5420,21 +5486,17 @@ let start update (fun state -> { state with capture_error = Some (Local_capture_failure message) }) | Journal_asset_import.Removed token -> - (match snapshot.direct_capture with - | Some capture -> - List.iter - (fun (staged : Journal_asset_import.staged) -> - if String.equal (Journal_asset_import.staged_token staged) token - then Journal_asset_import.discard_staged_file staged) - (Journal_capture.pending_attachments capture) - | None -> ()); update (fun state -> - { state with - direct_capture = - Option.map - (fun capture -> Journal_capture.remove_attachment capture ~token) - state.direct_capture - }) + match current_composer_owner state with + | Some owner when not (composer_saving state owner) -> + map_composer_capture state owner ~f:(fun capture -> + List.iter + (fun staged -> + if String.equal (Journal_asset_import.staged_token staged) token + then Journal_asset_import.discard_staged_file staged) + (Journal_capture.pending_attachments capture); + Journal_capture.remove_attachment capture ~token) + | _ -> state) | Journal_asset_import.Picked_batch (items, request_id, error) -> update (fun state -> Root_navigation.step state (Capture_assets_picked (items, request_id, error))) @@ -5972,11 +6034,19 @@ let start in let equal_capture_presentation left right = left.direct_capture == right.direct_capture + && current_composer_owner left = current_composer_owner right + && (let owner = current_composer_owner left in + match owner with + | None -> true + | Some owner -> + composer_capture left owner == composer_capture right owner + && composer_saving left owner = composer_saving right owner) && left.modal = right.modal && left.graph_state.generation = right.graph_state.generation && left.write_enabled = right.write_enabled && left.capture_error == right.capture_error && left.capture_picker_armed = right.capture_picker_armed + && left.capture_pick_owner = right.capture_pick_owner && left.capture_pick_request = right.capture_pick_request && left.capture_pick_source = right.capture_pick_source && left.import_completion = right.import_completion @@ -6043,8 +6113,9 @@ let start | No_modal, No_modal -> true | Capture_sheet, Capture_sheet -> equal_capture_presentation left right | Append_sheet, Append_sheet -> - Journal_routes.active_entry_id left.routes - = Journal_routes.active_entry_id right.routes + equal_capture_presentation left right + && Journal_routes.active_entry_id left.routes + = Journal_routes.active_entry_id right.routes && Option.equal (fun left right -> Journal_detail.child_capture left == Journal_detail.child_capture right @@ -6086,9 +6157,24 @@ let start && Option.is_none state.pending_status in let capture_assets state ~camera = + let owner = + Option.value (current_composer_owner state) ~default:(picker_owner state) + in + let dispatch = + match owner with + | Timeline_composer -> dispatch + | Detail_composer _ -> + let scope = Detail_outline.scope state.routes in + { dispatch with + send = (fun action -> dispatch.send (Detail_action (scope, action))) + } + in { request = Journal_asset_import.staged_request - ~id:(if state.capture_picker_armed then state.capture_pick_request else 0) + ~id: + (if state.capture_picker_armed && state.capture_pick_owner = Some owner + then state.capture_pick_request + else 0) ~source:state.capture_pick_source ~max_selections: (Journal_capture.attachment_limit @@ -6096,7 +6182,7 @@ let start ~none:0 ~some:(fun capture -> List.length (Journal_capture.pending_attachments capture)) - state.direct_capture) + (composer_capture state owner)) () ; camera ; completion = state.import_completion @@ -6111,6 +6197,43 @@ let start dispatch.send (Capture_asset (Journal_asset_import.Removed token))) } in + let append_composer state ~render = + Option.bind (Journal_routes.detail state.routes) (fun detail -> + Option.map + (fun capture -> + render + ~scope:"journal-append" + ~placeholder:"Append a block" + ~capture + ~saving:(Journal_detail.mode detail = Journal_detail.Saving_child) + ~enabled:state.write_enabled + ~on_edit:dispatch + ~on_toggle: + (Ui.Event.Handler.create (function + | Ui.Event.Payload.Bool selected + when selected + <> (Journal_capture.task_state capture = Journal_model.Todo) -> + dispatch.send + (Detail_action + ( Detail_outline.scope state.routes + , Detail_task_intent (Journal_capture.source capture) )) + | _ -> ())) + ~on_save: + (bind_action + dispatch + (Detail_action + ( Detail_outline.scope state.routes + , match Journal_detail.mode detail with + | Failed _ -> Detail_retry + | _ -> Detail_submit (Journal_capture.source capture) ))) + ~error: + (match Journal_detail.mode detail with + | Failed message -> Some message + | _ -> Option.map capture_failure_message state.capture_error) + ~assets: + (Some (capture_assets state ~camera:(state.environment.platform = "ios")))) + (Journal_detail.child_capture detail)) + in let floating_capture state = (* On iOS the capture composer expands directly into the bottom bar instead of presenting a sheet. It is mounted through the body @@ -6147,14 +6270,19 @@ let start ~assets:(Some (capture_assets state ~camera:true)) ]) state.direct_capture + | Append_sheet when state.environment.platform = "ios" -> + append_composer state ~render:composer_content | _ -> None in let floating_capture = region "capture" ~equal:equal_capture_presentation (fun current -> - if current.environment.platform = "ios" && current.modal = Capture_sheet + if + current.environment.platform = "ios" + && (current.modal = Capture_sheet || current.modal = Append_sheet) then V.stack [ V.tap_area ~on_press:(bind_action dispatch Close_composer) () + |> V.with_test_id (Ui.Test_id.string "journal-composer-dismiss") ; V.column ~spacing:0. [ V.spacer () @@ -6249,7 +6377,10 @@ let start in let capture_adapter = region "capture-import" ~equal:equal_capture_presentation (fun current -> - match current.direct_capture with + let owner = + Option.value (current_composer_owner current) ~default:(picker_owner current) + in + match composer_capture current owner with | None -> V.empty () | Some capture -> let assets = @@ -6257,7 +6388,10 @@ let start in Journal_asset_import.view ~key:(Ui.Key.string "journal-capture-asset-import") - ~enabled:(current.write_enabled && Journal_capture.can_attach capture) + ~enabled: + (current.write_enabled + && (not (composer_saving current owner)) + && Journal_capture.can_attach capture) ~completion:assets.completion ~request:assets.request ~pending:(Journal_capture.pending_attachments capture) @@ -6409,42 +6543,9 @@ let start (capture_assets state ~camera:(state.environment.platform = "ios")))) state.direct_capture | Append_sheet -> - Option.bind (Journal_routes.detail state.routes) (fun detail -> - Option.map - (fun capture -> - composer_page - ~scope:"journal-append" - ~placeholder:"Append a block" - ~saving:(Journal_detail.mode detail = Journal_detail.Saving_child) - ~capture - ~enabled:state.write_enabled - ~on_edit:dispatch - ~on_toggle: - (Ui.Event.Handler.create (function - | Ui.Event.Payload.Bool selected - when selected - <> (Journal_capture.task_state capture = Journal_model.Todo) - -> - dispatch.send - (Detail_action - ( Detail_outline.scope state.routes - , Detail_task_intent (Journal_capture.source capture) )) - | _ -> ())) - ~on_save: - (bind_action - dispatch - (Detail_action - ( Detail_outline.scope state.routes - , match Journal_detail.mode detail with - | Failed _ -> Detail_retry - | _ -> Detail_submit (Journal_capture.source capture) ))) - ~on_close:(bind_action dispatch Close_composer) - ~assets:None - ~error: - (match Journal_detail.mode detail with - | Failed message -> Some message - | _ -> Option.map capture_failure_message state.capture_error)) - (Journal_detail.child_capture detail)) + append_composer + state + ~render:(composer_page ~on_close:(bind_action dispatch Close_composer)) | Status_sheet block_id -> Option.map (fun block -> status_sheet_page ~tokens ~block dispatch) @@ -6478,7 +6579,9 @@ let start let modal = region "modal" ~equal:equal_modal_presentation (fun current -> let content = - if current.modal = Capture_sheet && current.environment.platform = "ios" + if + (current.modal = Capture_sheet || current.modal = Append_sheet) + && current.environment.platform = "ios" then None else modal current in @@ -6844,7 +6947,6 @@ module For_testing = struct let detail_page ~routes ~write_enabled on_action = let send = function | Detail_action (_, Open_append) -> on_action Append - | Detail_action (_, Open_asset_import) -> on_action Attach_file | Detail_action (_, Detail_open id) -> on_action (Open_block id) | Detail_action (_, Detail_delete id) -> on_action (Delete_block id) | Detail_action (_, Detail_expand id) -> on_action (Set_expanded (id, true)) diff --git a/app/journal_capture.ml b/app/journal_capture.ml index 8f34b91..1bc0041 100644 --- a/app/journal_capture.ml +++ b/app/journal_capture.ml @@ -152,6 +152,12 @@ let can_attach t = t.phase <> Saving && List.length t.pending_attachments < attachment_limit ;; +let submission_source t = + match source_is_blank (source t), t.pending_attachments with + | true, first :: _ -> Journal_asset_import.staged_title first + | _ -> source t +;; + let add_attachment t staged = let duplicate = List.exists @@ -227,11 +233,7 @@ let admit_save t ~mutation_id ~block_id ~sibling_order ~calendar_generation ~cre else ( (* A blank capture only containing attachments names its block after the first pick so the entry stays visible on the timeline. *) - let source = - match source_is_blank (source t), t.pending_attachments with - | true, first :: _ -> Journal_asset_import.staged_title first - | _ -> source t - in + let source = submission_source t in let command : Journal_graph_projection.capture = { mutation_id ; block_id diff --git a/app/journal_capture.mli b/app/journal_capture.mli index 69629c6..c8680de 100644 --- a/app/journal_capture.mli +++ b/app/journal_capture.mli @@ -46,6 +46,9 @@ val attachment_limit : int (** Assets picked for this draft, awaiting attach-on-save. *) val pending_attachments : t -> Journal_asset_import.staged list +(** Exact draft text, or the first attachment's friendly title for a blank draft. *) +val submission_source : t -> string + (** Whether another attachment may be added (not saving, under the limit). *) val can_attach : t -> bool diff --git a/app/journal_detail.ml b/app/journal_detail.ml index f4360bc..7ab5466 100644 --- a/app/journal_detail.ml +++ b/app/journal_detail.ml @@ -199,6 +199,41 @@ let session_id (t : t) = ID.Text_input.Session_id.of_int64 t.session_number let composer_revision (t : t) = t.composer_revision let reveal_id (t : t) = t.reveal_id let child_capture (t : t) = t.child_capture + +let map_child_capture (t : t) ~f = + match t.mode, t.child_capture with + | Saving_child, _ | _, None -> t + | (Reading | Failed _), Some capture -> + let next = f capture in + if next == capture + then t + else { t with child_capture = Some next; mode = Reading; pending = None } +;; + +let matching_attachment_imports pending capture ~root_id ~child ~parent = + if matching_child pending ~root_id ~child ~parent + then + Option.bind capture (fun capture -> + match Journal_capture.pending_attachments capture with + | [] -> None + | items -> Some (Journal_model.id child, items)) + else None +;; + +let child_attachment_imports (t : t) ~child ~parent = + matching_attachment_imports t.pending t.child_capture ~root_id:t.root_id ~child ~parent +;; + +let retained_attachment_imports draft ~child ~parent = + matching_attachment_imports + draft.draft_pending + (Some draft.draft_capture) + ~root_id:draft.draft_parent + ~child + ~parent +;; + +let retained_attachments draft = Journal_capture.pending_attachments draft.draft_capture let expanded (t : t) ~block_id = (branch t block_id).expanded let continuation (t : t) ~parent_id = (branch t parent_id).continuation let request_back _ = `Close @@ -419,7 +454,7 @@ let admit_child ; creation_time ; parent_block_id = t.root_id ; expected_parent_revision = Journal_model.revision (root t) - ; source = Journal_capture.source capture + ; source = Journal_capture.submission_source capture ; task_state = Journal_capture.task_state capture } in diff --git a/app/journal_detail.mli b/app/journal_detail.mli index 690a918..69396a8 100644 --- a/app/journal_detail.mli +++ b/app/journal_detail.mli @@ -50,6 +50,17 @@ val composer_revision : t -> int64 val reveal_id : t -> string option val request_back : t -> [ `Close ] val child_capture : t -> Journal_capture.t option + +(** Update the owned draft only while editable; an admitted child remains frozen. *) +val map_child_capture : t -> f:(Journal_capture.t -> Journal_capture.t) -> t + +(** Staged imports for this exact child completion, before retiring its draft. *) +val child_attachment_imports + : t + -> child:Journal_model.t + -> parent:Journal_model.t + -> (string * Journal_asset_import.staged list) option + val update_child_source : t -> string -> t val apply_child_edit : t -> Journal_view.Event.Payload.text_edit -> t val toggle_child_task : t -> t @@ -74,6 +85,14 @@ val undo_delete : t -> staged_delete -> t (** Only composer ownership is retained; reopening loads a fresh outline projection. *) type retained_composer +val retained_attachments : retained_composer -> Journal_asset_import.staged list + +val retained_attachment_imports + : retained_composer + -> child:Journal_model.t + -> parent:Journal_model.t + -> (string * Journal_asset_import.staged list) option + val retain_composer : t -> retained_composer option val restore_composer : t -> retained_composer -> t diff --git a/app/journal_header.ml b/app/journal_header.ml index bb62410..2118ae3 100644 --- a/app/journal_header.ml +++ b/app/journal_header.ml @@ -53,15 +53,48 @@ let date_header ~title = () ;; -let detail ~actions body = +let capture_control ~enabled ~on_capture = + V.buttons + ~actions: + [ V.buttons_action + ~label:"Capture" + ~icon:"square.and.pencil" + ~on_press: + (Ui.Event.Handler.create (fun payload -> + if enabled () then Ui.Event.Handler.Private.invoke on_capture payload)) + () + ] + () + |> V.semantics ~properties:(Ui.Semantics.create ~label:"Capture" ()) +;; + +let bottom_controls ~key ~controls body = + Ui.Native_widget.widget + chrome + ~key:(Ui.Key.string key) + ~props:(`Assoc [ "mode", `String "bottom-controls" ]) + ~on_event:(fun _ -> ()) + ~children:[ V.Body.Private.to_widget body; controls ] + () + |> V.Body.static +;; + +let detail ~capture_enabled ~on_capture body = + let capture = + capture_control ~enabled:(fun () -> capture_enabled) ~on_capture + |> test_id "journal-detail-capture-open" + in Ui.Native_widget.widget chrome ~key:(Ui.Key.string "journal-detail-header") ~props:(`Assoc [ "mode", `String "detail"; "title", `String "Block" ]) ~on_event:(fun _ -> ()) - ~children:[ V.Body.Private.to_widget body; V.buttons ~actions () ] + ~children:[ V.Body.Private.to_widget body ] () |> V.Body.static + |> bottom_controls + ~key:"journal-detail-bottom-controls" + ~controls:(V.row ~spacing:16. [ V.spacer (); capture ]) ;; type account_action = @@ -252,22 +285,12 @@ let view_impl in let capture = (* buttons has no disabled state — guard the handler instead. *) - V.buttons - ~actions: - [ V.buttons_action - ~label:"Capture" - ~icon:"square.and.pencil" - ~on_press: - (Ui.Event.Handler.create (fun payload -> - if - match presentation_signal with - | None -> initial.capture_enabled - | Some signal -> (Signal.sample signal).capture_enabled - then Ui.Event.Handler.Private.invoke on_capture payload)) - () - ] - () - |> V.semantics ~properties:(Ui.Semantics.create ~label:"Capture" ()) + capture_control + ~enabled:(fun () -> + match presentation_signal with + | None -> initial.capture_enabled + | Some signal -> (Signal.sample signal).capture_enabled) + ~on_capture |> test_id "journal-capture-open" in let destinations = @@ -373,18 +396,12 @@ let view_impl then (* The native inset reserves scrolling space; each buttons composite supplies its own glass without system toolbar chrome. *) - Ui.Native_widget.widget - chrome - ~key:(Ui.Key.string "journal-bottom-controls") - ~props:(`Assoc [ "mode", `String "bottom-controls" ]) - ~on_event:(fun _ -> ()) - ~children: - [ V.Body.Private.to_widget body - ; V.row ~spacing:16. [ destinations; V.spacer (); capture ] - |> test_id "journal-bottom-controls" - ] - () - |> V.Body.static + bottom_controls + ~key:"journal-bottom-controls" + ~controls: + (V.row ~spacing:16. [ destinations; V.spacer (); capture ] + |> test_id "journal-bottom-controls") + body else body ;; diff --git a/app/journal_header.mli b/app/journal_header.mli index e04298d..7a32d86 100644 --- a/app/journal_header.mli +++ b/app/journal_header.mli @@ -68,6 +68,7 @@ val feedback val date_header : title:string -> Journal_view.View.t val detail - : actions:Journal_view.View.buttons_action list + : capture_enabled:bool + -> on_capture:Journal_view.Event.Handler.t -> Journal_view.View.Body.t -> Journal_view.View.Body.t diff --git a/app/journal_routes.ml b/app/journal_routes.ml index fc05577..19ec1f4 100644 --- a/app/journal_routes.ml +++ b/app/journal_routes.ml @@ -168,6 +168,57 @@ type retained_drafts = ; next_rank : int64 } +let retained_attachments retained = + Drafts.fold + (fun _ draft items -> + List.rev_append (Journal_detail.retained_attachments draft.composer) items) + retained.composers + [] +;; + +let pending_attachments t = + let items = + Drafts.fold + (fun _ draft items -> + List.rev_append (Journal_detail.retained_attachments draft.composer) items) + t.drafts + [] + in + Owners.fold + (fun _ view items -> + match view with + | Detail_view { detail; _ } -> + Option.fold + ~none:items + ~some:(fun capture -> + List.rev_append (Journal_capture.pending_attachments capture) items) + (Journal_detail.child_capture detail) + | _ -> items) + t.owners + items +;; + +let child_attachment_imports t ~child ~parent = + let found = + Drafts.fold + (fun _ draft found -> + match found with + | Some _ -> found + | None -> + Journal_detail.retained_attachment_imports draft.composer ~child ~parent) + t.drafts + None + in + Owners.fold + (fun _ view found -> + match found, view with + | None, Detail_view { detail; _ } -> + Journal_detail.child_attachment_imports detail ~child ~parent + | _ -> found) + t.owners + found +;; + let retain_drafts ~interrupted t = let t = retain_live_composers t in { composers = diff --git a/app/journal_routes.mli b/app/journal_routes.mli index 1361915..7e1f5a0 100644 --- a/app/journal_routes.mli +++ b/app/journal_routes.mli @@ -74,6 +74,14 @@ val apply_detail_failure val detail : t -> Journal_detail.t option val update_detail : t -> Journal_detail.t -> t val apply_child_created : t -> child:Journal_model.t -> parent:Journal_model.t -> t +val pending_attachments : t -> Journal_asset_import.staged list + +val child_attachment_imports + : t + -> child:Journal_model.t + -> parent:Journal_model.t + -> (string * Journal_asset_import.staged list) option + val apply_child_failure : t -> block_id:string -> message:string -> t val back : t -> t val background : t -> t @@ -111,5 +119,6 @@ end (** Process-local composer storage; the application owns its graph identity. *) type retained_drafts +val retained_attachments : retained_drafts -> Journal_asset_import.staged list val retain_drafts : interrupted:bool -> t -> retained_drafts val restore_drafts : t -> retained_drafts -> t diff --git a/swift/JournalChrome.swift b/swift/JournalChrome.swift index ecfe711..e5f6cf1 100644 --- a/swift/JournalChrome.swift +++ b/swift/JournalChrome.swift @@ -166,7 +166,7 @@ struct JournalSheetDismissButton: SwiftUI.View { SectionDate(title: title) } case .detail: - if let title = properties?.title, context.childIDs.count == 2 { + if let title = properties?.title, context.childIDs.count == 1 { GeometryReader { bounds in child(0).frame(width: bounds.size.width, height: bounds.size.height) } @@ -174,11 +174,6 @@ struct JournalSheetDismissButton: SwiftUI.View { .toolbar(.visible, for: .navigationBar) #endif .navigationTitle(title) - .toolbar { - ToolbarItem(placement: .primaryAction) { - child(1).fixedSize() - } - } } case .toolbarControl: if context.childIDs.count == 1 { diff --git a/test/application_view_test.ml b/test/application_view_test.ml index a13446e..343850d 100644 --- a/test/application_view_test.ml +++ b/test/application_view_test.ml @@ -396,6 +396,7 @@ let run_favorites_native_visibility ?(check_error_control = false) ?(check_ios_capture = false) ?(check_capture_imports = false) + ?detail_capture ?(on_initialized = fun () -> ()) ?media_rows ?(shared_media = false) @@ -438,6 +439,13 @@ let run_favorites_native_visibility let import_probe = Atomic.make false in let import_probed = Atomic.make false in let staged_paths = ref [] in + let detail_captures = Atomic.make [] in + let child_attempts = Atomic.make 0 in + let child_release = + Atomic.make + (not (List.mem detail_capture [ Some "duplicate"; Some "offscreen-attachments" ])) + in + let import_targets = Atomic.make [] in let scope : Service.asset_scope = { account = { managed_sync_origin = Uri.of_string "https://example.invalid" @@ -576,7 +584,9 @@ let run_favorites_native_visibility ~push_topic_count:6 ~merge_push:Service.coalesce_push ~concurrency: - (if check_capture_imports then Concurrent { max_in_flight = 2 } else Serial) + (if check_capture_imports || Option.is_some detail_capture + then Concurrent { max_in_flight = 2 } + else Serial) ~init:(fun context _ -> worker_context := Some context; W.Session_context.emit @@ -656,7 +666,9 @@ let run_favorites_native_visibility Ok Service.Client_command_completed | Client_command _ | Asset_command _ -> Ok Service.Client_command_completed | Acquire_asset_file _ | Acquire_imported_file _ -> Ok (Service.Asset_file None) - | Import_asset { graph_generation; source } when check_capture_imports -> + | Import_asset { graph_generation; source } + when check_capture_imports || Option.is_some detail_capture -> + Atomic.set import_targets (Atomic.get import_targets @ [ source.target ]); Atomic.set imports_entered (Atomic.get imports_entered @ [ source.title ]); if source.title = "First attachment" then ( @@ -760,10 +772,33 @@ let run_favorites_native_visibility V2_changes_acknowledged { generation = "g"; through = "p2" } | V2_get_block { block; _ } -> V2_block_outcome - (match List.find_index (G.Uuid.equal block) roots with - | Some n -> - V2_present_block { value = timeline_block n; revision = "root" } - | None -> V2_missing_block { uuid = block; revision = "p" }) + (match + List.find_opt + (fun (parent, (root : P.v2_block_tree)) -> + let _ = parent in + G.Uuid.equal root.uuid block) + (Atomic.get detail_captures) + with + | Some (parent, root) -> + let value = timeline_block 0 in + V2_present_block + { value = + { value with + block = + { value.block with + uuid = root.uuid + ; title = root.title + ; parent + ; order = "999" + } + } + ; revision = "appended" + } + | None -> + (match List.find_index (G.Uuid.equal block) roots with + | Some n -> + V2_present_block { value = timeline_block n; revision = "root" } + | None -> V2_missing_block { uuid = block; revision = "p" })) | V2_get_block_summary { block; _ } when check_capture_imports -> let root = Atomic.get capture_root |> Option.get in let page = Atomic.get capture_page |> Option.get in @@ -865,6 +900,45 @@ let run_favorites_native_visibility (match known with | Some page -> V2_present_page { page; revision = "p" } | None -> V2_missing_page { uuid = requested; revision = "p" }) + | V2_get_children { parent; _ } when Option.is_some detail_capture -> + let existing = + if G.Uuid.equal parent (uuid 1) + then ( + let value = timeline_block 1 in + [ { P.value = { value with block = { value.block with parent } } + ; revision = "child" + } + ]) + else [] + in + V2_children_outcome + { parent + ; revision_scope = V2_children_revision parent + ; scope_revision = "children" + ; items = + existing + @ List.filter_map + (fun (owner, (root : P.v2_block_tree)) -> + if not (G.Uuid.equal owner parent) + then None + else ( + let value = timeline_block 0 in + Some + { P.value = + { value with + block = + { value.block with + uuid = root.uuid + ; title = root.title + ; parent + ; order = "999" + } + } + ; revision = "appended" + })) + (Atomic.get detail_captures) + ; next_cursor = None + } | V2_get_children { parent; _ } when Atomic.get detail_fixture && G.Uuid.equal parent (uuid 1) -> let value = timeline_block 1 in @@ -949,6 +1023,26 @@ let run_favorites_native_visibility else []) ; next_cursor = None } + | V2_insert_blocks { mutation_id; parent; roots = [ root ]; _ } + when Option.is_some detail_capture -> + Atomic.incr child_attempts; + while not (Atomic.get child_release) do + Eio.Time.Mono.sleep (W.Request_context.clock context) 0.001 + done; + if detail_capture = Some "failure" && Atomic.get child_attempts = 1 + then + V2_failed + { code = Logseq_db_worker.Error.code_string Invalid_request + ; message = "Fixture child write failed" } + else ( + Atomic.set detail_captures (Atomic.get detail_captures @ [ parent, root ]); + V2_mutation_committed + { mutation_id + ; status = V2_applied + ; generation = "g" + ; before_projection_revision = "p" + ; after_projection_revision = "p2" + }) | V2_insert_blocks { mutation_id; parent; roots = [ root ]; _ } when check_capture_imports -> let page = Atomic.get capture_page |> Option.get in @@ -1116,6 +1210,7 @@ let run_favorites_native_visibility in Fun.protect ~finally:(fun () -> + Atomic.set child_release true; Atomic.set import_release true; Atomic.set acquire_release true; ignore (hooks.dispose ()); @@ -1125,7 +1220,7 @@ let run_favorites_native_visibility (fun () -> hooks.init 2 2 startup |> consume; on_initialized (); - if check_ios_capture || check_capture_imports + if check_ios_capture || check_capture_imports || Option.is_some detail_capture then ( let snapshot = { Journal_environment.fallback with platform = "ios" } in let payload = @@ -1147,6 +1242,246 @@ let run_favorites_native_visibility wait "Journals mounted" (fun () -> Option.is_some (find "accessibility-label" "Favorites") && Option.is_some (find "text" "Timeline fixture 2")); + (* Detail/Routes already own text admission, retry, outline ordering and + retained drafts, covered by their pure public reducer tests. They + cannot reproduce native Capture placement, shared Composer wiring, + picker callbacks or asynchronous Worker import ownership. Exercise + only that missing boundary through the public mounted Application. *) + Option.iter + (fun scenario -> + let all key value = + Hashtbl.fold + (fun id values ids -> + if List.assoc_opt key values = Some (`String value) then id :: ids else ids) + props [] + in + let enter n = + let label = + Option.get (find "text" (Printf.sprintf "Timeline fixture %d" n)) + in + dispatch (Lui_protocol.Press (ancestor_property label "press-enabled")); + wait "Detail root loads" (fun () -> + Option.is_some + (find + "accessibility-identifier" + ("detail-block:" ^ G.Uuid.to_string (uuid (n + 1))))) + in + let open_capture () = + let scope = find "accessibility-identifier" "journal-detail-capture-open" in + Alcotest.(check bool) + "Detail has its own bottom Capture control" + true + (Option.is_some scope); + let rec inside id root = + id = root + || Option.fold + ~none:false + ~some:(fun parent -> inside parent root) + (Hashtbl.find_opt parents id) + in + let label = + all "accessibility-label" "Capture" + |> List.find (fun id -> inside id (Option.get scope)) + in + dispatch (Lui_protocol.Press label); + wait "Detail expands the shared Composer" (fun () -> + Option.is_some (find "accessibility-identifier" "journal-append-composer")) + in + let send () = Option.get (find "accessibility-label" "Send") in + let edit text = + dispatch + (Lui_protocol.TextChanged + (Option.get (find "style-class" "composer-input"), text)) + in + let collapse () = + dispatch + (Lui_protocol.Press + (Option.get + (find "accessibility-identifier" "journal-composer-dismiss"))); + wait "collapse removes Composer" (fun () -> + Option.is_none (find "style-class" "composer-input")) + in + let back () = + let nav = Option.get (find "_extension" "navigation-stack") in + let payload () = + let revision = + List.assoc "revision" (Hashtbl.find props nav) + |> Yojson.Safe.Util.to_int + in + Yojson.Safe.to_string + (`Assoc [ "revision", `Int revision; "length", `Int 0 ]) + in + hooks.extension_event nav "path-changed" (payload ()) |> consume; + hooks.extension_event nav "settled" (payload ()) |> consume + in + enter 0; + Alcotest.(check bool) + "old Append toolbar action is removed" + true + (Option.is_none (find "accessibility-label" "Append")); + Alcotest.(check bool) + "old Attach file toolbar action is removed" + true + (Option.is_none (find "accessibility-label" "Attach file")); + open_capture (); + dispatch (Lui_protocol.Press (send ())); + edit " \n "; + dispatch (Lui_protocol.Press (send ())); + settle_worker (); + Alcotest.(check int) + "empty and whitespace cannot insert a child" + 0 + (Atomic.get child_attempts); + edit + (if scenario = "attachment-only" then "" else "Detail child 中文\nsecond line"); + if + List.mem + scenario + [ "attachments"; "attachment-only"; "offscreen-attachments" ] + then ( + dispatch (Lui_protocol.Press (Option.get (find "accessibility-label" "文件"))); + let adapter, request = + Hashtbl.fold + (fun id values found -> + match + List.assoc_opt "_extension" values, List.assoc_opt "payload" values + with + | Some (`String "journal-asset-import"), Some (`String payload) -> + let open Yojson.Safe.Util in + let request = member "request" (Yojson.Safe.from_string payload) in + if + member "staged" request = `Bool true + && member "id" request |> to_int > 0 + then Some (id, request) + else found + | _ -> found) + props + None + |> Option.get + in + let item index title = + let path = Filename.temp_file "journal-import-" ".txt" in + staged_paths := !staged_paths @ [ path ]; + `Assoc + [ "operation", `String (G.Uuid.to_string (uuid (30000 + index))) + ; "asset", `String (G.Uuid.to_string (uuid (31000 + index))) + ; "localMutation", `String (G.Uuid.to_string (uuid (32000 + index))) + ; "metadataMutation", `String (G.Uuid.to_string (uuid (33000 + index))) + ; "path", `String path + ; "title", `String title + ; "type", `String "txt" + ; "sourceIdentity", `String title + ] + in + let batch = + `Assoc + [ "action", `String "picked-batch" + ; "request", request + ; ( "items" + , `List [ item 1 "First attachment"; item 2 "Second attachment" ] ) + ] + in + hooks.extension_event + adapter + "event" + (Yojson.Safe.to_string + (`Assoc + [ "id", `Int 1; "payload", `String (Yojson.Safe.to_string batch) ])) + |> consume; + wait "Detail stages both Composer attachments" (fun () -> + List.length + (all + "accessibility-identifier" + ("composer-attachment:" ^ G.Uuid.to_string (uuid 30002))) + = 1)); + collapse (); + open_capture (); + if scenario <> "attachment-only" + then + Alcotest.(check bool) + "cancel/reopen retains exact multiline draft" + true + (Option.is_some (find "text" "Detail child 中文\nsecond line")); + if scenario = "navigation" + then ( + let stale_send = send () in + back (); + enter 2; + dispatch (Lui_protocol.Press stale_send); + settle_worker (); + Alcotest.(check int) + "old root Send cannot submit after navigation" + 0 + (Atomic.get child_attempts); + open_capture (); + edit "New root draft"); + dispatch (Lui_protocol.Press (send ())); + wait "child command is admitted" (fun () -> Atomic.get child_attempts = 1); + if scenario = "duplicate" + then ( + dispatch (Lui_protocol.Press (send ())); + Alcotest.(check int) + "repeated Send while Saving cannot duplicate" + 1 + (Atomic.get child_attempts); + Atomic.set child_release true); + if scenario = "offscreen-attachments" + then ( + back (); + enter 2; + open_capture (); + edit "Keep new root draft"; + Atomic.set child_release true); + if scenario = "failure" + then ( + wait "write failure is visible in Composer" (fun () -> + Option.is_some (find "text" "Fixture child write failed")); + collapse (); + open_capture (); + dispatch (Lui_protocol.Press (send ()))); + wait "one committed child is retained" (fun () -> + List.length (Atomic.get detail_captures) = 1); + let parent, child = List.hd (Atomic.get detail_captures) in + Alcotest.(check string) + "Send targets current Detail root" + (G.Uuid.to_string (uuid (if scenario = "navigation" then 3 else 1))) + (G.Uuid.to_string parent); + Alcotest.(check string) + "child preserves source or attachment-only friendly title" + (if scenario = "navigation" + then "New root draft" + else if scenario = "attachment-only" + then "First attachment" + else "Detail child 中文\nsecond line") + child.title; + if !staged_paths <> [] + then ( + wait "first import reaches Worker" (fun () -> + Atomic.get imports_entered <> []); + Alcotest.(check (list string)) + "second import waits for first terminal result" + [ "First attachment" ] + (Atomic.get imports_entered); + Atomic.set import_release true; + wait "both imports finish" (fun () -> Atomic.get imports_finished = 2); + wait "all staging files are released" (fun () -> + List.for_all (fun p -> not (Sys.file_exists p)) !staged_paths); + Alcotest.(check (list string)) + "imports target the newly appended child" + [ G.Uuid.to_string child.uuid; G.Uuid.to_string child.uuid ] + (List.map G.Uuid.to_string (Atomic.get import_targets)); + if scenario = "offscreen-attachments" + then + Alcotest.(check bool) + "old completion preserves new root Composer" + true + (Option.is_some (find "text" "Keep new root draft"))); + if scenario <> "offscreen-attachments" + then + wait "completion closes Detail Composer" (fun () -> + Option.is_none (find "style-class" "composer-input"))) + detail_capture; + if Option.is_none detail_capture then ( (* Routes reducers cannot own LUI mount/subscription retention. The public mounted Application boundary reproduces top-only teardown. *) if check_navigation @@ -2565,7 +2900,7 @@ let run_favorites_native_visibility "accessibility-identifier" ("detail-block:" ^ G.Uuid.to_string (List.nth roots 1)))); dispatch - (Lui_protocol.Press (Option.get (find "accessibility-label" "Append"))); + (Lui_protocol.Press (Option.get (find "accessibility-label" "Capture"))); let editor = Option.get (find "style-class" "composer-input") in Alcotest.(check bool) "append sheet title is present" @@ -2640,7 +2975,7 @@ let run_favorites_native_visibility (List.length (List.filter (G.Uuid.equal (favorite_target_uuid 64)) - (Atomic.get queried)))))) + (Atomic.get queried))))))) ;; (* The root reducer has no Worker accepted-ID/terminal-event boundary. This @@ -3868,6 +4203,19 @@ let () = ; Alcotest.test_case "fixture early exit releases Worker" `Quick (fun () -> test_fixture_exception_cleanup Exit) ] ) + ; ( "Detail shared Composer" + , List.map + (fun scenario -> + Alcotest.test_case scenario `Quick (fun () -> + run_favorites_native_visibility ~detail_capture:scenario ())) + [ "text" + ; "navigation" + ; "duplicate" + ; "failure" + ; "attachments" + ; "attachment-only" + ; "offscreen-attachments" + ] ) ; ( "Capture attachment imports" , [ Alcotest.test_case "ordered partial failure and staging release" diff --git a/test/journal_adaptive_test.ml b/test/journal_adaptive_test.ml index fd1811e..449e981 100644 --- a/test/journal_adaptive_test.ml +++ b/test/journal_adaptive_test.ml @@ -558,9 +558,7 @@ let test_detail_capsules_mount_outside_toolbars () = (List.mem command !actions = allowed) "%s action guard changed" label) - [ "Append", Application.For_testing.Append, actions_enabled - ; "Attach file", Application.For_testing.Attach_file, actions_enabled - ])) + [ "Capture", Application.For_testing.Append, actions_enabled ])) [ loading, true, false; loaded, false, false; loaded, true, true ] ;; diff --git a/test/journal_semantics_test.ml b/test/journal_semantics_test.ml index 4682e72..7189673 100644 --- a/test/journal_semantics_test.ml +++ b/test/journal_semantics_test.ml @@ -2305,8 +2305,70 @@ let test_document_preview_preserves_friendly_title_path_and_lease () = [ "pdf"; "txt" ] ;; +let test_detail_uses_shared_capture_control () = + let projection : Journal_graph_projection.detail = + { root = block (); children = { blocks = []; continuation = None } } + in + let routes = + Journal_routes.create () + |> fun routes -> + Journal_routes.open_detail routes ~block_id ~request_generation:1L + |> fun routes -> + Journal_routes.apply_detail_response routes ~request_generation:1L projection + in + List.iter + (fun write_enabled -> + let actions = ref [] in + let body = + Application.For_testing.detail_page ~routes ~write_enabled (fun action -> + actions := action :: !actions) + in + with_mounted (V.Body.Private.to_widget body) (fun app ops -> + (* Pure routes cannot reproduce native positional-slot mounting. The + public renderer must retain its body without an empty toolbar slot. *) + let header = + List.find_map + (function + | Lui_protocol.SetExtensionProp (id, "payload", StringValue value) + when Yojson.Basic.Util.member "mode" (Yojson.Basic.from_string value) + = `String "detail" -> Some id + | _ -> None) + (ops ()) + |> Option.get + in + let slots = + List.filter_map + (function + | Lui_protocol.InsertChild (parent, child, _) when parent = header -> + Some child + | _ -> None) + (ops ()) + in + require + (List.length slots = 1) + "Detail native Chrome retains an empty toolbar slot"; + let captures = + mounted_nodes (ops ()) AccessibilityLabel (StringValue "Capture") + in + require (List.length captures = 1) "Detail must have one shared Capture control"; + List.iter + (fun label -> + require + (mounted_nodes (ops ()) AccessibilityLabel (StringValue label) = []) + "Detail retains removed toolbar action %s" + label) + [ "Append"; "Attach file" ]; + ignore (Lui_app.dispatch_event app (Press (List.hd captures))); + ignore (Lui_app.flush app); + require + (!actions = if write_enabled then [ Application.For_testing.Append ] else []) + "Detail Capture failed to dispatch through its current write gate")) + [ true; false ] +;; + let tests = - [ ( "timeline asset child preview title" + [ "Detail shared Capture control", test_detail_uses_shared_capture_control + ; ( "timeline asset child preview title" , test_timeline_document_title_follows_its_asset_child ) ; ( "native sheet navigation and cancellation" , test_sheet_has_native_navigation_and_keeps_cancel_delivery ) From 5e7761dbb7b5e811a738a79431a5ec50f569fc60 Mon Sep 17 00:00:00 2001 From: rcmerci Date: Sat, 10 Oct 2026 16:19:57 +0800 Subject: [PATCH 2/2] Update merged LUI Composer and fence attachment draft ownership --- app/application.ml | 299 +++++++++++++++++++++++----------- app/journal_detail.ml | 14 ++ app/journal_detail.mli | 7 + app/journal_routes.ml | 66 +++++++- app/journal_routes.mli | 20 +++ logseq_journal.opam | 2 +- logseq_journal.opam.locked | 2 +- test/application_view_test.ml | 82 ++++++++-- test/journal_routes_test.ml | 72 +++++++- test/source_boundary_test.ml | 2 +- 10 files changed, 455 insertions(+), 111 deletions(-) diff --git a/app/application.ml b/app/application.ml index a39f5b6..dac12e9 100644 --- a/app/application.ml +++ b/app/application.ml @@ -272,7 +272,9 @@ type state = ; capture_pick_owner : composer_owner option ; capture_pick_source : Journal_asset_import.source ; capture_imports : capture_import_batch option + ; failed_capture_imports : capture_import_batch list ; capture_error : capture_failure option + ; capture_error_owner : composer_owner option ; timeline_notice : timeline_notice option ; write_enabled : bool ; graph_ready : bool @@ -360,7 +362,9 @@ let initial_state = ; capture_pick_owner = None ; capture_pick_source = Journal_asset_import.Files ; capture_imports = None + ; failed_capture_imports = [] ; capture_error = None + ; capture_error_owner = None ; timeline_notice = None ; write_enabled = false ; graph_ready = false @@ -396,20 +400,10 @@ let current_composer_owner state = | _ -> None ;; -let composer_routes state = function - | Timeline_composer -> None - | Detail_composer (entry_id, generation) -> - Option.bind (Journal_routes.at_entry state.routes ~entry_id) (fun routes -> - if Journal_routes.detail_request_generation routes = generation - then Some routes - else None) -;; - let composer_capture state = function | Timeline_composer -> state.direct_capture - | Detail_composer _ as owner -> - Option.bind (composer_routes state owner) (fun routes -> - Option.bind (Journal_routes.detail routes) Journal_detail.child_capture) + | Detail_composer (entry_id, request_generation) -> + Journal_routes.child_capture_at state.routes ~entry_id ~request_generation ;; let composer_saving state = function @@ -418,38 +412,51 @@ let composer_saving state = function ~none:false ~some:(fun capture -> Journal_capture.phase capture = Saving) state.direct_capture - | Detail_composer _ as owner -> - Option.fold - ~none:false - ~some:(fun detail -> Journal_detail.mode detail = Saving_child) - (Option.bind (composer_routes state owner) Journal_routes.detail) + | Detail_composer (entry_id, request_generation) -> + Journal_routes.child_saving_at state.routes ~entry_id ~request_generation ;; let map_composer_capture state owner ~f = match owner with | Timeline_composer -> { state with direct_capture = Option.map f state.direct_capture } - | Detail_composer (entry_id, _) -> - (match Option.bind (composer_routes state owner) Journal_routes.detail with - | None -> state - | Some detail -> - let next = Journal_detail.map_child_capture detail ~f in - if next == detail - then state - else - { state with - routes = Journal_routes.update_detail_at state.routes ~entry_id next - }) + | Detail_composer (entry_id, request_generation) -> + let routes = + Journal_routes.map_child_capture_at state.routes ~entry_id ~request_generation ~f + in + if routes == state.routes then state else { state with routes } ;; -let picker_owner state = Option.value state.capture_pick_owner ~default:Timeline_composer +let composer_error state owner = + if Option.value state.capture_error_owner ~default:Timeline_composer = owner + then state.capture_error + else None +;; + +let set_composer_error state owner error = + match error with + | Some _ -> { state with capture_error = error; capture_error_owner = Some owner } + | None when state.capture_error = None && state.capture_error_owner = None -> state + | None when Option.value state.capture_error_owner ~default:Timeline_composer = owner -> + { state with capture_error = None; capture_error_owner = None } + | None -> state +;; -let picker_owner_is_current state = function - | Timeline_composer -> true - | Detail_composer (entry_id, generation) -> - Journal_routes.active_entry_id state.routes = Some entry_id - && Journal_routes.detail_request_generation state.routes = generation +let discard_import_batches batches = + List.iter + (fun batch -> List.iter Journal_asset_import.discard_staged_file batch.batch_items) + batches ;; +let retain_import_failure state batch = + if batch.batch_generation <> state.graph_state.generation + then ( + discard_import_batches [ batch ]; + state) + else { state with failed_capture_imports = state.failed_capture_imports @ [ batch ] } +;; + +let picker_owner state = Option.value state.capture_pick_owner ~default:Timeline_composer + let feed_projection_context (calendar : Journal_calendar.t) = { local_day = Journal_calendar.local_day calendar ; projection_fingerprint = Journal_calendar.projection_fingerprint calendar @@ -497,6 +504,7 @@ let track_capture_session state = ;; let clear_graph_surface ?(discard_capture = true) state = + discard_import_batches state.failed_capture_imports; if discard_capture then List.iter @@ -529,6 +537,8 @@ let clear_graph_surface ?(discard_capture = true) state = ; pending_delete = None ; pending_status = None ; capture_error = None + ; capture_error_owner = None + ; failed_capture_imports = [] ; timeline_notice = None ; write_enabled = false ; graph_ready = false @@ -1123,7 +1133,20 @@ let apply_worker_response_unstaged state (response : Journal_graph_runtime.respo { state with direct_capture = (if completed then None else state.direct_capture) ; modal = (if completed && state.modal = Capture_sheet then No_modal else state.modal) - ; capture_error = (if completed then None else state.capture_error) + ; capture_error = + (if + completed + && Option.value state.capture_error_owner ~default:Timeline_composer + = Timeline_composer + then None + else state.capture_error) + ; capture_error_owner = + (if + completed + && Option.value state.capture_error_owner ~default:Timeline_composer + = Timeline_composer + then None + else state.capture_error_owner) ; capture_imports = (match Option.bind completed_capture (fun capture -> @@ -1216,6 +1239,14 @@ let apply_worker_response_unstaged state (response : Journal_graph_runtime.respo <> None in let routes = Journal_routes.apply_child_created state.routes ~child ~parent in + let state = + match state.capture_error_owner with + | Some (Detail_composer _ as owner) + when composer_capture state owner <> None + && composer_capture { state with routes } owner = None -> + set_composer_error state owner None + | _ -> state + in let completed_active = was_editing && Option.bind (Journal_routes.detail routes) Journal_detail.child_capture = None @@ -1243,8 +1274,11 @@ let apply_worker_response_unstaged state (response : Journal_graph_runtime.respo | Subtree_deleted { block_id; parent; timeline_entry_update; _ } -> (match state.pending_delete with | Some pending when String.equal pending.block_id block_id -> + let routes, released = Journal_routes.commit_delete state.routes ~block_id in + List.iter Journal_asset_import.discard_staged_file released; { state with - timeline = + routes + ; timeline = Option.fold ~none: (Option.fold @@ -1303,6 +1337,7 @@ let apply_worker_response_unstaged state (response : Journal_graph_runtime.respo | Worker_failure _ -> Worker_capture_failure (latest_worker_error state) | Projection_failure message -> Local_capture_failure message) + ; capture_error_owner = Some Timeline_composer }) | Status_mutation -> (match state.pending_status with @@ -1380,6 +1415,7 @@ module Root_navigation = struct ; graph_ready = false ; feed_loaded = false ; capture_error = None + ; capture_error_owner = None ; timeline_notice = None } ;; @@ -1459,7 +1495,6 @@ module Root_navigation = struct (match composer_capture state owner with | Some capture when request_id = Some state.capture_pick_request - && picker_owner_is_current state owner && (not (composer_saving state owner)) && (state.capture_picker_armed || @@ -1486,13 +1521,17 @@ module Root_navigation = struct | None -> None in let state = map_composer_capture state owner ~f:(fun _ -> capture) in + let state = + set_composer_error + state + owner + (Option.map (fun message -> Local_capture_failure message) error) + in { state with capture_picker_armed = (match event with | Capture_assets_picked _ -> false | _ -> state.capture_picker_armed) - ; capture_error = - Option.map (fun message -> Local_capture_failure message) error } | None | Some _ -> List.iter (discard_from owned) items; @@ -1514,6 +1553,7 @@ module Root_navigation = struct in if next == capture then state else { state with direct_capture = Some next }) | Capture_edited source -> + let state = set_composer_error state Timeline_composer None in let capture, next_local_sequence = match state.direct_capture with | None -> @@ -1526,18 +1566,14 @@ module Root_navigation = struct (match state.direct_capture with | Some previous -> previous == capture | None -> false) - && state.capture_error = None + && composer_error state Timeline_composer = None then state - else - { state with - direct_capture = Some capture - ; next_local_sequence - ; capture_error = None - } + else { state with direct_capture = Some capture; next_local_sequence } | Capture_admitted capture -> + let state = set_composer_error state Timeline_composer None in (match state.direct_capture with - | Some previous when previous == capture && state.capture_error = None -> state - | _ -> { state with direct_capture = Some capture; capture_error = None }) + | Some previous when previous == capture -> state + | _ -> { state with direct_capture = Some capture }) | Completed response -> apply_worker_response state response | Graph_replaced { generation; graph_id } -> let state = replace_graph state generation graph_id in @@ -1579,6 +1615,7 @@ type command = | Close_diagnostics | Open_error_info | Dismiss_operation_error + | Retry_capture_imports | Close_error_info | Switch_graph | Sign_out @@ -1793,9 +1830,12 @@ let operation_feedback ~scope ~state dispatch body = ( summary , V.buttons ~actions: - [ action "Details" "exclamationmark.circle" Open_error_info - ; action "Dismiss" "xmark" Dismiss_operation_error - ] + ((if state.failed_capture_imports = [] + then [] + else [ action "Retry imports" "arrow.clockwise" Retry_capture_imports ]) + @ [ action "Details" "exclamationmark.circle" Open_error_info + ; action "Dismiss" "xmark" Dismiss_operation_error + ]) () |> V.with_test_id (Ui.Test_id.string (scope ^ "-operation-actions")) ) in @@ -4333,14 +4373,20 @@ let start | Worker.Response { request_id; outcome = Completed (Asset_imported result); _ } -> let pending = Hashtbl.find_opt import_worker_requests request_id in Hashtbl.remove import_worker_requests request_id; - (match pending with - | Some (_, operation, _) -> - (match Hashtbl.find_opt capture_staged_items operation with - | Some staged -> - Hashtbl.remove capture_staged_items operation; - Journal_asset_import.discard_staged_file staged - | None -> ()) - | None -> ()); + let capture_completion = + match pending with + | Some (_, operation, _) -> + (match Hashtbl.find_opt capture_staged_items operation with + | None -> Effect.ignore + | Some batch -> + Hashtbl.remove capture_staged_items operation; + (match result with + | Ok _ -> + discard_import_batches [ batch ]; + Effect.ignore + | Error _ -> set_state (fun state -> retain_import_failure state batch))) + | None -> Effect.ignore + in let completion = let current = match pending with @@ -4405,16 +4451,20 @@ let start })) ]) in - Effect.bind completion ~f:(fun () -> !drain_capture_imports ()) + Effect.bind + (Effect.many [ capture_completion; completion ]) + ~f:(fun () -> !drain_capture_imports ()) | Worker.Response { request_id; outcome = Failed _ | Cancelled | Shutdown; _ } when Hashtbl.mem import_worker_requests request_id -> let generation, operation, owner = Hashtbl.find import_worker_requests request_id in Hashtbl.remove import_worker_requests request_id; - (match Hashtbl.find_opt capture_staged_items operation with - | Some staged -> - Hashtbl.remove capture_staged_items operation; - Journal_asset_import.discard_staged_file staged - | None -> ()); + let capture_completion = + match Hashtbl.find_opt capture_staged_items operation with + | None -> Effect.ignore + | Some batch -> + Hashtbl.remove capture_staged_items operation; + set_state (fun state -> retain_import_failure state batch) + in let completion = set_state (fun state -> if @@ -4439,7 +4489,9 @@ let start Some (operation, Some "Import was interrupted. Select the file again.") }) in - Effect.bind completion ~f:(fun () -> !drain_capture_imports ()) + Effect.bind + (Effect.many [ capture_completion; completion ]) + ~f:(fun () -> !drain_capture_imports ()) | Worker.Response { outcome = Completed (Asset_file _); _ } -> Effect.ignore | Worker.Response { outcome = Completed Client_command_completed; _ } -> Effect.ignore | Worker.Response { outcome = Completed (Graph_state graph_state); _ } @@ -4875,17 +4927,31 @@ let start with | Accepted request_id -> Hashtbl.replace import_worker_requests request_id (generation, operation, None); - Hashtbl.replace capture_staged_items operation staged; + Hashtbl.replace + capture_staged_items + operation + { batch_generation = generation + ; batch_target = Logseq_db_types.Graph_types.Uuid.to_string target + ; batch_items = [ staged ] + }; None | Full | Not_ready | Stopping -> - Some "Import is temporarily unavailable. Select the file again.") + Some + "Import is temporarily unavailable. Retry imports when the graph is ready.") in match failure with | None -> Effect.ignore | Some message -> - Journal_asset_import.discard_staged_file staged; Effect.bind (set_state (fun state -> + let state = + retain_import_failure + state + { batch_generation = generation + ; batch_target = Logseq_db_types.Graph_types.Uuid.to_string target + ; batch_items = [ staged ] + } + in if state.graph_state.generation <> generation then state else @@ -5105,6 +5171,7 @@ let start { state with capture_error = Some (Local_capture_failure (Journal_calendar.error_message error)) + ; capture_error_owner = Some Timeline_composer }) | Ok calendar -> Journal_graph_runtime.set_calendar graph_runtime calendar; @@ -5129,6 +5196,7 @@ let start { state with direct_capture = Some capture ; capture_error = Some (Local_capture_failure message) + ; capture_error_owner = Some Timeline_composer }) | Ok (_, None) -> Effect.ignore | Ok (capture, Some request) -> @@ -5439,7 +5507,9 @@ let start (Journal_routes.active_entry_id snapshot.routes) ); None | Full | Not_ready | Stopping -> - Some "Import is temporarily unavailable. Select the file again."))) + Some + "Import is temporarily unavailable. Retry imports when the graph \ + is ready."))) ~f:(function | None -> Effect.ignore | Some message -> @@ -5484,7 +5554,10 @@ let start else state) | Journal_asset_import.Unavailable message -> update (fun state -> - { state with capture_error = Some (Local_capture_failure message) }) + set_composer_error + { state with capture_picker_armed = false } + (picker_owner state) + (Some (Local_capture_failure message))) | Journal_asset_import.Removed token -> update (fun state -> match current_composer_owner state with @@ -5553,11 +5626,31 @@ let start && Option.is_none (operation_failure state.timeline_notice) then state else { state with modal = Error_info }) + | Retry_capture_imports when snapshot.write_enabled -> + let batches = snapshot.failed_capture_imports in + Effect.bind + (update (fun state -> + { state with failed_capture_imports = []; timeline_notice = None })) + ~f:(fun () -> + List.iter + (fun batch -> + match Logseq_db_types.Graph_types.Uuid.of_string batch.batch_target with + | Error _ -> discard_import_batches [ batch ] + | Ok target -> + List.iter + (fun staged -> + Queue.add + (batch.batch_generation, target, staged) + capture_import_queue) + batch.batch_items) + batches; + !drain_capture_imports ()) | Dismiss_operation_error -> update (fun state -> match state.timeline_notice with | Some (Delete_failed _ | Status_failed _ | Copy_failed _ | Import_failed _) -> - { state with timeline_notice = None } + discard_import_batches state.failed_capture_imports; + { state with timeline_notice = None; failed_capture_imports = [] } | None | Some Delete_undo -> state) | Close_error_info -> update (fun state -> { state with modal = No_modal }) | Switch_graph -> @@ -5672,10 +5765,12 @@ let start (match Journal_calendar.Sampler.sample calendar_sampler with | Error error -> update (fun state -> - { state with - capture_error = - Some (Local_capture_failure (Journal_calendar.error_message error)) - }) + set_composer_error + state + (Detail_composer + ( Option.get (Journal_routes.active_entry_id state.routes) + , Journal_routes.detail_request_generation state.routes )) + (Some (Local_capture_failure (Journal_calendar.error_message error)))) | Ok calendar -> Journal_graph_runtime.set_calendar graph_runtime calendar; let creation_time = Journal_time.of_calendar calendar |> Result.get_ok in @@ -5702,7 +5797,12 @@ let start (match admission with | Error message -> update (fun state -> - { state with capture_error = Some (Local_capture_failure message) }) + set_composer_error + state + (Detail_composer + ( Option.get (Journal_routes.active_entry_id state.routes) + , Journal_routes.detail_request_generation state.routes )) + (Some (Local_capture_failure message))) | Ok (_, None) -> Effect.ignore | Ok (detail, Some request) -> with_direct_request @@ -5710,6 +5810,7 @@ let start calendar = Some calendar ; routes = Journal_routes.update_detail snapshot.routes detail ; capture_error = None + ; capture_error_owner = None ; next_local_sequence = Int64.succ number } request)) @@ -6045,6 +6146,7 @@ let start && left.graph_state.generation = right.graph_state.generation && left.write_enabled = right.write_enabled && left.capture_error == right.capture_error + && left.capture_error_owner = right.capture_error_owner && left.capture_picker_armed = right.capture_picker_armed && left.capture_pick_owner = right.capture_pick_owner && left.capture_pick_request = right.capture_pick_request @@ -6126,6 +6228,7 @@ let start = Journal_routes.detail_request_generation right.routes && left.write_enabled = right.write_enabled && left.capture_error == right.capture_error + && left.capture_error_owner = right.capture_error_owner | _ -> left == right in let equal_shell_presentation left right = @@ -6156,9 +6259,17 @@ let start && Option.is_none state.pending_delete && Option.is_none state.pending_status in - let capture_assets state ~camera = + let capture_assets ?owner state ~camera = let owner = - Option.value (current_composer_owner state) ~default:(picker_owner state) + Option.value + owner + ~default: + (Option.value (current_composer_owner state) ~default:(picker_owner state)) + in + let on_event payload = + match Journal_asset_import.decode_event payload with + | Ok event -> dispatch.send (Capture_asset event) + | Error _ -> () in let dispatch = match owner with @@ -6187,11 +6298,7 @@ let start ; camera ; completion = state.import_completion ; on_attach = (fun source -> dispatch.send (Capture_attach source)) - ; on_event = - (fun payload -> - match Journal_asset_import.decode_event payload with - | Ok event -> dispatch.send (Capture_asset event) - | Error _ -> ()) + ; on_event ; on_remove = (fun token -> dispatch.send (Capture_asset (Journal_asset_import.Removed token))) @@ -6229,7 +6336,10 @@ let start ~error: (match Journal_detail.mode detail with | Failed message -> Some message - | _ -> Option.map capture_failure_message state.capture_error) + | _ -> + Option.map + capture_failure_message + (Option.bind (current_composer_owner state) (composer_error state))) ~assets: (Some (capture_assets state ~camera:(state.environment.platform = "ios")))) (Journal_detail.child_capture detail)) @@ -6261,7 +6371,7 @@ let start | _ -> ())) ~on_save:(bind_action dispatch Capture_submit) ~error: - (match state.capture_error with + (match composer_error state Timeline_composer with | Some failure -> Some (capture_failure_message failure) | None -> (match Journal_capture.phase capture with @@ -6378,13 +6488,16 @@ let start let capture_adapter = region "capture-import" ~equal:equal_capture_presentation (fun current -> let owner = - Option.value (current_composer_owner current) ~default:(picker_owner current) + if current.capture_picker_armed + then picker_owner current + else + Option.value (current_composer_owner current) ~default:(picker_owner current) in match composer_capture current owner with | None -> V.empty () | Some capture -> let assets = - capture_assets current ~camera:(current.environment.platform = "ios") + capture_assets ~owner current ~camera:(current.environment.platform = "ios") in Journal_asset_import.view ~key:(Ui.Key.string "journal-capture-asset-import") @@ -6532,7 +6645,7 @@ let start ~on_save:(bind_action dispatch Capture_submit) ~on_close:(bind_action dispatch Close_composer) ~error: - (match state.capture_error with + (match composer_error state Timeline_composer with | Some failure -> Some (capture_failure_message failure) | None -> (match Journal_capture.phase capture with @@ -6614,7 +6727,9 @@ let start let feedback = region "feedback" - ~equal:(fun left right -> left.timeline_notice = right.timeline_notice) + ~equal:(fun left right -> + left.timeline_notice = right.timeline_notice + && left.failed_capture_imports == right.failed_capture_imports) (fun current -> if current.timeline_notice = None then V.empty () @@ -6722,9 +6837,7 @@ let start (fun (_, _, staged) -> Journal_asset_import.discard_staged_file staged) capture_import_queue; Queue.clear capture_import_queue; - Hashtbl.iter - (fun _ staged -> Journal_asset_import.discard_staged_file staged) - capture_staged_items; + Hashtbl.iter (fun _ batch -> discard_import_batches [ batch ]) capture_staged_items; Hashtbl.clear capture_staged_items in let context = diff --git a/app/journal_detail.ml b/app/journal_detail.ml index 7ab5466..4b0c54c 100644 --- a/app/journal_detail.ml +++ b/app/journal_detail.ml @@ -234,6 +234,20 @@ let retained_attachment_imports draft ~child ~parent = ;; let retained_attachments draft = Journal_capture.pending_attachments draft.draft_capture +let retained_capture draft = draft.draft_capture +let retained_saving draft = draft.draft_mode = Saving_child + +let map_retained_capture draft ~f = + if retained_saving draft + then draft + else ( + let capture = f draft.draft_capture in + if capture == draft.draft_capture + then draft + else + { draft with draft_capture = capture; draft_mode = Reading; draft_pending = None }) +;; + let expanded (t : t) ~block_id = (branch t block_id).expanded let continuation (t : t) ~parent_id = (branch t parent_id).continuation let request_back _ = `Close diff --git a/app/journal_detail.mli b/app/journal_detail.mli index 69396a8..e94b35a 100644 --- a/app/journal_detail.mli +++ b/app/journal_detail.mli @@ -86,6 +86,13 @@ val undo_delete : t -> staged_delete -> t type retained_composer val retained_attachments : retained_composer -> Journal_asset_import.staged list +val retained_capture : retained_composer -> Journal_capture.t +val retained_saving : retained_composer -> bool + +val map_retained_capture + : retained_composer + -> f:(Journal_capture.t -> Journal_capture.t) + -> retained_composer val retained_attachment_imports : retained_composer diff --git a/app/journal_routes.ml b/app/journal_routes.ml index 19ec1f4..0ccfec8 100644 --- a/app/journal_routes.ml +++ b/app/journal_routes.ml @@ -40,6 +40,7 @@ type view = type detached = { block_id : string + ; request_generation : int64 ; rank : int64 ; composer : Journal_detail.retained_composer } @@ -142,14 +143,17 @@ let track_detail_session t detail = let retain_owner t owner_id = match Owners.find_opt owner_id t.owners with - | Some (Detail_view { block_id; detail; _ }) -> + | Some (Detail_view { block_id; detail; request_generation }) -> let t = track_detail_session t detail in (match Journal_detail.retain_composer detail with | None -> { t with drafts = Drafts.remove owner_id t.drafts } | Some composer -> { t with drafts = - Drafts.add owner_id { block_id; composer; rank = t.next_retained } t.drafts + Drafts.add + owner_id + { block_id; request_generation; composer; rank = t.next_retained } + t.drafts ; next_retained = Int64.succ t.next_retained }) | _ -> t @@ -198,6 +202,64 @@ let pending_attachments t = items ;; +let child_capture_at t ~entry_id ~request_generation = + match Owners.find_opt entry_id t.owners with + | Some (Detail_view owner) when owner.request_generation = request_generation -> + Journal_detail.child_capture owner.detail + | _ -> + Option.bind (Drafts.find_opt entry_id t.drafts) (fun draft -> + if draft.request_generation = request_generation + then Some (Journal_detail.retained_capture draft.composer) + else None) +;; + +let child_saving_at t ~entry_id ~request_generation = + match Owners.find_opt entry_id t.owners with + | Some (Detail_view owner) when owner.request_generation = request_generation -> + Journal_detail.mode owner.detail = Saving_child + | _ -> + Option.fold + ~none:false + ~some:(fun draft -> + draft.request_generation = request_generation + && Journal_detail.retained_saving draft.composer) + (Drafts.find_opt entry_id t.drafts) +;; + +let map_child_capture_at t ~entry_id ~request_generation ~f = + match Owners.find_opt entry_id t.owners with + | Some (Detail_view owner) when owner.request_generation = request_generation -> + let next = Journal_detail.map_child_capture owner.detail ~f in + if next == owner.detail + then t + else + { t with + owners = Owners.add entry_id (Detail_view { owner with detail = next }) t.owners + } + | _ -> + (match Drafts.find_opt entry_id t.drafts with + | Some draft when draft.request_generation = request_generation -> + let composer = Journal_detail.map_retained_capture draft.composer ~f in + if composer == draft.composer + then t + else { t with drafts = Drafts.add entry_id { draft with composer } t.drafts } + | _ -> t) +;; + +let commit_delete t ~block_id = + let retired, kept = + Drafts.partition (fun _ draft -> draft.block_id = block_id) t.drafts + in + let files = + Drafts.fold + (fun _ draft files -> + List.rev_append (Journal_detail.retained_attachments draft.composer) files) + retired + [] + in + if Drafts.is_empty retired then t, [] else { t with drafts = kept }, files +;; + let child_attachment_imports t ~child ~parent = let found = Drafts.fold diff --git a/app/journal_routes.mli b/app/journal_routes.mli index 7e1f5a0..d785baf 100644 --- a/app/journal_routes.mli +++ b/app/journal_routes.mli @@ -76,6 +76,26 @@ val update_detail : t -> Journal_detail.t -> t val apply_child_created : t -> child:Journal_model.t -> parent:Journal_model.t -> t val pending_attachments : t -> Journal_asset_import.staged list +(** Picker completion ownership survives navigation; stale generations are ignored. *) +val child_capture_at + : t + -> entry_id:string + -> request_generation:int64 + -> Journal_capture.t option + +val child_saving_at : t -> entry_id:string -> request_generation:int64 -> bool + +val map_child_capture_at + : t + -> entry_id:string + -> request_generation:int64 + -> f:(Journal_capture.t -> Journal_capture.t) + -> t + +(** Retire committed root drafts and return their files to the application for cleanup. + Staging a delete and Undo do not retire them. *) +val commit_delete : t -> block_id:string -> t * Journal_asset_import.staged list + val child_attachment_imports : t -> child:Journal_model.t diff --git a/logseq_journal.opam b/logseq_journal.opam index ecc184a..bf3d305 100644 --- a/logseq_journal.opam +++ b/logseq_journal.opam @@ -46,7 +46,7 @@ depends: [ ] pin-depends: [ ["rrbvec.dev" "git+https://github.com/logseq/rrbvec.git#main"] - ["lui.0.1.0" "git+https://github.com/logseq/lui.git#91aecb52a1cba2faaf23aac1d64a0bd1cb6549e7"] + ["lui.0.1.0" "git+https://github.com/logseq/lui.git#8de2abae573499692dbcc169a2738c434944a0a8"] ["ocaml-signal.0.1.0" "git+https://github.com/logseq/ocaml-signal.git#main"] ["datascript_ocaml.dev" "git+https://github.com/logseq/datascript-ocaml.git#0561660e4894faee250d551ab2a32a6b5c25a5fb"] ["datascript-ocaml-native.dev" "git+https://github.com/logseq/datascript-ocaml.git#0561660e4894faee250d551ab2a32a6b5c25a5fb"] diff --git a/logseq_journal.opam.locked b/logseq_journal.opam.locked index 38ef7a3..13f2e7b 100644 --- a/logseq_journal.opam.locked +++ b/logseq_journal.opam.locked @@ -23,7 +23,7 @@ pin-depends: [ ] [ "lui.0.1.0" - "git+https://github.com/logseq/lui.git#91aecb52a1cba2faaf23aac1d64a0bd1cb6549e7" + "git+https://github.com/logseq/lui.git#8de2abae573499692dbcc169a2738c434944a0a8" ] [ "melange-edn-core.0.5.0" diff --git a/test/application_view_test.ml b/test/application_view_test.ml index 343850d..5ceee13 100644 --- a/test/application_view_test.ml +++ b/test/application_view_test.ml @@ -670,7 +670,14 @@ let run_favorites_native_visibility when check_capture_imports || Option.is_some detail_capture -> Atomic.set import_targets (Atomic.get import_targets @ [ source.target ]); Atomic.set imports_entered (Atomic.get imports_entered @ [ source.title ]); + if source.title = "First attachment" then + while not (Atomic.get import_release) do + Eio.Time.Mono.sleep (W.Request_context.clock context) 0.001 + done; if source.title = "First attachment" + && detail_capture <> Some "selection-error" + && (detail_capture <> Some "retry-imports" + || List.length (List.filter (( = ) "First attachment") (Atomic.get imports_entered)) = 1) then ( while not (Atomic.get import_release) do Eio.Time.Mono.sleep (W.Request_context.clock context) 0.001 @@ -1337,7 +1344,7 @@ let run_favorites_native_visibility if List.mem scenario - [ "attachments"; "attachment-only"; "offscreen-attachments" ] + [ "attachments"; "attachment-only"; "offscreen-attachments"; "selection-error"; "picker-owner"; "retry-imports" ] then ( dispatch (Lui_protocol.Press (Option.get (find "accessibility-label" "文件"))); let adapter, request = @@ -1373,27 +1380,49 @@ let run_favorites_native_visibility ; "sourceIdentity", `String title ] in + let first = item 1 "First attachment" in + let second = item 2 "Second attachment" in let batch = `Assoc [ "action", `String "picked-batch" ; "request", request ; ( "items" - , `List [ item 1 "First attachment"; item 2 "Second attachment" ] ) + , `List [ first; second ] ) ] in + let batch = if scenario = "selection-error" then + match batch with `Assoc values -> `Assoc (("error", `String "Fixture partial selection failed") :: values) | _ -> assert false + else batch in + if scenario = "picker-owner" then ( + collapse (); back (); enter 2; open_capture (); edit "Keep other root draft"; + let open Yojson.Safe.Util in + wait "armed picker stays owned during another Composer" (fun () -> + match List.assoc_opt "payload" (Hashtbl.find props adapter) with + | Some (`String payload) -> member "id" (member "request" (Yojson.Safe.from_string payload)) = member "id" request + | _ -> false)); hooks.extension_event adapter "event" (Yojson.Safe.to_string (`Assoc - [ "id", `Int 1; "payload", `String (Yojson.Safe.to_string batch) ])) - |> consume; + [ "id", `Int 1; "payload", `String (Yojson.Safe.to_string batch) ])) |> consume; + if scenario = "picker-owner" then ( + settle_worker (); + Alcotest.(check bool) "late selection cannot overwrite other draft" true + (Option.is_some (find "text" "Keep other root draft")); + collapse (); back (); enter 0; open_capture ()); wait "Detail stages both Composer attachments" (fun () -> List.length (all "accessibility-identifier" ("composer-attachment:" ^ G.Uuid.to_string (uuid 30002))) - = 1)); + = 1); + if scenario = "selection-error" then ( + wait "partial selection failure is visible on its owner" (fun () -> Option.is_some (find "text" "Fixture partial selection failed")); + collapse (); back (); enter 2; open_capture (); + Alcotest.(check bool) "other Detail cannot show prior picker failure" true + (Option.is_none (find "text" "Fixture partial selection failed")); + collapse (); back (); enter 0; open_capture ())); collapse (); open_capture (); if scenario <> "attachment-only" @@ -1464,11 +1493,20 @@ let run_favorites_native_visibility (Atomic.get imports_entered); Atomic.set import_release true; wait "both imports finish" (fun () -> Atomic.get imports_finished = 2); + if scenario = "retry-imports" then ( + wait "failed import exposes target retry" (fun () -> Option.is_some (find "accessibility-label" "Retry imports")); + Alcotest.(check bool) "failed import keeps its own staged file" true (Sys.file_exists (List.hd !staged_paths)); + dispatch (Lui_protocol.Press (Option.get (find "accessibility-label" "Retry imports"))); + wait "same child import retries" (fun () -> Atomic.get imports_finished = 3); + Alcotest.(check int) "import retry does not create another child" 1 (Atomic.get child_attempts)); + if not (List.mem scenario [ "retry-imports"; "selection-error" ]) then ( + wait "failed import offers explicit discard" (fun () -> Option.is_some (find "accessibility-label" "Dismiss")); + dispatch (Lui_protocol.Press (Option.get (find "accessibility-label" "Dismiss")))); wait "all staging files are released" (fun () -> List.for_all (fun p -> not (Sys.file_exists p)) !staged_paths); Alcotest.(check (list string)) "imports target the newly appended child" - [ G.Uuid.to_string child.uuid; G.Uuid.to_string child.uuid ] + (List.init (if scenario = "retry-imports" then 3 else 2) (fun _ -> G.Uuid.to_string child.uuid)) (List.map G.Uuid.to_string (Atomic.get import_targets)); if scenario = "offscreen-attachments" then @@ -1477,9 +1515,18 @@ let run_favorites_native_visibility true (Option.is_some (find "text" "Keep new root draft"))); if scenario <> "offscreen-attachments" - then + then ( wait "completion closes Detail Composer" (fun () -> - Option.is_none (find "style-class" "composer-input"))) + Option.is_none (find "style-class" "composer-input")); + if scenario = "selection-error" then ( + open_capture (); + Alcotest.(check bool) "completed Detail retires picker failure" true + (Option.is_none (find "text" "Fixture partial selection failed")); + collapse (); back (); + dispatch (Lui_protocol.Press (Option.get (find "accessibility-label" "Capture"))); + wait "Timeline Composer opens" (fun () -> Option.is_some (find "accessibility-identifier" "journal-capture-composer")); + Alcotest.(check bool) "Detail picker failure cannot leak to Timeline" true + (Option.is_none (find "text" "Fixture partial selection failed"))))) detail_capture; if Option.is_none detail_capture then ( (* Routes reducers cannot own LUI mount/subscription retention. The @@ -2637,11 +2684,13 @@ let run_favorites_native_visibility ; "sourceIdentity", `String title ] in + let first = item 1 "First attachment" in + let second = item 2 "Second attachment" in let batch = `Assoc [ "action", `String "picked-batch" ; "request", request - ; "items", `List [ item 1 "First attachment"; item 2 "Second attachment" ] + ; "items", `List [ first; second ] ] in hooks.extension_event @@ -2675,8 +2724,10 @@ let run_favorites_native_visibility Atomic.set import_release true; wait "second attachment continues after first failure" (fun () -> Atomic.get imports_finished = 2); - wait "terminal import responses release both staging files" (fun () -> - List.for_all (fun path -> not (Sys.file_exists path)) !staged_paths); + wait "success releases staging while failure retains retry bytes" (fun () -> + match !staged_paths with + | [failed; successful] -> Sys.file_exists failed && not (Sys.file_exists successful) + | _ -> false); Alcotest.(check (list string)) "batch preserves picker order" [ "First attachment"; "Second attachment" ] @@ -2686,7 +2737,11 @@ let run_favorites_native_visibility dispatch (Lui_protocol.Press (Option.get (find "accessibility-label" "Error info"))); wait "Error info retains the first batch failure" (fun () -> - Option.is_some (find "text" "Fixture first attachment failed"))); + Option.is_some (find "text" "Fixture first attachment failed")); + dispatch (Lui_protocol.Press (Option.get (find "accessibility-label" "Close"))); + dispatch (Lui_protocol.Press (Option.get (find "accessibility-label" "Dismiss"))); + wait "explicit discard releases failed retry bytes" (fun () -> + List.for_all (fun path -> not (Sys.file_exists path)) !staged_paths)); if check_ios_capture then ( dispatch @@ -4215,6 +4270,9 @@ let () = ; "attachments" ; "attachment-only" ; "offscreen-attachments" + ; "selection-error" + ; "picker-owner" + ; "retry-imports" ] ) ; ( "Capture attachment imports" , [ Alcotest.test_case diff --git a/test/journal_routes_test.ml b/test/journal_routes_test.ml index d35452a..b509af9 100644 --- a/test/journal_routes_test.ml +++ b/test/journal_routes_test.ml @@ -2548,8 +2548,78 @@ let test_multiselect_graph_owner_and_cancel () = | _ -> assert false) ;; +let test_retained_detail_picker_owner_and_terminal_delete () = + let module R = Journal_routes in + let root = block () in + let live = load_parent (R.create ()) root 980L in + let owner = entry_id live in + let live = + R.update_detail + live + (Journal_detail.update_child_source (Option.get (R.detail live)) "Owned draft") + in + let path = Filename.temp_file "journal-import-" ".txt" in + Fun.protect + ~finally:(fun () -> if Sys.file_exists path then Sys.remove path) + (fun () -> + let pick = + staged_pick + ~operation:"70000000-0000-4000-a000-00000000a090" + ~path + ~title:"Retained selection" + ~file_type:"txt" + in + let detached = R.pop_to_root live in + let updated = + R.map_child_capture_at + detached + ~entry_id:owner + ~request_generation:980L + ~f:(fun capture -> Journal_capture.add_attachment capture pick) + in + require + (List.length + (Journal_capture.pending_attachments + (Option.get + (R.child_capture_at updated ~entry_id:owner ~request_generation:980L))) + = 1) + "late picker lost the detached draft owner"; + require + (R.map_child_capture_at + updated + ~entry_id:owner + ~request_generation:979L + ~f:(fun capture -> + Journal_capture.remove_attachment + capture + ~token:(Journal_asset_import.staged_token pick)) + == updated) + "stale picker generation modified a retained draft"; + let reopened = load_parent updated root 990L in + let staged, undo = R.stage_delete reopened ~block_id:(Journal_model.id root) in + require + (List.length (R.pending_attachments staged) = 1) + "Undo staging retired the attachment too early"; + let restored = R.undo_delete staged (Option.get undo) in + require + (List.length (R.pending_attachments restored) = 1) + "Undo failed to restore the attachment"; + let committed, released = + R.commit_delete staged ~block_id:(Journal_model.id root) + in + require + (List.length released = 1 && R.pending_attachments committed = []) + "committed root deletion retained a draft or lost cleanup ownership"; + let fresh = R.pop_to_root committed |> fun t -> load_parent t root 1000L in + require + (Journal_detail.child_capture (Option.get (R.detail fresh)) = None) + "committed deletion revived its retired draft") +;; + let tests = - [ "typed path prefix and loading owners", test_typed_path_prefix_and_loading_owners + [ ( "retained picker and terminal delete" + , test_retained_detail_picker_owner_and_terminal_delete ) + ; "typed path prefix and loading owners", test_typed_path_prefix_and_loading_owners ; "multiselect batch boundary", test_multiselect_batch_boundary ; "multiselect order duplicates and readd", test_multiselect_order_duplicates_and_readd ; ( "multiselect rejected resource ownership" diff --git a/test/source_boundary_test.ml b/test/source_boundary_test.ml index af1256d..ce03cf7 100644 --- a/test/source_boundary_test.ml +++ b/test/source_boundary_test.ml @@ -1125,7 +1125,7 @@ let () = (* Journal uses the upstream native host ABI decoder; keep the OCaml installation and Apple package on the same validated revision. *) let compatible_lui_revision = - "git+https://github.com/logseq/lui.git#91aecb52a1cba2faaf23aac1d64a0bd1cb6549e7" + "git+https://github.com/logseq/lui.git#8de2abae573499692dbcc169a2738c434944a0a8" in List.iter (fun relative -> require_occurrences root relative compatible_lui_revision 1)