Show the folder picker on the platform that needs it most, and upload at once
Two faults, both of my own making, reported from the tablet as "I cannot select a location" and "it does not upload". **The picker button was gated on `target-selected == 1`.** That index was Remote's position while both targets were offered. Making the target list platform-aware narrowed Android's to Remote alone, so Remote became index 0 and the button disappeared — on the one platform where the picker is the *only* way to set a destination, since a device folder is not reachable there at all. It is gated on a boolean derived from the target now. An index into a list whose length varies is not a fact about the target, and writing it as one is what made a correct change break the thing it was meant to fix. **A queued export waited for a sync pass.** Staging first is deliberate — an export is finished on disk the moment it is written, and offline is then just a longer queue — but nothing drained the outbox until the next sync, so "Queued for Exports" sat unchanged and read, fairly, as an upload that never happened. A finished batch that wrote anything now drains immediately. The sync-pass drain stays: the first makes an upload feel immediate, the second is what eventually delivers the exports made in a tunnel. Committed without the parallel session's in-flight collection work, which is mid-save and does not compile; verified by stashing it and building this tree alone. 281 dr-ui tests pass, clippy clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -889,6 +889,11 @@ pub fn drain_batch(
|
||||
rx: Receiver<BatchMessage>,
|
||||
total: usize,
|
||||
reporting: Reporting,
|
||||
// Run when the batch finishes having written something. A queued export
|
||||
// is finished on disk but not where the user asked for it, and waiting
|
||||
// for the next sync pass to notice reads — correctly — as an export that
|
||||
// did not upload.
|
||||
on_exported: impl Fn() + 'static,
|
||||
) {
|
||||
let job = activity.begin(
|
||||
crate::activity::Kind::Export,
|
||||
@@ -958,6 +963,9 @@ pub fn drain_batch(
|
||||
job.finish(text.clone());
|
||||
}
|
||||
settle(&w, reporting, &text);
|
||||
if exported > 0 {
|
||||
on_exported();
|
||||
}
|
||||
stop_timer(&held);
|
||||
return;
|
||||
}
|
||||
|
||||
+58
-1
@@ -376,6 +376,47 @@ fn batch_request(
|
||||
}
|
||||
}
|
||||
|
||||
/// TRACES: FR-EXP-7 | FR-NC-10
|
||||
/// Send anything waiting in the outbox to the server.
|
||||
///
|
||||
/// Called after an export and again on every sync pass. Both, deliberately:
|
||||
/// the first is what makes an upload feel immediate, and the second is what
|
||||
/// eventually delivers the exports made while the train was in a tunnel.
|
||||
/// Running it twice over an empty outbox costs a directory listing.
|
||||
fn drain_outbox(library: &Rc<library_ui::LibraryController>) {
|
||||
// Offline is not a failure worth reporting here — the entries stay
|
||||
// staged and the next pass takes them.
|
||||
if library.is_offline() {
|
||||
return;
|
||||
}
|
||||
let Some((creds, session)) = library.session() else {
|
||||
return;
|
||||
};
|
||||
let outbox = export::outbox_dir(&session.server, &session.user_id);
|
||||
if export::pending_count(&outbox) == 0 {
|
||||
return;
|
||||
}
|
||||
|
||||
let rx = export::spawn_upload(creds, session.user_id.clone(), session.root.clone(), outbox);
|
||||
std::thread::spawn(move || {
|
||||
while let Ok(msg) = rx.recv() {
|
||||
match msg {
|
||||
export::UploadMessage::Status(s) => log::info!("export: {s}"),
|
||||
export::UploadMessage::Finished {
|
||||
uploaded,
|
||||
remaining,
|
||||
error,
|
||||
} => {
|
||||
log::info!("export: {uploaded} uploaded, {remaining} still queued");
|
||||
if let Some(e) = error {
|
||||
log::warn!("export upload stopped: {e}");
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
/// What the export button should say, given where an export would go.
|
||||
///
|
||||
/// The label carries the destination because the button is the only place the
|
||||
@@ -1547,7 +1588,23 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
|
||||
*cancel.borrow_mut() = token.clone();
|
||||
|
||||
let rx = export::spawn_batch(request, token);
|
||||
export::drain_batch(window.as_weak(), &activity, &drain, rx, total, to);
|
||||
|
||||
// Upload the moment the batch is done rather than waiting
|
||||
// for a sync pass. An export bound for the server is
|
||||
// complete on disk the instant it is staged, but it is not
|
||||
// where the user asked for it until this runs — and
|
||||
// "Queued for Exports" sitting unchanged until somebody
|
||||
// presses Sync reads as an export that did not upload.
|
||||
let library_for_drain = library.clone();
|
||||
export::drain_batch(
|
||||
window.as_weak(),
|
||||
&activity,
|
||||
&drain,
|
||||
rx,
|
||||
total,
|
||||
to,
|
||||
move || drain_outbox(&library_for_drain),
|
||||
);
|
||||
},
|
||||
)
|
||||
};
|
||||
|
||||
@@ -201,6 +201,9 @@ pub fn render(window: &AppWindow, controller: &SettingsController) {
|
||||
window.set_settings_target_labels(labels(targets.iter().map(|t| t.label())));
|
||||
window.set_settings_target_selected(index_of(targets, &s.export.target));
|
||||
window.set_settings_destination(s.export.active_destination().into());
|
||||
// Derived from the target itself, never from its position in a list whose
|
||||
// length differs by platform.
|
||||
window.set_settings_browse_available(s.export.target.is_remote());
|
||||
// The field means different things either side of the choice, and a
|
||||
// placeholder saying which is cheaper than a paragraph under it.
|
||||
// The placeholder names what an empty field *means*, which differs by
|
||||
|
||||
@@ -616,6 +616,7 @@ export component AppWindow inherits Window {
|
||||
in property <[string]> settings-browse-entries;
|
||||
in property <bool> settings-browse-loading: false;
|
||||
in property <bool> settings-browse-at-root: true;
|
||||
in property <bool> settings-browse-available: false;
|
||||
in property <[string]> settings-target-labels;
|
||||
in property <int> settings-target-selected: 0;
|
||||
in property <string> settings-error: "";
|
||||
@@ -810,6 +811,7 @@ in property <bool> panel-visible: true;
|
||||
browse-entries: root.settings-browse-entries;
|
||||
browse-loading: root.settings-browse-loading;
|
||||
browse-at-root: root.settings-browse-at-root;
|
||||
browse-available: root.settings-browse-available;
|
||||
target-labels: root.settings-target-labels;
|
||||
target-selected: root.settings-target-selected;
|
||||
error: root.settings-error;
|
||||
|
||||
@@ -126,6 +126,15 @@ export component SettingsPage inherits Rectangle {
|
||||
in property <bool> browse-loading: false;
|
||||
/// At the library root, so there is nowhere up to go.
|
||||
in property <bool> browse-at-root: true;
|
||||
/// Whether the destination is one that can be walked.
|
||||
///
|
||||
/// A boolean from Rust rather than a test on `target-selected`. The index
|
||||
/// was hardcoded to 1, which was Remote's position while both targets were
|
||||
/// offered — and the moment Android's list narrowed to Remote alone, that
|
||||
/// index became 0 and the button vanished on the one platform where it is
|
||||
/// the *only* way to set a destination. An index into a list whose length
|
||||
/// varies is not a fact about the target.
|
||||
in property <bool> browse-available: false;
|
||||
|
||||
callback format-picked(int);
|
||||
callback quality-changed(int);
|
||||
@@ -561,7 +570,7 @@ export component SettingsPage inherits Rectangle {
|
||||
// exact spelling of a path three levels down is how a
|
||||
// destination silently becomes a new folder at the
|
||||
// root.
|
||||
if root.target-selected == 1 && !root.browse-open: HorizontalLayout {
|
||||
if root.browse-available && !root.browse-open: HorizontalLayout {
|
||||
alignment: start;
|
||||
Button {
|
||||
text: "Choose folder…";
|
||||
|
||||
Reference in New Issue
Block a user