-
Notifications
You must be signed in to change notification settings - Fork 4
fix: no balance error in hooks #81
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
Conversation
✅ Deploy Preview for kleros-escrow-v2 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
WalkthroughThe Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration 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: 3
🧹 Outside diff range and nitpick comments (1)
web/src/pages/MyTransactions/TransactionDetails/PreviewCardButtons/RaiseDisputeButton.tsx (1)
73-73
: Use optional chaining for cleaner conditional function callsInstead of using the logical AND operator to conditionally call
toggleModal
, you can utilize optional chaining for a more concise and readable solution.Update the code:
- toggleModal && toggleModal(); + toggleModal?.();Also applies to: 105-105
🧰 Tools
🪛 Biome
[error] 73-73: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- web/src/pages/MyTransactions/TransactionDetails/PreviewCardButtons/RaiseDisputeButton.tsx (2 hunks)
🧰 Additional context used
🪛 Biome
web/src/pages/MyTransactions/TransactionDetails/PreviewCardButtons/RaiseDisputeButton.tsx
[error] 73-73: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 105-105: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (1)
web/src/pages/MyTransactions/TransactionDetails/PreviewCardButtons/RaiseDisputeButton.tsx (1)
73-73
: Adhere to static analysis recommendationsThe static analysis tool suggests changing to an optional chain at lines 73 and 105. This improves code clarity and aligns with best practices.
The recommended change was already addressed in a previous comment.
Also applies to: 105-105
🧰 Tools
🪛 Biome
[error] 73-73: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
if (isBuyer) { | ||
if (simulateBuyerError) { | ||
console.log({simulateBuyerError}); | ||
let errorMessage = "An error occurred during simulation."; | ||
if (simulateBuyerError.message.includes("insufficient funds")) { | ||
errorMessage = "You don't have enough balance to raise a dispute."; | ||
} | ||
// Use wrapWithToast to display the error | ||
wrapWithToast(() => Promise.reject(new Error(errorMessage)), publicClient); | ||
return; | ||
} | ||
|
||
if (isUndefined(payArbitrationFeeByBuyerConfig)) { | ||
console.log({ payArbitrationFeeByBuyerConfig }); | ||
console.log({ simulateBuyerError }); | ||
wrapWithToast(() => Promise.reject(new Error("Unable to prepare transaction.")), publicClient); | ||
return; | ||
} | ||
|
||
if (!isUndefined(payArbitrationFeeByBuyer)) { | ||
setIsSending(true); | ||
wrapWithToast(() => payArbitrationFeeByBuyer(payArbitrationFeeByBuyerConfig.request), publicClient) | ||
.then((wrapResult) => { | ||
setIsSending(false); | ||
if (wrapResult.status) { | ||
toggleModal && toggleModal(); | ||
refetchQuery([["refetchOnBlock", "useTransactionDetailsQuery"]]); | ||
} | ||
}) | ||
.catch((error) => { | ||
console.error("Error raising dispute as buyer:", error); | ||
setIsSending(false); | ||
}); | ||
} |
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.
🛠️ Refactor suggestion
Refactor to eliminate duplicated code in buyer and seller dispute handling
The logic within the buyer and seller sections is nearly identical. Refactoring this code can improve maintainability and reduce the potential for errors due to code duplication.
Consider abstracting the common logic into a single function:
const handleRaiseDispute = () => {
const isBuyerOrSeller = isBuyer ? 'Buyer' : 'Seller';
const simulateError = isBuyer ? simulateBuyerError : simulateSellerError;
const payArbitrationFeeConfig = isBuyer ? payArbitrationFeeByBuyerConfig : payArbitrationFeeBySellerConfig;
const payArbitrationFee = isBuyer ? payArbitrationFeeByBuyer : payArbitrationFeeBySeller;
if (simulateError) {
console.log({ simulateError });
let errorMessage = "An error occurred during simulation.";
if (simulateError?.message?.includes("insufficient funds")) {
errorMessage = "You don't have enough balance to raise a dispute.";
}
wrapWithToast(() => Promise.reject(new Error(errorMessage)), publicClient);
return;
}
if (isUndefined(payArbitrationFeeConfig)) {
console.log({ payArbitrationFeeConfig });
console.log({ simulateError });
wrapWithToast(() => Promise.reject(new Error("Unable to prepare transaction.")), publicClient);
return;
}
if (!isUndefined(payArbitrationFee)) {
setIsSending(true);
wrapWithToast(() => payArbitrationFee(payArbitrationFeeConfig.request), publicClient)
.then((wrapResult) => {
setIsSending(false);
if (wrapResult.status) {
toggleModal?.();
refetchQuery([["refetchOnBlock", "useTransactionDetailsQuery"]]);
}
})
.catch((error) => {
console.error(`Error raising dispute as ${isBuyerOrSeller.toLowerCase()}:`, error);
setIsSending(false);
});
}
};
Also applies to: 83-113
🧰 Tools
🪛 Biome
[error] 73-73: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
if (simulateSellerError.message.includes("insufficient funds")) { | ||
errorMessage = "You don't have enough balance to raise a dispute."; | ||
} |
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.
Ensure safe access to 'message' property of 'simulateSellerError'
Similarly, in the error handling for simulateSellerError
, it's important to verify that simulateSellerError
and simulateSellerError.message
exist before accessing them to prevent potential runtime errors.
Modify the code as follows:
- if (simulateSellerError.message.includes("insufficient funds")) {
+ if (simulateSellerError?.message?.includes("insufficient funds")) {
📝 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.
if (simulateSellerError.message.includes("insufficient funds")) { | |
errorMessage = "You don't have enough balance to raise a dispute."; | |
} | |
if (simulateSellerError?.message?.includes("insufficient funds")) { | |
errorMessage = "You don't have enough balance to raise a dispute."; | |
} |
if (simulateBuyerError) { | ||
console.log({simulateBuyerError}); | ||
let errorMessage = "An error occurred during simulation."; | ||
if (simulateBuyerError.message.includes("insufficient funds")) { | ||
errorMessage = "You don't have enough balance to raise a dispute."; | ||
} |
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.
Ensure safe access to 'message' property of 'simulateBuyerError'
In the error handling for simulateBuyerError
, accessing simulateBuyerError.message
without confirming its existence could lead to runtime errors if simulateBuyerError
is undefined
or if message
is not present.
Apply the following change to safely access the message
property:
- if (simulateBuyerError.message.includes("insufficient funds")) {
+ if (simulateBuyerError?.message?.includes("insufficient funds")) {
📝 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.
if (simulateBuyerError) { | |
console.log({simulateBuyerError}); | |
let errorMessage = "An error occurred during simulation."; | |
if (simulateBuyerError.message.includes("insufficient funds")) { | |
errorMessage = "You don't have enough balance to raise a dispute."; | |
} | |
if (simulateBuyerError) { | |
console.log({simulateBuyerError}); | |
let errorMessage = "An error occurred during simulation."; | |
if (simulateBuyerError?.message?.includes("insufficient funds")) { | |
errorMessage = "You don't have enough balance to raise a dispute."; | |
} |
continued in #80 |
PR-Codex overview
This PR enhances the
RaiseDisputeButton
component by adding loading states for the arbitration fee configurations based on whether the user is a buyer or a seller, improving user experience during asynchronous operations.Detailed summary
isPreparingBuyerConfig
andisPreparingSellerConfig
for buyer and seller arbitration fee configurations.useSimulateEscrowUniversalPayArbitrationFeeByBuyer
anduseSimulateEscrowUniversalPayArbitrationFeeBySeller
hooks to include aquery
object for enabling/disabling based on user role.Button
component to reflect loading states for both buyer and seller configurations.Summary by CodeRabbit
New Features
Bug Fixes