Skip to content

Bad actors in SDK#708

Merged
feruzm merged 4 commits into
developfrom
bad
Mar 18, 2026
Merged

Bad actors in SDK#708
feruzm merged 4 commits into
developfrom
bad

Conversation

@feruzm
Copy link
Copy Markdown
Member

@feruzm feruzm commented Mar 18, 2026

Summary by CodeRabbit

  • New Features

    • Added warning messages when interacting with flagged accounts across delegation, transfers, and wallet forms.
  • Improvements

    • Bad-actor checks now use dynamically fetched, regularly refreshed data.
    • Validation now handles case-insensitive matches and safer absent-data behavior.
    • Removed the minimum video duration alert from the video upload flow.
  • Tests

    • Added tests covering bad-actor warnings, case-insensitivity, and empty/undefined lists.
  • Chores

    • SDK and wallet packages updated with new release entries.

@feruzm feruzm marked this pull request as draft March 18, 2026 17:24
@feruzm feruzm marked this pull request as ready for review March 18, 2026 17:24
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 18, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 16311888-f2ed-44aa-87c5-f382ffedee7f

📥 Commits

Reviewing files that changed from the base of the PR and between 8e6258b and 7bcce8a.

📒 Files selected for processing (4)
  • packages/sdk/CHANGELOG.md
  • packages/sdk/package.json
  • packages/wallets/CHANGELOG.md
  • packages/wallets/package.json
✅ Files skipped from review due to trivial changes (1)
  • packages/wallets/CHANGELOG.md

📝 Walkthrough

Walkthrough

Replaces a static bad-actors JSON with a runtime React Query fetch (returns a Set) exposed from the SDK; multiple components now call getBadActorsQueryOptions() and validate via badActors?.has(...). Also removes a video-upload minimum-duration alert and its i18n key.

Changes

Cohort / File(s) Summary
SDK Bad-Actors Module
packages/sdk/src/modules/bad-actors/..., packages/sdk/src/modules/core/query-keys.ts, packages/sdk/src/index.ts
Add getBadActorsQueryOptions() (fetches remote list → Set), export bad-actors module, and add QueryKeys.badActors.
RC & Transfer Logic
apps/web/src/app/(dynamicPages)/profile/[username]/_components/rc-delegation/index.tsx, apps/web/src/features/shared/transfer/use-debounce-transfer-account-data.ts
Remove static JSON import; use useQuery(getBadActorsQueryOptions()); switch validation from .includes() to guarded badActors?.has(value.toLowerCase()); update effect dependencies.
Wallet Operation UIs
apps/web/src/features/wallet/operations/wallet-operation-card-username-form.tsx, apps/web/src/features/wallet/operations/wallet-operation-withdraw-routes-form.tsx
Fetch bad actors via React Query; compute badActorWarning and render runtime warning UI (withdraw routes uses AnimatePresence).
Video Upload UI & i18n
apps/web/src/features/shared/video-upload-threespeak/index.tsx, apps/web/src/features/i18n/locales/en-US.json
Removed minimum-duration Alert from video-upload UI and deleted min-duration-alert translation key.
Tests
apps/web/src/specs/features/wallet/withdraw-routes-form.spec.tsx
Add tests for bad-actor warning rendering, case-insensitivity, and behavior when bad-actors list is undefined/empty.
Changelogs & Packages
packages/sdk/CHANGELOG.md, packages/sdk/package.json, packages/wallets/CHANGELOG.md, packages/wallets/package.json
Bump SDK to 2.0.30, wallets to 1.5.55 and record changelog entries for the bad-actors addition.

Sequence Diagram(s)

sequenceDiagram
    participant Component as Component
    participant ReactQuery as React Query
    participant Cache as Query Cache
    participant Remote as Remote API

    Component->>ReactQuery: useQuery(getBadActorsQueryOptions())
    alt Cache Hit (staleTime < 24h)
        ReactQuery->>Cache: check ["bad-actors","list"]
        Cache-->>ReactQuery: return cached Set
        ReactQuery-->>Component: badActors Set
    else Cache Miss / Stale
        ReactQuery->>Remote: fetch(bad-actors URL)
        Remote-->>ReactQuery: text response
        ReactQuery->>ReactQuery: parse lines → Set (lowercased)
        ReactQuery->>Cache: store Set (gcTime: Infinity)
        ReactQuery-->>Component: badActors Set
    end
    Component->>Component: badActors?.has(username.toLowerCase())
    Component->>Component: render warning if matched
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I hopped from file to fetch and then,
I swapped the list for network zen,
Lines lowercased, cached to rest,
Warnings now pop where names protest,
A tiny rabbit cheer — dynamic's been! 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: moving bad actors data from a static import to a dynamic, runtime-fetched implementation via the SDK, which is the central theme across multiple files in this changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch bad
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/web/src/features/wallet/operations/wallet-operation-withdraw-routes-form.tsx (1)

103-112: ⚠️ Potential issue | 🟡 Minor

Missing AnimatePresence wrapper for percent error animation.

The percent field error uses motion.div with exit animation but isn't wrapped in AnimatePresence, so the exit animation won't work. This is inconsistent with the account field above.

🔧 Proposed fix
+          <AnimatePresence>
           {methods.formState.errors.percent && (
             <motion.div
               initial={{ opacity: 0, height: 0 }}
               animate={{ opacity: 1, height: "auto" }}
               exit={{ opacity: 0, height: 0 }}
               className="text-red text-xs px-3 pt-0.5"
             >
               {methods.formState.errors.percent.message}
             </motion.div>
           )}
+          </AnimatePresence>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/web/src/features/wallet/operations/wallet-operation-withdraw-routes-form.tsx`
around lines 103 - 112, The percent error block uses a motion.div with an exit
animation but isn't wrapped in AnimatePresence, so its exit animation never
runs; wrap the conditional rendering of methods.formState.errors.percent in an
<AnimatePresence> and keep the motion.div as the child (similar to the account
error above) so the exit prop on the motion.div will be honored; ensure you
import AnimatePresence from framer-motion if not already.
🧹 Nitpick comments (2)
apps/web/src/features/wallet/operations/wallet-operation-withdraw-routes-form.tsx (1)

83-93: Consider normalizing account and avoiding watch() in render for performance.

Two concerns:

  1. Case sensitivity: methods.watch("account") is not normalized, unlike wallet-operation-card-username-form.tsx which lowercases the username. This may cause inconsistent bad actor detection.

  2. Performance: methods.watch() called during render triggers a re-render on every keystroke. Consider using a debounced value like the username form does, or moving the check to a useMemo with the watched value.

♻️ Proposed approach using watched value with normalization
 export function WalletOperationWithdrawRoutesForm({ onSubmit, initialValues }: Props) {
   const { activeUser } = useActiveAccount();
   const { data: badActors } = useQuery(getBadActorsQueryOptions());
+  const accountValue = methods.watch("account");
+  const isBadActor = useMemo(
+    () => badActors?.has(accountValue?.toLowerCase() ?? ""),
+    [badActors, accountValue]
+  );
   const methods = useForm<WithdrawRoutesFormData>({

Then use isBadActor in the JSX:

-            {badActors?.has(methods.watch("account")) && (
+            {isBadActor && (
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/web/src/features/wallet/operations/wallet-operation-withdraw-routes-form.tsx`
around lines 83 - 93, The JSX is calling methods.watch("account") directly which
is unnormalized (case-sensitive) and causes re-renders on every keystroke;
instead, derive a normalized, debounced watched value (lowercase like
wallet-operation-card-username-form) outside of render—e.g. use the form's watch
hook once, pass it through a small debounce or useMemo to produce
accountNormalized = watchedAccount?.toLowerCase(), then compute isBadActor =
badActors?.has(accountNormalized) and use isBadActor in the JSX (the motion div
with key "bad-actor-warning") so the rendering uses a stable, normalized boolean
rather than calling methods.watch inline.
apps/web/src/app/(dynamicPages)/profile/[username]/_components/rc-delegation/index.tsx (1)

10-13: Consider consolidating @tanstack/react-query imports.

Both useQuery (line 11) and useQueryClient (line 13) are imported from the same package. Consider combining them into a single import statement for cleaner code.

🧹 Suggested consolidation
 import { getBadActorsQueryOptions } from "@ecency/sdk";
-import { useQuery } from "@tanstack/react-query";
 import { getAccountFullQueryOptions } from "@ecency/sdk";
-import { useQueryClient } from "@tanstack/react-query";
+import { useQuery, useQueryClient } from "@tanstack/react-query";
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/web/src/app/`(dynamicPages)/profile/[username]/_components/rc-delegation/index.tsx
around lines 10 - 13, The imports from `@tanstack/react-query` are duplicated;
consolidate them by combining useQuery and useQueryClient into a single import
statement (replace the separate useQuery and useQueryClient imports with one
import that includes both useQuery and useQueryClient) so the module is imported
once and the symbols useQuery and useQueryClient are imported together.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/web/src/features/shared/transfer/use-debounce-transfer-account-data.ts`:
- Line 96: Normalize the username before checking badActors to avoid
case-sensitivity mismatches: when setting the warning via setToWarning, use a
normalized form of the `to` string (e.g., `to?.toLowerCase()` or equivalent
after trimming/validating) for the `badActors?.has(...)` lookup so it matches
the normalization used elsewhere (see `wallet-operation-card-username-form`
behavior); also guard against `to` being null/undefined before calling
normalization and keep the i18next.t("transfer.to-bad-actor") message unchanged.

---

Outside diff comments:
In
`@apps/web/src/features/wallet/operations/wallet-operation-withdraw-routes-form.tsx`:
- Around line 103-112: The percent error block uses a motion.div with an exit
animation but isn't wrapped in AnimatePresence, so its exit animation never
runs; wrap the conditional rendering of methods.formState.errors.percent in an
<AnimatePresence> and keep the motion.div as the child (similar to the account
error above) so the exit prop on the motion.div will be honored; ensure you
import AnimatePresence from framer-motion if not already.

---

Nitpick comments:
In
`@apps/web/src/app/`(dynamicPages)/profile/[username]/_components/rc-delegation/index.tsx:
- Around line 10-13: The imports from `@tanstack/react-query` are duplicated;
consolidate them by combining useQuery and useQueryClient into a single import
statement (replace the separate useQuery and useQueryClient imports with one
import that includes both useQuery and useQueryClient) so the module is imported
once and the symbols useQuery and useQueryClient are imported together.

In
`@apps/web/src/features/wallet/operations/wallet-operation-withdraw-routes-form.tsx`:
- Around line 83-93: The JSX is calling methods.watch("account") directly which
is unnormalized (case-sensitive) and causes re-renders on every keystroke;
instead, derive a normalized, debounced watched value (lowercase like
wallet-operation-card-username-form) outside of render—e.g. use the form's watch
hook once, pass it through a small debounce or useMemo to produce
accountNormalized = watchedAccount?.toLowerCase(), then compute isBadActor =
badActors?.has(accountNormalized) and use isBadActor in the JSX (the motion div
with key "bad-actor-warning") so the rendering uses a stable, normalized boolean
rather than calling methods.watch inline.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 61b9cd90-f373-4823-8f3a-318e79a10eac

📥 Commits

Reviewing files that changed from the base of the PR and between 9fad301 and 99e9274.

⛔ Files ignored due to path filters (7)
  • packages/sdk/dist/browser/index.d.ts is excluded by !**/dist/**
  • packages/sdk/dist/browser/index.js is excluded by !**/dist/**
  • packages/sdk/dist/browser/index.js.map is excluded by !**/dist/**, !**/*.map
  • packages/sdk/dist/node/index.cjs is excluded by !**/dist/**
  • packages/sdk/dist/node/index.cjs.map is excluded by !**/dist/**, !**/*.map
  • packages/sdk/dist/node/index.mjs is excluded by !**/dist/**
  • packages/sdk/dist/node/index.mjs.map is excluded by !**/dist/**, !**/*.map
📒 Files selected for processing (11)
  • apps/web/src/app/(dynamicPages)/profile/[username]/_components/rc-delegation/index.tsx
  • apps/web/src/features/i18n/locales/en-US.json
  • apps/web/src/features/shared/transfer/use-debounce-transfer-account-data.ts
  • apps/web/src/features/shared/video-upload-threespeak/index.tsx
  • apps/web/src/features/wallet/operations/wallet-operation-card-username-form.tsx
  • apps/web/src/features/wallet/operations/wallet-operation-withdraw-routes-form.tsx
  • packages/sdk/src/index.ts
  • packages/sdk/src/modules/bad-actors/index.ts
  • packages/sdk/src/modules/bad-actors/queries/get-bad-actors-query-options.ts
  • packages/sdk/src/modules/bad-actors/queries/index.ts
  • packages/sdk/src/modules/core/query-keys.ts
💤 Files with no reviewable changes (2)
  • apps/web/src/features/shared/video-upload-threespeak/index.tsx
  • apps/web/src/features/i18n/locales/en-US.json

Comment thread apps/web/src/features/shared/transfer/use-debounce-transfer-account-data.ts Outdated
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

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@apps/web/src/features/wallet/operations/wallet-operation-withdraw-routes-form.tsx`:
- Around line 72-94: The dynamic validation and warning <motion.div> messages
inside AnimatePresence (the account error branch using
methods.formState.errors.account and the bad-actor branch checking
badActors.has(methods.watch("account")?.toLowerCase())) lack live-region
semantics and won't be reliably announced by screen readers; update both
animated containers to include accessible attributes such as role="status" (or
role="alert" for immediate interruption) and an appropriate aria-live value
(e.g., aria-live="polite" or "assertive") so screen readers announce the
messages, and ensure these attributes are set on the same <motion.div> elements
that render the messages so the announcements occur when they appear/exit.
- Line 83: The membership check can miss inputs with leading/trailing spaces;
update the expression that currently uses
methods.watch("account")?.toLowerCase() so it normalizes whitespace first (e.g.,
call .trim() before .toLowerCase()) and preserves the existing optional
chaining, e.g., methods.watch("account")?.trim()?.toLowerCase(), ensuring
badActors (the Set) contains comparable normalized values for accurate matching.
- Line 37: Add unit tests for the bad-actor warning in the
WalletOperationWithdrawRoutesForm component: create two tests that render the
form and assert warning visibility using screen.getByRole for the text key
'transfer.to-bad-actor' — one test should mock the
useQuery(getBadActorsQueryOptions()) result (the hook used on the component) to
return a badActors list that includes the recipient account and assert the
warning is visible, the other should mock the hook to return a list that does
not include the recipient and assert the warning is not present/hidden. Ensure
the tests target the WalletOperationWithdrawRoutesForm rendering flow and use
screen.getByRole as required.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 608df8bf-238e-4029-8a05-7f15bb83dff6

📥 Commits

Reviewing files that changed from the base of the PR and between 99e9274 and 1820a50.

📒 Files selected for processing (3)
  • apps/web/src/app/(dynamicPages)/profile/[username]/_components/rc-delegation/index.tsx
  • apps/web/src/features/shared/transfer/use-debounce-transfer-account-data.ts
  • apps/web/src/features/wallet/operations/wallet-operation-withdraw-routes-form.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
  • apps/web/src/features/shared/transfer/use-debounce-transfer-account-data.ts
  • apps/web/src/app/(dynamicPages)/profile/[username]/_components/rc-delegation/index.tsx


export function WalletOperationWithdrawRoutesForm({ onSubmit, initialValues }: Props) {
const { activeUser } = useActiveAccount();
const { data: badActors } = useQuery(getBadActorsQueryOptions());
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.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) Find tests related to withdraw-routes form
fd -t f '(wallet-operation-withdraw-routes-form|withdraw-routes).*(test|spec)\.(ts|tsx)$' apps/web/src/features

# 2) Find assertions covering bad-actor warning behavior
rg -n --type=tsx -C2 'to-bad-actor|transfer\.to-bad-actor|getBadActorsQueryOptions|badActors\.has' \
  apps/web/src -g '*.{test,spec}.tsx'

Repository: ecency/vision-next

Length of output: 89


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) Find the implementation file
echo "=== Implementation file ==="
fd -t f 'wallet-operation-withdraw-routes-form' apps/web/src/features

# 2) Find any test files related to withdraw-routes form
echo -e "\n=== Test files for withdraw-routes form ==="
fd -t f -e spec.ts -e spec.tsx -e test.ts -e test.tsx apps/web/src/features/wallet

# 3) Search for bad-actor or getBadActorsQueryOptions in spec/test files
echo -e "\n=== References to bad-actor or getBadActorsQueryOptions in tests ==="
rg -n 'badActors|getBadActorsQueryOptions|bad-actor' -g '*.spec.{ts,tsx}' -g '*.test.{ts,tsx}' apps/web/src/features/wallet

# 4) Check the implementation file to understand what lines 37 and 72-94 contain
echo -e "\n=== Implementation file content (lines 30-100) ==="
sed -n '30,100p' apps/web/src/features/wallet/operations/wallet-operation-withdraw-routes-form.tsx

Repository: ecency/vision-next

Length of output: 651


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Read the implementation file to see lines 37 and 72-94
echo "=== Implementation file content ==="
cat -n apps/web/src/features/wallet/operations/wallet-operation-withdraw-routes-form.tsx | sed -n '30,100p'

# Find test files in wallet directory correctly
echo -e "\n=== Test files in wallet features ==="
fd -e spec.ts -e spec.tsx -e test.ts -e test.tsx . apps/web/src/features/wallet

# Search for bad-actor or getBadActorsQueryOptions in all test files
echo -e "\n=== Search for badActors references in tests ==="
rg -n 'badActors|getBadActorsQueryOptions|bad-actor' -g '*.spec.{ts,tsx}' -g '*.test.{ts,tsx}' apps/web/src/features

# Also check if there are any general wallet tests
echo -e "\n=== All spec/test files in wallet ==="
find apps/web/src/features/wallet -type f \( -name '*.spec.ts' -o -name '*.spec.tsx' -o -name '*.test.ts' -o -name '*.test.tsx' \)

Repository: ecency/vision-next

Length of output: 3298


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Search for any test file that might reference this form
echo "=== Search for form name in any test files ==="
rg -n 'WalletOperationWithdrawRoutesForm|withdraw-routes-form' -g '*.spec.{ts,tsx}' -g '*.test.{ts,tsx}' apps/web/src

# Check if there are ANY test files in the wallet operations directory
echo -e "\n=== Check wallet operations directory structure ==="
find apps/web/src/features/wallet/operations -type f | head -20

# Search for any wallet operation form tests
echo -e "\n=== Search for wallet operation form tests ==="
find apps/web/src/features/wallet -type f -name '*.spec.ts*' -o -name '*.test.ts*'

# Check what spec files exist in the wallet features directory tree
echo -e "\n=== All spec files in wallet directory ==="
find apps/web/src/features/wallet -type f \( -name '*.spec.ts' -o -name '*.spec.tsx' \)

# Search for "badActors" anywhere in the codebase to see usage patterns
echo -e "\n=== References to badActors in entire codebase ==="
rg -n 'badActors' apps/web/src --max-count=10

Repository: ecency/vision-next

Length of output: 108


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# List all files in the operations directory
echo "=== Files in wallet operations directory ==="
ls -la apps/web/src/features/wallet/operations/

# List all files in the wallet directory recursively
echo -e "\n=== All files in wallet directory (first 30) ==="
find apps/web/src/features/wallet -type f | head -30

# Check if any spec/test files exist at all
echo -e "\n=== Count of spec/test files in wallet ==="
find apps/web/src/features/wallet -type f \( -name '*.spec.ts' -o -name '*.spec.tsx' -o -name '*.test.ts' -o -name '*.test.tsx' \) | wc -l

# Simple grep for badActors to verify the implementation uses it
echo -e "\n=== Verify badActors usage in implementation ==="
grep -n "badActors" apps/web/src/features/wallet/operations/wallet-operation-withdraw-routes-form.tsx

Repository: ecency/vision-next

Length of output: 3797


Add tests for bad-actor warning behavior.

The new bad-actor warning (lines 83–92) introduces user-visible behavior but has no test coverage. Per coding guidelines, all new features in @ecency/web require tests. Add tests covering:

  • Warning displayed when the recipient account is in the bad-actors list
  • Warning hidden when the recipient account is not in the bad-actors list

Test should use screen.getByRole to verify warning text visibility (transfer.to-bad-actor).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/web/src/features/wallet/operations/wallet-operation-withdraw-routes-form.tsx`
at line 37, Add unit tests for the bad-actor warning in the
WalletOperationWithdrawRoutesForm component: create two tests that render the
form and assert warning visibility using screen.getByRole for the text key
'transfer.to-bad-actor' — one test should mock the
useQuery(getBadActorsQueryOptions()) result (the hook used on the component) to
return a badActors list that includes the recipient account and assert the
warning is visible, the other should mock the hook to return a list that does
not include the recipient and assert the warning is not present/hidden. Ensure
the tests target the WalletOperationWithdrawRoutesForm rendering flow and use
screen.getByRole as required.

Comment on lines +72 to +94
<AnimatePresence>
{methods.formState.errors.account && (
<motion.div
initial={{ opacity: 0, height: 0 }}
animate={{ opacity: 1, height: "auto" }}
exit={{ opacity: 0, height: 0 }}
className="text-red text-xs px-3 pt-0.5"
>
{methods.formState.errors.account.message}
</motion.div>
)}
{badActors?.has(methods.watch("account")?.toLowerCase()) && (
<motion.div
initial={{ opacity: 0, height: 0 }}
animate={{ opacity: 1, height: "auto" }}
exit={{ opacity: 0, height: 0 }}
key="bad-actor-warning"
className="text-warning-default text-xs px-3 pt-0.5"
>
{i18next.t("transfer.to-bad-actor")}
</motion.div>
)}
</AnimatePresence>
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.

⚠️ Potential issue | 🟠 Major

Add live-region semantics to dynamic validation/warning messages.

These messages are injected dynamically but are not announced reliably by screen readers. Please add role + aria-live on the animated message containers.

♿ Minimal accessibility patch
             {methods.formState.errors.account && (
               <motion.div
+                role="alert"
+                aria-live="polite"
                 initial={{ opacity: 0, height: 0 }}
                 animate={{ opacity: 1, height: "auto" }}
                 exit={{ opacity: 0, height: 0 }}
                 className="text-red text-xs px-3 pt-0.5"
               >
                 {methods.formState.errors.account.message}
               </motion.div>
             )}
             {badActors?.has(methods.watch("account")?.toLowerCase()) && (
               <motion.div
+                role="status"
+                aria-live="polite"
                 initial={{ opacity: 0, height: 0 }}
                 animate={{ opacity: 1, height: "auto" }}
                 exit={{ opacity: 0, height: 0 }}
                 key="bad-actor-warning"
                 className="text-warning-default text-xs px-3 pt-0.5"
               >
                 {i18next.t("transfer.to-bad-actor")}
               </motion.div>
             )}
@@
             {methods.formState.errors.percent && (
               <motion.div
+                role="alert"
+                aria-live="polite"
                 initial={{ opacity: 0, height: 0 }}
                 animate={{ opacity: 1, height: "auto" }}
                 exit={{ opacity: 0, height: 0 }}
                 className="text-red text-xs px-3 pt-0.5"
               >
                 {methods.formState.errors.percent.message}
               </motion.div>
             )}

Also applies to: 103-114

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/web/src/features/wallet/operations/wallet-operation-withdraw-routes-form.tsx`
around lines 72 - 94, The dynamic validation and warning <motion.div> messages
inside AnimatePresence (the account error branch using
methods.formState.errors.account and the bad-actor branch checking
badActors.has(methods.watch("account")?.toLowerCase())) lack live-region
semantics and won't be reliably announced by screen readers; update both
animated containers to include accessible attributes such as role="status" (or
role="alert" for immediate interruption) and an appropriate aria-live value
(e.g., aria-live="polite" or "assertive") so screen readers announce the
messages, and ensure these attributes are set on the same <motion.div> elements
that render the messages so the announcements occur when they appear/exit.

Comment thread apps/web/src/features/wallet/operations/wallet-operation-withdraw-routes-form.tsx Outdated
@feruzm feruzm added the patch Bug fixes and patches (1.0.0 → 1.0.1), add this only if any packages/ have patch changes in PR label Mar 18, 2026
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: 2

🧹 Nitpick comments (1)
apps/web/src/specs/features/wallet/withdraw-routes-form.spec.tsx (1)

9-15: Avoid mocking useQuery directly in this component behavior test.

Mocking React Query internals here couples tests to implementation details and bypasses the renderWithQueryClient flow you already use. Prefer seeding query data via query client (or mocking the network layer) and keeping useQuery real.

Based on learnings: "Use renderWithQueryClient from src/specs/test-utils.tsx for testing components that use React Query" and "Do not test library code (React Query, Zustand); focus on testing application logic".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/src/specs/features/wallet/withdraw-routes-form.spec.tsx` around
lines 9 - 15, Remove the vi.mock of "@tanstack/react-query" (which overrides
useQuery) and instead let the real useQuery run; seed the query cache via the
test query client or mock the network responses and render the component with
the existing renderWithQueryClient helper so the component uses the real React
Query flow (reference: useQuery in the diff and renderWithQueryClient in
src/specs/test-utils.tsx); update withdraw-routes-form.spec.tsx to populate the
queryClient with the required query keys/data or stub the network calls before
calling renderWithQueryClient, and delete the vi.mock block.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/web/src/specs/features/wallet/withdraw-routes-form.spec.tsx`:
- Around line 21-23: The mock for getBadActorsQueryOptions uses a hardcoded
queryKey ["bad-actors"] which doesn't match the SDK implementation; update the
mock in withdraw-routes-form.spec.tsx so getBadActorsQueryOptions returns
queryKey: QueryKeys.badActors.list() (or the exact array value returned by that
method) to keep cache keys consistent with the real implementation and prevent
test/cache mismatches.
- Around line 34-38: The test currently calls (useActiveAccount as
any).mockReturnValue without first mocking the module; add an explicit jest.mock
call for "@/core/hooks/use-active-account" at the top of the test file before
using useActiveAccount and then call mockReturnValue on the mocked function
(reference: useActiveAccount). Also update the SDK mock that returns/observes
the bad-actors query to use the canonical query key QueryKeys.badActors.list()
from `@ecency/sdk` instead of the hardcoded ["bad-actors"] string (reference:
QueryKeys.badActors.list()) so the mocked query key matches production code.

---

Nitpick comments:
In `@apps/web/src/specs/features/wallet/withdraw-routes-form.spec.tsx`:
- Around line 9-15: Remove the vi.mock of "@tanstack/react-query" (which
overrides useQuery) and instead let the real useQuery run; seed the query cache
via the test query client or mock the network responses and render the component
with the existing renderWithQueryClient helper so the component uses the real
React Query flow (reference: useQuery in the diff and renderWithQueryClient in
src/specs/test-utils.tsx); update withdraw-routes-form.spec.tsx to populate the
queryClient with the required query keys/data or stub the network calls before
calling renderWithQueryClient, and delete the vi.mock block.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3df039de-1a3a-448d-9547-a003f82c3623

📥 Commits

Reviewing files that changed from the base of the PR and between 1820a50 and 8e6258b.

📒 Files selected for processing (3)
  • apps/web/src/features/shared/transfer/use-debounce-transfer-account-data.ts
  • apps/web/src/features/wallet/operations/wallet-operation-withdraw-routes-form.tsx
  • apps/web/src/specs/features/wallet/withdraw-routes-form.spec.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
  • apps/web/src/features/shared/transfer/use-debounce-transfer-account-data.ts
  • apps/web/src/features/wallet/operations/wallet-operation-withdraw-routes-form.tsx

Comment on lines +21 to +23
getBadActorsQueryOptions: vi.fn(() => ({
queryKey: ["bad-actors"],
queryFn: vi.fn()
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.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, let's find and examine the SDK's QueryKeys definition
fd -t f "query-keys" --extension ts

Repository: ecency/vision-next

Length of output: 105


🏁 Script executed:

# Check the structure of the test file to understand the context
cat -n apps/web/src/specs/features/wallet/withdraw-routes-form.spec.tsx | head -50

Repository: ecency/vision-next

Length of output: 1919


🏁 Script executed:

# Search for badActors in the SDK
rg "badActors" --type ts -A 2 -B 2

Repository: ecency/vision-next

Length of output: 8969


Use SDK QueryKeys instead of hardcoded query key array to maintain cache-key consistency.

At Line 22, the mock uses ["bad-actors"] but the actual SDK's getBadActorsQueryOptions uses QueryKeys.badActors.list(), which returns ["bad-actors", "list"]. The test mock should match the real implementation.

🧩 Proposed fix
     ...actual,
     getBadActorsQueryOptions: vi.fn(() => ({
-      queryKey: ["bad-actors"],
+      queryKey: actual.QueryKeys.badActors.list(),
       queryFn: vi.fn()
     }))
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/src/specs/features/wallet/withdraw-routes-form.spec.tsx` around
lines 21 - 23, The mock for getBadActorsQueryOptions uses a hardcoded queryKey
["bad-actors"] which doesn't match the SDK implementation; update the mock in
withdraw-routes-form.spec.tsx so getBadActorsQueryOptions returns queryKey:
QueryKeys.badActors.list() (or the exact array value returned by that method) to
keep cache keys consistent with the real implementation and prevent test/cache
mismatches.

Comment on lines +34 to +38
(useActiveAccount as any).mockReturnValue({
activeUser: { username: "testuser" },
username: "testuser",
account: { name: "testuser" }
});
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.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

cat -n apps/web/src/specs/features/wallet/withdraw-routes-form.spec.tsx | head -50

Repository: ecency/vision-next

Length of output: 1919


🏁 Script executed:

rg "QueryKeys\." --type ts --type tsx -A 2 | head -30

Repository: ecency/vision-next

Length of output: 89


🏁 Script executed:

rg "QueryKeys\." -A 2 | head -40

Repository: ecency/vision-next

Length of output: 3621


🏁 Script executed:

rg "QueryKeys\s*=|QueryKeys\s*:" -A 20 | head -80

Repository: ecency/vision-next

Length of output: 50376


🏁 Script executed:

rg "badActors" -A 2 -B 2

Repository: ecency/vision-next

Length of output: 50376


🏁 Script executed:

grep -r "badActors" packages/sdk/src --include="*.ts" -A 3

Repository: ecency/vision-next

Length of output: 777


useActiveAccount is used as a mock without mocking its module first.

At Line 34, this can fail at runtime (mockReturnValue on a real function). Please explicitly mock @/core/hooks/use-active-account before using it as a mock. Additionally, replace the hardcoded query key ["bad-actors"] in the SDK mock with QueryKeys.badActors.list() to use the canonical query key from @ecency/sdk.

🔧 Proposed fix
 import { useActiveAccount } from "@/core/hooks/use-active-account";
 import { useQuery } from "@tanstack/react-query";

+vi.mock("@/core/hooks/use-active-account", () => ({
+  useActiveAccount: vi.fn()
+}));
+
 vi.mock("@tanstack/react-query", async () => {
   const actual = await vi.importActual("@tanstack/react-query");
   return {
     ...actual,
     useQuery: vi.fn()
   };
 });

 vi.mock("@ecency/sdk", async () => {
   const actual = await vi.importActual("@ecency/sdk");
   return {
     ...actual,
     getBadActorsQueryOptions: vi.fn(() => ({
-      queryKey: ["bad-actors"],
+      queryKey: QueryKeys.badActors.list(),
       queryFn: vi.fn()
     }))
   };
 });
@@
-    (useActiveAccount as any).mockReturnValue({
+    vi.mocked(useActiveAccount).mockReturnValue({
       activeUser: { username: "testuser" },
       username: "testuser",
       account: { name: "testuser" }
     });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/src/specs/features/wallet/withdraw-routes-form.spec.tsx` around
lines 34 - 38, The test currently calls (useActiveAccount as
any).mockReturnValue without first mocking the module; add an explicit jest.mock
call for "@/core/hooks/use-active-account" at the top of the test file before
using useActiveAccount and then call mockReturnValue on the mocked function
(reference: useActiveAccount). Also update the SDK mock that returns/observes
the bad-actors query to use the canonical query key QueryKeys.badActors.list()
from `@ecency/sdk` instead of the hardcoded ["bad-actors"] string (reference:
QueryKeys.badActors.list()) so the mocked query key matches production code.

@feruzm feruzm merged commit 150f4af into develop Mar 18, 2026
1 check passed
@feruzm feruzm deleted the bad branch March 18, 2026 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

patch Bug fixes and patches (1.0.0 → 1.0.1), add this only if any packages/ have patch changes in PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant