fix: API editor query parameters not auto-updating URL #40133
Conversation
WalkthroughThis pull request introduces a new custom hook, Changes
Sequence Diagram(s)sequenceDiagram
participant T as TabbedControls
participant H as useSyncParamsToPath
participant R as Redux
participant U as URL Manager
T->>H: Initialize with form name & config property
H->>R: Retrieve current form state (useSelector)
H->>U: Extract and compare URL query parameters
alt Parameters changed
H->>R: Dispatch state update (useDispatch)
H->>U: Update URL with new query parameters
end
H-->>T: Return synchronized state
sequenceDiagram
participant S as syncApiParamsSaga
participant E as API Editor Form
participant A as Action Configuration
S->>E: Autofill API editor with current path & parameters
S->>A: Dispatch yield put updating combined API path
A-->>S: Confirm updated action configuration
Assessment against linked issues
Poem
✨ Finishing Touches
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. 🪧 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
Documentation and Community
|
github-actions
bot
added
Bug
vivek-appsmith
changed the title
|
🔴 There's new test files in JS, please port to TS and re-trigger Cypress tests:
|
1 similar comment
|
🔴 There's new test files in JS, please port to TS and re-trigger Cypress tests:
|
vivek-appsmith
force-pushed
the
fix/40045-api-query-params-url-update
branch
from
4755137 to
ca36a96
Compare
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/14312547407. |
|
Deploy-Preview-URL: https://ce-40133.dp.appsmith.com |
|
/ok-to-test |
|
Whoops! Looks like you're using an outdated method of running the Cypress suite. |
|
🔴 There's new test files in JS, please port to TS and re-trigger Cypress tests:
|
1 similar comment
|
🔴 There's new test files in JS, please port to TS and re-trigger Cypress tests:
|
vivek-appsmith
force-pushed
the
fix/40045-api-query-params-url-update
branch
from
01e66c8 to
668ece1
Compare
vivek-appsmith
force-pushed
the
fix/40045-api-query-params-url-update
branch
2 times, most recently
from
401d4ac to
6de39d5
Compare
|
Test cases not created, can be taken up as a different task. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app/client/src/components/formControls/CustomActionsConfigControl/index.tsx (2)
48-75: Validate duplicate keys scenario.
The mapping approach correctly checks functional equivalence but silently merges duplicate keys. Confirm if duplicates are expected or need distinct handling.
77-222: Hook logic is solid.
The ref-based checks effectively prevent loops. Consider splitting the sync logic to smaller private functions for clarity if the logic grows.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/client/src/components/formControls/CustomActionsConfigControl/index.tsx(3 hunks)app/client/src/sagas/ApiPaneSagas.ts(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
app/client/src/sagas/ApiPaneSagas.ts (1)
app/client/src/actions/pluginActionActions.ts (1)
setActionProperty(315-322)
⏰ Context from checks skipped due to timeout of 90000ms (11)
- GitHub Check: perform-test / ci-test / ci-test (16)
- GitHub Check: perform-test / ci-test / ci-test (15)
- GitHub Check: perform-test / ci-test / ci-test (14)
- GitHub Check: perform-test / ci-test / ci-test (13)
- GitHub Check: perform-test / ci-test / ci-test (10)
- GitHub Check: perform-test / ci-test / ci-test (5)
- GitHub Check: perform-test / ci-test / ci-test (4)
- GitHub Check: perform-test / ci-test / ci-test (3)
- GitHub Check: perform-test / ci-test / ci-test (2)
- GitHub Check: perform-test / ci-test / ci-test (1)
- GitHub Check: perform-test / ci-test / ci-test (0)
🔇 Additional comments (3)
app/client/src/sagas/ApiPaneSagas.ts (1)
127-135: Looks good.
This additional dispatch call properly synchronizes the action's path. There are no immediate concerns.app/client/src/components/formControls/CustomActionsConfigControl/index.tsx (2)
1-17: No issues with these imports.
They accurately bring in Redux utilities, form selectors, and helper functions needed for syncing.
225-226: Great integration of the new hook.
CallinguseSyncParamsToPathinTabbedControlsensures query params stay in sync with the path.
API Editor Query Parameters URL Auto-Update Fix
Issue
When adding or editing query parameters in the API editor, the URL wasn't automatically being updated to reflect these changes. This creates a disconnect between the parameters in the Params tab and the actual URL being used, as reported in issue #40045.
Fix
Implemented proper synchronization between query parameters and the URL in the API editor using a two-pronged approach:
Component-level solution with React hooks:
useSyncParamsToPathhook that handles bidirectional synchronizationRedux saga enhancement:
syncApiParamsSagato ensure the action model is updated when params changeThese changes ensure:
Changes
syncApiParamsSagato handle API action data updatesTest Strategy
No new test cases were added for the following reasons:
Related Issue
Fixes #40045
Automation
/ok-to-test tags="@tag.Datasource"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/14332565088
Commit: 6de39d5
Cypress dashboard.
Tags:
@tag.DatasourceSpec:
Tue, 08 Apr 2025 12:29:38 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit