EDM-1982: Simple modal for seeing the login command #450
Conversation
WalkthroughThis change introduces a new login command feature for the Flight Control CLI. The implementation spans backend API endpoints for retrieving authenticated login commands, frontend UI components for displaying them in a modal, and environment configuration updates to support an external server URL. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant PageNav as PageNavigation
participant Modal as LoginCommandModal
participant API as /login-command API
participant AuthHandler as GetLoginCommand Handler
User->>PageNav: Click "Copy login command" button
PageNav->>Modal: Show modal (showLoginCommandModal = true)
Modal->>API: proxyFetch('login-command')
API->>AuthHandler: GET /login-command
AuthHandler->>AuthHandler: Get enabled providers<br/>Build login commands<br/>Sort by default priority
AuthHandler-->>API: LoginCommandResponse (JSON)
API-->>Modal: Login commands with providers
Modal->>Modal: Render commands in ClipboardCopy<br/>Show provider names if multiple
Modal-->>User: Display login commands for copying
User->>Modal: Click ClipboardCopy to copy command
Note over Modal: "Copied!" feedback shown
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
celdrake
force-pushed
the
EDM-1982-simple-copy-login-command
branch
from
495fbfd to
8600536
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
apps/standalone/scripts/setup_env.sh (1)
65-65: Add FLIGHTCTL_SERVER_EXTERNAL to the environment summary output.The new
FLIGHTCTL_SERVER_EXTERNALvariable is exported but not included in the summary output (lines 90-94). Users won't see this value when the script runs.🔎 Proposed fix
Add the following line to the summary section:
echo "🌐 Environment variables set:" >&2 echo " FLIGHTCTL_SERVER_INSECURE_SKIP_VERIFY=$FLIGHTCTL_SERVER_INSECURE_SKIP_VERIFY" >&2 echo " FLIGHTCTL_SERVER=$FLIGHTCTL_SERVER" >&2 +echo " FLIGHTCTL_SERVER_EXTERNAL=$FLIGHTCTL_SERVER_EXTERNAL" >&2 echo " FLIGHTCTL_CLI_ARTIFACTS_SERVER=${FLIGHTCTL_CLI_ARTIFACTS_SERVER:-'(disabled)'}" >&2 echo " FLIGHTCTL_ALERTMANAGER_PROXY=${FLIGHTCTL_ALERTMANAGER_PROXY:-'(disabled)'}" >&2proxy/auth/auth.go (1)
780-792: Consider returning a specific message when no enabled providers are available.If all providers are disabled, the handler returns a 200 status with an empty commands array. Consider returning a 404 or a specific error message to distinguish this case from a successful response with no providers configured.
🔎 Proposed enhancement
}) } +if len(commands) == 0 { + respondWithError(w, http.StatusNotFound, "No enabled authentication providers available") + return +} + response := LoginCommandResponse{ Commands: commands, }libs/ui-components/src/components/modals/LoginCommandModal/LoginCommandModal.tsx (2)
35-78: Add cleanup to prevent setState on unmounted component.The
useEffecthook lacks a cleanup function to cancel in-flight requests. If the modal is closed (component unmounts) while the fetch is in progress, callingsetStateon an unmounted component will trigger a React warning and could cause issues.🔎 Add AbortController for proper cleanup
React.useEffect(() => { + const abortController = new AbortController(); + const fetchLoginCommand = async () => { try { setLoading(true); setError(undefined); const response = await proxyFetch('login-command', { method: 'GET', + signal: abortController.signal, }); if (!response.ok) { let errorMessage = t('Failed to retrieve login command'); try { const contentType = response.headers.get('content-type'); if (contentType && contentType.includes('application/json')) { const errorData = (await response.json()) as { error?: string }; errorMessage = errorData.error || errorMessage; } else { const text = await response.text(); if (text) { errorMessage = text; } } } catch (parseErr) { // If parsing fails, use default error message } setError(errorMessage); return; } const result = (await response.json()) as { commands: LoginCommand[] }; if (result.commands.length === 0) { setError(t('No authentication providers available')); return; } setLoginCommands(result.commands); } catch (err) { + // Don't set error state if the request was aborted + if (err instanceof Error && err.name === 'AbortError') { + return; + } setError(getErrorMessage(err)); } finally { - setLoading(false); + if (!abortController.signal.aborted) { + setLoading(false); + } } }; void fetchLoginCommand(); + + return () => { + abortController.abort(); + }; }, [t, proxyFetch]);
64-64: Consider validating the API response structure.The type assertion
as { commands: LoginCommand[] }assumes the API response matches the expected structure. While this is a common pattern, adding runtime validation would make the code more defensive against API changes.🔎 Optional: Add runtime validation
const result = (await response.json()) as { commands: LoginCommand[] }; + if (!result || !Array.isArray(result.commands)) { + setError(t('Invalid response format from server')); + return; + } if (result.commands.length === 0) { setError(t('No authentication providers available')); return; }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
proxy/go.sumis excluded by!**/*.sum
📒 Files selected for processing (9)
apps/standalone/scripts/setup_env.shlibs/i18n/locales/en/translation.jsonlibs/ui-components/src/components/common/CopyButton.tsxlibs/ui-components/src/components/common/PageNavigation.tsxlibs/ui-components/src/components/modals/LoginCommandModal/LoginCommandModal.tsxproxy/app.goproxy/auth/auth.goproxy/config/config.goproxy/go.mod
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-01-07T10:11:14.375Z
Learnt from: rawagner
Repo: flightctl/flightctl-ui PR: 181
File: proxy/config/ocp.go:16-16
Timestamp: 2025-01-07T10:11:14.375Z
Learning: In the flightctl-ui repository, the `RBACNs` variable used in `proxy/config/ocp.go` is defined in `proxy/config/config.go` as a package-level variable that retrieves its value from the environment variable `K8S_RBAC_NS`.
Applied to files:
proxy/config/config.goapps/standalone/scripts/setup_env.sh
📚 Learning: 2025-11-20T09:24:41.991Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 378
File: libs/ui-components/src/components/AuthProvider/CreateAuthProvider/utils.ts:288-326
Timestamp: 2025-11-20T09:24:41.991Z
Learning: In the flightctl-ui repository, the CreateAuthProvider form and its utilities (including getAuthProvider in libs/ui-components/src/components/AuthProvider/CreateAuthProvider/utils.ts) are specifically designed for CRUD operations on OIDC and OAuth2 authentication providers only. Other provider types (K8s, AAP, OpenShift) are not supported by this form and should be rejected with an error if loading is attempted for editing.
Applied to files:
proxy/auth/auth.golibs/ui-components/src/components/modals/LoginCommandModal/LoginCommandModal.tsx
📚 Learning: 2025-02-11T10:20:50.496Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 208
File: libs/ui-components/src/components/Device/DeviceDetails/DeviceDetailsPage.tsx:30-44
Timestamp: 2025-02-11T10:20:50.496Z
Learning: In the FlightCtl UI, error handling for device decommissioning is implemented in the modal component created by `useDecommissionAction`. Failed PUT requests show an error in the modal with retry/close options.
Applied to files:
libs/i18n/locales/en/translation.json
📚 Learning: 2025-11-17T15:53:01.194Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 376
File: libs/ui-components/src/components/Repository/CreateRepository/CreateRepositoryForm.tsx:274-275
Timestamp: 2025-11-17T15:53:01.194Z
Learning: In PR #376 (flightctl/flightctl-ui), the permission system was updated to fetch all user permissions once at login via the Flight Control API. The `checkPermissions` function from `usePermissionsContext` is a pure calculation on cached permission arrays, not an async fetch, so loading state handling is generally not required at individual component call sites.
Applied to files:
libs/ui-components/src/components/common/PageNavigation.tsx
🧬 Code graph analysis (1)
libs/ui-components/src/components/modals/LoginCommandModal/LoginCommandModal.tsx (4)
proxy/auth/auth.go (1)
LoginCommand(30-34)libs/ui-components/src/hooks/useTranslation.ts (1)
useTranslation(5-8)libs/types/scripts/openapi-typescript.js (1)
response(22-22)libs/ui-components/src/utils/error.ts (1)
getErrorMessage(6-17)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: integration-tests
- GitHub Check: Lint
- GitHub Check: Build
- GitHub Check: Build ocp plugin
🔇 Additional comments (10)
proxy/go.mod (1)
8-8: LGTM!Updating to a stable tagged version (v1.0.0) from a pseudo-version is a good practice for dependency management.
libs/i18n/locales/en/translation.json (1)
212-212: LGTM!The new translation keys are well-structured and support the login command modal feature. The messaging is clear and consistent with the existing translation patterns.
Also applies to: 237-237, 836-841
proxy/app.go (1)
75-75: LGTM!The new
/login-commandroute is correctly placed within the authentication block and follows the existing pattern for login-related endpoints.proxy/config/config.go (1)
11-11: LGTM!The new
FctlApiExternalUrlvariable follows the established pattern for environment-based configuration and uses an appropriate default value.libs/ui-components/src/components/common/CopyButton.tsx (1)
16-35: LGTM!The enhancements provide good user feedback:
- The "Copied!" tooltip confirmation improves UX
- State management and timeout cleanup are correctly implemented
- Moving the onClick handler to the Button element is more semantically appropriate
Also applies to: 38-38, 42-45
proxy/auth/auth.go (1)
657-710: LGTM!The helper function correctly extracts display names and enabled states for all provider types. The implementation properly handles the discriminated union pattern used by the provider specs.
libs/ui-components/src/components/common/PageNavigation.tsx (1)
22-22: LGTM!The integration of the login command modal is clean and follows the existing pattern used for the organization selector modal. The button placement and state management are appropriate.
Also applies to: 110-110, 131-141, 175-181
libs/ui-components/src/components/modals/LoginCommandModal/LoginCommandModal.tsx (3)
44-62: LGTM! Comprehensive error handling.The error handling logic is well-implemented with appropriate fallbacks:
- Checks content-type before parsing JSON
- Falls back to plain text if JSON parsing fails
- Uses a default translated message if all parsing fails
- Defensive try-catch prevents error handling from itself throwing errors
80-127: LGTM! Well-designed UX that adapts to context.The rendering logic demonstrates good UX design:
- Dynamic title adapts based on the number of providers (lines 80-83)
- Provider names are shown only when there are multiple providers (lines 114-120)
- Loading, error, and success states are clearly separated and handled appropriately
ClipboardCopycomponent provides excellent copy-to-clipboard UX with tooltips- All user-facing strings are properly translated
13-13: No changes needed. The Modal components imported from@patternfly/react-core/nextfollow the consistent pattern used throughout the codebase for importing modal components across 24+ files. This is the project's standard convention, not an experimental or unstable API.
| TextContent, | ||
| TextVariants, | ||
| } from '@patternfly/react-core'; | ||
| import { Modal, ModalBody, ModalFooter, ModalHeader } from '@patternfly/react-core/next'; |
There was a problem hiding this comment.
Needs migrating to PF6.
celdrake
force-pushed
the
EDM-1982-simple-copy-login-command
branch
from
8600536 to
20b15d7
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
libs/ui-components/src/components/modals/LoginCommandModal/LoginCommandModal.tsx (1)
13-13: Verify PatternFly Modal import path.A previous comment indicated this needs "migrating to PF6." The
/nextimport path may need verification against the project's current PatternFly version.Based on past review comment. Please verify the correct import path for Modal components in the current PatternFly version being used.
proxy/auth/auth.go (1)
726-760: Fix logic for determining when to omit the provider flag.The check at line 753 uses
providersCountwhich is incremented during the loop, causing the first enabled provider to generate a command without--providerwhile subsequent providers include it. This creates inconsistent commands.🔎 Recommended fix
Build all commands first, then adjust based on the final count:
providersCount := 0 var commands []LoginCommand for _, provider := range *authConfig.Providers { if provider.Metadata.Name == nil { continue } providerName := *provider.Metadata.Name providerTypeStr, err := provider.Spec.Discriminator() if err != nil { log.GetLogger().WithError(err).Warnf("Failed to determine provider type for %s", providerName) continue } isEnabled, displayName := getDisplayNameFromProviderSpec(&provider.Spec) if !isEnabled { continue } providersCount++ - var command string - providerFlag := "web" if providerTypeStr == ProviderTypeK8s { providerFlag = "token=<your-token>" } - if providersCount == 1 || providerTypeStr == ProviderTypeK8s { - // --token cannot be used with --provider - command = fmt.Sprintf("flightctl login %s --%s", config.FctlApiExternalUrl, providerFlag) - } else { - command = fmt.Sprintf("flightctl login %s --provider=%s --%s", config.FctlApiExternalUrl, providerName, providerFlag) - } - commands = append(commands, LoginCommand{ProviderName: providerName, DisplayName: displayName, Command: command}) + commands = append(commands, LoginCommand{ + ProviderName: providerName, + DisplayName: displayName, + Command: "", // Will be set below + providerType: providerTypeStr, // Add temporary field + providerFlag: providerFlag, // Add temporary field + }) } +// Now generate commands with correct logic +for i := range commands { + if providersCount == 1 || commands[i].providerType == ProviderTypeK8s { + commands[i].Command = fmt.Sprintf("flightctl login %s --%s", config.FctlApiExternalUrl, commands[i].providerFlag) + } else { + commands[i].Command = fmt.Sprintf("flightctl login %s --provider=%s --%s", config.FctlApiExternalUrl, commands[i].ProviderName, commands[i].providerFlag) + } +}Alternatively, adjust the LoginCommand struct temporarily or do a two-pass approach.
Based on past review comment.
🧹 Nitpick comments (1)
proxy/auth/auth.go (1)
657-710: Consider extracting the common pattern to reduce duplication.The five switch cases follow an identical pattern (get spec, extract displayName, extract enabled). While functional, this adds maintenance burden when adding new provider types or changing the logic.
💡 Potential simplification approach
Consider defining a common interface that all provider specs implement, or use a helper that extracts both fields in a single call. This would reduce the ~50 lines of repetitive code to a more maintainable structure.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
proxy/go.sumis excluded by!**/*.sum
📒 Files selected for processing (9)
apps/standalone/scripts/setup_env.shlibs/i18n/locales/en/translation.jsonlibs/ui-components/src/components/common/CopyButton.tsxlibs/ui-components/src/components/common/PageNavigation.tsxlibs/ui-components/src/components/modals/LoginCommandModal/LoginCommandModal.tsxproxy/app.goproxy/auth/auth.goproxy/config/config.goproxy/go.mod
🚧 Files skipped from review as they are similar to previous changes (3)
- libs/i18n/locales/en/translation.json
- proxy/go.mod
- proxy/config/config.go
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-11-17T15:53:01.194Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 376
File: libs/ui-components/src/components/Repository/CreateRepository/CreateRepositoryForm.tsx:274-275
Timestamp: 2025-11-17T15:53:01.194Z
Learning: In PR #376 (flightctl/flightctl-ui), the permission system was updated to fetch all user permissions once at login via the Flight Control API. The `checkPermissions` function from `usePermissionsContext` is a pure calculation on cached permission arrays, not an async fetch, so loading state handling is generally not required at individual component call sites.
Applied to files:
libs/ui-components/src/components/common/PageNavigation.tsx
📚 Learning: 2025-11-20T09:24:41.991Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 378
File: libs/ui-components/src/components/AuthProvider/CreateAuthProvider/utils.ts:288-326
Timestamp: 2025-11-20T09:24:41.991Z
Learning: In the flightctl-ui repository, the CreateAuthProvider form and its utilities (including getAuthProvider in libs/ui-components/src/components/AuthProvider/CreateAuthProvider/utils.ts) are specifically designed for CRUD operations on OIDC and OAuth2 authentication providers only. Other provider types (K8s, AAP, OpenShift) are not supported by this form and should be rejected with an error if loading is attempted for editing.
Applied to files:
libs/ui-components/src/components/modals/LoginCommandModal/LoginCommandModal.tsxproxy/auth/auth.go
📚 Learning: 2025-01-07T10:11:14.375Z
Learnt from: rawagner
Repo: flightctl/flightctl-ui PR: 181
File: proxy/config/ocp.go:16-16
Timestamp: 2025-01-07T10:11:14.375Z
Learning: In the flightctl-ui repository, the `RBACNs` variable used in `proxy/config/ocp.go` is defined in `proxy/config/config.go` as a package-level variable that retrieves its value from the environment variable `K8S_RBAC_NS`.
Applied to files:
apps/standalone/scripts/setup_env.sh
📚 Learning: 2025-11-20T07:33:34.656Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 378
File: libs/ui-components/src/utils/authProvider.ts:5-18
Timestamp: 2025-11-20T07:33:34.656Z
Learning: In the flightctl-ui repository, for AuthProvider objects, the metadata.name field is always present at runtime as it serves as the ID of the authprovider. The OpenAPI schema incorrectly defines it as optional (name?: string in ObjectMeta), but the type assertion `as string` is appropriate when accessing provider.metadata.name since it's guaranteed to exist.
Applied to files:
proxy/auth/auth.go
🧬 Code graph analysis (2)
libs/ui-components/src/components/modals/LoginCommandModal/LoginCommandModal.tsx (4)
proxy/auth/auth.go (1)
LoginCommand(30-34)libs/ui-components/src/hooks/useTranslation.ts (1)
useTranslation(5-8)libs/types/scripts/openapi-typescript.js (1)
response(22-22)libs/ui-components/src/utils/error.ts (1)
getErrorMessage(6-17)
proxy/auth/auth.go (5)
libs/types/index.ts (1)
AuthProviderSpec(27-27)libs/types/models/AuthProviderSpec.ts (1)
AuthProviderSpec(10-10)proxy/auth/common.go (5)
ProviderTypeOIDC(23-23)ProviderTypeOAuth2(25-25)ProviderTypeOpenShift(26-26)ProviderTypeK8s(22-22)ProviderTypeAAP(24-24)proxy/log/log.go (1)
GetLogger(17-19)proxy/config/config.go (1)
FctlApiExternalUrl(11-11)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: integration-tests
- GitHub Check: Build
- GitHub Check: Build ocp plugin
- GitHub Check: Lint
🔇 Additional comments (11)
apps/standalone/scripts/setup_env.sh (1)
65-65: LGTM!The addition of
FLIGHTCTL_SERVER_EXTERNALfollows the existing pattern and correctly uses nip.io for wildcard DNS resolution in development environments.Also applies to: 93-93
proxy/auth/auth.go (3)
30-38: LGTM!The data structures are well-defined with appropriate JSON tags for API serialization.
762-779: LGTM!The sorting logic correctly prioritizes the default provider while maintaining the original order for other providers.
781-794: LGTM!Response handling follows standard patterns with appropriate error handling and content-type headers.
proxy/app.go (1)
75-75: LGTM!The route registration follows existing patterns and is appropriately placed with other authentication-related endpoints.
libs/ui-components/src/components/common/CopyButton.tsx (1)
16-35: LGTM!The copy feedback mechanism is well-implemented with proper cleanup in the useEffect. The 1-second timeout provides clear user feedback, and moving the onClick handler to the Button (instead of the icon) improves accessibility.
Also applies to: 38-38, 42-46
libs/ui-components/src/components/common/PageNavigation.tsx (1)
22-22: LGTM!The modal integration follows standard patterns with proper accessibility attributes (aria-label) and tooltip for the trigger button.
Also applies to: 110-110, 131-141, 175-181
libs/ui-components/src/components/modals/LoginCommandModal/LoginCommandModal.tsx (4)
35-78: LGTM!The fetch logic is robust with comprehensive error handling for both JSON and text responses. The dependency array is appropriate, assuming
proxyFetchis a stable reference fromuseFetch.
80-83: LGTM!The dynamic title appropriately adjusts messaging based on whether users need to select from multiple providers or use a single command.
85-128: LGTM!The modal correctly handles loading, error, and success states. The conditional display of provider names (only when multiple providers exist) is a thoughtful UX decision.
130-136: LGTM!Standard modal footer with primary close button.
celdrake
force-pushed
the
EDM-1982-simple-copy-login-command
branch
from
20b15d7 to
218be04
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @proxy/auth/auth.go:
- Around line 746-759: The command-generation logic incorrectly omits --provider
based on providersCount==1; instead determine the server default provider ahead
of the loop (extract defaultProviderName from the sorted providers/config) and
inside the loop build the command using providerTypeStr/ProviderTypeK8s to set
providerFlag, but omit --provider only when providerName == defaultProviderName
(still allow k8s token branch to use token=<your-token> and include --provider
when providerName differs from defaultProviderName); update the condition that
builds command (where LoginCommand is appended) to check
providerName==defaultProviderName rather than providersCount==1.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
proxy/go.sumis excluded by!**/*.sum
📒 Files selected for processing (9)
apps/standalone/scripts/setup_env.shlibs/i18n/locales/en/translation.jsonlibs/ui-components/src/components/common/CopyButton.tsxlibs/ui-components/src/components/common/PageNavigation.tsxlibs/ui-components/src/components/modals/LoginCommandModal/LoginCommandModal.tsxproxy/app.goproxy/auth/auth.goproxy/config/config.goproxy/go.mod
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/standalone/scripts/setup_env.sh
- libs/ui-components/src/components/common/CopyButton.tsx
- proxy/config/config.go
- proxy/go.mod
- proxy/app.go
- libs/ui-components/src/components/common/PageNavigation.tsx
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: rawagner
Repo: flightctl/flightctl-ui PR: 209
File: apps/standalone/src/app/utils/apiCalls.ts:14-16
Timestamp: 2025-02-18T12:04:42.579Z
Learning: In the flightctl-ui codebase, error handling for API calls (like logout) is implemented by the caller rather than within the API utility functions themselves.
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 376
File: libs/ui-components/src/components/Repository/CreateRepository/CreateRepositoryForm.tsx:274-275
Timestamp: 2025-11-17T15:53:01.194Z
Learning: In PR #376 (flightctl/flightctl-ui), the permission system was updated to fetch all user permissions once at login via the Flight Control API. The `checkPermissions` function from `usePermissionsContext` is a pure calculation on cached permission arrays, not an async fetch, so loading state handling is generally not required at individual component call sites.
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 371
File: libs/types/models/AppType.ts:10-10
Timestamp: 2025-10-29T16:47:29.614Z
Learning: PR #371 (flightctl/flightctl-ui) adds the AppTypeQuadlet enum member and related types as a preparatory change. Full implementation of quadlet application support in the UI will be added later, after backend support is available.
📚 Learning: 2025-11-20T09:24:41.991Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 378
File: libs/ui-components/src/components/AuthProvider/CreateAuthProvider/utils.ts:288-326
Timestamp: 2025-11-20T09:24:41.991Z
Learning: In the flightctl-ui repository, the CreateAuthProvider form and its utilities (including getAuthProvider in libs/ui-components/src/components/AuthProvider/CreateAuthProvider/utils.ts) are specifically designed for CRUD operations on OIDC and OAuth2 authentication providers only. Other provider types (K8s, AAP, OpenShift) are not supported by this form and should be rejected with an error if loading is attempted for editing.
Applied to files:
libs/ui-components/src/components/modals/LoginCommandModal/LoginCommandModal.tsxproxy/auth/auth.go
📚 Learning: 2025-11-20T07:33:34.656Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 378
File: libs/ui-components/src/utils/authProvider.ts:5-18
Timestamp: 2025-11-20T07:33:34.656Z
Learning: In the flightctl-ui repository, for AuthProvider objects, the metadata.name field is always present at runtime as it serves as the ID of the authprovider. The OpenAPI schema incorrectly defines it as optional (name?: string in ObjectMeta), but the type assertion `as string` is appropriate when accessing provider.metadata.name since it's guaranteed to exist.
Applied to files:
proxy/auth/auth.go
📚 Learning: 2025-02-11T10:20:50.496Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 208
File: libs/ui-components/src/components/Device/DeviceDetails/DeviceDetailsPage.tsx:30-44
Timestamp: 2025-02-11T10:20:50.496Z
Learning: In the FlightCtl UI, error handling for device decommissioning is implemented in the modal component created by `useDecommissionAction`. Failed PUT requests show an error in the modal with retry/close options.
Applied to files:
libs/i18n/locales/en/translation.json
🧬 Code graph analysis (2)
libs/ui-components/src/components/modals/LoginCommandModal/LoginCommandModal.tsx (4)
proxy/auth/auth.go (1)
LoginCommand(30-34)libs/ui-components/src/hooks/useTranslation.ts (1)
useTranslation(5-8)libs/types/scripts/openapi-typescript.js (1)
response(22-22)libs/ui-components/src/utils/error.ts (1)
getErrorMessage(6-17)
proxy/auth/auth.go (5)
libs/types/index.ts (1)
AuthProviderSpec(28-28)libs/types/models/AuthProviderSpec.ts (1)
AuthProviderSpec(10-10)proxy/auth/common.go (5)
ProviderTypeOIDC(23-23)ProviderTypeOAuth2(25-25)ProviderTypeOpenShift(26-26)ProviderTypeK8s(22-22)ProviderTypeAAP(24-24)proxy/log/log.go (1)
GetLogger(17-19)proxy/config/config.go (1)
FctlApiExternalUrl(11-11)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: integration-tests
- GitHub Check: Build ocp plugin
- GitHub Check: Build
- GitHub Check: Lint
🔇 Additional comments (8)
libs/i18n/locales/en/translation.json (1)
212-212: LGTM!The new translation keys are well-organized and follow the existing patterns in the file. They properly support the new login command modal functionality.
Also applies to: 238-238, 837-842
libs/ui-components/src/components/modals/LoginCommandModal/LoginCommandModal.tsx (4)
1-16: LGTM!Imports are clean and use the appropriate PatternFly 5 modal components from the
nextpath.
18-26: LGTM!The
LoginCommandtype correctly mirrors the backend struct defined inproxy/auth/auth.go(lines 30-34).
35-78: LGTM!The fetch logic handles all error cases appropriately:
- Network errors via catch block
- Non-OK responses with JSON/text parsing
- Empty providers list as a specific error case
The empty catch block on line 57-59 is intentional to fall back to the default error message when response parsing fails.
85-136: LGTM!The render logic is well-structured with proper conditional rendering for loading, error, and success states. The
ClipboardCopycomponent withhoverTipandclickTipprovides good UX feedback.proxy/auth/auth.go (3)
30-38: LGTM!The struct definitions are clean with appropriate JSON tags that align with the frontend
LoginCommandtype.
657-710: LGTM!The helper function properly handles all provider types with safe nil checks before dereferencing optional fields. The default
isEnabled = falseensures disabled-by-default behavior.
762-794: LGTM!The sorting logic correctly places the default provider first while maintaining stable order for remaining providers. The response handling with proper Content-Type header is appropriate.
2 participants
Opens a simple modal that shows the login command for all enabled providers.
Tokens are not displayed, users should go through the web flow for the chosen provider.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.