[MAIN-IMP] Add job-focused proxy link dialog and fix dropdown stacking interactions - #107
Merged
moda20 merged 5 commits intoAug 27, 2026
Conversation
disabling pointer events on job dropdown when another dialog is on top of it (same for the search bar)
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds end-to-end job-proxy linking with new proxy schemas, service endpoints, hook mutations, dialog UI, and jobs-table integration. It also refactors ChangesJob-Proxy Linking Feature
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ActionDropdown
participant JobProxyLinkDialog
participant useProxies
participant systemService
User->>ActionDropdown: selects Linked Proxies
ActionDropdown->>JobProxyLinkDialog: opens with job details
JobProxyLinkDialog->>systemService: getJobProxies(jobId)
systemService-->>JobProxyLinkDialog: linked proxy IDs
User->>JobProxyLinkDialog: selects strategy and proxies
JobProxyLinkDialog->>useProxies: proxyActions(LINK_TO_JOBS, data)
useProxies->>systemService: addProxiesToASingleJob(jobId, proxyIds)
systemService-->>useProxies: update response
JobProxyLinkDialog->>ActionDropdown: onProxyStrategyChange(strategy, proxyId)
ActionDropdown->>ActionDropdown: dispatches jobActions.UPDATE
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/components/custom/general/ScrollableList.tsxParsing error: ESLint was configured to run on
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. Comment |
Update managedSelect component to better handle keyboard shortcuts and disabled prop
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/components/custom/jobsTable/actionDropdown.tsx`:
- Around line 174-186: The updateProxyStrategy callback function uses
JSON.parse() without error handling on row.param, which can throw an exception
if the param contains invalid JSON and break the proxy strategy update action.
Wrap the JSON.parse(row.param || "{}") call in a try-catch block to safely
handle parsing errors, falling back to an empty object {} if the parse fails,
ensuring the function continues execution and properly updates the proxyConfig
regardless of the param validity.
In `@src/components/custom/system/JobProxyLinkDialog.tsx`:
- Around line 69-72: The JSON.parse call in the useMemo hook for proxyConfig
lacks error handling and will throw an exception if jobDetails.param contains
malformed JSON, breaking the dialog render. Wrap the JSON.parse statement in a
try-catch block in the useMemo function to defensively handle parsing errors,
and return an empty object {} as the fallback when parsing fails. This will
ensure the dialog can still open even with invalid param data.
- Around line 164-168: The callback function passed to form.handleSubmit is not
returning the Promise from the handleSubmit(v) call, which causes async failures
and submission state to not propagate correctly. In the onSubmit handler within
the form.handleSubmit callback, return the Promise from handleSubmit(v) so that
the async operation completes and any errors are properly handled by the form
submission chain. This ensures that submission state remains accurate and
failures propagate as expected.
- Around line 75-87: The form.setValue() call in the select callback of useQuery
executes before the form object is declared, which will cause a runtime error.
Move the useForm hook initialization (with JobProxyLinkUpdateSchema) above the
useQuery hook so form is available. Then remove the form.setValue side effect
from the select callback entirely. Create a separate useEffect hook that watches
the jobProxyList data and calls form.setValue when the data changes. Finally,
add an enabled condition to the useQuery to check that jobDetails?.id exists
before executing the query, preventing unnecessary API calls when the job ID is
missing.
In `@src/models/proxies.ts`:
- Around line 79-84: The JobProxyLinkUpdateSchema has a contract mismatch
between the schema definition and its usage. The proxies field is marked as
optional with z.array(z.any()).optional() but the consumer code in useProxies
assumes it's always present and directly calls .map() on it, creating a
potential runtime error. Additionally, strategy accepts any string but the form
enforces ProxyStrategyOptionEnum values. Remove the .optional() modifier from
the proxies field to make it required since JobProxyLinkDialog always provides a
proxies array, and replace the z.string().optional() for strategy with a
z.enum() or z.union() constraint that validates only the allowed
ProxyStrategyOptionEnum values, ensuring the schema accurately reflects the
actual contract between the form producer and consumer hook.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c94580f5-1115-40da-b5e6-87fa5d141787
📒 Files selected for processing (11)
package.jsonsrc/components/custom/DrawerMenuConfigurator.tsxsrc/components/custom/ManagedSelect.tsxsrc/components/custom/SearchBar.tsxsrc/components/custom/jobsTable/actionDropdown.tsxsrc/components/custom/system/JobProxyLinkDialog.tsxsrc/components/custom/system/ProxyConfigDialog.tsxsrc/components/ui/input-base.tsxsrc/hooks/useProxies.tsxsrc/models/proxies.tssrc/services/SystemService.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/components/custom/system/JobProxyLinkDialog.tsx (2)
97-105: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueMissing dependencies in
handlePickingStrategycallback.The callback references
form.setValueandstrategySelectRef.currentbut has an empty dependency array. WhileformfromuseFormis typically stable, the explicit empty array[]can be misleading. Consider addingformto deps for clarity.Suggested fix
}, - [], + [form], )🤖 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/custom/system/JobProxyLinkDialog.tsx` around lines 97 - 105, The handlePickingStrategy callback function uses the form.setValue method but has an empty dependency array. Add the form object to the dependency array of the useCallback hook for clarity and to properly declare all dependencies used within the callback. This ensures the callback correctly tracks its external dependencies and prevents potential issues if the form reference were to change.
107-134:⚠️ Potential issue | 🟠 Major | ⚡ Quick winIncorrect and missing dependencies in
handleSubmitcallback.The dependency array includes
isDialogOpenwhich is not used in the callback body, while missing several identifiers that are used:proxyActions,onProxyStrategyChange, andsetDialogState. This can cause stale closure bugs.Proposed fix
}, - [onChange, isDialogOpen, proxyConfig], + [onChange, proxyActions, proxyConfig, onProxyStrategyChange, setDialogState], )🤖 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/custom/system/JobProxyLinkDialog.tsx` around lines 107 - 134, The dependency array for the handleSubmit useCallback is incomplete and includes an unused dependency. Remove isDialogOpen from the dependency array since it is not used in the callback body. Add the missing dependencies that are actually used within the callback: proxyActions which is invoked to link jobs, onProxyStrategyChange which is called conditionally based on strategy changes, and setDialogState which is called to close the dialog. This will prevent stale closure bugs and ensure the callback is properly memoized.src/components/custom/jobsTable/actionDropdown.tsx (1)
176-196:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winLogic continues after parse error, corrupting data.
When
safeJsonParsereturns an error,oldParamcontains theErrorobject (not a parsed result). The code then callsset(oldParam, ...)on the Error object and serializes it withJSON.stringify, sending corrupted data to the backend. The function should return early on error or resetoldParamto{}.Proposed fix
const updateProxyStrategy = useCallback( (newStrategy?: string, specificProxyId?: string) => { - const { data: oldParam, error } = safeJsonParse(row.param || "{}") + let { data: oldParam, error } = safeJsonParse(row.param || "{}") if (error) { toast({ title: "Failed to parse Job params. this is a JSON parsing issue.", - description: oldParam?.toString(), + description: (oldParam as Error)?.message, variant: "destructive", }) + oldParam = {} // Reset to empty object to avoid corrupting data } set(oldParam, "proxyConfig.proxyStrategy", newStrategy)🤖 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/custom/jobsTable/actionDropdown.tsx` around lines 176 - 196, In the updateProxyStrategy function, when safeJsonParse detects an error, oldParam is set to the Error object instead of the parsed data, but the code continues to process it with the set() and unset() calls, causing corrupted data to be serialized. Add an early return statement immediately after the error toast is shown to prevent further processing when JSON parsing fails, or alternatively reset oldParam to an empty object {} when an error is detected so that subsequent operations work on valid data.
🤖 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.
Outside diff comments:
In `@src/components/custom/jobsTable/actionDropdown.tsx`:
- Around line 176-196: In the updateProxyStrategy function, when safeJsonParse
detects an error, oldParam is set to the Error object instead of the parsed
data, but the code continues to process it with the set() and unset() calls,
causing corrupted data to be serialized. Add an early return statement
immediately after the error toast is shown to prevent further processing when
JSON parsing fails, or alternatively reset oldParam to an empty object {} when
an error is detected so that subsequent operations work on valid data.
In `@src/components/custom/system/JobProxyLinkDialog.tsx`:
- Around line 97-105: The handlePickingStrategy callback function uses the
form.setValue method but has an empty dependency array. Add the form object to
the dependency array of the useCallback hook for clarity and to properly declare
all dependencies used within the callback. This ensures the callback correctly
tracks its external dependencies and prevents potential issues if the form
reference were to change.
- Around line 107-134: The dependency array for the handleSubmit useCallback is
incomplete and includes an unused dependency. Remove isDialogOpen from the
dependency array since it is not used in the callback body. Add the missing
dependencies that are actually used within the callback: proxyActions which is
invoked to link jobs, onProxyStrategyChange which is called conditionally based
on strategy changes, and setDialogState which is called to close the dialog.
This will prevent stale closure bugs and ensure the callback is properly
memoized.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3e10f237-5e7e-4a1f-b269-0fbfcfa09ff9
📒 Files selected for processing (5)
src/components/custom/ManagedSelect.tsxsrc/components/custom/jobsTable/actionDropdown.tsxsrc/components/custom/system/JobProxyLinkDialog.tsxsrc/services/SystemService.tssrc/utils/generalUtils.ts
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.
Improvements:
ProxyUpdateSchemaandProxyConfigUpdateTypeto shared models/proxies.tsNotes
getJobProxiesandaddProxiesToJobDrawerMenuConfiguratorhotkeys to only fire when drawer is open, this was a bug.How Has This Been Tested?
Related Issues
Screenshots (if applicable)
Additional Context
Summary by CodeRabbit