fix: prevent request to provider-proxy when deployment is closed #1660
Conversation
WalkthroughThe changes update service access patterns and link prefetching in several components. Service dependencies are shifted from direct imports to context-based access via hooks, and a notifications API service is relocated from one service container to another. Additionally, prefetching is disabled for specific navigation links to optimize resource loading. Testing utilities and Jest configuration were also refined for improved coverage and request mocking. Changes
Sequence Diagram(s)sequenceDiagram
participant DeploymentDetail
participant useServices
participant analyticsService
participant getDeploymentDetail
participant getLeases
participant leaseRefs
DeploymentDetail->>useServices: Access analyticsService
DeploymentDetail->>getDeploymentDetail: Await deployment detail
DeploymentDetail->>getLeases: Await leases
getDeploymentDetail-->>DeploymentDetail: Deployment data
getLeases-->>DeploymentDetail: Lease data
alt deployment.state == "active"
DeploymentDetail->>leaseRefs: Refresh lease statuses
end
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed 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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
stalniy
force-pushed
the
fix/lease-after-close
branch
from
6362671 to
ca38adf
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1660 +/- ##
===========================================
+ Coverage 41.17% 72.26% +31.09%
===========================================
Files 908 590 -318
Lines 23704 13504 -10200
Branches 4672 2239 -2433
===========================================
Hits 9759 9759
+ Misses 13665 3436 -10229
- Partials 280 309 +29
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
stalniy
force-pushed
the
fix/lease-after-close
branch
2 times, most recently
from
3fcc0b2 to
af5363f
Compare
stalniy
force-pushed
the
fix/lease-after-close
branch
from
af5363f to
acfcded
Compare
stalniy
force-pushed
the
fix/lease-after-close
branch
from
acfcded to
f7082d0
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/deploy-web/jest.config.ts(1 hunks)apps/deploy-web/src/components/alerts/DeploymentAlertsContainer/DeploymentAlertsContainer.spec.tsx(2 hunks)apps/deploy-web/src/components/deployments/DeploymentDetail.tsx(4 hunks)apps/deploy-web/src/components/new-deployment/TemplateList.tsx(2 hunks)apps/deploy-web/src/components/templates/TemplateBox.tsx(1 hunks)apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx(0 hunks)apps/deploy-web/src/lib/nextjs/defineApiHandler/defineApiHandler.spec.ts(1 hunks)apps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.spec.ts(3 hunks)apps/deploy-web/src/services/http/http-browser.service.ts(2 hunks)
💤 Files with no reviewable changes (1)
- apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx
✅ Files skipped from review due to trivial changes (1)
- apps/deploy-web/src/lib/nextjs/defineApiHandler/defineApiHandler.spec.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/deploy-web/src/components/templates/TemplateBox.tsx
- apps/deploy-web/src/services/http/http-browser.service.ts
- apps/deploy-web/src/components/new-deployment/TemplateList.tsx
- apps/deploy-web/src/components/deployments/DeploymentDetail.tsx
🧰 Additional context used
📓 Path-based instructions (2)
`apps/{deploy-web,provider-console}/**/*.spec.tsx`: Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
apps/{deploy-web,provider-console}/**/*.spec.tsx: UsequeryBymethods instead ofgetBymethods in test expectations in.spec.tsxfiles
📄 Source: CodeRabbit Inference Engine (.cursor/rules/query-by-in-tests.mdc)
List of files the instruction was applied to:
apps/deploy-web/src/components/alerts/DeploymentAlertsContainer/DeploymentAlertsContainer.spec.tsx
`**/*.spec.{ts,tsx}`: Use `setup` function instead of `beforeEach` in test files...
**/*.spec.{ts,tsx}: Usesetupfunction instead ofbeforeEachin test files
setupfunction must be at the bottom of the rootdescribeblock
setupfunction creates an object under test and returns it
setupfunction should accept a single parameter with inline type definition
Don't use shared state insetupfunction
Don't specify return type ofsetupfunction
📄 Source: CodeRabbit Inference Engine (.cursor/rules/setup-instead-of-before-each.mdc)
List of files the instruction was applied to:
apps/deploy-web/src/components/alerts/DeploymentAlertsContainer/DeploymentAlertsContainer.spec.tsxapps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.spec.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-06-30T12:11:50.570Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
apps/deploy-web/src/components/alerts/DeploymentAlertsContainer/DeploymentAlertsContainer.spec.tsx (6)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-06-30T12:11:50.570Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use shared state in `setup` function
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function should accept a single parameter with inline type definition
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't specify return type of `setup` function
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns it
apps/deploy-web/jest.config.ts (7)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use shared state in `setup` function
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-06-30T12:11:50.570Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't specify return type of `setup` function
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function should accept a single parameter with inline type definition
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns it
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function must be at the bottom of the root `describe` block
apps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.spec.ts (6)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-06-30T12:11:50.570Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns it
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function should accept a single parameter with inline type definition
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't specify return type of `setup` function
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use shared state in `setup` function
⏰ 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). (3)
- GitHub Check: validate (apps/deploy-web) / validate-unsafe
- GitHub Check: validate / validate-app
- GitHub Check: test-build
🔇 Additional comments (6)
apps/deploy-web/src/components/alerts/DeploymentAlertsContainer/DeploymentAlertsContainer.spec.tsx (1)
19-19: Good addition of async test utilities.Adding
waitForto handle asynchronous test scenarios is appropriate and aligns with the test changes.apps/deploy-web/jest.config.ts (1)
39-46: Excellent refactoring of coverage configuration.Moving coverage settings to project-specific configurations provides better granular control over coverage collection for different test environments (jsdom vs node). This improves test organization and ensures accurate coverage reporting for both browser and server-side code.
apps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.spec.ts (4)
177-177: Good use of the new createRequest helper.The test now uses the standardized
createRequesthelper function, which provides more consistent and realistic request mocking with appropriate default headers.
182-182: Improved assertion with partial matching.Using
expect.objectContaining(headers)provides more flexible and robust assertions for header validation.
315-315: Consistent use of createRequest helper.The setup function now uses the standardized
createRequest()helper, improving consistency across test cases.
334-347: Excellent test utility function.The
createRequesthelper function provides standardized request mocking with realistic default headers includinghost,content-type, and variousx-forwarded-*headers. This improves test consistency and provides more realistic request scenarios.
| await waitFor(() => { | ||
| expect(screen.getByTestId("alert-config-success-notification")).toBeInTheDocument(); | ||
| }); |
There was a problem hiding this comment.
Use queryBy method instead of getBy method.
The coding guidelines specify that .spec.tsx files should use queryBy methods instead of getBy methods in test expectations.
await waitFor(() => {
- expect(screen.getByTestId("alert-config-success-notification")).toBeInTheDocument();
+ expect(screen.queryByTestId("alert-config-success-notification")).toBeInTheDocument();
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await waitFor(() => { | |
| expect(screen.getByTestId("alert-config-success-notification")).toBeInTheDocument(); | |
| }); | |
| await waitFor(() => { | |
| expect(screen.queryByTestId("alert-config-success-notification")).toBeInTheDocument(); | |
| }); |
🤖 Prompt for AI Agents
In
apps/deploy-web/src/components/alerts/DeploymentAlertsContainer/DeploymentAlertsContainer.spec.tsx
around lines 54 to 56, replace the use of screen.getByTestId with
screen.queryByTestId in the waitFor expectation. This change aligns with the
coding guidelines that require using queryBy methods instead of getBy methods in
test files. Adjust the assertion to check that the element returned by
queryByTestId is not null or is present in the document.
There was a problem hiding this comment.
it's better to use getBy with toBeInTheDocument because it will throw an error and will show DOM state. queryBy will return null and we will not see DOM state
There was a problem hiding this comment.
@stalniy, that's an excellent point! You're absolutely right that getBy with toBeInTheDocument provides much better debugging information when tests fail. The error message will include the current DOM state, which is invaluable for understanding what went wrong, whereas queryBy returning null doesn't give us that context.
Thank you for the clarification - I'll update my understanding accordingly.
✏️ Learnings added
Learnt from: stalniy
PR: akash-network/console#1660
File: apps/deploy-web/src/components/alerts/DeploymentAlertsContainer/DeploymentAlertsContainer.spec.tsx:54-56
Timestamp: 2025-07-11T10:46:43.700Z
Learning: In apps/{deploy-web,provider-console}/**/*.spec.tsx files: Use `getBy` methods instead of `queryBy` methods when testing element presence with `toBeInTheDocument()` because `getBy` throws an error and shows DOM state when element is not found, providing better debugging information than `queryBy` which returns null.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
⛔ Learnings removed
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-06-30T12:11:50.570Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
🧠 Learnings used
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-06-30T12:11:50.570Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Why
Bug, currently after deployment is closed, app sends request to check its lease which fails with 404 error
Summary by CodeRabbit
New Features
Performance
Chores