Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat(web): evidence moderation #1743

Merged
merged 2 commits into from
Nov 18, 2024
Merged

feat(web): evidence moderation #1743

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5d915e5
fix(web): evidence-sensor
Harman-singh-waraich Nov 18, 2024
519a555
refactor(web): evidence-spam-check
Harman-singh-waraich Nov 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions web/.env.devnet.public
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 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 IDs
  • web/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

# devtools
export REACT_APP_GRAPH_API_KEY=
2 changes: 2 additions & 0 deletions web/src/consts/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,5 @@ export const genesisBlock = () => (isProductionDeployment() ? GENESIS_BLOCK_ARBM

export const INVALID_DISPUTE_DATA_ERROR = `The dispute data is not valid, please vote "Refuse to arbitrate"`;
export const RPC_ERROR = `RPC Error: Unable to fetch dispute data. Please avoid voting.`;

export const spamEvidencesIds: string[] = (import.meta.env.REACT_APP_SPAM_EVIDENCES_IDS ?? "").split(",");
52 changes: 49 additions & 3 deletions web/src/pages/Cases/CaseDetails/Evidence/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useCallback, useRef, useState } from "react";
import React, { useCallback, useMemo, useRef, useState } from "react";
import styled from "styled-components";

import { useParams } from "react-router-dom";
Expand All @@ -17,6 +17,8 @@ import EvidenceCard from "components/EvidenceCard";
import { SkeletonEvidenceCard } from "components/StyledSkeleton";

import EvidenceSearch from "./EvidenceSearch";
import { Divider } from "components/Divider";
import { spamEvidencesIds } from "src/consts";

const Container = styled.div`
width: 100%;
Expand Down Expand Up @@ -54,12 +56,19 @@ const ScrollButton = styled(Button)`
}
`;

const SpamLabel = styled.label`
color: ${({ theme }) => theme.primaryBlue};
align-self: center;
cursor: pointer;
`;

const Evidence: React.FC = () => {
const { id } = useParams();
const { data: disputeData } = useDisputeDetailsQuery(id);
const ref = useRef<HTMLDivElement>(null);
const [search, setSearch] = useState<string>();
const [debouncedSearch, setDebouncedSearch] = useState<string>();
const [showSpam, setShowSpam] = useState(false);

const { data } = useEvidences(disputeData?.dispute?.externalDisputeId?.toString(), debouncedSearch);

Expand All @@ -74,12 +83,22 @@ const Evidence: React.FC = () => {
latestEvidence.scrollIntoView({ behavior: "smooth" });
}, [ref]);

console.log({ data });

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

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 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]);
console.log({ evidences });

return (
<Container ref={ref}>
<EvidenceSearch {...{ search, setSearch, evidenceGroup: disputeData?.dispute?.externalDisputeId }} />
<ScrollButton small Icon={DownArrow} text="Scroll to latest" onClick={scrollToLatest} />
{data ? (
data.evidences.map(({ evidence, sender, timestamp, name, description, fileURI, evidenceIndex }) => (
{evidences?.realEvidences ? (
evidences?.realEvidences.map(({ evidence, sender, timestamp, name, description, fileURI, evidenceIndex }) => (
<EvidenceCard
key={timestamp}
index={parseInt(evidenceIndex)}
Expand All @@ -90,9 +109,36 @@ const Evidence: React.FC = () => {
) : (
<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 }}
/>
)
)
) : (
<SpamLabel onClick={() => setShowSpam(true)}>Show likely spam</SpamLabel>
)}
</>
) : null}
{data && data.evidences.length === 0 ? <StyledLabel>There is no evidence submitted yet</StyledLabel> : null}
</Container>
);
};

const isSpam = (id: string) => {
for (const spamId of spamEvidencesIds) {
if (id == spamId) return true;
}

return false;
};

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix equality check and optimize spam check performance

The current implementation has two issues:

  1. Uses loose equality (==) which could lead to unexpected behavior
  2. 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.

Suggested change
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);
};

export default Evidence;
Loading