diff --git a/docs/adr/001-no-listall-method.md b/docs/adr/001-no-listall-method.md new file mode 100644 index 0000000..8947ba0 --- /dev/null +++ b/docs/adr/001-no-listall-method.md @@ -0,0 +1,112 @@ +# ADR-001: No `listAll()` Method in Documents Client + +**Status:** Accepted +**Date:** 2026-01-16 +**Context:** Response to client feature request (REQ-2 in suggested-improvements.md) + +## Context + +A client integration team requested a `listAll()` method that would: +1. Return all documents regardless of status in a single call +2. Support filtering by status codes +3. Optionally auto-paginate to fetch all pages + +The Binect REST API exposes two distinct endpoints for listing documents: +- `GET /documents` - Returns shippable documents (status 2) +- `GET /documents/errors` - Returns erroneous documents (status 7) + +There is no single API endpoint that returns documents across all statuses. + +## Decision + +We will **not** implement a `listAll()` method on the `DocumentsClient` class. + +## Rationale + +### 1. Violates Core Design Principle: 1:1 Semantic Mapping + +The SDK's architecture mandates a "1:1 semantic mapping to REST endpoints" (CLAUDE.md). A `listAll()` method would need to: +- Call multiple API endpoints (`/documents` and `/documents/errors`) +- Merge and deduplicate results +- Present a unified interface that doesn't exist in the underlying API + +This creates abstraction that obscures actual API behavior. + +### 2. Violates Transparency Principle + +The design constraint "Transparency over abstraction: Developers must reason about actual API calls" means developers should understand exactly which API calls are being made. A `listAll()` method hides the fact that multiple calls occur, making it harder to: +- Debug network issues +- Understand rate limiting implications +- Reason about data freshness (documents may change between calls) + +### 3. Incomplete Coverage Cannot Be Fixed + +Even if we implemented `listAll()`, we cannot retrieve documents in all statuses: +- Status 1 (IN_PREPARATION) - No list endpoint +- Status 3 (PRODUCTION_QUEUE) - No list endpoint +- Status 4 (PRINTING) - No list endpoint +- Status 5 (SENT) - No list endpoint +- Status 6 (CANCELED) - No list endpoint + +A method named `listAll()` that doesn't actually list all documents would be misleading. + +### 4. Convenience Layer Alternative Exists + +Developers who need to combine results can do so explicitly: + +```typescript +import { fetchAllPages } from '@binect/js'; + +// Explicit composition - developer knows exactly what's happening +const [shippable, erroneous] = await Promise.all([ + fetchAllPages((limit, offset) => client.documents.list({ limit, offset })), + fetchAllPages((limit, offset) => client.documents.listErrors({ limit, offset })), +]); + +const allAvailable = [...shippable, ...erroneous]; +``` + +This approach: +- Makes the multiple API calls explicit +- Allows parallel execution for better performance +- Gives developers control over error handling per endpoint +- Doesn't pretend to offer functionality the API doesn't support + +## Consequences + +### Positive +- SDK remains a transparent wrapper over the API +- No false promises about "all documents" when API doesn't support it +- Developers understand the actual API limitations +- Easier to maintain as SDK behavior matches API exactly + +### Negative +- Developers must write slightly more code to combine results +- May be perceived as less convenient than competing SDKs + +## Alternatives Considered + +### Alternative 1: Add `listAll()` to Convenience Layer (helpers.ts) + +We considered adding a helper function rather than a method: + +```typescript +export async function listAllDocuments(client: BinectClient): Promise +``` + +**Rejected because:** Even as a helper, the name "listAll" is misleading since it cannot list documents in statuses 1, 3, 4, 5, or 6. A more honest name would be `listShippableAndErroneous()`, which provides little value over explicit composition. + +### Alternative 2: Add Status Filter to Existing Methods + +We considered adding a status filter parameter to `list()`: + +```typescript +await client.documents.list({ status: [2, 7] }); +``` + +**Rejected because:** The API doesn't support this. We would be inventing client-side filtering semantics that don't map to any API capability. + +## References + +- CLAUDE.md - Design Constraints section +- suggested-improvements.md - REQ-2: Add Method to List All Documents diff --git a/src/helpers.ts b/src/helpers.ts index a8f792c..38eb27d 100644 --- a/src/helpers.ts +++ b/src/helpers.ts @@ -5,7 +5,7 @@ * They provide convenient predicates and utilities for common operations. */ -import { DocumentStatus, type Document, type Sending, type ValidationMessage } from './types.js'; +import { DocumentStatus, type Document, type ListResponse, type Sending, type ValidationMessage } from './types.js'; /** * Helper to get status code from document or sending @@ -71,6 +71,18 @@ export function isCanceled(doc: Document | Sending): boolean { return getStatusCode(doc) === DocumentStatus.CANCELED; } +/** + * Check if a document is in production (status 3 or 4). + * This includes documents in the production queue and those currently printing. + */ +export function isInProduction(doc: Document | Sending): boolean { + const status = getStatusCode(doc); + return ( + status === DocumentStatus.PRODUCTION_QUEUE || + status === DocumentStatus.PRINTING + ); +} + /** * Check if a document is in a terminal state (sent, canceled, or erroneous). */ @@ -140,6 +152,30 @@ export function hasWarnings(doc: Document): boolean { return getWarnings(doc).length > 0; } +/** + * Get a summary of all error messages from a document. + * Returns undefined if the document has no errors. + * + * @param doc - The document to extract error messages from + * @param separator - Separator between messages (default: '; ') + * @returns Concatenated error messages or undefined if no errors + * + * @example + * ```typescript + * const doc = await client.documents.get(id); + * if (isErroneous(doc)) { + * console.error('Upload failed:', getErrorSummary(doc)); + * } + * ``` + */ +export function getErrorSummary(doc: Document, separator: string = '; '): string | undefined { + const errors = getErrors(doc); + if (errors.length === 0) { + return undefined; + } + return errors.map((e) => e.message).join(separator); +} + // ============================================================================ // Status Description // ============================================================================ @@ -272,3 +308,66 @@ export async function waitForShippable( options ); } + +// ============================================================================ +// Pagination Utilities +// ============================================================================ + +/** + * Options for fetchAllPages helper. + */ +export interface FetchAllPagesOptions { + /** Number of items per page (default: 100) */ + pageSize?: number; + /** Abort signal for cancellation */ + signal?: AbortSignal; +} + +/** + * Fetch all pages from a paginated list endpoint. + * This is an opt-in convenience helper for retrieving complete result sets. + * + * @param fetchPage - Function that fetches a single page given limit and offset + * @param options - Pagination options + * @returns Array of all items from all pages + * + * @example + * ```typescript + * // Fetch all shippable documents + * const allDocs = await fetchAllPages( + * (limit, offset) => client.documents.list({ limit, offset }) + * ); + * + * // Fetch all erroneous documents with custom page size + * const allErrors = await fetchAllPages( + * (limit, offset) => client.documents.listErrors({ limit, offset }), + * { pageSize: 50 } + * ); + * ``` + */ +export async function fetchAllPages( + fetchPage: (limit: number, offset: number) => Promise>, + options: FetchAllPagesOptions = {} +): Promise { + const pageSize = options.pageSize ?? 100; + const allItems: T[] = []; + let offset = 0; + + while (true) { + if (options.signal?.aborted) { + throw new Error('Fetch aborted'); + } + + const response = await fetchPage(pageSize, offset); + allItems.push(...response.items); + + // Check if we've fetched all items + if (response.items.length < pageSize || allItems.length >= response.total) { + break; + } + + offset += pageSize; + } + + return allItems; +} diff --git a/src/index.ts b/src/index.ts index 54d1db3..4a1d58e 100644 --- a/src/index.ts +++ b/src/index.ts @@ -77,12 +77,14 @@ export { isCanceled, isTerminal, isCancelable, + isInProduction, // Validation helpers getErrors, getWarnings, getInfoMessages, hasErrors, hasWarnings, + getErrorSummary, // Status description getStatusDescription, // Base64 utilities @@ -92,4 +94,7 @@ export { pollUntil, waitForShippable, type PollOptions, + // Pagination utilities + fetchAllPages, + type FetchAllPagesOptions, } from './helpers.js'; diff --git a/src/types.ts b/src/types.ts index 1c42ed4..e0013c2 100644 --- a/src/types.ts +++ b/src/types.ts @@ -313,12 +313,35 @@ export interface Document { } /** - * List response wrapper + * Response structure for paginated list operations. + * + * @remarks + * The actual data is in the `items` array, not `data` or `results`. + * + * @example + * ```typescript + * const response = await client.documents.list(); + * + * // Access the documents array via `items` + * for (const doc of response.items) { + * console.log(doc.filename, doc.status.text); + * } + * + * // Pagination information + * console.log(`Showing ${response.items.length} of ${response.total} documents`); + * + * // Check if more pages exist + * const hasMore = response.offset + response.items.length < response.total; + * ``` */ export interface ListResponse { + /** Array of items returned by the query */ items: T[]; + /** Total number of items matching the query (across all pages) */ total: number; + /** Maximum number of items per page */ limit: number; + /** Number of items skipped (for pagination) */ offset: number; }