feat(web): evidence moderation #1743
Conversation
WalkthroughThis pull request introduces several enhancements related to spam evidence management within the application. A new environment variable, Changes
Possibly related PRs
Suggested labels
Suggested reviewers
🪧 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 (
|
✅ Deploy Preview for kleros-v2-testnet-devtools ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for kleros-v2-university ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ 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 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (5)
web/.env.devnet.public (1)
10-10: Add documentation for the formatThe variable's value format is not immediately clear and could benefit from documentation.
+# Format: comma-separated list of case-evidence pairs (e.g., "caseId-evidenceId,caseId-evidenceId") export REACT_APP_SPAM_EVIDENCES_IDS="0-2,3-1"web/src/consts/index.ts (1)
46-46: Add documentation for the spam evidence ID formatPlease add JSDoc comments to document:
- The expected format of the environment variable
- The structure of each spam evidence ID
- Example usage
Add this documentation above the constant:
+/** + * Array of spam evidence IDs parsed from REACT_APP_SPAM_EVIDENCES_IDS environment variable. + * + * @example + * // Environment variable format: "groupId-evidenceId,groupId-evidenceId" + * REACT_APP_SPAM_EVIDENCES_IDS="0-2,3-1" + */ export const spamEvidencesIds: string[] = (import.meta.env.REACT_APP_SPAM_EVIDENCES_IDS ?? "").split(",");web/src/pages/Cases/CaseDetails/Evidence/index.tsx (3)
59-63: Consider adding hover state for better UXThe spam label would benefit from a hover state to provide better visual feedback to users.
const SpamLabel = styled.label` color: ${({ theme }) => theme.primaryBlue}; align-self: center; cursor: pointer; + &:hover { + text-decoration: underline; + } `;
88-93: Optimize evidence filteringThe current implementation makes two passes over the data array. This can be optimized to use a single pass with reduce.
const evidences = useMemo(() => { if (!data?.evidences) return; - const spamEvidences = data.evidences.filter((evidence) => isSpam(evidence.id)); - const realEvidences = data.evidences.filter((evidence) => !isSpam(evidence.id)); - return { realEvidences, spamEvidences }; + return data.evidences.reduce( + (acc, evidence) => { + if (isSpam(evidence.id)) { + acc.spamEvidences.push(evidence); + } else { + acc.realEvidences.push(evidence); + } + return acc; + }, + { realEvidences: [], spamEvidences: [] } + ); }, [data]);
Line range hint
100-130: Refactor evidence rendering for better maintainabilityThe current implementation duplicates the EvidenceCard rendering logic. Consider extracting it into a reusable component.
+const EvidenceList = ({ evidences }) => ( + evidences?.map(({ evidence, sender, timestamp, name, description, fileURI, evidenceIndex }) => ( + <EvidenceCard + key={timestamp} + index={parseInt(evidenceIndex)} + sender={sender?.id} + {...{ evidence, timestamp, name, description, fileURI }} + /> + )) +); {evidences?.realEvidences ? ( - evidences?.realEvidences.map(({ evidence, sender, timestamp, name, description, fileURI, evidenceIndex }) => ( - <EvidenceCard - key={timestamp} - index={parseInt(evidenceIndex)} - sender={sender?.id} - {...{ evidence, timestamp, name, description, fileURI }} - /> - )) + <EvidenceList evidences={evidences.realEvidences} /> ) : ( <SkeletonEvidenceCard /> )} {evidences?.spamEvidences.length !== 0 ? ( <> <Divider /> {showSpam ? ( - evidences?.spamEvidences.map( - ({ evidence, sender, timestamp, name, description, fileURI, evidenceIndex }) => ( - <EvidenceCard - key={timestamp} - index={parseInt(evidenceIndex)} - sender={sender?.id} - {...{ evidence, timestamp, name, description, fileURI }} - /> - ) - ) + <EvidenceList evidences={evidences.spamEvidences} /> ) : ( <SpamLabel onClick={() => setShowSpam(true)}>Show likely spam</SpamLabel> )} </> ) : null}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
web/.env.devnet.public(1 hunks)web/src/consts/index.ts(1 hunks)web/src/pages/Cases/CaseDetails/Evidence/index.tsx(5 hunks)
🔇 Additional comments (1)
web/src/consts/index.ts (1)
46-46: 🛠️ Refactor suggestion
Add validation and proper parsing for spam evidence IDs
The current implementation has potential issues:
- No validation of the environment variable format
- No trimming of whitespace
- The ID format (e.g., "0-2,3-1") needs parsing into a more usable structure
Consider this improved implementation:
-export const spamEvidencesIds: string[] = (import.meta.env.REACT_APP_SPAM_EVIDENCES_IDS ?? "").split(",");
+interface SpamEvidence {
+ groupId: number;
+ evidenceId: number;
+}
+
+export const spamEvidencesIds: SpamEvidence[] = (import.meta.env.REACT_APP_SPAM_EVIDENCES_IDS ?? "")
+ .split(",")
+ .filter(Boolean)
+ .map((id) => id.trim())
+ .map((id) => {
+ const [groupId, evidenceId] = id.split("-").map(Number);
+ if (isNaN(groupId) || isNaN(evidenceId)) {
+ throw new Error(`Invalid spam evidence ID format: ${id}`);
+ }
+ return { groupId, evidenceId };
+ });Let's verify how this constant is used in the Evidence component:
| @@ -7,5 +7,6 @@ export REACT_APP_GENESIS_BLOCK_ARBSEPOLIA=3084598 | |||
| export REACT_APP_ATLAS_URI=http://localhost:3000 | |||
| export REACT_APP_DEVTOOLS_URL=https://dev--kleros-v2-testnet-devtools.netlify.app | |||
| export NODE_OPTIONS='--max-old-space-size=7680' | |||
| export REACT_APP_SPAM_EVIDENCES_IDS="0-2,3-1" | |||
There was a problem hiding this comment.
💡 Codebase verification
Parsing logic needs improvement for case-evidence pairs
The current implementation in web/src/consts/index.ts only splits by comma, but the format "0-2,3-1" requires parsing both the comma-separated values and the hyphen-separated case-evidence pairs. The code in Evidence/index.tsx compares the full string without parsing the pairs properly.
web/src/consts/index.ts: Add proper parsing to split both by comma and hyphen to extract case and evidence IDsweb/src/pages/Cases/CaseDetails/Evidence/index.tsx: Update comparison logic to match against the correct case-evidence pair structure
🔗 Analysis chain
Verify the parsing logic for the evidence IDs format
The format "0-2,3-1" appears to represent case-evidence ID pairs. Ensure robust parsing in the consuming code.
Consider using a more explicit JSON format for better maintainability:
-export REACT_APP_SPAM_EVIDENCES_IDS="0-2,3-1"
+export REACT_APP_SPAM_EVIDENCES_IDS='[{"caseId":0,"evidenceId":2},{"caseId":3,"evidenceId":1}]'🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check how this environment variable is parsed and validate error handling
# Expected: Find parsing logic and error handling for the format "case-evidence,case-evidence"
# Search for direct environment variable usage
rg "REACT_APP_SPAM_EVIDENCES_IDS" -A 5
# Search for potential parsing logic
ast-grep --pattern 'split($$$)'
Length of output: 203
Script:
#!/bin/bash
# Let's check how these IDs are used in the codebase
rg "spamEvidencesIds" -A 5
# Also check for any validation or error handling around these IDs
ast-grep --pattern 'const spamEvidencesIds = $$$'
Length of output: 1065
| @@ -74,12 +83,22 @@ const Evidence: React.FC = () => { | |||
| latestEvidence.scrollIntoView({ behavior: "smooth" }); | |||
| }, [ref]); | |||
|
|
|||
| console.log({ data }); | |||
There was a problem hiding this comment.
Remove console.log statements
Console.log statements should be removed before pushing to production as they can impact performance and expose sensitive information.
- console.log({ data });
- console.log({ evidences });Also applies to: 94-94
| const isSpam = (id: string) => { | ||
| for (const spamId of spamEvidencesIds) { | ||
| if (id == spamId) return true; | ||
| } | ||
|
|
||
| return false; | ||
| }; |
There was a problem hiding this comment.
Fix equality check and optimize spam check performance
The current implementation has two issues:
- Uses loose equality (==) which could lead to unexpected behavior
- Iterates through array for each check when a Set would be more efficient
+const spamIdsSet = new Set(spamEvidencesIds);
+
const isSpam = (id: string) => {
- for (const spamId of spamEvidencesIds) {
- if (id == spamId) return true;
- }
-
- return false;
+ return spamIdsSet.has(id);
};📝 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.
| const isSpam = (id: string) => { | |
| for (const spamId of spamEvidencesIds) { | |
| if (id == spamId) return true; | |
| } | |
| return false; | |
| }; | |
| const spamIdsSet = new Set(spamEvidencesIds); | |
| const isSpam = (id: string) => { | |
| return spamIdsSet.has(id); | |
| }; |
|
Code Climate has analyzed commit 519a555 and detected 2 issues on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
web/src/pages/Cases/CaseDetails/Evidence/index.tsx (3)
71-71: Optimize the useMemo implementation.The current implementation could be improved for better type safety and performance.
Consider applying these improvements:
- const evidences = useMemo(() => { - if (!data?.evidences) return; - const spamEvidences = data.evidences.filter((evidence) => isSpam(evidence.id)); - const realEvidences = data.evidences.filter((evidence) => !isSpam(evidence.id)); - return { realEvidences, spamEvidences }; - }, [data]); + const evidences = useMemo(() => { + if (!data?.evidences) return { realEvidences: [], spamEvidences: [] }; + return { + spamEvidences: data.evidences.filter((evidence) => isSpam(evidence.id)), + realEvidences: data.evidences.filter((evidence) => !isSpam(evidence.id)) + }; + }, [data?.evidences]);Changes:
- Explicit return type for the undefined case
- More specific dependency array
- Single pass through the data array
Also applies to: 86-91
Line range hint
97-127: Consider extracting the EvidenceCard rendering logic.The EvidenceCard rendering logic is duplicated between real and spam evidences.
Consider extracting the common logic:
+ const renderEvidenceCard = useCallback( + ({ evidence, sender, timestamp, name, description, fileURI, evidenceIndex }) => ( + <EvidenceCard + key={timestamp} + index={parseInt(evidenceIndex)} + sender={sender?.id} + {...{ evidence, timestamp, name, description, fileURI }} + /> + ), + [] + ); {evidences?.realEvidences ? ( - evidences?.realEvidences.map(({ evidence, sender, timestamp, name, description, fileURI, evidenceIndex }) => ( - <EvidenceCard - key={timestamp} - index={parseInt(evidenceIndex)} - sender={sender?.id} - {...{ evidence, timestamp, name, description, fileURI }} - /> - )) + evidences.realEvidences.map(renderEvidenceCard) ) : ( <SkeletonEvidenceCard /> )} {evidences?.spamEvidences.length !== 0 && ( <> <Divider /> {showSpam ? ( - evidences?.spamEvidences.map( - ({ evidence, sender, timestamp, name, description, fileURI, evidenceIndex }) => ( - <EvidenceCard - key={timestamp} - index={parseInt(evidenceIndex)} - sender={sender?.id} - {...{ evidence, timestamp, name, description, fileURI }} - /> - ) - ) + evidences.spamEvidences.map(renderEvidenceCard) ) : ( <SpamLabel onClick={() => setShowSpam(true)}>Show likely spam</SpamLabel> )} </> )}
Error handling needs to be implemented for evidence fetching
The component currently lacks error handling for the
useEvidenceshook. While there is a global QueryClientProvider, it doesn't provide specific error handling for query failures. The component should handle potential errors to prevent silent failures and provide feedback to users.- const { data } = useEvidences(disputeData?.dispute?.externalDisputeId?.toString(), debouncedSearch); + const { data, error, isError } = useEvidences(disputeData?.dispute?.externalDisputeId?.toString(), debouncedSearch); + if (isError) { + return <div>Failed to load evidence: {error?.message}</div>; + }🔗 Analysis chain
Line range hint
64-132: Verify error handling for evidence fetching.The component should handle potential errors when fetching evidence data to prevent crashes and provide feedback to users.
Let's check if error boundaries exist in parent components:
Consider adding error handling:
const { data } = useEvidences(disputeData?.dispute?.externalDisputeId?.toString(), debouncedSearch); + const { error } = useEvidences(disputeData?.dispute?.externalDisputeId?.toString(), debouncedSearch); + + if (error) { + return <div>Error loading evidences: {error.message}</div>; + }🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for error boundary implementation in parent components # Check for error boundary implementations echo "Checking for error boundary implementations..." rg -l "ErrorBoundary|componentDidCatch|getDerivedStateFromError" src/pages/Cases/ # Check for error handling in parent components echo "Checking for error handling in parent components..." ast-grep --pattern 'catch (error) { $$$ }' src/pages/Cases/Length of output: 483
Script:
#!/bin/bash # Let's try searching from the repository root with correct paths echo "Checking for error boundary implementations..." rg -l "ErrorBoundary|componentDidCatch|getDerivedStateFromError" web/src/pages/Cases/ echo "Checking for error handling in parent components..." ast-grep --pattern 'catch (error) { $$$ }' web/src/pages/Cases/ # Also check for error handling in the Evidence component's query hook echo "Checking error handling in useEvidences hook..." rg "useEvidences" -A 5 web/src/Length of output: 2132
Script:
#!/bin/bash # Let's check the useQuery implementation and error handling echo "Checking useQuery implementation..." rg "useQuery" -A 10 web/src/hooks/queries/useEvidences.ts # Let's also check if there's any global error handling setup echo "Checking for global error handling..." rg -l "QueryClientProvider|ErrorBoundary" web/src/Length of output: 1211
Script:
#!/bin/bash # Let's check the global error handling setup in these files echo "Checking QueryClientProvider setup..." cat web/src/context/QueryClientProvider.tsx echo "Checking app.tsx error handling..." cat web/src/app.tsxLength of output: 4742
Script:
#!/bin/bash # Let's check if there's error handling in SentryRoutes echo "Checking SentryRoutes implementation..." cat web/src/utils/sentry.tsx # Also check if there's any error handling in the Cases component echo "Checking Cases component..." cat web/src/pages/Cases/index.tsxLength of output: 1179
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
web/src/pages/Cases/CaseDetails/Evidence/index.tsx(5 hunks)
🔇 Additional comments (1)
web/src/pages/Cases/CaseDetails/Evidence/index.tsx (1)
1-1: LGTM: Import changes and styled component implementation.
The new imports and styled component implementation follow React and styled-components best practices.
Also applies to: 20-21, 59-63
PR-Codex overview
This PR introduces functionality to handle and display spam evidences in the
Evidencecomponent, enhancing the user experience by allowing users to view or hide likely spam submissions.Detailed summary
REACT_APP_SPAM_EVIDENCES_IDSenvironment variable to.env.devnet.public.spamEvidencesIdsconstant inweb/src/consts/index.ts.Evidencecomponent to filter and display spam evidences.SpamLabel.isSpamfunction to check if an evidence ID is in the spam list.Summary by CodeRabbit
New Features
Enhancements
Refactor