fix(ENG-11467): Add secret field editing with placeholder view#51
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds functionality to handle secret field editing with placeholder views, allowing users to edit previously saved secret values in the integration picker. The implementation introduces a placeholder pattern (__secretvalue:**redacted**) to mask secret values while providing an "Edit" button to update them.
Key changes:
- New utility functions to detect and format secret placeholder values
- State management for tracking which secret fields are being edited
- UI updates to show masked values with an edit button for secret fields
- Cleanup logic to exclude unmodified secret placeholders from form submission
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
src/modules/integration-picker/utils/secretPlaceholder.ts |
New utility module with functions to identify and format secret placeholder strings |
src/modules/integration-picker/types.ts |
Added optional secrets field to AccountData interface for storing secret values |
src/modules/integration-picker/hooks/useIntegrationPicker.ts |
Added editingSecrets state management and logic to handle secret field values from accountData.secrets |
src/modules/integration-picker/components/IntegrationFields.tsx |
Implemented UI for displaying masked secret values with edit functionality and conditional field rendering |
src/modules/integration-picker/components/views/IntegrationFormView.tsx |
Passed editingSecrets props through to child components |
src/modules/integration-picker/components/IntegrationPickerContent.tsx |
Passed editingSecrets props through to child components |
src/modules/integration-picker/IntegrationPicker.tsx |
Passed editingSecrets state from hook to child components |
package.json |
Updated @stackone/malachite dependency from ^0.24.0 to ^0.24.1 |
package-lock.json |
Updated lock file to reflect the new @stackone/malachite version |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }: UseIntegrationPickerProps) => { | ||
| const [selectedIntegration, setSelectedIntegration] = useState<Integration | null>(null); | ||
| const [formData, setFormData] = useState<Record<string, string>>({}); | ||
| const [editingSecrets, setEditingSecrets] = useState<Set<string>>(new Set()); |
There was a problem hiding this comment.
The editingSecrets state is not reset when the integration changes or when forms are reset. This could lead to stale editing state being carried over when:
- A user switches between different integrations
- The form is reset via
resetConnectionStateorresetAllErrors
Consider adding an effect to clear editingSecrets when selectedIntegration changes:
useEffect(() => {
setEditingSecrets(new Set());
}, [selectedIntegration]);Or include it in the reset functions at lines 674 and 678.
There was a problem hiding this comment.
this might be relevant @joeStackOne can you verify it's not a concern?
There was a problem hiding this comment.
I don't see when an integration will change but added it as a defensive mechanism in the future when we want to change an already connected integration completely.
Summary by cubic
Shows and edits secret fields correctly when editing an account. Secrets now display as redacted placeholders with an Edit action, fixing the blank secret fields reported in ENG-11467.
Bug Fixes
Dependencies
Written for commit 31a1489. Summary will update automatically on new commits.