feat(cli): add sales report downloads - #5
Conversation
There was a problem hiding this comment.
This change adds App Store Connect sales report download support to the CLI. It introduces a new 'reports' crate that defines the API for requesting sales reports, including query parameter construction and a trait for downloading the gzip-compressed TSV files. The CLI is updated to include a 'sales-reports' command group, with a 'download' subcommand that retrieves, decompresses, parses, and outputs the sales report as a JSON array, preserving Apple's original column names and values as strings. The implementation includes error handling for malformed or unexpected TSV data, and tests for the TSV-to-JSON conversion logic. The core client is extended to support raw byte responses and custom headers, required for the sales report endpoint. Reviewers should focus on the TSV parsing logic, error handling, and the new API surface in the reports crate.
Automated review by siGit Code Review · commit 821bdd2
| .iter() | ||
| .map(str::to_owned) | ||
| .collect::<Vec<_>>(); | ||
| if headers.is_empty() || headers.iter().any(|header| header.is_empty()) { |
There was a problem hiding this comment.
warning — The check for empty or duplicate TSV headers will cause the entire report to fail if Apple adds a blank or duplicate column; consider whether this is the desired strictness for production use.
| let mut rows = Vec::new(); | ||
| for (index, record) in reader.records().enumerate() { | ||
| let record = record.with_context(|| format!("parsing TSV row {}", index + 2))?; | ||
| if record.len() != headers.len() { |
There was a problem hiding this comment.
warning — Bailing on any row with a column count mismatch will abort the whole report; if Apple adds trailing columns, this could break parsing. Consider whether partial parsing or a warning would be preferable.
| /// Domain APIs use this for endpoints such as sales reports that return a | ||
| /// compressed file instead of a JSON:API document. | ||
| pub async fn request_bytes<B: Serialize>( | ||
| &self, |
There was a problem hiding this comment.
warning — The new request_bytes method exposes raw byte responses but does not restrict which endpoints can use it; ensure this is not misused for endpoints expecting JSON.
| "/v1/salesReports", | ||
| &request.query(), | ||
| None::<&()>, | ||
| &[("Accept", "application/a-gzip")], |
There was a problem hiding this comment.
warning — The Accept header is hardcoded to 'application/a-gzip'; if Apple changes the required value or adds variants, this may break downloads without clear diagnostics.
Add App Store Connect sales report downloads to the CLI, including account-level sales report retrieval, gzip TSV parsing, and JSON output. Co-Authored-By: siGit Code <sigit@sigit.si>
Reviewed-by: sigit-code[bot] <279100073+sigit-code[bot]@users.noreply.github.com>
821bdd2 to
bc5ad26
Compare
There was a problem hiding this comment.
This change adds App Store Connect sales report downloads to the CLI. It introduces a new 'smbcloud-ascapi-reports' crate for handling the download of sales reports as gzip-compressed TSV files, exposes a CLI subcommand for requesting and converting these reports to JSON, and includes robust TSV-to-JSON conversion logic that handles edge cases like missing, blank, or duplicate headers. The core client is extended to support raw byte responses and custom headers. Tests are included for both the TSV parsing and the new query logic. Reviewers should focus on the TSV-to-JSON conversion, the handling of edge cases in headers, and the integration of the new download flow in the CLI.
Automated review by siGit Code Review · commit bc5ad26
| for (index, record) in reader.records().enumerate() { | ||
| let record = record.with_context(|| format!("parsing TSV row {}", index + 2))?; | ||
| let mut row_headers = headers.clone(); | ||
| let mut used_row_headers = row_headers.iter().cloned().collect::<HashSet<_>>(); |
There was a problem hiding this comment.
warning — Cloning headers for every row may be inefficient for large reports; consider reusing or referencing headers unless per-row header mutation is required.
| let mut used_row_headers = row_headers.iter().cloned().collect::<HashSet<_>>(); | ||
| for column in row_headers.len()..record.len() { | ||
| let base = format!("extra_column_{}", column + 1); | ||
| let mut name = base.clone(); |
There was a problem hiding this comment.
warning — Extra columns in a row are named 'extra_column_N', but this may mask data if Apple adds columns to the end of the header; consider warning or erroring if extra columns appear.
| .enumerate() | ||
| .map(|(index, header)| { | ||
| let base = if header.is_empty() { | ||
| format!("column_{}", index + 1) |
There was a problem hiding this comment.
warning — Blank headers are replaced with 'column_N', but if Apple adds a real column with this name, it could collide; consider a less generic prefix or warning on collision.
| /// Domain APIs use this for endpoints such as sales reports that return a | ||
| /// compressed file instead of a JSON:API document. | ||
| pub async fn request_bytes<B: Serialize>( | ||
| &self, |
There was a problem hiding this comment.
warning — request_bytes does not allow the caller to specify a timeout or streaming; large reports may cause memory or timeout issues.
| version, | ||
| }) | ||
| .await?; | ||
| let rows = sales_report_as_json(&report)?; |
There was a problem hiding this comment.
warning — No explicit error handling for empty or malformed reports beyond header check; consider more granular error messages for common Apple report issues.
Add App Store Connect sales report downloads to the CLI, including account-level sales report retrieval, gzip TSV parsing, and JSON output.