Move consumer secret input to dialog on Try Out page#1282
Merged
Conversation
|
mehara-rothila seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughMoved the inline consumer-secret input into a modal dialog gated by Changes
Sequence DiagramsequenceDiagram
participant User
participant TryOut as TryOutController
participant Dialog as ConsumerSecretDialog
participant Service as TokenService
User->>TryOut: Click "GET TEST KEY"
alt isConsumerSecretRequired
TryOut->>Dialog: open (secretDialogOpen = true)
Dialog->>User: prompt for secret
User->>Dialog: enter secret & click "Generate"
Dialog->>TryOut: generateAccessToken(secretOverride)
TryOut->>Service: application.generateToken(secretOverride)
Service-->>TryOut: return access token
TryOut->>Dialog: close
TryOut->>User: show token
else
TryOut->>Service: generateAccessToken()
Service-->>TryOut: return access token
TryOut->>User: show token
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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)
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 |
mehara-rothila
requested review from
Krishanx92,
lasanthaS,
piyumaldk and
tharikaGitHub
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
portals/devportal/src/main/webapp/source/src/app/components/Shared/ApiTryOut/TryOutController.jsx (2)
617-618: Consider removing dead code.Since
enableGetTestKeyButtonis now alwaystrue, the!enableGetTestKeyButtoncondition at line 998 is always false and effectively dead code. Consider either:
- Removing the
enableGetTestKeyButtonvariable and the condition at line 998- Or keeping it as a placeholder if there's intent to add conditional logic later
♻️ Suggested cleanup
- // Consumer secret is now collected via dialog, so GET TEST KEY is always enabled. - const enableGetTestKeyButton = true;And at line 998:
|| (!ksGenerated && securitySchemeType === 'OAUTH') - || !enableGetTestKeyButton} + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@portals/devportal/src/main/webapp/source/src/app/components/Shared/ApiTryOut/TryOutController.jsx` around lines 617 - 618, The constant enableGetTestKeyButton is always true, making the !enableGetTestKeyButton branch dead code; remove the unused constant declaration (enableGetTestKeyButton) and delete the conditional block that checks !enableGetTestKeyButton in TryOutController.jsx (or, if you intend to keep a toggle placeholder, retain the constant but remove or comment out the unreachable !enableGetTestKeyButton branch so there is no dead code).
862-874: Consider clearing the consumer secret after successful token generation.After the dialog closes and token generation is triggered, the
consumerSecretremains in component state. For improved security hygiene, consider clearing the sensitive secret after it's been used.🛡️ Suggested fix
<Button variant='contained' onClick={() => { setSecretDialogOpen(false); - generateAccessToken(consumerSecret); + const secretValue = consumerSecret; + setConsumerSecret(''); + generateAccessToken(secretValue); }} disabled={!consumerSecret?.trim()} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@portals/devportal/src/main/webapp/source/src/app/components/Shared/ApiTryOut/TryOutController.jsx` around lines 862 - 874, After generating the token, clear the stored consumer secret from state to avoid leaving sensitive data in memory: update the onClick handler on the Button that currently calls setSecretDialogOpen(false) and generateAccessToken(consumerSecret) so that after generateAccessToken completes successfully you also call setConsumerSecret('') (e.g., await generateAccessToken(consumerSecret) or use .then(...)) while still closing the dialog with setSecretDialogOpen(false); ensure you only clear consumerSecret on successful generation rather than on failure.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@portals/devportal/src/main/webapp/source/src/app/components/Shared/ApiTryOut/TryOutController.jsx`:
- Around line 617-618: The constant enableGetTestKeyButton is always true,
making the !enableGetTestKeyButton branch dead code; remove the unused constant
declaration (enableGetTestKeyButton) and delete the conditional block that
checks !enableGetTestKeyButton in TryOutController.jsx (or, if you intend to
keep a toggle placeholder, retain the constant but remove or comment out the
unreachable !enableGetTestKeyButton branch so there is no dead code).
- Around line 862-874: After generating the token, clear the stored consumer
secret from state to avoid leaving sensitive data in memory: update the onClick
handler on the Button that currently calls setSecretDialogOpen(false) and
generateAccessToken(consumerSecret) so that after generateAccessToken completes
successfully you also call setConsumerSecret('') (e.g., await
generateAccessToken(consumerSecret) or use .then(...)) while still closing the
dialog with setSecretDialogOpen(false); ensure you only clear consumerSecret on
successful generation rather than on failure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3df48390-c3f0-46bd-9b40-fcef63034268
📒 Files selected for processing (1)
portals/devportal/src/main/webapp/source/src/app/components/Shared/ApiTryOut/TryOutController.jsx
There was a problem hiding this comment.
Pull request overview
Moves the OAuth consumer secret input (when multiple client secrets are enabled) from an inline field into a modal dialog opened via GET TEST KEY, reducing Try Out page UI clutter and aligning with existing dialog patterns.
Changes:
- Introduces an MUI
Dialogto collect the consumer secret only when needed (multiple-secrets OAuth). - Updates
generateAccessTokento accept an optional secret override. - Adjusts UI hints/tooltips and keeps GET TEST KEY enabled, gating “Generate” on non-empty secret.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...ls/devportal/src/main/webapp/source/src/app/components/Shared/ApiTryOut/TryOutController.jsx
Show resolved
Hide resolved
...ls/devportal/src/main/webapp/source/src/app/components/Shared/ApiTryOut/TryOutController.jsx
Outdated
Show resolved
Hide resolved
...ls/devportal/src/main/webapp/source/src/app/components/Shared/ApiTryOut/TryOutController.jsx
Show resolved
Hide resolved
...ls/devportal/src/main/webapp/source/src/app/components/Shared/ApiTryOut/TryOutController.jsx
Outdated
Show resolved
Hide resolved
added 3 commits
March 24, 2026 17:23
When multiple client secrets are enabled, the consumer secret field was displayed inline on the DevPortal Try Out page. This moves the field into a popup dialog that opens when clicking GET TEST KEY, keeping the page clean and consistent with existing dialog patterns. Resolves #4809
… autoFocus - Remove unused enableGetTestKeyButton constant and its reference in disabled prop - Clear consumer secret when dialog is closed via onClose (consistency with Cancel) - Add autoFocus to consumer secret TextField for accessibility - Clear consumer secret state after generating token (security hygiene)
mehara-rothila
force-pushed
the
fix-4809-tryit-ui-improvements
branch
from
2065ea7 to
cec66be
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@portals/devportal/src/main/webapp/source/src/app/components/Shared/ApiTryOut/TryOutController.jsx`:
- Around line 799-801: The Dialog onClose handlers (e.g., the Dialog using
secretDialogOpen) only clear consumerSecret but neglect to reset the plaintext
toggle; update each onClose callback in TryOutController.jsx (all Dialog
instances that use secretDialogOpen / another dialog state) to also call
setShowSecret(false) so showSecret is reset whenever the dialog is dismissed
(e.g., change onClose={() => { setSecretDialogOpen(false);
setConsumerSecret(''); }} to include setShowSecret(false)). Ensure you apply the
same change to the other Dialog onClose handlers referenced (around the other
occurrences).
- Around line 862-870: The Generate buttons can fire concurrent token requests
because they're not gated by the loading flag; update the dialog's Generate
button and the page-level "GET TEST KEY" button to also be disabled when
isUpdating is true (e.g., change disabled={!consumerSecret?.trim()} to
disabled={isUpdating || !consumerSecret?.trim()} and add disabled={isUpdating}
to the GET TEST KEY button), and add a fast-guard inside
generateAccessToken(token) to return early if isUpdating is true to prevent
starting a second request while one is in progress; reference the
generateAccessToken function, the isUpdating state, and the
consumerSecret/setConsumerSecret handlers when making these changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5b090649-2c70-4f22-92b8-3cc58441f0cc
📒 Files selected for processing (1)
portals/devportal/src/main/webapp/source/src/app/components/Shared/ApiTryOut/TryOutController.jsx
...ls/devportal/src/main/webapp/source/src/app/components/Shared/ApiTryOut/TryOutController.jsx
Outdated
Show resolved
Hide resolved
...ls/devportal/src/main/webapp/source/src/app/components/Shared/ApiTryOut/TryOutController.jsx
Outdated
Show resolved
Hide resolved
mehara-rothila
force-pushed
the
fix-4809-tryit-ui-improvements
branch
from
77276ae to
46fb829
Compare
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
portals/devportal/src/main/webapp/source/src/app/components/Shared/ApiTryOut/TryOutController.jsx (1)
617-617: Minor: Consider clarifying the comment.The comment states GET TEST KEY is "always enabled," but the button still has disable conditions (no user, no subscriptions, keys not generated, isUpdating). Consider rephrasing to clarify that consumer secret is no longer a prerequisite for enabling the button.
📝 Suggested clarification
- // Consumer secret is now collected via dialog, so GET TEST KEY is always enabled. + // Consumer secret is now collected via dialog, so it no longer gates the GET TEST KEY button.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@portals/devportal/src/main/webapp/source/src/app/components/Shared/ApiTryOut/TryOutController.jsx` at line 617, Update the inline comment in TryOutController.jsx that reads "// Consumer secret is now collected via dialog, so GET TEST KEY is always enabled." to clarify that the consumer secret is no longer a prerequisite for enabling the "GET TEST KEY" button but other disable conditions (e.g., no user, no subscriptions, keys not generated, or isUpdating) still apply; locate the comment immediately before the GET TEST KEY enable/disable logic in the TryOutController component and rephrase it to reflect those remaining conditions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@portals/devportal/src/main/webapp/source/src/app/components/Shared/ApiTryOut/TryOutController.jsx`:
- Line 617: Update the inline comment in TryOutController.jsx that reads "//
Consumer secret is now collected via dialog, so GET TEST KEY is always enabled."
to clarify that the consumer secret is no longer a prerequisite for enabling the
"GET TEST KEY" button but other disable conditions (e.g., no user, no
subscriptions, keys not generated, or isUpdating) still apply; locate the
comment immediately before the GET TEST KEY enable/disable logic in the
TryOutController component and rephrase it to reflect those remaining
conditions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7f3d21dc-3a32-4a9d-86e4-a71a2051b234
📒 Files selected for processing (1)
portals/devportal/src/main/webapp/source/src/app/components/Shared/ApiTryOut/TryOutController.jsx
PasanT9
approved these changes
Mar 27, 2026
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
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.
Purpose
When multiple client secrets are enabled, the consumer secret field was displayed inline on the DevPortal Try Out page, cluttering the UI.
Approach
Moved the consumer secret input into a popup dialog (MUI
Dialog) that opens when clicking GET TEST KEY, consistent with existing patterns (e.g.,NewSecretDialog).TextFieldwithDialogcomponentFile changed:
portals/devportal/.../Shared/ApiTryOut/TryOutController.jsx(89 insertions, 55 deletions)Summary by CodeRabbit
New Features
Bug Fixes