fix: restore saved credentials when switching back to original auth mode#7911
fix: restore saved credentials when switching back to original auth mode#7911pooja-bruno wants to merge 1 commit intousebruno:mainfrom
Conversation
pooja-bruno
requested review from
bijin-bruno,
helloanoop,
lohit-bruno,
naman-bruno and
sid-bruno
as code owners
WalkthroughAuth-mode reducers for requests, collections, and folders now conditionally restore saved auth payloads when switching to a mode matching the underlying saved data. Three reducers ( ChangesAuth Mode Preservation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Suggested labels
Suggested reviewers
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 unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
tests/request/auth-mode-switch.spec.ts (3)
33-37: ⚖️ Poor tradeoff
readFieldis coupled to the CodeMirror v5 DOM API — worth adata-testidfallback.return editor.evaluate((el: any) => (el as any).CodeMirror?.getValue() ?? '');
el.CodeMirroris a CM5-specific instance property. If Bruno ever upgrades to CodeMirror 6 (which attaches its state differently), this evaluates to''for every assertion, silently turning allreadFieldassertions into vacuous passes. Wiring adata-testidattribute to the underlying<textarea>that CM5 maintains in sync, or anaria-label, would makereadFieldCM-version-agnostic.As per coding guidelines: "Add
data-testidto testable elements for Playwright."🤖 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 `@tests/request/auth-mode-switch.spec.ts` around lines 33 - 37, readField currently relies solely on the CodeMirror v5 DOM API (reading el.CodeMirror.getValue()), which will silently break if the editor is upgraded; update readField to first try the existing CodeMirror path and then fall back to reading the underlying <textarea> value via a test id or accessible label (e.g. query for textarea[data-testid="${labelText}"] or textarea[aria-label="${labelText}"] and return its .value) so assertions remain valid across CM versions; also ensure the component under test includes a stable data-testid on the editor textarea so fieldEditor/readField can locate it.
84-124: ⚡ Quick winCollection and Folder tests only cover the happy path — consider adding parity steps with the Request test.
The Request test (lines 44–82) includes 5 well-structured steps:
- Save Bearer credentials
- Bearer → Basic → Bearer restores token ✓ (the main fix)
- Switching to a non-saved mode shows empty fields
- Switching to a third unrelated mode also leaves fields empty
- Multiple round-trips to the saved mode still restore correctly
The Collection (lines 84–101) and Folder (lines 104–124) tests only cover step 2. If a bug were introduced that only affects Collection- or Folder-level auth mode switching (e.g., the
updateCollectionAuth/updateFolderAuthreducers that clear auth), steps 3–5 would silently pass in the Collection/Folder paths. Adding at least step 3 (non-saved mode shows empty fields) would give meaningful regression coverage for the collection/folder reducers independently.🤖 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 `@tests/request/auth-mode-switch.spec.ts` around lines 84 - 124, Add the missing parity check that switching to a non-saved auth mode clears fields: after the "Save Bearer" step in both the Collection and Folder tests (identify blocks that call createCollection/createFolder, selectAuthMode, typeIntoField, and click Save) add a step that selects a different mode that was never saved (e.g., selectAuthMode(page, 'API Key') or 'Basic Auth' as appropriate) and assert using readField(...) that the relevant fields are empty; then switch back to 'Bearer Token' and assert the saved token is restored with expect.poll(() => readField(page, 'Token')).toBe('collection-bearer-token' / 'folder-bearer-token') to mirror the Request test's non-saved-mode coverage.
16-21: 💤 Low valueStatic analysis flag on
new RegExp(...)is a false positive here.
labelTextis always passed as a static string literal ('Token','Username','Password') with no regex metacharacters, so the ReDoS warning is inapplicable. The exact-boundary regex (^...$) is the right approach to avoid partial-text false matches on labels like "Token" vs "Bearer Token."If you want to silence the linter cleanly:
✨ Suggested fix
- .filter({ hasText: new RegExp(`^${labelText}$`) }) + .filter({ hasText: labelText, exact: true })Playwright's
filter({ hasText: ..., exact: true })does case-sensitive exact string matching, removing the regex entirely.🤖 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 `@tests/request/auth-mode-switch.spec.ts` around lines 16 - 21, The linter flags the use of new RegExp in the fieldEditor helper; replace the regex-based match with Playwright's exact string match to silence the false positive and keep exact-boundary behavior: update the fieldEditor function (named fieldEditor) to use filter({ hasText: labelText, exact: true }) instead of filter({ hasText: new RegExp(`^${labelText}$`) }) so it performs case-sensitive exact matching for labels like "Token" vs "Bearer Token".
🤖 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.
Nitpick comments:
In `@tests/request/auth-mode-switch.spec.ts`:
- Around line 33-37: readField currently relies solely on the CodeMirror v5 DOM
API (reading el.CodeMirror.getValue()), which will silently break if the editor
is upgraded; update readField to first try the existing CodeMirror path and then
fall back to reading the underlying <textarea> value via a test id or accessible
label (e.g. query for textarea[data-testid="${labelText}"] or
textarea[aria-label="${labelText}"] and return its .value) so assertions remain
valid across CM versions; also ensure the component under test includes a stable
data-testid on the editor textarea so fieldEditor/readField can locate it.
- Around line 84-124: Add the missing parity check that switching to a non-saved
auth mode clears fields: after the "Save Bearer" step in both the Collection and
Folder tests (identify blocks that call createCollection/createFolder,
selectAuthMode, typeIntoField, and click Save) add a step that selects a
different mode that was never saved (e.g., selectAuthMode(page, 'API Key') or
'Basic Auth' as appropriate) and assert using readField(...) that the relevant
fields are empty; then switch back to 'Bearer Token' and assert the saved token
is restored with expect.poll(() => readField(page,
'Token')).toBe('collection-bearer-token' / 'folder-bearer-token') to mirror the
Request test's non-saved-mode coverage.
- Around line 16-21: The linter flags the use of new RegExp in the fieldEditor
helper; replace the regex-based match with Playwright's exact string match to
silence the false positive and keep exact-boundary behavior: update the
fieldEditor function (named fieldEditor) to use filter({ hasText: labelText,
exact: true }) instead of filter({ hasText: new RegExp(`^${labelText}$`) }) so
it performs case-sensitive exact matching for labels like "Token" vs "Bearer
Token".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 29932aca-edfd-42f5-868b-6a6d426a9699
📒 Files selected for processing (2)
packages/bruno-app/src/providers/ReduxStore/slices/collections/index.jstests/request/auth-mode-switch.spec.ts
1 participant
Description
JIRA
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.
Publishing to New Package Managers
Please see here for more information.
Summary by CodeRabbit
Bug Fixes