Addition of non-TrueNAS Managed App visualization & stats

[alexandrabain]

Changes:
[Note dependent on https://github.com/truenas/middleware/pull/17514]

1. Section to display Non-TrueNAS deployed app i.e., apps deployed through tools such as Dockge or through Docker CLI

• Includes display only, no lifecycle management of these Apps
• Does Include Resources

2. Added header row for the TrueNAS managed Apps
3. Added collapse ability on the section headers
4. Added overall Apps Utilization Row.

Testing:

Tested new and upgraded deployments, with a variety of apps in both categories.
Tested existing controls for TrueNAS Managed apps remain functional
Tested sorting of application names worked in each group
Tested filtering works across each group.

Downstream

Affects	Reasoning
Documentation	Revised UI with a new section for Other Apps and with a collapsible section
Testing	Need to expand to include having apps deployed means other than just the TrueNAS Managed option to test the table - recommend Auto tests.

[claude]

Claude finished @alexandrabain's task —— View job

---

Code Review Complete! 🎉

Great work on this feature addition! The implementation is solid overall with good test coverage and modern Angular patterns. Here's my feedback:

---

✅ Strengths

The PR demonstrates strong architectural decisions with signal-based reactive patterns, comprehensive test coverage (89.22%), and well-structured utility functions that handle edge cases properly.

---

🔴 Critical Issue: Memory Leak

src/app/pages/apps/components/installed-apps/installed-apps-list/installed-apps-list.component.ts:617-619

There's a memory leak in the totalUtilization$ observable. The operator order is incorrect:

readonly totalUtilization$ = this.installedAppsStore.installedApps$.pipe(
  // ... transformations ...
  takeUntilDestroyed(this.destroyRef),
  shareReplay({ bufferSize: 1, refCount: true }),  // ❌ WRONG ORDER
);

Problem: When takeUntilDestroyed comes BEFORE shareReplay, template subscribers (| async) subscribe downstream of takeUntilDestroyed, so it cannot clean up the upstream sources. The combineLatest inside continues polling even after component destruction.

Solution: Place takeUntilDestroyed BEFORE shareReplay:

readonly totalUtilization$ = this.installedAppsStore.installedApps$.pipe(
  // ... transformations ...
  takeUntilDestroyed(this.destroyRef),           // ✅ First
  shareReplay({ bufferSize: 1, refCount: true }), // ✅ Second
);

Why this matters: When takeUntilDestroyed is BEFORE shareReplay, it properly completes the source stream on component destruction, stopping all upstream polling operations.

Note: I can see from your commit history (141c2d1) that you've already identified and attempted to fix this - make sure the correct order is in the final version! 🔍

---

⚠️ Other Issues

1. Missing null check in app-info-card effect

src/app/pages/apps/components/installed-apps/app-info-card/app-info-card.component.ts:95-99

The effect doesn't check if app() exists before calling updateRollbackSetup:

protected rollbackUpdateButtonSetEffect = effect(() => {
  const app = this.app();
  this.isRollbackPossible.set(false);
  this.updateRollbackSetup(app.name);  // ⚠️ What if app is undefined?
});

Suggestion: Add a null check or use allowSignalWrites: true if intentional:

protected rollbackUpdateButtonSetEffect = effect(() => {
  const app = this.app();
  if (!app) return;
  this.isRollbackPossible.set(false);
  this.updateRollbackSetup(app.name);
});

2. Potential navigation issue with external apps

src/app/pages/apps/components/installed-apps/installed-apps-list/installed-apps-list.component.ts:523-533

The selectFirstApp() method assumes apps have metadata.train and id, but external apps might have different metadata structures:

private selectFirstApp(): void {
  const [firstApp] = this.sortedApps();
  if (firstApp.metadata.train && firstApp.id) {  // ⚠️ Safe for external apps?
    this.location.replaceState(['/apps', 'installed', firstApp.metadata.train, firstApp.id].join('/'));
  }
  // ...
}

Suggestion: Add explicit check for external apps or ensure external apps always have these properties.

3. Consider debouncing the selection change signal

src/app/pages/apps/components/installed-apps/installed-apps-list/installed-apps-list.component.ts:270-276

Every checkbox toggle triggers a signal update, which recomputes multiple computed signals. For bulk operations, this could cause performance issues:

toggleAppSelection(appId: string): void {
  this.selection.toggle(appId);
  this.selectionChanged.update((count) => count + 1); // Triggers all computed signals
}

Suggestion: Consider batching updates or using untracked() for bulk operations.

---

💡 Minor Suggestions

1. Accessibility enhancement: The collapsible headers (installed-apps-list.component.html:131-146) have good ARIA attributes, but consider adding role="region" to the content sections for better screen reader navigation.

2. i18n completeness: The new strings like "Apps managed by TrueNAS" and "External Apps" will need translation keys added. Make sure they're in the i18n files.

3. Console.error in production: At line 605 of installed-apps-list.component.ts, there's a console.error in the catchError. Consider using a proper logging service instead for production environments.

---

📊 Test Coverage

Excellent test coverage! The new tests comprehensively cover:

• External app filtering and separation ✅
• Collapsible sections ✅
• Total utilization calculations with edge cases ✅
• Null/undefined handling ✅
• Network stats aggregation ✅

---

🎯 Summary

The implementation is well-architected with proper separation of TrueNAS and external apps. Fix the memory leak operator order (critical), add the null check in the effect (important), and verify external app metadata handling. The rest is solid! 🚀

---

[codecov]

Codecov Report

❌ Patch coverage is 89.22156% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.38%. Comparing base (b2f31fe) to head (141c2d1).
⚠️ Report is 5 commits behind head on master.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...stalled-apps-list/installed-apps-list.component.ts	88.28%	13 Missing ⚠️
...onents/installed-apps/app-row/app-row.component.ts	81.81%	2 Missing ⚠️
...ponents/installed-apps/installed-apps.component.ts	83.33%	2 Missing ⚠️
src/app/pages/apps/utils/network-stats.utils.ts	95.23%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #12775      +/-   ##
==========================================
+ Coverage   86.35%   86.38%   +0.02%     
==========================================
  Files        1832     1834       +2     
  Lines       68493    68574      +81     
  Branches     8392     8419      +27     
==========================================
+ Hits        59149    59236      +87     
+ Misses       9344     9338       -6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.