feat(deployment): implements deploy link constructor#2483
Conversation
📝 WalkthroughWalkthroughAdds a new ShareDeployButton component (with tests) and integrates it into ManifestEdit; also adds getBaseUrl utility. The share component builds deploy snippets, opens a modal, and supports copy-to-clipboard with snackbar feedback. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant ManifestEdit
participant ShareDeployButton
participant Popup
participant Clipboard
participant Snackbar
User->>ManifestEdit: Clicks Share button
ManifestEdit->>ShareDeployButton: Delegate click
ShareDeployButton->>Popup: Open CreateCustom modal (tabs)
User->>Popup: Select tab and click Copy
Popup->>ShareDeployButton: Invoke copy handler with snippet
ShareDeployButton->>Clipboard: copyTextToClipboard(snippet)
Clipboard-->>ShareDeployButton: success
ShareDeployButton->>Snackbar: show success message
Snackbar-->>User: display confirmation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
Codecov Report❌ Patch coverage is
❌ Your project status has failed because the head coverage (79.57%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #2483 +/- ##
==========================================
- Coverage 50.83% 50.63% -0.20%
==========================================
Files 1065 1056 -9
Lines 29376 29094 -282
Branches 6517 6488 -29
==========================================
- Hits 14934 14733 -201
+ Misses 14033 14026 -7
+ Partials 409 335 -74
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @apps/deploy-web/src/components/new-deployment/ShareDeployButton.tsx:
- Around line 93-102: Remove the incorrect aria-haspopup="true" from the copy
buttons that call copySnippet (the Button wrapping <Copy /> which invokes
copySnippet(deployButtonData.markdownSnippet)) and the other copy buttons
mentioned; these buttons only copy to clipboard and show a snackbar, so delete
the aria-haspopup prop from those Button components (also update the Button
instances that call copySnippet at the other two locations referenced in the
review).
🧹 Nitpick comments (2)
apps/deploy-web/src/components/new-deployment/ShareDeployButton.tsx (2)
43-78: Consider extracting repeated TabsTrigger className to reduce duplication.The three TabsTrigger components share nearly identical className strings. While functional, extracting the common classes would improve maintainability.
♻️ Proposed refactor
const tabTriggerClassName = "flex-1 cursor-pointer rounded-none border-0 border-b-2 border-l-0 border-r-0 border-t-0 border-b-transparent bg-transparent py-1.5 shadow-none transition-colors data-[state=active]:border-b-neutral-900 data-[state=active]:bg-transparent data-[state=active]:shadow-none hover:bg-transparent focus-visible:outline-none focus-visible:ring-0 focus-visible:ring-offset-0 dark:data-[state=active]:border-b-neutral-100"; const getTabTextClassName = (isActive: boolean) => `text-sm font-normal leading-5 transition-colors ${isActive ? "text-neutral-950 dark:text-[var(--text-light)]" : "text-neutral-500 hover:text-neutral-700 dark:text-neutral-400 dark:hover:text-neutral-300"}`;Then use these in each TabsTrigger.
186-186: Unnecessary template literal.The template literal is redundant since
UrlService.newDeployment()already returns a string.♻️ Proposed fix
- const deployUrl = `${UrlService.newDeployment(urlParams)}`; + const deployUrl = UrlService.newDeployment(urlParams);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/deploy-web/public/images/deploy-with-akash-btn.svgis excluded by!**/*.svg
📒 Files selected for processing (2)
apps/deploy-web/src/components/new-deployment/ManifestEdit.tsxapps/deploy-web/src/components/new-deployment/ShareDeployButton.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{ts,tsx,js}: Never use typeanyor cast to typeany. Always define the proper TypeScript types.
Never use deprecated methods from libraries.
Don't add unnecessary comments to the code.
Files:
apps/deploy-web/src/components/new-deployment/ShareDeployButton.tsxapps/deploy-web/src/components/new-deployment/ManifestEdit.tsx
⏰ 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). (2)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
🔇 Additional comments (8)
apps/deploy-web/src/components/new-deployment/ShareDeployButton.tsx (6)
1-12: LGTM!Imports are well-organized, using appropriate UI components, context hooks, and utilities.
14-26: LGTM!Props interfaces are properly typed with clear structure for the deployment button data.
158-161: Empty string values will be treated as null.The
|| nulloperator returns null for empty strings. If an empty string is a valid intentional value for any of these env vars, this could cause unexpected behavior. Consider using?? nullif onlyundefinedshould fall back to null.
144-150: LGTM!The component properly handles undefined services and correctly checks for public HTTPS repositories before enabling the share functionality.
200-203: LGTM!Copy functionality with snackbar feedback is well-implemented.
227-232: LGTM!Conditional rendering based on
isPublicRepocorrectly prevents the share button from appearing for private repositories.apps/deploy-web/src/components/new-deployment/ManifestEdit.tsx (2)
43-43: LGTM!Import added correctly for the new ShareDeployButton component.
314-336: LGTM!The layout integration is clean. The ShareDeployButton is properly positioned alongside the Create Deployment button, and the flex-grow wrapper ensures proper spacing. The
servicesprop is correctly passed from theuseImportSimpleSdlhook result.
apps/deploy-web/src/components/new-deployment/ShareDeployButton.tsx
Outdated
Show resolved
Hide resolved
0a5be89 to
89cbea8
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/deploy-web/src/components/new-deployment/ShareDeployButton/ShareDeployButton.tsx (1)
63-98: Consider extracting repeated tab styling to reduce duplication.The three
TabsTriggercomponents share nearly identical className strings (~200+ characters each). Extracting the common styling would improve maintainability.♻️ Suggested refactor
+const tabTriggerClassName = "flex-1 cursor-pointer rounded-none border-0 border-b-2 border-l-0 border-r-0 border-t-0 border-b-transparent bg-transparent py-1.5 shadow-none transition-colors data-[state=active]:border-b-neutral-900 data-[state=active]:bg-transparent data-[state=active]:shadow-none hover:bg-transparent focus-visible:outline-none focus-visible:ring-0 focus-visible:ring-offset-0 dark:data-[state=active]:border-b-neutral-100"; + +const getTabTextClassName = (isActive: boolean) => + `text-sm font-normal leading-5 transition-colors ${isActive ? "text-neutral-950 dark:text-[var(--text-light)]" : "text-neutral-500 hover:text-neutral-700 dark:text-neutral-400 dark:hover:text-neutral-300"}`; <d.TabsTrigger value="markdown" - className="flex-1 cursor-pointer rounded-none border-0 border-b-2 border-l-0 border-r-0 border-t-0 border-b-transparent bg-transparent py-1.5 shadow-none transition-colors data-[state=active]:border-b-neutral-900 data-[state=active]:bg-transparent data-[state=active]:shadow-none hover:bg-transparent focus-visible:outline-none focus-visible:ring-0 focus-visible:ring-offset-0 dark:data-[state=active]:border-b-neutral-100" + className={tabTriggerClassName} > <div className="flex items-center justify-center gap-2 px-2.5 py-2"> - <span - className={`text-sm font-normal leading-5 transition-colors ${activeTab === "markdown" ? "text-neutral-950 dark:text-[var(--text-light)]" : "text-neutral-500 hover:text-neutral-700 dark:text-neutral-400 dark:hover:text-neutral-300"}`} - > + <span className={getTabTextClassName(activeTab === "markdown")}> Markdown </span> </div> </d.TabsTrigger>Apply similar changes to the HTML and URL tabs.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/deploy-web/public/images/deploy-with-akash-btn.svgis excluded by!**/*.svg
📒 Files selected for processing (4)
apps/deploy-web/src/components/new-deployment/ManifestEdit.tsxapps/deploy-web/src/components/new-deployment/ShareDeployButton/ShareDeployButton.spec.tsxapps/deploy-web/src/components/new-deployment/ShareDeployButton/ShareDeployButton.tsxapps/deploy-web/src/utils/urlUtils.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/deploy-web/src/components/new-deployment/ManifestEdit.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{ts,tsx,js}: Never use typeanyor cast to typeany. Always define the proper TypeScript types.
Never use deprecated methods from libraries.
Don't add unnecessary comments to the code.
Files:
apps/deploy-web/src/utils/urlUtils.tsapps/deploy-web/src/components/new-deployment/ShareDeployButton/ShareDeployButton.tsxapps/deploy-web/src/components/new-deployment/ShareDeployButton/ShareDeployButton.spec.tsx
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under testUse
setupfunction instead ofbeforeEachin test files. Thesetupfunction must be at the bottom of the rootdescribeblock, should create an object under test and return it, accept a single parameter with inline type definition, avoid shared state, and not have a specified return type.
**/*.spec.{ts,tsx}: Use<Subject>.namein the root describe suite description instead of hardcoded class/service name strings to enable automated refactoring tools to find all references
Use either a method name or a condition starting with 'when' for nested suite descriptions in tests
Use present simple, 3rd person singular for test descriptions without prepending 'should'
Files:
apps/deploy-web/src/components/new-deployment/ShareDeployButton/ShareDeployButton.spec.tsx
{apps/deploy-web,apps/provider-console}/**/*.spec.tsx
📄 CodeRabbit inference engine (.cursor/rules/query-by-in-tests.mdc)
Use
queryBymethods instead ofgetBymethods in test expectations
Files:
apps/deploy-web/src/components/new-deployment/ShareDeployButton/ShareDeployButton.spec.tsx
🧠 Learnings (4)
📚 Learning: 2025-11-25T17:45:49.180Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-11-25T17:45:49.180Z
Learning: Applies to {apps/deploy-web,apps/provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations
Applied to files:
apps/deploy-web/src/components/new-deployment/ShareDeployButton/ShareDeployButton.spec.tsx
📚 Learning: 2025-11-25T17:45:44.790Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/no-jest-mock.mdc:0-0
Timestamp: 2025-11-25T17:45:44.790Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use `jest.mock()` in test files. Instead, use `jest-mock-extended` to create mocks and pass mocks as dependencies to the service under test
Applied to files:
apps/deploy-web/src/components/new-deployment/ShareDeployButton/ShareDeployButton.spec.tsx
📚 Learning: 2025-07-11T10:46:43.711Z
Learnt from: stalniy
Repo: akash-network/console PR: 1660
File: apps/deploy-web/src/components/alerts/DeploymentAlertsContainer/DeploymentAlertsContainer.spec.tsx:54-56
Timestamp: 2025-07-11T10:46:43.711Z
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.
Applied to files:
apps/deploy-web/src/components/new-deployment/ShareDeployButton/ShareDeployButton.spec.tsx
📚 Learning: 2025-11-25T17:45:52.965Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-11-25T17:45:52.965Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files. The `setup` function must be at the bottom of the root `describe` block, should create an object under test and return it, accept a single parameter with inline type definition, avoid shared state, and not have a specified return type.
Applied to files:
apps/deploy-web/src/components/new-deployment/ShareDeployButton/ShareDeployButton.spec.tsx
🧬 Code graph analysis (1)
apps/deploy-web/src/components/new-deployment/ShareDeployButton/ShareDeployButton.spec.tsx (2)
apps/deploy-web/src/components/new-deployment/ShareDeployButton/ShareDeployButton.tsx (2)
ShareDeployButton(171-277)DEPENDENCIES(14-21)apps/deploy-web/src/config/remote-deploy.config.ts (1)
protectedEnvironmentVariables(7-24)
⏰ 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). (2)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
🔇 Additional comments (5)
apps/deploy-web/src/utils/urlUtils.ts (1)
23-28: LGTM!The
getBaseUrl()function correctly handles SSR vs browser environments by checkingtypeof windowand falling back to the existingdomainNameconstant.apps/deploy-web/src/components/new-deployment/ShareDeployButton/ShareDeployButton.tsx (2)
14-21: LGTM!The dependency injection pattern via
DEPENDENCIESexport enables clean testing by allowing mock injection for hooks and utilities.
171-277: LGTM!The component cleanly implements visibility logic based on public repo detection, constructs deployment URLs with the injected
getBaseUrl, and properly handles the modal flow with dependency injection for the main component hooks.apps/deploy-web/src/components/new-deployment/ShareDeployButton/ShareDeployButton.spec.tsx (2)
74-109: LGTM!Good approach to test modal content by extracting and rendering the
messageprop from the mock call. The test covers all three tabs and verifies the expected URL construction with encoded parameters.
111-164: LGTM!The
setupfunction follows all coding guidelines: positioned at the bottom of the describe block, accepts a single parameter with inline type definition, avoids shared state, creates fresh mocks via dependency injection, and has no explicit return type. Based on learnings, this correctly avoidsjest.mock()in favor of passing mocks as dependencies.
89cbea8 to
818c0ca
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/deploy-web/src/components/new-deployment/ShareDeployButton/ShareDeployButton.tsx (2)
28-46: Remove unusedSnackbarfrom dependencies interface.
Snackbaris declared in theShareDeployButtonContentProps.dependenciesinterface but is never used withinShareDeployButtonContent. The snackbar notification is handled in the parent component'scopySnippetcallback.Suggested fix
interface ShareDeployButtonContentProps { deployButtonData: { deployUrl: string; markdownSnippet: string; htmlSnippet: string; buttonImageUrl: string; }; copySnippet: (snippet: string) => void; dependencies: { Button: typeof Button; Input: typeof Input; - Snackbar: typeof Snackbar; Tabs: typeof Tabs; TabsContent: typeof TabsContent; TabsList: typeof TabsList; TabsTrigger: typeof TabsTrigger; Copy: typeof Copy; }; }Also update the caller (lines 255-264) accordingly.
63-98: Consider extracting duplicated TabsTrigger classNames.The
classNameprop for eachTabsTrigger(lines 65, 77, 89) is identical across all three tabs. Extracting this to a constant would improve maintainability and reduce repetition.Suggested approach
const tabTriggerClassName = "flex-1 cursor-pointer rounded-none border-0 border-b-2 border-l-0 border-r-0 border-t-0 border-b-transparent bg-transparent py-1.5 shadow-none transition-colors data-[state=active]:border-b-neutral-900 data-[state=active]:bg-transparent data-[state=active]:shadow-none hover:bg-transparent focus-visible:outline-none focus-visible:ring-0 focus-visible:ring-offset-0 dark:data-[state=active]:border-b-neutral-100"; // Then use: <d.TabsTrigger value="markdown" className={tabTriggerClassName}>
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/deploy-web/public/images/deploy-with-akash-btn.svgis excluded by!**/*.svg
📒 Files selected for processing (4)
apps/deploy-web/src/components/new-deployment/ManifestEdit.tsxapps/deploy-web/src/components/new-deployment/ShareDeployButton/ShareDeployButton.spec.tsxapps/deploy-web/src/components/new-deployment/ShareDeployButton/ShareDeployButton.tsxapps/deploy-web/src/utils/urlUtils.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/deploy-web/src/components/new-deployment/ManifestEdit.tsx
- apps/deploy-web/src/components/new-deployment/ShareDeployButton/ShareDeployButton.spec.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{ts,tsx,js}: Never use typeanyor cast to typeany. Always define the proper TypeScript types.
Never use deprecated methods from libraries.
Don't add unnecessary comments to the code.
Files:
apps/deploy-web/src/utils/urlUtils.tsapps/deploy-web/src/components/new-deployment/ShareDeployButton/ShareDeployButton.tsx
🧬 Code graph analysis (1)
apps/deploy-web/src/components/new-deployment/ShareDeployButton/ShareDeployButton.tsx (3)
apps/deploy-web/src/types/sdlBuilder.ts (1)
ServiceType(440-440)apps/deploy-web/src/config/remote-deploy.config.ts (1)
protectedEnvironmentVariables(7-24)apps/deploy-web/src/utils/urlUtils.ts (1)
UrlService(30-123)
⏰ 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 / validate-app
- GitHub Check: test-build
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (6)
apps/deploy-web/src/utils/urlUtils.ts (1)
23-28: LGTM!The
getBaseUrl()function follows the existing pattern in this file for handling SSR vs client-side contexts (e.g., lines 62-63, 71, 77). This provides a clean, centralized utility for base URL resolution.apps/deploy-web/src/components/new-deployment/ShareDeployButton/ShareDeployButton.tsx (5)
14-26: LGTM!The dependency injection pattern via
DEPENDENCIESis a good approach for testability, allowing mocks to be passed in tests.
175-180: LGTM!The public repo detection logic is sensible—checking for an HTTPS URL in
REPO_URLis a reasonable heuristic for identifying publicly accessible repositories.
182-228: LGTM!The
deployButtonDatacomputation correctly extracts environment variables and constructs the deploy URL with appropriate parameters. Good use ofuseMemoto avoid recomputation on every render.
249-266: Hardcoded dependencies in modal may affect testability.The
ShareDeployButtonContentrendered inside the modal receives hardcoded component references (lines 255-264) rather than using the injecteddependenciesprop. This means tests that mockDEPENDENCIESwon't affect the components rendered inside the modal.If this is intentional (e.g., only testing the button visibility logic), this is fine. Otherwise, consider passing through the dependencies from the parent.
272-276: LGTM!Clean conditional rendering with appropriate accessibility attributes. The component correctly returns
nullwhen no public repository is detected.
refs #2470
Screen.Recording.2026-01-13.at.17.12.41.mov
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.