feat(web): add editable request params panel#118
Conversation
Implement full CRUD support for query parameters in the request workspace. Adds draft controller for param state management, editable form UI with add/remove/save/discard actions, and utilities to rebuild URLs with updated query strings. Params now persist to the .http file on save.
Greptile SummaryThis PR promotes the query-params tab from a read-only display to a full edit-save-discard panel, mirroring the existing headers and body draft controllers. The utility work (
Confidence Score: 2/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant UI as UI (ParamsPanel)
participant PDC as ParamDraftController
participant HKWSP as useHttpRequestWorkspace
participant PEDF as persistAllDirtyDrafts
participant RE as request-editing utils
participant WS as WorkspaceStore
UI->>PDC: onParamChange / onAddParam / onRemoveParam
PDC->>PDC: setDraft('params', ...) + markDirty()
UI->>PDC: onSave()
PDC->>RE: buildUrlWithQueryRows(sourceUrl, draft.params)
RE-->>PDC: nextUrl
PDC->>RE: applyRequestEditsToContent(content, requestIndex, nextUrl, sourceHeaders)
RE-->>PDC: rewrite result
PDC->>WS: setFileContent(nextContent)
PDC->>WS: saveFile(path)
WS-->>PDC: saved
PDC->>PDC: setDraft('isDirty', false)
PDC->>WS: reloadRequests(path)
PDC->>WS: refetchRequestDetails()
Note over PEDF,PDC: On Ctrl+S / Execute
PEDF->>PDC: isDirty()?
PDC-->>PEDF: true/false
alt dirty
PEDF->>PDC: onSave()
PDC-->>PEDF: resolved
PEDF->>PDC: isDirty()? (check for save error)
PDC-->>PEDF: false (success) / true (error → abort)
end
PEDF->>HKWSP: headerDraft.onSave() (if dirty)
PEDF->>HKWSP: bodyDraft.onSave() (if dirty)
Last reviewed commit: 04f15a7 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
packages/web/src/components/request-workspace/use-request-param-draft-controller.ts
Outdated
Show resolved
Hide resolved
| const nextUrl = buildUrlWithQueryRows(sourceUrl, draft.params); | ||
| const rewrite = applyRequestEditsToContent( | ||
| currentContent, | ||
| request.index, | ||
| nextUrl, | ||
| input.sourceHeaders() | ||
| ); |
There was a problem hiding this comment.
Unsaved changes on other tabs are silently dropped on save
When onSave is called, it rewrites the file using input.sourceHeaders() — the persisted (server-side) headers, not any in-progress header draft edits. If a user simultaneously has unsaved header changes and then saves their param edits, their header draft is silently discarded from the written file.
The same issue exists in the opposite direction: saving headers calls applyRequestEditsToContent with the saved URL (not the current param draft). This is an architectural constraint of the independent draft model, but it may surprise users. At minimum, a comment explaining this limitation would help future maintainers, and ideally the UI could warn when another tab has unsaved changes before allowing a save.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 029b07d7cc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return [encodedKey]; | ||
| } | ||
|
|
||
| return [`${encodedKey}=${encodeQueryComponent(row.value)}`]; |
There was a problem hiding this comment.
This re-encodes every edited query value with encodeURIComponent, which converts {{...}} placeholders into %7B%7B...%7D%7D; in this codebase interpolation depends on literal braces (see packages/core/src/interpolate.ts), so saving params can silently break runtime variables/resolvers in URLs such as ?t={{$timestamp()}} and send incorrect requests.
Useful? React with 👍 / 👎.
packages/web/src/components/request-workspace/use-request-param-draft-controller.ts
Outdated
Show resolved
Hide resolved
| const persistRewrite = async (path: string, nextContent: string) => { | ||
| input.setFileContent(nextContent); | ||
| setDraft('isSaving', true); | ||
| setDraft('saveError', undefined); | ||
|
|
||
| try { | ||
| await input.saveFile(path); | ||
| setDraft('isDirty', false); | ||
| await input.reloadRequests(path); | ||
| await input.refetchRequestDetails(); | ||
| } catch (error) { | ||
| setDraft('saveError', toErrorMessage(error)); | ||
| } finally { | ||
| setDraft('isSaving', false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Stale isDirty flag after request switch mid-save
persistRewrite is an async function that holds references to setDraft over multiple await points. If the user switches to a different request between await input.saveFile(path) completing and the final setDraft('isDirty', false) / setDraft('isSaving', false) calls, those mutations are applied to the new request's draft state, not the one that was being saved.
A concrete race scenario:
- User edits request A's params →
isDirty = true - User clicks Save →
isSaving = true, save begins - While the
saveFilenetwork call is in-flight, user switches to request B and makes edits →replaceDraftresets draft to B's state,markDirty()setsisDirty = true - Request A's
saveFilecompletes →setDraft('isDirty', false)incorrectly clears request B's dirty flag - B's unsaved changes are now silently invisible — the Unsaved badge disappears and Save is disabled
A guard can prevent the late callbacks from touching unrelated state:
const persistRewrite = async (path: string, nextContent: string, forRequestKey: string) => {
input.setFileContent(nextContent);
setDraft('isSaving', true);
setDraft('saveError', undefined);
try {
await input.saveFile(path);
if (draft.requestKey !== forRequestKey) return; // request changed mid-save
setDraft('isDirty', false);
await input.reloadRequests(path);
await input.refetchRequestDetails();
} catch (error) {
if (draft.requestKey !== forRequestKey) return;
setDraft('saveError', toErrorMessage(error));
} finally {
if (draft.requestKey === forRequestKey) {
setDraft('isSaving', false);
}
}
};And call it as await persistRewrite(currentPath, rewrite.content, requestDraftKey() ?? '').
| const onSave = async () => { | ||
| if (draft.isSaving) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
onSave proceeds even when the draft is clean
onSave guards against a concurrent save (draft.isSaving) but never guards against a redundant save when draft.isDirty is false. As a result, calling onSave on a clean draft (e.g. via a programmatic path or a keyboard shortcut that bypasses the disabled-button guard) will silently rewrite and overwrite the .http file with identical content, triggering a full reloadRequests + refetchRequestDetails cycle unnecessarily.
| const onSave = async () => { | |
| if (draft.isSaving) { | |
| return; | |
| } | |
| const onSave = async () => { | |
| if (draft.isSaving || !draft.isDirty) { | |
| return; | |
| } |
| const persistAllDirtyDrafts = async (): Promise<boolean> => { | ||
| const drafts = [ | ||
| httpWorkspace.drafts.param, | ||
| httpWorkspace.drafts.header, | ||
| httpWorkspace.drafts.body | ||
| ]; | ||
|
|
||
| for (const draft of drafts) { | ||
| if (!draft.isDirty()) { | ||
| continue; | ||
| } | ||
|
|
||
| await draft.onSave(); | ||
| if (draft.isDirty()) { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| return true; | ||
| }; |
There was a problem hiding this comment.
Partial-save leaves state inconsistent with no user feedback
persistAllDirtyDrafts saves drafts sequentially and returns false on the first failure. When more than one draft is dirty, earlier drafts that succeeded are already flushed to disk, but later drafts that haven't run yet are silently skipped. The function returns false to the caller, which only acts as a gate — it does not surface which drafts were saved and which weren't.
For example: if a user has both param edits and header edits outstanding and the param save fails:
- The save error appears in the Params panel
- The header draft is not saved but remains dirty
handleHttpSave(andhandleHttpExecute) both bail out, so the file is not sent
But if the param save succeeds and the header save fails:
- Param changes are now on disk
- Header changes are still in the draft, shown as "Unsaved"
persistAllDirtyDraftsreturnsfalse, blocking execution- The user has no indication that their params were already written
At minimum, a code comment explaining this partial-save contract would help future maintainers reason about the failure modes. Ideally each tab's save error should remain visible so the user understands which draft failed.
| let paramDraftRef: ReturnType<typeof useRequestParamDraftController> | undefined; | ||
|
|
||
| // Keep cross-tab edits in sync: header saves include unsaved param drafts when present. | ||
| const sourceUrlForHeaderSave = () => { | ||
| const requestUrl = selectedRequest()?.url; | ||
| if (!requestUrl) { | ||
| return undefined; | ||
| } | ||
|
|
||
| const paramDraft = paramDraftRef; | ||
| if (!paramDraft || !paramDraft.isDirty()) { | ||
| return requestUrl; | ||
| } | ||
|
|
||
| return buildUrlWithQueryRows(requestUrl, paramDraft.draftParams()); | ||
| }; |
There was a problem hiding this comment.
paramDraftRef introduces a mutable temporal dependency
paramDraftRef is declared as undefined, used inside sourceUrlForHeaderSave (which is passed to headerDraft at initialization time), and then assigned after headerDraft is constructed:
let paramDraftRef: ReturnType<typeof useRequestParamDraftController> | undefined;
const sourceUrlForHeaderSave = () => {
const paramDraft = paramDraftRef; // undefined during headerDraft init
...
};
const headerDraft = useRequestHeaderDraftController({ sourceUrl: sourceUrlForHeaderSave, ... });
// ...
const paramDraft = useRequestParamDraftController(...);
paramDraftRef = paramDraft; // assigned afterThis is safe only because sourceUrlForHeaderSave is never called during headerDraft's constructor — it is only invoked later when the user triggers a save. But this is an implicit contract not enforced by the type system. If useRequestHeaderDraftController's initialization ever calls sourceUrl() eagerly (e.g. to pre-populate state), paramDraftRef will be undefined and the cross-tab sync will silently degrade to returning the raw requestUrl.
A more robust pattern would be to initialise paramDraft first and pass a lambda that closes over the already-initialised value, or to restructure so headerDraft is initialized after paramDraft.
|
rolling this up myself |
1 participant
Summary
This PR implements an editable query parameters panel for the request workspace, bringing it to feature parity with headers and body editing.
Changes: