refactor: move hardcoded request type values to constants#7312
Open
sparkigniter wants to merge 4 commits intousebruno:mainfrom
Open
refactor: move hardcoded request type values to constants#7312sparkigniter wants to merge 4 commits intousebruno:mainfrom
sparkigniter wants to merge 4 commits intousebruno:mainfrom
Conversation
sparkigniter
requested review from
bijin-bruno,
helloanoop,
lohit-bruno and
naman-bruno
as code owners
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughReplaces scattered hard-coded request-type string literals with a centralized Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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 (7)
packages/bruno-app/src/components/RequestPane/QueryUrl/index.js (1)
89-92:⚠️ Potential issue | 🟡 MinorInconsistent:
handleGraphqlPastestill uses hardcoded string.
handleHttpPaste(line 164) correctly usesREQUEST_TYPES.HTTP_REQUEST, buthandleGraphqlPastestill checks against the literal'graphql-request'. Should useREQUEST_TYPES.GRAPHQL_REQUESTfor consistency.🔧 Proposed fix
const handleGraphqlPaste = useCallback((event) => { - if (item.type !== 'graphql-request') { + if (item.type !== REQUEST_TYPES.GRAPHQL_REQUEST) { return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/RequestPane/QueryUrl/index.js` around lines 89 - 92, Replace the hardcoded type check in handleGraphqlPaste with the shared enum constant: change the conditional that compares item.type to the string 'graphql-request' to instead compare to REQUEST_TYPES.GRAPHQL_REQUEST (consistent with handleHttpPaste which uses REQUEST_TYPES.HTTP_REQUEST), ensuring you import or reference REQUEST_TYPES where handleGraphqlPaste is defined.packages/bruno-app/src/components/ResponsePane/ResponsePaneActions/index.js (1)
166-166:⚠️ Potential issue | 🟡 MinorMissed constant usage: hardcoded string literal remains.
Line 166 still uses the hardcoded string
'graphql-request'while line 150 correctly usesREQUEST_TYPES.GRAPHQL_REQUEST. This should be consistent with the refactoring pattern.🔧 Proposed fix
- {item.type !== 'graphql-request' && <ResponseBookmark ref={bookmarkButtonRef} item={item} collection={collection} responseSize={responseSize} />} + {item.type !== REQUEST_TYPES.GRAPHQL_REQUEST && <ResponseBookmark ref={bookmarkButtonRef} item={item} collection={collection} responseSize={responseSize} />}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/ResponsePane/ResponsePaneActions/index.js` at line 166, Replace the hardcoded string check in ResponsePaneActions (the conditional rendering that uses item.type !== 'graphql-request') with the refactored constant: use REQUEST_TYPES.GRAPHQL_REQUEST instead of the string literal so it matches the earlier check and the new pattern (update the conditional that controls rendering of ResponseBookmark which references item.type to compare against REQUEST_TYPES.GRAPHQL_REQUEST).packages/bruno-app/src/components/Sidebar/NewRequest/index.js (2)
59-72:⚠️ Potential issue | 🟡 MinorInconsistent:
identifyCurlRequestTypemixes hardcoded strings with constants.Lines 61 and 67 use hardcoded
'graphql-request'while line 71 usesREQUEST_TYPES.HTTP_REQUEST. Apply constants consistently throughout.🔧 Proposed fix
const identifyCurlRequestType = (url, headers, body) => { if (url.endsWith('/graphql')) { - setCurlRequestTypeDetected('graphql-request'); + setCurlRequestTypeDetected(REQUEST_TYPES.GRAPHQL_REQUEST); return; } const contentType = headers?.find((h) => h.name.toLowerCase() === 'content-type')?.value; if (contentType && contentType.includes('application/graphql')) { - setCurlRequestTypeDetected('graphql-request'); + setCurlRequestTypeDetected(REQUEST_TYPES.GRAPHQL_REQUEST); return; } setCurlRequestTypeDetected(REQUEST_TYPES.HTTP_REQUEST); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/Sidebar/NewRequest/index.js` around lines 59 - 72, identifyCurlRequestType currently sets 'graphql-request' as a hardcoded string in two places; replace those with the matching constant from REQUEST_TYPES (e.g., REQUEST_TYPES.GRAPHQL_REQUEST) and use that constant everywhere instead of the literal, updating both the url.endsWith('/graphql') branch and the contentType check; if REQUEST_TYPES does not yet expose a GRAPHQL_REQUEST key, add it and use that key when calling setCurlRequestTypeDetected to keep constants consistent with the existing REQUEST_TYPES.HTTP_REQUEST usage.
549-568:⚠️ Potential issue | 🟡 MinorInconsistent: Dropdown
curlRequestTypeChangecalls use hardcoded strings.The dropdown items at lines 554 and 563 pass hardcoded strings
'http-request'and'graphql-request'tocurlRequestTypeChange. These should use the constants.🔧 Proposed fix
<div className="dropdown-item" onClick={() => { dropdownTippyRef.current.hide(); - curlRequestTypeChange('http-request'); + curlRequestTypeChange(REQUEST_TYPES.HTTP_REQUEST); }} > HTTP </div> <div className="dropdown-item" onClick={() => { dropdownTippyRef.current.hide(); - curlRequestTypeChange('graphql-request'); + curlRequestTypeChange(REQUEST_TYPES.GRAPHQL_REQUEST); }} > GraphQL </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/Sidebar/NewRequest/index.js` around lines 549 - 568, Replace the hardcoded strings passed to curlRequestTypeChange in the Dropdown items with the project constants for request types: instead of 'http-request' and 'graphql-request' use the defined constants (e.g., the module's HTTP and GRAPHQL request-type constants) and ensure the constants are imported at the top of the file; keep the existing Dropdown, dropdownTippyRef.current.hide(), and curlRequestTypeChange(...) calls but pass the constant identifiers rather than string literals.packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/RequestMethod/index.js (1)
6-10:⚠️ Potential issue | 🟡 MinorInconsistent:
getMethodFlagsstill uses hardcoded strings.Line 38 correctly uses
REQUEST_TYPESconstants, butgetMethodFlagsat lines 7-9 still compares against hardcoded strings. Apply constants consistently.🔧 Proposed fix
const getMethodFlags = (item) => ({ - isGrpc: item.type === 'grpc-request', - isWS: item.type === 'ws-request', - isGraphQL: item.type === 'graphql-request' + isGrpc: item.type === REQUEST_TYPES.GRPC_REQUEST, + isWS: item.type === REQUEST_TYPES.WS_REQUEST, + isGraphQL: item.type === REQUEST_TYPES.GRAPHQL_REQUEST });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/RequestMethod/index.js` around lines 6 - 10, getMethodFlags uses hardcoded type strings; update it to use the REQUEST_TYPES constants instead. Locate getMethodFlags and replace comparisons like item.type === 'grpc-request' / 'ws-request' / 'graphql-request' with the corresponding REQUEST_TYPES properties (e.g. REQUEST_TYPES.GRPC, REQUEST_TYPES.WS, REQUEST_TYPES.GRAPHQL or the exact keys used elsewhere in the codebase) so the function consistently relies on REQUEST_TYPES.packages/bruno-app/src/components/CreateUntitledRequest/index.js (1)
47-47:⚠️ Potential issue | 🟡 MinorInconsistent:
handleCreateGraphQLRequeststill uses hardcoded string.
handleCreateHttpRequest(line 25) usesREQUEST_TYPES.HTTP_REQUEST, buthandleCreateGraphQLRequestuses the literal'graphql-request'. Apply the constant consistently.🔧 Proposed fix
- requestType: 'graphql-request', + requestType: REQUEST_TYPES.GRAPHQL_REQUEST,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/CreateUntitledRequest/index.js` at line 47, The GraphQL create handler is using a hardcoded string; update handleCreateGraphQLRequest to use the same REQUEST_TYPES constant as handleCreateHttpRequest (i.e., replace 'graphql-request' with REQUEST_TYPES.GRAPHQL_REQUEST) and ensure REQUEST_TYPES is referenced/imported where handleCreateGraphQLRequest is defined so both handlers use the shared constant.packages/bruno-app/src/utils/collections/export.js (1)
11-19:⚠️ Potential issue | 🟡 MinorUID cleanup misses WebSocket requests and uses the wrong assertions path.
Line 11 excludes
REQUEST_TYPES.WS_REQUEST, and Line 16 targetsrequest.vars.assertionsinstead ofrequest.assertions. This leaves assertion/body/header-related UIDs behind in exported request payloads.🔧 Proposed fix
- if ([REQUEST_TYPES.HTTP_REQUEST, REQUEST_TYPES.GRAPHQL_REQUEST, REQUEST_TYPES.GRPC_REQUEST].includes(item.type)) { + if ([REQUEST_TYPES.HTTP_REQUEST, REQUEST_TYPES.GRAPHQL_REQUEST, REQUEST_TYPES.GRPC_REQUEST, REQUEST_TYPES.WS_REQUEST].includes(item.type)) { each(get(item, 'request.headers'), (header) => delete header.uid); each(get(item, 'request.params'), (param) => delete param.uid); each(get(item, 'request.vars.req'), (v) => delete v.uid); each(get(item, 'request.vars.res'), (v) => delete v.uid); - each(get(item, 'request.vars.assertions'), (a) => delete a.uid); + each(get(item, 'request.assertions'), (a) => delete a.uid); each(get(item, 'request.body.multipartForm'), (param) => delete param.uid); each(get(item, 'request.body.formUrlEncoded'), (param) => delete param.uid); each(get(item, 'request.body.file'), (param) => delete param.uid);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/utils/collections/export.js` around lines 11 - 19, The UID cleanup block that runs for HTTP/GraphQL/gRPC requests should also include WebSocket requests and should target the correct assertions path; update the includes array to add REQUEST_TYPES.WS_REQUEST and change the assertions cleanup from get(item, 'request.vars.assertions') to get(item, 'request.assertions') in the same block (the code around the conditional handling these items and the each(get(item, ...)) calls) so headers, params, body and assertions UIDs are removed for WS requests too.
🧹 Nitpick comments (1)
packages/bruno-app/src/components/RequestPane/QueryUrl/index.js (1)
104-104: Consider using constant for thegetRequestFromCurlCommandparameter.The hardcoded
'graphql-request'passed togetRequestFromCurlCommandcould useREQUEST_TYPES.GRAPHQL_REQUESTfor consistency.🔧 Suggested change
- const request = getRequestFromCurlCommand(pastedData, 'graphql-request'); + const request = getRequestFromCurlCommand(pastedData, REQUEST_TYPES.GRAPHQL_REQUEST);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/RequestPane/QueryUrl/index.js` at line 104, Replace the hardcoded string 'graphql-request' passed to getRequestFromCurlCommand with the enum/constant REQUEST_TYPES.GRAPHQL_REQUEST; update the call in QueryUrl (the getRequestFromCurlCommand(...) invocation) to use REQUEST_TYPES.GRAPHQL_REQUEST and add the necessary import for REQUEST_TYPES where it's defined so the code compiles and remains consistent with other request-type usages.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/index.js`:
- Line 94: The examples-check incorrectly restricts example expansion to GRAPHQL
requests: update the hasExamples expression in the CollectionItem component to
detect HTTP requests instead of REQUEST_TYPES.GRAPHQL_REQUEST by using the
correct HTTP request type constant from REQUEST_TYPES (so that
isItemARequest(item) && item.type === REQUEST_TYPES.<HTTP_REQUEST_CONSTANT> &&
item.examples && item.examples.length > 0); this will re-enable HTTP example
expansion and keep the examples boolean in sync with the component's render
gate.
In `@packages/bruno-app/src/utils/curl/index.js`:
- Line 7: The default parameter for getRequestFromCurlCommand is hardcoded to
'http-request' but the function body expects REQUEST_TYPES.HTTP_REQUEST; change
the function signature to use REQUEST_TYPES.HTTP_REQUEST as the default (ensure
REQUEST_TYPES is imported/available in this module) so the default and body are
consistent; update any local references if needed to match the constant name.
---
Outside diff comments:
In `@packages/bruno-app/src/components/CreateUntitledRequest/index.js`:
- Line 47: The GraphQL create handler is using a hardcoded string; update
handleCreateGraphQLRequest to use the same REQUEST_TYPES constant as
handleCreateHttpRequest (i.e., replace 'graphql-request' with
REQUEST_TYPES.GRAPHQL_REQUEST) and ensure REQUEST_TYPES is referenced/imported
where handleCreateGraphQLRequest is defined so both handlers use the shared
constant.
In `@packages/bruno-app/src/components/RequestPane/QueryUrl/index.js`:
- Around line 89-92: Replace the hardcoded type check in handleGraphqlPaste with
the shared enum constant: change the conditional that compares item.type to the
string 'graphql-request' to instead compare to REQUEST_TYPES.GRAPHQL_REQUEST
(consistent with handleHttpPaste which uses REQUEST_TYPES.HTTP_REQUEST),
ensuring you import or reference REQUEST_TYPES where handleGraphqlPaste is
defined.
In `@packages/bruno-app/src/components/ResponsePane/ResponsePaneActions/index.js`:
- Line 166: Replace the hardcoded string check in ResponsePaneActions (the
conditional rendering that uses item.type !== 'graphql-request') with the
refactored constant: use REQUEST_TYPES.GRAPHQL_REQUEST instead of the string
literal so it matches the earlier check and the new pattern (update the
conditional that controls rendering of ResponseBookmark which references
item.type to compare against REQUEST_TYPES.GRAPHQL_REQUEST).
In
`@packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/RequestMethod/index.js`:
- Around line 6-10: getMethodFlags uses hardcoded type strings; update it to use
the REQUEST_TYPES constants instead. Locate getMethodFlags and replace
comparisons like item.type === 'grpc-request' / 'ws-request' / 'graphql-request'
with the corresponding REQUEST_TYPES properties (e.g. REQUEST_TYPES.GRPC,
REQUEST_TYPES.WS, REQUEST_TYPES.GRAPHQL or the exact keys used elsewhere in the
codebase) so the function consistently relies on REQUEST_TYPES.
In `@packages/bruno-app/src/components/Sidebar/NewRequest/index.js`:
- Around line 59-72: identifyCurlRequestType currently sets 'graphql-request' as
a hardcoded string in two places; replace those with the matching constant from
REQUEST_TYPES (e.g., REQUEST_TYPES.GRAPHQL_REQUEST) and use that constant
everywhere instead of the literal, updating both the url.endsWith('/graphql')
branch and the contentType check; if REQUEST_TYPES does not yet expose a
GRAPHQL_REQUEST key, add it and use that key when calling
setCurlRequestTypeDetected to keep constants consistent with the existing
REQUEST_TYPES.HTTP_REQUEST usage.
- Around line 549-568: Replace the hardcoded strings passed to
curlRequestTypeChange in the Dropdown items with the project constants for
request types: instead of 'http-request' and 'graphql-request' use the defined
constants (e.g., the module's HTTP and GRAPHQL request-type constants) and
ensure the constants are imported at the top of the file; keep the existing
Dropdown, dropdownTippyRef.current.hide(), and curlRequestTypeChange(...) calls
but pass the constant identifiers rather than string literals.
In `@packages/bruno-app/src/utils/collections/export.js`:
- Around line 11-19: The UID cleanup block that runs for HTTP/GraphQL/gRPC
requests should also include WebSocket requests and should target the correct
assertions path; update the includes array to add REQUEST_TYPES.WS_REQUEST and
change the assertions cleanup from get(item, 'request.vars.assertions') to
get(item, 'request.assertions') in the same block (the code around the
conditional handling these items and the each(get(item, ...)) calls) so headers,
params, body and assertions UIDs are removed for WS requests too.
---
Nitpick comments:
In `@packages/bruno-app/src/components/RequestPane/QueryUrl/index.js`:
- Line 104: Replace the hardcoded string 'graphql-request' passed to
getRequestFromCurlCommand with the enum/constant REQUEST_TYPES.GRAPHQL_REQUEST;
update the call in QueryUrl (the getRequestFromCurlCommand(...) invocation) to
use REQUEST_TYPES.GRAPHQL_REQUEST and add the necessary import for REQUEST_TYPES
where it's defined so the code compiles and remains consistent with other
request-type usages.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
packages/bruno-app/src/components/CreateTransientRequest/index.jspackages/bruno-app/src/components/CreateUntitledRequest/index.jspackages/bruno-app/src/components/RequestPane/QueryUrl/index.jspackages/bruno-app/src/components/RequestTabPanel/index.jspackages/bruno-app/src/components/ResponsePane/ResponseBookmark/index.jspackages/bruno-app/src/components/ResponsePane/ResponsePaneActions/index.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/RequestMethod/index.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/index.jspackages/bruno-app/src/components/Sidebar/NewRequest/index.jspackages/bruno-app/src/providers/ReduxStore/slices/collections/actions.jspackages/bruno-app/src/utils/collections/export.jspackages/bruno-app/src/utils/collections/index.jspackages/bruno-app/src/utils/common/constants.jspackages/bruno-app/src/utils/curl/index.jspackages/bruno-app/src/utils/network/index.jspackages/bruno-app/src/utils/tabs/index.js
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/bruno-app/src/utils/collections/index.js (1)
854-856: Correct usage of constants, but inconsistent with other parts of the file.The change correctly replaces hardcoded strings with
REQUEST_TYPESconstants. However, there are remaining hardcoded request type strings in unchanged code within this same file:
- Line 303:
si.type === 'grpc-request'- Line 710:
_item.type === 'grpc-request'- Line 716:
_item.type === 'ws-request'- Line 723:
['grpc-request', 'ws-request'].includes(_item.type)Consider updating these for consistency in a follow-up or within this PR.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/utils/collections/index.js` around lines 854 - 856, Several places still use hardcoded request type strings; replace those literals with the REQUEST_TYPES constants for consistency. Specifically, update occurrences where code checks si.type === 'grpc-request' and where _item.type === 'grpc-request' / _item.type === 'ws-request' and the array check ['grpc-request','ws-request'].includes(_item.type) to use REQUEST_TYPES.GRPC_REQUEST and REQUEST_TYPES.WS_REQUEST (e.g., si.type === REQUEST_TYPES.GRPC_REQUEST, _item.type === REQUEST_TYPES.GRPC_REQUEST, _item.type === REQUEST_TYPES.WS_REQUEST, and [REQUEST_TYPES.GRPC_REQUEST, REQUEST_TYPES.WS_REQUEST].includes(_item.type)). Ensure you import or reference REQUEST_TYPES the same way as in isItemARequest so the module compiles.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/bruno-app/src/utils/collections/index.js`:
- Around line 854-856: Several places still use hardcoded request type strings;
replace those literals with the REQUEST_TYPES constants for consistency.
Specifically, update occurrences where code checks si.type === 'grpc-request'
and where _item.type === 'grpc-request' / _item.type === 'ws-request' and the
array check ['grpc-request','ws-request'].includes(_item.type) to use
REQUEST_TYPES.GRPC_REQUEST and REQUEST_TYPES.WS_REQUEST (e.g., si.type ===
REQUEST_TYPES.GRPC_REQUEST, _item.type === REQUEST_TYPES.GRPC_REQUEST,
_item.type === REQUEST_TYPES.WS_REQUEST, and [REQUEST_TYPES.GRPC_REQUEST,
REQUEST_TYPES.WS_REQUEST].includes(_item.type)). Ensure you import or reference
REQUEST_TYPES the same way as in isItemARequest so the module compiles.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/index.jspackages/bruno-app/src/providers/ReduxStore/slices/collections/actions.jspackages/bruno-app/src/utils/collections/index.js
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js
- packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/index.js
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.
Description
Contribution Checklist:
Note: Keeping the PR small and focused helps make it easier to review and merge. If you have multiple changes you want to make, please consider submitting them as separate pull requests.
🛠 Refactor: Centralize Request Type Constants
This PR removes hardcoded request type string literals across the codebase and replaces them with a centralized
REQUEST_TYPESconstant.🧹 What Changed
REQUEST_TYPESconstant to define all supported request types."http","graphql", etc.) with references toREQUEST_TYPES.🎯 Why This Change?
🧪 Impact
📌 Example
Before:
After:
Publishing to New Package Managers
Please see here for more information.
Summary by CodeRabbit