Skip to content

fix: success view for request and claim#409

Merged
Hugo0 merged 4 commits intodevelopfrom
fix/request-payment-success
Oct 3, 2024
Merged

fix: success view for request and claim#409
Hugo0 merged 4 commits intodevelopfrom
fix/request-payment-success

Conversation

@jjramirezn
Copy link
Copy Markdown
Contributor

@jjramirezn jjramirezn commented Oct 3, 2024

Fixes

  1. For both claim and request, the destination address was not being fetched because utils was not being imported. This fixes that by importing it
  2. For request view, the source chain was wrong, now it uses the selected one (because when paying a request the selected chain is the source one)

Chores

  • Remove some ts-ignore
  • Rename loopUntilSuccessful to more self-explaining fetchDestinationChain

Summary by CodeRabbit

  • New Features

    • Improved transaction handling in the SuccessClaimLinkView and SuccessView components, enhancing the fetching of destination chain transaction details.
    • Introduced new structured types for better handling of blockchain data and transaction statuses.
  • Bug Fixes

    • Updated logic for fetching transaction URLs to ensure accurate linking based on the current context.
  • Refactor

    • Replaced outdated methods with more efficient alternatives for fetching transaction data.
    • Streamlined component logic for better performance and clarity.

@vercel
Copy link
Copy Markdown

vercel bot commented Oct 3, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
peanut-ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 3, 2024 0:37am

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Oct 3, 2024

📝 Walkthrough

Walkthrough

The pull request introduces modifications to several components related to transaction handling in a blockchain context. The loopUntilSuccess function is replaced by fetchDestinationChain in both the SuccessClaimLinkView and SuccessView components, altering how destination chain transaction details are fetched. Additionally, new types are defined in the utility functions to enhance the structure of transaction status responses. These changes streamline the logic for fetching and displaying transaction details across the components.

Changes

File Change Summary
src/components/Claim/Link/Onchain/Success.view.tsx Replaced loopUntilSuccess with fetchDestinationChain in useEffect for fetching transaction details.
src/components/Request/Pay/Views/Success.view.tsx Replaced loopUntilSuccess with fetchDestinationChain; removed explorerUrlWithTx, added sourceUrlWithTx. Updated useEffect and rendering logic accordingly.
src/components/utils/utils.ts Defined new types ISquidChainData and ISquidStatusResponse. Updated checkTransactionStatus to return a promise of ISquidStatusResponse. Renamed loopUntilSuccess to fetchDestinationChain and updated its logic.

Possibly related PRs

  • Integrate UI with request links #375: Changes in Success.view.tsx involve replacing loopUntilSuccess with fetchDestinationChain, directly related to the changes in SuccessClaimLinkView.
  • Fix build #377: Modifications in the useClaimLink hook include changes to switchNetworkUtil, potentially relating to transaction handling logic impacted by the main PR.
  • Fix fetching tx fee #379: Updates in Initial.view.tsx enhance transaction handling and fee estimation, aligning with changes in fetching transaction details.
  • fix: request success message #392: Changes in Success.view.tsx focus on user feedback related to payment links, relevant to the overall transaction success flow.
  • fix: remaining ux issues #401: Modifications to the TokenSelector component may indirectly relate to user experience improvements in transaction handling and success messaging.

Suggested labels

Being worked on

Suggested reviewers

  • Hugo0

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?

❤️ 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.

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 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
Copy Markdown
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: 3

🧹 Outside diff range and nitpick comments (3)
src/components/Claim/Link/Onchain/Success.view.tsx (1)

9-9: LGTM! Consider grouping related imports.

The import of fetchDestinationChain addresses the issue mentioned in the PR objectives. Good job on fixing the missing utility function import.

Consider grouping this import with other utility imports for better code organization. For example, you could move it near the import * as utils from '@/utils' line.

src/components/utils/utils.ts (2)

Line range hint 88-106: Add error handling in 'fetchDestinationChain' to handle exceptions from 'checkTransactionStatus'

The asynchronous call to checkTransactionStatus inside setInterval does not handle exceptions. If an error occurs (e.g., network failure), it could cause the interval function to stop executing without clearing the interval, potentially leading to unintended behavior. Wrap the call in a try-catch block to handle errors gracefully and ensure that clearInterval is called when necessary.

Apply this diff to fix the issue:

 export async function fetchDestinationChain(
     txHash: string,
     setExplorerUrlDestChainWithTxHash: (value: { transactionId: string; transactionUrl: string }) => void
 ) {
-    let intervalId = setInterval(async () => {
+    let intervalId = setInterval(async () => {
+       try {
         const result = await checkTransactionStatus(txHash)
 
         if (result.squidTransactionStatus === 'success') {
             const explorerUrl = utils.getExplorerUrl(result.toChain.chainData.chainId.toString())
             if (explorerUrl) {
                 setExplorerUrlDestChainWithTxHash({
                     transactionUrl: explorerUrl + '/tx/' + result.toChain.transactionId,
                     transactionId: result.toChain.transactionId,
                 })
             } else {
                 setExplorerUrlDestChainWithTxHash({
                     transactionUrl: result.toChain.transactionUrl,
                     transactionId: result.toChain.transactionId,
                 })
             }
             clearInterval(intervalId)
         } else {
             console.log('Checking status again...')
         }
+       } catch (error) {
+           console.error('Error fetching transaction status:', error)
+           clearInterval(intervalId)
+       }
     }, 5000)
 }

86-86: Use 'const' instead of 'let' for 'intervalId'

Since intervalId is not reassigned after initialization, using const improves code readability and indicates that the variable will not change.

Apply this diff:

 export async function fetchDestinationChain(
     txHash: string,
     setExplorerUrlDestChainWithTxHash: (value: { transactionId: string; transactionUrl: string }) => void
 ) {
-    let intervalId = setInterval(async () => {
+    const intervalId = setInterval(async () => {
         // existing code
     }, 5000)
 }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 02de9e5 and 4510cb5.

📒 Files selected for processing (3)
  • src/components/Claim/Link/Onchain/Success.view.tsx (2 hunks)
  • src/components/Request/Pay/Views/Success.view.tsx (2 hunks)
  • src/components/utils/utils.ts (2 hunks)
🔇 Additional comments (5)
src/components/Claim/Link/Onchain/Success.view.tsx (1)

49-49: LGTM! Verify the implementation of fetchDestinationChain.

The replacement of loopUntilSuccess with fetchDestinationChain improves code clarity by using a more descriptive function name. This change aligns with the PR objectives.

To ensure the new function works as expected, please run the following script to verify its implementation:

✅ Verification successful

LGTM! The replacement of loopUntilSuccess with fetchDestinationChain has been successfully verified.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the implementation of fetchDestinationChain function

# Test: Check if the function is defined and exported correctly
ast-grep --lang typescript --pattern 'export function fetchDestinationChain($_,$_) { $$$ }'

# Test: Ensure the function is using the correct parameters
ast-grep --lang typescript --pattern 'function fetchDestinationChain(transactionHash, setExplorerUrlDestChainWithTxHash) { $$$ }'

# Test: Check for any remaining references to loopUntilSuccess
rg 'loopUntilSuccess'

Length of output: 2183


Script:

#!/bin/bash
# Description: Verify usage of fetchDestinationChain in Success.view.tsx

# Test: Check if fetchDestinationChain is imported correctly
ast-grep --lang typescript --pattern 'import { fetchDestinationChain } from $_' src/components/Claim/Link/Onchain/Success.view.tsx

# Test: Ensure fetchDestinationChain is called with the correct parameters
ast-grep --lang typescript --pattern 'fetchDestinationChain(transactionHash, setExplorerUrlDestChainWithTxHash)' src/components/Claim/Link/Onchain/Success.view.tsx

# Test: Confirm there are no references to loopUntilSuccess in Success.view.tsx
rg 'loopUntilSuccess' src/components/Claim/Link/Onchain/Success.view.tsx

Length of output: 716


Script:

#!/bin/bash
# Description: Refined verification of fetchDestinationChain usage in Success.view.tsx

# Test: Check if fetchDestinationChain is imported correctly
ast-grep --lang typescript --pattern '
import { fetchDestinationChain } from $_
' src/components/Claim/Link/Onchain/Success.view.tsx

# Test: Ensure fetchDestinationChain is called with the correct parameters
ast-grep --lang typescript --pattern '
fetchDestinationChain(transactionHash, setExplorerUrlDestChainWithTxHash)
' src/components/Claim/Link/Onchain/Success.view.tsx

# Test: Confirm there are no references to loopUntilSuccess in Success.view.tsx
rg 'loopUntilSuccess' src/components/Claim/Link/Onchain/Success.view.tsx

Length of output: 720


Script:

#!/bin/bash
# Description: Refined verification of fetchDestinationChain usage in Success.view.tsx using ripgrep

# Test: Check if fetchDestinationChain is imported correctly
rg "import\s+\{\s*fetchDestinationChain\s*\}\s+from\s+['\"][^'\"]+['\"]" src/components/Claim/Link/Onchain/Success.view.tsx

# Test: Ensure fetchDestinationChain is called with the correct parameters
rg "fetchDestinationChain\s*\(\s*transactionHash\s*,\s*setExplorerUrlDestChainWithTxHash\s*\)" src/components/Claim/Link/Onchain/Success.view.tsx

# Test: Confirm there are no references to loopUntilSuccess in Success.view.tsx
rg 'loopUntilSuccess' src/components/Claim/Link/Onchain/Success.view.tsx

Length of output: 503

src/components/Request/Pay/Views/Success.view.tsx (4)

6-6: Verify the import of fetchDestinationChain.

Ensure that the fetchDestinationChain function is correctly exported from @/components/utils/utils to prevent any import errors at runtime.


10-10: Confirm availability of selectedChainID in tokenSelectorContext.

Check that selectedChainID is provided by tokenSelectorContext to avoid undefined values and potential errors when accessing the context.


11-13: Dependency array in useMemo is appropriate.

The useMemo hook correctly recalculates sourceUrlWithTx when transactionHash or selectedChainID changes, ensuring the URL stays up-to-date.


40-40: Ensure sourceUrlWithTx constructs a valid URL.

Verify that sourceUrlWithTx generates the correct source chain transaction URL using selectedChainID and transactionHash, so users are directed to the appropriate transaction details.

Comment on lines +11 to +13
const sourceUrlWithTx = useMemo(
() => `${utils.getExplorerUrl(selectedChainID)}/tx/${transactionHash}`,
[transactionHash, selectedChainID]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice

let intervalId = setInterval(async () => {
const result = await checkTransactionStatus(txHash)

//@ts-ignore
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<3

@Hugo0 Hugo0 merged commit ba36e3c into develop Oct 3, 2024
@jjramirezn jjramirezn deleted the fix/request-payment-success branch October 3, 2024 14:24
@coderabbitai coderabbitai bot mentioned this pull request Oct 24, 2024
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