-
Notifications
You must be signed in to change notification settings - Fork 1.9k
fix(oci): pass provider UID to update credentials forms #9746
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(oci): pass provider UID to update credentials forms #9746
Conversation
The update credentials flow was missing the provider UID that is required for OCI form validation. The OCI tenancy field is a hidden field that gets auto-populated from the provider UID, but it was empty in the update flow causing form validation to fail silently. Changes: - Fetch provider data in update-credentials page to get the UID - Pass providerUid prop to UpdateViaCredentialsForm - Pass providerUid prop to UpdateViaRoleForm - Pass providerUid prop to UpdateViaServiceAccountForm
|
Please add an entry to the corresponding |
|
✅ Conflict Markers Resolved All conflict markers have been successfully resolved in this pull request. |
🔒 Container Security ScanImage: ✅ No Vulnerabilities DetectedThe container image passed all security checks. No known CVEs were found.📋 Resources:
|
alejandrobailo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @andoniaf , few things to improve
Before Merge:
1. Add error handling for getProvider failure (critical)
2. Add E2E test for OCI update flow with UID validation
3. Consider validation for missing/invalid providerId
Nice to Have:
- Extract FormData creation pattern to helper if used elsewhere
…-update-credentials-missing-uid
Address PR review comments: - Add error handling for getProvider failure (redirect to /providers) - Add validation for missing providerId (redirect to /providers) - Add E2E test for OCI update credentials flow (@PROVIDER-E2E-013) - Add page object methods for update credentials interactions - Document new test case in providers.md
Fix E2E test failure where clickProviderRowActions was matching 2 elements (copy button and dropdown trigger). Use aria-haspopup="true" attribute to specifically target the dropdown trigger button.
HeroUI uses aria-haspopup="menu" not aria-haspopup="true" for dropdown triggers. Fix the selector in clickProviderRowActions to match correctly.
- Use row.locator("button").last() for dropdown trigger selector
- Add stabilization wait and force click for dropdown menu items
- Add verifyOCIUpdateCredentialsPageLoaded for update flow (Tenancy OCID hidden)
- Add verifyTestConnectionPageLoaded for update flow navigation
- Fix clickNext to exclude Next.js dev tools button with exact match
Thanks @alejandrobailo, "Before Merge" all done:
|
alejandrobailo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, nice work!
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation and see the Github Action logs for details |
Summary
providerUidprop required for OCI form validationChanges
update-credentials/page.tsxto get the UID (matching existing behavior inadd-credentials/page.tsx)providerUidprop toUpdateViaCredentialsForm,UpdateViaRoleForm, andUpdateViaServiceAccountFormTest plan
Checklist
UI
API
License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.