replace chain with chain_id in insight playground #7099
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes update the usage of the parameter "chain" to "chain_id" throughout the playground blueprint UI logic, ensuring consistent naming and handling. Deprecated parameters are now excluded from form schema generation. Additionally, the type definition for blueprints in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BlueprintPlaygroundUI
participant modifyParametersForPlayground
participant Form
User->>BlueprintPlaygroundUI: Interacts with playground UI
BlueprintPlaygroundUI->>modifyParametersForPlayground: Prepare parameters
modifyParametersForPlayground->>modifyParametersForPlayground: Remove "chain", require "chain_id", skip deprecated
modifyParametersForPlayground-->>BlueprintPlaygroundUI: Return modified parameters
BlueprintPlaygroundUI->>Form: Render form with "chain_id" and non-deprecated parameters
Form->>BlueprintPlaygroundUI: Submit form values
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ Context from checks skipped due to timeout of 90000ms (4)
✨ Finishing Touches
🪧 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 (
|
github-actions
bot
added
the
Playground
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
apps/playground-web/src/app/insight/[blueprint_slug]/blueprint-playground.client.tsx
Outdated
Show resolved
Hide resolved
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7099 +/- ##
=======================================
Coverage 55.81% 55.81%
=======================================
Files 900 900
Lines 57848 57848
Branches 4067 4067
=======================================
Hits 32288 32288
Misses 25454 25454
Partials 106 106
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/playground-web/src/app/insight/[blueprint_slug]/blueprint-playground.client.tsx (1)
501-506: Remove debugging log statement.The
console.logstatement should be removed as it appears to be added for debugging purposes and isn't necessary for production code.- console.log("chainIds", chainIds); props.form.setValue("chain_id", chainIds, {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/playground-web/src/app/insight/[blueprint_slug]/blueprint-playground.client.tsx(6 hunks)apps/playground-web/src/app/insight/page.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/playground-web/src/app/insight/[blueprint_slug]/blueprint-playground.client.tsx (1)
apps/playground-web/src/components/blocks/NetworkSelectors.tsx (1)
MultiNetworkSelector(15-129)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Lint Packages
- GitHub Check: Size
- GitHub Check: Analyze (javascript)
🔇 Additional comments (8)
apps/playground-web/src/app/insight/page.tsx (1)
44-44: Good addition of deprecated flag support.The type definition now includes an optional
deprecatedboolean property, which aligns with the changes in the blueprint playground client that filter out deprecated parameters.apps/playground-web/src/app/insight/[blueprint_slug]/blueprint-playground.client.tsx (7)
112-115: Parameter name update fromchaintochain_id.The code now correctly identifies the chain parameter using
chain_idinstead ofchain. This is consistent with the PR objective to replace all instances ofchainwithchain_id.
125-129: Good handling of deprecated parameter removal.This addition explicitly removes the
chainparameter if present, which ensures a clean transition to usingchain_idinstead. This helps prevent duplicate or conflicting parameters in the API calls.
172-174: Updated default value assignment to usechain_id.The default value assignment has been updated to use
chain_idinstead ofchain, maintaining consistency with the parameter rename throughout the codebase.
475-478: Consistent UI rendering condition update.The grid layout condition has been updated to check for
chain_idinstead ofchain, ensuring the UI renders correctly with the updated parameter name.
494-515: Updated MultiNetworkSelector handling.The selector component now properly uses
chain_idfor its conditional rendering and data binding, which is consistent with the parameter rename throughout the application.
498-499: Type casting forchain_idwatch looks good.The form watch method now correctly references
chain_idinstead ofchain.
831-833: Good implementation of deprecated parameter skipping.This new code skips parameters marked as deprecated when creating the form schema. This is an important enhancement that prevents deprecated parameters from appearing in the UI, as specified in the PR objectives.
size-limit report 📦
|
iuwqyir
force-pushed
the
05-20-replace_chain_with_chain_id_in_insight_playground
branch
from
483ce36 to
7e98519
Compare
iuwqyir
force-pushed
the
05-20-replace_chain_with_chain_id_in_insight_playground
branch
from
7e98519 to
bde14b9
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/playground-web/src/app/insight/utils.ts (1)
9-10: Inconsistency with existing constants in codebase.This new constant
THIRDWEB_INSIGHT_API_DOMAINuses a different environment variable name (NEXT_PUBLIC_INSIGHT_URL) and default value ("insight.thirdweb.com") compared to a similar constant inapps/dashboard/src/constants/urls.tswhich usesNEXT_PUBLIC_INSIGHT_API_URLand defaults to "insight.thirdweb-dev.com".Consider aligning the naming and defaults with existing patterns in the codebase, or documenting why they differ.
apps/playground-web/src/app/insight/[blueprint_slug]/blueprint-playground.client.tsx (1)
905-906: Remove ESLint directive unless necessary.The code contains an ESLint directive (
// eslint-disable-next-line no-restricted-syntax) that may not be needed anymore depending on your ESLint configuration.Consider removing this comment if the ESLint rule is no longer applicable or documenting why the exception is necessary.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/playground-web/src/app/insight/[blueprint_slug]/blueprint-playground.client.tsx(6 hunks)apps/playground-web/src/app/insight/page.tsx(1 hunks)apps/playground-web/src/app/insight/utils.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/playground-web/src/app/insight/page.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/playground-web/src/app/insight/utils.ts (1)
apps/dashboard/src/constants/urls.ts (1)
THIRDWEB_INSIGHT_API_DOMAIN(24-25)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Size
- GitHub Check: Lint Packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (8)
apps/playground-web/src/app/insight/utils.ts (1)
46-46: LGTM: Using configurable domain for API endpoint.The change to use the configurable
THIRDWEB_INSIGHT_API_DOMAINconstant improves flexibility by allowing the API domain to be configured via environment variables.apps/playground-web/src/app/insight/[blueprint_slug]/blueprint-playground.client.tsx (7)
112-115: LGTM: Parameter name updated from "chain" to "chain_id".This change correctly updates the parameter name from "chain" to "chain_id", which is consistent with the PR objective.
125-129: LGTM: Added code to remove deprecated "chain" parameter.This new code block correctly removes the deprecated "chain" parameter from the parameters list, which is consistent with the PR objective to replace "chain" with "chain_id".
172-173: LGTM: Default value handling updated for "chain_id" parameter.Updated the default value handling to use "chain_id" instead of "chain", maintaining consistency with the parameter renaming.
475-478: LGTM: Updated conditional rendering for "chain_id" parameter.This change correctly updates the conditional check from "chain" to "chain_id", ensuring the proper grid layout is applied.
494-499: LGTM: Updated MultiNetworkSelector to use "chain_id" parameter.This change correctly updates the MultiNetworkSelector to use the renamed "chain_id" parameter for tracking selected chain IDs.
500-505: LGTM: Updated form value setter to use "chain_id" parameter.This change correctly updates the form setValue call to use "chain_id" instead of "chain", completing the parameter renaming.
830-832: LGTM: Skip deprecated parameters when creating the form schema.This new code properly implements the logic to skip deprecated parameters when generating the form schema, which supports the PR objective of handling deprecated parameters like "chain".
Merge activity
|
## [Playground] Fix: Update chain parameter handling in blueprint playground
## Notes for the reviewer
This PR updates the blueprint playground to use `chain_id` instead of `chain` parameter throughout the codebase. It also adds logic to remove deprecated parameters from the form schema and specifically removes the `chain` parameter if it's present.
## How to test
Test the blueprint playground with various chain selections to ensure the chain_id parameter is correctly populated and that deprecated parameters are not displayed in the form.
<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
## Summary by CodeRabbit
- **Refactor**
- Updated all references from "chain" to "chain_id" for improved consistency across the playground interface.
- Deprecated parameters are now excluded from form generation and display.
- **New Features**
- Blueprint listings can now indicate if a blueprint is deprecated.
- **Chores**
- Made the API domain configurable via environment variables with a default fallback.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
<!-- start pr-codex -->
---
## PR-Codex overview
This PR introduces changes to the `BlueprintSection`, `fetchBlueprintSpec`, and other components to enhance the handling of blueprint parameters, including the addition of a `deprecated` flag and renaming of parameters for consistency. It also updates the API URL handling.
### Detailed summary
- Added optional `deprecated` property to `blueprints` in `BlueprintSection`.
- Introduced `THIRDWEB_INSIGHT_API_DOMAIN` for dynamic API URL in `fetchBlueprintSpec`.
- Renamed parameter `chain` to `chain_id` in multiple places for consistency.
- Removed `chain` parameter if present in `modifyParametersForPlayground`.
- Updated form handling to use `chain_id` instead of `chain` in `ParameterSection`.
- Skipped deprecated parameters in schema creation with `createParametersFormSchema`.
> ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your question}`
<!-- end pr-codex -->
graphite-app
bot
force-pushed
the
05-20-replace_chain_with_chain_id_in_insight_playground
branch
from
bde14b9 to
438c019
Compare
graphite-app
bot
deleted the
05-20-replace_chain_with_chain_id_in_insight_playground
branch
graphite-app
bot
removed
the
merge-queue
[Playground] Fix: Update chain parameter handling in blueprint playground
Notes for the reviewer
This PR updates the blueprint playground to use
chain_idinstead ofchainparameter throughout the codebase. It also adds logic to remove deprecated parameters from the form schema and specifically removes thechainparameter if it's present.How to test
Test the blueprint playground with various chain selections to ensure the chain_id parameter is correctly populated and that deprecated parameters are not displayed in the form.
Summary by CodeRabbit
Refactor
New Features
Chores
PR-Codex overview
This PR primarily focuses on updating the
BlueprintSectionand related functionality to handle a new optionaldeprecatedproperty for blueprints, along with renaming parameters for consistency and improving the handling of API domain configuration.Detailed summary
BlueprintSectionto include an optionaldeprecatedproperty inblueprints.THIRDWEB_INSIGHT_API_DOMAINfor API domain configuration.chainparameter tochain_idin multiple locations for consistency.chainparameter from parameters if present.chain_idparameter.