Skip to content

EDM-2422 split application and systemd status#373

Merged
celdrake merged 2 commits into
flightctl:mainfrom
celdrake:EDM-2422-split-application-and-systemd-status
Nov 18, 2025
Merged

EDM-2422 split application and systemd status#373
celdrake merged 2 commits into
flightctl:mainfrom
celdrake:EDM-2422-split-application-and-systemd-status

Conversation

@celdrake

@celdrake celdrake commented Nov 7, 2025

Copy link
Copy Markdown
Collaborator

Adapts the UI to the changes in flightctl/flightctl#1918

  • Removes systemd services from the Applications table
  • Allows to define the systemd services for an individual device when editing the device
  • Adds the systemd services with the new status fields.
  • Removes "Track systemd services" from device details page for fleetless devices. Systemd services can be edited like any other part of the spec via the Wizard.

Final design with dark theme:
system-services

With latest changes:
system-d-final

Summary by CodeRabbit

  • New Features

    • Added a "System services" card and table in device details with enable/load/active-substate badges and empty-state guidance.
    • System services surfaced in device and fleet edit flows and pre-populated in forms.
  • Bug Fixes

    • Updated English translations for system services and related status labels.
  • Refactor

    • Simplified Applications table UI (removed Type column, per-row hover control) and narrowed DeviceApplications surface.
  • Chores

    • Consolidated systemd-related form patching and initial value handling.

@coderabbitai

coderabbitai Bot commented Nov 7, 2025

Copy link
Copy Markdown

Walkthrough

Removes systemd management from ApplicationsTable and DeviceApplications, introduces SystemdUnitsTable and DeviceSystemdUnits, updates device/fleet edit helpers to surface systemd unit patterns, and adjusts translations and CSS related to system services.

Changes

Cohort / File(s) Summary
Translation updates
libs/i18n/locales/en/translation.json
Reordered and added system service translation keys; removed some old keys and added new keys like "Enable", "Load state", "System services table", "No system services found", "System services can be configured via the device specification", and "Track systemd services".
Applications table refactor
libs/ui-components/src/components/DetailsPage/Tables/ApplicationsTable.css, libs/ui-components/src/components/DetailsPage/Tables/ApplicationsTable.tsx
Removed hover-to-show CSS for per-row button; removed systemd-related props, UI, state and logic. ApplicationsTable now only accepts appsStatus and specApps and renders a simplified status column.
New systemd table component
libs/ui-components/src/components/DetailsPage/Tables/SystemdUnitsTable.tsx
Added SystemdUnitsTable component with unit name+icon, enable/load/status columns, per-row status labels, and an empty state for no units.
Device details integration
libs/ui-components/src/components/Device/DeviceDetails/DeviceApplications.tsx, libs/ui-components/src/components/Device/DeviceDetails/DeviceDetailsTab.tsx, libs/ui-components/src/components/Device/DeviceDetails/DeviceSystemdUnits.tsx
DeviceApplications signature simplified (removed canEdit/refetch); added DeviceSystemdUnits component which passes device.status.systemd to SystemdUnitsTable; DeviceDetailsTab renders DeviceSystemdUnits alongside DeviceApplications.
Edit device wizard helpers & forms
libs/ui-components/src/components/Device/EditDeviceWizard/EditDeviceWizard.tsx, libs/ui-components/src/components/Device/EditDeviceWizard/deviceSpecUtils.ts, libs/ui-components/src/components/Device/EditDeviceWizard/steps/DeviceTemplateStep.tsx, libs/ui-components/src/components/Device/EditDeviceWizard/utils.ts
New exported helper getSystemdUnitsValues(deviceSpec?); EditDeviceWizard uses it for initial systemdUnits; DeviceTemplateStep renders SystemdUnitsForm unconditionally; utils.ts composes string-list patches for systemd matchPatterns into device patches.
Fleet create initial values
libs/ui-components/src/components/Fleet/CreateFleet/utils.ts
getInitialValues() updated to call getSystemdUnitsValues() to initialize systemdUnits from fleet template spec.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant DetailsTab as DeviceDetailsTab
  participant Apps as DeviceApplications
  participant AppsTbl as ApplicationsTable
  participant SysdComp as DeviceSystemdUnits
  participant SysdTbl as SystemdUnitsTable

  DetailsTab->>Apps: render(device)
  Apps->>AppsTbl: render(appsStatus, specApps)
  DetailsTab->>SysdComp: render(device)
  SysdComp->>SysdTbl: render(systemdUnitsStatus)
  Note right of SysdTbl#D0E8FF: Shows unit icon + enable/load/status badges
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Focus areas:
    • Call sites of DeviceApplications (updated prop signature).
    • SystemdUnitsTable: icon mapping, label rendering, translations, and empty-state text.
    • getDevicePatches changes in EditDeviceWizard/utils.ts for correct string-list patch semantics.
    • getSystemdUnitsValues usage in EditDeviceWizard and Fleet initializers.

Possibly related PRs

Suggested reviewers

  • rawagner
  • amalykhi

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 title 'EDM-2422 split application and systemd status' accurately reflects the main change: separating application and systemd unit handling in the UI.
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 ad2c388 and 3f90af4.

📒 Files selected for processing (2)
  • libs/i18n/locales/en/translation.json (3 hunks)
  • libs/ui-components/src/components/DetailsPage/Tables/SystemdUnitsTable.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • libs/ui-components/src/components/DetailsPage/Tables/SystemdUnitsTable.tsx
🧰 Additional context used
🧠 Learnings (1)
📓 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: 227
File: libs/ui-components/src/components/Device/EditDeviceWizard/steps/ReviewTrackedSystemdServices.tsx:3-3
Timestamp: 2025-03-12T08:41:34.720Z
Learning: In the flightctl-ui codebase, SystemdUnitsFormValues is defined as { systemdUnits: SystemdUnitFormValue[] }, allowing for direct destructuring of the systemdUnits property in component props.
🔇 Additional comments (2)
libs/i18n/locales/en/translation.json (2)

128-132: LGTM – New systemd service translation keys properly added.

The new translation strings supporting systemd services management are properly added and follow consistent naming conventions. The previously reported typo ("No system sevices found") appears to have been corrected to "No system services found" on line 131.


124-124: The review comment's categorization of translation keys is incorrect.

The "Type" key at line 124 is not a systemd-related key—it's a generic translation key used in Repository and Conditions tables (RepositoryList.tsx, RepositoryGeneralDetailsCard.tsx, ConditionsTable.tsx). The actual systemd-related keys are properly referenced:

  • "System services" (line 181) → used in DeviceSystemdUnits.tsx:20
  • "System services table" (line 130) → used in SystemdUnitsTable.tsx:71
  • "Track systemd services" (line 322) → used in SystemdUnitsModal.tsx:39

All systemd-related translation keys are consistently used in their intended components. The PR does not introduce a mismatch between translation keys and component usage.

Likely an incorrect or invalid review comment.


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 force-pushed the EDM-2422-split-application-and-systemd-status branch from bc2be7e to 1b6c6eb Compare November 7, 2025 10:31
Comment thread libs/ui-components/src/components/Device/DeviceDetails/DeviceSystemdUnits.tsx Outdated
@celdrake celdrake force-pushed the EDM-2422-split-application-and-systemd-status branch 4 times, most recently from 8f6f4cf to 32e80e0 Compare November 13, 2025 12:06
@celdrake celdrake marked this pull request as ready for review November 13, 2025 12:06

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Nitpick comments (1)
libs/ui-components/src/components/DetailsPage/Tables/ApplicationsTable.tsx (1)

19-28: Consider using a Set for more concise logic.

The current approach works correctly, but using a Set would be more efficient and concise.

Apply this diff:

-  // Includes applications already reported in status as well as those that are only in the spec yet
-  const allAppNames: string[] = [];
-  specApps.forEach((app) => {
-    allAppNames.push(app);
-  });
-  appsStatus.forEach((appStatus) => {
-    if (!allAppNames.includes(appStatus.name)) {
-      allAppNames.push(appStatus.name);
-    }
-  });
+  // Includes applications already reported in status as well as those that are only in the spec yet
+  const allAppNames = Array.from(new Set([...specApps, ...appsStatus.map((s) => s.name)]));
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ce054d1 and 32e80e0.

📒 Files selected for processing (12)
  • libs/i18n/locales/en/translation.json (3 hunks)
  • libs/ui-components/src/components/DetailsPage/Tables/ApplicationsTable.css (0 hunks)
  • libs/ui-components/src/components/DetailsPage/Tables/ApplicationsTable.tsx (1 hunks)
  • libs/ui-components/src/components/DetailsPage/Tables/SystemdUnitsTable.tsx (1 hunks)
  • libs/ui-components/src/components/Device/DeviceDetails/DeviceApplications.tsx (1 hunks)
  • libs/ui-components/src/components/Device/DeviceDetails/DeviceDetailsTab.tsx (3 hunks)
  • libs/ui-components/src/components/Device/DeviceDetails/DeviceSystemdUnits.tsx (1 hunks)
  • libs/ui-components/src/components/Device/EditDeviceWizard/EditDeviceWizard.tsx (2 hunks)
  • libs/ui-components/src/components/Device/EditDeviceWizard/deviceSpecUtils.ts (2 hunks)
  • libs/ui-components/src/components/Device/EditDeviceWizard/steps/DeviceTemplateStep.tsx (1 hunks)
  • libs/ui-components/src/components/Device/EditDeviceWizard/utils.ts (2 hunks)
  • libs/ui-components/src/components/Fleet/CreateFleet/utils.ts (2 hunks)
💤 Files with no reviewable changes (1)
  • libs/ui-components/src/components/DetailsPage/Tables/ApplicationsTable.css
🧰 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: 227
File: libs/ui-components/src/components/Device/EditDeviceWizard/steps/ReviewTrackedSystemdServices.tsx:3-3
Timestamp: 2025-03-12T08:41:34.720Z
Learning: In the flightctl-ui codebase, SystemdUnitsFormValues is defined as { systemdUnits: SystemdUnitFormValue[] }, allowing for direct destructuring of the systemdUnits property in component props.
📚 Learning: 2025-03-12T08:41:34.720Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 227
File: libs/ui-components/src/components/Device/EditDeviceWizard/steps/ReviewTrackedSystemdServices.tsx:3-3
Timestamp: 2025-03-12T08:41:34.720Z
Learning: In the flightctl-ui codebase, SystemdUnitsFormValues is defined as { systemdUnits: SystemdUnitFormValue[] }, allowing for direct destructuring of the systemdUnits property in component props.

Applied to files:

  • libs/ui-components/src/components/Device/EditDeviceWizard/steps/DeviceTemplateStep.tsx
  • libs/ui-components/src/components/Device/DeviceDetails/DeviceDetailsTab.tsx
  • libs/ui-components/src/components/Fleet/CreateFleet/utils.ts
  • libs/ui-components/src/components/DetailsPage/Tables/SystemdUnitsTable.tsx
  • libs/ui-components/src/components/Device/EditDeviceWizard/EditDeviceWizard.tsx
  • libs/ui-components/src/components/Device/DeviceDetails/DeviceSystemdUnits.tsx
  • libs/ui-components/src/components/Device/EditDeviceWizard/deviceSpecUtils.ts
📚 Learning: 2025-03-12T09:09:44.139Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 227
File: libs/ui-components/src/components/Device/EditDeviceWizard/EditDeviceWizard.tsx:87-87
Timestamp: 2025-03-12T09:09:44.139Z
Learning: The `getUpdatePolicyValues` function in libs/ui-components/src/components/Fleet/CreateFleet/fleetSpecUtils.ts is designed to handle undefined input through its optional parameter definition and consistent use of optional chaining.

Applied to files:

  • libs/ui-components/src/components/Device/EditDeviceWizard/steps/DeviceTemplateStep.tsx
  • libs/ui-components/src/components/Fleet/CreateFleet/utils.ts
  • libs/ui-components/src/components/Device/EditDeviceWizard/EditDeviceWizard.tsx
  • libs/ui-components/src/components/Device/EditDeviceWizard/deviceSpecUtils.ts
  • libs/ui-components/src/components/Device/EditDeviceWizard/utils.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/ui-components/src/components/Device/EditDeviceWizard/steps/DeviceTemplateStep.tsx
📚 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/ui-components/src/components/Device/EditDeviceWizard/steps/DeviceTemplateStep.tsx
📚 Learning: 2025-02-11T10:24:15.684Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 208
File: libs/ui-components/src/components/Device/DevicesPage/EnrolledDevicesTable.tsx:142-150
Timestamp: 2025-02-11T10:24:15.684Z
Learning: In the FlightCtl UI, device tables (`EnrolledDevicesTable` and `DecommissionedDevicesTable`) are swapped based on the `onlyDecommissioned` state, so their respective switches can have hardcoded values (`false` and `true`).

Applied to files:

  • libs/ui-components/src/components/Device/DeviceDetails/DeviceDetailsTab.tsx
📚 Learning: 2025-03-12T09:09:44.139Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 227
File: libs/ui-components/src/components/Device/EditDeviceWizard/EditDeviceWizard.tsx:87-87
Timestamp: 2025-03-12T09:09:44.139Z
Learning: The `getUpdatePolicyValues` function in libs/ui-components/src/components/Fleet/CreateFleet/fleetSpecUtils.ts is designed to safely handle undefined values for the updatePolicy parameter.

Applied to files:

  • libs/ui-components/src/components/Fleet/CreateFleet/utils.ts
  • libs/ui-components/src/components/Device/EditDeviceWizard/EditDeviceWizard.tsx
  • libs/ui-components/src/components/Device/EditDeviceWizard/deviceSpecUtils.ts
  • libs/ui-components/src/components/Device/EditDeviceWizard/utils.ts
🧬 Code graph analysis (8)
libs/ui-components/src/components/Fleet/CreateFleet/utils.ts (1)
libs/ui-components/src/components/Device/EditDeviceWizard/deviceSpecUtils.ts (1)
  • getSystemdUnitsValues (413-420)
libs/ui-components/src/components/Device/DeviceDetails/DeviceApplications.tsx (2)
libs/ui-components/src/hooks/useTranslation.ts (1)
  • useTranslation (5-8)
libs/ui-components/src/types/deviceSpec.ts (1)
  • isImageAppProvider (88-89)
libs/ui-components/src/components/DetailsPage/Tables/SystemdUnitsTable.tsx (1)
libs/ui-components/src/hooks/useTranslation.ts (1)
  • useTranslation (5-8)
libs/ui-components/src/components/Device/EditDeviceWizard/EditDeviceWizard.tsx (1)
libs/ui-components/src/components/Device/EditDeviceWizard/deviceSpecUtils.ts (1)
  • getSystemdUnitsValues (413-420)
libs/ui-components/src/components/Device/DeviceDetails/DeviceSystemdUnits.tsx (2)
libs/ui-components/src/hooks/useTranslation.ts (1)
  • useTranslation (5-8)
libs/ui-components/src/components/DetailsPage/DetailsPageCard.tsx (1)
  • DetailsPageCardBody (8-10)
libs/ui-components/src/components/Device/EditDeviceWizard/deviceSpecUtils.ts (2)
libs/types/models/DeviceSpec.ts (1)
  • DeviceSpec (15-44)
libs/ui-components/src/types/deviceSpec.ts (1)
  • SystemdUnitFormValue (173-176)
libs/ui-components/src/components/Device/EditDeviceWizard/utils.ts (1)
libs/ui-components/src/utils/patch.ts (1)
  • getStringListPatches (63-94)
libs/ui-components/src/components/DetailsPage/Tables/ApplicationsTable.tsx (2)
libs/types/models/DeviceApplicationStatus.ts (1)
  • DeviceApplicationStatus (7-25)
libs/ui-components/src/hooks/useTranslation.ts (1)
  • useTranslation (5-8)
⏰ 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). (3)
  • GitHub Check: Build
  • GitHub Check: Lint
  • GitHub Check: Build ocp plugin
🔇 Additional comments (9)
libs/ui-components/src/components/Device/EditDeviceWizard/utils.ts (1)

92-99: LGTM! Systemd unit patching logic is well-implemented.

The use of getStringListPatches correctly handles add/remove/replace operations for systemd matchPatterns, and the value builder properly wraps the patterns in the API-required { matchPatterns: list } structure. This follows the same pattern used for other spec patches in the codebase.

libs/ui-components/src/components/Device/EditDeviceWizard/steps/DeviceTemplateStep.tsx (1)

148-148: LGTM! SystemdUnitsForm now available for individual devices.

Rendering SystemdUnitsForm unconditionally (removing the isFleet gate) correctly implements the PR objective of allowing systemd services configuration for individual devices, not just fleets. The component respects the isReadOnly prop to control editability.

libs/ui-components/src/components/Device/DeviceDetails/DeviceDetailsTab.tsx (1)

161-165: LGTM! Clean separation of applications and systemd units.

The changes correctly separate systemd units into a dedicated DeviceSystemdUnits component displayed alongside DeviceApplications. The simplified prop surface for DeviceApplications (now only accepting device) aligns with the component's updated API as noted in the PR context.

libs/ui-components/src/components/Device/EditDeviceWizard/EditDeviceWizard.tsx (1)

91-91: LGTM! Proper initialization of systemd units from device spec.

Using getSystemdUnitsValues(device.spec) correctly populates the initial systemd units for the edit form, replacing the previous empty array default. This ensures existing systemd configurations are properly loaded for editing.

libs/ui-components/src/components/Device/DeviceDetails/DeviceSystemdUnits.tsx (1)

1-30: LGTM! Well-structured component for displaying systemd units.

The component follows existing patterns from similar detail page cards, properly handles undefined systemd status with a fallback empty array, and correctly passes the data to SystemdUnitsTable for rendering.

libs/ui-components/src/components/Device/EditDeviceWizard/deviceSpecUtils.ts (1)

413-420: LGTM! Clean helper function for systemd units initialization.

The implementation properly handles optional chaining through the device spec hierarchy and provides a sensible empty array default. This centralizes the logic for deriving systemd unit form values from device specs.

libs/ui-components/src/components/Fleet/CreateFleet/utils.ts (1)

240-240: LGTM! Good refactoring to eliminate code duplication.

Replacing manual mapping with getSystemdUnitsValues centralizes the systemd units initialization logic, making the Fleet and Device edit flows consistent while reducing code duplication.

libs/ui-components/src/components/Device/DeviceDetails/DeviceApplications.tsx (1)

1-39: LGTM! Clean separation of concerns.

The refactoring successfully removes systemd tracking functionality from the applications component, simplifying its API and focusing it solely on application display. The component now cleanly delegates to ApplicationsTable with only the relevant props.

libs/ui-components/src/components/DetailsPage/Tables/ApplicationsTable.tsx (1)

30-64: LGTM! Clean refactoring that focuses on applications only.

The table rendering successfully removes systemd-related functionality while maintaining clear application display logic. The fallback handling for apps without status is appropriate.

Comment thread libs/i18n/locales/en/translation.json Outdated
Comment thread libs/i18n/locales/en/translation.json Outdated
Comment thread libs/ui-components/src/components/DetailsPage/Tables/SystemdUnitsTable.tsx Outdated
Comment thread libs/ui-components/src/components/DetailsPage/Tables/SystemdUnitsTable.tsx Outdated
Comment thread libs/ui-components/src/components/DetailsPage/Tables/SystemdUnitsTable.tsx Outdated
@celdrake celdrake force-pushed the EDM-2422-split-application-and-systemd-status branch from 32e80e0 to 87250b2 Compare November 13, 2025 12:32
@celdrake celdrake force-pushed the EDM-2422-split-application-and-systemd-status branch from 87250b2 to 6de2b1a Compare November 13, 2025 12:35

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
libs/ui-components/src/components/DetailsPage/Tables/ApplicationsTable.tsx (1)

20-28: Consider using a Set for better performance.

The current approach using includes within a loop has O(n²) complexity. For typical application counts this is fine, but could be optimized.

-  const allAppNames: string[] = [];
-  specApps.forEach((app) => {
-    allAppNames.push(app);
-  });
-  appsStatus.forEach((appStatus) => {
-    if (!allAppNames.includes(appStatus.name)) {
-      allAppNames.push(appStatus.name);
-    }
-  });
+  const allAppNames = Array.from(new Set([...specApps, ...appsStatus.map((app) => app.name)]));
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 87250b2 and 6de2b1a.

📒 Files selected for processing (12)
  • libs/i18n/locales/en/translation.json (3 hunks)
  • libs/ui-components/src/components/DetailsPage/Tables/ApplicationsTable.css (0 hunks)
  • libs/ui-components/src/components/DetailsPage/Tables/ApplicationsTable.tsx (1 hunks)
  • libs/ui-components/src/components/DetailsPage/Tables/SystemdUnitsTable.tsx (1 hunks)
  • libs/ui-components/src/components/Device/DeviceDetails/DeviceApplications.tsx (1 hunks)
  • libs/ui-components/src/components/Device/DeviceDetails/DeviceDetailsTab.tsx (3 hunks)
  • libs/ui-components/src/components/Device/DeviceDetails/DeviceSystemdUnits.tsx (1 hunks)
  • libs/ui-components/src/components/Device/EditDeviceWizard/EditDeviceWizard.tsx (2 hunks)
  • libs/ui-components/src/components/Device/EditDeviceWizard/deviceSpecUtils.ts (2 hunks)
  • libs/ui-components/src/components/Device/EditDeviceWizard/steps/DeviceTemplateStep.tsx (1 hunks)
  • libs/ui-components/src/components/Device/EditDeviceWizard/utils.ts (2 hunks)
  • libs/ui-components/src/components/Fleet/CreateFleet/utils.ts (2 hunks)
💤 Files with no reviewable changes (1)
  • libs/ui-components/src/components/DetailsPage/Tables/ApplicationsTable.css
🚧 Files skipped from review as they are similar to previous changes (6)
  • libs/ui-components/src/components/Fleet/CreateFleet/utils.ts
  • libs/ui-components/src/components/Device/EditDeviceWizard/steps/DeviceTemplateStep.tsx
  • libs/ui-components/src/components/Device/DeviceDetails/DeviceDetailsTab.tsx
  • libs/ui-components/src/components/DetailsPage/Tables/SystemdUnitsTable.tsx
  • libs/ui-components/src/components/Device/DeviceDetails/DeviceSystemdUnits.tsx
  • libs/ui-components/src/components/Device/EditDeviceWizard/deviceSpecUtils.ts
🧰 Additional context used
🧠 Learnings (4)
📓 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: 227
File: libs/ui-components/src/components/Device/EditDeviceWizard/steps/ReviewTrackedSystemdServices.tsx:3-3
Timestamp: 2025-03-12T08:41:34.720Z
Learning: In the flightctl-ui codebase, SystemdUnitsFormValues is defined as { systemdUnits: SystemdUnitFormValue[] }, allowing for direct destructuring of the systemdUnits property in component props.
📚 Learning: 2025-03-12T08:41:34.720Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 227
File: libs/ui-components/src/components/Device/EditDeviceWizard/steps/ReviewTrackedSystemdServices.tsx:3-3
Timestamp: 2025-03-12T08:41:34.720Z
Learning: In the flightctl-ui codebase, SystemdUnitsFormValues is defined as { systemdUnits: SystemdUnitFormValue[] }, allowing for direct destructuring of the systemdUnits property in component props.

Applied to files:

  • libs/ui-components/src/components/Device/EditDeviceWizard/EditDeviceWizard.tsx
📚 Learning: 2025-03-12T09:09:44.139Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 227
File: libs/ui-components/src/components/Device/EditDeviceWizard/EditDeviceWizard.tsx:87-87
Timestamp: 2025-03-12T09:09:44.139Z
Learning: The `getUpdatePolicyValues` function in libs/ui-components/src/components/Fleet/CreateFleet/fleetSpecUtils.ts is designed to safely handle undefined values for the updatePolicy parameter.

Applied to files:

  • libs/ui-components/src/components/Device/EditDeviceWizard/EditDeviceWizard.tsx
  • libs/ui-components/src/components/Device/EditDeviceWizard/utils.ts
📚 Learning: 2025-03-12T09:09:44.139Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 227
File: libs/ui-components/src/components/Device/EditDeviceWizard/EditDeviceWizard.tsx:87-87
Timestamp: 2025-03-12T09:09:44.139Z
Learning: The `getUpdatePolicyValues` function in libs/ui-components/src/components/Fleet/CreateFleet/fleetSpecUtils.ts is designed to handle undefined input through its optional parameter definition and consistent use of optional chaining.

Applied to files:

  • libs/ui-components/src/components/Device/EditDeviceWizard/EditDeviceWizard.tsx
  • libs/ui-components/src/components/Device/EditDeviceWizard/utils.ts
🧬 Code graph analysis (4)
libs/ui-components/src/components/DetailsPage/Tables/ApplicationsTable.tsx (2)
libs/types/models/DeviceApplicationStatus.ts (1)
  • DeviceApplicationStatus (7-25)
libs/ui-components/src/hooks/useTranslation.ts (1)
  • useTranslation (5-8)
libs/ui-components/src/components/Device/EditDeviceWizard/EditDeviceWizard.tsx (1)
libs/ui-components/src/components/Device/EditDeviceWizard/deviceSpecUtils.ts (1)
  • getSystemdUnitsValues (413-420)
libs/ui-components/src/components/Device/EditDeviceWizard/utils.ts (1)
libs/ui-components/src/utils/patch.ts (1)
  • getStringListPatches (63-94)
libs/ui-components/src/components/Device/DeviceDetails/DeviceApplications.tsx (3)
libs/ui-components/src/hooks/useTranslation.ts (1)
  • useTranslation (5-8)
libs/ui-components/src/types/deviceSpec.ts (1)
  • isImageAppProvider (88-89)
libs/ui-components/src/components/DetailsPage/DetailsPageCard.tsx (1)
  • DetailsPageCardBody (8-10)
⏰ 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). (1)
  • GitHub Check: integration-tests
🔇 Additional comments (11)
libs/i18n/locales/en/translation.json (3)

128-134: Translation keys are correct and well-named, with previous typos fixed.

The new systemd-related translations (Enable state, Load state, Active state, Sub state) properly represent standard systemd unit status fields. The "Active satte" and "No system sevices" typos flagged in prior reviews have been corrected. Keys are semantically appropriate and align with the PR objective to separate systemd service management from applications.


183-183: Systemd services section label is consistent with new UI components.

The reintroduction of "System services" at line 183 aligns with the PR changes, providing a section label for systemd service management in device specs and fleet configurations.


324-324: Track systemd services option retained for backward compatibility.

The "Track systemd services" key is preserved, consistent with the PR's goal of allowing systemd service definition at the device level while maintaining the option label for relevant UI flows.

libs/ui-components/src/components/DetailsPage/Tables/ApplicationsTable.tsx (2)

9-16: LGTM! Props simplified correctly.

The component signature has been properly simplified to handle only application-related data, with systemd functionality now managed separately.


41-58: LGTM! Rendering logic is clean and handles edge cases well.

The component correctly handles applications that are in the spec but not yet reported in status by providing sensible defaults.

libs/ui-components/src/components/Device/EditDeviceWizard/EditDeviceWizard.tsx (2)

31-36: LGTM! Import addition is correct.

The new helper function is properly imported alongside the existing device spec utilities.


91-91: LGTM! Systemd units properly initialized from device spec.

The change correctly populates initial systemd units using the new helper function, maintaining consistency with how other spec fields are initialized.

libs/ui-components/src/components/Device/EditDeviceWizard/utils.ts (2)

12-17: LGTM! Import addition is appropriate.

The getStringListPatches utility is correctly imported for generating systemd unit patches.


92-99: LGTM! Systemd patches correctly integrated.

The patch generation logic follows the established pattern used for applications and properly handles add/remove/replace operations for systemd match patterns.

libs/ui-components/src/components/Device/DeviceDetails/DeviceApplications.tsx (2)

1-12: LGTM! Component signature properly simplified.

The component now has a clear, focused responsibility handling only application display, with systemd functionality appropriately separated.


14-37: LGTM! Rendering logic is clean and focused.

The component correctly passes only application-related data to the ApplicationsTable, maintaining proper separation from systemd functionality.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
libs/ui-components/src/components/DetailsPage/Tables/SystemdUnitsTable.tsx (1)

31-31: Remove redundant key prop.

The key prop on line 31 is redundant because the key is already set on the SystemdUnitRow component when it's rendered in the map at line 71. React only needs the key on the component returned from the map, not on elements inside the component.

Apply this diff:

-    <Tr key={unitStatus.unit}>
+    <Tr>
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6de2b1a and ad2c388.

📒 Files selected for processing (2)
  • libs/i18n/locales/en/translation.json (3 hunks)
  • libs/ui-components/src/components/DetailsPage/Tables/SystemdUnitsTable.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • libs/i18n/locales/en/translation.json
🧰 Additional context used
🧠 Learnings (3)
📓 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: 227
File: libs/ui-components/src/components/Device/EditDeviceWizard/steps/ReviewTrackedSystemdServices.tsx:3-3
Timestamp: 2025-03-12T08:41:34.720Z
Learning: In the flightctl-ui codebase, SystemdUnitsFormValues is defined as { systemdUnits: SystemdUnitFormValue[] }, allowing for direct destructuring of the systemdUnits property in component props.
📚 Learning: 2025-03-12T08:41:34.720Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 227
File: libs/ui-components/src/components/Device/EditDeviceWizard/steps/ReviewTrackedSystemdServices.tsx:3-3
Timestamp: 2025-03-12T08:41:34.720Z
Learning: In the flightctl-ui codebase, SystemdUnitsFormValues is defined as { systemdUnits: SystemdUnitFormValue[] }, allowing for direct destructuring of the systemdUnits property in component props.

Applied to files:

  • libs/ui-components/src/components/DetailsPage/Tables/SystemdUnitsTable.tsx
📚 Learning: 2025-02-11T10:24:15.684Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 208
File: libs/ui-components/src/components/Device/DevicesPage/EnrolledDevicesTable.tsx:142-150
Timestamp: 2025-02-11T10:24:15.684Z
Learning: In the FlightCtl UI, device tables (`EnrolledDevicesTable` and `DecommissionedDevicesTable`) are swapped based on the `onlyDecommissioned` state, so their respective switches can have hardcoded values (`false` and `true`).

Applied to files:

  • libs/ui-components/src/components/DetailsPage/Tables/SystemdUnitsTable.tsx
🧬 Code graph analysis (1)
libs/ui-components/src/components/DetailsPage/Tables/SystemdUnitsTable.tsx (1)
libs/ui-components/src/hooks/useTranslation.ts (1)
  • useTranslation (5-8)

@celdrake celdrake force-pushed the EDM-2422-split-application-and-systemd-status branch from ad2c388 to 3f90af4 Compare November 17, 2025 15:44
@celdrake celdrake merged commit 4fa79ec into flightctl:main Nov 18, 2025
6 checks passed
@celdrake celdrake deleted the EDM-2422-split-application-and-systemd-status branch November 18, 2025 10:06
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