Skip to content

feat(ios): offer to trash the original when removing an offline copy - #228

Closed
altaywtf wants to merge 17 commits into
nextfrom
feat/offline-remove-original
Closed

feat(ios): offer to trash the original when removing an offline copy#228
altaywtf wants to merge 17 commits into
nextfrom
feat/offline-remove-original

Conversation

@altaywtf

Copy link
Copy Markdown
Member

Problem

Removing an offline download only cleared the local copy; there was no way to also let go of the put.io original, and any such action must stay explicit, follow the account's Trash setting, and never report success the server did not confirm.

Refs #106

Solution

  • Removing a download (swipe, context menu, or multi-select) now confirms through a three-choice dialog: Remove download (default, local only), Remove download and move original to Trash (destructive; worded …and delete original when Trash is disabled, with no recovery claim), and Cancel. The title names the file or count and the message explains that other devices signed in to the account are affected.
  • The original choice is disabled while account preferences are stale or saving, the same gate the Files browser applies, because PutioRuntime.deleteFile refuses those requests.
  • PutioOfflineQueue.removeDeletingOriginals removes the local copies first, then asks PutioRuntime.deleteFile for each original. A remote failure never resurrects the local copy: the app reports which originals are still on put.io with a Try again action for transient causes, and no retry when put.io no longer has the file. A notFound answer counts as done, and confirmed originals reconcile loaded file lists.
  • Originals put.io has not answered for are persisted in the queue document as pendingOriginals, written in the same document update as the local removal. restore() retries them on the next launch and surfaces failures in the report. OK gives them up; Try again keeps them owed.
  • Failures merge into originalFailure per file, so concurrent removals never erase each other's retry targets; a confirmed original drops out.
  • Each target records the Trash mode the confirmation promised. Before each request pass the account preferences are refreshed from the server. If the setting cannot be confirmed, nothing is sent and every original is reported as retryable; if it differs from the promised mode, the queue refuses the request with a non-retryable "setting changed" failure instead of deleting permanently under a move-to-Trash confirmation. The report's title and message describe the authorized mode and name any original that was not sent.
  • Confirmed originals from every path, including the restored retry, reach folderRefreshRequests so cached Files lists reconcile.
  • The deleteOriginal request refuses when the signed-in account is not the queue's owner, so a request that outlives a sign-out stays owed instead of running under another account. purgeAccountStorage() (Clear Data, Destroy Account) drops owed originals and the report and ends requests in flight; a queue whose session has ended (the session's authentication generation moved on) starts no original work and writes nothing, so late answers never overwrite the next shell's document.
  • Both choices carry distinct titles and identifiers so VoiceOver hears different actions.
  • Seeded API: the first delete of the root video (412) fails with a 503 so the journey proves the honest failure report and retry. Harness contract gains runtime-downloads-remove.png (36 screenshots).

Proof

  • mise run verify passed locally.
  • PutioFeatureTests OfflineDownloadsTests cover local-before-remote ordering, plain removal never touching the original, partial remote failure with retryable targets, pending originals surviving a relaunch and being given up on dismissal, retry keeping them owed, a restored retry reconciling lists, an account purge ending in-flight requests, a queue whose session ended writing nothing, a changed Trash setting refusing the retry, merged failures across concurrent requests, error-reason mapping, and Trash-aware copy.
  • mise run harness -- journey --platform ios --scenario files-browser passed 1/1; the Downloads preflight asserts the dialog title, other-devices message, both labeled actions, the failure report after the seeded 503, and the successful retry.
  • Confirmation dialog capture: runtime-downloads-remove.png

Remove confirmation

Removing a download now confirms through a three-choice dialog: the
default removes only the local copy, a separately worded destructive
choice also moves the put.io original to Trash (or deletes it when Trash
is disabled), and Cancel keeps both. The local removal completes before
the remote request, and a remote failure is reported with retry instead
of being folded into success.

Refs #106
The delete request and its failure report now live on the offline queue
instead of a view-owned task, so leaving Downloads mid-request neither
cancels the remaining deletes nor hides the retry. The order test asserts
the local copy is gone before the server is asked.
…sing originals as done

A confirmed original deletion requests a refresh of every loaded folder so
Files and Search drop the row. put.io answering not-found counts as the
end state already holding instead of a contradictory failure. The seeded
root listing drops the video after the accepted retry so the journey
proves the reconciliation.
Copilot AI lite review requested due to automatic review settings September 13, 2026 03:27
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 13, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-13T03:34:47.497933Z 5ec837e PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The downloads removal confirmation dialog’s copy can drift if trashEnabled updates while the dialog is shown, making the UI text inconsistent with the promised destructive mode captured in the pending targets.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds an explicit, Trash-setting-aware option to delete the put.io original when removing an offline download, while preserving the “local only” path as the default and persisting any owed remote deletions for honest retry/reporting across relaunch.

Changes:

  • Replaces the downloads removal alert with a three-option confirmation dialog (local-only default vs destructive “remove + delete/trash original”), plus a failure report with retry.
  • Extends the offline queue to persist and retry “pending originals” and to sequence local removal before remote deletion, with session-bound safety.
  • Updates harness fixtures, docs, and UI/unit tests to cover the new dialog, seeded transient failure, and retry flow.
File summaries
File Description
Tools/PutioHarness/Tests/PutioHarnessKitTests/ModelsTests.swift Updates harness contract tests to include the new downloads removal screenshot attachment.
Tools/PutioHarness/Sources/PutioHarnessKit/Models.swift Adds runtime-downloads-remove to the contract attachment list.
Tests/iOSUITests/Sources/DownloadsJourneyTests.swift Expands the iOS downloads journey to assert dialog wording/accessibility, seeded remote failure report, and retry behavior.
Tests/iOS/Sources/OfflineDownloadsTests.swift Adds extensive unit coverage for local-before-remote ordering, retry persistence, session/purge safety, and Trash-setting drift handling.
Packages/PutioCore/Sources/PutioCore/Session/PutioSessionStore.swift Exposes and documents authenticationGeneration to let long-lived work detect session boundaries.
docs/HARNESS.md Updates journey description and screenshot count/list to include the new downloads removal capture.
Apps/Shared/Sources/HarnessSeededAPI.swift Seeds a transient failure on the first delete of file 412 and models the “trashed” state in listings for retry proof.
Apps/iOS/Sources/PutioApp.swift Wires Trash/preference gating into the downloads UI and hooks originals-deleted notifications to folder refresh requests.
Apps/iOS/Sources/OfflineRemoval.swift Introduces removal outcome/failure models and copy generation for confirmation + failure reporting.
Apps/iOS/Sources/OfflineDownloadsView.swift Implements the new confirmation dialog actions and the remote-failure alert with retry/OK flows and accessibility identifiers.
Apps/iOS/Sources/OfflineDownloads.swift Persists pendingOriginals, adds original-delete orchestration, merges failures across concurrent requests, and retries owed deletions on restore.
Review details
  • Files reviewed: 11/11 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@State private var pendingRemoval: [PutioOfflineRemovalTarget]?
@State private var detailItem: PutioOfflineItem?

private var copy: PutioOfflineRemovalCopy { PutioOfflineRemovalCopy(trashEnabled: trashEnabled) }
@altaywtf

Copy link
Copy Markdown
Member Author

Closing; this lands as a stack of three smaller PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants