Skip to content

refactor: move provider-active-leases-graph-data to modules#1301

Merged
baktun14 merged 1 commit intoakash-network:mainfrom
jzsfkzm:refactor/1272-provider-active-leases-graph-data
May 12, 2025
Merged

refactor: move provider-active-leases-graph-data to modules#1301
baktun14 merged 1 commit intoakash-network:mainfrom
jzsfkzm:refactor/1272-provider-active-leases-graph-data

Conversation

@jzsfkzm
Copy link
Contributor

@jzsfkzm jzsfkzm commented May 7, 2025

refs #1272

Summary by CodeRabbit

  • New Features

    • Added a new API endpoint to retrieve active leases graph data for a specific provider: /v1/providers/{providerAddress}/active-leases-graph-data.
    • Introduced comprehensive validation for provider addresses using a standardized schema.
  • Bug Fixes

    • Updated internal and external references to the active leases graph data endpoint to use the new nested URL structure.
  • Tests

    • Added functional tests for the new provider active leases graph data endpoint.
    • Improved and refactored test data seeders for deployments, leases, providers, and related entities.
  • Refactor

    • Simplified and standardized schema validation and test data generation for improved maintainability and consistency.
  • Chores

    • Removed deprecated endpoints and related code for the old provider active leases graph data route.

@jzsfkzm jzsfkzm requested a review from a team as a code owner May 7, 2025 19:11
@codecov
Copy link

codecov bot commented May 7, 2025

Codecov Report

Attention: Patch coverage is 90.90909% with 3 lines in your changes missing coverage. Please review.

Project coverage is 32.34%. Comparing base (9d64416) to head (f4094a0).
Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
.../services/provider-stats/provider-stats.service.ts 91.66% 1 Missing ⚠️
apps/api/src/routers/legacyRouter.ts 0.00% 1 Missing ⚠️
apps/deploy-web/src/utils/apiUtils.ts 0.00% 1 Missing ⚠️

❌ Your patch status has failed because the patch coverage (0.00%) is below the target coverage (30.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1301      +/-   ##
==========================================
+ Coverage   32.26%   32.34%   +0.07%     
==========================================
  Files         754      755       +1     
  Lines       18832    18839       +7     
  Branches     3553     3546       -7     
==========================================
+ Hits         6077     6094      +17     
+ Misses      12153    11993     -160     
- Partials      602      752     +150     
Flag Coverage Δ *Carryforward flag
api 61.74% <93.75%> (+0.22%) ⬆️
deploy-web 12.53% <0.00%> (ø)
notifications 87.07% <ø> (ø) Carriedforward from 9d64416
provider-proxy 80.09% <ø> (ø) Carriedforward from 9d64416

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

Files with missing lines Coverage Δ
...i/src/deployment/http-schemas/deployment.schema.ts 100.00% <100.00%> (ø)
...ovider/controllers/provider/provider.controller.ts 80.95% <100.00%> (+4.48%) ⬆️
...s/api/src/provider/http-schemas/provider.schema.ts 100.00% <100.00%> (ø)
.../src/provider/routes/providers/providers.router.ts 100.00% <100.00%> (ø)
apps/api/src/services/db/statsService.ts 16.66% <ø> (+0.45%) ⬆️
apps/api/src/utils/schema.ts 100.00% <100.00%> (ø)
.../services/provider-stats/provider-stats.service.ts 91.66% <91.66%> (ø)
apps/api/src/routers/legacyRouter.ts 35.18% <0.00%> (ø)
apps/deploy-web/src/utils/apiUtils.ts 22.44% <0.00%> (ø)

... and 49 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jzsfkzm jzsfkzm force-pushed the refactor/1272-provider-active-leases-graph-data branch from 2dc093f to f4094a0 Compare May 8, 2025 18:23
@coderabbitai
Copy link
Contributor

coderabbitai bot commented May 8, 2025

Walkthrough

This update introduces a new /v1/providers/{providerAddress}/active-leases-graph-data API endpoint for retrieving provider active leases graph data, refactors related schema validation and service logic, and restructures test seeders for improved type safety and database integration. The legacy flat endpoint and its route are removed, and all relevant tests and utilities are updated accordingly.

Changes

File(s) Change Summary
apps/api/src/deployment/http-schemas/deployment.schema.ts, apps/api/src/utils/schema.ts Replaced manual address validation with a reusable AkashAddressSchema for Bech32 address validation.
apps/api/src/provider/controllers/provider/provider.controller.ts Injected ProviderStatsService and added a new method to fetch provider active leases graph data.
apps/api/src/provider/http-schemas/provider.schema.ts Added schemas and types for provider active leases graph data parameters and responses.
apps/api/src/provider/routes/providers/providers.router.ts Added a new GET route /v1/providers/{providerAddress}/active-leases-graph-data with validation and handler.
apps/api/src/provider/services/provider-stats/provider-stats.service.ts Introduced ProviderStatsService with a method to query and structure active leases graph data.
apps/api/src/routers/legacyRouter.ts, apps/deploy-web/src/utils/apiUtils.ts Updated legacy redirect and frontend utility to use the new nested endpoint path.
apps/api/src/routes/v1/index.ts, apps/api/src/routes/v1/providerActiveLeasesGraphData.ts Removed the old flat route and its export, deleting the corresponding route file.
apps/api/src/services/db/statsService.ts Removed the old database function for fetching provider active leases graph data.
apps/api/test/functional/deployments.spec.ts Updated lease seeder usage to LeaseApiResponseSeeder for test data.
apps/api/test/functional/providers.spec.ts Added tests for the new provider active leases graph data endpoint and updated seeder usage.
apps/api/test/seeders/day.seeder.ts, apps/api/test/seeders/deployment-group.seeder.ts, apps/api/test/seeders/deployment.seeder.ts, apps/api/test/seeders/lease.seeder.ts, apps/api/test/seeders/provider.seeder.ts Refactored and expanded seeder classes for type safety, direct database creation, and improved data generation.
apps/api/test/seeders/lease-api-response.seeder.ts Added a new seeder for generating lease API response mock data.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Router
    participant Controller
    participant StatsService
    participant DB

    Client->>Router: GET /v1/providers/{providerAddress}/active-leases-graph-data
    Router->>Controller: getProviderActiveLeasesGraphData(providerAddress)
    Controller->>StatsService: getProviderActiveLeasesGraphData(providerAddress)
    StatsService->>DB: Query active leases per day for provider
    DB-->>StatsService: Lease counts per day
    StatsService-->>Controller: Structured graph data
    Controller-->>Router: Graph data response
    Router-->>Client: JSON response (graph data)
Loading

Poem

🐇
A hop and a skip, a new endpoint appears,
For providers’ lease graphs, let’s all give three cheers!
The schemas are tidy, the seeders refined,
With tests and controllers all neatly aligned.
From flat to nested, the routes now delight—
Data for leases, just one GET and you’re right!
—Rabbit, the code poet

Tip

⚡️ Faster reviews with caching
  • CodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure Review - Disable Cache at either the organization or repository level. If you prefer to disable all data retention across your organization, simply turn off the Data Retention setting under your Organization Settings.

Enjoy the performance boost—your workflow just got faster.

✨ Finishing Touches
  • 📝 Generate Docstrings

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

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 (9)
apps/deploy-web/src/utils/apiUtils.ts (1)

106-106: Updated URL to match new API endpoint structure.

The URL path has been correctly updated to align with the backend changes, maintaining frontend-backend consistency.

Consider addressing the static analysis hint by using the class name instead of this in this static context:

-    return `${this.baseApiUrl}/v1/providers/${providerAddress}/active-leases-graph-data`;
+    return `${ApiUrlService.baseApiUrl}/v1/providers/${providerAddress}/active-leases-graph-data`;
🧰 Tools
🪛 Biome (1.9.4)

[error] 106-106: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)

apps/api/test/seeders/deployment-group.seeder.ts (1)

5-20: Consider using plain functions instead of a static-only class

The class only contains static methods, which is generally considered an anti-pattern. Using plain functions would be more idiomatic and avoid potential confusion with the this keyword in static methods.

-export class DeploymentGroupSeeder {
-  static create(overrides: Partial<CreationAttributes<DeploymentGroup>> = {}): CreationAttributes<DeploymentGroup> {
+export function createDeploymentGroup(overrides: Partial<CreationAttributes<DeploymentGroup>> = {}): CreationAttributes<DeploymentGroup> {
     return {
       id: overrides.id || faker.string.uuid(),
       deploymentId: overrides.deploymentId || faker.string.uuid(),
       owner: overrides.owner || faker.string.alphanumeric(44),
       dseq: overrides.dseq || faker.string.numeric(20),
       gseq: overrides.gseq || faker.number.int({ min: 1, max: 100 })
     };
   }
 
-  static async createInDatabase(overrides: Partial<CreationAttributes<DeploymentGroup>> = {}): Promise<DeploymentGroup> {
-    const seed = this.create(overrides);
+export async function createDeploymentGroupInDatabase(overrides: Partial<CreationAttributes<DeploymentGroup>> = {}): Promise<DeploymentGroup> {
+    const seed = createDeploymentGroup(overrides);
     return await DeploymentGroup.create(seed);
   }
-}
🧰 Tools
🪛 Biome (1.9.4)

[error] 5-20: Avoid classes that contain only static members.

Prefer using simple functions instead of classes with only static members.

(lint/complexity/noStaticOnlyClass)


[error] 17-17: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)

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

21-24: Consider using class name instead of this in static method

In the static createInDatabase method, you're using this.create(). While this works, it's generally recommended to use the class name directly in static methods for clarity.

static async createInDatabase(overrides: Partial<CreationAttributes<Deployment>> = {}): Promise<Deployment> {
-  const seed = this.create(overrides);
+  const seed = DeploymentSeeder.create(overrides);
  return await Deployment.create(seed);
}
🧰 Tools
🪛 Biome (1.9.4)

[error] 22-22: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)

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

38-41: Same issue with this in static method

Similar to the DeploymentSeeder, consider using the class name instead of this in the static method.

static async createInDatabase(overrides: Partial<CreationAttributes<Provider>> = {}): Promise<Provider> {
-  const seed = this.create(overrides);
+  const seed = ProviderSeeder.create(overrides);
  return await Provider.create(seed);
}
🧰 Tools
🪛 Biome (1.9.4)

[error] 39-39: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)

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

18-21: Fix static reference in static method

There's a potential issue with using this in a static context which can be confusing.

  static async createInDatabase(overrides: Partial<CreationAttributes<Day>> = {}): Promise<Day> {
-    const seed = this.create(overrides);
+    const seed = DaySeeder.create(overrides);
    return await Day.create(seed);
  }
🧰 Tools
🪛 Biome (1.9.4)

[error] 19-19: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)

apps/api/test/seeders/lease-api-response.seeder.ts (2)

122-124: Fix static reference in createMany method

Using this in a static context can be confusing and should be avoided.

  static createMany(count: number, input: LeaseInput = {}): LeaseOutput[] {
-    return Array.from({ length: count }, () => this.create(input));
+    return Array.from({ length: count }, () => LeaseApiResponseSeeder.create(input));
  }
🧰 Tools
🪛 Biome (1.9.4)

[error] 123-123: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)


61-125: Consider using simple functions instead of a static class

The static analysis tool suggests avoiding classes that contain only static members. For better maintainability and following functional programming principles, consider refactoring to simple functions.

-export class LeaseApiResponseSeeder {
-  static create(input: LeaseInput = {}): LeaseOutput {
+export const createLeaseApiResponse = (input: LeaseInput = {}): LeaseOutput => {
     // existing implementation
   }

-  static createMany(count: number, input: LeaseInput = {}): LeaseOutput[] {
-    return Array.from({ length: count }, () => this.create(input));
+export const createManyLeaseApiResponses = (count: number, input: LeaseInput = {}): LeaseOutput[] => {
+    return Array.from({ length: count }, () => createLeaseApiResponse(input));
   }
-}
🧰 Tools
🪛 Biome (1.9.4)

[error] 61-125: Avoid classes that contain only static members.

Prefer using simple functions instead of classes with only static members.

(lint/complexity/noStaticOnlyClass)


[error] 123-123: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)

apps/api/src/provider/services/provider-stats/provider-stats.service.ts (1)

11-22: Consider optimizing SQL query and adding index hint

The query could be optimized for better performance:

  `SELECT "date" AS date, COUNT(l."id") AS count
    FROM "day" d
    LEFT JOIN "lease" l
-        ON l."providerAddress" = :providerAddress
+        ON l."providerAddress" = :providerAddress  /* indexed */
        AND l."createdHeight" <= d."lastBlockHeightYet"
        AND COALESCE(l."closedHeight", l."predictedClosedHeight") > d."lastBlockHeightYet"
        AND (l."predictedClosedHeight" IS NULL OR l."predictedClosedHeight" > d."lastBlockHeightYet")
    INNER JOIN "provider" p
-        ON p."owner" = :providerAddress
+        ON p."owner" = :providerAddress  /* indexed */
    WHERE d."lastBlockHeightYet" >= p."createdHeight"
    GROUP BY "date"
    ORDER BY "date" ASC`

Consider adding a comment indicating that you expect these columns to be indexed for optimal performance. This query might benefit from indexes on lease.providerAddress and provider.owner.

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

30-33: Address static analysis warning about 'this' in static context.

The new createInDatabase method is a valuable addition for persisting seed data directly to the database. However, there's a minor issue with using this in a static context.

static async createInDatabase(overrides: Partial<CreationAttributes<Lease>> = {}): Promise<Lease> {
-  const seed = this.create(overrides);
+  const seed = LeaseSeeder.create(overrides);
  return await Lease.create(seed);
}
🧰 Tools
🪛 Biome (1.9.4)

[error] 31-31: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9d64416 and f4094a0.

📒 Files selected for processing (19)
  • apps/api/src/deployment/http-schemas/deployment.schema.ts (2 hunks)
  • apps/api/src/provider/controllers/provider/provider.controller.ts (2 hunks)
  • apps/api/src/provider/http-schemas/provider.schema.ts (2 hunks)
  • apps/api/src/provider/routes/providers/providers.router.ts (3 hunks)
  • apps/api/src/provider/services/provider-stats/provider-stats.service.ts (1 hunks)
  • apps/api/src/routers/legacyRouter.ts (1 hunks)
  • apps/api/src/routes/v1/index.ts (0 hunks)
  • apps/api/src/routes/v1/providerActiveLeasesGraphData.ts (0 hunks)
  • apps/api/src/services/db/statsService.ts (1 hunks)
  • apps/api/src/utils/schema.ts (1 hunks)
  • apps/api/test/functional/deployments.spec.ts (3 hunks)
  • apps/api/test/functional/providers.spec.ts (2 hunks)
  • apps/api/test/seeders/day.seeder.ts (1 hunks)
  • apps/api/test/seeders/deployment-group.seeder.ts (1 hunks)
  • apps/api/test/seeders/deployment.seeder.ts (1 hunks)
  • apps/api/test/seeders/lease-api-response.seeder.ts (1 hunks)
  • apps/api/test/seeders/lease.seeder.ts (1 hunks)
  • apps/api/test/seeders/provider.seeder.ts (1 hunks)
  • apps/deploy-web/src/utils/apiUtils.ts (1 hunks)
💤 Files with no reviewable changes (2)
  • apps/api/src/routes/v1/index.ts
  • apps/api/src/routes/v1/providerActiveLeasesGraphData.ts
🧰 Additional context used
🧬 Code Graph Analysis (8)
apps/api/test/functional/deployments.spec.ts (1)
apps/api/test/seeders/lease-api-response.seeder.ts (1)
  • LeaseApiResponseSeeder (61-125)
apps/api/src/deployment/http-schemas/deployment.schema.ts (2)
apps/api/src/utils/schema.ts (1)
  • AkashAddressSchema (5-5)
apps/api/src/utils/constants.ts (1)
  • openApiExampleAddress (14-14)
apps/api/test/functional/providers.spec.ts (5)
apps/api/test/seeders/provider.seeder.ts (1)
  • ProviderSeeder (5-42)
apps/api/test/seeders/day.seeder.ts (1)
  • DaySeeder (5-22)
apps/api/test/seeders/deployment.seeder.ts (1)
  • DeploymentSeeder (5-25)
apps/api/test/seeders/deployment-group.seeder.ts (1)
  • DeploymentGroupSeeder (5-20)
apps/api/test/seeders/lease.seeder.ts (1)
  • LeaseSeeder (5-34)
apps/api/src/provider/services/provider-stats/provider-stats.service.ts (4)
apps/api/src/provider/controllers/provider/provider.controller.ts (1)
  • singleton (11-40)
apps/api/src/db/dbConnection.ts (1)
  • chainDb (20-30)
apps/api/src/types/graph.ts (1)
  • ProviderActiveLeasesStats (12-15)
apps/indexer/drizzle/schema.ts (1)
  • day (92-108)
apps/api/src/provider/routes/providers/providers.router.ts (1)
apps/api/src/provider/http-schemas/provider.schema.ts (2)
  • ProviderActiveLeasesGraphDataParamsSchema (96-98)
  • ProviderActiveLeasesGraphDataResponseSchema (100-115)
apps/api/test/seeders/deployment.seeder.ts (2)
packages/database/dbSchemas/akash/index.ts (1)
  • Deployment (1-1)
apps/indexer/src/indexers/akashStatsIndexer.ts (1)
  • seed (889-891)
apps/api/test/seeders/lease-api-response.seeder.ts (2)
apps/api/test/seeders/akash-address.seeder.ts (1)
  • AkashAddressSeeder (3-7)
apps/api/test/seeders/denom.seeder.ts (1)
  • DenomSeeder (4-8)
apps/api/test/seeders/lease.seeder.ts (2)
packages/database/dbSchemas/akash/index.ts (1)
  • Lease (11-11)
apps/indexer/src/indexers/akashStatsIndexer.ts (1)
  • seed (889-891)
🪛 Biome (1.9.4)
apps/deploy-web/src/utils/apiUtils.ts

[error] 106-106: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)

apps/api/test/seeders/deployment-group.seeder.ts

[error] 5-20: Avoid classes that contain only static members.

Prefer using simple functions instead of classes with only static members.

(lint/complexity/noStaticOnlyClass)


[error] 17-17: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)

apps/api/test/seeders/day.seeder.ts

[error] 19-19: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)

apps/api/test/seeders/deployment.seeder.ts

[error] 5-25: Avoid classes that contain only static members.

Prefer using simple functions instead of classes with only static members.

(lint/complexity/noStaticOnlyClass)


[error] 22-22: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)

apps/api/test/seeders/provider.seeder.ts

[error] 39-39: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)

apps/api/test/seeders/lease-api-response.seeder.ts

[error] 61-125: Avoid classes that contain only static members.

Prefer using simple functions instead of classes with only static members.

(lint/complexity/noStaticOnlyClass)


[error] 123-123: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)

apps/api/test/seeders/lease.seeder.ts

[error] 31-31: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)

⏰ Context from checks skipped due to timeout of 90000ms (4)
  • GitHub Check: validate-deploy-web
  • GitHub Check: test-deploy-web-build
  • GitHub Check: validate-api
  • GitHub Check: test-api-build
🔇 Additional comments (36)
apps/api/src/utils/schema.ts (1)

1-5: Clean and reusable address validation schema!

The introduction of AkashAddressSchema centralizes Bech32 address validation with the "akash" prefix across the API. This follows good practices by:

  1. Creating a reusable schema component
  2. Providing a clear error message
  3. Abstracting the validation logic

This will help maintain consistency in address validation throughout the application.

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

24-24: Updated import to use the new seeder class.

The import statement has been updated to use LeaseApiResponseSeeder instead of the previous LeaseSeeder, aligning with the broader refactoring of test seeders.


148-152: Updated usage to LeaseApiResponseSeeder.

The call to LeaseSeeder.createMany() has been replaced with LeaseApiResponseSeeder.createMany(), which provides better type safety and a more detailed lease response structure as shown in the seeder implementation.


176-180: Updated usage to LeaseApiResponseSeeder.

Similar to the previous instance, this call has been updated to use the new seeder class, maintaining consistency throughout the test suite.

apps/api/src/routers/legacyRouter.ts (1)

183-183: Updated redirect to match new RESTful endpoint structure.

The legacy route now redirects to the new nested endpoint /v1/providers/${address}/active-leases-graph-data which follows RESTful API principles by placing the resource under the provider's context.

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

5-5: Good refactoring to use centralized address validation

The change to import AkashAddressSchema instead of isValidBech32Address helps centralize address validation logic and follows good DRY principles.


135-138: Schema refactoring improves consistency

Using the centralized AkashAddressSchema while preserving the OpenAPI metadata improves code maintainability and ensures consistent validation across the codebase.

apps/api/src/provider/controllers/provider/provider.controller.ts (3)

8-8: Good addition of ProviderStatsService import

Correctly importing the new service that will provide the active leases graph data functionality.


16-17: Good use of dependency injection

Adding the ProviderStatsService as a constructor dependency follows proper dependency injection patterns.


37-39: Clean implementation of the new endpoint method

The method is concise and correctly delegates to the injected service, following the single responsibility principle.

apps/api/src/services/db/statsService.ts (2)

8-8: Clean up of imports

The ProviderActiveLeasesStats import was removed since it's only needed in the module that now contains the refactored functionality.


1-403: Good removal of functionality as part of modularization

The removal of the getProviderActiveLeasesGraphData function from this file is consistent with the PR objective of moving provider-active-leases-graph-data to the modules structure. This improves code organization and modularity.

apps/api/src/provider/routes/providers/providers.router.ts (3)

6-11: Import organization looks good

The imports are well-organized and correctly include the necessary schemas for the new active leases graph data endpoint.


33-53: Good route definition with proper OpenAPI documentation

The new route for provider active leases graph data is well-defined with:

  • Clear path structure following RESTful conventions
  • Appropriate tagging with both "Analytics" and "Providers"
  • Proper parameter validation schema
  • Well-documented response schemas for both success and error cases

This follows the module-based architecture pattern being implemented.


64-69: Handler implementation is clean and follows established patterns

The route handler correctly:

  1. Extracts the validated provider address from parameters
  2. Delegates to the appropriate controller method
  3. Returns the JSON response with the graph data

The implementation is succinct and follows the project's patterns.

apps/api/test/seeders/deployment.seeder.ts (2)

1-4: Imports look good

The imports include the necessary Deployment model, faker library for generating test data, and the Sequelize CreationAttributes type.


5-19: Well-structured seeder with comprehensive field coverage

The DeploymentSeeder.create method properly handles all fields of the Deployment model, with good defaults and support for custom overrides. This will provide flexibility in test scenarios.

🧰 Tools
🪛 Biome (1.9.4)

[error] 5-25: Avoid classes that contain only static members.

Prefer using simple functions instead of classes with only static members.

(lint/complexity/noStaticOnlyClass)

apps/api/test/seeders/provider.seeder.ts (2)

1-3: Import is now properly updated

Good job updating the Provider import from a type-only import to a value import, which is necessary for database creation.


5-36: Thorough provider seeder implementation

The ProviderSeeder.create method comprehensively covers all fields of the Provider model with realistic defaults using faker. The attention to detail with different types of fields (strings, booleans, dates, numbers) ensures high-quality test data.

🧰 Tools
🪛 Biome (1.9.4)

[error] 5-42: Avoid classes that contain only static members.

Prefer using simple functions instead of classes with only static members.

(lint/complexity/noStaticOnlyClass)

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

11-15: Good update to use the new seeder classes

The imports have been properly updated to use the newly refactored seeder classes, which helps maintain consistency across the codebase.


19-19: Small update to use the new ProviderSeeder

Good job updating the provider seeds generation to use the new ProviderSeeder class's create method.


133-212: Comprehensive test for the new endpoint

This test is well-structured and thorough:

  1. It sets up a realistic test scenario with multiple days and leases
  2. The test data is created with the appropriate relationships between entities
  3. It properly validates both the status code and response structure
  4. The test verifies the correct counts in the response based on the test data

The test coverage is excellent for validating the new endpoint's functionality.

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

1-16: Approve type-safe refactoring of DaySeeder.create method

The refactoring significantly improves type safety by returning CreationAttributes<Day> instead of a plain object. The method now properly handles each field with direct property assignments and appropriate fallback values, making the code more maintainable and easier to understand.

apps/api/test/seeders/lease-api-response.seeder.ts (2)

6-59: Well-structured interfaces for API testing

The LeaseInput and LeaseOutput interfaces are well-defined with clear type definitions, providing a solid foundation for testing lease-related API responses. The nested structure accurately represents the API contract.


61-120: Comprehensive LeaseApiResponseSeeder.create implementation

The implementation effectively generates realistic lease data for testing scenarios:

  • Properly handles all required and optional fields
  • Uses conditional logic for state-dependent fields like closed_on
  • Correctly derives escrow payment state from the lease state
  • Preserves data consistency between related fields (e.g., same denom used throughout)
🧰 Tools
🪛 Biome (1.9.4)

[error] 61-125: Avoid classes that contain only static members.

Prefer using simple functions instead of classes with only static members.

(lint/complexity/noStaticOnlyClass)

apps/api/src/provider/http-schemas/provider.schema.ts (4)

2-4: LGTM: Clean imports

Good organization of imports with a clear separation between external and internal imports.


96-98: Approve using standardized AkashAddressSchema

The use of the reusable AkashAddressSchema with OpenAPI example metadata is a good practice for consistent address validation across the API.


100-115: Well-structured response schema with OpenAPI metadata

The schema provides a clear structure for the graph data response with appropriate field types and OpenAPI examples that will help with API documentation.


119-120: Appropriate type exports

The TypeScript type exports are correctly inferred from the Zod schemas, maintaining type safety throughout the application.

apps/api/src/provider/services/provider-stats/provider-stats.service.ts (4)

7-8: LGTM: Singleton service pattern

Using the singleton pattern via the @singleton() decorator is appropriate for this service as it doesn't need multiple instances.


29-45: Proper handling of insufficient data

The method correctly handles the case when there's insufficient data (less than 2 days) by returning default values. This prevents runtime errors when trying to access non-existent array elements.


46-64: Clear data transformation for API response

The transformation from raw query results to the API response format is well implemented:

  • Correctly extracts current and previous day values
  • Maps results to the expected snapshot format
  • Provides the required comparison structure

9-28:

Details

❌ Incorrect review comment

SQL query has potential edge cases

The SQL query for retrieving active leases is well-structured, but has a few potential issues to consider:

  1. The LEFT JOIN with the lease table combined with the GROUP BY might include days with no leases but might be misleading when counting
  2. The repeated provider address condition in both the JOIN and WHERE clauses

🏁 Script executed:

#!/bin/bash
# Verify if the provider exists in the provider table before querying
# This will help understand potential null data scenarios

# Check if the address format check exists elsewhere
rg -A 3 "AkashAddressSchema"

# Check provider table structure to understand columns
fd -e ts -p "provider.ts" --exec cat {} 

Length of output: 60913


SQL query is correct as written
The LEFT JOIN + COUNT(l.id) will return 0 for days with no active leases (as intended), and the INNER JOIN on provider filters out days before the provider’s creation and guarantees the provider exists. The duplicate use of :providerAddress in both joins is deliberate to scope the lease join and enforce the provider filter. No changes needed.

Likely an incorrect or invalid review comment.

apps/api/test/seeders/lease.seeder.ts (3)

1-3: Improved imports with model and type references.

The imports have been updated to include the Lease model and Sequelize's CreationAttributes type, which provides better type safety and aligns with the Sequelize ORM pattern.


6-6: Enhanced type safety with Sequelize's CreationAttributes.

Using CreationAttributes<Lease> as the return type provides better type checking and ensures the seeder output matches exactly what the Lease model expects.


8-26: Well-structured seed data generation with comprehensive field coverage.

The seeder now generates a complete set of fields needed for the Lease model, with sensible default values and proper overriding capability. The use of faker for generating realistic test data is a good practice.

@baktun14 baktun14 merged commit f1c004f into akash-network:main May 12, 2025
26 of 27 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.

2 participants

Comments