Skip to content

feat(bid): marks managed bids list with certs requirement#1955

Merged
ygrishajev merged 1 commit intomainfrom
feature/managed-bids
Sep 24, 2025
Merged

feat(bid): marks managed bids list with certs requirement#1955
ygrishajev merged 1 commit intomainfrom
feature/managed-bids

Conversation

@ygrishajev
Copy link
Contributor

@ygrishajev ygrishajev commented Sep 23, 2025

refs #1913

Summary by CodeRabbit

  • New Features

    • Added GET /v1/bids/{dseq} to fetch bids by deployment sequence.
    • Bid responses now include isCertificateRequired.
    • SDK exposes richer Bid details (resources_offer) and new Attribute/DeploymentResource types.
  • Refactor

    • Bids listing simplified to require only dseq; listing logic reorganized behind a dedicated service and router.
  • Tests

    • Bids functional tests rewritten to use data-driven seeders and wallet test helpers.
  • Chores

    • Commit message scopes updated to include “bid”.

@ygrishajev ygrishajev requested a review from a team as a code owner September 23, 2025 16:59
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 23, 2025

Walkthrough

Refactors bid listing: introduces BidService to fetch and annotate bids with certificate requirement, updates BidController to delegate to the service, adds GET /v1/bids/{dseq} route, extends HTTP SDK bid types, updates schemas and repository helper, rewrites functional tests and seeders, and adds "bid" scope to commitlint.

Changes

Cohort / File(s) Summary
Commitlint config
.\commitlintrc.json
Adds "bid" to rules.scope-enum.
Bid controller & service
apps/api/src/bid/controllers/bid/bid.controller.ts, apps/api/src/bid/services/bid/bid.service.ts
Controller now injects BidService and delegates list(dseq). New BidService authenticates, resolves a wallet, fetches bids via BidHttpService, and decorates each bid with isCertificateRequired using ProviderService.
Routes
apps/api/src/bid/routes/bids/bids.router.ts
Introduces/exports bidsRouter, updates existing list route to use query dseq only, and adds new GET /v1/bids/{dseq} OpenAPI route.
Schemas
apps/api/src/bid/http-schemas/bid.schema.ts
Adds isCertificateRequired: z.boolean() to BidResponseSchema; removes userId from ListBidsQuerySchema.
HTTP SDK types
packages/http-sdk/src/bid/bid-http.service.ts
Exports new types Attribute and DeploymentResource_V3; extends exported Bid to include resources_offer: { resources: DeploymentResource_V3; count: number }[].
Billing repository
apps/api/src/billing/repositories/user-wallet/user-wallet.repository.ts
Adds findFirst() method delegating to findOneBy().
Tests & seeders
apps/api/test/functional/bids.spec.ts, apps/api/test/seeders/bid.seeder.ts, apps/api/test/seeders/*, @test/services/wallet-testing.service
Rewrites functional tests to use WalletTestingService, introduces BidSeeder, createAkashAddress, createProvider; parameterizes endpoints and replaces many mocks with data-driven setup and nock responses.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Client
  participant Router as BidsRouter
  participant Controller as BidController
  participant Service as BidService
  participant Auth as AuthService
  participant WalletRepo as UserWalletRepository
  participant BidHTTP as BidHttpService
  participant Provider as ProviderService

  Client->>Router: GET /v1/bids/{dseq} or GET /v1/bids?dseq=...
  Router->>Controller: list(dseq)
  Controller->>Service: list(dseq)

  rect rgba(220,235,255,0.4)
    Service->>Auth: resolve current user/ability
    Auth-->>Service: user/ability
    Service->>WalletRepo: findFirst(...) → wallet.address
  end

  Service->>BidHTTP: listBids({ dseq, owner: wallet.address })
  BidHTTP-->>Service: bids[]

  loop per bid
    Service->>Provider: getByAddress(bid.provider)
    Provider-->>Service: provider | undefined
    note right of Service: compute isCertificateRequired (no provider OR akashVersion < 0.10.0)
  end

  Service-->>Controller: bids with isCertificateRequired
  Controller-->>Router: { data: bids }
  Router-->>Client: 200 JSON
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~40 minutes

Possibly related PRs

Suggested reviewers

  • baktun14

Poem

I hop through routes with nimble cheer,
New bids arrive and seeds appear,
A service finds what certs must show,
Tests planted deep so truths may grow,
Thump-thump — the rabbit signs the PR with a cheer! 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly and accurately describes the primary change — adding a certificate-requirement marker to the managed bids list (new isCertificateRequired field and related service/controller/router/test updates). It is a single, focused sentence that directly reflects the main changes in the diff.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/managed-bids

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

@codecov
Copy link

codecov bot commented Sep 23, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 44.91%. Comparing base (ef104f3) to head (9855c73).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1955      +/-   ##
==========================================
+ Coverage   44.86%   44.91%   +0.05%     
==========================================
  Files         995      996       +1     
  Lines       28058    28080      +22     
  Branches     7338     7325      -13     
==========================================
+ Hits        12587    12611      +24     
- Misses      14265    14344      +79     
+ Partials     1206     1125      -81     
Flag Coverage Δ *Carryforward flag
api 81.38% <100.00%> (+0.07%) ⬆️
deploy-web 23.13% <ø> (ø)
log-collector 75.35% <ø> (ø)
notifications 87.94% <ø> (ø)
provider-console 81.48% <ø> (ø) Carriedforward from ef104f3
provider-proxy 84.47% <ø> (ø) Carriedforward from ef104f3

*This pull request uses carry forward flags. Click here to find out more.

Files with missing lines Coverage Δ
apps/api/src/bid/controllers/bid/bid.controller.ts 100.00% <100.00%> (ø)
apps/api/src/bid/http-schemas/bid.schema.ts 100.00% <ø> (ø)
apps/api/src/bid/routes/bids/bids.router.ts 100.00% <100.00%> (ø)
apps/api/src/bid/services/bid/bid.service.ts 100.00% <100.00%> (ø)
...repositories/user-wallet/user-wallet.repository.ts 91.07% <100.00%> (+0.33%) ⬆️

... and 34 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@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.

Actionable comments posted: 1

🧹 Nitpick comments (5)
packages/http-sdk/src/bid/bid-http.service.ts (1)

99-103: URL-encode query parameters

Avoid edge cases by encoding owner and dseq.

-    const response = this.extractData(await this.get<RestAkashBidListResponse>(`/akash/market/v1beta4/bids/list?filters.owner=${owner}&filters.dseq=${dseq}`));
+    const qOwner = encodeURIComponent(owner);
+    const qDseq = encodeURIComponent(dseq);
+    const url = `/akash/market/v1beta4/bids/list?filters.owner=${qOwner}&filters.dseq=${qDseq}`;
+    const response = this.extractData(await this.get<RestAkashBidListResponse>(url));
apps/api/src/bid/routes/bids/bids.router.ts (1)

36-54: Prefer a dedicated path params schema for clarity

Reusing ListBidsQuerySchema for path params works but is semantically off. Consider a ListBidsPathParamsSchema = z.object({ dseq: z.string() }) for OpenAPI accuracy.

apps/api/test/functional/bids.spec.ts (1)

32-67: Align with test guidelines: move setup to bottom and accept a single param

Per repo guidelines: place setup at the bottom of the root describe, and have it accept a single parameter object (even if unused).

Example shape:

// at the bottom of the root describe
function setup(_: { /* add optional overrides here if needed */ } = {}) {
  // ...
}
apps/api/src/bid/services/bid/bid.service.ts (1)

19-28: Avoid N+1 provider lookups (optional)

If many bids share providers, cache provider lookups by owner to reduce calls.

apps/api/src/billing/repositories/user-wallet/user-wallet.repository.ts (1)

105-107: Add explicit return type to findFirst; findOneBy supports optional query

BaseRepository.findOneBy(query?: Partial): Promise<Output | undefined> — calling it with no args is allowed. Annotate findFirst to avoid widening.

Location: apps/api/src/billing/repositories/user-wallet/user-wallet.repository.ts:105-107

-  async findFirst() {
-    return this.findOneBy();
-  }
+  async findFirst(): Promise<UserWalletOutput | undefined> {
+    return this.findOneBy();
+  }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ef104f3 and 494aed2.

📒 Files selected for processing (9)
  • .commitlintrc.json (1 hunks)
  • apps/api/src/bid/controllers/bid/bid.controller.ts (1 hunks)
  • apps/api/src/bid/http-schemas/bid.schema.ts (1 hunks)
  • apps/api/src/bid/routes/bids/bids.router.ts (2 hunks)
  • apps/api/src/bid/services/bid/bid.service.ts (1 hunks)
  • apps/api/src/billing/repositories/user-wallet/user-wallet.repository.ts (1 hunks)
  • apps/api/test/functional/bids.spec.ts (1 hunks)
  • apps/api/test/seeders/bid.seeder.ts (1 hunks)
  • packages/http-sdk/src/bid/bid-http.service.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

Never use type any or cast to type any. Always define the proper TypeScript types.

Files:

  • apps/api/src/billing/repositories/user-wallet/user-wallet.repository.ts
  • apps/api/test/seeders/bid.seeder.ts
  • apps/api/src/bid/http-schemas/bid.schema.ts
  • packages/http-sdk/src/bid/bid-http.service.ts
  • apps/api/src/bid/routes/bids/bids.router.ts
  • apps/api/test/functional/bids.spec.ts
  • apps/api/src/bid/controllers/bid/bid.controller.ts
  • apps/api/src/bid/services/bid/bid.service.ts
**/*.{js,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code

Files:

  • apps/api/src/billing/repositories/user-wallet/user-wallet.repository.ts
  • apps/api/test/seeders/bid.seeder.ts
  • apps/api/src/bid/http-schemas/bid.schema.ts
  • packages/http-sdk/src/bid/bid-http.service.ts
  • apps/api/src/bid/routes/bids/bids.router.ts
  • apps/api/test/functional/bids.spec.ts
  • apps/api/src/bid/controllers/bid/bid.controller.ts
  • apps/api/src/bid/services/bid/bid.service.ts
**/*.spec.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)

Don't use jest.mock() to mock dependencies in test files. Instead, use jest-mock-extended to create mocks and pass mocks as dependencies to the service under test.

**/*.spec.{ts,tsx}: Use setup function instead of beforeEach in test files
setup function must be at the bottom of the root describe block in test files
setup function creates an object under test and returns it
setup function should accept a single parameter with inline type definition
Don't use shared state in setup function
Don't specify return type of setup function

Files:

  • apps/api/test/functional/bids.spec.ts
🧬 Code graph analysis (5)
apps/api/test/seeders/bid.seeder.ts (1)
packages/http-sdk/src/bid/bid-http.service.ts (1)
  • Bid (42-84)
apps/api/src/bid/routes/bids/bids.router.ts (2)
apps/api/src/core/services/open-api-hono-handler/open-api-hono-handler.ts (1)
  • OpenApiHonoHandler (11-26)
apps/api/src/bid/http-schemas/bid.schema.ts (2)
  • ListBidsQuerySchema (106-108)
  • ListBidsResponseSchema (110-112)
apps/api/test/functional/bids.spec.ts (4)
apps/api/test/services/wallet-testing.service.ts (1)
  • WalletTestingService (15-157)
apps/api/test/seeders/provider.seeder.ts (1)
  • createProvider (39-41)
apps/api/src/utils/constants.ts (1)
  • apiNodeUrl (19-19)
apps/api/test/seeders/bid.seeder.ts (1)
  • BidSeeder (7-97)
apps/api/src/bid/controllers/bid/bid.controller.ts (4)
apps/api/src/bid/services/bid/bid.service.ts (1)
  • singleton (10-37)
apps/api/src/billing/repositories/user-wallet/user-wallet.repository.ts (1)
  • singleton (35-153)
apps/api/src/auth/services/auth.service.ts (1)
  • Protected (39-55)
apps/api/src/bid/http-schemas/bid.schema.ts (1)
  • ListBidsResponse (115-115)
apps/api/src/bid/services/bid/bid.service.ts (3)
apps/api/src/bid/controllers/bid/bid.controller.ts (1)
  • singleton (7-17)
packages/http-sdk/src/bid/bid-http.service.ts (2)
  • BidHttpService (94-104)
  • Bid (42-84)
apps/indexer/drizzle/schema.ts (2)
  • bid (47-69)
  • provider (205-237)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
  • GitHub Check: validate / validate-app
  • GitHub Check: test-build
  • GitHub Check: validate / validate-app
  • GitHub Check: test-build
  • GitHub Check: validate / validate-app
  • GitHub Check: test-build
  • GitHub Check: Validate local packages
🔇 Additional comments (10)
.commitlintrc.json (1)

31-32: LGTM: add "bid" to allowed commit scopes — docs checked

CONTRIBUTING.md only references the Conventional Commits standard (CONTRIBUTING.md:29); no explicit allowed-scope list found, so no documentation updates required.

apps/api/src/bid/http-schemas/bid.schema.ts (2)

102-104: Schema extension LGTM

isCertificateRequired: z.boolean() aligns with the service response.


106-108: Breaking change: userId removed from query schema — confirm external clients

Repo search found no internal callers passing userId to /v1/bids; ListBidsQuerySchema now only has dseq (apps/api/src/bid/http-schemas/bid.schema.ts) and is referenced in apps/api/src/bid/routes/bids/bids.router.ts and apps/api/test/functional/bids.spec.ts. If external clients rely on userId, provide a deprecation window or add an alias to avoid breaking changes.

packages/http-sdk/src/bid/bid-http.service.ts (1)

5-41: Type additions LGTM

Exported types and resources_offer shape match API/test payloads.

apps/api/src/bid/routes/bids/bids.router.ts (3)

8-9: Router export LGTM

Public bidsRouter is fine for composition.


30-34: Query-based list handler LGTM

Validation and delegation look correct.


56-59: Path-based list handler LGTM

Correct use of param validation and shared controller.

apps/api/test/seeders/bid.seeder.ts (1)

15-93: Seeder looks solid

Shape matches the exported Bid type; values are realistic. Using merge enables easy overrides.

apps/api/src/bid/controllers/bid/bid.controller.ts (1)

9-16: Controller simplification LGTM

Delegating to BidService and returning { data } matches the HTTP schema. Protected rule remains in place.

apps/api/test/functional/bids.spec.ts (1)

14-29: Parametrized tests LGTM

Covers both endpoints and asserts cert requirement per provider version.

baktun14
baktun14 previously approved these changes Sep 24, 2025
Copy link
Contributor

@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.

Actionable comments posted: 0

🧹 Nitpick comments (4)
apps/api/test/seeders/bid.seeder.ts (1)

25-28: Avoid precision loss for 18‑decimal amounts

Using JS numbers with 18 fraction digits can lose precision and produce inconsistent strings. Prefer generating a fixed‑precision string.

Apply this diff:

-          amount: faker.number.float({ min: 1, max: 1000, fractionDigits: 18 }).toString()
+          amount: faker.number.float({ min: 1, max: 1000 }).toFixed(18)
apps/api/src/bid/services/bid/bid.service.ts (1)

27-28: Minor: avoid re‑binding within map

Bind once or use an arrow function to reduce minor overhead.

Apply this diff:

-    return await Promise.all(bids.map(this.decorateWithCertRequirement.bind(this)));
+    return await Promise.all(bids.map(b => this.decorateWithCertRequirement(b)));
apps/api/src/bid/routes/bids/bids.router.ts (1)

36-43: Use a dedicated params schema for path params (clarifies OpenAPI)

Reusing ListBidsQuerySchema for params works but can confuse docs. Define ListBidsParamsSchema for path params.

Apply this diff:

-  request: {
-    params: ListBidsQuerySchema
-  },
+  request: {
+    params: ListBidsQuerySchema // consider a dedicated `ListBidsParamsSchema`
+  },

Follow up: consider extracting ListBidsParamsSchema = z.object({ dseq: z.string() }) and using it here.

apps/api/test/functional/bids.spec.ts (1)

32-67: Align with test setup guidelines (place setup at bottom of root describe)

Per guidelines, setup should be at the bottom of the root describe and can accept a single parameter.

Apply this diff to move setup below helper(s) and keep it last in the root describe:

-  async function setup() {
+  // ... keep helper(s) like expectBid above ...
+  async function setup() {
     const walletService = new WalletTestingService(app);
     const user = await walletService.createUserAndWallet();
     const dseq = faker.number.int({ min: 1000000, max: 9999999 }).toString();
     const providers = [
       {
         owner: createAkashAddress(),
         akashVersion: "0.8.0"
       },
       {
         owner: createAkashAddress(),
         akashVersion: "0.10.0"
       }
     ];
 
     await Promise.all(providers.map(async provider => createProvider(provider)));
 
     nock(apiNodeUrl, { allowUnmocked: true })
       .get("/akash/market/v1beta4/bids/list")
       .query({
         "filters.owner": user.wallet.address,
         "filters.dseq": dseq
       })
       .reply(200, {
         bids: [
           BidSeeder.create({ dseq: dseq, owner: user.wallet.address, provider: providers[0].owner }),
           BidSeeder.create({ dseq: dseq, owner: user.wallet.address, provider: providers[1].owner })
         ]
       });
 
     return {
       dseq,
       user,
       providers
     };
   }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 494aed2 and f1d2e0a.

⛔ Files ignored due to path filters (1)
  • apps/api/test/functional/__snapshots__/docs.spec.ts.snap is excluded by !**/*.snap
📒 Files selected for processing (9)
  • .commitlintrc.json (1 hunks)
  • apps/api/src/bid/controllers/bid/bid.controller.ts (1 hunks)
  • apps/api/src/bid/http-schemas/bid.schema.ts (1 hunks)
  • apps/api/src/bid/routes/bids/bids.router.ts (2 hunks)
  • apps/api/src/bid/services/bid/bid.service.ts (1 hunks)
  • apps/api/src/billing/repositories/user-wallet/user-wallet.repository.ts (1 hunks)
  • apps/api/test/functional/bids.spec.ts (1 hunks)
  • apps/api/test/seeders/bid.seeder.ts (1 hunks)
  • packages/http-sdk/src/bid/bid-http.service.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • packages/http-sdk/src/bid/bid-http.service.ts
  • .commitlintrc.json
  • apps/api/src/bid/http-schemas/bid.schema.ts
  • apps/api/src/billing/repositories/user-wallet/user-wallet.repository.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.spec.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)

Don't use jest.mock() to mock dependencies in test files. Instead, use jest-mock-extended to create mocks and pass mocks as dependencies to the service under test.

**/*.spec.{ts,tsx}: Use setup function instead of beforeEach in test files
setup function must be at the bottom of the root describe block in test files
setup function creates an object under test and returns it
setup function should accept a single parameter with inline type definition
Don't use shared state in setup function
Don't specify return type of setup function

Files:

  • apps/api/test/functional/bids.spec.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

Never use type any or cast to type any. Always define the proper TypeScript types.

Files:

  • apps/api/test/functional/bids.spec.ts
  • apps/api/test/seeders/bid.seeder.ts
  • apps/api/src/bid/services/bid/bid.service.ts
  • apps/api/src/bid/routes/bids/bids.router.ts
  • apps/api/src/bid/controllers/bid/bid.controller.ts
**/*.{js,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code

Files:

  • apps/api/test/functional/bids.spec.ts
  • apps/api/test/seeders/bid.seeder.ts
  • apps/api/src/bid/services/bid/bid.service.ts
  • apps/api/src/bid/routes/bids/bids.router.ts
  • apps/api/src/bid/controllers/bid/bid.controller.ts
🧬 Code graph analysis (5)
apps/api/test/functional/bids.spec.ts (4)
apps/api/test/services/wallet-testing.service.ts (1)
  • WalletTestingService (15-157)
apps/api/test/seeders/provider.seeder.ts (1)
  • createProvider (39-41)
apps/api/src/utils/constants.ts (1)
  • apiNodeUrl (19-19)
apps/api/test/seeders/bid.seeder.ts (1)
  • BidSeeder (7-97)
apps/api/test/seeders/bid.seeder.ts (1)
packages/http-sdk/src/bid/bid-http.service.ts (1)
  • Bid (42-84)
apps/api/src/bid/services/bid/bid.service.ts (3)
apps/api/src/bid/controllers/bid/bid.controller.ts (1)
  • singleton (7-17)
packages/http-sdk/src/bid/bid-http.service.ts (2)
  • BidHttpService (94-104)
  • Bid (42-84)
apps/indexer/drizzle/schema.ts (2)
  • bid (47-69)
  • provider (205-237)
apps/api/src/bid/routes/bids/bids.router.ts (2)
apps/api/src/core/services/open-api-hono-handler/open-api-hono-handler.ts (1)
  • OpenApiHonoHandler (11-26)
apps/api/src/bid/http-schemas/bid.schema.ts (2)
  • ListBidsQuerySchema (106-108)
  • ListBidsResponseSchema (110-112)
apps/api/src/bid/controllers/bid/bid.controller.ts (4)
apps/api/src/bid/services/bid/bid.service.ts (1)
  • singleton (10-37)
apps/api/src/billing/repositories/user-wallet/user-wallet.repository.ts (1)
  • singleton (35-153)
apps/api/src/auth/services/auth.service.ts (1)
  • Protected (39-55)
apps/api/src/bid/http-schemas/bid.schema.ts (1)
  • ListBidsResponse (115-115)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
  • GitHub Check: validate / validate-app
  • GitHub Check: test-build
  • GitHub Check: validate / validate-app
  • GitHub Check: test-build
  • GitHub Check: validate / validate-app
  • GitHub Check: test-build
  • GitHub Check: Validate local packages
🔇 Additional comments (8)
apps/api/src/bid/services/bid/bid.service.ts (2)

22-23: Confirm wallet selection semantics

Using findFirst() returns the first accessible wallet. If users can have multiple wallets, this may be nondeterministic.

Do we guarantee a single wallet per user, or should we select by a specific criterion (e.g., default wallet)?


30-35: Coerce provider version before semver compare to handle common prefixes (e.g., "v0.10.0")

Current check treats non‑semver strings as “require cert,” which will incorrectly flag providers reporting versions like "v0.10.0". Coerce first, then compare.

Apply this diff:

   private async decorateWithCertRequirement<T extends Bid>(bid: T): Promise<T & { isCertificateRequired: boolean }> {
     const provider = await this.providerService.getProvider(bid.bid.bid_id.provider);
-    return {
-      ...bid,
-      isCertificateRequired: !provider || !semver.valid(provider.akashVersion) || semver.lt(provider.akashVersion, "0.10.0")
-    };
+    const coerced = provider?.akashVersion ? semver.coerce(provider.akashVersion) : null;
+    const isCertificateRequired = !coerced || semver.lt(coerced, "0.10.0");
+    return { ...bid, isCertificateRequired };
   }
apps/api/src/bid/routes/bids/bids.router.ts (2)

31-34: LGTM: query‑based list route wiring

Parsing dseq from query and delegating to controller looks correct.


57-58: LGTM: path param validation

Using c.req.valid("param") to extract dseq aligns with Hono validation.

apps/api/src/bid/controllers/bid/bid.controller.ts (1)

11-16: Controller delegation looks good

Auth guard + delegation to BidService and typed response shape are clean.

apps/api/test/functional/bids.spec.ts (3)

14-28: End‑to‑end assertions cover both routes and cert flag states

Good coverage across /v1/bids/:dseq and /v1/bids?dseq=:dseq with realistic fixtures.


19-20: Header case: verify Authorization is case‑insensitive in the stack

Some frameworks normalize header names; others don’t. Ensure Authorization is read case‑insensitively.

If needed, change to "Authorization" to match convention.


49-55: LGTM: precise API mock for bid list

Nock target and query match the HTTP SDK call signature.

Copy link
Contributor

@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.

Actionable comments posted: 1

🧹 Nitpick comments (6)
apps/api/test/seeders/bid.seeder.ts (3)

95-96: Avoid passing undefined to lodash.merge; mutate and return defaultBid

Prevents odd typing and is clearer. Behavior unchanged.

-    return merge(defaultBid, overrides);
+    if (overrides) {
+      merge(defaultBid, overrides);
+    }
+    return defaultBid;

78-91: Use integer strings for escrow Coin amounts (on-chain Coins are integers)

Balance/transferred/funds should be integer-encoded strings. Price can stay decimal. Aligning here reduces surprises in downstream consumers.

-        balance: {
-          denom: "uakt",
-          amount: faker.number.float({ min: 100000, max: 1000000 }).toString()
-        },
+        balance: {
+          denom: "uakt",
+          amount: faker.number.int({ min: 100000, max: 1000000 }).toString()
+        },
         transferred: {
           denom: "uakt",
-          amount: "0.000000000000000000"
+          amount: "0"
         },
         settled_at: faker.string.numeric(8),
         depositor: provider,
         funds: {
           denom: "uakt",
-          amount: "0.000000000000000000"
+          amount: "0"
         }

If the upstream API truly returns DecCoins for these fields, disregard and keep decimals. Please confirm against the chain’s REST schema used here.


30-69: Array overrides will be merged index-wise; consider replace semantics

lodash.merge merges arrays by index. If you expect overrides.resources_offer (or storage/endpoints) to fully replace defaults, switch to mergeWith and replace arrays.

Example:

import { mergeWith } from "lodash";

function mergeReplaceArrays<T>(target: T, source: Partial<T> | undefined): T {
  if (!source) return target;
  return mergeWith(target, source, (objValue, srcValue) => {
    if (Array.isArray(objValue) && Array.isArray(srcValue)) {
      return srcValue;
    }
  });
}

Then use mergeReplaceArrays(defaultBid, overrides).

apps/api/src/bid/routes/bids/bids.router.ts (1)

36-44: Use a dedicated params schema for path variables

Reusing the query schema for params works but is semantically confusing in OpenAPI. Prefer a distinct ListBidsParamsSchema to document path param location.

-  request: {
-    params: ListBidsQuerySchema
-  },
+  request: {
+    params: ListBidsParamsSchema
+  },

Add in apps/api/src/bid/http-schemas/bid.schema.ts:

export const ListBidsParamsSchema = z.object({ dseq: z.string() });
apps/api/test/functional/bids.spec.ts (2)

14-29: Assert HTTP status before parsing JSON

Improves test diagnostics and ensures we only parse on success.

-      const response = await app.request(path.replace(":dseq", dseq), {
+      const response = await app.request(path.replace(":dseq", dseq), {
         method: "GET",
         headers: new Headers({ "Content-Type": "application/json", authorization: `Bearer ${user.token}` })
       });
+      expect(response.status).toBe(200);

48-54: Avoid allowUnmocked: true to keep tests deterministic

Blocking unexpected network calls reduces flakiness.

-    nock(apiNodeUrl, { allowUnmocked: true })
+    nock(apiNodeUrl)
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f1d2e0a and 9855c73.

⛔ Files ignored due to path filters (1)
  • apps/api/test/functional/__snapshots__/docs.spec.ts.snap is excluded by !**/*.snap
📒 Files selected for processing (9)
  • .commitlintrc.json (1 hunks)
  • apps/api/src/bid/controllers/bid/bid.controller.ts (1 hunks)
  • apps/api/src/bid/http-schemas/bid.schema.ts (1 hunks)
  • apps/api/src/bid/routes/bids/bids.router.ts (2 hunks)
  • apps/api/src/bid/services/bid/bid.service.ts (1 hunks)
  • apps/api/src/billing/repositories/user-wallet/user-wallet.repository.ts (1 hunks)
  • apps/api/test/functional/bids.spec.ts (1 hunks)
  • apps/api/test/seeders/bid.seeder.ts (1 hunks)
  • packages/http-sdk/src/bid/bid-http.service.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
  • apps/api/src/bid/http-schemas/bid.schema.ts
  • .commitlintrc.json
  • apps/api/src/bid/services/bid/bid.service.ts
  • apps/api/src/bid/controllers/bid/bid.controller.ts
  • packages/http-sdk/src/bid/bid-http.service.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

Never use type any or cast to type any. Always define the proper TypeScript types.

Files:

  • apps/api/test/seeders/bid.seeder.ts
  • apps/api/src/billing/repositories/user-wallet/user-wallet.repository.ts
  • apps/api/test/functional/bids.spec.ts
  • apps/api/src/bid/routes/bids/bids.router.ts
**/*.{js,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code

Files:

  • apps/api/test/seeders/bid.seeder.ts
  • apps/api/src/billing/repositories/user-wallet/user-wallet.repository.ts
  • apps/api/test/functional/bids.spec.ts
  • apps/api/src/bid/routes/bids/bids.router.ts
**/*.spec.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)

Don't use jest.mock() to mock dependencies in test files. Instead, use jest-mock-extended to create mocks and pass mocks as dependencies to the service under test.

**/*.spec.{ts,tsx}: Use setup function instead of beforeEach in test files
setup function must be at the bottom of the root describe block in test files
setup function creates an object under test and returns it
setup function should accept a single parameter with inline type definition
Don't use shared state in setup function
Don't specify return type of setup function

Files:

  • apps/api/test/functional/bids.spec.ts
🧬 Code graph analysis (3)
apps/api/test/seeders/bid.seeder.ts (1)
packages/http-sdk/src/bid/bid-http.service.ts (1)
  • Bid (42-84)
apps/api/test/functional/bids.spec.ts (4)
apps/api/test/services/wallet-testing.service.ts (1)
  • WalletTestingService (15-157)
apps/api/test/seeders/provider.seeder.ts (1)
  • createProvider (39-41)
apps/api/src/utils/constants.ts (1)
  • apiNodeUrl (19-19)
apps/api/test/seeders/bid.seeder.ts (1)
  • BidSeeder (7-97)
apps/api/src/bid/routes/bids/bids.router.ts (2)
apps/api/src/core/services/open-api-hono-handler/open-api-hono-handler.ts (1)
  • OpenApiHonoHandler (11-26)
apps/api/src/bid/http-schemas/bid.schema.ts (2)
  • ListBidsQuerySchema (106-108)
  • ListBidsResponseSchema (110-112)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
  • GitHub Check: validate / validate-app
  • GitHub Check: test-build
  • GitHub Check: validate / validate-app
  • GitHub Check: test-build
  • GitHub Check: validate / validate-app
  • GitHub Check: test-build
  • GitHub Check: Validate local packages
🔇 Additional comments (6)
apps/api/src/billing/repositories/user-wallet/user-wallet.repository.ts (1)

105-107: Add explicit return type to UserWalletRepository.findFirst — usage verified

Add the explicit return type Promise<UserWalletOutput | undefined> to avoid widening; verified the only repository.findFirst call is ability-scoped (apps/api/src/bid/services/bid/bid.service.ts:22). Other .findFirst occurrences are direct query-cursor calls or tests.

-  async findFirst() {
-    return this.findOneBy();
-  }
+  async findFirst(): Promise<UserWalletOutput | undefined> {
+    return this.findOneBy();
+  }
apps/api/src/bid/routes/bids/bids.router.ts (3)

8-8: Good: single public router export

Consistent named export simplifies consumers.


30-34: Query-list route looks good

Validation, controller call, and JSON 200 response are correct.


56-59: LGTM — path-param handler; named export verified

Correct use of c.req.valid("param") and controller delegation. bids.router exports a named bidsRouter (no export default found); callers import it as a named export in apps/api/src/rest-app.ts and apps/api/src/routes/deployment/index.ts.

apps/api/test/functional/bids.spec.ts (2)

7-10: Imports/readiness look good

Seeders and testing service usage is aligned.


68-78: Helper expectation reads well

Clear target checks and keeps assertions DRY.

@ygrishajev ygrishajev merged commit 75fed6f into main Sep 24, 2025
64 checks passed
@ygrishajev ygrishajev deleted the feature/managed-bids branch September 24, 2025 09:26
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

Comments