Initialize filter on Run job envs page based on what tokens user holds#516
Open
andreip136 wants to merge 1 commit into
Open
Initialize filter on Run job envs page based on what tokens user holds#516andreip136 wants to merge 1 commit into
andreip136 wants to merge 1 commit into
Conversation
andreip136
requested review from
alexcos20,
bogdanfazakas,
dnsi0,
giurgiur99,
mihaisc and
ndrpp
as code owners
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthrough
ChangesAuto fee token detection and form sync
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/run-job/select-environment.tsx (1)
68-73:⚠️ Potential issue | 🟠 Major
freemust be derived from thefilterscontext like other field values.With
enableReinitialize: true, hard-codinginitialValues.freetofalsecauses the form to reset the toggle to unchecked whenever filters change, even iffilters.freeistrue. This desyncs the UI from the active filter state in the context.Suggested fix
initialValues: { feeToken: Array.isArray(filters.feeToken) ? '' : (filters.feeToken ?? ''), - free: false, + free: filters.free ?? false, gpuName: filters.gpuName ?? [],🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/run-job/select-environment.tsx` around lines 68 - 73, The `free` field in the formik initialValues is hardcoded to `false`, which causes the form to reset whenever filters change due to `enableReinitialize: true`, even when `filters.free` is `true`. This desynchronizes the UI from the active filter state. In the useFormik hook's initialValues object within the select-environment.tsx component, change the `free` field from `free: false` to derive it from the `filters` context like the other field values (e.g., use `free: filters.free ?? false`), ensuring the form state reflects the actual filter state when it reinitializes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/context/run-job-envs-context.tsx`:
- Around line 73-75: The setFilters call in the effect is overwriting the entire
filter state with DEFAULT_FILTERS and only the computed feeToken, which can wipe
out any active user filters set in between when the async effect resolves.
Instead of using setFilters({ ...DEFAULT_FILTERS, feeToken }), use a functional
state update approach with setFilters that preserves the previous filter state
and only updates the feeToken property. This ensures existing user filters
remain intact and are not lost when this effect completes asynchronously.
- Around line 60-75: The Promise.all() chain in the tokenEntries mapping lacks
error handling for failures in ocean.getBalance() or ocean.getUserFunds() calls.
Add a .catch() handler after the existing .then() block to gracefully handle any
promise rejections. The catch handler should log the error appropriately and
either set the filters with default values or skip the filter update to prevent
unhandled promise rejections and ensure the component remains functional even if
token balance fetching fails.
---
Outside diff comments:
In `@src/components/run-job/select-environment.tsx`:
- Around line 68-73: The `free` field in the formik initialValues is hardcoded
to `false`, which causes the form to reset whenever filters change due to
`enableReinitialize: true`, even when `filters.free` is `true`. This
desynchronizes the UI from the active filter state. In the useFormik hook's
initialValues object within the select-environment.tsx component, change the
`free` field from `free: false` to derive it from the `filters` context like the
other field values (e.g., use `free: filters.free ?? false`), ensuring the form
state reflects the actual filter state when it reinitializes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ba521a2d-990d-4825-8320-1242b008b08b
📒 Files selected for processing (2)
src/components/run-job/select-environment.tsxsrc/context/run-job-envs-context.tsx
Comment on lines
+60
to
+75
| Promise.all( | ||
| tokenEntries.map(async (token) => { | ||
| const [walletBalance, escrowBalance] = await Promise.all([ | ||
| ocean.getBalance(token.address, account.address!), | ||
| ocean.getUserFunds(token.address, account.address!), | ||
| ]); | ||
| return { address: token.address, hasBalance: Number(walletBalance) > 0 || Number(escrowBalance) > 0 }; | ||
| }) | ||
| ).then((results) => { | ||
| if (cancelled) { | ||
| return; | ||
| } | ||
| const held = results.filter((r) => r.hasBalance); | ||
| const feeToken = held.length === 1 ? held[0].address : DEFAULT_FILTERS.feeToken; | ||
| setFilters({ ...DEFAULT_FILTERS, feeToken }); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/context/run-job-envs-context.tsx | head -100Repository: oceanprotocol/nodes-dashboard
Length of output: 4360
🏁 Script executed:
cat -n src/context/run-job-envs-context.tsx | sed -n '53,85p'Repository: oceanprotocol/nodes-dashboard
Length of output: 1593
🏁 Script executed:
# Check if there's a .catch() handler after the .then() that might continue beyond line 75
cat -n src/context/run-job-envs-context.tsx | sed -n '60,80p'Repository: oceanprotocol/nodes-dashboard
Length of output: 961
Handle token-balance fetch failures explicitly.
The Promise.all() chain on lines 60–75 lacks error handling. If either ocean.getBalance() or ocean.getUserFunds() fails, the entire promise rejects without a .catch() handler, causing an unhandled promise rejection and preventing the fee token filter from auto-populating.
Suggested fix
- Promise.all(
- tokenEntries.map(async (token) => {
+ const loadHeldTokens = async () => {
+ const results = await Promise.allSettled(
+ tokenEntries.map(async (token) => {
const [walletBalance, escrowBalance] = await Promise.all([
ocean.getBalance(token.address, account.address!),
ocean.getUserFunds(token.address, account.address!),
]);
return { address: token.address, hasBalance: Number(walletBalance) > 0 || Number(escrowBalance) > 0 };
- })
- ).then((results) => {
+ })
+ );
+
if (cancelled) {
return;
}
- const held = results.filter((r) => r.hasBalance);
+ const held = results
+ .filter((r): r is PromiseFulfilledResult<{ address: string; hasBalance: boolean }> => r.status === 'fulfilled')
+ .map((r) => r.value)
+ .filter((r) => r.hasBalance);
const feeToken = held.length === 1 ? held[0].address : DEFAULT_FILTERS.feeToken;
setFilters({ ...DEFAULT_FILTERS, feeToken });
- });
+ };
+
+ loadHeldTokens().catch((error) => {
+ console.error('Failed to auto-detect fee token:', error);
+ });📝 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
| Promise.all( | |
| tokenEntries.map(async (token) => { | |
| const [walletBalance, escrowBalance] = await Promise.all([ | |
| ocean.getBalance(token.address, account.address!), | |
| ocean.getUserFunds(token.address, account.address!), | |
| ]); | |
| return { address: token.address, hasBalance: Number(walletBalance) > 0 || Number(escrowBalance) > 0 }; | |
| }) | |
| ).then((results) => { | |
| if (cancelled) { | |
| return; | |
| } | |
| const held = results.filter((r) => r.hasBalance); | |
| const feeToken = held.length === 1 ? held[0].address : DEFAULT_FILTERS.feeToken; | |
| setFilters({ ...DEFAULT_FILTERS, feeToken }); | |
| }); | |
| const loadHeldTokens = async () => { | |
| const results = await Promise.allSettled( | |
| tokenEntries.map(async (token) => { | |
| const [walletBalance, escrowBalance] = await Promise.all([ | |
| ocean.getBalance(token.address, account.address!), | |
| ocean.getUserFunds(token.address, account.address!), | |
| ]); | |
| return { address: token.address, hasBalance: Number(walletBalance) > 0 || Number(escrowBalance) > 0 }; | |
| }) | |
| ); | |
| if (cancelled) { | |
| return; | |
| } | |
| const held = results | |
| .filter((r): r is PromiseFulfilledResult<{ address: string; hasBalance: boolean }> => r.status === 'fulfilled') | |
| .map((r) => r.value) | |
| .filter((r) => r.hasBalance); | |
| const feeToken = held.length === 1 ? held[0].address : DEFAULT_FILTERS.feeToken; | |
| setFilters({ ...DEFAULT_FILTERS, feeToken }); | |
| }; | |
| loadHeldTokens().catch((error) => { | |
| console.error('Failed to auto-detect fee token:', error); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/context/run-job-envs-context.tsx` around lines 60 - 75, The Promise.all()
chain in the tokenEntries mapping lacks error handling for failures in
ocean.getBalance() or ocean.getUserFunds() calls. Add a .catch() handler after
the existing .then() block to gracefully handle any promise rejections. The
catch handler should log the error appropriately and either set the filters with
default values or skip the filter update to prevent unhandled promise rejections
and ensure the component remains functional even if token balance fetching
fails.
Comment on lines
+73
to
+75
| const feeToken = held.length === 1 ? held[0].address : DEFAULT_FILTERS.feeToken; | ||
| setFilters({ ...DEFAULT_FILTERS, feeToken }); | ||
| }); |
There was a problem hiding this comment.
Avoid overwriting the full filter state during auto-selection.
Setting setFilters({ ...DEFAULT_FILTERS, feeToken }) can wipe active user filters when this async effect resolves later, which then propagates into form reset/reload behavior.
Suggested fix
- const feeToken = held.length === 1 ? held[0].address : DEFAULT_FILTERS.feeToken;
- setFilters({ ...DEFAULT_FILTERS, feeToken });
+ const feeToken = held.length === 1 ? held[0].address : undefined;
+ setFilters((prev) => {
+ const nextFeeToken = feeToken ?? prev.feeToken ?? DEFAULT_FILTERS.feeToken;
+ if (prev.feeToken === nextFeeToken) return prev;
+ return { ...prev, feeToken: nextFeeToken };
+ });📝 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 feeToken = held.length === 1 ? held[0].address : DEFAULT_FILTERS.feeToken; | |
| setFilters({ ...DEFAULT_FILTERS, feeToken }); | |
| }); | |
| const feeToken = held.length === 1 ? held[0].address : undefined; | |
| setFilters((prev) => { | |
| const nextFeeToken = feeToken ?? prev.feeToken ?? DEFAULT_FILTERS.feeToken; | |
| if (prev.feeToken === nextFeeToken) return prev; | |
| return { ...prev, feeToken: nextFeeToken }; | |
| }); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/context/run-job-envs-context.tsx` around lines 73 - 75, The setFilters
call in the effect is overwriting the entire filter state with DEFAULT_FILTERS
and only the computed feeToken, which can wipe out any active user filters set
in between when the async effect resolves. Instead of using setFilters({
...DEFAULT_FILTERS, feeToken }), use a functional state update approach with
setFilters that preserves the previous filter state and only updates the
feeToken property. This ensures existing user filters remain intact and are not
lost when this effect completes asynchronously.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes # .
Changes proposed in this PR:
Summary by CodeRabbit
Bug Fixes
New Features