Skip to content

feat: add transaction write builder and request API enhancements#3

Open
spiflicate wants to merge 1 commit intomainfrom
copilot/staged-pr-20260315-015743
Open

feat: add transaction write builder and request API enhancements#3
spiflicate wants to merge 1 commit intomainfrom
copilot/staged-pr-20260315-015743

Conversation

@spiflicate
Copy link
Owner

@spiflicate spiflicate commented Mar 15, 2026

Summary by CodeRabbit

Release Notes

  • New Features
    • Added transaction management capabilities: create, edit, and cancel transactions with a new fluent builder.
    • Introduced new query filters (game types, codes, seasons, availability) for enhanced searching.
    • Added comprehensive transaction workflow examples.

Copilot AI review requested due to automatic review settings March 15, 2026 05:57
@coderabbitai
Copy link

coderabbitai bot commented Mar 15, 2026

📝 Walkthrough

Walkthrough

This PR introduces transaction write support to the Yahoo Fantasy API client by adding a TransactionBuilder class for constructing add/drop and trade payloads, extending RequestBuilder with create(), edit(), and cancel() methods for write operations, and providing comprehensive examples and test coverage.

Changes

Cohort / File(s) Summary
Transaction Builder Core
src/request/transaction.ts, src/request/index.ts, src/index.ts
Introduces new TransactionBuilder class supporting fluent API for add/drop and trade transaction payloads with mode inference and validation. Exports re-added to public API surface.
Request Builder Extensions
src/request/builder.ts
Extends RequestBuilder with write support: new create(), edit(), cancel() methods; XML serialization via serializeToYahooXml(); dispatch path routing; query parameter methods (isAvailable, gameTypes, gameCodes, seasons); POST/PUT/DELETE handling with pending write cleanup.
Examples
examples/request-builder/02-transactions.ts
New example script demonstrating 18 transaction scenarios: list/filter/fetch transactions, add/drop players, propose/accept/reject trades, waiver edits, and cancellations with optional API execution.
Type Documentation
src/types/responses/league.ts
Minor documentation update: clarified renewed field description from "in a previous season" to "in the following season".
New Test Suites
tests/unit/transaction-builder.test.ts, tests/unit/request-builder.test.ts
Comprehensive unit tests for TransactionBuilder (add/drop/trade payload construction, mode inference, error handling) and RequestBuilder write flows (path dispatch, XML serialization, stage validation).
Updated Tests
tests/integration/auth/oauth1.test.ts, tests/integration/workflows/e2e.test.ts, tests/unit/client/RequestBuilderClient.test.ts, tests/unit/errors.test.ts, tests/unit/formatters.test.ts
Refactored tests to align with new response shapes; simplified API call chains; reordered imports; expanded test coverage for game filtering and transaction construction.

Sequence Diagram

sequenceDiagram
    actor Client
    participant RequestBuilder
    participant TransactionBuilder
    participant XMLSerializer
    participant HttpClient
    participant YahooAPI

    Client->>RequestBuilder: create(transactionBuilder)
    RequestBuilder->>TransactionBuilder: toPayload()
    TransactionBuilder-->>RequestBuilder: {transaction: {...}}
    RequestBuilder->>XMLSerializer: serializeToYahooXml(payload)
    XMLSerializer-->>RequestBuilder: <transaction>...</transaction>
    RequestBuilder->>RequestBuilder: setPendingWriteRequest(POST, path, xml)
    RequestBuilder->>HttpClient: execute()
    HttpClient->>YahooAPI: POST /transactions (xml body)
    YahooAPI-->>HttpClient: 200 OK
    HttpClient-->>RequestBuilder: response
    RequestBuilder->>RequestBuilder: clearPendingWriteRequest()
    RequestBuilder-->>Client: result

    Client->>RequestBuilder: transaction(key).edit(edits)
    RequestBuilder->>XMLSerializer: serializeToYahooXml(edits)
    XMLSerializer-->>RequestBuilder: <transaction>...</transaction>
    RequestBuilder->>RequestBuilder: setPendingWriteRequest(PUT, dispatchPath, xml)
    RequestBuilder->>HttpClient: execute()
    HttpClient->>YahooAPI: PUT /transactions/key (xml body)
    YahooAPI-->>HttpClient: 200 OK
    HttpClient-->>RequestBuilder: response
    RequestBuilder->>RequestBuilder: clearPendingWriteRequest()
    RequestBuilder-->>Client: result
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

Poem

🐰 A builder hops in, with transactions in tow,
Add, drop, trade proposals—the fluent flows grow!
XML serialized, dispatch paths align,
Transaction writes now—the API design!
Tests hop and play, examples abound,
In the Warren of Requests, new magic we've found! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: add transaction write builder and request API enhancements' accurately summarizes the main changes: introduction of a TransactionBuilder for write operations and extensions to the RequestBuilder API.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch copilot/staged-pr-20260315-015743
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds first-class support for composing and executing Yahoo transaction write requests (POST/PUT/DELETE) via the fluent request builder, including a dedicated TransactionBuilder for common transaction payloads.

Changes:

  • Introduce TransactionBuilder for add/drop and pending trade payload construction.
  • Enhance RequestBuilder with transaction write helpers (create, edit, cancel) and automatic XML serialization for transaction write bodies.
  • Add games collection filter helpers (isAvailable, gameTypes, gameCodes, seasons) and expand unit/integration test coverage + new transactions example.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/unit/transaction-builder.test.ts New unit tests covering TransactionBuilder payload construction and validation.
tests/unit/request-builder.test.ts Adds coverage for games filter helpers, XML serialization, and transaction write flows (create/edit/cancel).
tests/unit/formatters.test.ts Import reordering/formatting only.
tests/unit/errors.test.ts Import reordering/formatting only.
tests/unit/client/RequestBuilderClient.test.ts Type-level coverage for new request-builder methods + runtime guardrail tests for invalid write-stage usage.
tests/integration/workflows/e2e.test.ts Formatting tweaks and a guard for optional userTeam.name comparison.
tests/integration/auth/oauth1.test.ts Migrates some “advanced” usage to the request builder, and adjusts expectations for players responses.
src/types/responses/league.ts Updates the doc comment for the renewed field.
src/request/transaction.ts New TransactionBuilder implementation (add/drop + trade modes).
src/request/index.ts Re-exports TransactionBuilder from the request module barrel.
src/request/builder.ts Adds XML serialization for transaction writes, new write helpers (create/edit/cancel), dispatch-path support, and games collection filter helpers.
src/index.ts Exposes TransactionBuilder from the package root.
examples/request-builder/02-transactions.ts New end-to-end example demonstrating transaction read/write paths and payloads.

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

Comment on lines +157 to +166
return {
type: 'drop',
player: {
player_key: this.dropPlayerKey,
transaction_data: {
type: 'drop',
source_team_key: this.forTeamKey,
},
},
};
.request()
.games()
// FIXME: verify what params should be available for the games collection
.param('is_available', 'true')
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
examples/request-builder/02-transactions.ts (1)

57-76: Consider logging parse errors in token loading.

The load() method silently swallows all errors including JSON parse failures. In path-only mode this is fine, but for live API usage, logging could help debug token file corruption.

💡 Optional: Log parsing errors
    async load(): Promise<OAuth2Tokens | null> {
       try {
          const data = await fs.readFile(tokenFile, 'utf-8');
          return JSON.parse(data);
-      } catch {
+      } catch (err) {
+         if ((err as NodeJS.ErrnoException).code !== 'ENOENT') {
+            console.warn('Failed to load tokens:', err);
+         }
          return null;
       }
    },
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@examples/request-builder/02-transactions.ts` around lines 57 - 76, The load()
method on the storage object currently swallows all errors; update
storage.load() to catch and log the error (including JSON parse failures) before
returning null so tokenFile/JSON corruption is visible when debugging live API
usage; reference the storage object and its load() method, include the caught
error in a descriptive log message (e.g., mentioning tokenFile and OAuth2Tokens)
and then return null as before.
tests/integration/auth/oauth1.test.ts (1)

120-139: Replace .param('is_available', 'true') with .isAvailable(true) to ensure proper normalization.

The test currently bypasses the isAvailable() method which normalizes boolean values to the numeric format '1' or '0' that the Yahoo Fantasy API expects. Using .param('is_available', 'true') sends the string 'true' directly, which does not match the API's numeric parameter format and will cause unexpected behavior.

Use .isAvailable(true) instead, which properly normalizes the value to '1' via the builder method.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/integration/auth/oauth1.test.ts` around lines 120 - 139, The test uses
.param('is_available', 'true') which bypasses the request builder's
normalization; replace that call with the builder method .isAvailable(true) so
the value is converted to the API-expected numeric format ('1'/'0') — update the
chain starting at .request().games() to call .isAvailable(true) instead of
.param('is_available', 'true') (leave the surrounding .execute() and assertions
unchanged).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@examples/request-builder/02-transactions.ts`:
- Around line 57-76: The load() method on the storage object currently swallows
all errors; update storage.load() to catch and log the error (including JSON
parse failures) before returning null so tokenFile/JSON corruption is visible
when debugging live API usage; reference the storage object and its load()
method, include the caught error in a descriptive log message (e.g., mentioning
tokenFile and OAuth2Tokens) and then return null as before.

In `@tests/integration/auth/oauth1.test.ts`:
- Around line 120-139: The test uses .param('is_available', 'true') which
bypasses the request builder's normalization; replace that call with the builder
method .isAvailable(true) so the value is converted to the API-expected numeric
format ('1'/'0') — update the chain starting at .request().games() to call
.isAvailable(true) instead of .param('is_available', 'true') (leave the
surrounding .execute() and assertions unchanged).

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9ed55186-6fbc-47f7-a63c-f6f78530bb49

📥 Commits

Reviewing files that changed from the base of the PR and between b5f6d0c and afd2b1e.

📒 Files selected for processing (13)
  • examples/request-builder/02-transactions.ts
  • src/index.ts
  • src/request/builder.ts
  • src/request/index.ts
  • src/request/transaction.ts
  • src/types/responses/league.ts
  • tests/integration/auth/oauth1.test.ts
  • tests/integration/workflows/e2e.test.ts
  • tests/unit/client/RequestBuilderClient.test.ts
  • tests/unit/errors.test.ts
  • tests/unit/formatters.test.ts
  • tests/unit/request-builder.test.ts
  • tests/unit/transaction-builder.test.ts

@greptile-apps
Copy link

greptile-apps bot commented Mar 15, 2026

Greptile Summary

This PR introduces a fluent transaction write API on top of the existing RequestBuilder, along with a standalone TransactionBuilder class for constructing typed add/drop and trade payloads. It also extends the games collection with new filter helpers (isAvailable, gameTypes, gameCodes, seasons) and adds auto XML serialization via fast-xml-parser for all transaction write paths.

Key changes:

  • TransactionBuilder (src/request/transaction.ts): new fluent builder that infers add/drop vs trade mode from the methods called, validates required fields, and produces a Record<string,unknown> payload consumed by RequestBuilder.create().
  • RequestBuilder write methods (src/request/builder.ts): adds create(), edit() (overloaded for resource and collection paths), and cancel() (overloaded similarly); a setPendingWriteRequest/clearPendingWriteRequest state machine routes execute() to the correct HTTP verb; post() and put() now auto-serialize object bodies to Yahoo XML when the path matches a transaction endpoint.
  • Games collection params: isAvailable(), gameTypes(), gameCodes(), seasons() helpers added and exposed through the type-level stage system.
  • Tests: extensive new unit tests for TransactionBuilder, XML serialization snapshots, write-path dispatch, and guard/throw behaviour; integration tests migrated from the legacy advanced() API to the new fluent API.
  • Example file: examples/request-builder/02-transactions.ts covers all 18 read/write transaction scenarios.

Notable issues found:

  • is_available is set to 'true' (string) in tests/integration/auth/oauth1.test.ts while the isAvailable() helper correctly uses '1'; the FIXME comment acknowledges this but leaves it unresolved.
  • TransactionBuilder.buildTradeTransaction() assigns type: 'drop' to players appended via dropPlayers(), which may not be the transaction data shape the Yahoo API expects within a pending_trade transaction.
  • TransactionBuilder has no note() method, so trade proposals cannot include a trade_note via the fluent API even though the raw-XML path supports it.
  • After execute() dispatches a write request, the pending write state is silently cleared; a second execute() on the same instance will perform a GET with no warning.

Confidence Score: 3/5

  • Safe to merge with low risk for read paths; the write path has a couple of unverified Yahoo API contract questions that could cause silent failures against the live API.
  • The core builder logic is well-structured and the unit/snapshot tests are thorough. However, two issues reduce confidence: (1) the is_available: 'true' integration test inconsistency is a known but unresolved FIXME, and (2) the type: 'drop' used for players in a pending-trade payload is unverified against the Yahoo API spec and could cause write operations to fail silently. These are scoped to the new transaction write functionality, not existing read paths.
  • src/request/transaction.ts (dropped-player type in trades) and tests/integration/auth/oauth1.test.ts (is_available value) need attention before the write flows are considered production-ready.

Important Files Changed

Filename Overview
src/request/builder.ts Core request builder extended with transaction write methods (create, edit, cancel), auto XML serialization via fast-xml-parser, and new games collection filter params; one-shot write state semantics could surprise callers.
src/request/transaction.ts New TransactionBuilder fluent class for add/drop and trade payloads; missing trade_note support, and droppedPlayers in trades use type: 'drop' which may be incorrect per the Yahoo API.
tests/integration/auth/oauth1.test.ts Integration tests updated to use new fluent API; contains a FIXME with is_available set to 'true' instead of the correct '1', inconsistent with the isAvailable() helper.
tests/unit/transaction-builder.test.ts New comprehensive unit tests for TransactionBuilder covering add, drop, add/drop with FAAB, trade, validation, and array-copy safety; well-structured and thorough.
tests/unit/request-builder.test.ts Extended with tests for create/edit/cancel write flows, XML serialization snapshots, and dispatch path routing; well-covers the new builder logic.

Sequence Diagram

sequenceDiagram
    participant User
    participant RequestBuilder
    participant TransactionBuilder
    participant XMLBuilder
    participant HttpClient

    Note over User,HttpClient: Fluent Write Flow (create / edit / cancel)

    User->>RequestBuilder: .league(key).transactions()
    User->>RequestBuilder: .create(new TransactionBuilder()...)
    RequestBuilder->>TransactionBuilder: toPayload()
    TransactionBuilder-->>RequestBuilder: Record<string,unknown>
    RequestBuilder->>RequestBuilder: setPendingWriteRequest(POST, payload)

    User->>RequestBuilder: .execute()
    RequestBuilder->>RequestBuilder: state.method === 'POST'
    RequestBuilder->>RequestBuilder: post(state.body, state.options)
    RequestBuilder->>RequestBuilder: normalizeWriteBody(path, data)
    RequestBuilder->>XMLBuilder: build(wrapped payload)
    XMLBuilder-->>RequestBuilder: XML string
    RequestBuilder->>HttpClient: post(path, xml, options)
    HttpClient-->>RequestBuilder: response
    RequestBuilder->>RequestBuilder: clearPendingWriteRequest()
    RequestBuilder-->>User: response

    Note over User,HttpClient: Direct write shortcut (post / put / delete)

    User->>RequestBuilder: .transaction(key).put(xmlOrObject)
    RequestBuilder->>RequestBuilder: normalizeWriteBody(path, data)
    RequestBuilder->>HttpClient: put(path, xml, options)
    HttpClient-->>RequestBuilder: response
    RequestBuilder-->>User: response
Loading

Comments Outside Diff (4)

  1. tests/integration/auth/oauth1.test.ts, line 1314-1316 (link)

    is_available value inconsistency

    The isAvailable() helper method normalizes boolean true to the string '1', but this call uses 'true' (a literal string). If the Yahoo API only accepts '1' as the truthy sentinel (which appears to be the case given the isAvailable() implementation), this test will likely fail or produce incorrect API behaviour when run against the live API.

    The FIXME comment in the code acknowledges uncertainty here, but the inconsistency between this raw param() call and the isAvailable() helper should be resolved before merging:

  2. src/request/transaction.ts, line 1232-1240 (link)

    Potentially incorrect type for dropped players in trade transactions

    Players appended via dropPlayers() are given type: 'drop' in their transaction_data. However, in the Yahoo Fantasy API, dropped players that are part of a trade proposal (sometimes called an "add/drop/trade") are expected to use type: 'pending_trade' with a destination_team_key of null or omitted — not a standalone type: 'drop'. Using type: 'drop' in a pending_trade transaction body may cause the API to reject the request or silently misinterpret the operation.

    If this scenario (droppedPlayers in a trade) is supported by the Yahoo API, consider verifying the exact transaction_data shape it expects and adding a corresponding integration test or example.

  3. src/request/transaction.ts, line 1032-1086 (link)

    Missing trade_note support in TransactionBuilder

    The TransactionBuilder has no way to attach a trade note to a proposed trade. The raw-XML helper buildProposeTradeBody() in examples/request-builder/02-transactions.ts (line 164) demonstrates that notes are a first-class part of trade proposals (<trade_note>). Users relying on the fluent builder for trades would have no equivalent capability, creating an asymmetry between the two approaches.

    Consider adding a note(text: string) method (or similar) that sets a trade_note field on buildTradeTransaction():

    note(text: string): this {
       this.tradeNote = text;
       return this;
    }

    and then including it in the trade payload:

    ...(this.tradeNote ? { trade_note: this.tradeNote } : {}),
  4. src/request/builder.ts, line 947-970 (link)

    Silent state reset after first execute() call

    After a write builder (set up via create(), edit(), or cancel()) is executed, clearPendingWriteRequest() resets state.method back to 'GET'. Any subsequent call to execute() on the same builder instance will silently perform a GET instead of the intended write operation — with no error or warning.

    This "one-shot" behaviour is easy to accidentally trigger, for example when storing a builder reference:

    const txBuilder = req().league(key).transactions().create(tx);
    await txBuilder.execute(); // POST ✓
    await txBuilder.execute(); // Silent GET ✗ — unexpected for callers

    Consider documenting this constraint explicitly, or throwing if execute() is called a second time on a builder whose write operation has already been dispatched.

Last reviewed commit: afd2b1e

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.

2 participants