Feat/batch disputes #1982
Feat/batch disputes #1982
Conversation
✅ Deploy Preview for kleros-v2-testnet ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
❌ Deploy Preview for kleros-v2-neo failed. Why did it fail? →
|
✅ Deploy Preview for kleros-v2-testnet-devtools ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
WalkthroughThis update introduces batch dispute creation to the Resolver feature, enabling users to create multiple identical disputes simultaneously. It adds new UI controls, state management, and transaction batching logic to support this capability, along with related routing, context, and hook enhancements throughout the application. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ResolverUI
participant NewDisputeContext
participant SubmitBatchDisputesButton
participant useTransactionBatcher
participant Blockchain
User->>ResolverUI: Navigates to Resolver landing page
ResolverUI->>NewDisputeContext: Initialize batch state
User->>ResolverUI: Enables "Create batch cases" and sets batch size
ResolverUI->>NewDisputeContext: Update isBatchCreation and batchSize
User->>SubmitBatchDisputesButton: Clicks "Submit batch"
SubmitBatchDisputesButton->>useTransactionBatcher: Prepare batch transactions
useTransactionBatcher->>Blockchain: Simulate and send batch
Blockchain-->>useTransactionBatcher: Return transaction result
useTransactionBatcher-->>SubmitBatchDisputesButton: Provide result/error
SubmitBatchDisputesButton->>NewDisputeContext: Reset dispute data on success
SubmitBatchDisputesButton->>ResolverUI: Navigate to cases page
Assessment against linked issues
Suggested labels
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 📜 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 (14)
🪧 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 (
|
❌ Deploy Preview for kleros-v2-university failed. Why did it fail? →
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (14)
web/src/pages/Resolver/NavigationButtons/SubmitDisputeButton.tsx (1)
93-93: Complex loading condition might benefit from extraction.While functional, the loading condition combines multiple states and a negation, making it harder to read at a glance.
Consider extracting this complex condition to a named variable for better readability:
- isLoading={(isSubmittingCase || isBalanceLoading || isLoadingConfig) && !insufficientBalance} + isLoading={isLoadingState && !insufficientBalance}With a useMemo above:
const isLoadingState = useMemo( () => isSubmittingCase || isBalanceLoading || isLoadingConfig, [isSubmittingCase, isBalanceLoading, isLoadingConfig] );web/src/hooks/queries/useRoundDetailsQuery.ts (1)
25-36: Query configuration could be more structured.While the implementation is functional, the query key structure could be improved.
Consider using a more structured query key array for better cache management:
- queryKey: [`roundDetailsQuery${disputeId}-${roundIndex}`], + queryKey: ['roundDetails', disputeId, roundIndex],This format is more standard in React Query and makes cache invalidation and management more predictable.
web/src/pages/AttachmentDisplay/index.tsx (2)
56-57: Use nullish coalescing for URL parameter validation.Good defensive programming with the null check for the title, but the URL parameter is only checked with a conditional render. Consider a more robust approach for both parameters.
- const url = searchParams.get("url"); + const url = searchParams.get("url") ?? ""; const title = searchParams.get("title") ?? "Attachment";
62-77: Consider adding a "No attachment URL provided" message.Currently, when no URL is provided, nothing is rendered. Consider adding a user-friendly message when the URL is missing.
{url ? ( <> <StyledExternalLink to={url} rel="noreferrer" target="_blank"> Open in new tab <StyledNewTabIcon /> </StyledExternalLink> <Suspense fallback={ <LoaderContainer> <Loader width={"48px"} height={"48px"} /> </LoaderContainer> } > <FileViewer url={url} /> </Suspense> </> - ) : null} + ) : ( + <div>No attachment URL provided.</div> + )}web/src/components/EnsureAuth.tsx (1)
48-50: Consider adding a default message for better UX.The message is only shown if provided. Consider adding a default message to guide users consistently.
- {message ? <StyledInfo>{message}</StyledInfo> : null} + <StyledInfo>{message ?? "Please sign in to continue"}</StyledInfo>web/src/pages/Resolver/index.tsx (1)
108-113: Consider extracting conditional rendering to a separate component.The conditional rendering of the heading and paragraph adds complexity to the main component. Consider extracting this to a separate component for better maintainability.
+const WelcomeMessage = () => ( + <> + <Heading>Justise as a Service</Heading> + <Paragraph>You send your disputes. Kleros sends back decisions.</Paragraph> + </> +); // Then in the main component: -{!isConnected || !isVerified ? ( - <> - <Heading>Justise as a Service</Heading> - <Paragraph>You send your disputes. Kleros sends back decisions.</Paragraph> - </> -) : null} +{!isConnected || !isVerified ? <WelcomeMessage /> : null}web/src/pages/Resolver/Landing.tsx (3)
113-114: Fix typo in label text.There's a typo in the label text: "exiting" should be "existing".
- label="Duplicate an exiting case." + label="Duplicate an existing case."
50-53: Add loading state indicator.Consider adding a loading indicator when fetching dispute data to improve the user experience.
// Add a new state variable const [isLoadingDispute, setIsLoadingDispute] = useState(false); // Update the debounce effect useDebounce(() => { if (disputeID) { setIsLoadingDispute(true); } setDebouncedDisputeID(disputeID); }, 500, [disputeID]); // Update the useEffect useEffect(() => { if (isUndefined(populatedDispute) || isUndefined(roundData) || isInvalidDispute) { setIsLoadingDispute(false); return; } // Rest of the useEffect... setIsLoadingDispute(false); }, [populatedDispute, roundData, isInvalidDispute, setDisputeData]); // Add loading indicator to UI {isLoadingDispute && <Loader width={"24px"} height={"24px"} />}
122-123: Show more specific error messages.The current error message is generic. Consider providing more specific error messages based on the type of error encountered.
- {isInvalidDispute ? <ErrorMsg>Error loading dispute data. Please use another dispute.</ErrorMsg> : null} + {isInvalidDispute && isErrorPopulatedDisputeQuery ? ( + <ErrorMsg>Error loading dispute data. The dispute may not exist or is invalid.</ErrorMsg> + ) : isInvalidDispute && isErrorRoundQuery ? ( + <ErrorMsg>Error loading round data. Please try another dispute.</ErrorMsg> + ) : isInvalidDispute ? ( + <ErrorMsg>Error loading dispute data. Please use another dispute.</ErrorMsg> + ) : null}web/src/pages/Resolver/NavigationButtons/SubmitBatchDisputesButton.tsx (2)
35-38: Improve balance check logic.The current insufficient balance calculation has a potential issue where it falls back to MIN_DISPUTE_BATCH_SIZE when batchSize is null. While this provides a safety net, it would be clearer to ensure batchSize is always a valid number before this calculation.
const insufficientBalance = useMemo(() => { const arbitrationCost = disputeData.arbitrationCost ? BigInt(disputeData.arbitrationCost) : BigInt(0); - return userBalance && userBalance.value < arbitrationCost * BigInt(batchSize ?? MIN_DISPUTE_BATCH_SIZE); + const effectiveBatchSize = batchSize ?? MIN_DISPUTE_BATCH_SIZE; + return userBalance && userBalance.value < arbitrationCost * BigInt(effectiveBatchSize); }, [userBalance, disputeData, batchSize]);
91-92: Enhance loading state clarity.The current loading state combines multiple loading conditions which might be confusing to users. Consider separating these into more specific states or providing more detailed feedback about what's happening.
- isLoading={(isSubmittingCase || isBalanceLoading || isLoadingConfig) && !insufficientBalance} + isLoading={(isSubmittingCase || isBalanceLoading || isLoadingConfig) && !insufficientBalance} + loadingText={isSubmittingCase ? "Submitting cases..." : isBalanceLoading ? "Checking balance..." : "Preparing transaction..."}web/src/pages/Resolver/Preview/index.tsx (2)
121-129: Enhance batch creation tooltip with more detailed explanation.The current tooltip message "Create multiple cases with same data" is quite brief. Consider providing more context about when and why a user might want to use batch creation.
- <WithHelpTooltip tooltipMsg="Create multiple cases with same data."> + <WithHelpTooltip tooltipMsg="Create multiple identical cases with the same dispute data. This is useful when you need to submit several similar disputes and want to save time and potentially gas costs.">
133-134: Add explanation for the minimum batch size requirement.The PlusMinusField has a minimum value of 2 but doesn't explain why this constraint exists. Consider adding a note or extending the field label to clarify this constraint.
- <FieldLabel>Number of cases to be created: {batchSize}</FieldLabel> + <FieldLabel>Number of cases to be created: {batchSize} (minimum 2)</FieldLabel>web/src/context/NewDisputeContext.tsx (1)
100-101: Consider user notification for persisted batch settings.The batch size is stored in localStorage which means it persists across sessions. If a user comes back days later, they might not remember they had batch creation enabled.
Consider adding a notification or visual indicator when batch creation settings are loaded from localStorage, especially if the values are non-default.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
web/src/app.tsx(2 hunks)web/src/components/DisputePreview/Policies.tsx(1 hunks)web/src/components/EnsureAuth.tsx(2 hunks)web/src/components/EvidenceCard.tsx(1 hunks)web/src/context/NewDisputeContext.tsx(4 hunks)web/src/hooks/queries/usePopulatedDisputeData.ts(1 hunks)web/src/hooks/queries/useRoundDetailsQuery.ts(1 hunks)web/src/hooks/useTransactionBatcher.tsx(3 hunks)web/src/pages/AttachmentDisplay/Header.tsx(2 hunks)web/src/pages/AttachmentDisplay/index.tsx(1 hunks)web/src/pages/Cases/AttachmentDisplay/index.tsx(0 hunks)web/src/pages/Cases/index.tsx(1 hunks)web/src/pages/Resolver/Landing.tsx(1 hunks)web/src/pages/Resolver/NavigationButtons/SubmitBatchDisputesButton.tsx(1 hunks)web/src/pages/Resolver/NavigationButtons/SubmitDisputeButton.tsx(4 hunks)web/src/pages/Resolver/NavigationButtons/index.tsx(2 hunks)web/src/pages/Resolver/Parameters/NotablePersons/PersonFields.tsx(4 hunks)web/src/pages/Resolver/Policy/index.tsx(3 hunks)web/src/pages/Resolver/Preview/index.tsx(5 hunks)web/src/pages/Resolver/index.tsx(4 hunks)
💤 Files with no reviewable changes (1)
- web/src/pages/Cases/AttachmentDisplay/index.tsx
🧰 Additional context used
🧬 Code Graph Analysis (8)
web/src/pages/Resolver/NavigationButtons/index.tsx (1)
web/src/context/NewDisputeContext.tsx (1)
useNewDisputeContext(88-94)
web/src/pages/Resolver/Parameters/NotablePersons/PersonFields.tsx (1)
web-devtools/src/utils/validateAddressOrEns.ts (1)
validateAddress(4-14)
web/src/pages/AttachmentDisplay/index.tsx (2)
web/src/styles/landscapeStyle.ts (1)
MAX_WIDTH_LANDSCAPE(3-3)web/src/components/ExternalLink.tsx (1)
ExternalLink(5-9)
web/src/pages/Resolver/Policy/index.tsx (4)
web/src/components/InternalLink.tsx (1)
InternalLink(4-8)web/src/styles/commonStyles.ts (1)
hoverShortTransitionTiming(3-5)web/src/utils/index.ts (2)
getFileUploaderMsg(24-39)isUndefined(5-6)kleros-app/src/utils/index.ts (1)
isUndefined(1-2)
web/src/pages/Resolver/index.tsx (1)
kleros-app/src/lib/atlas/providers/AtlasProvider.tsx (1)
useAtlasProvider(367-373)
web/src/hooks/queries/useRoundDetailsQuery.ts (1)
web/src/consts/index.ts (1)
STALE_TIME(10-10)
web/src/components/EnsureAuth.tsx (1)
kleros-app/src/lib/atlas/providers/AtlasProvider.tsx (1)
useAtlasProvider(367-373)
web/src/context/NewDisputeContext.tsx (1)
web-devtools/src/hooks/useLocalStorage.ts (1)
useLocalStorage(3-22)
⏰ Context from checks skipped due to timeout of 90000ms (12)
- GitHub Check: Redirect rules - kleros-v2-neo
- GitHub Check: Header rules - kleros-v2-neo
- GitHub Check: Pages changed - kleros-v2-neo
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Redirect rules - kleros-v2-university
- GitHub Check: Header rules - kleros-v2-university
- GitHub Check: Pages changed - kleros-v2-university
🔇 Additional comments (37)
web/src/hooks/useTransactionBatcher.tsx (4)
40-42: Good addition of total value calculationThe calculation correctly aggregates all transaction values into a single
BigInt, properly handling undefined values by defaulting to zero.
48-48: Good exposure of error detailsIncluding the error from the simulation hook will help with debugging and improved error handling in components using this hook.
58-58: Correct application of totalValueUsing the aggregated
totalValuefor the simulation ensures accurate gas estimation and proper transaction handling.
67-67: API updated properlyThe hook's return value now includes the error object, completing the proper API extension.
web/src/hooks/queries/usePopulatedDisputeData.ts (1)
9-9: Import reordering is fineThis change is purely stylistic and has no functional impact on the code.
web/src/pages/AttachmentDisplay/Header.tsx (2)
4-4: Simplified importsGood cleanup by removing unused imports and only keeping what's needed.
67-67: Improved component designRefactoring the component to accept a
titleprop directly makes it more reusable and follows good component design practices by moving control to the parent component.web/src/app.tsx (2)
29-29: Proper import addedThe import for the new AttachmentDisplay component is correctly added.
108-115: Well-structured route additionThe new route for attachments follows the established pattern used throughout the application, including proper usage of Suspense and fallback loader.
web/src/pages/Cases/index.tsx (1)
4-5: LGTM!The reorganization of imports is clean and maintains the same functionality.
web/src/pages/Resolver/Parameters/NotablePersons/PersonFields.tsx (2)
47-47: LGTM!Adding this guard clause is a good defensive programming practice to prevent errors when
publicClientis undefined.
88-88: LGTM!Converting the short-circuit expression to an explicit
ifstatement improves readability.web/src/pages/Resolver/NavigationButtons/index.tsx (4)
4-6: Good imports organization for context and utilities.The additions of
useNewDisputeContextandisUndefinedutility are appropriate for implementing the conditional rendering logic needed for batch dispute creation.
12-12: New component import aligns with batch features requirement.Importing the new
SubmitBatchDisputesButtoncomponent supports the batch dispute creation capability mentioned in the PR objectives.
29-31: Clean implementation of conditional button selection.Good use of the context to determine if batch creation is active, and elegant conditional assignment of the appropriate button component based on this flag.
35-35: Improved conditional rendering logic.Using the
isUndefinedutility creates cleaner code than null checks. The conditional logic appropriately renders either the submit button or navigation button based on the presence of a next route.web/src/pages/Resolver/NavigationButtons/SubmitDisputeButton.tsx (4)
46-51: Enhanced error and loading state management.Good restructuring of the hook return values, with improved naming (
isLoadingConfiginstead ofisLoading) and addingisErrorfor better error state handling.
67-74: Comprehensive button disabled state logic.The button disabled state now correctly includes all possible disabling conditions, including the new
isErrorflag. The dependency array is properly updated to include all dependencies.
95-95: Improved safety with publicClient check.The additional check for
publicClientexistence before proceeding with submission prevents potential runtime errors.
134-143: Good function export for template validation.Exporting the
isTemplateValidfunction allows for reuse in other components, promoting code reusability and consistency in validation logic.web/src/hooks/queries/useRoundDetailsQuery.ts (3)
10-19: Well-structured GraphQL query definition.The query is clearly defined and focuses on retrieving only the necessary data (court ID and vote count).
21-23: Good conditional query enablement.The hook correctly checks for the presence of both required parameters before enabling the query execution.
31-35: Efficient use of GraphQL batching.Good use of the batching context with a unique ID for each request, which helps optimize network requests.
web/src/pages/Resolver/Policy/index.tsx (2)
62-79: Well-styled components with proper hover effects.The styled components follow project patterns and implement clean hover transitions. Good use of theme colors and SVG styling.
116-121: Enhances user experience with policy file inspection.Adding the ability to inspect uploaded policy files directly aligns with the PR objective of improving the Policy tab, especially useful when a dispute is duplicated and the policy file needs to be reviewed.
web/src/pages/AttachmentDisplay/index.tsx (1)
17-25: Good use of responsive styling.The responsive container with dynamic padding based on viewport width provides an excellent user experience across different device sizes.
web/src/components/EnsureAuth.tsx (2)
26-27: Good addition of optional props for customizability.Adding
messageandbuttonTextprops enhances the component's flexibility and reusability across different contexts.
11-23: Well-structured component styling.The styled components have appropriate spacing and alignment properties. The
StyledInfocomponent properly resets margins and padding.web/src/pages/Resolver/index.tsx (3)
80-94: Good addition of styled typography components.The styled
HeadingandParagraphcomponents provide consistent typography with proper theme integration.
128-129: Good update to routing structure.The route changes to include a landing page and redirecting the index to
/createprovide a better user flow for initiating dispute creation.
115-115: Good use of customized button text.Providing custom button text to the
EnsureAuthcomponent leverages the new prop added to that component.web/src/pages/Resolver/Landing.tsx (3)
65-68: Good validation logic for dispute existence.The validation logic to check for invalid disputes is comprehensive, considering loading state, errors, and data existence.
115-120: Good input sanitization for numeric values.The input sanitization to allow only numeric characters is appropriate for a dispute ID field.
70-99:⚠️ Potential issueRemove disputeData from useEffect dependencies.
Including
disputeDatain the dependencies array while also setting it inside the effect could potentially cause infinite loops. Remove it from the dependencies since you're already spreading its previous value.- }, [populatedDispute, roundData, isInvalidDispute]); + }, [populatedDispute, roundData, isInvalidDispute, setDisputeData]);Likely an incorrect or invalid review comment.
web/src/pages/Resolver/NavigationButtons/SubmitBatchDisputesButton.tsx (1)
95-98:✅ Verification successful
Enhance transaction success verification.
The current implementation only checks if
res.statusis truthy andres.resultis not undefined. For blockchain transactions, it's important to verify the transaction was actually mined and confirmed, not just submitted.
🏁 Script executed:
#!/bin/bash # Check how transaction results are handled in other parts of the codebase rg "wrapWithToast.*then.*status" --type ts --type tsxLength of output: 83
I’ll search for all
wrapWithToastusages without relying on file-type flags:
🏁 Script executed:
#!/bin/bash # Find every wrapWithToast call in .ts and .tsx files and show a few lines of context rg "wrapWithToast" -g "*.ts" -g "*.tsx" -n -A 5Length of output: 28627
To verify how
wrapWithToastdetermines success and whether it awaits confirmation, let’s inspect its implementation:
🏁 Script executed:
#!/bin/bash # Show the wrapWithToast implementation sed -n '1,200p' web/src/utils/wrapWithToast.tsLength of output: 1671
No changes needed—
wrapWithToastalready awaits transaction confirmations.The
wrapWithToasthelper callspublicClient.waitForTransactionReceipt({ hash, confirmations: 2 })and checksres.status === "success"before resolving. Your.then((res) => …)logic inSubmitBatchDisputesButton.tsxtherefore operates on confirmed, mined transactions.web/src/pages/Resolver/Preview/index.tsx (1)
86-93: Clarify the purpose of the Overlay div.The Overlay div is positioned absolutely over the StyledCard but doesn't have any visible content or clear purpose. This could interfere with user interactions with the card content.
Is this Overlay meant to block interactions with the card when batch mode is enabled? If so, it should have some visual indication or conditional rendering based on the isBatchCreation state.
web/src/context/NewDisputeContext.tsx (1)
113-119: Review the route change detection logic.The current implementation resets all dispute data when navigating away from routes containing "/resolver" or "/attachment". This approach might be too broad and could lead to data loss in unexpected scenarios.
Consider using a more specific route matching pattern or adding additional checks to ensure data is only reset when appropriate. For example, you might want to preserve data when navigating between different resolver screens.
web/src/pages/Resolver/NavigationButtons/SubmitBatchDisputesButton.tsx
Outdated
Show resolved
Hide resolved
|
Code Climate has analyzed commit d5547dd and detected 12 issues on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
|
closes #1970 #1971
PR-Codex overview
This PR focuses on enhancing the
AttachmentDisplayfeature in a dispute resolution application, improving how attachments are handled and displayed, and adding batch dispute creation functionality.Detailed summary
AttachmentDisplaycomponent.Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Refactor