Skip to content

feat(cli): add sales report downloads - #5

Open
setoelkahfi wants to merge 2 commits into
developmentfrom
feature/sales-reports
Open

feat(cli): add sales report downloads#5
setoelkahfi wants to merge 2 commits into
developmentfrom
feature/sales-reports

Conversation

@setoelkahfi

Copy link
Copy Markdown
Contributor

Add App Store Connect sales report downloads to the CLI, including account-level sales report retrieval, gzip TSV parsing, and JSON output.

@setoelkahfi setoelkahfi self-assigned this Sep 12, 2026

@sigit-code sigit-code Bot 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.

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

Comment thread crates/cli/src/main.rs Outdated
.iter()
.map(str::to_owned)
.collect::<Vec<_>>();
if headers.is_empty() || headers.iter().any(|header| header.is_empty()) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

Comment thread crates/cli/src/main.rs Outdated
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() {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

Comment thread crates/core/src/client.rs
/// 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,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

Comment thread crates/reports/src/lib.rs
"/v1/salesReports",
&request.query(),
None::<&()>,
&[("Accept", "application/a-gzip")],

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

setoelkahfi and others added 2 commits September 12, 2026 19:08
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>

@sigit-code sigit-code Bot 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.

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

Comment thread crates/cli/src/main.rs
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<_>>();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

warning — Cloning headers for every row may be inefficient for large reports; consider reusing or referencing headers unless per-row header mutation is required.

Comment thread crates/cli/src/main.rs
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();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

Comment thread crates/cli/src/main.rs
.enumerate()
.map(|(index, header)| {
let base = if header.is_empty() {
format!("column_{}", index + 1)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

Comment thread crates/core/src/client.rs
/// 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,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

warning — request_bytes does not allow the caller to specify a timeout or streaming; large reports may cause memory or timeout issues.

Comment thread crates/cli/src/main.rs
version,
})
.await?;
let rows = sales_report_as_json(&report)?;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

warning — No explicit error handling for empty or malformed reports beyond header check; consider more granular error messages for common Apple report issues.

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.

1 participant