-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Add Media Library upload data types and tracker events #25619
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
crazytonyli
wants to merge
4
commits into
trunk
Choose a base branch
from
task/media-v2-upload-foundation
base: trunk
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
de0286a
Add Media Library upload data types and tracker events
crazytonyli 655eccc
Address review feedback on upload analytics and strings
crazytonyli 158d784
Remove MediaUploadPolicy.init default values
crazytonyli 2e38b10
Tighten MediaTrackerAdapter upload-event test assertions
crazytonyli File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
16 changes: 16 additions & 0 deletions
16
Modules/Sources/WordPressMediaLibrary/Models/FailedUpload.swift
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| import Foundation | ||
|
|
||
| struct FailedUpload: Identifiable, Sendable { | ||
| let id: UUID | ||
| let displayName: String | ||
| let kind: MediaKind | ||
| /// Localized error message. The uploader stores | ||
| /// `(error as NSError).localizedDescription` for HTTP failures and a | ||
| /// localized materializer-error message for pre-upload failures. | ||
| let errorMessage: String | ||
| /// True when the actor can rerun the upload from the stored params + | ||
| /// temp file. False for materialization failures, where the original | ||
| /// `MediaCreateParams` / temp file were never produced — the | ||
| /// Uploads-screen row should offer Dismiss only. | ||
| let isRetryable: Bool | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
69 changes: 69 additions & 0 deletions
69
Modules/Sources/WordPressMediaLibrary/Models/MediaUploadPolicy.swift
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| import Foundation | ||
| import UniformTypeIdentifiers | ||
|
|
||
| /// Upload policy injected by the app target. The module honors this struct | ||
| /// but never derives it — `Blog.allowedFileTypes`, user-media settings, etc. | ||
| /// stay on the app side. Picker affordance and upload validation are split | ||
| /// because the materializer validates the effective post-transform type and | ||
| /// extension, not just the source file the picker exposed. | ||
| public struct MediaUploadPolicy: Sendable { | ||
| /// UTTypes the document picker (`.fileImporter`) offers. May include | ||
| /// broad fallbacks like `.content` when the server allow-list is empty. | ||
| /// **Not** the upload validator. Photos and camera pickers do not read | ||
| /// this field — they have their own hard-coded image/video filters. | ||
| let filePickerContentTypes: [UTType] | ||
|
|
||
| /// Real upload allow/deny gate. Called by the materializer just before | ||
| /// enqueue with the *effective* `(UTType, file-extension)` pair after | ||
| /// any transform. App target typically backs this with | ||
| /// `Blog.allowedFileTypes` + the default mobile-allowed-extensions list. | ||
| let isAllowedForUpload: @Sendable (_ contentType: UTType, _ fileExtension: String) -> Bool | ||
|
|
||
| /// Resize the longest edge of images to at most this many pixels. `nil` | ||
| /// means no cap. Applied before JPEG re-encode. | ||
| let imageMaxDimension: Int? | ||
|
|
||
| /// JPEG quality for re-encoded images (0.0...1.0). Used both when | ||
| /// resizing and when converting HEIC → JPEG. | ||
| let imageJpegQuality: Double | ||
|
|
||
| /// If true, HEIC sources are converted to JPEG before upload. | ||
| let convertHEICToJPEG: Bool | ||
|
|
||
| /// Video duration cap in seconds. Over-duration videos are rejected | ||
| /// (V1 parity, no trim). | ||
| let videoMaxDurationSeconds: TimeInterval? | ||
|
|
||
| /// `AVAssetExportSession` preset name. Controls quality only. | ||
| let videoExportPreset: String | ||
|
|
||
| /// Output container UTType for re-exported videos. Default | ||
| /// `.mpeg4Movie`. Drives the file extension of the materialized temp | ||
| /// file and the effective MIME type the validator checks against. | ||
| let videoOutputContentType: UTType | ||
|
|
||
| /// If true, strip GPS EXIF before upload. | ||
| let stripImageLocation: Bool | ||
|
|
||
| public init( | ||
| filePickerContentTypes: [UTType], | ||
| isAllowedForUpload: @escaping @Sendable (UTType, String) -> Bool, | ||
| imageMaxDimension: Int? = nil, | ||
| imageJpegQuality: Double = 0.9, | ||
| convertHEICToJPEG: Bool = true, | ||
| videoMaxDurationSeconds: TimeInterval? = nil, | ||
| videoExportPreset: String, | ||
| videoOutputContentType: UTType = .mpeg4Movie, | ||
| stripImageLocation: Bool = false | ||
| ) { | ||
| self.filePickerContentTypes = filePickerContentTypes | ||
| self.isAllowedForUpload = isAllowedForUpload | ||
| self.imageMaxDimension = imageMaxDimension | ||
| self.imageJpegQuality = imageJpegQuality | ||
| self.convertHEICToJPEG = convertHEICToJPEG | ||
| self.videoMaxDurationSeconds = videoMaxDurationSeconds | ||
| self.videoExportPreset = videoExportPreset | ||
| self.videoOutputContentType = videoOutputContentType | ||
| self.stripImageLocation = stripImageLocation | ||
| } | ||
| } | ||
11 changes: 11 additions & 0 deletions
11
Modules/Sources/WordPressMediaLibrary/Models/PendingUpload.swift
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| import Foundation | ||
| import WordPressAPI | ||
|
|
||
| /// View-model-facing surface of an in-flight upload. The actor stores a | ||
| /// richer internal value with the `Task` handle and owned temp-file URL. | ||
| struct PendingUpload: Identifiable, Sendable { | ||
| let id: UUID | ||
| let displayName: String // basename of the temp file | ||
| let kind: MediaKind // for icon + Uploads-row rendering | ||
| let progress: Progress // bound to ProgressView directly | ||
| } |
88 changes: 88 additions & 0 deletions
88
Modules/Sources/WordPressMediaLibrary/Models/UploadSource.swift
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| import Foundation | ||
| import UIKit | ||
| import UniformTypeIdentifiers | ||
|
|
||
| /// Picker-output payload that the materializer consumes. Variants carry the | ||
| /// source-of-origin needed for analytics — `MediaLibraryViewModel` reads | ||
| /// the case to fire `.mediaLibraryAdded(source:kind:)` *before* enqueueing, | ||
| /// so the actor never has to derive analytics from picker shape. | ||
| enum UploadSource: @unchecked Sendable { | ||
| /// `PHPickerResult.itemProvider` plus its `suggestedName` (typically | ||
| /// "IMG_1234" or nil) and a UTType hint from the picker selection. | ||
| case photoLibrary(itemProvider: NSItemProvider, suggestedName: String?, hint: UTType) | ||
|
|
||
| /// Captured image from the camera. `Date` is the capture moment used | ||
| /// for the filename pattern `IMG_<yyyy-MM-dd HH-mm-ss>.jpg`. | ||
| case cameraImage(UIImage, capturedAt: Date) | ||
|
|
||
| /// Captured video file from the camera, already at a temp URL. | ||
| case cameraVideo(URL, capturedAt: Date) | ||
|
|
||
| /// File-importer URL. Materializer reads it under | ||
| /// `startAccessingSecurityScopedResource()`. | ||
| case file(URL) | ||
|
|
||
| /// Remote-URL source for external pickers (Stock Photos). The | ||
| /// materializer downloads bytes via `RemoteDownloader` before dispatching | ||
| /// to the image / GIF / disallowed branches. | ||
| case remoteURL(RemoteURL) | ||
|
|
||
| /// Image Playground (iOS 18.1+) returns a local file URL in our app | ||
| /// sandbox. The materializer copies bytes without security-scoped access | ||
| /// and dispatches to `materializeFileImage`. | ||
| case imagePlayground(URL, suggestedName: String) | ||
| } | ||
|
|
||
| extension UploadSource { | ||
| /// Internal carrier for `.remoteURL`. The public boundary type | ||
| /// `ExternalRemoteMedia` is converted to this in the view model before | ||
| /// enqueueing — keeps `UploadSource` module-internal. | ||
| struct RemoteURL: Sendable { | ||
| let url: URL | ||
| let suggestedName: String | ||
| let contentType: UTType | ||
| let caption: String? | ||
| } | ||
| } | ||
|
|
||
| extension UploadSource { | ||
| /// Fraction of the overall upload progress allocated to the | ||
| /// materialization stage. On-device sources are fast to materialize | ||
| /// relative to the upload itself. | ||
| var materializationProgressWeight: Double { | ||
| switch self { | ||
| case .photoLibrary, .cameraImage, .cameraVideo, .file, .imagePlayground: | ||
| return 0.05 | ||
| case .remoteURL: | ||
| // Remote sources download the full file during materialization, then | ||
| // upload the same bytes, so split the bar evenly between the two. | ||
| // The real download-vs-upload time ratio varies by network, so this | ||
| // weight only affects how smoothly the row advances, not correctness. | ||
| return 0.5 | ||
| } | ||
| } | ||
|
|
||
| /// Best-effort media kind derived from the picker payload before the | ||
| /// upload is materialized, used for the pre-enqueue analytics event and | ||
| /// the initial Uploads-row icon. The materializer later derives the | ||
| /// authoritative kind from the post-transform content type. | ||
| var estimatedKind: MediaKind { | ||
| switch self { | ||
| case .photoLibrary(_, _, let hint): | ||
| return MediaKind(estimating: hint) | ||
| case .cameraImage: | ||
| return .image | ||
| case .cameraVideo: | ||
| return .video | ||
| case .file(let url): | ||
| let contentType = | ||
| (try? url.resourceValues(forKeys: [.contentTypeKey]))?.contentType | ||
| ?? UTType(filenameExtension: url.pathExtension) | ||
| return contentType.map { MediaKind(estimating: $0) } ?? .document | ||
| case .remoteURL(let remote): | ||
| return MediaKind(estimating: remote.contentType) | ||
| case .imagePlayground: | ||
| return .image | ||
| } | ||
| } | ||
| } |
41 changes: 41 additions & 0 deletions
41
Modules/Sources/WordPressMediaLibrary/Models/UploaderState.swift
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| import Foundation | ||
|
|
||
| /// One row in the upload queue, in submission order. Failing in-flight | ||
| /// keeps the row at its original position so the Uploads screen does not | ||
| /// reshuffle when an upload transitions to failed (or back to pending | ||
| /// after Retry). | ||
| enum UploadEntry: Identifiable, Sendable { | ||
| case pending(PendingUpload) | ||
| case failed(FailedUpload) | ||
|
|
||
| var id: UUID { | ||
| switch self { | ||
| case .pending(let p): return p.id | ||
| case .failed(let f): return f.id | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// Snapshot of the uploader's queue. Emitted whenever any entry changes. | ||
| /// `entries` preserves submission order; `pendingCount` / `failedCount` | ||
| /// are derived for the banner. | ||
| struct UploaderState: Sendable { | ||
| let entries: [UploadEntry] | ||
|
|
||
| init(entries: [UploadEntry] = []) { | ||
| self.entries = entries | ||
| } | ||
|
|
||
| var isEmpty: Bool { entries.isEmpty } | ||
|
|
||
| var pendingCount: Int { pending.count } | ||
| var failedCount: Int { failed.count } | ||
|
|
||
| var pending: [PendingUpload] { | ||
| entries.compactMap { if case .pending(let p) = $0 { return p } else { return nil } } | ||
| } | ||
|
|
||
| var failed: [FailedUpload] { | ||
| entries.compactMap { if case .failed(let f) = $0 { return f } else { return nil } } | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit – I think this defaulted to
truebefore, and it probably still should?Follow-on nit: should this apply to videos as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have removed these default values in 158d784