Skip to content

feat: add SellerPerformanceReport support#926

Merged
abuzuhri merged 4 commits into
abuzuhri:mainfrom
ProNotion:feature/seller-performance-report
Apr 25, 2026
Merged

feat: add SellerPerformanceReport support#926
abuzuhri merged 4 commits into
abuzuhri:mainfrom
ProNotion:feature/seller-performance-report

Conversation

@ProNotion

Copy link
Copy Markdown
Collaborator

Summary

  • Implements GET_V2_SELLER_PERFORMANCE_REPORT via a new SellerPerformanceReport class, parsing the JSON response into strongly-typed SellerPerformanceMetrics objects
  • Exposes GetSellerPerformance() and GetSellerPerformanceAsync() on ReportManager, with XML doc comments
  • Covers account health status and score, order defect rates (AFN/MFN), late shipment rate, pre-fulfilment cancellation rate, valid tracking rate, on-time delivery rate, invoice defect rate, and policy violations
  • Adds a Stream constructor alongside the file path constructor for consistency with existing report classes
  • All internal JSON deserialization helpers are correctly scoped as internal
  • README updated with usage example

Test plan

  • Verify GetSellerPerformance() returns a populated SellerPerformanceReport against a live sandbox or real credentials
  • Verify GetSellerPerformanceAsync() behaves correctly with cancellation
  • Verify the Stream constructor parses correctly when passed a stream directly

ProNotion and others added 4 commits December 4, 2024 07:04
Implements GET_V2_SELLER_PERFORMANCE_REPORT via the ReportManager, parsing
the JSON response into strongly-typed SellerPerformanceReport and
SellerPerformanceMetrics classes covering account health, order defect rates
(AFN/MFN), late shipment rate, pre-fulfilment cancellation rate, valid
tracking rate, on-time delivery rate, invoice defect rate, and policy
violations. Adds Stream constructor alongside the file path constructor for
consistency with existing report classes. README updated with usage example.
@sonarqubecloud

Copy link
Copy Markdown

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.

Pull request overview

Adds support for the SP-API Seller Performance V2 report by introducing a new report parser/model and exposing it via ReportManager, plus documenting usage in the README.

Changes:

  • Added SellerPerformanceReport to deserialize GET_V2_SELLER_PERFORMANCE_REPORT JSON into strongly-typed metric objects.
  • Exposed GetSellerPerformance() / GetSellerPerformanceAsync() on ReportManager.
  • Updated README with a usage example.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
Source/FikaAmazonAPI/ReportGeneration/SellerPerformanceReport.cs New JSON-based report parser + public metric models and internal deserialization DTOs.
Source/FikaAmazonAPI/ReportGeneration/ReportManager.cs Adds sync/async entrypoints to request and download the seller performance report.
README.md Documents how to call the new report method.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +13 to +23
/// <summary>
/// Represents the collection of account status objects
/// for each marketplace identified in the Seller Performance report.
/// </summary>
public List<AccountStatus> AccountStatuses { get; set; }

/// <summary>
/// Collection of marketplace-specific performance data
/// </summary>
public List<SellerPerformanceMetrics> Metrics { get; set; } = new List<SellerPerformanceMetrics>();

Copilot AI Apr 20, 2026

Copy link

Choose a reason for hiding this comment

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

AccountStatuses is left uninitialized unless the JSON contains accountStatuses, so consumers may hit null-reference checks while other report classes typically expose empty lists (e.g., InventoryAgingReport.Data). Initialize AccountStatuses to an empty list by default and consider assigning an empty list when the JSON omits the field.

Copilot uses AI. Check for mistakes.
if (stream == null)
return;

using (var reader = new StreamReader(stream))

Copilot AI Apr 20, 2026

Copy link

Choose a reason for hiding this comment

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

The Stream constructor disposes the provided stream because StreamReader is disposed without leaveOpen: true. Other report parsers deliberately leave streams open (see ReportParser.ParseCsvLines(... leaveOpen: true)) and callers may want to reuse the stream after constructing the report. Use a StreamReader with leaveOpen: true (and optionally reset stream.Position to 0 when seekable) to match the codebase’s stream-handling behavior.

Suggested change
using (var reader = new StreamReader(stream))
using (var reader = new StreamReader(stream, System.Text.Encoding.UTF8, true, 1024, true))

Copilot uses AI. Check for mistakes.
Comment on lines +68 to +74
MarketplaceId = metric.MarketplaceId,
RefNumber = refNumber,
SnapshotDate = DateTime.UtcNow,

// Account Health Rating
AccountHealthStatus = metric.AccountHealthRating?.AhrStatus,
AccountHealthScore = metric.AccountHealthRating?.AhrScore,

Copilot AI Apr 20, 2026

Copy link

Choose a reason for hiding this comment

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

SnapshotDate is documented as “Date of the report”, but the parser sets it to DateTime.UtcNow, which can be unrelated to the report’s actual reporting period. Consider deriving it from the report payload (e.g., reportingDateRange) or rename the property/doc to clarify it’s the client parse time.

Copilot uses AI. Check for mistakes.
Comment on lines +561 to +564

#region Performance

/// <summary>

Copilot AI Apr 20, 2026

Copy link

Choose a reason for hiding this comment

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

There is already a #region Performance earlier in this file (currently empty). Adding another #region Performance at the end creates duplicate regions and makes navigation confusing. Either move these methods into the existing region or remove the earlier empty region.

Copilot uses AI. Check for mistakes.
@abuzuhri abuzuhri merged commit 7e13f0c into abuzuhri:main Apr 25, 2026
7 checks passed
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.

3 participants