-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: copiable-addresses, execute-button and etherscan-redirection #62
feat: copiable-addresses, execute-button and etherscan-redirection #62
Conversation
✅ Deploy Preview for kleros-escrow-v2 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
WalkthroughThis update enhances several components across the project, focusing on integrating new properties, refactoring existing logic, and improving user experiences. Key changes include adding a Changes
Sequence DiagramsequenceDiagram
participant User
participant PreviewCard
participant Header
participant ExecuteTransactionButton
participant Backend
User->>+PreviewCard: Load Component
PreviewCard->>+Header: Pass transactionHash
Header-->>-PreviewCard: Render with Etherscan link
User->>+ExecuteTransactionButton: Click Execute
ExecuteTransactionButton->>+Backend: Send Transaction Request
Backend-->>-ExecuteTransactionButton: Response with Status
ExecuteTransactionButton-->>-User: Update UI based on Status
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this 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
Outside diff range and nitpick comments (1)
web/src/utils/shortenAddress.ts (1)
Line range hint
6-14
: Ensure proper error handling ofgetAddress
. Consider adding a specific error message for better user feedback.- throw new TypeError("Invalid input, address can't be parsed"); + throw new TypeError("Invalid input, address can't be parsed: " + address);
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- web/src/components/PreviewCard/Terms/Description.tsx (3 hunks)
- web/src/components/TransactionInfo/index.tsx (4 hunks)
- web/src/utils/shortenAddress.ts (1 hunks)
Additional comments not posted (3)
web/src/utils/shortenAddress.ts (1)
4-4
: Good addition to handle empty addresses to prevent errors.web/src/components/PreviewCard/Terms/Description.tsx (1)
4-7
: Integration ofCopiable
anduseEnsName
enhances address display and interaction. Ensure that the fallback toshortenAddress
behaves correctly when ENS names are not available.Also applies to: 18-21, 23-25, 53-58
Verification successful
The code in
web/src/components/PreviewCard/Terms/Description.tsx
correctly implements a fallback toshortenAddress
when ENS names are not available, as shown by the lines:const displayBuyerAddress = buyerEns.data || shortenAddress(buyerAddress); const displaySellerAddress = sellerEns.data || shortenAddress(sellerAddress);This ensures that if
buyerEns.data
orsellerEns.data
isundefined
, the shortened address will be used instead.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the fallback behavior when ENS names are not available. # Test: Search for usages of `useEnsName` and check the fallback logic. rg --type jsx $'useEnsName'Length of output: 54
Script:
#!/bin/bash # Description: Verify the fallback behavior when ENS names are not available. # Test: Search for usages of `useEnsName` and check the fallback logic. rg --glob '*.tsx' 'useEnsName'Length of output: 980
Script:
#!/bin/bash # Description: Extract relevant code sections from `PreviewCard/Terms/Description.tsx` to inspect the fallback logic. # Extract lines containing `useEnsName` and surrounding context to verify fallback behavior. rg --glob 'web/src/components/PreviewCard/Terms/Description.tsx' -A 5 -B 5 'useEnsName'Length of output: 2015
web/src/components/TransactionInfo/index.tsx (1)
4-9
: Consistent use ofCopiable
anduseEnsName
across components. Verify the component's behavior in different contexts, especially in preview mode.Also applies to: 97-157
Verification successful
Consistent use of
isPreview
across multiple components and files has been verified. TheTransactionInfo
component and its related components handleisPreview
correctly.
web/src/components/TransactionInfo/index.tsx
contains multiple instances ofisPreview
being used for conditional rendering, ensuring proper behavior in preview mode.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the behavior of `TransactionInfo` in preview mode. # Test: Search for conditional rendering based on `isPreview`. rg --type jsx $'isPreview'Length of output: 53
Script:
#!/bin/bash # Description: Verify the behavior of `TransactionInfo` in preview mode. # Test: Search for conditional rendering based on `isPreview`. rg --type tsx $'isPreview'Length of output: 53
Script:
#!/bin/bash # Description: Verify the behavior of `TransactionInfo` in preview mode. # Test: Search for conditional rendering based on `isPreview`. rg --type tsx 'isPreview'Length of output: 53
Script:
#!/bin/bash # Description: Verify the behavior of `TransactionInfo` in preview mode. # Test: Search for conditional rendering based on `isPreview`. rg 'isPreview'Length of output: 5302
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- web/src/components/PreviewCard/Terms/Description.tsx (3 hunks)
- web/src/components/TransactionInfo/index.tsx (4 hunks)
- web/src/utils/shortenAddress.ts (1 hunks)
Additional comments not posted (5)
web/src/utils/shortenAddress.ts (1)
Line range hint
6-17
: Ensure robust error handling for address parsing.The function correctly handles cases where the address cannot be parsed by throwing a
TypeError
. This is a good practice for error management in utility functions.web/src/components/PreviewCard/Terms/Description.tsx (2)
4-7
: Ensure all new imports are used effectively throughout the component.The imports for
Copiable
,useEnsName
, andshortenAddress
are correctly utilized within the component to enhance address display functionality.
53-70
: Validate the logic for displaying addresses with ENS support.The logic to display either the ENS name or the shortened address using a ternary operator is a clean and effective way to handle conditional rendering based on available data.
Also applies to: 76-84
web/src/components/TransactionInfo/index.tsx (2)
4-9
: Ensure all new imports are used effectively throughout the component.The imports for
Copiable
,useEnsName
, andshortenAddress
are correctly utilized within the component, similar toDescription.tsx
, to enhance the address display functionality.
Line range hint
97-157
: Review the conditional rendering of addresses in theTransactionInfo
component.The conditional rendering based on the
isPreview
prop is a good use of React's capabilities to adapt component behavior based on props.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
web/src/assets/svgs/icons/etherscan.svg
is excluded by!**/*.svg
Files selected for processing (6)
- subgraph/mappings/escrow.ts (8 hunks)
- subgraph/schema.graphql (1 hunks)
- web/src/components/PreviewCard/Header.tsx (2 hunks)
- web/src/components/PreviewCard/index.tsx (3 hunks)
- web/src/hooks/queries/useTransactionsQuery.ts (1 hunks)
- web/src/pages/MyTransactions/TransactionDetails/index.tsx (2 hunks)
Additional comments not posted (6)
subgraph/schema.graphql (1)
44-44
: The addition of thetransactionHash
field as a non-nullable type is a robust choice for tracking transactions in theEscrow
schema.web/src/hooks/queries/useTransactionsQuery.ts (1)
23-23
: The addition of thetransactionHash
field in theTransactionDetails
fragment aligns well with the schema changes and ensures that transaction details are fetched correctly.Verification successful
The
transactionHash
field is being correctly utilized in multiple components and contexts within the codebase. It is referenced in components such asPreviewCard
andTransactionDetails
, ensuring that the transaction details, including thetransactionHash
, are properly integrated and displayed in the UI.
- Confirmed usage in
PreviewCard
andTransactionDetails
components.- Proper integration within the
useTransactionsQuery
hook.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct usage of the transactionHash field in UI components. # Test: Search for the usage of the TransactionDetails fragment in UI components. fd --exec rg 'TransactionDetails'Length of output: 63055
Script:
#!/bin/bash # Description: Verify the correct usage of the transactionHash field in UI components. # Test: Search for the usage of the transactionHash field in the relevant files. fd --exec rg 'transactionHash' ./web/src/pages/MyTransactions ./web/src/components ./web/src/hooks/queries ./web/src/contextLength of output: 428358
web/src/components/PreviewCard/index.tsx (1)
57-57
: The integration of thetransactionHash
property in thePreviewCard
component and its correct propagation to theHeader
component is well-implemented. Ensure that the full integration path oftransactionHash
from the GraphQL layer to the UI is tested.Also applies to: 102-102
Verification successful
The initial verification shows that the
transactionHash
property is used in various files. To ensure that the full integration path from the GraphQL layer to the UI is correctly implemented, we need to verify the following:
- GraphQL Schema: Ensure
transactionHash
is defined in the GraphQL schema.- GraphQL Queries: Verify that
transactionHash
is queried in GraphQL queries.- React Components: Check that
transactionHash
is passed down and used in the React components.Let's generate shell scripts to verify these aspects.
The
transactionHash
property is correctly defined in the GraphQL schema, queried in the GraphQL queries, and used in the React components. This confirms the full integration path from the GraphQL layer to the UI.
subgraph/schema.graphql
:transactionHash: Bytes!
web/src/hooks/queries/useTransactionsQuery.ts
:transactionHash
web/src/components/PreviewCard/index.tsx
:transactionHash
is passed to theHeader
component and used.The integration of the
transactionHash
property is well-implemented.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the full integration path of the transactionHash property from the GraphQL layer to the UI. # Test: Search for the usage of the transactionHash property in the UI components. fd --exec rg 'transactionHash'Length of output: 11803
Script:
#!/bin/bash # Verify that `transactionHash` is defined in the GraphQL schema rg 'transactionHash' --glob '*.graphql' # Verify that `transactionHash` is queried in GraphQL queries rg 'transactionHash' --glob '*.ts' # Verify that `transactionHash` is used in React components rg 'transactionHash' --glob '*.tsx'Length of output: 1258
web/src/pages/MyTransactions/TransactionDetails/index.tsx (1)
100-100
: ThetransactionHash
is passed correctly to thePreviewCard
component. Confirm thatPreviewCard
handles this new prop effectively.Verification successful
The
PreviewCard
component inweb/src/components/PreviewCard/index.tsx
does handle thetransactionHash
prop. It is defined as a string and is used within the component.
web/src/components/PreviewCard/index.tsx
transactionHash: string;
transactionHash,
<Header {...{ escrowType, escrowTitle, status, transactionHash, isCard: false }} />
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify if `PreviewCard` component handles `transactionHash` prop. ast-grep --lang javascript --pattern $'const PreviewCard = $_ => { $$$ transactionHash: $_, $$$ }'Length of output: 105
Script:
#!/bin/bash # Locate the PreviewCard component definition rg 'const PreviewCard' --files-with-matches | xargs -I {} sh -c 'echo "File: {}"; rg "const PreviewCard" {}' # Inspect the implementation of PreviewCard for handling the transactionHash prop rg 'PreviewCard' | xargs -I {} sh -c 'echo "File: {}"; rg "transactionHash" {}'Length of output: 3315
Script:
#!/bin/bash # Inspect the PreviewCard component implementation for handling the transactionHash prop rg 'transactionHash' web/src/components/PreviewCard/index.tsxLength of output: 195
subgraph/mappings/escrow.ts (2)
135-135
: ThetransactionHash
parameter is correctly used in thehandleNativeTransactionCreated
andhandleERC20TransactionCreated
events to create or update escrow records. This is a crucial update for tracking transactions directly in the subgraph. Ensure that the transaction hash is always available when these events are triggered.Also applies to: 167-167
Line range hint
25-37
: The addition oftransactionHash
to thecreateEscrow
function is well-implemented. It ensures that every new escrow record now tracks the transaction hash. Ensure that all calls tocreateEscrow
are updated to pass this new parameter.Verification successful
The search results show that the
createEscrow
function is called in two places within thesubgraph/mappings/escrow.ts
file:
- Inside the
handleNativeTransactionCreated
function- Inside the
handleERC20TransactionCreated
functionBoth instances include the
transactionHash
parameter as expected.
handleNativeTransactionCreated
:createEscrow(escrowId, event.transaction.hash)
handleERC20TransactionCreated
:createEscrow(escrowId, event.transaction.hash)
This confirms that all calls to
createEscrow
have been updated to pass the newtransactionHash
parameter.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all calls to `createEscrow` include the new `transactionHash` parameter. ast-grep --lang javascript --pattern $'createEscrow($$$, $_)'Length of output: 62
Script:
#!/bin/bash # Description: Find all instances of `createEscrow` calls and verify they include the `transactionHash` parameter. rg 'createEscrow\(' --context 2Length of output: 1182
There was a problem hiding this 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
Outside diff range and nitpick comments (2)
web/src/pages/MyTransactions/TransactionDetails/PreviewCardButtons/index.tsx (2)
21-27
: TheIPreviewCardButtons
interface is well-defined, clearly indicating the props required for the component. Ensure that these props are properly documented to improve maintainability.
Line range hint
93-134
: The conditional logic for displaying theExecuteTransactionButton
is sound, ensuring it only appears under specific conditions. However, consider adding a comment explaining the conditionstatus === "NoDispute" && currentTime >= settlementTimeout
for clarity.+ // Display the ExecuteTransactionButton only when there is no dispute and the current time is beyond the settlement timeout.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- web/src/components/PreviewCard/index.tsx (5 hunks)
- web/src/pages/MyTransactions/TransactionDetails/PreviewCardButtons/AcceptSettlementButton.tsx (1 hunks)
- web/src/pages/MyTransactions/TransactionDetails/PreviewCardButtons/ExecuteTransactionButton.tsx (1 hunks)
- web/src/pages/MyTransactions/TransactionDetails/PreviewCardButtons/index.tsx (4 hunks)
Files skipped from review due to trivial changes (1)
- web/src/pages/MyTransactions/TransactionDetails/PreviewCardButtons/AcceptSettlementButton.tsx
Files skipped from review as they are similar to previous changes (1)
- web/src/components/PreviewCard/index.tsx
Additional context used
Learnings (1)
web/src/pages/MyTransactions/TransactionDetails/PreviewCardButtons/index.tsx (1)
User: kemuru PR: kleros/escrow-v2#60 File: web/src/pages/NewTransaction/NavigationButtons/DepositPaymentButton.tsx:180-180 Timestamp: 2024-06-11T17:11:32.906Z Learning: The `refetchAllowance` function call is necessary in the `DepositPaymentButton` component to ensure the frontend updates correctly after an approval action.
Additional comments not posted (1)
web/src/pages/MyTransactions/TransactionDetails/PreviewCardButtons/index.tsx (1)
12-12
: The import ofExecuteTransactionButton
is correctly placed and follows the project's modular structure.
web/src/pages/MyTransactions/TransactionDetails/PreviewCardButtons/ExecuteTransactionButton.tsx
Show resolved
Hide resolved
There was a problem hiding this 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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- web/src/hooks/queries/useUserSettings.tsx (2 hunks)
- web/src/pages/NewTransaction/NavigationButtons/NextButton.tsx (1 hunks)
Additional comments not posted (2)
web/src/hooks/queries/useUserSettings.tsx (1)
27-27
: Proper implementation of error handling usingreact-toastify
.Ensure that the
OPTIONS
object fromutils/wrapWithToast
is appropriately configured to handle the toast appearance and behavior as expected.web/src/pages/NewTransaction/NavigationButtons/NextButton.tsx (1)
92-92
: Updated email validation logic to include empty string check.Ensure this logic works as expected in scenarios where the email is empty, a valid email, or invalid. Consider adding unit tests to cover these cases.
Verification successful
The provided context confirms that the email validation logic has been updated to include an empty string check. This ensures that
notificationEmail
is either the same asuserSettings.email
or an empty string. This change appears to be a good practice to ensure robustness.The new logic is as follows:
![userSettings.email, ""].includes(notificationEmail)This checks if
notificationEmail
is neither the user's saved email nor an empty string, which is a valid check for ensuring a proper email is provided.Here are the key points from the code:
notificationEmail
is checked againstuserSettings.email
and an empty string.- The logic is part of a broader validation that includes other fields and conditions.
The change aligns with the expected behavior from a user experience perspective, ensuring that invalid or empty emails are handled correctly.
Next Steps
To ensure comprehensive validation, it would be beneficial to add unit tests covering various scenarios, such as:
notificationEmail
is the same asuserSettings.email
.notificationEmail
is an empty string.notificationEmail
is a valid email but different fromuserSettings.email
.notificationEmail
is an invalid email.These tests will help confirm that the logic works as expected in all edge cases.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the email validation logic correctly handles various edge cases. # Test: Check if the empty string and undefined cases are handled correctly. cat web/src/pages/NewTransaction/NavigationButtons/NextButton.tsx | grep -A 20 'notificationEmail'Length of output: 2409
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
PR-Codex overview
This PR adds
transactionHash
to various components and functions, updates button logic, and improves address display in the UI.Detailed summary
transactionHash
to components and functionsSummary by CodeRabbit
New Features
ExecuteTransactionButton
component for executing transactions.PreviewCard
andTransactionInfo
components to display and copy ENS names for addresses.Improvements
PreviewCard
component now includestransactionHash
.AcceptSettlementButton
by removingtoggleModal
.Bug Fixes
shortenAddress
function to handle empty addresses.UI Enhancements
Header
component now links to Etherscan based ontransactionHash
.Error Handling
Refactor
Buttons
component toPreviewCardButtons
with conditional logic forExecuteTransactionButton
.GraphQL Schema
transactionHash
field toEscrow
type.