Skip to content

EDM-2632: Update types with new ApplicationStatus when there are no apps#386

Merged
celdrake merged 1 commit into
flightctl:mainfrom
celdrake:EDM-2632-no-applications-status
Nov 26, 2025
Merged

EDM-2632: Update types with new ApplicationStatus when there are no apps#386
celdrake merged 1 commit into
flightctl:mainfrom
celdrake:EDM-2632-no-applications-status

Conversation

@celdrake

@celdrake celdrake commented Nov 26, 2025

Copy link
Copy Markdown
Collaborator

Adds the new "ApplicationsSummaryStatusNoApplications" to the UI

Without applications defined:
no-applicatoins

After applications are defined and reported:
Screenshot From 2025-11-26 09-56-10

Summary by CodeRabbit

  • New Features

    • Added a new "No applications" status display with a visual indicator when no applications are present.
  • Documentation

    • Updated resource limit field descriptions for improved clarity.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai

coderabbitai Bot commented Nov 26, 2025

Copy link
Copy Markdown

Walkthrough

This pull request refactors the authentication provider specification from assignment-based authorization to OAuth2 configuration, makes the appType property required across provider specs, adds support for a "no applications" status state, and updates associated UI components and localizations accordingly.

Changes

Cohort / File(s) Summary
OAuth2 Authorization Refactor
libs/types/models/AapProviderSpec.ts
Replaced organizationAssignment and roleAssignment fields with OAuth2 configuration fields: authorizationUrl, tokenUrl, clientId, clientSecret, and scopes. Removed related imports.
Required appType Property
libs/types/models/ApplicationProviderSpec.ts, libs/ui-components/src/types/extraTypes.ts
Made appType property required instead of optional across both files, enforcing explicit app type specification.
No Applications Status
libs/types/models/ApplicationsSummaryStatusType.ts, libs/ui-components/src/utils/status/applications.ts, libs/i18n/locales/en/translation.json
Added new enum value ApplicationsSummaryStatusNoApplications, corresponding status item with AsleepIcon, and English localization string for "No applications".
Provider Type Guards
libs/ui-components/src/types/deviceSpec.ts
Removed isInlineAppProvider guard and expanded isImageAppProvider to narrow to ApplicationProviderSpecFixed & ImageApplicationProviderSpec; updated imports accordingly.
Documentation
libs/types/models/ApplicationResourceLimits.ts
Updated inline comments for cpu and memory fields to reflect format constraints and generalized descriptions.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant AppProvider as App Provider<br/>(ApplicationProviderSpec)
    participant OAuth2 as OAuth2 Config<br/>(AapProviderSpec)
    participant StatusUI as Status Display<br/>(applications.ts)

    Note over Client,StatusUI: OAuth2 Authorization Flow
    Client->>AppProvider: Create with appType (required)
    AppProvider->>OAuth2: Initialize OAuth2 config
    OAuth2->>OAuth2: Store authorizationUrl, tokenUrl,<br/>clientId, clientSecret, scopes
    
    Note over Client,StatusUI: No Applications Scenario
    Client->>StatusUI: Check application status
    alt Has Applications
        StatusUI->>StatusUI: Display existing status
    else No Applications
        StatusUI->>StatusUI: Show ApplicationsSummaryStatusNoApplications<br/>with AsleepIcon
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • AapProviderSpec refactor: Replace field-based assignments with OAuth2 configuration requires verification that all consumer code handles the new fields correctly
  • Breaking changes: Making appType required across multiple type files may affect existing instantiation patterns; verify all call sites provide this property
  • Type guard removal: Removal of isInlineAppProvider and modifications to isImageAppProvider need validation that dependent code paths still function correctly
  • Status integration: Verify the new "No applications" status propagates correctly through UI rendering logic and that the AsleepIcon import path is accurate

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title directly and accurately summarizes the main objective: adding a new ApplicationStatus type to handle the case when no applications exist.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7c0cf98 and a2e3d06.

📒 Files selected for processing (8)
  • libs/i18n/locales/en/translation.json (1 hunks)
  • libs/types/models/AapProviderSpec.ts (1 hunks)
  • libs/types/models/ApplicationProviderSpec.ts (1 hunks)
  • libs/types/models/ApplicationResourceLimits.ts (1 hunks)
  • libs/types/models/ApplicationsSummaryStatusType.ts (1 hunks)
  • libs/ui-components/src/types/deviceSpec.ts (2 hunks)
  • libs/ui-components/src/types/extraTypes.ts (1 hunks)
  • libs/ui-components/src/utils/status/applications.ts (2 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
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.
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.
📚 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:

  • libs/types/models/ApplicationProviderSpec.ts
  • libs/ui-components/src/types/extraTypes.ts
📚 Learning: 2025-03-19T08:55:03.335Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 240
File: libs/ui-components/src/components/Device/DeviceDetails/DeviceApplications.tsx:44-50
Timestamp: 2025-03-19T08:55:03.335Z
Learning: In FlightCtl, the name field is required for Inline applications but optional for Image-based applications. This requirement is enforced by validation logic even though TypeScript type definitions might not explicitly show this distinction.

Applied to files:

  • libs/types/models/ApplicationProviderSpec.ts
  • libs/ui-components/src/types/extraTypes.ts
  • libs/ui-components/src/types/deviceSpec.ts
📚 Learning: 2025-03-20T12:37:36.986Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 240
File: libs/ui-components/src/types/deviceSpec.ts:0-0
Timestamp: 2025-03-20T12:37:36.986Z
Learning: In the FlightCtl UI project, `name` is required for inline applications but optional for image applications. This is enforced at the type level in the `InlineAppForm` type.

Applied to files:

  • libs/types/models/ApplicationProviderSpec.ts
  • libs/ui-components/src/types/extraTypes.ts
  • libs/ui-components/src/types/deviceSpec.ts
📚 Learning: 2025-10-29T16:47:29.614Z
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.

Applied to files:

  • libs/ui-components/src/types/extraTypes.ts
📚 Learning: 2025-11-24T13:42:36.897Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 381
File: libs/ui-components/src/components/form/validations.ts:71-88
Timestamp: 2025-11-24T13:42:36.897Z
Learning: In FlightCtl, quadlet applications intentionally support only a subset of Podman's quadlet file extensions (.container, .volume, .network, .image, .pod) because other valid Podman extensions (.build, .artifact, .kube) don't work well with FlightCtl's declarative approach. This is a deliberate design decision reflected in supportedQuadletExtensions and unsupportedQuadletExtensions constants in libs/ui-components/src/components/form/validations.ts.

Applied to files:

  • libs/ui-components/src/types/deviceSpec.ts
📚 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/types/models/AapProviderSpec.ts
🧬 Code graph analysis (2)
libs/types/models/ApplicationProviderSpec.ts (1)
libs/types/index.ts (1)
  • AppType (19-19)
libs/ui-components/src/types/deviceSpec.ts (3)
libs/ui-components/src/types/extraTypes.ts (1)
  • ApplicationProviderSpecFixed (47-51)
libs/types/models/ImageApplicationProviderSpec.ts (1)
  • ImageApplicationProviderSpec (8-18)
libs/types/index.ts (1)
  • ImageApplicationProviderSpec (121-121)
⏰ 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 (8)
libs/types/models/ApplicationResourceLimits.ts (1)

10-16: LGTM! Documentation updates clarify format restrictions.

The updated comments provide clearer guidance that format restrictions depend on the application type, which is helpful context for consumers of this type.

libs/i18n/locales/en/translation.json (1)

1071-1071: LGTM! Translation entry for new "No applications" status.

The new localization entry properly supports the ApplicationsSummaryStatusNoApplications status introduced in this PR.

libs/types/models/ApplicationsSummaryStatusType.ts (1)

13-13: LGTM! New status enum for "no applications" state.

The new ApplicationsSummaryStatusNoApplications enum member appropriately represents the case where no applications are defined, aligning with the PR objectives.

libs/ui-components/src/utils/status/applications.ts (2)

8-8: LGTM! Appropriate icon import for the new status.

The AsleepIcon is a semantically appropriate choice for representing the "no applications" state.


27-32: LGTM! New status item properly configured.

The new NoApplications status item is well-structured with:

  • Appropriate info level (informational, not an error)
  • Localized label using t('No applications')
  • Custom icon for visual distinction
  • Logical placement in the status order
libs/ui-components/src/types/deviceSpec.ts (1)

118-120: I'll help you verify the removal of isInlineAppProvider and the updated isImageAppProvider signature. Let me execute the verification script to check for any remaining usages.
<function_calls>

#!/bin/bash

Check for any remaining usages of the removed isInlineAppProvider

echo "Checking for usages of removed isInlineAppProvider..."
rg -n 'isInlineAppProvider' --type ts

echo -e "\nChecking for usages of isImageAppProvider to verify parameter types..."
rg -n -A3 'isImageAppProvider(' --type ts


</function_calls>

Now let me also examine the current state of the deviceSpec.ts file to understand the function signatures and their context:
<function_calls>

#!/bin/bash

Get the file content around the relevant lines

echo "=== Checking deviceSpec.ts file content ==="
cat -n libs/ui-components/src/types/deviceSpec.ts | head -130 | tail -40


</function_calls>

libs/ui-components/src/types/extraTypes.ts (1)

46-51: Repository access issue prevents automated verification.

I was unable to verify the breaking change claim about appType becoming required in ApplicationProviderSpecFixed because the repository failed to clone in the sandbox environment. The review comment's concern—that all construction sites must now provide the appType field when this field transitioned from optional to required—is valid but remains unverified.

To complete this verification, you would need to:

  1. Search for all usages of ApplicationProviderSpecFixed in the codebase
  2. Identify where objects with this type are constructed (object literals, function returns, etc.)
  3. Confirm each location provides the appType field
  4. Check for any type errors or missing field warnings during compilation
libs/types/models/AapProviderSpec.ts (1)

8-45: Unable to verify review comment due to repository access constraints.

I attempted multiple shell script executions to verify the claims in this review comment, but the flightctl/flightctl-ui repository cannot be cloned in the sandbox environment. This prevents me from:

  1. Checking for any remaining references to the removed fields (organizationAssignment, roleAssignment)
  2. Confirming the OAuth2 field additions to AapProviderSpec
  3. Verifying how AAP provider specifications are used throughout the codebase
  4. Validating whether UI code has been updated for the OAuth2 flow

The review comment makes specific, testable claims about a breaking change to the AapProviderSpec type (removed and added fields), but without access to the codebase, I cannot determine if these changes are accurate or if they have been properly handled throughout the UI.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@celdrake celdrake merged commit 26dd34c into flightctl:main Nov 26, 2025
6 checks passed
@celdrake celdrake deleted the EDM-2632-no-applications-status branch November 26, 2025 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants