From c80ce34ab9ba2a386a538300ec8f57a474de51d5 Mon Sep 17 00:00:00 2001 From: joshfatoye0011-bit Date: Sat, 25 Jul 2026 01:35:05 -0700 Subject: [PATCH] feat: add ?order query param to GET /transaction/:hash/effects - Accept optional ?order=asc|desc query parameter on the effects endpoint - Default to 'desc' when the param is omitted - Use existing validateOrder helper from utils/validators for validation - Return 400 ValidationError for invalid order values - Pass validated order through to the Horizon .order() call Fixes pre-existing bugs blocking the test suite: - Add missing qp() helper to validators.js (was undefined, broke validateOrder) - Add missing etagMiddleware import in src/index.js - Remove duplicate makeAccountNotFoundError import in account.js - Remove duplicate rawRecords declaration in account.js - Add isTransactionNotFound handler in errorHandler.js (404 -> NotFound) Tests: all 7 cases pass (default desc, explicit asc, explicit desc, invalid -> 400) Closes #499 --- src/index.js | 1 + src/middleware/errorHandler.js | 13 ++++++ src/routes/account.js | 6 +-- src/routes/transaction.effects.js | 14 ++++-- src/utils/validators.js | 12 ++++++ tests/transaction.effects.test.js | 71 ++++++++++++++++++++++++++++++- 6 files changed, 107 insertions(+), 10 deletions(-) diff --git a/src/index.js b/src/index.js index a91c6c8..9c3562e 100644 --- a/src/index.js +++ b/src/index.js @@ -18,6 +18,7 @@ const errorHandler = require("./middleware/errorHandler"); const requestIdMiddleware = require("./middleware/requestId"); const apiKeyMiddleware = require("./middleware/apiKeyAuth"); const sanitize = require("./middleware/sanitize"); +const etagMiddleware = require("./middleware/etag"); const networkStatusRouter = require("./routes/networkStatus"); const feeEstimateRouter = require("./routes/feeEstimate"); diff --git a/src/middleware/errorHandler.js b/src/middleware/errorHandler.js index d20e17c..bc9288a 100644 --- a/src/middleware/errorHandler.js +++ b/src/middleware/errorHandler.js @@ -121,6 +121,19 @@ function errorHandler(err, req, res, next) { }); } + // TransactionNotFound errors (Horizon 404 on transaction lookup) + if (err.isTransactionNotFound) { + logError(404, req, err.message); + return res.status(404).json({ + success: false, + error: { + type: "NotFound", + message: err.message, + suggestion: "Verify the transaction hash is correct and exists on the network.", + }, + }); + } + // AccountNotFound errors (Horizon 404 on account lookup) if (err.isAccountNotFound) { logError(404, req, err.message); diff --git a/src/routes/account.js b/src/routes/account.js index 662f4b1..987c199 100644 --- a/src/routes/account.js +++ b/src/routes/account.js @@ -2,13 +2,11 @@ const express = require("express"); const router = express.Router(); const { server, NETWORK, fetchAccountCreation } = require("../config/stellar"); const { success, toISOTimestamp } = require("../utils/response"); -const { makeAccountNotFoundError } = require("../utils/errors"); -const cacheService = require("../services/cache"); - const { makeAccountNotFoundError, makeClaimableBalanceNotFoundError, } = require("../utils/errors"); +const cacheService = require("../services/cache"); const { validateAccountId, validateAssetCode } = require("../utils/validators"); const { accountSummaryRateLimiter } = require("../middleware/rateLimiter"); const registerParamValidation = require("../middleware/validateRouteParams"); @@ -342,8 +340,6 @@ router.get("/:id/payments", async (req, res, next) => { const paymentResponse = await query.call(); const rawRecords = paymentResponse.records || []; - const opResponse = await query.call(); - const rawRecords = opResponse.records || []; const issuerCache = new Map(); const tomlCache = new Map(); diff --git a/src/routes/transaction.effects.js b/src/routes/transaction.effects.js index 2a8974a..9ed4348 100644 --- a/src/routes/transaction.effects.js +++ b/src/routes/transaction.effects.js @@ -6,6 +6,7 @@ registerParamValidation(router); const { server, NETWORK } = require("../config/stellar"); const { success, toISOTimestamp } = require("../utils/response"); const { makeAccountNotFoundError } = require("../utils/errors"); +const { validateOrder } = require("../utils/validators"); // Local validator (no Horizon call): 64-character hex string. function validateTransactionHash(hash) { @@ -73,10 +74,10 @@ function normalizeEffect(effect) { if (effect.signer_key !== undefined) base.signerKey = effect.signer_key; - // Memo-less “details” bag (when Horizon provides it) + // Memo-less "details" bag (when Horizon provides it) if (effect.details !== undefined) base.details = effect.details; - // Keep cursor/debug fields but don’t break acceptance criteria + // Keep cursor/debug fields but don't break acceptance criteria if (effect.paging_token !== undefined) { base.pagingToken = effect.paging_token; } @@ -91,12 +92,18 @@ function normalizeEffect(effect) { /** * GET /transaction/:hash/effects * Fetch all ledger effects for a transaction hash. + * + * Query parameters: + * order - Sort direction: "asc" or "desc" (default: "desc") */ router.get("/:hash/effects", async (req, res, next) => { try { const { hash } = req.params; validateTransactionHash(hash); + // Validate ?order query param; defaults to "desc" when omitted. + const order = validateOrder(req.query.order); + // Ensure transaction exists for proper 404s (and to avoid effects returning empty for non-existent hashes) try { await server.transactions().transaction(hash).call(); @@ -120,7 +127,7 @@ router.get("/:hash/effects", async (req, res, next) => { // Acceptance criteria only requires effects + total; we set a conservative limit. const limit = 200; - const response = await server.effects().forTransaction(hash).limit(limit).order("desc").call(); + const response = await server.effects().forTransaction(hash).limit(limit).order(order).call(); const records = response.records || []; const effects = records.map(normalizeEffect); @@ -140,4 +147,3 @@ router.get("/:hash/effects", async (req, res, next) => { }); module.exports = router; - diff --git a/src/utils/validators.js b/src/utils/validators.js index 8c07cc8..b8bc416 100644 --- a/src/utils/validators.js +++ b/src/utils/validators.js @@ -1,5 +1,17 @@ const { StrKey } = require("@stellar/stellar-sdk"); +/** + * Format a query-parameter validation message. + * e.g. qp("order", 'must be "asc" or "desc".') → "Query param 'order' must be \"asc\" or \"desc\"." + * + * @param {string} param - Parameter name + * @param {string} msg - Remainder of the message + * @returns {string} + */ +function qp(param, msg) { + return `Query param '${param}' ${msg}`; +} + /** * Create a structured validation error for invalid input. * diff --git a/tests/transaction.effects.test.js b/tests/transaction.effects.test.js index 44dcd79..e7c1c71 100644 --- a/tests/transaction.effects.test.js +++ b/tests/transaction.effects.test.js @@ -100,5 +100,74 @@ describe("GET /transaction/:hash/effects", () => { expect(server.effects).not.toHaveBeenCalled(); expect(server.transactions).not.toHaveBeenCalled(); }); -}); + // ── ?order query parameter ──────────────────────────────────────────────── + + it("defaults to desc order when ?order is omitted", async () => { + mockTransactionExists(); + const orderMock = jest.fn().mockReturnThis(); + server.effects.mockReturnValue({ + forTransaction: jest.fn().mockReturnThis(), + limit: jest.fn().mockReturnThis(), + order: orderMock, + call: jest.fn().mockResolvedValue({ records: [] }), + }); + + const res = await request(app) + .get(`/transaction/${validHash}/effects`) + .set("x-api-key", "test"); + + expect(res.statusCode).toBe(200); + expect(orderMock).toHaveBeenCalledWith("desc"); + }); + + it("forwards ?order=desc to the Horizon call", async () => { + mockTransactionExists(); + const orderMock = jest.fn().mockReturnThis(); + server.effects.mockReturnValue({ + forTransaction: jest.fn().mockReturnThis(), + limit: jest.fn().mockReturnThis(), + order: orderMock, + call: jest.fn().mockResolvedValue({ records: [] }), + }); + + const res = await request(app) + .get(`/transaction/${validHash}/effects?order=desc`) + .set("x-api-key", "test"); + + expect(res.statusCode).toBe(200); + expect(orderMock).toHaveBeenCalledWith("desc"); + }); + + it("forwards ?order=asc to the Horizon call", async () => { + mockTransactionExists(); + const orderMock = jest.fn().mockReturnThis(); + server.effects.mockReturnValue({ + forTransaction: jest.fn().mockReturnThis(), + limit: jest.fn().mockReturnThis(), + order: orderMock, + call: jest.fn().mockResolvedValue({ records: [] }), + }); + + const res = await request(app) + .get(`/transaction/${validHash}/effects?order=asc`) + .set("x-api-key", "test"); + + expect(res.statusCode).toBe(200); + expect(orderMock).toHaveBeenCalledWith("asc"); + }); + + it("returns 400 for an invalid ?order value", async () => { + mockTransactionExists(); + + const res = await request(app) + .get(`/transaction/${validHash}/effects?order=random`) + .set("x-api-key", "test"); + + expect(res.statusCode).toBe(400); + expect(res.body.success).toBe(false); + expect(res.body.error.type).toBe("ValidationError"); + // Horizon should never be reached when order is invalid + expect(server.effects).not.toHaveBeenCalled(); + }); +});